diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..3d8223f --- /dev/null +++ b/deny.toml @@ -0,0 +1,27 @@ +[advisories] +version = 2 +yanked = "warn" + +[licenses] +version = 2 +allow = [ + "MIT", + "Apache-2.0", + "ISC", + "BSD-2-Clause", + "BSD-3-Clause", + "CC0-1.0", + "Unicode-3.0", + "Zlib", + "MPL-2.0", +] +confidence-threshold = 0.8 + +[bans] +multiple-versions = "warn" +wildcards = "allow" + +[sources] +unknown-registry = "warn" +unknown-git = "warn" +allow-registry = ["https://github.com/rust-lang/crates.io-index"] diff --git a/src/shadow-core/src/atomic.rs b/src/shadow-core/src/atomic.rs index 7287704..a2e4a5d 100644 --- a/src/shadow-core/src/atomic.rs +++ b/src/shadow-core/src/atomic.rs @@ -13,6 +13,7 @@ use std::fs::{self, File}; use std::io::{self, Write}; +use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::AsRawFd; use std::path::{Path, PathBuf}; @@ -37,16 +38,18 @@ where let tmp_path = tmp_path_for(target); - let mut tmp_file = - File::create(&tmp_path).map_err(|e| ShadowError::IoPath(e, tmp_path.clone()))?; + // Determine permissions: preserve original if target exists, otherwise 0600. + // Set mode at creation time to avoid any window where the file is world-readable. + let mode = fs::metadata(target) + .map(|m| std::os::unix::fs::PermissionsExt::mode(&m.permissions())) + .unwrap_or(0o600); - // Preserve original file permissions if the target exists. - if let Ok(meta) = fs::metadata(target) { - let permissions = meta.permissions(); - tmp_file - .set_permissions(permissions) - .map_err(|e| ShadowError::IoPath(e, tmp_path.clone()))?; - } + let mut tmp_file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(mode) + .open(&tmp_path) + .map_err(|e| ShadowError::IoPath(e, tmp_path.clone()))?; let result = f(&mut tmp_file); diff --git a/src/shadow-core/src/error.rs b/src/shadow-core/src/error.rs index 16e256b..6325b33 100644 --- a/src/shadow-core/src/error.rs +++ b/src/shadow-core/src/error.rs @@ -5,67 +5,43 @@ //! Unified error types for shadow-rs utilities. -use std::fmt; use std::io; +use std::path::PathBuf; + +use thiserror::Error; /// Result type alias used across all shadow-rs utilities. pub type ShadowResult = Result; /// Top-level error type for shadow-rs operations. -#[derive(Debug)] +#[derive(Debug, Error)] pub enum ShadowError { - /// I/O error with optional context. - Io(io::Error), + /// I/O error. + #[error("{0}")] + Io(#[from] io::Error), /// I/O error with path context. - IoPath(io::Error, std::path::PathBuf), + #[error("{path}: {source}", path = .1.display(), source = .0)] + IoPath(#[source] io::Error, PathBuf), /// File format parse error. + #[error("parse error: {0}")] Parse(String), /// Lock acquisition failed. + #[error("lock error: {0}")] Lock(String), /// Validation error (invalid username, UID range, etc.). + #[error("{0}")] Validation(String), /// Authentication error (PAM failure, wrong password, etc.). + #[error("authentication error: {0}")] Auth(String), /// Permission denied. + #[error("permission denied: {0}")] Permission(String), /// Generic error with message. + #[error("{0}")] Other(String), } -impl fmt::Display for ShadowError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Io(e) => write!(f, "{e}"), - Self::IoPath(e, path) => write!(f, "{}: {e}", path.display()), - Self::Parse(msg) => write!(f, "parse error: {msg}"), - Self::Lock(msg) => write!(f, "lock error: {msg}"), - Self::Auth(msg) => write!(f, "authentication error: {msg}"), - Self::Permission(msg) => write!(f, "permission denied: {msg}"), - Self::Validation(msg) | Self::Other(msg) => write!(f, "{msg}"), - } - } -} - -impl std::error::Error for ShadowError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::Io(e) | Self::IoPath(e, _) => Some(e), - Self::Parse(_) - | Self::Lock(_) - | Self::Validation(_) - | Self::Auth(_) - | Self::Permission(_) - | Self::Other(_) => None, - } - } -} - -impl From for ShadowError { - fn from(e: io::Error) -> Self { - Self::Io(e) - } -} - /// Print an error message prefixed with the utility name to stderr. #[macro_export] macro_rules! show_error { diff --git a/src/shadow-core/src/lock.rs b/src/shadow-core/src/lock.rs index 6b4bd07..2040eed 100644 --- a/src/shadow-core/src/lock.rs +++ b/src/shadow-core/src/lock.rs @@ -14,12 +14,11 @@ use std::fs; use std::io::Write; +use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; use std::thread; use std::time::{Duration, Instant}; -use nix::fcntl::{self, OFlag}; -use nix::sys::stat::Mode; use nix::unistd; use crate::error::ShadowError; @@ -52,32 +51,65 @@ impl FileLock { /// Acquire a lock with a custom timeout. /// + /// Uses the classic lock-via-link pattern to avoid TOCTOU races: + /// 1. Write our PID to a unique temp file + /// 2. Try to `hard_link` it to the lock path (atomic on POSIX) + /// 3. If link fails (lock exists), check for staleness and retry + /// + /// Even if two processes both detect a stale lock and both remove it, + /// only one will succeed at the subsequent `hard_link`, so mutual + /// exclusion is never violated. + /// /// # Errors /// /// Returns `ShadowError::Lock` if the lock cannot be acquired within the timeout. pub fn acquire_with_timeout(file_path: &Path, timeout: Duration) -> Result { let lock_path = lock_path_for(file_path); let deadline = Instant::now() + timeout; + let tmp_path = tmp_lock_path(&lock_path); + // Write our PID to the temp file once, then try to link it in a loop. + write_pid_file(&tmp_path)?; + + let result = Self::acquire_loop(&lock_path, &tmp_path, deadline); + + // Always clean up our temp file, regardless of success or failure. + let _ = fs::remove_file(&tmp_path); + + result + } + + /// Inner acquisition loop. Separated so the caller can guarantee temp file cleanup. + fn acquire_loop( + lock_path: &Path, + tmp_path: &Path, + deadline: Instant, + ) -> Result { loop { - if try_create_lock(&lock_path).is_ok() { + // Attempt to hard-link our temp file to the lock path. hard_link is + // atomic: it either creates the destination or fails, so two + // processes can never both succeed for the same lock_path. + if fs::hard_link(tmp_path, lock_path).is_ok() { return Ok(Self { - lock_path, + lock_path: lock_path.to_owned(), released: false, }); } - // Lock file exists — check if it's stale. - if is_stale_lock(&lock_path) { - // Remove stale lock and retry immediately. - let _ = fs::remove_file(&lock_path); + // Link failed — lock file exists. Check if it's stale. + if is_stale_lock(lock_path) { + // Remove the stale lock. If another process already removed it + // and re-acquired, our remove may fail or remove the wrong file, + // but the subsequent hard_link attempt is the real arbiter: + // it will fail atomically if someone else got there first. + let _ = fs::remove_file(lock_path); continue; } if Instant::now() >= deadline { return Err(ShadowError::Lock(format!( - "cannot acquire lock {}: timed out after {timeout:?}", - lock_path.display() + "cannot acquire lock {}: timed out", + lock_path.display(), ))); } @@ -116,21 +148,29 @@ fn lock_path_for(file_path: &Path) -> PathBuf { PathBuf::from(lock) } -/// Try to atomically create the lock file. Write our PID into it. -fn try_create_lock(lock_path: &Path) -> Result<(), ShadowError> { - // O_CREAT | O_EXCL ensures atomic creation — fails if the file exists. - let fd = fcntl::open( - lock_path, - OFlag::O_CREAT | OFlag::O_EXCL | OFlag::O_WRONLY, - Mode::from_bits_truncate(0o600), - ) - .map_err(|e| ShadowError::Lock(format!("cannot create {}: {e}", lock_path.display())))?; +/// Compute a unique temp file path for the lock-via-link pattern. +/// +/// Uses PID to avoid collisions between concurrent processes. +fn tmp_lock_path(lock_path: &Path) -> PathBuf { + let pid = std::process::id(); + let mut tmp = lock_path.as_os_str().to_owned(); + tmp.push(format!(".{pid}.tmp")); + PathBuf::from(tmp) +} + +/// Write our PID to a temp file for later hard-linking. +fn write_pid_file(tmp_path: &Path) -> Result<(), ShadowError> { + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(tmp_path) + .map_err(|e| ShadowError::Lock(format!("cannot create {}: {e}", tmp_path.display())))?; - // Write our PID for stale detection. - // SAFETY: fd is a valid file descriptor we just created. - let mut file = unsafe { std::fs::File::from_raw_fd(fd) }; let pid = unistd::getpid(); - let _ = write!(file, "{pid}"); + write!(file, "{pid}") + .map_err(|e| ShadowError::Lock(format!("cannot write {}: {e}", tmp_path.display())))?; Ok(()) } @@ -155,8 +195,6 @@ fn is_stale_lock(lock_path: &Path) -> bool { nix::sys::signal::kill(pid, None).is_err() } -use std::os::unix::io::FromRawFd; - #[cfg(test)] mod tests { use super::*; diff --git a/src/shadow-core/src/nscd.rs b/src/shadow-core/src/nscd.rs index 7e061d9..dac7e6b 100644 --- a/src/shadow-core/src/nscd.rs +++ b/src/shadow-core/src/nscd.rs @@ -19,8 +19,11 @@ use std::process::Command; /// Silently succeeds if `nscd`/`sssd` is not installed or not running — /// this matches GNU shadow-utils behavior. pub fn invalidate_cache(database: &str) { - // nscd -i - let _ = Command::new("nscd").arg("-i").arg(database).status(); + // Use absolute paths to avoid PATH-based lookups in setuid context. + let _ = Command::new("/usr/sbin/nscd") + .arg("-i") + .arg(database) + .status(); // sssd: sss_cache with the appropriate flag let flag = match database { @@ -28,5 +31,5 @@ pub fn invalidate_cache(database: &str) { "group" => "-G", _ => return, }; - let _ = Command::new("sss_cache").arg(flag).status(); + let _ = Command::new("/usr/sbin/sss_cache").arg(flag).status(); } diff --git a/src/shadow-core/src/passwd.rs b/src/shadow-core/src/passwd.rs index 58721c8..a908101 100644 --- a/src/shadow-core/src/passwd.rs +++ b/src/shadow-core/src/passwd.rs @@ -54,29 +54,50 @@ impl FromStr for PasswdEntry { type Err = ShadowError; fn from_str(line: &str) -> Result { - let fields: Vec<&str> = line.split(':').collect(); - if fields.len() != 7 { - return Err(ShadowError::Parse(format!( - "expected 7 colon-separated fields, got {}", - fields.len() - ))); + // Use splitn(8) to detect extra fields without allocating a Vec. + let mut fields = line.splitn(8, ':'); + + let name = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing name".into()))?; + let passwd = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing passwd".into()))?; + let uid_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing uid".into()))?; + let gid_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing gid".into()))?; + let gecos = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing gecos".into()))?; + let home = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing home".into()))?; + let shell = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing shell".into()))?; + + if fields.next().is_some() { + return Err(ShadowError::Parse("too many fields".into())); } - let uid = fields[2] + let uid = uid_str .parse::() - .map_err(|e| ShadowError::Parse(format!("invalid UID '{}': {e}", fields[2])))?; - let gid = fields[3] + .map_err(|e| ShadowError::Parse(format!("invalid UID '{uid_str}': {e}")))?; + let gid = gid_str .parse::() - .map_err(|e| ShadowError::Parse(format!("invalid GID '{}': {e}", fields[3])))?; + .map_err(|e| ShadowError::Parse(format!("invalid GID '{gid_str}': {e}")))?; Ok(Self { - name: fields[0].to_string(), - passwd: fields[1].to_string(), + name: name.to_string(), + passwd: passwd.to_string(), uid, gid, - gecos: fields[4].to_string(), - home: fields[5].to_string(), - shell: fields[6].to_string(), + gecos: gecos.to_string(), + home: home.to_string(), + shell: shell.to_string(), }) } } diff --git a/src/shadow-core/src/shadow.rs b/src/shadow-core/src/shadow.rs index 8ec7e6f..3cc0cdc 100644 --- a/src/shadow-core/src/shadow.rs +++ b/src/shadow-core/src/shadow.rs @@ -148,24 +148,51 @@ impl FromStr for ShadowEntry { type Err = ShadowError; fn from_str(line: &str) -> Result { - let fields: Vec<&str> = line.split(':').collect(); - if fields.len() != 9 { - return Err(ShadowError::Parse(format!( - "expected 9 colon-separated fields, got {}", - fields.len() - ))); + // Use splitn(10) to detect extra fields without allocating a Vec. + let mut fields = line.splitn(10, ':'); + + let name = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing name".into()))?; + let passwd = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing passwd".into()))?; + let last_change_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing last_change".into()))?; + let min_age_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing min_age".into()))?; + let max_age_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing max_age".into()))?; + let warn_days_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing warn_days".into()))?; + let inactive_days_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing inactive_days".into()))?; + let expire_date_str = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing expire_date".into()))?; + let reserved = fields + .next() + .ok_or_else(|| ShadowError::Parse("missing reserved".into()))?; + + if fields.next().is_some() { + return Err(ShadowError::Parse("too many fields".into())); } Ok(Self { - name: fields[0].to_string(), - passwd: fields[1].to_string(), - last_change: parse_optional_field(fields[2])?, - min_age: parse_optional_field(fields[3])?, - max_age: parse_optional_field(fields[4])?, - warn_days: parse_optional_field(fields[5])?, - inactive_days: parse_optional_field(fields[6])?, - expire_date: parse_optional_field(fields[7])?, - reserved: fields[8].to_string(), + name: name.to_string(), + passwd: passwd.to_string(), + last_change: parse_optional_field(last_change_str)?, + min_age: parse_optional_field(min_age_str)?, + max_age: parse_optional_field(max_age_str)?, + warn_days: parse_optional_field(warn_days_str)?, + inactive_days: parse_optional_field(inactive_days_str)?, + expire_date: parse_optional_field(expire_date_str)?, + reserved: reserved.to_string(), }) } }