From 6ea1a4888322aee8bf6ec2da646cc95efd408df9 Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Wed, 10 Jun 2026 12:36:37 +0200 Subject: [PATCH 1/4] shadow-core, tools: source the EACCES message from the OS (#159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manual root-privilege guards across the account tools printed a hardcoded "Permission denied." string. Route that text through libc's strerror instead (via std::io::Error::from_raw_os_error), so the wording comes from the host OS and is translated by glibc on localized systems — the same way GNU coreutils renders system errors (e.g. cat: Is a directory). This also keeps the string out of our source tree, shrinking the residual clean-room surface that motivated #159. Add shadow_core::os_error with strerror(errno) and permission_denied(), and convert the 13 caller_is_root() guards in chage, chpasswd, groupadd, groupdel, groupmod, passwd, useradd, userdel, usermod to use it. Scope: this covers the errno-mappable class only. True I/O errors already render the OS strerror via ShadowError::Io/IoPath. Domain errors (e.g. "group already exists") have no OS equivalent and stay in-tree. The gettext-catalog idea was evaluated and declined in #159 (the English msgid must remain in source as the lookup key, so it does not remove the exposure). Note: output changes from "Permission denied." to the OS text "Permission denied" (no trailing period; translated off-English). No tests asserted the old literal. --- src/shadow-core/src/lib.rs | 1 + src/shadow-core/src/os_error.rs | 44 +++++++++++++++++++++++++++++++++ src/uu/chage/src/chage.rs | 9 +++++-- src/uu/chpasswd/src/chpasswd.rs | 4 ++- src/uu/groupadd/src/groupadd.rs | 2 +- src/uu/groupdel/src/groupdel.rs | 2 +- src/uu/groupmod/src/groupmod.rs | 2 +- src/uu/passwd/src/passwd.rs | 14 ++++++++--- src/uu/useradd/src/useradd.rs | 2 +- src/uu/userdel/src/userdel.rs | 4 ++- src/uu/usermod/src/usermod.rs | 2 +- 11 files changed, 74 insertions(+), 12 deletions(-) create mode 100644 src/shadow-core/src/os_error.rs diff --git a/src/shadow-core/src/lib.rs b/src/shadow-core/src/lib.rs index 773373f..a16c6a7 100644 --- a/src/shadow-core/src/lib.rs +++ b/src/shadow-core/src/lib.rs @@ -10,6 +10,7 @@ pub mod cli; pub mod error; +pub mod os_error; pub mod passwd; pub mod validate; diff --git a/src/shadow-core/src/os_error.rs b/src/shadow-core/src/os_error.rs new file mode 100644 index 0000000..bc70ae1 --- /dev/null +++ b/src/shadow-core/src/os_error.rs @@ -0,0 +1,44 @@ +// This file is part of the shadow-rs package. +// +// For the full copyright and license information, please view the LICENSE +// file that was distributed with this source code. + +//! Error text sourced from the operating system instead of hardcoded. +//! +//! Wording for conditions that map to a libc `errno` is taken from +//! `strerror` (via [`std::io::Error`]) rather than carried as a string +//! literal in our tree. This keeps the text matching the host OS and lets +//! glibc translate it on localized systems — the same way GNU coreutils +//! renders system errors (e.g. `cat: /tmp: Is a directory`). See issue #159. + +/// The operating system's message for a raw `errno`. +/// +/// For example `EACCES` renders as "Permission denied" on English locales +/// and the translated equivalent elsewhere. On targets whose libc does not +/// translate (musl), this is the untranslated English text. +#[must_use] +pub fn strerror(errno: i32) -> String { + std::io::Error::from_raw_os_error(errno).to_string() +} + +/// The OS message for `EACCES` ("Permission denied"), sourced from libc. +#[must_use] +pub fn permission_denied() -> String { + strerror(libc::EACCES) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn permission_denied_is_nonempty_and_os_sourced() { + // We assert the shape, not the exact text: the wording comes from the + // host libc and may be localized, so hardcoding it would defeat the + // purpose of this module. + let msg = permission_denied(); + assert!(!msg.is_empty()); + // Same value libc would give for EACCES directly. + assert_eq!(msg, strerror(libc::EACCES)); + } +} diff --git a/src/uu/chage/src/chage.rs b/src/uu/chage/src/chage.rs index cdaff08..801fca3 100644 --- a/src/uu/chage/src/chage.rs +++ b/src/uu/chage/src/chage.rs @@ -274,7 +274,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let current_user = shadow_core::hardening::current_username() .map_err(|e| ChageError::UnexpectedFailure(e.to_string()))?; if current_user != *login { - return Err(ChageError::PermissionDenied("Permission denied.".into()).into()); + return Err(ChageError::PermissionDenied( + shadow_core::os_error::permission_denied(), + ) + .into()); } } return cmd_list(&root, login); @@ -282,7 +285,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // All modification flags require root. if !shadow_core::hardening::caller_is_root() { - return Err(ChageError::PermissionDenied("Permission denied.".into()).into()); + return Err( + ChageError::PermissionDenied(shadow_core::os_error::permission_denied()).into(), + ); } if !has_modifications { diff --git a/src/uu/chpasswd/src/chpasswd.rs b/src/uu/chpasswd/src/chpasswd.rs index c1f6b0c..6e58be5 100644 --- a/src/uu/chpasswd/src/chpasswd.rs +++ b/src/uu/chpasswd/src/chpasswd.rs @@ -186,7 +186,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // chpasswd always requires root. if !shadow_core::hardening::caller_is_root() { - return Err(ChpasswdError::PermissionDenied("Permission denied.".into()).into()); + return Err( + ChpasswdError::PermissionDenied(shadow_core::os_error::permission_denied()).into(), + ); } let is_encrypted = matches.get_flag(options::ENCRYPTED); diff --git a/src/uu/groupadd/src/groupadd.rs b/src/uu/groupadd/src/groupadd.rs index a65644c..d83ed41 100644 --- a/src/uu/groupadd/src/groupadd.rs +++ b/src/uu/groupadd/src/groupadd.rs @@ -109,7 +109,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; if !shadow_core::hardening::caller_is_root() { - uucore::show_error!("Permission denied."); + uucore::show_error!("{}", shadow_core::os_error::permission_denied()); return Err(GroupaddError::AlreadyPrinted(1).into()); } diff --git a/src/uu/groupdel/src/groupdel.rs b/src/uu/groupdel/src/groupdel.rs index 39d7c83..32a7da2 100644 --- a/src/uu/groupdel/src/groupdel.rs +++ b/src/uu/groupdel/src/groupdel.rs @@ -97,7 +97,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; if !shadow_core::hardening::caller_is_root() { - uucore::show_error!("Permission denied."); + uucore::show_error!("{}", shadow_core::os_error::permission_denied()); return Err(GroupdelError::AlreadyPrinted(1).into()); } diff --git a/src/uu/groupmod/src/groupmod.rs b/src/uu/groupmod/src/groupmod.rs index dc7d337..0f326cb 100644 --- a/src/uu/groupmod/src/groupmod.rs +++ b/src/uu/groupmod/src/groupmod.rs @@ -109,7 +109,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; if !shadow_core::hardening::caller_is_root() { - uucore::show_error!("Permission denied."); + uucore::show_error!("{}", shadow_core::os_error::permission_denied()); return Err(GroupmodError::AlreadyPrinted(1).into()); } diff --git a/src/uu/passwd/src/passwd.rs b/src/uu/passwd/src/passwd.rs index cafcf92..d3f3fd2 100644 --- a/src/uu/passwd/src/passwd.rs +++ b/src/uu/passwd/src/passwd.rs @@ -188,12 +188,18 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Non-root users can only view their own status. if !shadow_core::hardening::caller_is_root() { if show_all { - return Err(PasswdError::PermissionDenied("Permission denied.".into()).into()); + return Err(PasswdError::PermissionDenied( + shadow_core::os_error::permission_denied(), + ) + .into()); } let current_user = shadow_core::hardening::current_username() .map_err(|e| PasswdError::UnexpectedFailure(e.to_string()))?; if current_user != target_user { - return Err(PasswdError::PermissionDenied("Permission denied.".into()).into()); + return Err(PasswdError::PermissionDenied( + shadow_core::os_error::permission_denied(), + ) + .into()); } } @@ -218,7 +224,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // caller to be root. Non-root users can only change their own password // (the default PAM path below). if (has_mutation || has_aging) && !shadow_core::hardening::caller_is_root() { - return Err(PasswdError::PermissionDenied("Permission denied.".into()).into()); + return Err( + PasswdError::PermissionDenied(shadow_core::os_error::permission_denied()).into(), + ); } // When a mutation flag and aging flags are both present, apply both in a diff --git a/src/uu/useradd/src/useradd.rs b/src/uu/useradd/src/useradd.rs index 4ce7a13..c6f1b9c 100644 --- a/src/uu/useradd/src/useradd.rs +++ b/src/uu/useradd/src/useradd.rs @@ -290,7 +290,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Only root can add users. if !shadow_core::hardening::caller_is_root() { - uucore::show_error!("Permission denied."); + uucore::show_error!("{}", shadow_core::os_error::permission_denied()); return Err(UseraddError::AlreadyPrinted(1).into()); } diff --git a/src/uu/userdel/src/userdel.rs b/src/uu/userdel/src/userdel.rs index b676b4e..a00ddce 100644 --- a/src/uu/userdel/src/userdel.rs +++ b/src/uu/userdel/src/userdel.rs @@ -97,7 +97,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Must be root. if !rustix::process::getuid().is_root() { - return Err(UserdelError::CantUpdatePasswd("Permission denied.".into()).into()); + return Err( + UserdelError::CantUpdatePasswd(shadow_core::os_error::permission_denied()).into(), + ); } // Read the user's home directory and UID from /etc/passwd BEFORE removing diff --git a/src/uu/usermod/src/usermod.rs b/src/uu/usermod/src/usermod.rs index 87d12fd..cb8d13f 100644 --- a/src/uu/usermod/src/usermod.rs +++ b/src/uu/usermod/src/usermod.rs @@ -99,7 +99,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let root = SysRoot::new(prefix); if !rustix::process::getuid().is_root() { - return Err(UsermodError::CantUpdate("Permission denied.".into()).into()); + return Err(UsermodError::CantUpdate(shadow_core::os_error::permission_denied()).into()); } // Block signals for the passwd lock→write critical section only. From 15f8ecc505c041c3cc623c5e2e80e8151acfb0f4 Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Wed, 10 Jun 2026 12:50:16 +0200 Subject: [PATCH 2/4] shadow-core: strip Rust's "(os error N)" suffix from strerror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit io::Error::from_raw_os_error(e).to_string() renders as "Permission denied (os error 13)" — Rust appends its own suffix to the libc text. Strip it so the message is the bare OS string (matching GNU/coreutils output), and add a regression test that the suffix is absent. Addresses review feedback on #171. --- src/shadow-core/src/os_error.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/shadow-core/src/os_error.rs b/src/shadow-core/src/os_error.rs index bc70ae1..e1f922c 100644 --- a/src/shadow-core/src/os_error.rs +++ b/src/shadow-core/src/os_error.rs @@ -18,7 +18,16 @@ /// translate (musl), this is the untranslated English text. #[must_use] pub fn strerror(errno: i32) -> String { - std::io::Error::from_raw_os_error(errno).to_string() + // `io::Error::from_raw_os_error` carries libc's `strerror` text, but its + // `Display` appends Rust's own " (os error N)" suffix. Strip that so the + // result is the bare OS message (matching GNU/coreutils output). The + // suffix is emitted verbatim in English regardless of locale, so a + // localized message cannot contain it earlier in the string. + let rendered = std::io::Error::from_raw_os_error(errno).to_string(); + match rendered.rfind(" (os error ") { + Some(cut) => rendered[..cut].to_string(), + None => rendered, + } } /// The OS message for `EACCES` ("Permission denied"), sourced from libc. @@ -41,4 +50,13 @@ mod tests { // Same value libc would give for EACCES directly. assert_eq!(msg, strerror(libc::EACCES)); } + + #[test] + fn strerror_drops_the_rust_os_error_suffix() { + // Regression guard: the bare OS message must not carry Rust's + // " (os error N)" rendering (the bug that made permission_denied() + // return "Permission denied (os error 13)"). + let msg = strerror(libc::EACCES); + assert!(!msg.contains("(os error"), "got: {msg:?}"); + } } From dbfec6e5b7017dd4d1e5e59049091591176aa833 Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Wed, 10 Jun 2026 13:34:04 +0200 Subject: [PATCH 3/4] shadow-core: use uucore::error::strip_errno instead of hand-rolling Replace the local " (os error N)" suffix stripping with uucore's existing strip_errno helper, per review on #171. Adds uucore (already in the dependency tree via the tool crates) to shadow-core. --- Cargo.lock | 1 + src/shadow-core/Cargo.toml | 1 + src/shadow-core/src/os_error.rs | 13 ++++--------- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b06d33e..a352bf4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -662,6 +662,7 @@ dependencies = [ "subtle", "tempfile", "thiserror", + "uucore", "zeroize", ] diff --git a/src/shadow-core/Cargo.toml b/src/shadow-core/Cargo.toml index 8bb8a97..ed335df 100644 --- a/src/shadow-core/Cargo.toml +++ b/src/shadow-core/Cargo.toml @@ -18,6 +18,7 @@ path = "src/lib.rs" libc = { workspace = true } rustix = { workspace = true } thiserror = { workspace = true } +uucore = { workspace = true } zeroize = { workspace = true } subtle = { workspace = true } landlock = { workspace = true, optional = true } diff --git a/src/shadow-core/src/os_error.rs b/src/shadow-core/src/os_error.rs index e1f922c..147a05a 100644 --- a/src/shadow-core/src/os_error.rs +++ b/src/shadow-core/src/os_error.rs @@ -19,15 +19,10 @@ #[must_use] pub fn strerror(errno: i32) -> String { // `io::Error::from_raw_os_error` carries libc's `strerror` text, but its - // `Display` appends Rust's own " (os error N)" suffix. Strip that so the - // result is the bare OS message (matching GNU/coreutils output). The - // suffix is emitted verbatim in English regardless of locale, so a - // localized message cannot contain it earlier in the string. - let rendered = std::io::Error::from_raw_os_error(errno).to_string(); - match rendered.rfind(" (os error ") { - Some(cut) => rendered[..cut].to_string(), - None => rendered, - } + // `Display` appends Rust's own " (os error N)" suffix. `strip_errno` (the + // same helper uucore uses for its own I/O errors) removes it, leaving the + // bare OS message — matching GNU/coreutils output. + uucore::error::strip_errno(&std::io::Error::from_raw_os_error(errno)) } /// The OS message for `EACCES` ("Permission denied"), sourced from libc. From 92f639dff255253963359b08d97c3ecf106c244f Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Wed, 10 Jun 2026 14:40:26 +0200 Subject: [PATCH 4/4] shadow-core: drop the single-use strerror wrapper Per review on #171, inline the one caller: permission_denied() now calls uucore::error::strip_errno directly. Also disambiguate the module doc now that the local strerror fn is gone (it refers to libc's strerror). --- src/shadow-core/src/os_error.rs | 51 +++++++++++---------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/src/shadow-core/src/os_error.rs b/src/shadow-core/src/os_error.rs index 147a05a..17e290c 100644 --- a/src/shadow-core/src/os_error.rs +++ b/src/shadow-core/src/os_error.rs @@ -5,30 +5,23 @@ //! Error text sourced from the operating system instead of hardcoded. //! -//! Wording for conditions that map to a libc `errno` is taken from -//! `strerror` (via [`std::io::Error`]) rather than carried as a string -//! literal in our tree. This keeps the text matching the host OS and lets -//! glibc translate it on localized systems — the same way GNU coreutils -//! renders system errors (e.g. `cat: /tmp: Is a directory`). See issue #159. - -/// The operating system's message for a raw `errno`. -/// -/// For example `EACCES` renders as "Permission denied" on English locales -/// and the translated equivalent elsewhere. On targets whose libc does not -/// translate (musl), this is the untranslated English text. -#[must_use] -pub fn strerror(errno: i32) -> String { - // `io::Error::from_raw_os_error` carries libc's `strerror` text, but its - // `Display` appends Rust's own " (os error N)" suffix. `strip_errno` (the - // same helper uucore uses for its own I/O errors) removes it, leaving the - // bare OS message — matching GNU/coreutils output. - uucore::error::strip_errno(&std::io::Error::from_raw_os_error(errno)) -} +//! Wording for conditions that map to a libc `errno` is taken from the OS +//! (libc's `strerror`, surfaced through [`std::io::Error`]) rather than +//! carried as a string literal in our tree. This keeps the text matching the +//! host OS and lets glibc translate it on localized systems — the same way +//! GNU coreutils renders system errors (e.g. `cat: /tmp: Is a directory`). +//! See issue #159. /// The OS message for `EACCES` ("Permission denied"), sourced from libc. +/// +/// Rendered as "Permission denied" on English locales and the translated +/// equivalent elsewhere; on a libc that does not translate (musl) it is the +/// untranslated English text. `strip_errno` (the helper uucore uses for its +/// own I/O errors) drops the " (os error N)" suffix that `io::Error`'s +/// `Display` appends, leaving the bare OS message — matching coreutils output. #[must_use] pub fn permission_denied() -> String { - strerror(libc::EACCES) + uucore::error::strip_errno(&std::io::Error::from_raw_os_error(libc::EACCES)) } #[cfg(test)] @@ -36,22 +29,12 @@ mod tests { use super::*; #[test] - fn permission_denied_is_nonempty_and_os_sourced() { - // We assert the shape, not the exact text: the wording comes from the - // host libc and may be localized, so hardcoding it would defeat the - // purpose of this module. + fn permission_denied_is_bare_os_message() { let msg = permission_denied(); + // Non-empty, and the bare OS text — not Rust's "... (os error 13)" + // rendering (the regression that suffix-stripping prevents). We assert + // the shape, not the exact wording, since libc may localize it. assert!(!msg.is_empty()); - // Same value libc would give for EACCES directly. - assert_eq!(msg, strerror(libc::EACCES)); - } - - #[test] - fn strerror_drops_the_rust_os_error_suffix() { - // Regression guard: the bare OS message must not carry Rust's - // " (os error N)" rendering (the bug that made permission_denied() - // return "Permission denied (os error 13)"). - let msg = strerror(libc::EACCES); assert!(!msg.contains("(os error"), "got: {msg:?}"); } }