Add SETENV support (#932)

src/sudo/env/environment.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::{hash_map::Entry, HashMap, HashSet},
+    collections::{HashMap, HashSet},
     ffi::{OsStr, OsString},
     os::unix::prelude::OsStrExt,
 };
@@ -63,17 +63,19 @@ fn add_extra_env(
     );
     environment.insert("SUDO_USER".into(), context.current_user.name.clone().into());
     // target user
-    if let Entry::Vacant(entry) = environment.entry("MAIL".into()) {
-        entry.insert(format!("{PATH_MAILDIR}/{}", context.target_user.name).into());
-    }
+    environment
+        .entry("MAIL".into())
+        .or_insert_with(|| format!("{PATH_MAILDIR}/{}", context.target_user.name).into());
     // The current SHELL variable should determine the shell to run when -s is passed, if none set use passwd entry
-    environment.insert("SHELL".into(), context.target_user.shell.clone().into());
+    environment
+        .entry("SHELL".into())
+        .or_insert_with(|| context.target_user.shell.clone().into());
     // HOME' Set to the home directory of the target user if -i or -H are specified, env_reset or always_set_home are
     // set in sudoers, or when the -s option is specified and set_home is set in sudoers.
     // Since we always want to do env_reset -> always set HOME
-    if let Entry::Vacant(entry) = environment.entry("HOME".into()) {
-        entry.insert(context.target_user.home.clone().into());
-    }
+    environment
+        .entry("HOME".into())
+        .or_insert_with(|| context.target_user.home.clone().into());
 
     match (
         environment.get(OsStr::new("LOGNAME")),
@@ -104,14 +106,13 @@ fn add_extra_env(
         environment.insert("PATH".into(), secure_path.into());
     }
     // If the PATH and TERM variables are not preserved from the user's environment, they will be set to default value
-    if !environment.contains_key(OsStr::new("PATH")) {
-        // If the PATH variable is not set, it will be set to default value
-        environment.insert("PATH".into(), PATH_DEFAULT.into());
-    }
+    environment
+        .entry("PATH".into())
+        .or_insert_with(|| PATH_DEFAULT.into());
     // If the TERM variable is not preserved from the user's environment, it will be set to default value
-    if !environment.contains_key(OsStr::new("TERM")) {
-        environment.insert("TERM".into(), "unknown".into());
-    }
+    environment
+        .entry("TERM".into())
+        .or_insert_with(|| "unknown".into());
     // The SUDO_PS1 variable requires special treatment as the PS1 variable must be set in the
     // target environment to the same value of SUDO_PS1 if the latter is set.
     if let Some(sudo_ps1_value) = sudo_ps1 {
@@ -238,6 +239,18 @@ pub fn get_target_environment(
     Ok(environment)
 }
 
+/// Extend the environment with user-supplied info
+pub fn dangerous_extend<S>(env: &mut Environment, user_override: impl IntoIterator<Item = (S, S)>)
+where
+    S: Into<OsString>,
+{
+    env.extend(
+        user_override
+            .into_iter()
+            .map(|(key, value)| (key.into(), value.into())),
+    )
+}
+
 #[cfg(test)]
 mod tests {
     use super::{is_safe_tz, should_keep, PATH_ZONEINFO};

src/sudo/pipeline.rs

@@ -56,31 +56,33 @@ impl<Policy: PolicyPlugin, Auth: AuthPlugin> Pipeline<Policy, Auth> {
         let mut context = build_context(ctx_opts, &pre)?;
 
         let policy = self.policy.judge(pre, &context)?;
-        let authorization = policy.authorization();
 
-        match authorization {
-            Authorization::Forbidden => {
-                return Err(Error::Authorization(context.current_user.name.to_string()));
-            }
-            Authorization::Allowed(auth) => {
-                self.apply_policy_to_context(&mut context, &policy)?;
-                self.auth_and_update_record_file(&context, auth)?;
-            }
-        }
+        let Authorization::Allowed(auth) = policy.authorization() else {
+            return Err(Error::Authorization(context.current_user.name.to_string()));
+        };
+        self.apply_policy_to_context(&mut context, &policy)?;
+        self.auth_and_update_record_file(&context, &auth)?;
 
         // build environment
         let additional_env = self.authenticator.pre_exec(&context.target_user.name)?;
 
         let current_env = environment::system_environment();
+        let (checked_vars, trusted_vars) = if auth.trust_environment {
+            (vec![], pipe_opts.user_requested_env_vars)
+        } else {
+            (pipe_opts.user_requested_env_vars, vec![])
+        };
 
-        let target_env = environment::get_target_environment(
+        let mut target_env = environment::get_target_environment(
             current_env,
             additional_env,
-            pipe_opts.user_requested_env_vars,
+            checked_vars,
             &context,
             &policy,
         )?;
 
+        environment::dangerous_extend(&mut target_env, trusted_vars);
+
         let pid = context.process.pid;
 
         // run command and return corresponding exit code
@@ -122,7 +124,7 @@ impl<Policy: PolicyPlugin, Auth: AuthPlugin> Pipeline<Policy, Auth> {
                 return Err(Error::Authorization(context.current_user.name.to_string()));
             }
             Authorization::Allowed(auth) => {
-                self.auth_and_update_record_file(&context, auth)?;
+                self.auth_and_update_record_file(&context, &auth)?;
             }
         }
 
@@ -132,11 +134,12 @@ impl<Policy: PolicyPlugin, Auth: AuthPlugin> Pipeline<Policy, Auth> {
     fn auth_and_update_record_file(
         &mut self,
         context: &Context,
-        AuthorizationAllowed {
+        &AuthorizationAllowed {
             must_authenticate,
             prior_validity,
             allowed_attempts,
-        }: AuthorizationAllowed,
+            ..
+        }: &AuthorizationAllowed,
     ) -> Result<(), Error> {
         let scope = RecordScope::for_process(&Process::new());
         let mut auth_status = determine_auth_status(

src/sudo/pipeline/list.rs

@@ -82,7 +82,7 @@ impl Pipeline<SudoersPolicy, PamAuthenticator<CLIConverser>> {
             sudoers.check_list_permission(&*context.current_user, &context.hostname, list_request);
         match judgement.authorization() {
             Authorization::Allowed(auth) => {
-                self.auth_and_update_record_file(context, auth)?;
+                self.auth_and_update_record_file(context, &auth)?;
                 Ok(ControlFlow::Continue(()))
             }
 

src/sudoers/ast.rs

@@ -8,7 +8,8 @@ use crate::common::{
 };
 
 /// The Sudoers file allows negating items with the exclamation mark.
-#[cfg_attr(test, derive(Debug, PartialEq, Eq))]
+#[cfg_attr(test, derive(Debug, Eq))]
+#[derive(Clone, PartialEq)]
 #[repr(u32)]
 pub enum Qualified<T> {
     Allow(T) = HARDENED_ENUM_VALUE_0,
@@ -80,18 +81,35 @@ pub enum Authenticate {
     Nopasswd = HARDENED_ENUM_VALUE_2,
 }
 
+#[derive(Copy, Clone, Default, PartialEq)]
+#[cfg_attr(test, derive(Debug, Eq))]
+#[repr(u32)]
+pub enum EnvironmentControl {
+    #[default]
+    Implicit = HARDENED_ENUM_VALUE_0,
+    // PASSWD:
+    Setenv = HARDENED_ENUM_VALUE_1,
+    // NOPASSWD:
+    Nosetenv = HARDENED_ENUM_VALUE_2,
+}
+
 /// Commands in /etc/sudoers can have attributes attached to them, such as NOPASSWD, NOEXEC, ...
 #[derive(Default, Clone, PartialEq)]
 #[cfg_attr(test, derive(Debug, Eq))]
 pub struct Tag {
-    pub authenticate: Authenticate,
-    pub cwd: Option<ChDir>,
+    pub(super) authenticate: Authenticate,
+    pub(super) cwd: Option<ChDir>,
+    pub(super) env: EnvironmentControl,
 }
 
 impl Tag {
     pub fn needs_passwd(&self) -> bool {
         matches!(self.authenticate, Authenticate::None | Authenticate::Passwd)
     }
+
+    pub fn allows_setenv(&self) -> bool {
+        matches!(self.env, EnvironmentControl::Setenv)
+    }
 }
 
 /// Commands with attached attributes.
@@ -337,6 +355,8 @@ pub type Modifier = Box<dyn Fn(&mut Tag)>;
 impl Parse for MetaOrTag {
     fn parse(stream: &mut impl CharStream) -> Parsed<Self> {
         use Meta::*;
+
+        let start_pos = stream.get_pos();
         let AliasName(keyword) = try_nonterminal(stream)?;
 
         let mut switch = |modifier: fn(&mut Tag)| {
@@ -345,13 +365,47 @@ impl Parse for MetaOrTag {
         };
 
         let result: Modifier = match keyword.as_str() {
+            // we do not support these, and that should make sudo-rs "fail safe"
+            "INTERCEPT" | "NOEXEC" => unrecoverable!(
+                pos = start_pos,
+                stream,
+                "NOEXEC and INTERCEPT are not supported by sudo-rs"
+            ),
+            // this is less fatal
+            "LOG_INPUT" | "NOLOG_INPUT" | "LOG_OUTPUT" | "NOLOG_OUTPUT" | "MAIL" | "NOMAIL" => {
+                eprintln_ignore_io_error!(
+                    "warning: {} tags are ignored by sudo-rs",
+                    keyword.as_str()
+                );
+                switch(|_| {})?
+            }
+
+            // 'FOLLOW' and 'NOFOLLOW' are only usable in a sudoedit context, which will result in
+            // a parse error elsewhere. 'EXEC' and 'NOINTERCEPT' are the default behaviour.
+            "FOLLOW" | "NOFOLLOW" | "EXEC" | "NOINTERCEPT" => switch(|_| {})?,
+
+            "SETENV" => switch(|tag| tag.env = EnvironmentControl::Setenv)?,
+            "NOSETENV" => switch(|tag| tag.env = EnvironmentControl::Nosetenv)?,
             "PASSWD" => switch(|tag| tag.authenticate = Authenticate::Passwd)?,
             "NOPASSWD" => switch(|tag| tag.authenticate = Authenticate::Nopasswd)?,
+
             "CWD" => {
                 expect_syntax('=', stream)?;
                 let path: ChDir = expect_nonterminal(stream)?;
                 Box::new(move |tag| tag.cwd = Some(path.clone()))
             }
+            // we do not support these, and that should make sudo-rs "fail safe"
+            spec @ ("CHROOT" | "TIMEOUT" | "NOTBEFORE" | "NOTAFTER") => unrecoverable!(
+                pos = start_pos,
+                stream,
+                "{spec} is not supported by sudo-rs"
+            ),
+            "ROLE" | "TYPE" => unrecoverable!(
+                pos = start_pos,
+                stream,
+                "role based access control is not yet supported by sudo-rs"
+            ),
+
             "ALL" => return make(MetaOrTag(All)),
             alias => return make(MetaOrTag(Alias(alias.to_string()))),
         };

src/sudoers/mod.rs

@@ -348,7 +348,16 @@ fn distribute_tags(
                 f(tag);
             }
 
-            Some((last_runas, (tag.clone(), cmd)))
+            let this_tag = match cmd {
+                Qualified::Allow(Meta::All) if tag.env != EnvironmentControl::Nosetenv => Tag {
+                    // "ALL" has an implicit "SETENV" that doesn't distribute
+                    env: EnvironmentControl::Setenv,
+                    ..tag.clone()
+                },
+                _ => tag.clone(),
+            };
+
+            Some((last_runas, (this_tag, cmd)))
         },
     )
 }

src/sudoers/policy.rs

@@ -40,6 +40,7 @@ pub struct AuthorizationAllowed {
     pub must_authenticate: bool,
     pub allowed_attempts: u16,
     pub prior_validity: Duration,
+    pub trust_environment: bool,
 }
 
 #[must_use]
@@ -57,6 +58,7 @@ impl Policy for Judgement {
             let valid_seconds = self.settings.int_value["timestamp_timeout"];
             Authorization::Allowed(AuthorizationAllowed {
                 must_authenticate: tag.needs_passwd(),
+                trust_environment: tag.allows_setenv(),
                 allowed_attempts,
                 prior_validity: Duration::seconds(valid_seconds),
             })
@@ -107,6 +109,7 @@ impl PreJudgementPolicy for Sudoers {
     fn validate_authorization(&self) -> Authorization {
         Authorization::Allowed(AuthorizationAllowed {
             must_authenticate: true,
+            trust_environment: false,
             allowed_attempts: self.settings.int_value["passwd_tries"].try_into().unwrap(),
             prior_validity: Duration::seconds(self.settings.int_value["timestamp_timeout"]),
         })
@@ -139,6 +142,7 @@ mod test {
             judge.authorization(),
             Authorization::Allowed(AuthorizationAllowed {
                 must_authenticate: true,
+                trust_environment: false,
                 allowed_attempts: 3,
                 prior_validity: Duration::minutes(15),
             })
@@ -148,6 +152,7 @@ mod test {
             judge.authorization(),
             Authorization::Allowed(AuthorizationAllowed {
                 must_authenticate: false,
+                trust_environment: false,
                 allowed_attempts: 3,
                 prior_validity: Duration::minutes(15),
             })