Refactor pipeline (#964)

src/common/context.rs

@@ -1,6 +1,4 @@
-use crate::common::resolve::AuthUser;
 use crate::common::{HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2};
-use crate::sudoers::AuthenticatingUser;
 use crate::system::{Group, Hostname, Process, User};
 
 use super::resolve::CurrentUser;
@@ -45,7 +43,6 @@ pub struct Context {
     // system
     pub hostname: Hostname,
     pub current_user: CurrentUser,
-    pub auth_user: AuthUser,
     pub process: Process,
     // policy
     pub use_pty: bool,
@@ -63,15 +60,10 @@ pub enum LaunchType {
 impl Context {
     pub fn build_from_options(
         sudo_options: OptionsForContext,
-        path: String,
-        auth_user: AuthenticatingUser,
+        secure_path: Option<&str>,
     ) -> Result<Context, Error> {
         let hostname = Hostname::resolve();
         let current_user = CurrentUser::resolve()?;
-        let auth_user = match auth_user {
-            AuthenticatingUser::InvokingUser => AuthUser::from_current_user(current_user.clone()),
-            AuthenticatingUser::Root => AuthUser::resolve_root_for_rootpw()?,
-        };
         let (target_user, target_group) =
             resolve_target_user_and_group(&sudo_options.user, &sudo_options.group, &current_user)?;
         let (launch, shell) = resolve_launch_and_shell(&sudo_options, &current_user, &target_user);
@@ -82,14 +74,24 @@ impl Context {
                 // FIXME `Default` is being used as `Option::None`
                 Default::default()
             }
-            _ => CommandAndArguments::build_from_args(shell, sudo_options.positional_args, &path),
+            _ => {
+                let system_path;
+
+                let path = if let Some(path) = secure_path {
+                    path
+                } else {
+                    system_path = std::env::var("PATH").unwrap_or_default();
+                    system_path.as_ref()
+                };
+
+                CommandAndArguments::build_from_args(shell, sudo_options.positional_args, path)
+            }
         };
 
         Ok(Context {
             hostname,
             command,
             current_user,
-            auth_user,
             target_user,
             target_group,
             use_session_records: !sudo_options.reset_timestamp,
@@ -108,7 +110,6 @@ impl Context {
 mod tests {
     use crate::{
         sudo::SudoAction,
-        sudoers::AuthenticatingUser,
         system::{interface::UserId, Hostname},
     };
     use std::collections::HashMap;
@@ -124,12 +125,7 @@ mod tests {
             .unwrap();
         let path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin";
         let (ctx_opts, _pipe_opts) = options.into();
-        let context = Context::build_from_options(
-            ctx_opts,
-            path.to_string(),
-            AuthenticatingUser::InvokingUser,
-        )
-        .unwrap();
+        let context = Context::build_from_options(ctx_opts, Some(path)).unwrap();
 
         let mut target_environment = HashMap::new();
         target_environment.insert("SUDO_USER".to_string(), context.current_user.name.clone());

src/sudo/env/environment.rs

@@ -5,7 +5,7 @@ use std::{
 };
 
 use crate::common::{CommandAndArguments, Context, Error};
-use crate::sudoers::Policy;
+use crate::sudoers::Restrictions;
 use crate::system::PATH_MAX;
 
 use super::wildcard_match::wildcard_match;
@@ -47,7 +47,7 @@ fn format_command(command_and_arguments: &CommandAndArguments) -> OsString {
 /// Construct sudo-specific environment variables
 fn add_extra_env(
     context: &Context,
-    cfg: &impl Policy,
+    cfg: &Restrictions,
     sudo_ps1: Option<OsString>,
     environment: &mut Environment,
 ) {
@@ -101,7 +101,7 @@ fn add_extra_env(
     }
 
     // Overwrite PATH when secure_path is set
-    if let Some(secure_path) = cfg.secure_path() {
+    if let Some(secure_path) = &cfg.path {
         // assign path by env path or secure_path configuration
         environment.insert("PATH".into(), secure_path.into());
     }
@@ -168,25 +168,25 @@ fn in_table(needle: &OsStr, haystack: &HashSet<String>) -> bool {
 }
 
 /// Determine whether a specific environment variable should be kept
-fn should_keep(key: &OsStr, value: &OsStr, cfg: &impl Policy) -> bool {
+fn should_keep(key: &OsStr, value: &OsStr, cfg: &Restrictions) -> bool {
     if value.as_bytes().starts_with("()".as_bytes()) {
         return false;
     }
 
-    if cfg.secure_path().is_some() && key == "PATH" {
+    if cfg.path.is_some() && key == "PATH" {
         return false;
     }
 
     if key == "TZ" {
-        return in_table(key, cfg.env_keep())
-            || (in_table(key, cfg.env_check()) && is_safe_tz(value.as_bytes()));
+        return in_table(key, cfg.env_keep)
+            || (in_table(key, cfg.env_check) && is_safe_tz(value.as_bytes()));
     }
 
-    if in_table(key, cfg.env_check()) {
+    if in_table(key, cfg.env_check) {
         return !value.as_bytes().iter().any(|c| *c == b'%' || *c == b'/');
     }
 
-    in_table(key, cfg.env_keep())
+    in_table(key, cfg.env_keep)
 }
 
 /// Construct the final environment from the current one and a sudo context
@@ -207,7 +207,7 @@ pub fn get_target_environment(
     additional_env: impl IntoIterator<Item = (OsString, OsString)>,
     user_override: Vec<(String, String)>,
     context: &Context,
-    settings: &impl Policy,
+    settings: &Restrictions,
 ) -> Result<Environment, Error> {
     // retrieve SUDO_PS1 value to set a PS1 value as additional environment
     let sudo_ps1 = current_env.get(OsStr::new("SUDO_PS1")).cloned();
@@ -254,7 +254,6 @@ where
 #[cfg(test)]
 mod tests {
     use super::{is_safe_tz, should_keep, PATH_ZONEINFO};
-    use crate::sudoers::Policy;
     use std::{collections::HashSet, ffi::OsStr};
 
     struct TestConfiguration {
@@ -263,32 +262,21 @@ mod tests {
         path: Option<String>,
     }
 
-    impl Policy for TestConfiguration {
-        fn env_keep(&self) -> &HashSet<String> {
-            &self.keep
-        }
-
-        fn env_check(&self) -> &HashSet<String> {
-            &self.check
-        }
-
-        fn secure_path(&self) -> Option<String> {
-            self.path.clone()
-        }
-
-        fn use_pty(&self) -> bool {
-            true
-        }
-
-        fn pwfeedback(&self) -> bool {
-            false
-        }
-    }
-
     impl TestConfiguration {
         pub fn check_should_keep(&self, key: &str, value: &str, expected: bool) {
             assert_eq!(
-                should_keep(OsStr::new(key), OsStr::new(value), self),
+                should_keep(
+                    OsStr::new(key),
+                    OsStr::new(value),
+                    &crate::sudoers::Restrictions {
+                        env_keep: &self.keep,
+                        env_check: &self.check,
+                        path: self.path.as_deref(),
+                        chdir: crate::sudoers::DirChange::Strict(None),
+                        trust_environment: false,
+                        use_pty: true,
+                    }
+                ),
                 expected,
                 "{} should {}",
                 key,

src/sudo/env/tests.rs

@@ -1,4 +1,4 @@
-use crate::common::resolve::{AuthUser, CurrentUser};
+use crate::common::resolve::CurrentUser;
 use crate::common::{CommandAndArguments, Context};
 use crate::sudo::{
     cli::{SudoAction, SudoRunOptions},
@@ -94,8 +94,6 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
         groups: vec![],
     });
 
-    let auth_user = AuthUser::from_current_user(current_user.clone());
-
     let current_group = Group {
         gid: GroupId::new(1000),
         name: Some("test".to_string()),
@@ -121,7 +119,6 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
         hostname: Hostname::fake("test-ubuntu"),
         command,
         current_user: current_user.clone(),
-        auth_user,
         target_user: if sudo_options.user.as_deref() == Some("test") {
             current_user.into()
         } else {
@@ -162,14 +159,21 @@ fn test_environment_variable_filtering() {
             .try_into_run()
             .ok()
             .unwrap();
-        let settings = crate::sudoers::Judgement::default();
+        let settings = crate::defaults::Settings::default();
         let context = create_test_context(&options);
         let resulting_env = get_target_environment(
             initial_env.clone(),
             HashMap::new(),
             Vec::new(),
             &context,
-            &settings,
+            &crate::sudoers::Restrictions {
+                env_keep: settings.env_keep(),
+                env_check: settings.env_check(),
+                path: settings.secure_path(),
+                use_pty: true,
+                chdir: crate::sudoers::DirChange::Strict(None),
+                trust_environment: false,
+            },
         )
         .unwrap();
 

src/sudo/mod.rs

@@ -1,7 +1,7 @@
 #![forbid(unsafe_code)]
 
 use crate::common::resolve::CurrentUser;
-use crate::common::{Context, Error};
+use crate::common::Error;
 use crate::log::dev_info;
 use crate::system::interface::UserId;
 use crate::system::kernel::kernel_check;
@@ -14,7 +14,7 @@ pub use cli::SudoAction;
 #[cfg(not(test))]
 use cli::SudoAction;
 use pam::PamAuthenticator;
-use pipeline::{Pipeline, PolicyPlugin};
+use pipeline::Pipeline;
 use std::path::Path;
 
 mod cli;
@@ -65,50 +65,6 @@ pub(crate) fn candidate_sudoers_file() -> &'static Path {
     file
 }
 
-#[derive(Default)]
-pub(crate) struct SudoersPolicy {}
-
-impl PolicyPlugin for SudoersPolicy {
-    type PreJudgementPolicy = crate::sudoers::Sudoers;
-    type Policy = crate::sudoers::Judgement;
-
-    fn init(&mut self) -> Result<Self::PreJudgementPolicy, Error> {
-        let sudoers_path = candidate_sudoers_file();
-
-        let (sudoers, syntax_errors) = crate::sudoers::Sudoers::open(sudoers_path)
-            .map_err(|e| Error::Configuration(format!("{e}")))?;
-
-        for crate::sudoers::Error {
-            source,
-            location,
-            message,
-        } in syntax_errors
-        {
-            let path = source.as_deref().unwrap_or(sudoers_path);
-            diagnostic::diagnostic!("{message}", path @ location);
-        }
-
-        Ok(sudoers)
-    }
-
-    fn judge(
-        &mut self,
-        pre: Self::PreJudgementPolicy,
-        context: &Context,
-    ) -> Result<Self::Policy, Error> {
-        Ok(pre.check(
-            &*context.current_user,
-            &context.hostname,
-            crate::sudoers::Request {
-                user: &context.target_user,
-                group: &context.target_group,
-                command: &context.command.command,
-                arguments: &context.command.arguments,
-            },
-        ))
-    }
-}
-
 fn sudo_process() -> Result<(), Error> {
     crate::log::SudoLogger::new("sudo: ").into_global_logger();
 
@@ -118,7 +74,6 @@ fn sudo_process() -> Result<(), Error> {
     kernel_check()?;
 
     let pipeline = Pipeline {
-        policy: SudoersPolicy::default(),
         authenticator: PamAuthenticator::new_cli(),
     };
 

src/sudo/pam.rs

@@ -1,14 +1,15 @@
 use std::ffi::OsString;
 
 use crate::common::context::LaunchType;
+use crate::common::resolve::AuthUser;
 use crate::common::{error::Error, Context};
 use crate::log::{dev_info, user_warn};
 use crate::pam::{CLIConverser, Converser, PamContext, PamError, PamErrorType, PamResult};
 use crate::system::term::current_tty_name;
 
 use super::pipeline::AuthPlugin;
 
-type PamBuilder<C> = dyn Fn(&Context) -> PamResult<PamContext<C>>;
+type PamBuilder<C> = dyn Fn(&Context, AuthUser) -> PamResult<PamContext<C>>;
 
 pub struct PamAuthenticator<C: Converser> {
     builder: Box<PamBuilder<C>>,
@@ -17,7 +18,7 @@ pub struct PamAuthenticator<C: Converser> {
 
 impl<C: Converser> PamAuthenticator<C> {
     fn new(
-        initializer: impl Fn(&Context) -> PamResult<PamContext<C>> + 'static,
+        initializer: impl Fn(&Context, AuthUser) -> PamResult<PamContext<C>> + 'static,
     ) -> PamAuthenticator<C> {
         PamAuthenticator {
             builder: Box::new(initializer),
@@ -28,23 +29,23 @@ impl<C: Converser> PamAuthenticator<C> {
 
 impl PamAuthenticator<CLIConverser> {
     pub fn new_cli() -> PamAuthenticator<CLIConverser> {
-        PamAuthenticator::new(|context| {
+        PamAuthenticator::new(|context, auth_user| {
             init_pam(
                 matches!(context.launch, LaunchType::Login),
                 matches!(context.launch, LaunchType::Shell),
                 context.stdin,
                 context.non_interactive,
                 context.password_feedback,
-                &context.auth_user.name,
+                &auth_user.name,
                 &context.current_user.name,
             )
         })
     }
 }
 
 impl<C: Converser> AuthPlugin for PamAuthenticator<C> {
-    fn init(&mut self, context: &Context) -> Result<(), Error> {
-        self.pam = Some((self.builder)(context)?);
+    fn init(&mut self, context: &Context, auth_user: AuthUser) -> Result<(), Error> {
+        self.pam = Some((self.builder)(context, auth_user)?);
         Ok(())
     }