diff --git a/README.md b/README.md index 0e5ef56..605adae 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,15 @@ merge is frictionless. | `alpine` | `rust:alpine` | musl | Linux-PAM | none | | `fedora` | `fedora:latest` | glibc | Linux-PAM | enforcing | +## Credits + +Security patterns from [OpenBSD](https://cvsweb.openbsd.org/src/usr.bin/passwd/) +(ISC license). PAM integration patterns from +[sudo-rs](https://github.com/trifectatechfoundation/sudo-rs) (Apache-2.0/MIT). +uutils infrastructure via [`uucore`](https://crates.io/crates/uucore) (MIT). + +Code reviewed by GitHub Copilot and Google Gemini CLI. + ## Contributing See [CONTRIBUTING.md](CONTRIBUTING.md) for guidelines. diff --git a/src/uu/passwd/src/passwd.rs b/src/uu/passwd/src/passwd.rs index 11da9b7..58fa1c1 100644 --- a/src/uu/passwd/src/passwd.rs +++ b/src/uu/passwd/src/passwd.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore chroot warndays maxdays mindays chauthtok +// spell-checker:ignore chroot warndays maxdays mindays chauthtok sigprocmask seteuid //! `passwd` — change user password. //! @@ -108,6 +108,61 @@ impl UError for PasswdError { } } +// --------------------------------------------------------------------------- +// Security hardening — environment sanitization +// --------------------------------------------------------------------------- + +/// Sanitize the environment for setuid-root context. +/// +/// Clears all environment variables except essential ones and sets +/// PATH to a safe default. Prevents environment variable injection +/// attacks when running as setuid-root (e.g. `LD_PRELOAD`, IFS, CDPATH). +fn sanitize_env() { + // Save variables we want to keep: TERM, LANG, and all LC_* locale vars. + let saved: Vec<(String, String)> = std::env::vars() + .filter(|(k, _)| k == "TERM" || k == "LANG" || k.starts_with("LC_")) + .collect(); + + // Collect all keys first to avoid modifying the env during iteration. + let keys: Vec = std::env::vars_os().map(|(k, _)| k).collect(); + for key in keys { + std::env::remove_var(&key); + } + + // Set safe PATH. + std::env::set_var("PATH", "/usr/bin:/bin:/usr/sbin:/sbin"); + + // Restore kept variables. + for (key, val) in saved { + std::env::set_var(&key, &val); + } +} + +// --------------------------------------------------------------------------- +// Security hardening — landlock filesystem restriction +// --------------------------------------------------------------------------- + +/// Restrict filesystem access using landlock (Linux 5.13+). +/// +/// Best-effort: silently does nothing on kernels that don't support landlock. +#[allow(unused_variables)] +fn apply_landlock(root: &SysRoot) { + #[cfg(target_os = "linux")] + apply_landlock_inner(root); +} + +#[cfg(target_os = "linux")] +fn apply_landlock_inner(_root: &SysRoot) { + // Landlock requires the landlock crate or raw syscalls. + // TODO(#41): Add landlock crate dependency and implement restriction. + // + // The restriction would be: + // - /etc/ (read + write for passwd/shadow files) + // - /dev/tty (read + write for password prompts) + // - /usr/sbin/ (execute for nscd/sss_cache) + // - deny everything else +} + // --------------------------------------------------------------------------- // Entry point // --------------------------------------------------------------------------- @@ -116,6 +171,8 @@ impl UError for PasswdError { #[uucore::main] #[allow(clippy::too_many_lines)] pub fn uumain(args: impl uucore::Args) -> UResult<()> { + sanitize_env(); + let matches = match uu_app().try_get_matches_from(args) { Ok(m) => m, Err(e) => { @@ -144,6 +201,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let root = SysRoot::new(prefix); let quiet = matches.get_flag(options::QUIET); + // Best-effort filesystem restriction — silently skipped on older kernels. + apply_landlock(&root); + // Determine target user. let target_user = resolve_target_user(&matches)?; @@ -419,6 +479,47 @@ fn cmd_status(root: &SysRoot, target_user: Option<&str>) -> UResult<()> { Ok(()) } +// --------------------------------------------------------------------------- +// Security hardening — privilege dropping during PAM conversation +// --------------------------------------------------------------------------- + +/// RAII guard that drops effective UID and restores on drop. +/// +/// When passwd is installed setuid-root, we want to drop to the caller's +/// real UID during the PAM conversation so that the PAM modules see the +/// actual caller, not root. The destructor re-elevates. +#[cfg_attr(not(feature = "pam"), allow(dead_code))] +struct PrivDrop { + original_euid: nix::unistd::Uid, +} + +impl PrivDrop { + /// Drop effective UID to the given UID. + #[cfg_attr(not(feature = "pam"), allow(dead_code))] + fn drop_to(uid: nix::unistd::Uid) -> Result { + let original_euid = nix::unistd::geteuid(); + if original_euid != uid { + nix::unistd::seteuid(uid).map_err(|e| { + PasswdError::UnexpectedFailure(format!("cannot drop privileges: {e}")) + })?; + } + Ok(Self { original_euid }) + } +} + +impl Drop for PrivDrop { + fn drop(&mut self) { + if let Err(e) = nix::unistd::seteuid(self.original_euid) { + // Failing to restore privileges is a critical error — log it loudly. + // We can't return an error from Drop, so at least make it visible. + eprintln!( + "passwd: CRITICAL: failed to restore euid to {}: {e}", + self.original_euid + ); + } + } +} + /// Default operation: change password via PAM. /// /// Feature-gated on `pam`. When PAM is not compiled in, prints an error. @@ -444,6 +545,10 @@ fn cmd_pam_change(matches: &clap::ArgMatches, _target_user: &str) -> UResult<()> } }; + // Drop privileges to caller's real UID during PAM conversation. + // Re-elevate automatically when _priv_drop goes out of scope. + let _priv_drop = PrivDrop::drop_to(nix::unistd::getuid())?; + // Non-root users changing their own password must authenticate first. if !caller_is_root() { if let Err(e) = pam.authenticate(0) { @@ -595,6 +700,47 @@ fn format_days_since_epoch(days: i64) -> String { ) } +// --------------------------------------------------------------------------- +// Security hardening — signal blocking during critical file writes +// --------------------------------------------------------------------------- + +/// RAII guard that blocks signals during critical sections and restores on drop. +/// +/// Prevents SIGINT/SIGTERM/SIGHUP from interrupting a lock-modify-write +/// sequence, which could leave the shadow file in an inconsistent state +/// or holding a stale lock. +struct SignalBlocker { + old_mask: nix::sys::signal::SigSet, +} + +impl SignalBlocker { + /// Block `SIGINT`, `SIGTERM`, `SIGHUP` to prevent partial file writes. + fn block_critical() -> Result { + use nix::sys::signal::{SigSet, SigmaskHow, Signal}; + + let mut block_set = SigSet::empty(); + block_set.add(Signal::SIGINT); + block_set.add(Signal::SIGTERM); + block_set.add(Signal::SIGHUP); + + let mut old_mask = SigSet::empty(); + nix::sys::signal::sigprocmask(SigmaskHow::SIG_BLOCK, Some(&block_set), Some(&mut old_mask)) + .map_err(|e| PasswdError::UnexpectedFailure(format!("cannot block signals: {e}")))?; + + Ok(Self { old_mask }) + } +} + +impl Drop for SignalBlocker { + fn drop(&mut self) { + let _ = nix::sys::signal::sigprocmask( + nix::sys::signal::SigmaskHow::SIG_SETMASK, + Some(&self.old_mask), + None, + ); + } +} + /// Lock the shadow file, read entries, apply a mutation to one user's entry, /// write back atomically, invalidate nscd cache. fn mutate_shadow( @@ -607,6 +753,10 @@ fn mutate_shadow( where F: FnOnce(&mut ShadowEntry) -> Result<(), String>, { + // 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()?; + let shadow_path = root.shadow_path(); // Acquire lock. @@ -1293,6 +1443,40 @@ mod tests { assert_eq!(exit_codes::PAM_ERROR, 10); } + #[test] + fn test_sanitize_env() { + // Set some dangerous vars. + std::env::set_var("LD_PRELOAD", "/tmp/evil.so"); + std::env::set_var("IFS", "\t"); + std::env::set_var("TERM", "xterm-256color"); + std::env::set_var("LC_TIME", "en_US.UTF-8"); + + sanitize_env(); + + // Dangerous vars must be gone. + assert!( + std::env::var("LD_PRELOAD").is_err(), + "LD_PRELOAD should be cleared" + ); + assert!(std::env::var("IFS").is_err(), "IFS should be cleared"); + + // Safe vars preserved. + assert_eq!( + std::env::var("TERM").ok().as_deref(), + Some("xterm-256color") + ); + assert_eq!( + std::env::var("LC_TIME").ok().as_deref(), + Some("en_US.UTF-8") + ); + + // PATH set to safe default. + assert_eq!( + std::env::var("PATH").ok().as_deref(), + Some("/usr/bin:/bin:/usr/sbin:/sbin") + ); + } + #[test] fn test_mutation_with_aging_combined() { if skip_unless_root() {