-
Notifications
You must be signed in to change notification settings - Fork 119
Smarter ssh authorized key search for system-reinstall-bootc #1146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,4 @@ | ||
use crate::{ | ||
prompt, | ||
users::{get_all_users_keys, UserKeys}, | ||
}; | ||
use crate::{prompt, users::get_all_users_keys}; | ||
use anyhow::{ensure, Context, Result}; | ||
|
||
const NO_SSH_PROMPT: &str = "None of the users on this system found have authorized SSH keys, \ | ||
|
@@ -10,7 +7,9 @@ const NO_SSH_PROMPT: &str = "None of the users on this system found have authori | |
|
||
fn prompt_single_user(user: &crate::users::UserKeys) -> Result<Vec<&crate::users::UserKeys>> { | ||
let prompt = format!( | ||
"Found only one user ({}) with {} SSH authorized keys. Would you like to import it and its keys to the system?", | ||
"Found only one user ({}) with {} SSH authorized keys.\n\ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion-for-followup: This would be more readable with |
||
Would you like to import its SSH authorized keys\n\ | ||
into the root user on the new bootc system?", | ||
user.user, | ||
user.num_keys(), | ||
); | ||
|
@@ -25,7 +24,10 @@ fn prompt_user_selection( | |
|
||
// TODO: Handle https://github.com/console-rs/dialoguer/issues/77 | ||
let selected_user_indices: Vec<usize> = dialoguer::MultiSelect::new() | ||
.with_prompt("Select the users you want to install in the system (along with their authorized SSH keys)") | ||
.with_prompt( | ||
"Select which user's SSH authorized keys you want to\n\ | ||
import into the root user of the new bootc system", | ||
) | ||
.items(&keys) | ||
.interact()?; | ||
|
||
|
@@ -62,18 +64,22 @@ pub(crate) fn ask_yes_no(prompt: &str, default: bool) -> Result<bool> { | |
.context("prompting") | ||
} | ||
|
||
/// For now we only support the root user. This function returns the root user's SSH | ||
/// authorized_keys. In the future, when bootc supports multiple users, this function will need to | ||
/// be updated to return the SSH authorized_keys for all the users selected by the user. | ||
pub(crate) fn get_root_key() -> Result<Option<UserKeys>> { | ||
/// Gather authorized keys for all user's of the host system | ||
/// prompt the user to select which users's keys will be imported | ||
/// into the target system's root user's authorized_keys file | ||
/// | ||
/// The keys are stored in a temporary file which is passed to | ||
/// the podman run invocation to be used by | ||
/// `bootc install to-existing-root --root-ssh-authorized-keys` | ||
pub(crate) fn get_ssh_keys(temp_key_file_path: &str) -> Result<()> { | ||
let users = get_all_users_keys()?; | ||
if users.is_empty() { | ||
ensure!( | ||
prompt::ask_yes_no(NO_SSH_PROMPT, false)?, | ||
"cancelled by user" | ||
); | ||
|
||
return Ok(None); | ||
return Ok(()); | ||
} | ||
|
||
let selected_users = if users.len() == 1 { | ||
|
@@ -82,12 +88,13 @@ pub(crate) fn get_root_key() -> Result<Option<UserKeys>> { | |
prompt_user_selection(&users)? | ||
}; | ||
|
||
ensure!( | ||
selected_users.iter().all(|x| x.user == "root"), | ||
"Only importing the root user keys is supported for now" | ||
); | ||
let keys = selected_users | ||
.into_iter() | ||
.map(|user_key| user_key.authorized_keys.as_str()) | ||
.collect::<Vec<&str>>() | ||
.join("\n"); | ||
ckyrouac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
let root_key = selected_users.into_iter().find(|x| x.user == "root"); | ||
std::fs::write(temp_key_file_path, keys.as_bytes())?; | ||
|
||
Ok(root_key.cloned()) | ||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,16 @@ | ||
use anyhow::{Context, Result}; | ||
use bootc_utils::CommandRunExt; | ||
use bootc_utils::PathQuotedDisplay; | ||
use rustix::fs::Uid; | ||
use rustix::process::geteuid; | ||
use rustix::process::getuid; | ||
use rustix::thread::set_thread_res_uid; | ||
use serde_json::Value; | ||
use std::collections::BTreeMap; | ||
use std::collections::BTreeSet; | ||
use std::fmt::Display; | ||
use std::fmt::Formatter; | ||
use std::os::unix::process::CommandExt; | ||
use std::process::Command; | ||
use uzers::os::unix::UserExt; | ||
|
||
|
@@ -82,7 +85,6 @@ impl Drop for UidChange { | |
pub(crate) struct UserKeys { | ||
pub(crate) user: String, | ||
pub(crate) authorized_keys: String, | ||
pub(crate) authorized_keys_path: String, | ||
} | ||
|
||
impl UserKeys { | ||
|
@@ -102,61 +104,135 @@ impl Display for UserKeys { | |
} | ||
} | ||
|
||
#[derive(Debug)] | ||
struct SshdConfig<'a> { | ||
authorized_keys_files: Vec<&'a str>, | ||
authorized_keys_command: &'a str, | ||
authorized_keys_command_user: &'a str, | ||
} | ||
|
||
impl<'a> SshdConfig<'a> { | ||
pub fn parse(sshd_output: &'a str) -> Result<SshdConfig<'a>> { | ||
let config = sshd_output | ||
.lines() | ||
.filter_map(|line| line.split_once(' ')) | ||
.collect::<BTreeMap<&str, &str>>(); | ||
|
||
let authorized_keys_files: Vec<&str> = config | ||
.get("authorizedkeysfile") | ||
.unwrap_or(&"none") | ||
.split_whitespace() | ||
.collect(); | ||
let authorized_keys_command = config.get("authorizedkeyscommand").unwrap_or(&"none"); | ||
let authorized_keys_command_user = | ||
config.get("authorizedkeyscommanduser").unwrap_or(&"none"); | ||
|
||
Ok(Self { | ||
authorized_keys_files, | ||
authorized_keys_command, | ||
authorized_keys_command_user, | ||
}) | ||
} | ||
} | ||
|
||
fn get_keys_from_files(user: &uzers::User, keyfiles: &Vec<&str>) -> Result<String> { | ||
let home_dir = user.home_dir(); | ||
let mut user_authorized_keys = String::new(); | ||
|
||
for keyfile in keyfiles { | ||
let user_authorized_keys_path = home_dir.join(keyfile); | ||
|
||
if !user_authorized_keys_path.exists() { | ||
tracing::debug!( | ||
"Skipping authorized key file {} for user {} because it doesn't exist", | ||
PathQuotedDisplay::new(&user_authorized_keys_path), | ||
user.name().to_string_lossy() | ||
); | ||
continue; | ||
} | ||
|
||
// Safety: The UID should be valid because we got it from uzers | ||
#[allow(unsafe_code)] | ||
let user_uid = unsafe { Uid::from_raw(user.uid()) }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (This is OK for now but I think per previous discussion we actually don't need to change uid for this, we can just use cap-std) |
||
|
||
// Change the effective uid for this scope, to avoid accidentally reading files we | ||
// shouldn't through symlinks | ||
let _uid_change = UidChange::new(user_uid)?; | ||
|
||
let key = std::fs::read_to_string(&user_authorized_keys_path) | ||
.context("Failed to read user's authorized keys")?; | ||
user_authorized_keys.push_str(key.as_str()); | ||
ckyrouac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
user_authorized_keys.push('\n'); | ||
} | ||
|
||
Ok(user_authorized_keys) | ||
} | ||
|
||
fn get_keys_from_command(command: &str, command_user: &str) -> Result<String> { | ||
ckyrouac marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let user_config = uzers::get_user_by_name(command_user).context(format!( | ||
"authorized_keys_command_user {} not found", | ||
command_user | ||
))?; | ||
|
||
let mut cmd = Command::new(command); | ||
cmd.uid(user_config.uid()); | ||
let output = cmd | ||
.run_get_string() | ||
.context(format!("running authorized_keys_command {}", command))?; | ||
Ok(output) | ||
} | ||
|
||
pub(crate) fn get_all_users_keys() -> Result<Vec<UserKeys>> { | ||
let loginctl_user_names = loginctl_users().context("enumerate users")?; | ||
|
||
let mut all_users_authorized_keys = Vec::new(); | ||
|
||
let sshd_output = Command::new("sshd") | ||
.arg("-T") | ||
.run_get_string() | ||
.context("running sshd -T")?; | ||
tracing::trace!("sshd output:\n {}", sshd_output); | ||
|
||
let sshd_config = SshdConfig::parse(sshd_output.as_str())?; | ||
tracing::debug!("parsed sshd config: {:?}", sshd_config); | ||
|
||
for user_name in loginctl_user_names { | ||
let user_info = uzers::get_user_by_name(user_name.as_str()) | ||
.context(format!("user {} not found", user_name))?; | ||
|
||
let home_dir = user_info.home_dir(); | ||
let user_authorized_keys_path = home_dir.join(".ssh/authorized_keys"); | ||
|
||
if !user_authorized_keys_path.exists() { | ||
tracing::debug!( | ||
"Skipping user {} because it doesn't have an SSH authorized_keys file", | ||
user_info.name().to_string_lossy() | ||
); | ||
continue; | ||
let mut user_authorized_keys = String::new(); | ||
if !sshd_config.authorized_keys_files.is_empty() { | ||
let keys = get_keys_from_files(&user_info, &sshd_config.authorized_keys_files)?; | ||
user_authorized_keys.push_str(keys.as_str()); | ||
} | ||
|
||
if sshd_config.authorized_keys_command != "none" { | ||
let keys = get_keys_from_command( | ||
&sshd_config.authorized_keys_command, | ||
&sshd_config.authorized_keys_command_user, | ||
)?; | ||
user_authorized_keys.push_str(keys.as_str()); | ||
}; | ||
|
||
let user_name = user_info | ||
.name() | ||
.to_str() | ||
.context("user name is not valid utf-8")?; | ||
|
||
let user_authorized_keys = { | ||
// Safety: The UID should be valid because we got it from uzers | ||
#[allow(unsafe_code)] | ||
let user_uid = unsafe { Uid::from_raw(user_info.uid()) }; | ||
|
||
// Change the effective uid for this scope, to avoid accidentally reading files we | ||
// shouldn't through symlinks | ||
let _uid_change = UidChange::new(user_uid)?; | ||
|
||
std::fs::read_to_string(&user_authorized_keys_path) | ||
.context("Failed to read user's authorized keys")? | ||
}; | ||
|
||
if user_authorized_keys.trim().is_empty() { | ||
tracing::debug!( | ||
"Skipping user {} because it has an empty SSH authorized_keys file", | ||
user_info.name().to_string_lossy() | ||
"Skipping user {} because it has no SSH authorized_keys", | ||
user_name | ||
); | ||
continue; | ||
} | ||
|
||
let user_keys = UserKeys { | ||
user: user_name.to_string(), | ||
authorized_keys: user_authorized_keys, | ||
authorized_keys_path: user_authorized_keys_path | ||
.to_str() | ||
.context("user's authorized_keys path is not valid utf-8")? | ||
.to_string(), | ||
}; | ||
|
||
tracing::trace!("Found user keys: {:?}", user_keys); | ||
tracing::debug!( | ||
"Found user {} with {} SSH authorized_keys", | ||
user_keys.user, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit-for-followup: Let's use
{ workspace = true }