Skip to content
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

Add SETENV support #932

Merged
merged 8 commits into from
Dec 18, 2024
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
43 changes: 28 additions & 15 deletions src/sudo/env/environment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::{
collections::{hash_map::Entry, HashMap, HashSet},
collections::{HashMap, HashSet},
ffi::{OsStr, OsString},
os::unix::prelude::OsStrExt,
};
Expand Down Expand Up @@ -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")),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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};
Expand Down
33 changes: 18 additions & 15 deletions src/sudo/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)?;
}
}

Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/sudo/pipeline/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(()))
}

Expand Down
60 changes: 57 additions & 3 deletions src/sudoers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)| {
Expand All @@ -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()))),
};
Expand Down
11 changes: 10 additions & 1 deletion src/sudoers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
},
)
}
Expand Down
5 changes: 5 additions & 0 deletions src/sudoers/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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),
})
Expand Down Expand Up @@ -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"]),
})
Expand Down Expand Up @@ -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),
})
Expand All @@ -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),
})
Expand Down
Loading
Loading