diff --git a/Cargo.toml b/Cargo.toml index dadc44f..7c2bfab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ uucore = "0.7" thiserror = "2" # Unix/Linux -nix = { version = "0.29", features = ["user", "fs", "process", "signal", "term"] } +nix = { version = "0.29", features = ["user", "fs", "process", "signal", "term", "resource"] } libc = "0.2" # Security diff --git a/src/shadow-core/src/atomic.rs b/src/shadow-core/src/atomic.rs index ae0148a..f1a70d4 100644 --- a/src/shadow-core/src/atomic.rs +++ b/src/shadow-core/src/atomic.rs @@ -20,6 +20,26 @@ use std::sync::atomic::{AtomicU64, Ordering}; use crate::error::ShadowError; +/// RAII guard that saves and restores the process umask. +/// +/// On creation, sets the umask to zero so that file mode bits passed to +/// `OpenOptions::mode()` are applied exactly. The original umask is restored +/// when the guard is dropped, even on error or panic paths. +struct UmaskGuard(nix::sys::stat::Mode); + +impl UmaskGuard { + /// Set umask to zero and return a guard that restores the original. + fn zero() -> Self { + Self(nix::sys::stat::umask(nix::sys::stat::Mode::empty())) + } +} + +impl Drop for UmaskGuard { + fn drop(&mut self) { + nix::sys::stat::umask(self.0); + } +} + /// Drop guard that auto-deletes a temporary file unless explicitly committed. /// /// Ensures the tmp file is cleaned up on any error path, including panics. @@ -76,6 +96,11 @@ where let mut guard = TmpGuard::new(tmp_path.clone()); + // Save and reset umask to ensure mode parameter is applied exactly. + // A caller could set a restrictive umask before invoking setuid passwd. + // The guard restores the original umask on any exit path. + let _umask = UmaskGuard::zero(); + let mut tmp_file = std::fs::OpenOptions::new() .write(true) .create_new(true) @@ -85,6 +110,18 @@ where f(&mut tmp_file)?; + // Zero-length output guard: a zero-length shadow file locks out all users. + // OpenBSD checks this in pw_mkdb before replacing the original. + let written = tmp_file + .metadata() + .map_err(|e| ShadowError::IoPath(e, tmp_path.clone()))? + .len(); + if written == 0 { + return Err(ShadowError::Other( + "refusing to write zero-length file".into(), + )); + } + // Flush and fsync. tmp_file .flush() diff --git a/src/shadow-core/src/lock.rs b/src/shadow-core/src/lock.rs index b651458..bccf677 100644 --- a/src/shadow-core/src/lock.rs +++ b/src/shadow-core/src/lock.rs @@ -268,6 +268,27 @@ mod tests { lock.release().unwrap(); } + #[test] + fn test_lock_file_has_cloexec() { + use std::os::unix::io::AsRawFd; + + // Rust's stdlib sets O_CLOEXEC by default on Linux. + // Verify the lock file FD won't leak to child processes. + let dir = tempfile::tempdir().expect("tempdir creation failed"); + let file = dir.path().join("test_file"); + fs::write(&file, "data").expect("failed to write test file"); + + let lock = FileLock::acquire(&file).expect("failed to acquire lock"); + + let f = fs::File::open(&lock.lock_path).expect("failed to open lock file"); + let fd = f.as_raw_fd(); + let flags = + nix::fcntl::fcntl(fd, nix::fcntl::FcntlArg::F_GETFD).expect("fcntl F_GETFD failed"); + assert!(flags & libc::FD_CLOEXEC != 0, "FD should have CLOEXEC set"); + + lock.release().expect("failed to release lock"); + } + #[test] fn test_lock_file_contains_pid() { let dir = tempfile::tempdir().unwrap(); diff --git a/src/uu/passwd/src/passwd.rs b/src/uu/passwd/src/passwd.rs index 58fa1c1..1a5b040 100644 --- a/src/uu/passwd/src/passwd.rs +++ b/src/uu/passwd/src/passwd.rs @@ -108,6 +108,41 @@ impl UError for PasswdError { } } +// --------------------------------------------------------------------------- +// Security hardening — process hardening +// --------------------------------------------------------------------------- + +/// Suppress core dumps and prevent ptrace attachment. +/// +/// OpenBSD's `pw_init()` sets `RLIMIT_CORE=0`. A core dump from a setuid +/// passwd process could expose password hashes and plaintext passwords. +fn suppress_core_dumps() { + // RLIMIT_CORE = 0: no core dumps. Best-effort — ignore errors. + let _ = nix::sys::resource::setrlimit(nix::sys::resource::Resource::RLIMIT_CORE, 0, 0); + // PR_SET_DUMPABLE = 0: prevent ptrace attachment and /proc/pid/mem reads. + // Linux-specific — silently skipped on other platforms. + #[cfg(target_os = "linux")] + { + // SAFETY: prctl with PR_SET_DUMPABLE is a simple flag set, no pointers. + unsafe { + libc::prctl(libc::PR_SET_DUMPABLE, 0); + } + } +} + +/// Raise `RLIMIT_FSIZE` to prevent truncated file writes. +/// +/// OpenBSD raises `RLIMIT_FSIZE` to infinity before file operations. A +/// malicious caller could `ulimit -f 1` before invoking setuid passwd, +/// causing /etc/shadow to be truncated mid-write. +fn raise_file_size_limit() { + let _ = nix::sys::resource::setrlimit( + nix::sys::resource::Resource::RLIMIT_FSIZE, + nix::sys::resource::RLIM_INFINITY, + nix::sys::resource::RLIM_INFINITY, + ); +} + // --------------------------------------------------------------------------- // Security hardening — environment sanitization // --------------------------------------------------------------------------- @@ -171,6 +206,8 @@ fn apply_landlock_inner(_root: &SysRoot) { #[uucore::main] #[allow(clippy::too_many_lines)] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + suppress_core_dumps(); + raise_file_size_limit(); sanitize_env(); let matches = match uu_app().try_get_matches_from(args) { @@ -293,6 +330,18 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }); } + // Prevent non-root from targeting other users (avoids timing-based + // user enumeration through PAM auth failure timing). + if !caller_is_root() { + let current = get_current_username()?; + if current != target_user { + return Err(PasswdError::PermissionDenied( + "You may not view or modify password information for another user.".into(), + ) + .into()); + } + } + // Default: password change via PAM. cmd_pam_change(&matches, &target_user) } @@ -520,10 +569,39 @@ impl Drop for PrivDrop { } } +/// Install a signal handler for SIGINT that prints "Password unchanged." +/// and exits cleanly. This matches OpenBSD's kbintr pattern. +/// +/// Only call this during interactive password input. The handler uses +/// async-signal-safe functions only (_exit, write). +fn install_interrupt_handler() { + // SAFETY: The handler uses only async-signal-safe operations + // (write to fd 2, _exit). No heap allocation or mutex locking. + unsafe { + let action = nix::sys::signal::SigAction::new( + nix::sys::signal::SigHandler::Handler(handle_interrupt), + nix::sys::signal::SaFlags::empty(), + nix::sys::signal::SigSet::empty(), + ); + let _ = nix::sys::signal::sigaction(nix::sys::signal::Signal::SIGINT, &action); + } +} + +extern "C" fn handle_interrupt(_sig: libc::c_int) { + // Async-signal-safe: only write() and _exit(). + // SAFETY: Writing to stderr fd and exiting — both are signal-safe. + unsafe { + libc::write(2, b"\nPassword unchanged.\n".as_ptr().cast(), 21); + libc::_exit(0); + } +} + /// Default operation: change password via PAM. /// /// Feature-gated on `pam`. When PAM is not compiled in, prints an error. fn cmd_pam_change(matches: &clap::ArgMatches, _target_user: &str) -> UResult<()> { + install_interrupt_handler(); + let _keep_tokens = matches.get_flag(options::KEEP_TOKENS); let _use_stdin = matches.get_flag(options::STDIN); let _repository = matches.get_one::(options::REPOSITORY); @@ -753,6 +831,12 @@ fn mutate_shadow( where F: FnOnce(&mut ShadowEntry) -> Result<(), String>, { + // Consolidate real + effective UID to root for file operations. + // Some filesystem configurations check real UID. + if nix::unistd::geteuid().is_root() { + let _ = nix::unistd::setuid(nix::unistd::Uid::from_raw(0)); + } + // Block signals for the entire critical section (lock → write → unlock). // The RAII guard restores the original signal mask when this function returns. let _signals = SignalBlocker::block_critical()?; @@ -1477,6 +1561,50 @@ mod tests { ); } + // ------------------------------------------------------------------- + // OpenBSD hardening tests + // ------------------------------------------------------------------- + + #[test] + fn test_core_dump_suppression() { + // After calling suppress_core_dumps(), RLIMIT_CORE should be 0. + suppress_core_dumps(); + let (soft, _hard) = + nix::sys::resource::getrlimit(nix::sys::resource::Resource::RLIMIT_CORE).unwrap(); + assert_eq!(soft, 0, "RLIMIT_CORE should be 0 after suppression"); + } + + #[test] + fn test_raise_file_size_limit() { + raise_file_size_limit(); + let (soft, _hard) = + nix::sys::resource::getrlimit(nix::sys::resource::Resource::RLIMIT_FSIZE).unwrap(); + // In environments where the hard limit is already restricted (containers, + // CI), we may not reach RLIM_INFINITY. Verify it's at least very large. + assert!( + soft >= 1024 * 1024 * 1024 || soft == nix::sys::resource::RLIM_INFINITY, + "RLIMIT_FSIZE should be raised (got {soft})" + ); + } + + #[test] + fn test_zero_length_write_rejected() { + // atomic_write should refuse to replace a file with zero-length output. + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("shadow"); + std::fs::write(&target, "original content\n").unwrap(); + + let result = shadow_core::atomic::atomic_write(&target, |_file| { + // Write nothing — zero-length output. + Ok(()) + }); + + assert!(result.is_err(), "zero-length write should be rejected"); + // Original file should be untouched. + let content = std::fs::read_to_string(&target).unwrap(); + assert_eq!(content, "original content\n"); + } + #[test] fn test_mutation_with_aging_combined() { if skip_unless_root() {