Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 116 additions & 47 deletions rust/agama-users/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
use crate::service;
use agama_utils::api::users::config::{FirstUserConfig, RootUserConfig, UserPassword};
use agama_utils::api::users::Config;
use std::fs;
use std::fs::{OpenOptions, Permissions};
use std::fs::{self, OpenOptions, Permissions};
use std::io::Write;
use std::ops::{Deref, DerefMut};
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
Expand Down Expand Up @@ -54,6 +53,7 @@ pub trait ModelAdapter: Send + 'static {
/// # Ok(())
/// # }
/// ```
#[derive(Debug)]
pub struct ChrootCommand {
command: Command,
}
Expand Down Expand Up @@ -106,18 +106,12 @@ impl Model {
}
}

/// Reads first user's data from given config and updates its setup accordingly
fn add_first_user(&self, user: &FirstUserConfig) -> Result<(), service::Error> {
let Some(ref user_name) = user.user_name else {
return Err(service::Error::MissingUserData);
};
let Some(ref user_password) = user.password else {
return Err(service::Error::MissingUserData);
};

fn useradd(&self, user_name: &str) -> Result<(), service::Error> {
let useradd = ChrootCommand::new(self.install_dir.clone())?
.cmd("useradd")
.args([user_name])
// Explicitly enforce creating home here, so even if some product has as default no
// home, we need it to be able to support user ssh keys.
.args(["-m", user_name])
.output()?;

if !useradd.status.success() {
Expand All @@ -128,31 +122,52 @@ impl Model {
)));
}

Ok(())
}

/// Reads first user's data from given config and updates its setup accordingly
fn add_first_user(&self, user: &FirstUserConfig) {
let Some(ref user_name) = user.user_name else {
tracing::warn!("user name is missing in first user config");
return;
};

if let Err(err) = self.useradd(user_name) {
tracing::error!("Failed to create first user: {:?}", err);
return;
}

let ssh_keys = user
.ssh_public_key
.as_ref()
.map(|k| k.to_vec())
.unwrap_or_default();

self.activate_ssh(
&PathBuf::from(format!("/home/{}/.ssh", user_name)),
&ssh_keys,
)?;
let keys_path = PathBuf::from(format!("home/{}/.ssh/authorized_keys", user_name));
self.activate_ssh(&keys_path, &ssh_keys, Some(user_name));

let _ = self.set_user_group(user_name);
self.set_user_password(user_name, user_password)?;
self.update_user_fullname(user)
self.set_user_group(user_name);
if let Some(ref user_password) = user.password {
if let Err(e) = self.set_user_password(user_name, user_password) {
tracing::error!("Failed to set user password: {e}");
}
};
if let Err(e) = self.update_user_fullname(user) {
tracing::error!("Failed to set user fullname: {e}");
}
}

/// Reads root's data from given config and updates root setup accordingly
fn add_root_user(&self, root: &RootUserConfig) -> Result<(), service::Error> {
fn add_root_user(&self, root: &RootUserConfig) {
if root.password.is_none() && root.ssh_public_key.is_none() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I would skip this if. You can do later if let Some(...). However, if you are already testing this code, we can refine it after beta 2.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, previously it raise error...but I agree that it looks kind of strange.

return Err(service::Error::MissingRootData);
return;
};

// set password for root if any
if let Some(ref root_password) = root.password {
self.set_user_password("root", root_password)?;
if let Err(e) = self.set_user_password("root", root_password) {
tracing::error!("Failed to set root password: {e}");
}
}

// store sshPublicKeys for root if any
Expand All @@ -162,22 +177,26 @@ impl Model {
.map(|k| k.to_vec())
.unwrap_or_default();

self.activate_ssh(&PathBuf::from("root/.ssh/authorized_keys"), &ssh_keys)?;

Ok(())
self.activate_ssh(Path::new("root/.ssh/authorized_keys"), &ssh_keys, None);
}

fn activate_ssh(&self, path: &PathBuf, ssh_keys: &[String]) -> Result<(), service::Error> {
if !ssh_keys.is_empty() {
// if some SSH keys were defined
// - update authorized_keys file
// - open SSH port and enable SSH service
self.update_authorized_keys(path, ssh_keys)?;
self.enable_sshd_service()?;
self.open_ssh_port()?;
fn activate_ssh(&self, path: &Path, ssh_keys: &[String], user: Option<&str>) {
if ssh_keys.is_empty() {
return;
}

Ok(())
// if some SSH keys were defined
// - update authorized_keys file
// - open SSH port and enable SSH service
if let Err(e) = self.update_authorized_keys(path, ssh_keys, user) {
tracing::error!("Failed to update authorized_keys file: {e}");
}
if let Err(e) = self.enable_sshd_service() {
tracing::error!("Failed to enable sshd service: {e}");
}
if let Err(e) = self.open_ssh_port() {
tracing::error!("Failed to open sshd port in firewall: {e}");
}
}

/// Sets password for given user name
Expand Down Expand Up @@ -218,11 +237,21 @@ impl Model {

/// Add user into the wheel group on best effort basis.
/// If the group doesn't exist, log the error and continue.
fn set_user_group(&self, user_name: &str) -> Result<(), service::Error> {
let usermod = ChrootCommand::new(self.install_dir.clone())?
fn set_user_group(&self, user_name: &str) {
let chroot = ChrootCommand::new(self.install_dir.clone());
let Ok(chroot) = chroot else {
tracing::error!("Failed to chroot: {:?}", chroot);
return;
};

let usermod = chroot
.cmd("usermod")
.args(["-a", "-G", "wheel", user_name])
.output()?;
.output();
let Ok(usermod) = usermod else {
tracing::error!("Failed to execute usermod {:?}", usermod);
return;
};

if !usermod.status.success() {
tracing::warn!(
Expand All @@ -231,18 +260,53 @@ impl Model {
usermod.status
);
}
}

/// Changes the owner and group of the target path inside the chroot environment.
fn chown(&self, user_name: &str, path: &Path) -> Result<(), service::Error> {
let abs_path = Path::new("/").join(path);
// unwrap here can be questionable if we want to support
// non-utf8 paths, but I expect more problems with that idea
let target_path = abs_path.to_str().unwrap().to_string();

let chown = ChrootCommand::new(self.install_dir.clone())?
.cmd("chown")
.args([format!("{}:", user_name), target_path])
.output()?;

if !chown.status.success() {
tracing::error!("chown failed {:?}", chown.stderr);
return Err(service::Error::CommandFailed(format!(
"Cannot set user for {:?}: {:?}",
path, chown.stderr
)));
}

Ok(())
}

/// Updates root's authorized_keys file with SSH key
fn update_authorized_keys(
&self,
keys_path: &PathBuf,
keys_path: &Path,
ssh_keys: &[String],
user: Option<&str>,
) -> Result<(), service::Error> {
let mode = 0o644;
let file_name = self.install_dir.join(keys_path);
// unwrap is safe here, because we always use absolute paths
let dir = file_name.parent().unwrap();

// if .ssh does not exist we need to create it, with proper user and perms
if !dir.exists() {
fs::create_dir_all(dir)?;
fs::set_permissions(dir, Permissions::from_mode(0o700))?;

if let Some(user_name) = user {
self.chown(user_name, keys_path.parent().unwrap())?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}
}

let mode = 0o644;
let mut authorized_keys_file = OpenOptions::new()
.create(true)
.append(true)
Expand All @@ -253,12 +317,17 @@ impl Model {
// sets mode also for an existing file
fs::set_permissions(&file_name, Permissions::from_mode(mode))?;

ssh_keys
.iter()
.try_for_each(|ssh_key| -> Result<(), service::Error> {
writeln!(authorized_keys_file, "{}", ssh_key.trim())?;
Ok(())
})
for ssh_key in ssh_keys {
writeln!(authorized_keys_file, "{}", ssh_key.trim())?;
}

authorized_keys_file.flush()?;

if let Some(user_name) = user {
self.chown(user_name, keys_path)?;
}

Ok(())
}

/// Enables sshd service in the target system
Expand Down Expand Up @@ -344,10 +413,10 @@ impl Model {
impl ModelAdapter for Model {
fn install(&self, config: &Config) -> Result<(), service::Error> {
if let Some(first_user) = &config.first_user {
self.add_first_user(first_user)?;
self.add_first_user(first_user);
}
if let Some(root_user) = &config.root {
self.add_root_user(root_user)?;
self.add_root_user(root_user);
}

Ok(())
Expand Down
5 changes: 5 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
-------------------------------------------------------------------
Tue Mar 17 07:00:30 UTC 2026 - Josef Reidinger <jreidinger@suse.com>

- fix the ssh keys for first user (gh#agama-project/agama#3290)

-------------------------------------------------------------------
Mon Mar 16 16:52:49 UTC 2026 - José Iván López González <jlopez@suse.com>

Expand Down
Loading