Skip to content

Commit

Permalink
Rename "encrypted password" to "hashed password" (#1833)
Browse files Browse the repository at this point in the history
- As discussed on the review meeting, technically it is not an
encryption (because the decryption part is missing) but hashing. That's
the correct term.

## Testing

- Updated unit tests
- Tested manually

```console
# agama config show | jq .user
{
  "fullName": "Agama",
  "userName": "agama",
  "password": "$5$t3gRfqthxk7ZsJw.$gpduJASf12Xk8aPtIllniJAwhbhlYvLcEUH0.4P9MWA",
  "hashedPassword": true,
  "autologin": false
}
```
  • Loading branch information
lslezak authored Dec 13, 2024
2 parents cae658e + c3f4f05 commit 1fcd19d
Show file tree
Hide file tree
Showing 22 changed files with 73 additions and 81 deletions.
2 changes: 1 addition & 1 deletion live/root/etc/systemd/system/live-password-cmdline.service
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Before=agama-web-server.service
Before=live-password-dialog.service
Before=live-password-systemd.service

# plain text password or encrypted password passed via kernel command line
# plain text password or hashed password passed via kernel command line
ConditionKernelCommandLine=|live.password
ConditionKernelCommandLine=|live.password_hash

Expand Down
12 changes: 6 additions & 6 deletions rust/agama-lib/share/profile.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -382,12 +382,12 @@
"examples": ["jane.doe"]
},
"password": {
"title": "User password (plain text or encrypted depending on the \"encryptedPassword\" field)",
"title": "User password (plain text or hashed depending on the \"hashedPassword\" field)",
"type": "string",
"examples": ["nots3cr3t"]
},
"encryptedPassword": {
"title": "Flag for encrypted password (true) or plain text password (false or not defined)",
"hashedPassword": {
"title": "Flag for hashed password (true) or plain text password (false or not defined)",
"type": "boolean"
}
},
Expand All @@ -399,11 +399,11 @@
"additionalProperties": false,
"properties": {
"password": {
"title": "Root password (plain text or encrypted depending on the \"encryptedPassword\" field)",
"title": "Root password (plain text or hashed depending on the \"hashedPassword\" field)",
"type": "string"
},
"encryptedPassword": {
"title": "Flag for encrypted password (true) or plain text password (false or not defined)",
"hashedPassword": {
"title": "Flag for hashed password (true) or plain text password (false or not defined)",
"type": "boolean"
},
"sshPublicKey": {
Expand Down
16 changes: 6 additions & 10 deletions rust/agama-lib/src/users/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +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 the password is hashed (true) or is plain text (false)
pub hashed_password: bool,
/// Whether auto-login should enabled or not
pub autologin: bool,
}
Expand All @@ -48,7 +48,7 @@ impl FirstUser {
full_name: data.0,
user_name: data.1,
password: data.2,
encrypted_password: data.3,
hashed_password: data.3,
autologin: data.4,
})
}
Expand All @@ -73,12 +73,8 @@ impl<'a> UsersClient<'a> {
}

/// SetRootPassword method
pub async fn set_root_password(
&self,
value: &str,
encrypted: bool,
) -> Result<u32, ServiceError> {
Ok(self.users_proxy.set_root_password(value, encrypted).await?)
pub async fn set_root_password(&self, value: &str, hashed: bool) -> Result<u32, ServiceError> {
Ok(self.users_proxy.set_root_password(value, hashed).await?)
}

pub async fn remove_root_password(&self) -> Result<u32, ServiceError> {
Expand Down Expand Up @@ -110,7 +106,7 @@ impl<'a> UsersClient<'a> {
&first_user.full_name,
&first_user.user_name,
&first_user.password,
first_user.encrypted_password,
first_user.hashed_password,
first_user.autologin,
std::collections::HashMap::new(),
)
Expand Down
10 changes: 3 additions & 7 deletions rust/agama-lib/src/users/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,11 @@ impl UsersHTTPClient {

/// SetRootPassword method.
/// Returns 0 if successful (always, for current backend)
pub async fn set_root_password(
&self,
value: &str,
encrypted: bool,
) -> Result<u32, ServiceError> {
pub async fn set_root_password(&self, value: &str, hashed: bool) -> Result<u32, ServiceError> {
let rps = RootPatchSettings {
sshkey: None,
password: Some(value.to_owned()),
encrypted_password: Some(encrypted),
hashed_password: Some(hashed),
};
let ret = self.client.patch("/users/root", &rps).await?;
Ok(ret)
Expand All @@ -84,7 +80,7 @@ impl UsersHTTPClient {
let rps = RootPatchSettings {
sshkey: Some(value.to_owned()),
password: None,
encrypted_password: None,
hashed_password: None,
};
let ret = self.client.patch("/users/root", &rps).await?;
Ok(ret)
Expand Down
4 changes: 2 additions & 2 deletions rust/agama-lib/src/users/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ pub struct RootPatchSettings {
pub sshkey: Option<String>,
/// empty string here means remove password for root
pub password: Option<String>,
/// specify if patched password is provided in encrypted form
pub encrypted_password: Option<bool>,
/// specify if patched password is provided in plain text (default) or hashed
pub hashed_password: Option<bool>,
}
6 changes: 3 additions & 3 deletions rust/agama-lib/src/users/proxies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use zbus::proxy;
/// * full name
/// * user name
/// * password
/// * encrypted_password (true = encrypted, false = plain text)
/// * hashed_password (true = hashed, false = plain text)
/// * auto-login (enabled or not)
/// * some optional and additional data
// NOTE: Manually added to this file.
Expand Down Expand Up @@ -79,13 +79,13 @@ pub trait Users1 {
full_name: &str,
user_name: &str,
password: &str,
encrypted_password: bool,
hashed_password: bool,
auto_login: bool,
data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>,
) -> zbus::Result<(bool, Vec<String>)>;

/// SetRootPassword method
fn set_root_password(&self, value: &str, encrypted: bool) -> zbus::Result<u32>;
fn set_root_password(&self, value: &str, hashed: bool) -> zbus::Result<u32>;

/// SetRootSSHKey method
#[zbus(name = "SetRootSSHKey")]
Expand Down
8 changes: 4 additions & 4 deletions rust/agama-lib/src/users/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +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 the password is hashed or is plain text
pub hashed_password: Option<bool>,
/// Whether auto-login should enabled or not
pub autologin: Option<bool>,
}
Expand All @@ -58,9 +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
/// Whether the password is hashed or is plain text
#[serde(skip_serializing)]
pub encrypted_password: Option<bool>,
pub hashed_password: Option<bool>,
/// Root SSH public key
pub ssh_public_key: Option<String>,
}
24 changes: 12 additions & 12 deletions rust/agama-lib/src/users/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +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),
hashed_password: Some(first_user.hashed_password),
};
let mut root_user = RootUserSettings::default();
let ssh_public_key = self.users_client.root_ssh_key().await?;
Expand Down Expand Up @@ -78,17 +78,17 @@ 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.unwrap_or_default(),
hashed_password: settings.hashed_password.unwrap_or_default(),
};
self.users_client.set_first_user(&first_user).await
}

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

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

Expand Down Expand Up @@ -128,7 +128,7 @@ mod test {
"fullName": "Tux",
"userName": "tux",
"password": "fish",
"encryptedPassword": false,
"hashedPassword": false,
"autologin": true
}"#,
);
Expand All @@ -153,13 +153,13 @@ mod test {
full_name: Some("Tux".to_owned()),
user_name: Some("tux".to_owned()),
password: Some("fish".to_owned()),
encrypted_password: Some(false),
hashed_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,
hashed_password: None,
ssh_public_key: Some("keykeykey".to_owned()),
};
let expected = UserSettings {
Expand All @@ -184,22 +184,22 @@ mod test {
when.method(PUT)
.path("/api/users/first")
.header("content-type", "application/json")
.body(r#"{"fullName":"Tux","userName":"tux","password":"fish","encryptedPassword":false,"autologin":true}"#);
.body(r#"{"fullName":"Tux","userName":"tux","password":"fish","hashedPassword":false,"autologin":true}"#);
then.status(200);
});
// note that we use 2 requests for root
let root_mock = server.mock(|when, then| {
when.method(PATCH)
.path("/api/users/root")
.header("content-type", "application/json")
.body(r#"{"sshkey":null,"password":"1234","encryptedPassword":false}"#);
.body(r#"{"sshkey":null,"password":"1234","hashedPassword":false}"#);
then.status(200).body("0");
});
let root_mock2 = server.mock(|when, then| {
when.method(PATCH)
.path("/api/users/root")
.header("content-type", "application/json")
.body(r#"{"sshkey":"keykeykey","password":null,"encryptedPassword":null}"#);
.body(r#"{"sshkey":"keykeykey","password":null,"hashedPassword":null}"#);
then.status(200).body("0");
});
let url = server.url("/api");
Expand All @@ -210,12 +210,12 @@ mod test {
full_name: Some("Tux".to_owned()),
user_name: Some("tux".to_owned()),
password: Some("fish".to_owned()),
encrypted_password: Some(false),
hashed_password: Some(false),
autologin: Some(true),
};
let root_user = RootUserSettings {
password: Some("1234".to_owned()),
encrypted_password: Some(false),
hashed_password: Some(false),
ssh_public_key: Some("keykeykey".to_owned()),
};
let settings = UserSettings {
Expand Down
4 changes: 2 additions & 2 deletions rust/agama-server/src/users/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ async fn first_user_changed_stream(
full_name: user.0,
user_name: user.1,
password: user.2,
encrypted_password: user.3,
hashed_password: user.3,
autologin: user.4,
};
return Some(Event::FirstUserChanged(user_struct));
Expand Down Expand Up @@ -243,7 +243,7 @@ async fn patch_root(
} else {
state
.users
.set_root_password(&password, config.encrypted_password == Some(true))
.set_root_password(&password, config.hashed_password == Some(true))
.await?
}
}
Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/autoyast/root_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def read
return {} unless root_user

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

public_key = root_user.authorized_keys.first
hsh["sshPublicKey"] = public_key if public_key
Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/autoyast/user_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def read
"password" => user.password.value.to_s
}

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

{ "user" => hsh }
end
Expand Down
10 changes: 5 additions & 5 deletions service/lib/agama/dbus/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def issues
USERS_INTERFACE = "org.opensuse.Agama.Users1"
private_constant :USERS_INTERFACE

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

Expand All @@ -70,9 +70,9 @@ def issues
dbus_reader :first_user, "(sssbba{sv})"

dbus_method :SetRootPassword,
"in Value:s, in Encrypted:b, out result:u" do |value, encrypted|
"in Value:s, in Hashed:b, out result:u" do |value, hashed|
logger.info "Setting Root Password"
backend.assign_root_password(value, encrypted)
backend.assign_root_password(value, hashed)

dbus_properties_changed(USERS_INTERFACE, { "RootPasswordSet" => !value.empty? }, [])
0
Expand All @@ -99,10 +99,10 @@ def issues
# 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, encrypted_password, auto_login, data|
|full_name, user_name, password, hashed_password, auto_login, data|
logger.info "Setting first user #{full_name}"
user_issues = backend.assign_first_user(full_name, user_name, password,
encrypted_password, auto_login, data)
hashed_password, auto_login, data)

if user_issues.empty?
dbus_properties_changed(USERS_INTERFACE, { "FirstUser" => first_user }, [])
Expand Down
10 changes: 5 additions & 5 deletions service/lib/agama/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ def root_ssh_key?
!root_ssh_key.empty?
end

def assign_root_password(value, encrypted)
pwd = if encrypted
def assign_root_password(value, hashed)
pwd = if hashed
Y2Users::Password.create_encrypted(value)
else
Y2Users::Password.create_plain(value)
Expand Down Expand Up @@ -99,16 +99,16 @@ 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 hashed_password [Boolean] true = hashed 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, encrypted_password, auto_login, _data)
def assign_first_user(full_name, user_name, password, hashed_password, auto_login, _data)
remove_first_user

user = Y2Users::User.new(user_name)
user.gecos = [full_name]
user.password = if encrypted_password
user.password = if hashed_password
Y2Users::Password.create_encrypted(password)
else
Y2Users::Password.create_plain(password)
Expand Down
10 changes: 5 additions & 5 deletions service/test/agama/users_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@
describe "#assign_root_password" do
let(:root_user) { instance_double(Y2Users::User) }

context "when the password is encrypted" do
it "sets the password as encrypted" do
subject.assign_root_password("encrypted", true)
context "when the password is hashed" do
it "sets the password as hashed" do
subject.assign_root_password("hashed", true)
root_user = users_config.users.root
expect(root_user.password).to eq(Y2Users::Password.create_encrypted("encrypted"))
expect(root_user.password).to eq(Y2Users::Password.create_encrypted("hashed"))
end
end

context "when the password is not encrypted" do
context "when the password is not hashed" do
it "sets the password in clear text" do
subject.assign_root_password("12345", false)
root_user = users_config.users.root
Expand Down
Loading

0 comments on commit 1fcd19d

Please sign in to comment.