From b8aca4df241609140263270d986d5fae4fb4c3f3 Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Wed, 22 Apr 2026 13:15:38 +0200 Subject: [PATCH 1/2] shadow-core: fix 6 Copilot findings from rustix migration (#147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. sigprocmask comment: "process-wide" → "per-thread" (correct on Linux; effectively process-wide for single-threaded shadow-rs tools) 2. sigemptyset/sigaddset: check return values, propagate errors 3. lookup_username_by_uid: restore NSS-backed lookup via getpwuid_r instead of reading /etc/passwd directly. Fixes regression for LDAP/SSSD/systemd-homed environments. 4. unsafe_code allowlist: update lib.rs comment to include process module 5. Doc comment: block_signals → block_critical_signals Fixes #147. --- src/shadow-core/src/hardening.rs | 38 ++++++--- src/shadow-core/src/lib.rs | 4 +- src/shadow-core/src/process.rs | 130 ++++++++++++++++++++++++++++--- 3 files changed, 150 insertions(+), 22 deletions(-) diff --git a/src/shadow-core/src/hardening.rs b/src/shadow-core/src/hardening.rs index cb1f33e..880ef48 100644 --- a/src/shadow-core/src/hardening.rs +++ b/src/shadow-core/src/hardening.rs @@ -142,27 +142,45 @@ pub fn current_username() -> Result { lookup_username_by_uid(uid) } -/// Look up a username by UID from `/etc/passwd`. +/// Look up a username by UID via NSS (`getpwuid_r`). +/// +/// Uses the system name-service switch, so it works with LDAP, SSSD, +/// systemd-homed, and other backends — not just `/etc/passwd`. pub fn lookup_username_by_uid(uid: u32) -> Result { - let entries = crate::passwd::read_passwd_file(std::path::Path::new("/etc/passwd"))?; - match entries.iter().find(|e| e.uid == uid) { - Some(entry) => Ok(entry.name.clone()), - None => Err(crate::error::ShadowError::Other( + match crate::process::lookup_username(uid) { + Ok(Some(name)) => Ok(name), + Ok(None) => Err(crate::error::ShadowError::Other( format!("cannot determine username for uid {uid}").into(), )), + Err(e) => Err(crate::error::ShadowError::Other( + format!("NSS lookup failed for uid {uid}: {e}").into(), + )), } } -/// Look up a passwd entry by UID from `/etc/passwd`. +/// Look up a passwd entry by UID via NSS (`getpwuid_r`). +/// +/// Uses the system name-service switch, so it works with LDAP, SSSD, +/// systemd-homed, and other backends — not just `/etc/passwd`. pub fn lookup_passwd_entry_by_uid( uid: u32, ) -> Result { - let entries = crate::passwd::read_passwd_file(std::path::Path::new("/etc/passwd"))?; - match entries.into_iter().find(|e| e.uid == uid) { - Some(entry) => Ok(entry), - None => Err(crate::error::ShadowError::Other( + match crate::process::getpwuid(uid) { + Ok(Some(pw)) => Ok(crate::passwd::PasswdEntry { + name: pw.name, + passwd: pw.passwd, + uid: pw.uid, + gid: pw.gid, + gecos: pw.gecos, + home: pw.home, + shell: pw.shell, + }), + Ok(None) => Err(crate::error::ShadowError::Other( format!("no passwd entry for uid {uid}").into(), )), + Err(e) => Err(crate::error::ShadowError::Other( + format!("NSS lookup failed for uid {uid}: {e}").into(), + )), } } diff --git a/src/shadow-core/src/lib.rs b/src/shadow-core/src/lib.rs index 643fb19..0fc89f7 100644 --- a/src/shadow-core/src/lib.rs +++ b/src/shadow-core/src/lib.rs @@ -27,8 +27,8 @@ pub mod login_defs; #[cfg(feature = "subid")] pub mod subid; -// PAM and crypt are C libraries — FFI inherently requires unsafe. -// These are the ONLY modules where unsafe_code is permitted. +// PAM, crypt, and process are C library boundaries — FFI inherently +// requires unsafe. These are the ONLY modules where unsafe_code is permitted. #[cfg(feature = "pam")] #[allow(unsafe_code)] pub mod pam; diff --git a/src/shadow-core/src/process.rs b/src/shadow-core/src/process.rs index 840c854..11dad63 100644 --- a/src/shadow-core/src/process.rs +++ b/src/shadow-core/src/process.rs @@ -2,14 +2,14 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore setuid seteuid setgid initgroups sigprocmask +// spell-checker:ignore setuid seteuid setgid initgroups sigprocmask getpwuid //! Process-level POSIX wrappers for setuid-root tools. //! //! These functions call libc directly because rustix intentionally does not -//! provide process-wide `setuid`/`setgid`/`sigprocmask` (they require libc -//! coordination for thread safety). The `libc` crate is already a dependency -//! for PAM FFI. +//! provide process-wide `setuid`/`setgid` or per-thread `sigprocmask` (they +//! require libc coordination for thread safety). The `libc` crate is already +//! a dependency for PAM FFI. //! //! This is one of the few modules that permits `unsafe` — all unsafe is //! confined to well-understood POSIX C library calls. @@ -98,10 +98,11 @@ pub fn execv(path: &CStr, argv: &[&CStr]) -> io::Error { } // --------------------------------------------------------------------------- -// Signal blocking (process-wide via libc) +// Signal blocking (per-thread via libc sigprocmask) // --------------------------------------------------------------------------- -/// A saved signal mask, used by [`block_signals`] and [`restore_signals`]. +/// A saved signal mask, used by [`block_critical_signals`] and +/// [`restore_signals`]. /// /// Wraps a `libc::sigset_t`. pub struct SavedSigSet { @@ -110,16 +111,27 @@ pub struct SavedSigSet { /// Block `SIGINT`, `SIGTERM`, `SIGHUP` and return the previous signal mask. /// +/// Calls `sigprocmask`, which modifies the *calling thread's* signal mask. +/// For single-threaded shadow-rs tools this is effectively process-wide. +/// /// Prevents these signals from interrupting a lock-modify-write sequence. pub fn block_critical_signals() -> io::Result { // SAFETY: sigemptyset, sigaddset, and sigprocmask are standard POSIX // functions. We initialize the sigset_t with sigemptyset before use. unsafe { let mut block_set: libc::sigset_t = std::mem::zeroed(); - libc::sigemptyset(&raw mut block_set); - libc::sigaddset(&raw mut block_set, libc::SIGINT); - libc::sigaddset(&raw mut block_set, libc::SIGTERM); - libc::sigaddset(&raw mut block_set, libc::SIGHUP); + if libc::sigemptyset(&raw mut block_set) != 0 { + return Err(io::Error::last_os_error()); + } + if libc::sigaddset(&raw mut block_set, libc::SIGINT) != 0 { + return Err(io::Error::last_os_error()); + } + if libc::sigaddset(&raw mut block_set, libc::SIGTERM) != 0 { + return Err(io::Error::last_os_error()); + } + if libc::sigaddset(&raw mut block_set, libc::SIGHUP) != 0 { + return Err(io::Error::last_os_error()); + } let mut old_set: libc::sigset_t = std::mem::zeroed(); let ret = libc::sigprocmask(libc::SIG_BLOCK, &raw const block_set, &raw mut old_set); @@ -147,3 +159,101 @@ pub fn restore_signals(saved: &SavedSigSet) -> io::Result<()> { Err(io::Error::last_os_error()) } } + +// --------------------------------------------------------------------------- +// NSS user lookup (getpwuid_r) +// --------------------------------------------------------------------------- + +/// Result of an NSS user lookup via `getpwuid_r`. +/// +/// Contains the fields from `struct passwd` that shadow-rs tools need. +/// Unlike reading `/etc/passwd` directly, this goes through NSS and works +/// with LDAP, SSSD, systemd-homed, and other name-service backends. +pub struct PwEntry { + /// Login name. + pub name: String, + /// Encrypted password (usually `x`). + pub passwd: String, + /// Numeric user ID. + pub uid: u32, + /// Numeric primary group ID. + pub gid: u32, + /// GECOS / comment field. + pub gecos: String, + /// Home directory. + pub home: String, + /// Login shell. + pub shell: String, +} + +/// Look up a user by UID via `getpwuid_r` (NSS-backed). +/// +/// Returns `None` if no user exists for the given UID. +/// Returns `Err` on system errors (e.g., I/O failure in NSS backend). +pub fn getpwuid(uid: u32) -> io::Result> { + // Start with a 1 KiB buffer; grow on ERANGE. + let mut buf_size: usize = 1024; + + loop { + let mut pwd: libc::passwd = unsafe { std::mem::zeroed() }; + let mut buf: Vec = vec![0u8; buf_size]; + let mut result: *mut libc::passwd = std::ptr::null_mut(); + + // SAFETY: getpwuid_r is a POSIX thread-safe function. We pass a + // properly sized buffer and a zeroed passwd struct. The result + // pointer tells us whether an entry was found. + let ret = unsafe { + libc::getpwuid_r( + uid, + &raw mut pwd, + buf.as_mut_ptr().cast::(), + buf_size, + &raw mut result, + ) + }; + + if ret == libc::ERANGE { + // Buffer too small — double and retry. + buf_size = buf_size.saturating_mul(2); + if buf_size > 1024 * 1024 { + return Err(io::Error::new( + io::ErrorKind::OutOfMemory, + "getpwuid_r buffer exceeded 1 MiB", + )); + } + continue; + } + + if ret != 0 { + return Err(io::Error::from_raw_os_error(ret)); + } + + if result.is_null() { + // No entry found for this UID. + return Ok(None); + } + + // SAFETY: getpwuid_r succeeded and `result` is non-null, so all + // string fields in `pwd` point into `buf` and are valid C strings. + let entry = unsafe { + PwEntry { + name: CStr::from_ptr(pwd.pw_name).to_string_lossy().into_owned(), + passwd: CStr::from_ptr(pwd.pw_passwd).to_string_lossy().into_owned(), + uid: pwd.pw_uid, + gid: pwd.pw_gid, + gecos: CStr::from_ptr(pwd.pw_gecos).to_string_lossy().into_owned(), + home: CStr::from_ptr(pwd.pw_dir).to_string_lossy().into_owned(), + shell: CStr::from_ptr(pwd.pw_shell).to_string_lossy().into_owned(), + } + }; + + return Ok(Some(entry)); + } +} + +/// Look up a username by UID via NSS (`getpwuid_r`). +/// +/// Convenience wrapper that returns just the login name. +pub fn lookup_username(uid: u32) -> io::Result> { + Ok(getpwuid(uid)?.map(|e| e.name)) +} From bd5861d9642299a55fa6801eb239c4e9b7a0aaad Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Wed, 22 Apr 2026 13:49:38 +0200 Subject: [PATCH 2/2] process: guard CStr::from_ptr against null NSS fields --- src/shadow-core/src/process.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/shadow-core/src/process.rs b/src/shadow-core/src/process.rs index 11dad63..95dc54b 100644 --- a/src/shadow-core/src/process.rs +++ b/src/shadow-core/src/process.rs @@ -192,6 +192,7 @@ pub struct PwEntry { /// Returns `Err` on system errors (e.g., I/O failure in NSS backend). pub fn getpwuid(uid: u32) -> io::Result> { // Start with a 1 KiB buffer; grow on ERANGE. + const MAX_BUF: usize = 1024 * 1024; let mut buf_size: usize = 1024; loop { @@ -215,10 +216,9 @@ pub fn getpwuid(uid: u32) -> io::Result> { if ret == libc::ERANGE { // Buffer too small — double and retry. buf_size = buf_size.saturating_mul(2); - if buf_size > 1024 * 1024 { - return Err(io::Error::new( - io::ErrorKind::OutOfMemory, - "getpwuid_r buffer exceeded 1 MiB", + if buf_size > MAX_BUF { + return Err(io::Error::other( + "getpwuid_r: ERANGE persists beyond 1 MiB buffer cap", )); } continue; @@ -233,17 +233,25 @@ pub fn getpwuid(uid: u32) -> io::Result> { return Ok(None); } - // SAFETY: getpwuid_r succeeded and `result` is non-null, so all - // string fields in `pwd` point into `buf` and are valid C strings. + // SAFETY: getpwuid_r succeeded and `result` is non-null, so `pwd` + // is populated. String fields should point into `buf`, but some NSS + // backends may return null for optional fields — guard defensively. let entry = unsafe { + let str_field = |ptr: *const libc::c_char| -> String { + if ptr.is_null() { + String::new() + } else { + CStr::from_ptr(ptr).to_string_lossy().into_owned() + } + }; PwEntry { - name: CStr::from_ptr(pwd.pw_name).to_string_lossy().into_owned(), - passwd: CStr::from_ptr(pwd.pw_passwd).to_string_lossy().into_owned(), + name: str_field(pwd.pw_name), + passwd: str_field(pwd.pw_passwd), uid: pwd.pw_uid, gid: pwd.pw_gid, - gecos: CStr::from_ptr(pwd.pw_gecos).to_string_lossy().into_owned(), - home: CStr::from_ptr(pwd.pw_dir).to_string_lossy().into_owned(), - shell: CStr::from_ptr(pwd.pw_shell).to_string_lossy().into_owned(), + gecos: str_field(pwd.pw_gecos), + home: str_field(pwd.pw_dir), + shell: str_field(pwd.pw_shell), } };