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 various assumptions in tests about how things are done on Linux #876

Merged
merged 3 commits into from
Oct 14, 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
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
Loading