Fix test suite failures with newer sudo versions (#929)

trifectatechfoundation · Dec 10, 2024 · 491e667 · 491e667
2 parents e972da0 + db99e38
commit 491e667
Show file tree

Hide file tree

Showing 20 changed files with 172 additions and 117 deletions.
diff --git a/test-framework/sudo-compliance-tests/src/sudo/env_reset.rs b/test-framework/sudo-compliance-tests/src/sudo/env_reset.rs
@@ -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"));
 
@@ -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"));
@@ -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"));

diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs
@@ -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(())
 }

diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list.rs
@@ -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};
 
@@ -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(())
 }

diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs
@@ -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)*)

diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/needs_auth.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list/needs_auth.rs
@@ -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])
@@ -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)

diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/not_allowed.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list/not_allowed.rs
@@ -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);
+            }
         }
     }
 

diff --git a/test-framework/sudo-compliance-tests/src/sudo/lecture.rs b/test-framework/sudo-compliance-tests/src/sudo/lecture.rs
@@ -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<()> {

diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudoers/include.rs b/test-framework/sudo-compliance-tests/src/sudo/sudoers/include.rs
@@ -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'"
     };
@@ -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'"
     };

diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudoers/run_as.rs b/test-framework/sudo-compliance-tests/src/sudo/sudoers/run_as.rs
@@ -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()?;
@@ -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(())
 }

diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudoers/runas_alias.rs b/test-framework/sudo-compliance-tests/src/sudo/sudoers/runas_alias.rs
@@ -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"))
@@ -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")
@@ -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()?;
 

diff --git a/test-framework/sudo-compliance-tests/src/sudo/timestamp.rs b/test-framework/sudo-compliance-tests/src/sudo/timestamp.rs
@@ -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(())
+}