Skip to content

Commit

Permalink
Fix various assumptions in tests about how things are done on Linux (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
squell authored Oct 14, 2024
2 parents 7935463 + 19f28bb commit 3900363
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 20 deletions.
7 changes: 6 additions & 1 deletion src/common/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,12 @@ mod tests {
let mut target_environment = HashMap::new();
target_environment.insert("SUDO_USER".to_string(), context.current_user.name.clone());

assert_eq!(context.command.command.to_str().unwrap(), "/usr/bin/echo");
if cfg!(target_os = "linux") {
// this assumes /bin is a symlink on /usr/bin, like it is on modern Debian/Ubuntu
assert_eq!(context.command.command.to_str().unwrap(), "/usr/bin/echo");
} else {
assert_eq!(context.command.command.to_str().unwrap(), "/bin/echo");
}
assert_eq!(context.command.arguments, ["hello"]);
assert_eq!(context.hostname, Hostname::resolve());
assert_eq!(context.target_user.uid, 0);
Expand Down
35 changes: 25 additions & 10 deletions src/common/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,11 @@ pub(crate) fn resolve_target_user_and_group(
// when no -u or -g is specified, default to root:root
(None, None) => {
target_user = User::from_name(cstr!("root"))?;
target_group = Group::from_name(cstr!("root"))?;
target_group = Group::from_name(if cfg!(target_os = "linux") {
cstr!("root")
} else {
cstr!("wheel")
})?;
}
_ => {}
}
Expand Down Expand Up @@ -223,6 +227,7 @@ mod tests {
use std::path::PathBuf;

use crate::common::resolve::CurrentUser;
use crate::system::ROOT_GROUP_NAME;

use super::{is_valid_executable, resolve_path, resolve_target_user_and_group, NameOrId};

Expand Down Expand Up @@ -274,7 +279,7 @@ mod tests {
// fallback to root
let (user, group) = resolve_target_user_and_group(&None, &None, &current_user).unwrap();
assert_eq!(user.name, "root");
assert_eq!(group.name, "root");
assert_eq!(group.name, ROOT_GROUP_NAME);

// unknown user
let result =
Expand All @@ -288,9 +293,10 @@ mod tests {

// fallback to current user when different group specified
let (user, group) =
resolve_target_user_and_group(&None, &Some("root".into()), &current_user).unwrap();
resolve_target_user_and_group(&None, &Some(ROOT_GROUP_NAME.into()), &current_user)
.unwrap();
assert_eq!(user.name, current_user.name);
assert_eq!(group.name, "root");
assert_eq!(group.name, ROOT_GROUP_NAME);

// fallback to current users group when no group specified
let (user, group) =
Expand All @@ -302,7 +308,7 @@ mod tests {
}

/// Resolve symlinks in all the directories leading up to a file, but
/// not the file itself; this alles sudo to specify a precise policy with
/// not the file itself; this allows sudo to specify a precise policy with
/// tools like busybox or pgrep (which is a symlink to pgrep on systems)
pub fn canonicalize<P: AsRef<Path>>(path: P) -> io::Result<PathBuf> {
let path = path.as_ref();
Expand Down Expand Up @@ -338,10 +344,19 @@ mod test {
canonicalize("/usr/bin/pkill").unwrap(),
Path::new("/usr/bin/pkill")
);
// this assumes /bin is a symlink on /usr/bin, like it is on modern Debian/Ubuntu
assert_eq!(
canonicalize("/bin/pkill").unwrap(),
Path::new("/usr/bin/pkill")
);
if cfg!(any(target_os = "linux", target_os = "macos")) {
// this assumes /bin is a symlink on /usr/bin, like it is on modern Debian/Ubuntu
assert_eq!(
canonicalize("/bin/pkill").unwrap(),
Path::new("/usr/bin/pkill")
);
} else if cfg!(target_os = "freebsd") {
assert_eq!(canonicalize("/bin/pkill").unwrap(), Path::new("/bin/pkill"));
} else {
panic!(
"canonicalization test not yet adapted for {}",
std::env::consts::OS
);
}
}
}
16 changes: 13 additions & 3 deletions src/system/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,19 @@ mod test {
// /etc/hosts should be readable and "secure" (if this test fails, you have been compromised)
assert!(std::fs::File::open("/etc/hosts").is_ok());
assert!(secure_open("/etc/hosts", false).is_ok());
// /var/log/utmp should be readable, but not secure (writeable by group other than root)
assert!(std::fs::File::open("/var/log/wtmp").is_ok());
assert!(secure_open("/var/log/wtmp", false).is_err());

// /tmp should be readable, but not secure (writeable by group other than root)
assert!(std::fs::File::open("/tmp").is_ok());
assert!(secure_open("/tmp", false).is_err());

#[cfg(target_os = "linux")]
{
// /var/log/utmp should be readable, but not secure (writeable by group other than root)
// It doesn't exist on many non-Linux systems however.
assert!(std::fs::File::open("/var/log/wtmp").is_ok());
assert!(secure_open("/var/log/wtmp", false).is_err());
}

// /etc/shadow should not be readable
assert!(std::fs::File::open("/etc/shadow").is_err());
assert!(secure_open("/etc/shadow", false).is_err());
Expand Down
13 changes: 10 additions & 3 deletions src/system/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,21 @@ impl UnixGroup for super::Group {

#[cfg(test)]
mod test {
use crate::system::{Group, User};
use std::ffi::CString;

use crate::system::{Group, User, ROOT_GROUP_NAME};

use super::*;

fn test_user(user: impl UnixUser, name_c: &CStr, uid: libc::uid_t) {
let name = name_c.to_str().unwrap();
assert!(user.has_name(name));
assert!(user.has_uid(uid));
assert!(user.in_group_by_name(name_c));
if user.has_name("root") {
assert!(user.in_group_by_name(CString::new(ROOT_GROUP_NAME).unwrap().as_c_str()));
} else {
assert!(user.in_group_by_name(name_c));
}
assert_eq!(user.is_root(), name == "root");
}

Expand All @@ -92,7 +98,8 @@ mod test {
#[test]
fn test_unix_group() {
let group = |name| Group::from_name(name).unwrap().unwrap();
test_group(group(cstr!("root")), "root", 0);
let root_group_cstr = CString::new(ROOT_GROUP_NAME).unwrap();
test_group(group(root_group_cstr.as_c_str()), ROOT_GROUP_NAME, 0);
test_group(group(cstr!("daemon")), "daemon", 1);
}

Expand Down
25 changes: 22 additions & 3 deletions src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,12 @@ pub fn make_zeroed_sigaction() -> libc::sigaction {
unsafe { std::mem::zeroed() }
}

#[cfg(all(test, target_os = "linux"))]
pub(crate) const ROOT_GROUP_NAME: &str = "root";

#[cfg(all(test, not(target_os = "linux")))]
pub(crate) const ROOT_GROUP_NAME: &str = "wheel";

#[cfg(test)]
mod tests {
use std::{
Expand All @@ -700,7 +706,7 @@ mod tests {
use super::{
fork, getpgrp, setpgid,
wait::{Wait, WaitOptions},
ForkResult, Group, User, WithProcess,
ForkResult, Group, User, WithProcess, ROOT_GROUP_NAME,
};

pub(super) fn tempfile() -> std::io::Result<std::fs::File> {
Expand All @@ -721,13 +727,26 @@ mod tests {

#[test]
fn test_get_user_and_group_by_id() {
let fixed_users = &[(0, "root"), (1, "daemon")];
let fixed_users = &[
(0, "root"),
(
User::from_name(cstr!("daemon")).unwrap().unwrap().uid,
"daemon",
),
];
for &(id, name) in fixed_users {
let root = User::from_uid(id).unwrap().unwrap();
assert_eq!(root.uid, id as libc::uid_t);
assert_eq!(root.name, name);
}
for &(id, name) in fixed_users {
let fixed_groups = &[
(0, ROOT_GROUP_NAME),
(
Group::from_name(cstr!("daemon")).unwrap().unwrap().gid,
"daemon",
),
];
for &(id, name) in fixed_groups {
let root = Group::from_gid(id).unwrap().unwrap();
assert_eq!(root.gid, id as libc::gid_t);
assert_eq!(root.name, name);
Expand Down

0 comments on commit 3900363

Please sign in to comment.