diff --git a/src/common/mod.rs b/src/common/mod.rs index 848609ec6..c7d65a0ce 100644 --- a/src/common/mod.rs +++ b/src/common/mod.rs @@ -1,5 +1,4 @@ #![forbid(unsafe_code)] -use std::{collections::HashMap, ffi::OsString}; pub use command::CommandAndArguments; pub use context::Context; @@ -15,8 +14,6 @@ mod path; pub mod resolve; mod string; -pub type Environment = HashMap; - // Hardened enum values used for critical enums to mitigate attacks like Rowhammer. // See for example https://arxiv.org/pdf/2309.02545.pdf // The values are copied from https://github.com/sudo-project/sudo/commit/7873f8334c8d31031f8cfa83bd97ac6029309e4f#diff-b8ac7ab4c3c4a75aed0bb5f7c5fd38b9ea6c81b7557f775e46c6f8aa115e02cd diff --git a/src/exec/mod.rs b/src/exec/mod.rs index e64b68506..1e9216490 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -16,7 +16,11 @@ use std::{ }; use crate::{ - common::Environment, + exec::no_pty::exec_no_pty, + log::dev_info, + system::{set_target_user, signal::SignalNumber, term::UserTerm}, +}; +use crate::{ log::dev_warn, system::{ interface::ProcessId, @@ -25,11 +29,6 @@ use crate::{ wait::{Wait, WaitError, WaitOptions}, }, }; -use crate::{ - exec::no_pty::exec_no_pty, - log::dev_info, - system::{set_target_user, signal::SignalNumber, term::UserTerm}, -}; use crate::{log::user_error, system::kill}; pub use interface::RunOptions; @@ -44,7 +43,10 @@ use self::{ /// /// Returns the [`ExitReason`] of the command and a function that restores the default handler for /// signals once its called. -pub fn run_command(options: &impl RunOptions, env: Environment) -> io::Result { +pub fn run_command( + options: &impl RunOptions, + env: impl IntoIterator, impl AsRef)>, +) -> io::Result { // FIXME: should we pipe the stdio streams? let qualified_path = options.command()?; let mut command = Command::new(qualified_path); diff --git a/src/pam/mod.rs b/src/pam/mod.rs index bf71b2364..e650162d4 100644 --- a/src/pam/mod.rs +++ b/src/pam/mod.rs @@ -1,5 +1,4 @@ use std::{ - collections::HashMap, ffi::{CStr, CString, OsStr, OsString}, os::raw::c_char, os::unix::prelude::OsStrExt, @@ -316,8 +315,8 @@ impl PamContext { } /// Get a full listing of the current PAM environment - pub fn env(&mut self) -> PamResult> { - let mut res = HashMap::new(); + pub fn env(&mut self) -> PamResult> { + let mut res = Vec::new(); // SAFETY: `self.pamh` contains a correct handle (obtained from `pam_start`). // The man page for pam_getenvlist states that: // The format of the memory is a malloc()'d array of char pointers, the last element @@ -348,7 +347,7 @@ impl PamContext { } }; if let Some((k, v)) = data { - res.insert(k, v); + res.push((k, v)); } // SAFETY: curr_str was obtained via libc::malloc() so we are responsible for freeing it. diff --git a/src/su/context.rs b/src/su/context.rs index 75ee0d07e..97551ce65 100644 --- a/src/su/context.rs +++ b/src/su/context.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashMap, env, ffi::OsString, fs, io, @@ -9,7 +10,7 @@ use crate::exec::RunOptions; use crate::log::user_warn; use crate::system::{Group, Process, User}; use crate::{ - common::{error::Error, resolve::CurrentUser, Environment}, + common::{error::Error, resolve::CurrentUser}, system::interface::ProcessId, }; use crate::{ @@ -17,6 +18,8 @@ use crate::{ system::interface::UserId, }; +type Environment = HashMap; + use super::cli::SuRunOptions; const VALID_LOGIN_SHELLS_LIST: &str = "/etc/shells"; diff --git a/src/sudo/env/environment.rs b/src/sudo/env/environment.rs index dd44e0b2e..9c240394b 100644 --- a/src/sudo/env/environment.rs +++ b/src/sudo/env/environment.rs @@ -1,10 +1,10 @@ use std::{ - collections::{hash_map::Entry, HashSet}, + collections::{hash_map::Entry, HashMap, HashSet}, ffi::{OsStr, OsString}, os::unix::prelude::OsStrExt, }; -use crate::common::{CommandAndArguments, Context, Environment}; +use crate::common::{CommandAndArguments, Context}; use crate::sudoers::Policy; use crate::system::PATH_MAX; @@ -14,6 +14,27 @@ const PATH_MAILDIR: &str = env!("PATH_MAILDIR"); const PATH_ZONEINFO: &str = env!("PATH_ZONEINFO"); const PATH_DEFAULT: &str = env!("SUDO_PATH_DEFAULT"); +#[cfg(not(test))] +pub struct Environment(HashMap); +#[cfg(test)] +#[derive(Clone, Debug, Default)] +pub struct Environment(pub(crate) HashMap); + +impl IntoIterator for Environment { + type Item = (OsString, OsString); + type IntoIter = as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl Environment { + pub fn from_system() -> Self { + Environment(std::env::vars_os().collect()) + } +} + /// check byte slice contains with given byte slice fn contains_subsequence(haystack: &[u8], needle: &[u8]) -> bool { haystack @@ -42,7 +63,7 @@ fn add_extra_env( context: &Context, cfg: &impl Policy, sudo_ps1: Option, - environment: &mut Environment, + environment: &mut HashMap, ) { // current user environment.insert("SUDO_COMMAND".into(), format_command(&context.command)); @@ -192,18 +213,18 @@ fn should_keep(key: &OsStr, value: &OsStr, cfg: &impl Policy) -> bool { /// Environment variables with a value beginning with ‘()’ are removed pub fn get_target_environment( current_env: Environment, - additional_env: Environment, + additional_env: impl IntoIterator, context: &Context, settings: &impl Policy, ) -> Environment { - let mut environment = Environment::default(); + let current_env = current_env.0; // retrieve SUDO_PS1 value to set a PS1 value as additional environment let sudo_ps1 = current_env.get(OsStr::new("SUDO_PS1")).cloned(); // variables preserved from the invoking user's environment by the // env_keep list take precedence over those in the PAM environment - environment.extend(additional_env); + let mut environment: HashMap<_, _> = additional_env.into_iter().collect(); environment.extend( current_env @@ -213,7 +234,7 @@ pub fn get_target_environment( add_extra_env(context, settings, sudo_ps1, &mut environment); - environment + Environment(environment) } #[cfg(test)] diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs index ea6e3897b..a6c592425 100644 --- a/src/sudo/env/tests.rs +++ b/src/sudo/env/tests.rs @@ -1,8 +1,8 @@ use crate::common::resolve::CurrentUser; -use crate::common::{CommandAndArguments, Context, Environment}; +use crate::common::{CommandAndArguments, Context}; use crate::sudo::{ cli::{SudoAction, SudoRunOptions}, - env::environment::get_target_environment, + env::environment::{get_target_environment, Environment}, }; use crate::system::interface::{GroupId, UserId}; use crate::system::{Group, Hostname, Process, User}; @@ -66,11 +66,12 @@ fn parse_env_commands(input: &str) -> Vec<(&str, Environment)> { .map(|e| { let (cmd, vars) = e.split_once('\n').unwrap(); - let vars: Environment = vars - .lines() - .map(|line| line.trim().split_once('=').unwrap()) - .map(|(k, v)| (k.into(), v.into())) - .collect(); + let vars = Environment( + vars.lines() + .map(|line| line.trim().split_once('=').unwrap()) + .map(|(k, v)| (k.into(), v.into())) + .collect(), + ); (cmd, vars) }) @@ -142,7 +143,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { fn environment_to_set(environment: Environment) -> HashSet { HashSet::from_iter( environment - .iter() + .into_iter() .map(|(k, v)| format!("{}={}", k.to_str().unwrap(), v.to_str().unwrap())), ) } diff --git a/src/sudo/pam.rs b/src/sudo/pam.rs index fdc04d4af..d8e1f190f 100644 --- a/src/sudo/pam.rs +++ b/src/sudo/pam.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::ffi::OsString; use crate::common::context::LaunchType; @@ -59,7 +58,7 @@ impl AuthPlugin for PamAuthenticator { Ok(()) } - fn pre_exec(&mut self, target_user: &str) -> Result, Error> { + fn pre_exec(&mut self, target_user: &str) -> Result, Error> { let pam = self .pam .as_mut() diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs index 3b8f0a98e..d960969a9 100644 --- a/src/sudo/pipeline.rs +++ b/src/sudo/pipeline.rs @@ -1,13 +1,13 @@ -use std::ffi::OsStr; +use std::ffi::{OsStr, OsString}; use std::process::exit; use super::cli::{SudoRunOptions, SudoValidateOptions}; use crate::common::context::OptionsForContext; use crate::common::resolve::CurrentUser; -use crate::common::{Context, Environment, Error}; +use crate::common::{Context, Error}; use crate::exec::{ExecOutput, ExitReason}; use crate::log::{auth_info, auth_warn}; -use crate::sudo::env::environment; +use crate::sudo::env::environment::{self, Environment}; use crate::sudo::Duration; use crate::sudoers::{Authorization, AuthorizationAllowed, DirChange, Policy, PreJudgementPolicy}; use crate::system::interface::UserId; @@ -32,7 +32,7 @@ pub trait PolicyPlugin { pub trait AuthPlugin { fn init(&mut self, context: &Context) -> Result<(), Error>; fn authenticate(&mut self, non_interactive: bool, max_tries: u16) -> Result<(), Error>; - fn pre_exec(&mut self, target_user: &str) -> Result; + fn pre_exec(&mut self, target_user: &str) -> Result, Error>; fn cleanup(&mut self); } @@ -73,9 +73,12 @@ impl Pipeline { let additional_env = self.authenticator.pre_exec(&context.target_user.name)?; // build environment - let current_env = std::env::vars_os().collect(); - let target_env = - environment::get_target_environment(current_env, additional_env, &context, &policy); + let target_env = environment::get_target_environment( + Environment::from_system(), + additional_env, + &context, + &policy, + ); let pid = context.process.pid;