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

Fix test suite failures with newer sudo versions #929

Merged
merged 7 commits into from
Dec 10, 2024
11 changes: 11 additions & 0 deletions test-framework/sudo-compliance-tests/src/sudo/env_reset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ fn some_vars_are_set() -> Result<()> {
// "Set to the login name of the user who invoked sudo"
assert_eq!(Some("root"), sudo_env.remove("SUDO_USER"));

// "Set to the home directory of the user who invoked sudo."
if let Some(val) = sudo_env.remove("SUDO_HOME") {
assert_eq!("/root", val);
}

// "Set to the same value as LOGNAME"
assert_eq!(Some("root"), sudo_env.remove("USER"));

Expand Down Expand Up @@ -145,6 +150,9 @@ fn user_dependent_vars() -> Result<()> {
assert_eq!(Some("0"), sudo_env.remove("SUDO_GID"));
assert_eq!(Some("0"), sudo_env.remove("SUDO_UID"));
assert_eq!(Some("root"), sudo_env.remove("SUDO_USER"));
if let Some(val) = sudo_env.remove("SUDO_HOME") {
assert_eq!("/root", val);
}

assert_eq!(Some(SUDO_ENV_DEFAULT_PATH), sudo_env.remove("PATH"));
assert_eq!(Some(SUDO_ENV_DEFAULT_TERM), sudo_env.remove("TERM"));
Expand Down Expand Up @@ -209,6 +217,9 @@ fn some_vars_are_preserved() -> Result<()> {
assert_eq!(Some("root"), sudo_env.remove("SUDO_USER"));
assert_eq!(Some("0"), sudo_env.remove("SUDO_UID"));
assert_eq!(Some("0"), sudo_env.remove("SUDO_GID"));
if let Some(val) = sudo_env.remove("SUDO_HOME") {
assert_eq!("/root", val);
}

// preserved
assert_eq!(Some(display), sudo_env.remove("DISPLAY"));
Expand Down
77 changes: 64 additions & 13 deletions test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,37 +129,88 @@ fn cwd_set_to_non_glob_value_then_cannot_use_that_path_with_chdir_flag() -> Resu
}

#[test]
#[ignore = "wontfix"]
fn any_chdir_value_is_accepted_if_it_matches_pwd_cwd_unset() -> Result<()> {
fn any_chdir_value_is_not_accepted_if_it_matches_pwd_cwd_unset() -> Result<()> {
let path = "/root";
let env = Env("ALL ALL=(ALL:ALL) NOPASSWD: ALL").build()?;
let stdout = Command::new("sh")

if sudo_test::is_original_sudo() {
let stdout = Command::new("sudo")
.arg("--version")
.output(&env)?
.stdout()?;
let version = stdout
.lines()
.next()
.unwrap()
.strip_prefix("Sudo version ")
.unwrap();
if version < "1.9.14" {
// Older sudo had a special case where --chdir is accepted if it matches the cwd even if
// it would otherwise be denied.
// FIXME remove once bookworm is oldstable
return Ok(());
}
}

let output = Command::new("sh")
.arg("-c")
.arg(format!("cd {path}; sudo --chdir {path} pwd"))
.output(&env)?
.stdout()?;
.output(&env)?;

assert_eq!(path, stdout);
assert!(!output.status().success());
assert_eq!(Some(1), output.status().code());

let diagnostic = if sudo_test::is_original_sudo() {
format!("you are not permitted to use the -D option with {BIN_PWD}")
} else {
format!("you are not allowed to use '--chdir {path}' with '{BIN_PWD}'")
};
assert_contains!(output.stderr(), diagnostic);

Ok(())
}

// NOTE unclear if we want to adopt this behavior
#[test]
#[ignore = "wontfix"]
fn any_chdir_value_is_accepted_if_it_matches_pwd_cwd_set() -> Result<()> {
fn any_chdir_value_is_not_accepted_if_it_matches_pwd_cwd_set() -> Result<()> {
let cwd_path = "/root";
let another_path = "/tmp";
let env = Env(format!("ALL ALL=(ALL:ALL) CWD={cwd_path} NOPASSWD: ALL")).build()?;
let stdout = Command::new("sh")

if sudo_test::is_original_sudo() {
let stdout = Command::new("sudo")
.arg("--version")
.output(&env)?
.stdout()?;
let version = stdout
.lines()
.next()
.unwrap()
.strip_prefix("Sudo version ")
.unwrap();
if version < "1.9.14" {
// Older sudo had a special case where --chdir is accepted if it matches the cwd even if
// it would otherwise be denied.
// FIXME remove once bookworm is oldstable
return Ok(());
}
}

let output = Command::new("sh")
.arg("-c")
.arg(format!(
"cd {another_path}; sudo --chdir {another_path} pwd"
))
.output(&env)?
.stdout()?;
.output(&env)?;

assert!(!output.status().success());
assert_eq!(Some(1), output.status().code());

assert_eq!(cwd_path, stdout);
let diagnostic = if sudo_test::is_original_sudo() {
format!("you are not permitted to use the -D option with {BIN_PWD}")
} else {
format!("you are not allowed to use '--chdir {another_path}' with '{BIN_PWD}'")
};
assert_contains!(output.stderr(), diagnostic);

Ok(())
}
Expand Down
7 changes: 5 additions & 2 deletions test-framework/sudo-compliance-tests/src/sudo/flag_list.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use sudo_test::{Command, Env, TextFile, User, BIN_FALSE, BIN_LS, BIN_PWD, BIN_TRUE};
use sudo_test::{Command, Env, TextFile, User, BIN_FALSE, BIN_LS, BIN_PWD, BIN_TRUE, ETC_SUDOERS};

use crate::{Result, PANIC_EXIT_CODE, PASSWORD, SUDOERS_ALL_ALL_NOPASSWD, USERNAME};

Expand Down Expand Up @@ -254,7 +254,10 @@ Sudoers entry:
\tALL"
);
let actual = output.stdout()?;
assert_eq!(actual, expected);
assert_eq!(
actual.replace(&format!("Sudoers entry: {ETC_SUDOERS}"), "Sudoers entry:"),
expected
);

Ok(())
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use sudo_test::{Command, Env, BIN_FALSE, BIN_LS, BIN_TRUE};
use sudo_test::{Command, Env, BIN_FALSE, BIN_LS, BIN_TRUE, ETC_SUDOERS};

use crate::{Result, HOSTNAME};

macro_rules! assert_snapshot {
($($tt:tt)*) => {
insta::with_settings!({
filters => vec![(BIN_LS, "<BIN_LS>")],
filters => vec![
(BIN_LS, "<BIN_LS>"),
(&format!("Sudoers entry: {ETC_SUDOERS}"), "Sudoers entry:"),
],
prepend_module_to_snapshot => false,
}, {
insta::assert_snapshot!($($tt)*)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use crate::{Result, USERNAME};

#[test]
fn when_other_user_is_self() -> Result<()> {
let env = Env("ALL ALL=(ALL:ALL) ALL").user(USERNAME).build()?;
let env = Env("Defaults !lecture
ALL ALL=(ALL:ALL) ALL").user(USERNAME).build()?;

let output = Command::new("sudo")
.args(["-S", "-l", "-U", USERNAME])
Expand All @@ -28,7 +29,8 @@ fn when_other_user_is_self() -> Result<()> {
fn other_user_has_nopasswd_tag() -> Result<()> {
let other_user = "ghost";
let env = Env(format!(
"{other_user} ALL=(ALL:ALL) NOPASSWD: ALL
"Defaults !lecture
{other_user} ALL=(ALL:ALL) NOPASSWD: ALL
{USERNAME} ALL=(ALL:ALL) ALL"
))
.user(USERNAME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,16 @@ fn flag_uppercase_u_plus_command() -> Result<()> {
assert!(!output.status().success());
assert_eq!(Some(1), output.status().code());

let command = if sudo_test::is_original_sudo() {
"list/usr/bin/true"
} else {
"list true"
};
let diagnostic =
format!("Sorry, user {USERNAME} is not allowed to execute '{command}' as {other_user} on {hostname}.");
assert_contains!(output.stderr(), diagnostic);
// This is the output of older sudo versions
if !output.stderr().contains(&format!(
"Sorry, user {USERNAME} is not allowed to execute 'list/usr/bin/true' \
as {other_user} on {hostname}."
)) {
// This is the output of newer sudo versions and sudo-rs
let diagnostic =
format!("Sorry, user {USERNAME} is not allowed to execute 'list true' as {other_user} on {hostname}.");
assert_contains!(output.stderr(), diagnostic);
}
}
}

Expand Down
22 changes: 0 additions & 22 deletions test-framework/sudo-compliance-tests/src/sudo/lecture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,28 +117,6 @@ fn negation_equals_never() -> Result<()> {
Ok(())
}

#[test]
fn double_negation_also_equals_never() -> Result<()> {
let env = Env([
SUDOERS_ROOT_ALL,
SUDOERS_USER_ALL_ALL,
"Defaults !!lecture",
])
.user(User(USERNAME).password(PASSWORD))
.build()?;

let output = Command::new("sudo")
.args(["-S", "true"])
.as_user(USERNAME)
.stdin(PASSWORD)
.output(&env)?;

assert!(output.status().success());
assert_not_contains!(output.stderr(), OG_SUDO_STANDARD_LECTURE);

Ok(())
}

/// Lectures are only shown when password is asked for
#[test]
fn root_user_lecture_not_shown() -> Result<()> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ fn include_loop_error_messages() -> Result<()> {
assert!(!output.status().success());
assert_eq!(Some(1), output.status().code());
let diagnostic = if sudo_test::is_original_sudo() {
"sudo: /etc/sudoers2: too many levels of includes"
"/etc/sudoers2: too many levels of includes"
} else {
"sudo-rs: include file limit reached opening '/etc/sudoers2'"
};
Expand All @@ -145,7 +145,7 @@ fn include_loop_not_fatal() -> Result<()> {

assert!(output.status().success());
let diagnostic = if sudo_test::is_original_sudo() {
"sudo: /etc/sudoers2: too many levels of includes"
"/etc/sudoers2: too many levels of includes"
} else {
"sudo-rs: include file limit reached opening '/etc/sudoers2'"
};
Expand Down
28 changes: 5 additions & 23 deletions test-framework/sudo-compliance-tests/src/sudo/sudoers/run_as.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,6 @@ macro_rules! assert_snapshot {
}

// "If both Runas_Lists are empty, the command may only be run as the invoking user."
#[test]
#[ignore = "gh134"]
fn when_empty_then_implicit_as_self_is_allowed() -> Result<()> {
let env = Env("ALL ALL=() NOPASSWD: ALL").user(USERNAME).build()?;

for user in ["root", USERNAME] {
Command::new("sudo")
.args(["true"])
.as_user(user)
.output(&env)?
.assert_success()?;
}

Ok(())
}

#[test]
fn when_empty_then_explicit_as_self_is_allowed() -> Result<()> {
let env = Env("ALL ALL=() NOPASSWD: ALL").user(USERNAME).build()?;
Expand Down Expand Up @@ -293,13 +277,11 @@ fn when_both_user_and_group_are_specified_then_as_that_group_is_allowed() -> Res
.group(GROUPNAME)
.build()?;

for user in ["root", USERNAME] {
Command::new("sudo")
.args(["-g", GROUPNAME, "true"])
.as_user(user)
.output(&env)?
.assert_success()?;
}
Command::new("sudo")
.args(["-g", GROUPNAME, "true"])
.as_user(USERNAME)
.output(&env)?
.assert_success()?;

Ok(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ fn when_only_username_is_given_group_arg_fails() -> Result<()> {
fn user_and_group_works_when_one_is_passed_as_arg() -> Result<()> {
let env = Env([
&format!("Runas_Alias OP = otheruser, {GROUPNAME}"),
&format!("{USERNAME} ALL = (OP:OP) NOPASSWD: ALL"),
&format!("{USERNAME} ALL = (OP,{USERNAME}:OP) NOPASSWD: ALL"),
])
.user(User(USERNAME))
.user(User("otheruser"))
Expand Down Expand Up @@ -346,7 +346,7 @@ fn different_aliases_user_and_group_works_when_one_is_passed_as_arg() -> Result<
let env = Env([
&format!("Runas_Alias GROUPALIAS = {GROUPNAME}"),
("Runas_Alias USERALIAS = otheruser"),
&format!("{USERNAME} ALL = (USERALIAS:GROUPALIAS) NOPASSWD: ALL"),
"ALL ALL = (USERALIAS:GROUPALIAS) NOPASSWD: ALL",
])
.user(USERNAME)
.user("otheruser")
Expand All @@ -361,7 +361,7 @@ fn different_aliases_user_and_group_works_when_one_is_passed_as_arg() -> Result<

Command::new("sudo")
.args(["-g", GROUPNAME, "true"])
.as_user(USERNAME)
.as_user("otheruser")
.output(&env)?
.assert_success()?;

Expand Down
24 changes: 24 additions & 0 deletions test-framework/sudo-compliance-tests/src/sudo/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,27 @@ fn cached_credential_not_shared_with_self_across_ttys() -> Result<()> {

Ok(())
}

#[test]
fn double_negation_also_equals_never() -> Result<()> {
let env = Env([
"Defaults !!use_pty".to_string(),
format!("{USERNAME} ALL=(ALL:ALL) ALL"),
])
.user(User(USERNAME).password(PASSWORD))
.build()?;

let output = Command::new("sh")
.arg("-c")
.arg(format!(
"echo {PASSWORD} | sudo -S true; sudo -u {USERNAME} sudo -n true && true"
))
.as_user(USERNAME)
.tty(true)
.output(&env)?;

assert!(!output.status().success());
assert_eq!(Some(1), output.status().code());

Ok(())
}
Loading
Loading