Repository: memorysafety/sudo-rs
Pull request #130 (2023-03-21 17:05:46)

Add parser for integer 'Default' values
What it says, this allows the parser to accept integer Default values.

This also adds support for restricting the range of accepted inputs for integer values (and their radix); this turns out to be necessary: for example, a umask value is always parsed as octal (i.e. not all integer values are "the same"). The parser itself should not be bothered with all those minutiae.

Still need to check if there are cases of hexadecimal integers in sudoers.


Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index ff7e4489..9aa0c661 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -266,7 +266,7 @@ jobs:
       - name: Rust Cache
         uses: Swatinem/rust-cache@v2
         with:
-          shared-key: "stable"
+          shared-key: "audit"
       - name: Run audit
         uses: actions-rs/cargo@v1
         with:
diff --git a/lib/sudo-common/src/context.rs b/lib/sudo-common/src/context.rs
index 3bb481fe..58fc79c6 100644
--- a/lib/sudo-common/src/context.rs
+++ b/lib/sudo-common/src/context.rs
@@ -102,14 +102,19 @@ fn resolve_current_user() -> Result<User, Error> {
 }
 
 fn resolve_target_user(target_name_or_id: &Option<String>) -> Result<User, Error> {
+    let is_default = target_name_or_id.is_none();
     let target_name_or_id = target_name_or_id.as_deref().unwrap_or("root");
 
-    match NameOrId::parse(target_name_or_id) {
+    let mut user = match NameOrId::parse(target_name_or_id) {
         Some(NameOrId::Name(name)) => User::from_name(name)?,
         Some(NameOrId::Id(uid)) => User::from_uid(uid)?,
         _ => None,
     }
-    .ok_or_else(|| Error::UserNotFound(target_name_or_id.to_string()))
+    .ok_or_else(|| Error::UserNotFound(target_name_or_id.to_string()))?;
+
+    user.is_default = is_default;
+
+    Ok(user)
 }
 
 fn resolve_target_group(
@@ -198,6 +203,7 @@ mod tests {
             shell: "/bin/sh".to_string(),
             passwd: String::new(),
             groups: None,
+            is_default: false,
         };
 
         assert_eq!(
diff --git a/lib/sudo-defaults/src/lib.rs b/lib/sudo-defaults/src/lib.rs
index 8925d1a1..2b10828a 100644
--- a/lib/sudo-defaults/src/lib.rs
+++ b/lib/sudo-defaults/src/lib.rs
@@ -1,10 +1,9 @@
 // FUTURE IDEA: use a representation that allows for more Rust-type structure rather than passing
 // strings around; some settings in sudoers file are more naturally represented like that, such as
 // "verifypw" and "logfile"
-#[derive(Debug)]
 pub enum SudoDefault {
     Flag(bool),
-    Integer(OptTuple<i128>),
+    Integer(OptTuple<i128>, fn(&str) -> Option<i128>),
     Text(OptTuple<&'static str>),
     List(&'static [&'static str]),
     Enum(OptTuple<StrEnum<'static>>),
@@ -32,7 +31,7 @@ defaults! {
     visiblepw                 = false
 
     passwd_tries              = 3
-    umask                     = 0o22 (!= 0o777)
+    umask                     = 0o22 (!= 0o777)    [0..=0o777; radix: 8]
 
     editor                    = "/usr/bin/editor"
     lecture_file              = ""
@@ -82,8 +81,8 @@ mod test {
         test! { match_group_by_gid => Flag(false) };
         test! { use_pty => Flag(false) };
         test! { visiblepw => Flag(false) };
-        test! { passwd_tries => Integer(OptTuple { default: 3, negated: None }) };
-        test! { umask => Integer(OptTuple { default: 18, negated: Some(511) }) };
+        test! { passwd_tries => Integer(OptTuple { default: 3, negated: None }, _) };
+        test! { umask => Integer(OptTuple { default: 18, negated: Some(511) }, _) };
         test! { editor => Text(OptTuple { default: "/usr/bin/editor", negated: None }) };
         test! { lecture_file => Text(_) };
         test! { secure_path => Text(OptTuple { default: "", negated: Some("") }) };
diff --git a/lib/sudo-defaults/src/settings_dsl.rs b/lib/sudo-defaults/src/settings_dsl.rs
index 6f764f00..fb50e19c 100644
--- a/lib/sudo-defaults/src/settings_dsl.rs
+++ b/lib/sudo-defaults/src/settings_dsl.rs
@@ -7,13 +7,13 @@ macro_rules! add_from {
         }
     };
 
-    ($ctor:ident, $type:ty, negatable) => {
+    ($ctor:ident, $type:ty, negatable$(, $vetting_function:expr)?) => {
         impl From<$type> for $crate::SudoDefault {
             fn from(value: $type) -> Self {
                 $crate::SudoDefault::$ctor(OptTuple {
                     default: value,
                     negated: None,
-                })
+                }$(, $vetting_function)?)
             }
         }
 
@@ -22,7 +22,7 @@ macro_rules! add_from {
                 $crate::SudoDefault::$ctor(OptTuple {
                     default: value,
                     negated: Some(neg),
-                })
+                }$(, $vetting_function)?)
             }
         }
     };
@@ -56,14 +56,19 @@ macro_rules! optional {
 }
 
 macro_rules! defaults {
-    ($($name:ident = $value:tt $((!= $negate:tt))? $([$($key:ident),*])?)*) => {
+    ($($name:ident = $value:tt $((!= $negate:tt))? $([$($key:ident),*])? $([$first:literal ..= $last:literal$(; radix: $radix: expr)?])?)*) => {
         pub const ALL_PARAMS: &'static [&'static str] = &[
             $(stringify!($name)),*
         ];
 
+        // because of the nature of radix and ranges, 'let mut result' is not always necessary, and
+        // a radix of 10 can also not always be avoided (and for uniformity, I would also not avoid this
+        // if this was hand-written code.
+        #[allow(unused_mut)]
+        #[allow(clippy::from_str_radix_10)]
         pub fn sudo_default(var: &str) -> Option<SudoDefault> {
             add_from!(Flag, bool);
-            add_from!(Integer, i128, negatable);
+            add_from!(Integer, i128, negatable, |text| i128::from_str_radix(text, 10).ok());
             add_from!(Text, &'static str, negatable);
             add_from!(List, &'static [&'static str]);
             add_from!(Enum, StrEnum<'static>, negatable);
@@ -76,7 +81,13 @@ macro_rules! defaults {
                               |key: &'static str| StrEnum::new(key, keys).unwrap_or_else(|| unreachable!())
                           })?];
                           let datum = restrict(sliceify!($value));
-                          tupleify!(datum$(, restrict($negate))?).into()
+                          let mut result = tupleify!(datum$(, restrict($negate))?).into();
+                          $(
+                              if let SudoDefault::Integer(_, ref mut checker) = &mut result {
+                                  *checker = |text| i128::from_str_radix(text, 10$(*0 + $radix)?).ok().filter(|val| ($first ..= $last).contains(val));
+                              }
+                          )?
+                          result
                     },
                     )*
                     _ => return None
diff --git a/lib/sudo-exec/src/lib.rs b/lib/sudo-exec/src/lib.rs
index e1c4ab8b..4f173afe 100644
--- a/lib/sudo-exec/src/lib.rs
+++ b/lib/sudo-exec/src/lib.rs
@@ -31,7 +31,12 @@ pub fn run_command(ctx: Context<'_>, env: Environment) -> io::Result<ExitStatus>
     // reset env and set filtered environment
     command.args(ctx.command.arguments).env_clear().envs(env);
     // set target user and groups
-    set_target_user(&mut command, ctx.target_user, ctx.target_group);
+    set_target_user(
+        &mut command,
+        ctx.current_user,
+        ctx.target_user,
+        ctx.target_group,
+    );
     // spawn and exec to command
     let mut child = command.spawn()?;
 
diff --git a/lib/sudo-system/src/lib.rs b/lib/sudo-system/src/lib.rs
index bf5dcbad..40f745dd 100644
--- a/lib/sudo-system/src/lib.rs
+++ b/lib/sudo-system/src/lib.rs
@@ -26,12 +26,44 @@ pub fn hostname() -> String {
 }
 
 /// set target user and groups (uid, gid, additional groups) for a command
-pub fn set_target_user(cmd: &mut std::process::Command, target_user: User, target_group: Group) {
+pub fn set_target_user(
+    cmd: &mut std::process::Command,
+    current_user: User,
+    target_user: User,
+    target_group: Group,
+) {
     use std::os::unix::process::CommandExt;
 
-    let uid = target_user.uid;
-    let gid = target_group.gid;
-    let groups = target_user.groups.unwrap_or_default();
+    // means that we are using the default user because `-u` was not passed.
+    let user_is_default = target_user.is_default;
+    // means that we are using the principal gid of the target user because `-g` was not passed or
+    // was passed with the principal gid.
+    let group_is_default = target_user.uid == target_group.gid;
+
+    let (uid, gid, groups) = if group_is_default {
+        // no `-g`: We just set the uid, gid and groups using the target user.
+        (
+            target_user.uid,
+            target_user.gid,
+            target_user.groups.unwrap_or_default(),
+        )
+    } else if user_is_default {
+        //  `-g` and no `-u`: The set uid must be the one of the current user and the set groups
+        //  must be the ones of the current user extended with the target group gid.
+        let mut groups = current_user.groups.unwrap_or_default();
+        if !groups.contains(&target_group.gid) {
+            groups.push(target_group.gid);
+        }
+        (current_user.uid, target_group.gid, groups)
+    } else {
+        // `-g` and `-u`: The set uid must be the one of the target user and the set groups must be
+        // the ones of the target group extended with the target group gid.
+        let mut groups = target_user.groups.unwrap_or_default();
+        if !groups.contains(&target_group.gid) {
+            groups.push(target_group.gid);
+        }
+        (target_user.uid, target_group.gid, groups)
+    };
 
     // we need to do this in a `pre_exec` call since the `groups` method in `process::Command` is unstable
     // see https://github.com/rust-lang/rust/blob/a01b4cc9f375f1b95fa8195daeea938d3d9c4c34/library/std/src/sys/unix/process/process_unix.rs#L329-L352
@@ -70,6 +102,7 @@ pub struct User {
     pub shell: String,
     pub passwd: String,
     pub groups: Option<Vec<libc::gid_t>>,
+    pub is_default: bool,
 }
 
 impl User {
@@ -83,6 +116,7 @@ impl User {
             shell: unsafe { string_from_ptr(pwd.pw_shell) },
             passwd: unsafe { string_from_ptr(pwd.pw_passwd) },
             groups: None,
+            is_default: false,
         }
     }
 
diff --git a/lib/sudoers/src/ast.rs b/lib/sudoers/src/ast.rs
index ba9e47f2..afc7ea14 100644
--- a/lib/sudoers/src/ast.rs
+++ b/lib/sudoers/src/ast.rs
@@ -312,7 +312,10 @@ impl Parse for ProtoCommandSpec {
                 "sha256" => 256,
                 "sha384" => 384,
                 "sha512" => 512,
-                "sudoedit" => todo!(), // note: special behaviour of forward slashes in wildcards, tread carefully
+                "sudoedit" => {
+                    // note: special behaviour of forward slashes in wildcards, tread carefully
+                    unrecoverable!(stream, "sudoedit is not yet supported");
+                }
                 _ => unrecoverable!(
                     pos = start_pos,
                     stream,
@@ -583,9 +586,12 @@ fn get_directive(
                 Some(Setting::Enum(OptTuple {
                     negated: Some(val), ..
                 })) => ConfigValue::Enum(val),
-                Some(Setting::Integer(OptTuple {
-                    negated: Some(val), ..
-                })) => ConfigValue::Num(val),
+                Some(Setting::Integer(
+                    OptTuple {
+                        negated: Some(val), ..
+                    },
+                    _checker,
+                )) => ConfigValue::Num(val),
                 _ => unrecoverable!(
                     pos = value_pos,
                     stream,
@@ -604,11 +610,23 @@ fn get_directive(
             } else if is_syntax('-', stream)? {
                 list_items(Mode::Del, name, cfg, stream)
             } else if is_syntax('=', stream)? {
+                let value_pos = stream.get_pos();
                 match cfg {
                     Setting::Flag(_) => {
                         unrecoverable!(stream, "can't assign to boolean setting `{name}'")
                     }
-                    Setting::Integer(_) => todo!(),
+                    Setting::Integer(_, checker) => {
+                        let Numeric(denotation) = expect_nonterminal(stream)?;
+                        if let Some(value) = checker(&denotation) {
+                            make(Defaults(name, ConfigValue::Num(value)))
+                        } else {
+                            unrecoverable!(
+                                pos = value_pos,
+                                stream,
+                                "`{denotation}' is not a valid value for {name}"
+                            );
+                        }
+                    }
                     Setting::List(_) => {
                         let items = parse_vars(stream)?;
                         make(Defaults(name, ConfigValue::List(Mode::Set, items)))
@@ -618,10 +636,13 @@ fn get_directive(
                         make(Defaults(name, ConfigValue::Text(text)))
                     }
                     Setting::Enum(sudo_defaults::OptTuple { default: key, .. }) => {
-                        let value_pos = stream.get_pos();
                         let text = text_item(stream)?;
                         let Some(value) = key.alt(&text) else {
-                            unrecoverable!(pos = value_pos, stream, "`{text}' is not a valid value for {name}");
+                            unrecoverable!(
+                                pos = value_pos,
+                                stream,
+                                "`{text}' is not a valid value for {name}"
+                            );
                         };
                         make(Defaults(name, ConfigValue::Enum(value)))
                     }
diff --git a/lib/sudoers/src/ast_names.rs b/lib/sudoers/src/ast_names.rs
index 8935e19d..6fd417a6 100644
--- a/lib/sudoers/src/ast_names.rs
+++ b/lib/sudoers/src/ast_names.rs
@@ -15,7 +15,7 @@ mod names {
         const DESCRIPTION: &'static str = "number";
     }
 
-    impl UserFriendly for tokens::Decimal {
+    impl UserFriendly for tokens::Numeric {
         const DESCRIPTION: &'static str = "number";
     }
 
diff --git a/lib/sudoers/src/lib.rs b/lib/sudoers/src/lib.rs
index cf92454c..7a7ed70c 100644
--- a/lib/sudoers/src/lib.rs
+++ b/lib/sudoers/src/lib.rs
@@ -311,7 +311,7 @@ impl Default for Settings {
                 SudoDefault::Enum(OptTuple { default, .. }) => {
                     this.enum_value.insert(key.to_string(), default);
                 }
-                SudoDefault::Integer(OptTuple { default, .. }) => {
+                SudoDefault::Integer(OptTuple { default, .. }, _) => {
                     this.int_value.insert(key.to_string(), default);
                 }
                 SudoDefault::List(default) => {
diff --git a/lib/sudoers/src/tokens.rs b/lib/sudoers/src/tokens.rs
index 1bb9e07e..2682882b 100644
--- a/lib/sudoers/src/tokens.rs
+++ b/lib/sudoers/src/tokens.rs
@@ -39,19 +39,17 @@ impl Token for Digits {
 }
 
 #[derive(Debug)]
-pub struct Decimal(pub i32);
+pub struct Numeric(pub String);
+
+impl Token for Numeric {
+    const MAX_LEN: usize = 38;
 
-impl Token for Decimal {
     fn construct(s: String) -> Result<Self, String> {
-        Ok(Decimal(s.parse().unwrap()))
+        Ok(Numeric(s))
     }
 
     fn accept(c: char) -> bool {
-        c.is_ascii_digit()
-    }
-
-    fn accept_1st(c: char) -> bool {
-        c.is_ascii_digit() || "+-".contains(c)
+        c.is_ascii_hexdigit()
     }
 }
 


Test patch:
diff --git a/lib/sudo-env/tests/env_tests.rs b/lib/sudo-env/tests/env_tests.rs
index a0b57d51..bb62af0c 100644
--- a/lib/sudo-env/tests/env_tests.rs
+++ b/lib/sudo-env/tests/env_tests.rs
@@ -86,6 +86,7 @@ fn create_test_context<'a>(sudo_options: &'a SudoOptions) -> Context<'a> {
         shell: "/bin/sh".to_string(),
         passwd: String::new(),
         groups: None,
+        is_default: false,
     };
 
     let current_group = Group {
@@ -104,6 +105,7 @@ fn create_test_context<'a>(sudo_options: &'a SudoOptions) -> Context<'a> {
         shell: "/bin/bash".to_string(),
         passwd: String::new(),
         groups: None,
+        is_default: false,
     };
 
     let root_group = Group {
diff --git a/lib/sudoers/src/test/mod.rs b/lib/sudoers/src/test/mod.rs
index 75aaaa10..00f68aac 100644
--- a/lib/sudoers/src/test/mod.rs
+++ b/lib/sudoers/src/test/mod.rs
@@ -296,8 +296,8 @@ fn default_set_test() {
         "Defaults !env_check",
         "Defaults env_check += \"FOO\"",
         "Defaults env_check += \"XYZZY\"",
-        //"Defaults umask = 0123",
-        //"Defaults passwd_tries = 5",
+        "Defaults umask = 0123",
+        "Defaults passwd_tries = 5",
         "Defaults lecture_file = \"/etc/sudoers\"",
         "Defaults secure_path = /etc"
     ]);
@@ -314,8 +314,14 @@ fn default_set_test() {
     );
     assert_eq!(settings.str_value["lecture_file"], "/etc/sudoers");
     assert_eq!(settings.str_value["secure_path"], "/etc");
-    //assert_eq!(settings.int_value["umask"], 0o123);
-    //assert_eq!(settings.int_value["passwd_tries"], 5);
+    assert_eq!(settings.int_value["umask"], 0o123);
+    assert_eq!(settings.int_value["passwd_tries"], 5);
+
+    assert!(parse_string::<Sudo>("Defaults umask = 789").is_err());
+    assert!(parse_string::<Sudo>("Defaults umask = 1234").is_err());
+    assert!(parse_string::<Sudo>("Defaults verifypw = \"sometimes\"").is_err());
+    assert!(parse_string::<Sudo>("Defaults verifypw = sometimes").is_err());
+    assert!(parse_string::<Sudo>("Defaults verifypw = never").is_ok());
 }
 
 #[test]
diff --git a/test-binaries/sudoers b/test-binaries/sudoers
index 0252683e..d11f7386 100644
--- a/test-binaries/sudoers
+++ b/test-binaries/sudoers
@@ -2,6 +2,7 @@ Defaults	env_reset
 Defaults	env_keep = "AAP NOOT MIES"
 Defaults	env_keep += "MIES WIM ZUS JET" * AAP
 Defaults	env_keep -= "AAP NOOT ZUS JET"
+Defaults	umask=778
 
 Defaults	mail_badpass
 Defaults	secure_path="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin"
diff --git a/test-framework/sudo-compliance-tests/src/flag_group.rs b/test-framework/sudo-compliance-tests/src/flag_group.rs
index 4a660f24..8b4da155 100644
--- a/test-framework/sudo-compliance-tests/src/flag_group.rs
+++ b/test-framework/sudo-compliance-tests/src/flag_group.rs
@@ -28,7 +28,6 @@ fn changes_the_group_id() -> Result<()> {
 }
 
 #[test]
-#[ignore]
 fn adds_group_to_groups_output() -> Result<()> {
     let extra_group = "rustaceans";
     let env = Env(SUDOERS_ALL_ALL_NOPASSWD)
diff --git a/test-framework/sudo-compliance-tests/src/sudoers.rs b/test-framework/sudo-compliance-tests/src/sudoers.rs
index 60a5df76..5fa05f98 100644
--- a/test-framework/sudo-compliance-tests/src/sudoers.rs
+++ b/test-framework/sudo-compliance-tests/src/sudoers.rs
@@ -3,6 +3,7 @@ use sudo_test::{Command, Env, TextFile, User};
 use crate::{Result, PASSWORD, SUDOERS_ROOT_ALL_NOPASSWD, USERNAME};
 
 mod cmnd;
+mod host_list;
 mod run_as;
 mod user_list;
 
diff --git a/test-framework/sudo-compliance-tests/src/sudoers/host_list.rs b/test-framework/sudo-compliance-tests/src/sudoers/host_list.rs
new file mode 100644
index 00000000..609d69d5
--- /dev/null
+++ b/test-framework/sudo-compliance-tests/src/sudoers/host_list.rs
@@ -0,0 +1,94 @@
+use sudo_test::{Command, Env};
+
+use crate::Result;
+
+#[test]
+fn given_specific_hostname_then_sudo_from_said_hostname_is_allowed() -> Result<()> {
+    let hostname = "container";
+    let env = Env(format!("ALL {hostname} = (ALL:ALL) ALL"))
+        .hostname(hostname)
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()
+}
+
+#[test]
+fn given_specific_hostname_then_sudo_from_different_hostname_is_rejected() -> Result<()> {
+    let env = Env("ALL remotehost = (ALL:ALL) ALL")
+        .hostname("container")
+        .build()?;
+
+    let output = Command::new("sudo").arg("true").exec(&env)?;
+
+    assert!(!output.status().success());
+    assert_eq!(Some(1), output.status().code());
+
+    if sudo_test::is_original_sudo() {
+        assert_contains!(
+            output.stderr(),
+            "root is not allowed to run sudo on container"
+        );
+    }
+
+    Ok(())
+}
+
+#[test]
+fn different() -> Result<()> {
+    let env = Env("ALL remotehost, container = (ALL:ALL) ALL")
+        .hostname("container")
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()
+}
+
+#[test]
+fn repeated() -> Result<()> {
+    let env = Env("ALL container, container = (ALL:ALL) ALL")
+        .hostname("container")
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()
+}
+
+#[test]
+fn negation_rejects() -> Result<()> {
+    let env = Env("ALL remotehost, !container = (ALL:ALL) ALL")
+        .hostname("container")
+        .build()?;
+
+    let output = Command::new("sudo").arg("true").exec(&env)?;
+
+    assert!(!output.status().success());
+    assert_eq!(Some(1), output.status().code());
+
+    if sudo_test::is_original_sudo() {
+        assert_contains!(
+            output.stderr(),
+            "root is not allowed to run sudo on container"
+        );
+    }
+
+    Ok(())
+}
+
+#[test]
+fn double_negative_is_positive() -> Result<()> {
+    let env = Env("ALL !!container = (ALL:ALL) ALL")
+        .hostname("container")
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()
+}
diff --git a/test-framework/sudo-compliance-tests/src/sudoers/user_list.rs b/test-framework/sudo-compliance-tests/src/sudoers/user_list.rs
index 759be8d5..5a0a83c6 100644
--- a/test-framework/sudo-compliance-tests/src/sudoers/user_list.rs
+++ b/test-framework/sudo-compliance-tests/src/sudoers/user_list.rs
@@ -3,7 +3,7 @@
 use pretty_assertions::assert_eq;
 use sudo_test::{Command, Env};
 
-use crate::{Result, USERNAME};
+use crate::{Result, PAMD_SUDO_PAM_PERMIT, USERNAME};
 
 #[test]
 fn no_match() -> Result<()> {
@@ -76,3 +76,96 @@ fn group_id() -> Result<()> {
         .exec(&env)?
         .assert_success()
 }
+
+#[test]
+fn many_different() -> Result<()> {
+    let env = Env(format!("root, {USERNAME} ALL=(ALL:ALL) NOPASSWD: ALL"))
+        .user(USERNAME)
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .as_user(USERNAME)
+        .exec(&env)?
+        .assert_success()
+}
+
+#[test]
+fn many_repeated() -> Result<()> {
+    let env = Env("root, root ALL=(ALL:ALL) NOPASSWD: ALL").build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()
+}
+
+#[test]
+fn double_negative_is_positive() -> Result<()> {
+    let env = Env("!!root ALL=(ALL:ALL) NOPASSWD: ALL")
+        .user(USERNAME)
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .exec(&env)?
+        .assert_success()
+}
+
+#[test]
+fn negation_excludes_group_members() -> Result<()> {
+    let env = Env("%users, !ghost ALL=(ALL:ALL) ALL")
+        // use PAM to avoid `ghost` getting a password prompt
+        .file("/etc/pam.d/sudo", PAMD_SUDO_PAM_PERMIT)
+        // the primary group of all new users is `users`
+        .user("ferris")
+        .user("ghost")
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .as_user("ferris")
+        .exec(&env)?
+        .assert_success()?;
+
+    let output = Command::new("sudo")
+        .arg("true")
+        .as_user("ghost")
+        .exec(&env)?;
+
+    assert!(!output.status().success());
+    assert_eq!(Some(1), output.status().code());
+
+    if sudo_test::is_original_sudo() {
+        assert_contains!(output.stderr(), "ghost is not in the sudoers file");
+    }
+
+    Ok(())
+}
+
+#[test]
+fn negation_is_order_sensitive() -> Result<()> {
+    // negated items at the start of a specifier list  are meaningless
+    let env = Env("!ghost, %users ALL=(ALL:ALL) NOPASSWD: ALL")
+        // the primary group of all new users is `users`
+        .user("ferris")
+        .user("ghost")
+        .build()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .as_user("ferris")
+        .exec(&env)?
+        .assert_success()?;
+
+    Command::new("sudo")
+        .arg("true")
+        .as_user("ghost")
+        .exec(&env)?
+        .assert_success()
+}
diff --git a/test-framework/sudo-test/src/docker.rs b/test-framework/sudo-test/src/docker.rs
index c0cccbf4..48aad076 100644
--- a/test-framework/sudo-test/src/docker.rs
+++ b/test-framework/sudo-test/src/docker.rs
@@ -21,11 +21,18 @@ pub struct Container {
 }
 
 impl Container {
-    pub fn new(image: &str) -> Result<Self> {
+    #[cfg(test)]
+    fn new(image: &str) -> Result<Self> {
+        Self::new_with_hostname(image, None)
+    }
+
+    pub fn new_with_hostname(image: &str, hostname: Option<&str>) -> Result<Self> {
         let mut docker_run = StdCommand::new("docker");
-        docker_run
-            .args(["run", "-d", "--rm", image])
-            .args(DOCKER_RUN_COMMAND);
+        docker_run.args(["run", "--detach"]);
+        if let Some(hostname) = hostname {
+            docker_run.args(["--hostname", hostname]);
+        }
+        docker_run.args(["--rm", image]).args(DOCKER_RUN_COMMAND);
         let id = run(&mut docker_run, None)?.stdout()?;
         validate_docker_id(&id, &docker_run)?;
 
diff --git a/test-framework/sudo-test/src/lib.rs b/test-framework/sudo-test/src/lib.rs
index c2f344c9..90cb4f75 100644
--- a/test-framework/sudo-test/src/lib.rs
+++ b/test-framework/sudo-test/src/lib.rs
@@ -99,6 +99,7 @@ impl Command {
 pub struct EnvBuilder {
     files: HashMap<AbsolutePath, TextFile>,
     groups: HashMap<Groupname, Group>,
+    hostname: Option<String>,
     users: HashMap<Username, User>,
 }
 
@@ -158,6 +159,12 @@ impl EnvBuilder {
         self
     }
 
+    /// Sets the hostname of the container to the specified string
+    pub fn hostname(&mut self, hostname: impl AsRef<str>) -> &mut Self {
+        self.hostname = Some(hostname.as_ref().to_string());
+        self
+    }
+
     /// builds the test environment
     ///
     /// # Panics
@@ -172,7 +179,7 @@ impl EnvBuilder {
             docker::build_base_image().expect("fatal error: could not build the base Docker image")
         });
 
-        let container = Container::new(BASE_IMAGE)?;
+        let container = Container::new_with_hostname(BASE_IMAGE, self.hostname.as_deref())?;
 
         let (mut usernames, user_ids) = getent_passwd(&container)?;
 
@@ -698,4 +705,16 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn setting_hostname_works() -> Result<()> {
+        let expected = "container";
+
+        let env = EnvBuilder::default().hostname(expected).build()?;
+
+        let actual = Command::new("hostname").exec(&env)?.stdout()?;
+        assert_eq!(expected, actual);
+
+        Ok(())
+    }
 }