Skip to content

Commit

Permalink
Support for encrypted passwords (#1771)
Browse files Browse the repository at this point in the history
## Problem

- The Agama autoinstallation and CLI accept the first user and the root
passwords only in plain text
- That's insecure, everybody who can access the installation profile
knows the root password

## Solution

- Support passing an already encrypted (hashed) password in the profile
- Similar to AutoYaST, an additional `encryptedPassword` boolean flag is
used to determine whether the specified password is encrypted (`true`
value) or plain text (`false` value or missing in the profile)

## Notes

- The web UI allows specifying only plain text passwords
- Encrypted passwords are long and hard to type and they need to be
encrypted externally

## Features

- Adapted schema definition
- Adapted the AutoYaST conversion tool
- When an encrypted password is set from Agama CLI then web UI resets
the flag back to plain text (it supports only plain text passwords)

## Testing

- Tested manually (both root user and first user), tested the AutoYaST
profile conversion
- Updated unit tests

---------

Co-authored-by: Imobach González Sosa <[email protected]>
  • Loading branch information
lslezak and imobachgs authored Nov 25, 2024
1 parent 9297d68 commit c738625
Show file tree
Hide file tree
Showing 22 changed files with 113 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci-doc-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
run: zypper modifyrepo -d repo-non-oss repo-openh264 repo-update && zypper ref

- name: Install Ruby development files and XML tooling
run: zypper --non-interactive install --no-recommends
run: zypper --non-interactive install --no-recommends --allow-downgrade
gcc gcc-c++ make libopenssl-devel ruby-devel augeas-devel diff
libxslt-devel libxml2-devel xmlstarlet 'rubygem(ruby-augeas)'

Expand Down
12 changes: 10 additions & 2 deletions rust/agama-lib/share/profile.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -391,9 +391,13 @@
"examples": ["jane.doe"]
},
"password": {
"title": "User password",
"title": "User password (plain text or encrypted depending on the \"passwordEncrypted\" field)",
"type": "string",
"examples": ["nots3cr3t"]
},
"passwordEncrypted": {
"title": "Flag for encrypted password (true) or plain text password (false or not defined)",
"type": "boolean"
}
},
"required": ["fullName", "userName", "password"]
Expand All @@ -404,9 +408,13 @@
"additionalProperties": false,
"properties": {
"password": {
"title": "Root password",
"title": "Root password (plain text or encrypted depending on the \"passwordEncrypted\" field)",
"type": "string"
},
"passwordEncrypted": {
"title": "Flag for encrypted password (true) or plain text password (false or not defined)",
"type": "boolean"
},
"sshPublicKey": {
"title": "SSH public key",
"type": "string"
Expand Down
6 changes: 5 additions & 1 deletion rust/agama-lib/src/users/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub struct FirstUser {
pub user_name: String,
/// First user's password (in clear text)
pub password: String,
/// Whether the password is encrypted (true) or is plain text (false)
pub encrypted_password: bool,
/// Whether auto-login should enabled or not
pub autologin: bool,
}
Expand All @@ -46,7 +48,8 @@ impl FirstUser {
full_name: data.0,
user_name: data.1,
password: data.2,
autologin: data.3,
encrypted_password: data.3,
autologin: data.4,
})
}
}
Expand Down Expand Up @@ -107,6 +110,7 @@ impl<'a> UsersClient<'a> {
&first_user.full_name,
&first_user.user_name,
&first_user.password,
first_user.encrypted_password,
first_user.autologin,
std::collections::HashMap::new(),
)
Expand Down
3 changes: 3 additions & 0 deletions rust/agama-lib/src/users/proxies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use zbus::proxy;
/// * full name
/// * user name
/// * password
/// * encrypted_password (true = encrypted, false = plain text)
/// * auto-login (enabled or not)
/// * some optional and additional data
// NOTE: Manually added to this file.
Expand All @@ -55,6 +56,7 @@ pub type FirstUser = (
String,
String,
bool,
bool,
std::collections::HashMap<String, zbus::zvariant::OwnedValue>,
);

Expand All @@ -77,6 +79,7 @@ pub trait Users1 {
full_name: &str,
user_name: &str,
password: &str,
encrypted_password: bool,
auto_login: bool,
data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>,
) -> zbus::Result<(bool, Vec<String>)>;
Expand Down
5 changes: 5 additions & 0 deletions rust/agama-lib/src/users/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub struct FirstUserSettings {
pub user_name: Option<String>,
/// First user's password (in clear text)
pub password: Option<String>,
/// Whether the password is encrypted or is plain text
pub encrypted_password: Option<bool>,
/// Whether auto-login should enabled or not
pub autologin: Option<bool>,
}
Expand All @@ -56,6 +58,9 @@ pub struct RootUserSettings {
/// Root's password (in clear text)
#[serde(skip_serializing)]
pub password: Option<String>,
/// Whether the password is encrypted or is plain text
#[serde(skip_serializing)]
pub encrypted_password: Option<bool>,
/// Root SSH public key
pub ssh_public_key: Option<String>,
}
13 changes: 11 additions & 2 deletions rust/agama-lib/src/users/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ impl UsersStore {
autologin: Some(first_user.autologin),
full_name: Some(first_user.full_name),
password: Some(first_user.password),
encrypted_password: Some(first_user.encrypted_password),
};
let mut root_user = RootUserSettings::default();
let ssh_public_key = self.users_client.root_ssh_key().await?;
Expand Down Expand Up @@ -77,16 +78,19 @@ impl UsersStore {
full_name: settings.full_name.clone().unwrap_or_default(),
autologin: settings.autologin.unwrap_or_default(),
password: settings.password.clone().unwrap_or_default(),
encrypted_password: settings.encrypted_password.clone().unwrap_or_default(),
..Default::default()
};
self.users_client.set_first_user(&first_user).await?;
Ok(())
}

async fn store_root_user(&self, settings: &RootUserSettings) -> Result<(), ServiceError> {
let encrypted_password = settings.encrypted_password.clone().unwrap_or_default();

if let Some(root_password) = &settings.password {
self.users_client
.set_root_password(root_password, false)
.set_root_password(root_password, encrypted_password)
.await?;
}

Expand Down Expand Up @@ -126,6 +130,7 @@ mod test {
"fullName": "Tux",
"userName": "tux",
"password": "fish",
"encryptedPassword": false,
"autologin": true
}"#,
);
Expand All @@ -150,11 +155,13 @@ mod test {
full_name: Some("Tux".to_owned()),
user_name: Some("tux".to_owned()),
password: Some("fish".to_owned()),
encrypted_password: Some(false),
autologin: Some(true),
};
let root_user = RootUserSettings {
// FIXME this is weird: no matter what HTTP reports, we end up with None
password: None,
encrypted_password: None,
ssh_public_key: Some("keykeykey".to_owned()),
};
let expected = UserSettings {
Expand All @@ -179,7 +186,7 @@ mod test {
when.method(PUT)
.path("/api/users/first")
.header("content-type", "application/json")
.body(r#"{"fullName":"Tux","userName":"tux","password":"fish","autologin":true}"#);
.body(r#"{"fullName":"Tux","userName":"tux","password":"fish","encryptedPassword":false,"autologin":true}"#);
then.status(200);
});
// note that we use 2 requests for root
Expand All @@ -205,10 +212,12 @@ mod test {
full_name: Some("Tux".to_owned()),
user_name: Some("tux".to_owned()),
password: Some("fish".to_owned()),
encrypted_password: Some(false),
autologin: Some(true),
};
let root_user = RootUserSettings {
password: Some("1234".to_owned()),
encrypted_password: Some(false),
ssh_public_key: Some("keykeykey".to_owned()),
};
let settings = UserSettings {
Expand Down
3 changes: 2 additions & 1 deletion rust/agama-server/src/users/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ async fn first_user_changed_stream(
full_name: user.0,
user_name: user.1,
password: user.2,
autologin: user.3,
encrypted_password: user.3,
autologin: user.4,
};
return Some(Event::FirstUserChanged(user_struct));
}
Expand Down
6 changes: 6 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Fri Nov 15 16:48:44 UTC 2024 - Ladislav Slezák <[email protected]>

- Allow using encrypted passord for root and the first user
(gh#agama-project/agama#1771)

-------------------------------------------------------------------
Thu Nov 14 14:45:47 UTC 2024 - Knut Alejandro Anderssen González <[email protected]>

Expand Down
3 changes: 3 additions & 0 deletions service/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ Lint/UselessAssignment:
# be less strict
Metrics/AbcSize:
Max: 32

Metrics/ParameterLists:
Max: 6
2 changes: 2 additions & 0 deletions service/lib/agama/autoyast/root_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ def read
return {} unless root_user

hsh = { "password" => root_user.password.value.to_s }
hsh["passwordEncrypted"] = true if root_user.password.value.encrypted?

public_key = root_user.authorized_keys.first
hsh["sshPublicKey"] = public_key if public_key
{ "root" => hsh }
Expand Down
3 changes: 3 additions & 0 deletions service/lib/agama/autoyast/user_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ def read
"fullName" => user.gecos.first.to_s,
"password" => user.password.value.to_s
}

hsh["passwordEncrypted"] = true if user.password.value.encrypted?

{ "user" => hsh }
end

Expand Down
14 changes: 9 additions & 5 deletions service/lib/agama/dbus/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ def issues
USERS_INTERFACE = "org.opensuse.Agama.Users1"
private_constant :USERS_INTERFACE

FUSER_SIG = "in FullName:s, in UserName:s, in Password:s, in AutoLogin:b, in data:a{sv}"
FUSER_SIG = "in FullName:s, in UserName:s, in Password:s, in EncryptedPassword:b, " \
"in AutoLogin:b, in data:a{sv}"
private_constant :FUSER_SIG

dbus_interface USERS_INTERFACE do
dbus_reader :root_password_set, "b"

dbus_reader :root_ssh_key, "s", dbus_name: "RootSSHKey"

dbus_reader :first_user, "(sssba{sv})"
dbus_reader :first_user, "(sssbba{sv})"

dbus_method :SetRootPassword,
"in Value:s, in Encrypted:b, out result:u" do |value, encrypted|
Expand Down Expand Up @@ -97,9 +98,11 @@ def issues
dbus_method :SetFirstUser,
# It returns an Struct with the first field with the result of the operation as a boolean
# and the second parameter as an array of issues found in case of failure
FUSER_SIG + ", out result:(bas)" do |full_name, user_name, password, auto_login, data|
FUSER_SIG + ", out result:(bas)" do
|full_name, user_name, password, encrypted_password, auto_login, data|
logger.info "Setting first user #{full_name}"
user_issues = backend.assign_first_user(full_name, user_name, password, auto_login, data)
user_issues = backend.assign_first_user(full_name, user_name, password,
encrypted_password, auto_login, data)

if user_issues.empty?
dbus_properties_changed(USERS_INTERFACE, { "FirstUser" => first_user }, [])
Expand Down Expand Up @@ -133,12 +136,13 @@ def root_ssh_key
def first_user
user = backend.first_user

return ["", "", "", false, {}] unless user
return ["", "", "", false, false, {}] unless user

[
user.full_name,
user.name,
user.password_content || "",
user.password&.value&.encrypted? || false,
backend.autologin?(user),
{}
]
Expand Down
10 changes: 8 additions & 2 deletions service/lib/agama/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,21 @@ def remove_root_password
# @param full_name [String]
# @param user_name [String]
# @param password [String]
# @param encrypted_password [Boolean] true = encrypted password, false = plain text password
# @param auto_login [Boolean]
# @param _data [Hash]
# @return [Array] the list of fatal issues found
def assign_first_user(full_name, user_name, password, auto_login, _data)
def assign_first_user(full_name, user_name, password, encrypted_password, auto_login, _data)
remove_first_user

user = Y2Users::User.new(user_name)
user.gecos = [full_name]
user.password = Y2Users::Password.create_plain(password)
user.password = if encrypted_password
Y2Users::Password.create_encrypted(password)
else
Y2Users::Password.create_plain(password)
end

fatal_issues = user.issues.map.select(&:error?)
return fatal_issues.map(&:message) unless fatal_issues.empty?

Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Fri Nov 15 16:48:44 UTC 2024 - Ladislav Slezák <[email protected]>

- Allow using encrypted password for root and the first user
(gh#agama-project/agama#1771)

-------------------------------------------------------------------
Thu Nov 14 15:34:17 UTC 2024 - Ancor Gonzalez Sosa <[email protected]>

Expand Down
9 changes: 6 additions & 3 deletions service/test/agama/dbus/users_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
require "agama/dbus/interfaces/service_status"
require "agama/dbus/users"
require "agama/users"
require "y2users"

describe Agama::DBus::Users do
subject { described_class.new(backend, logger) }
Expand Down Expand Up @@ -69,24 +70,26 @@
let(:user) { nil }

it "returns default data" do
expect(subject.first_user).to eq(["", "", "", false, {}])
expect(subject.first_user).to eq(["", "", "", false, false, {}])
end
end

context "if there is an user" do
let(:password) { Y2Users::Password.create_encrypted("12345") }
let(:user) do
instance_double(Y2Users::User,
full_name: "Test user",
name: "test",
password_content: "12345")
password: password,
password_content: password.value.to_s)
end

before do
allow(backend).to receive(:autologin?).with(user).and_return(true)
end

it "returns the first user data" do
expect(subject.first_user).to eq(["Test user", "test", "12345", true, {}])
expect(subject.first_user).to eq(["Test user", "test", password.value.to_s, true, true, {}])
end
end
end
Expand Down
Loading

0 comments on commit c738625

Please sign in to comment.