From 15f61e25317194ea7486a0b888a2bd901bd8a21c Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Sun, 6 Sep 2026 14:10:10 +0200 Subject: [PATCH 1/2] chgpasswd: set group passwords in batch chgpasswd is chpasswd's counterpart for groups: it reads group:password lines from stdin and applies them. It is the seventeenth tool and the second of the nine that both Debian and Fedora ship and we did not. It follows chpasswd's rule, which is what makes a batch tool safe to run unattended: every line is parsed, every group resolved and every password hashed before any file is written, so a batch naming one group that does not exist changes nothing rather than applying the lines above it and stopping. The two account files are then written in one transaction. Where the hash goes was established by running the GNU tool, not by reading it. With /etc/gshadow present the hash belongs there and /etc/group keeps the `x` placeholder -- /etc/group is world-readable. Without it the hash goes into /etc/group, and no gshadow file is created: conjuring one up would change how every other tool on the host reads group passwords. Three deliberate divergences, all refusals: - `-c NONE`, which GNU honours by storing the password as clear text in /etc/gshadow. A readable group password is worth no more than none at all, and `-e` already writes a field verbatim when that is what is wanted. The message says so, or an operator would just try `-c MD5`. - `-m` and `-c MD5`/`-c DES`, as chpasswd already refuses them. - An empty password in plaintext mode: hashing "" yields a valid hash that a bare Enter matches, which is a group anyone can enter rather than a group with no password. `-e ''` remains the way to clear the field. A field containing a colon is refused, since it would split the gshadow line and corrupt the file. The GNU tool refuses it too and likewise leaves the file untouched, so this is agreement, not divergence. The e2e suite checks the tools against each other rather than only against the files: the password chgpasswd sets is the one sg then accepts from a non-member, and a wrong one is refused. A hash written in a format nothing can verify would pass a file-shape assertion and fail this one. Verified: 798 tests on debian, alpine and fedora; make check clean; 273 e2e assertions against a real install; 44 GNU comparisons. --- CHANGELOG.md | 10 + Cargo.lock | 12 + Cargo.toml | 5 +- Makefile | 4 +- README.md | 9 +- docs/man/chgpasswd.8.md | 141 ++++++ src/bin/completions.rs | 4 + src/bin/shadow-rs.rs | 25 +- src/uu/chgpasswd/Cargo.toml | 35 ++ src/uu/chgpasswd/locales/en-US.ftl | 2 + src/uu/chgpasswd/src/chgpasswd.rs | 727 +++++++++++++++++++++++++++++ src/uu/chgpasswd/src/main.rs | 6 + tests/by-util/test_chgpasswd.rs | 320 +++++++++++++ tests/by-util/test_multicall.rs | 21 +- tests/e2e/deploy-test.sh | 53 ++- tests/gnu-compat.sh | 1 + tests/tests.rs | 2 + 17 files changed, 1362 insertions(+), 15 deletions(-) create mode 100644 docs/man/chgpasswd.8.md create mode 100644 src/uu/chgpasswd/Cargo.toml create mode 100644 src/uu/chgpasswd/locales/en-US.ftl create mode 100644 src/uu/chgpasswd/src/chgpasswd.rs create mode 100644 src/uu/chgpasswd/src/main.rs create mode 100644 tests/by-util/test_chgpasswd.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cb47515..1babe80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- `chgpasswd`, the seventeenth tool: it sets group passwords in batch from + stdin, the counterpart of `chpasswd` for groups. Every line is resolved and + hashed before any file is written, so a batch naming one group that does not + exist changes nothing. The hash goes to `/etc/gshadow` where that file + exists, with `x` left in `/etc/group`, and into `/etc/group` where it does + not — without creating a gshadow file, which would change how the rest of the + host reads group passwords. `-c NONE`, which GNU honours by storing the + password as clear text, is refused; `-e` already writes a field verbatim when + that is genuinely wanted + - `sg`, the sixteenth tool: it runs a single command with a different primary group. `sg` and `newgrp` decide who may enter a group by the same rules and enter it the same way — on a GNU system they are one binary reached through a diff --git a/Cargo.lock b/Cargo.lock index 73f963d..93eaaef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -715,6 +715,17 @@ dependencies = [ "uucore", ] +[[package]] +name = "uu_chgpasswd" +version = "0.4.0" +dependencies = [ + "clap", + "shadow-core", + "tempfile", + "uucore", + "zeroize", +] + [[package]] name = "uu_chpasswd" version = "0.4.0" @@ -844,6 +855,7 @@ dependencies = [ "tempfile", "uu_chage", "uu_chfn", + "uu_chgpasswd", "uu_chpasswd", "uu_chsh", "uu_gpasswd", diff --git a/Cargo.toml b/Cargo.toml index e5536d6..93440a2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "src/uu/chage", "src/uu/gpasswd", "src/uu/sg", + "src/uu/chgpasswd", ] [workspace.package] @@ -94,11 +95,12 @@ chsh = { optional = true, version = "0.4.0", package = "uu_chsh", path = "src/uu newgrp = { optional = true, version = "0.4.0", package = "uu_newgrp", path = "src/uu/newgrp" } gpasswd = { optional = true, version = "0.4.0", package = "uu_gpasswd", path = "src/uu/gpasswd" } sg = { optional = true, version = "0.4.0", package = "uu_sg", path = "src/uu/sg" } +chgpasswd = { optional = true, version = "0.4.0", package = "uu_chgpasswd", path = "src/uu/chgpasswd" } [features] default = ["passwd", "pwck", "useradd", "userdel", "usermod", "chpasswd", "chage", "groupadd", "groupdel", "groupmod", "grpck", "chfn", "chsh", "newgrp", "gpasswd", - "sg"] + "sg", "chgpasswd"] # PAM authentication (requires libpam-dev). The `?` matters: without it, # asking for PAM would drag in the three applets that can use it even when the @@ -134,6 +136,7 @@ groupdel = { version = "0.4.0", package = "uu_groupdel", path = "src/uu/groupdel groupmod = { version = "0.4.0", package = "uu_groupmod", path = "src/uu/groupmod" } newgrp = { version = "0.4.0", package = "uu_newgrp", path = "src/uu/newgrp" } sg = { version = "0.4.0", package = "uu_sg", path = "src/uu/sg" } +chgpasswd = { version = "0.4.0", package = "uu_chgpasswd", path = "src/uu/chgpasswd" } passwd = { version = "0.4.0", package = "uu_passwd", path = "src/uu/passwd" } useradd = { version = "0.4.0", package = "uu_useradd", path = "src/uu/useradd" } userdel = { version = "0.4.0", package = "uu_userdel", path = "src/uu/userdel" } diff --git a/Makefile b/Makefile index 82db79f..715bec5 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ SBINDIR ?= $(PREFIX)/sbin SETUID_TOOLS = passwd chfn chsh newgrp gpasswd sg # Root-only tools (no setuid; fail at getuid() check for non-root callers). -ROOT_TOOLS = useradd userdel usermod chpasswd \ +ROOT_TOOLS = useradd userdel usermod chpasswd chgpasswd \ groupadd groupdel groupmod pwck grpck # Tools an ordinary user runs, and which therefore go in bin rather than sbin: @@ -107,7 +107,7 @@ test-arm64: build-arm64 test-gnu-compat: bash tests/gnu-compat.sh -# Default install: 16 standalone per-tool binaries, with the setuid layout and +# Default install: 17 standalone per-tool binaries, with the setuid layout and # the bin/sbin split GNU shadow-utils uses. Only $(SETUID_TOOLS) are setuid. install: build @for tool in $(SETUID_TOOLS); do \ diff --git a/README.md b/README.md index 485574e..c2364be 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ default-in-Ubuntu in under 3 years. This project follows that playbook. | `newgrp` | **Implemented.** Effective group change with crypt verification. | | `gpasswd` | **Implemented.** Group membership, administrators, and group password. | | `sg` | **Implemented.** Runs one command in another group; shares `newgrp`'s authorization. | +| `chgpasswd` | **Implemented.** Batch group passwords, applied all-or-nothing. | ## Building @@ -89,9 +90,9 @@ docker compose run --rm debian cargo build --release ### Install -Default install: 16 standalone per-tool binaries with least-privilege setuid +Default install: 17 standalone per-tool binaries with least-privilege setuid layout matching GNU shadow-utils. Only `passwd`, `chfn`, `chsh`, `newgrp`, -`gpasswd` and `sg` are installed setuid-root; the other 10 are plain `0755`. +`gpasswd` and `sg` are installed setuid-root; the other 11 are plain `0755`. ```shell sudo make install PREFIX=/usr/local @@ -142,8 +143,8 @@ would: tar xzf uu_shadow-x86_64-unknown-linux-gnu.tar.gz # or the -musl-static one sudo install -o root -g root -m 4755 \ uu_shadow-*/shadow-rs /usr/local/bin/shadow-rs -for tool in passwd chfn chsh newgrp gpasswd sg chage chpasswd groupadd groupdel \ - groupmod grpck pwck useradd userdel usermod; do +for tool in passwd chfn chsh newgrp gpasswd sg chage chpasswd chgpasswd groupadd \ + groupdel groupmod grpck pwck useradd userdel usermod; do sudo ln -sf shadow-rs "/usr/local/bin/$tool" done ``` diff --git a/docs/man/chgpasswd.8.md b/docs/man/chgpasswd.8.md new file mode 100644 index 0000000..adbff45 --- /dev/null +++ b/docs/man/chgpasswd.8.md @@ -0,0 +1,141 @@ +# chgpasswd(8) - update group passwords in batch mode + +## NAME + +chgpasswd - update group passwords in batch mode + +## SYNOPSIS + +**chgpasswd** [*options*] + +## DESCRIPTION + +The **chgpasswd** command reads a list of group and password pairs from +standard input and uses it to update a set of existing groups. It is +**chpasswd**(8)'s counterpart for groups. + +Each line is of the form: + +``` +group_name:password +``` + +Only the first colon separates the two fields, so a password may itself +contain colons — though one written into /etc/gshadow would split the line and +corrupt the file, and is refused. + +By default the supplied passwords are in clear text and are hashed before +being stored. The scheme comes from **ENCRYPT_METHOD** in /etc/login.defs, so +group passwords are hashed the same way as everything else on the host. + +## ALL OR NOTHING + +Every line is parsed, every named group is resolved, and every password is +hashed **before** any file is written. A batch naming one group that does not +exist changes nothing at all, rather than applying the lines before the bad +one and stopping. + +The account files are then written in one locked transaction, so a concurrent +**gpasswd**(1) or **groupmod**(8) cannot interleave with it. + +## WHERE THE PASSWORD IS STORED + +On a system with /etc/gshadow, the hash is written there and the group's +password field in /etc/group is set to `x`, which is what marks the password +as living in the shadowed file. /etc/group is world-readable; /etc/gshadow is +not. + +On a system without /etc/gshadow, the hash is written into /etc/group itself. +**chgpasswd** does not create a gshadow file: doing so would change how every +other tool on the host reads group passwords. + +A group present in /etc/group with no /etc/gshadow line gets one, carrying the +membership /etc/group already records. + +## OPTIONS + +**-c**, **--crypt-method** *METHOD* +: Use *METHOD* to hash the passwords instead of the configured default. + Supported: **SHA256**, **SHA512**, **YESCRYPT**. + +**-e**, **--encrypted** +: The supplied passwords are already hashed and are stored verbatim. This is + the only mode that may write an empty field, which is how a group password + is cleared. + +**-m**, **--md5** +: Rejected. See DIFFERENCES FROM GNU SHADOW below. + +**-R**, **--root** *CHROOT_DIR* +: Apply changes in *CHROOT_DIR* and use its configuration files. + +**-s**, **--sha-rounds** *ROUNDS* +: Iteration count for the SHA-2 schemes. Requires **-c**: a rounds count + without a scheme that takes one is meaningless, and ignoring it silently + would write a password the caller did not ask for. + +**-P**, **--prefix** *PREFIX_DIR* +: Read and write the account files under *PREFIX_DIR* without chrooting. + +## DIFFERENCES FROM GNU SHADOW + +**-m** and **-c MD5**, and **-c DES**, are refused rather than honoured. Both +schemes are broken, and a group password hashed with either is worth little +more than none at all. + +**-c NONE** is refused. GNU accepts it and stores the password as clear text +in /etc/gshadow. If a field really is to be written verbatim, **-e** does that +explicitly. + +An empty password in plaintext mode is refused: hashing an empty string +produces a valid hash that a bare Enter matches, which is a group anyone can +enter, not a group with no password. + +## EXIT STATUS + +**0** +: Success. Empty input succeeds having done nothing. + +**1** +: The passwords could not be changed. Nothing was written. + +**2** +: Invalid command syntax. + +**3** +: The **--root** directory could not be entered. + +## FILES + +/etc/group +: Group account information. + +/etc/gshadow +: Secure group account information. + +/etc/login.defs +: Shadow password suite configuration, read for **ENCRYPT_METHOD**. + +## EXAMPLES + +Set one group password: + +``` +# echo 'staff:correct horse battery staple' | chgpasswd +``` + +Apply a batch from a file, choosing the scheme: + +``` +# chgpasswd -c SHA512 < group-passwords.txt +``` + +Clear a group's password: + +``` +# echo 'staff:' | chgpasswd -e +``` + +## SEE ALSO + +chpasswd(8), gpasswd(1), group(5), gshadow(5), login.defs(5), newgrp(1), sg(1) diff --git a/src/bin/completions.rs b/src/bin/completions.rs index aaeaca7..fd6218e 100644 --- a/src/bin/completions.rs +++ b/src/bin/completions.rs @@ -27,6 +27,8 @@ fn get_tool_app(name: &str) -> Option { "chage" => Some(chage::uu_app()), #[cfg(feature = "chfn")] "chfn" => Some(chfn::uu_app()), + #[cfg(feature = "chgpasswd")] + "chgpasswd" => Some(chgpasswd::uu_app()), #[cfg(feature = "chpasswd")] "chpasswd" => Some(chpasswd::uu_app()), #[cfg(feature = "chsh")] @@ -66,6 +68,8 @@ fn all_tool_names() -> Vec<&'static str> { names.push("chage"); #[cfg(feature = "chfn")] names.push("chfn"); + #[cfg(feature = "chgpasswd")] + names.push("chgpasswd"); #[cfg(feature = "chpasswd")] names.push("chpasswd"); #[cfg(feature = "chsh")] diff --git a/src/bin/shadow-rs.rs b/src/bin/shadow-rs.rs index b787eba..9a47896 100644 --- a/src/bin/shadow-rs.rs +++ b/src/bin/shadow-rs.rs @@ -39,11 +39,13 @@ const SETUID_APPLETS: [&str; 6] = ["passwd", "chfn", "chsh", "newgrp", "gpasswd" // nothing, and the binding is then not mutated. #[allow(unused_mut)] fn applets() -> Vec<(&'static str, Applet)> { - let mut table: Vec<(&'static str, Applet)> = Vec::with_capacity(16); + let mut table: Vec<(&'static str, Applet)> = Vec::with_capacity(17); #[cfg(feature = "chage")] table.push(("chage", |a| chage::uumain(a.iter().cloned()))); #[cfg(feature = "chfn")] table.push(("chfn", |a| chfn::uumain(a.iter().cloned()))); + #[cfg(feature = "chgpasswd")] + table.push(("chgpasswd", |a| chgpasswd::uumain(a.iter().cloned()))); #[cfg(feature = "chpasswd")] table.push(("chpasswd", |a| chpasswd::uumain(a.iter().cloned()))); #[cfg(feature = "chsh")] @@ -230,9 +232,24 @@ fn print_available_utils() { mod tests { use super::*; - const ALL_TOOLS: [&str; 16] = [ - "chage", "chfn", "chpasswd", "chsh", "gpasswd", "groupadd", "groupdel", "groupmod", - "grpck", "newgrp", "passwd", "pwck", "sg", "useradd", "userdel", "usermod", + const ALL_TOOLS: [&str; 17] = [ + "chage", + "chfn", + "chgpasswd", + "chpasswd", + "chsh", + "gpasswd", + "groupadd", + "groupdel", + "groupmod", + "grpck", + "newgrp", + "passwd", + "pwck", + "sg", + "useradd", + "userdel", + "usermod", ]; // The table drives both dispatch and `--list`, so it must contain only diff --git a/src/uu/chgpasswd/Cargo.toml b/src/uu/chgpasswd/Cargo.toml new file mode 100644 index 0000000..e6b49da --- /dev/null +++ b/src/uu/chgpasswd/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "uu_chgpasswd" +version.workspace = true +edition.workspace = true +license.workspace = true +repository.workspace = true +homepage.workspace = true +keywords.workspace = true +categories.workspace = true +rust-version.workspace = true +description = "chgpasswd ~ (shadow-rs) update group passwords in batch" + +[lib] +path = "src/chgpasswd.rs" + +[[bin]] +name = "chgpasswd" +path = "src/main.rs" + +[dependencies] +clap = { workspace = true } +shadow-core = { workspace = true, features = ["crypt"] } +uucore = { workspace = true } +zeroize = { workspace = true } + +[dev-dependencies] +tempfile = { workspace = true } + +[lints] +workspace = true + +# Distributed via the `shadow-rs` multicall binary in the workspace root +# package, not as a standalone archive (see dist-workspace.toml, issue #207). +[package.metadata.dist] +dist = false diff --git a/src/uu/chgpasswd/locales/en-US.ftl b/src/uu/chgpasswd/locales/en-US.ftl new file mode 100644 index 0000000..8b13ce0 --- /dev/null +++ b/src/uu/chgpasswd/locales/en-US.ftl @@ -0,0 +1,2 @@ +chgpasswd-about = Update group passwords in batch mode +chgpasswd-usage = chgpasswd [options] diff --git a/src/uu/chgpasswd/src/chgpasswd.rs b/src/uu/chgpasswd/src/chgpasswd.rs new file mode 100644 index 0000000..fbce623 --- /dev/null +++ b/src/uu/chgpasswd/src/chgpasswd.rs @@ -0,0 +1,727 @@ +// 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. +// spell-checker:ignore chgpasswd chpasswd gshadow chroot nscd sysroot yescrypt + +//! `chgpasswd` — update group passwords in batch mode. +//! +//! Drop-in replacement for GNU shadow-utils `chgpasswd(8)`. Reads +//! `group:password` pairs from stdin and updates `/etc/gshadow`, or +//! `/etc/group` on a system that has no gshadow file. +//! +//! It is `chpasswd(8)`'s counterpart for groups, and follows the same rule: +//! every line is resolved before any is written, so a batch naming one group +//! that does not exist changes nothing at all. + +use std::fmt; +use std::io::{self, BufRead}; +use std::path::Path; + +use clap::{Arg, ArgAction, Command}; + +use shadow_core::group::GroupEntry; +use shadow_core::gshadow::GshadowEntry; +use shadow_core::nscd; +use shadow_core::sysroot::SysRoot; +use shadow_core::transaction::{self, Commit, LockedFile}; + +use uucore::error::{UError, UResult}; + +mod options { + pub const CRYPT_METHOD: &str = "crypt-method"; + pub const ENCRYPTED: &str = "encrypted"; + pub const MD5: &str = "md5"; + pub const ROOT: &str = "root"; + pub const SHA_ROUNDS: &str = "sha-rounds"; + pub const PREFIX: &str = "prefix"; +} + +// --------------------------------------------------------------------------- +// Error type +// --------------------------------------------------------------------------- + +/// Errors that the `chgpasswd` utility can produce. +/// +/// GNU `chgpasswd(8)` exits 1 for a failure, 2 for invalid command syntax and +/// 3 for a chroot directory it cannot enter. +#[derive(Debug)] +enum ChgpasswdError { + /// Exit 1 — insufficient privileges. + PermissionDenied(String), + /// Exit 1 — an unexpected runtime failure. + UnexpectedFailure(String), + /// Exit 1 — could not acquire a lock on an account file. + FileBusy(String), + /// Exit 1 — invalid input line. + InvalidInput(String), + /// Exit 3 — the `--root` directory could not be entered. + CantChroot(String), +} + +impl fmt::Display for ChgpasswdError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::PermissionDenied(msg) + | Self::UnexpectedFailure(msg) + | Self::FileBusy(msg) + | Self::InvalidInput(msg) + | Self::CantChroot(msg) => f.write_str(msg), + } + } +} + +impl std::error::Error for ChgpasswdError {} + +impl UError for ChgpasswdError { + fn code(&self) -> i32 { + match self { + Self::PermissionDenied(_) + | Self::UnexpectedFailure(_) + | Self::FileBusy(_) + | Self::InvalidInput(_) => 1, + Self::CantChroot(_) => 3, + } + } +} + +// --------------------------------------------------------------------------- +// Input parsing +// --------------------------------------------------------------------------- + +/// A parsed `group:password` pair from stdin. +/// +/// The password field is `Zeroizing` so it is scrubbed when dropped rather +/// than left in freed heap for a core dump to expose. +struct PasswordPair { + group: String, + password: zeroize::Zeroizing, + /// Input line the pair came from, for error messages. + line_number: usize, +} + +/// Parse one input line into a `group:password` pair. +/// +/// The password is everything after the first colon, **verbatim**: a hash may +/// contain colons, and trailing whitespace is part of a password, so the line +/// is not trimmed. Only a trailing CR from CRLF input is removed. +fn parse_input_line(line: &str, line_number: usize) -> Result { + let line = line.strip_suffix('\r').unwrap_or(line); + + // A line with no colon carries no password, which is how the GNU tool + // words it -- and a blank line is that same case, not something to skip. + let Some(colon_pos) = line.find(':') else { + return Err(ChgpasswdError::InvalidInput(format!( + "line {line_number}: missing new password" + ))); + }; + + let group = &line[..colon_pos]; + let password = &line[colon_pos + 1..]; + + if group.is_empty() { + return Err(ChgpasswdError::InvalidInput(format!( + "line {line_number}: missing group name" + ))); + } + + Ok(PasswordPair { + group: group.to_string(), + password: zeroize::Zeroizing::new(password.to_string()), + line_number, + }) +} + +/// Read every `group:password` pair from stdin. +/// +/// Empty input is not an error: `chgpasswd < /dev/null` succeeds having done +/// nothing, which is what the GNU tool does and what a script driving it from +/// a possibly-empty list depends on. +fn read_pairs_from_stdin() -> Result, ChgpasswdError> { + let stdin = io::stdin(); + let reader = stdin.lock(); + let mut pairs = Vec::new(); + + for (idx, line) in reader.lines().enumerate() { + // Every line carries a password; own it in a Zeroizing so the buffer + // is scrubbed when it drops. + let line = + zeroize::Zeroizing::new(line.map_err(|e| { + ChgpasswdError::UnexpectedFailure(format!("error reading stdin: {e}")) + })?); + pairs.push(parse_input_line(&line, idx + 1)?); + } + + Ok(pairs) +} + +/// Refuse an empty password in plaintext mode. +/// +/// Hashing `""` produces a valid hash, and the group then accepts a bare +/// Enter from any non-member. Only `-e`, which takes a pre-computed field, may +/// carry an empty value -- that is how a `!` lock is written. +fn reject_empty_plaintext(pairs: &[PasswordPair], plaintext: bool) -> Result<(), ChgpasswdError> { + if !plaintext { + return Ok(()); + } + match pairs.iter().find(|p| p.password.is_empty()) { + Some(pair) => Err(ChgpasswdError::InvalidInput(format!( + "line {}: no password supplied for '{}'", + pair.line_number, pair.group + ))), + None => Ok(()), + } +} + +// --------------------------------------------------------------------------- +// Entry point +// --------------------------------------------------------------------------- + +#[uucore::main] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + shadow_core::hardening::harden_process(); + + // chgpasswd(8) exits 2 for invalid command syntax; other failures are 1. + let Some(matches) = shadow_core::cli::parse_args(uu_app(), args, |_| 2)? else { + return Ok(()); + }; + + if let Some(chroot_dir) = matches.get_one::(options::ROOT) { + shadow_core::hardening::chroot_into(Path::new(chroot_dir)) + .map_err(|e| ChgpasswdError::CantChroot(e.to_string()))?; + } + + let prefix = matches.get_one::(options::PREFIX).map(Path::new); + let root = SysRoot::new(prefix); + + if !shadow_core::hardening::caller_is_root() { + return Err( + ChgpasswdError::PermissionDenied(shadow_core::os_error::permission_denied()).into(), + ); + } + + let is_encrypted = matches.get_flag(options::ENCRYPTED); + let crypt_method = matches.get_one::(options::CRYPT_METHOD); + + if matches.get_flag(options::MD5) { + return Err(ChgpasswdError::UnexpectedFailure( + "MD5 is insecure and not supported; use -c SHA512 instead".into(), + ) + .into()); + } + + let sha_rounds = parse_sha_rounds(matches.get_one::(options::SHA_ROUNDS).copied())?; + + let hash_config = if is_encrypted { + None + } else { + let method = resolve_crypt_method(crypt_method.map(String::as_str), &root)?; + if sha_rounds.is_some() && method == shadow_core::crypt::CryptMethod::Yescrypt { + return Err(ChgpasswdError::UnexpectedFailure( + "--sha-rounds is not supported with YESCRYPT".into(), + ) + .into()); + } + Some((method, sha_rounds)) + }; + + let pairs = read_pairs_from_stdin()?; + reject_empty_plaintext(&pairs, hash_config.is_some())?; + + if pairs.is_empty() { + return Ok(()); + } + + apply_password_changes(&root, &pairs, hash_config.as_ref()) +} + +/// Validate `--sha-rounds`, which must fit a `u32` to reach crypt(3). +fn parse_sha_rounds(value: Option) -> Result, ChgpasswdError> { + let Some(rounds) = value else { + return Ok(None); + }; + u32::try_from(rounds).map(Some).map_err(|_| { + ChgpasswdError::UnexpectedFailure(format!( + "invalid value for --sha-rounds '{rounds}': must be between 1 and {}", + u32::MAX + )) + }) +} + +/// Build the clap `Command` for `chgpasswd`. +#[must_use] +pub fn uu_app() -> Command { + Command::new("chgpasswd") + .about("Read group:password pairs from stdin and apply them") + .override_usage("chgpasswd [options]") + .version(shadow_core::cli::VERSION) + .after_help(shadow_core::cli::AFTER_HELP) + .arg( + Arg::new(options::CRYPT_METHOD) + .short('c') + .long("crypt-method") + .help("hashing scheme to apply (SHA256, SHA512, YESCRYPT, ...)") + .value_name("METHOD") + .value_parser(["SHA256", "SHA512", "YESCRYPT", "DES", "MD5", "NONE"]), + ) + .arg( + // chgpasswd(8): the -c, -e and -m flags are exclusive. + Arg::new(options::ENCRYPTED) + .short('e') + .long("encrypted") + .help("treat input passwords as already hashed") + .conflicts_with_all([options::CRYPT_METHOD, options::MD5]) + .action(ArgAction::SetTrue), + ) + .arg( + Arg::new(options::MD5) + .short('m') + .long("md5") + .help("rejected: MD5 is insecure and unsupported (use -c SHA512)") + .action(ArgAction::SetTrue), + ) + .arg( + Arg::new(options::ROOT) + .short('R') + .long("root") + .help("chroot into CHROOT_DIR before applying changes") + .value_name("CHROOT_DIR"), + ) + .arg( + // A rounds count without a scheme that takes one is meaningless, + // and ignoring it silently wrote a password the caller did not ask + // for. + Arg::new(options::SHA_ROUNDS) + .short('s') + .long("sha-rounds") + .help("iteration count when hashing with SHA-2 (requires -c)") + .value_name("ROUNDS") + .requires(options::CRYPT_METHOD) + .value_parser(clap::value_parser!(i64).range(1..)), + ) + .arg( + Arg::new(options::PREFIX) + .short('P') + .long("prefix") + .help("directory prefix") + .value_name("PREFIX_DIR"), + ) +} + +// --------------------------------------------------------------------------- +// Command implementation +// --------------------------------------------------------------------------- + +/// Apply every group password change in one locked transaction. +fn apply_password_changes( + root: &SysRoot, + pairs: &[PasswordPair], + hash_config: Option<&(shadow_core::crypt::CryptMethod, Option)>, +) -> UResult<()> { + // Hash before taking any lock. crypt(3) with yescrypt or a high rounds= + // count is deliberately slow, and a batch of them would otherwise hold the + // account-file locks, with signals blocked, for the whole run. + let mut hashed: Vec<(&str, String)> = Vec::with_capacity(pairs.len()); + for pair in pairs { + let hash = match hash_config { + Some((method, rounds)) => { + shadow_core::crypt::hash_password(&pair.password, *method, *rounds).map_err( + |e| { + ChgpasswdError::UnexpectedFailure(format!( + "failed to hash password for '{}': {e}", + pair.group + )) + }, + )? + } + None => pair.password.to_string(), + }; + hashed.push((pair.group.as_str(), hash)); + } + + let group_path = root.group_path(); + let gshadow_path = root.gshadow_path(); + // A system with no gshadow keeps group passwords in /etc/group itself, and + // chgpasswd must not conjure the file into existence: creating it would + // change how every other tool on the host reads group passwords. + let gshadow_exists = gshadow_path.exists(); + + let mut group_file = open_locked::(&group_path)?; + let mut gshadow_file = if gshadow_exists { + Some(open_locked::(&gshadow_path)?) + } else { + None + }; + + // Resolve every line before writing any, so one unknown group in the + // middle of a batch leaves both files untouched rather than half applied. + let index: std::collections::HashMap<&str, usize> = group_file + .entries() + .iter() + .enumerate() + .map(|(i, e)| (e.name.as_str(), i)) + .collect(); + + let mut targets = Vec::with_capacity(hashed.len()); + for ((name, hash), pair) in hashed.iter().zip(pairs) { + let Some(&i) = index.get(name) else { + return Err(ChgpasswdError::InvalidInput(format!( + "line {}: group '{name}' does not exist", + pair.line_number + )) + .into()); + }; + targets.push((i, name, hash)); + } + + for (i, name, hash) in targets { + match gshadow_file.as_mut() { + // With a gshadow file the hash belongs there, and /etc/group + // carries the `x` placeholder that says so. + Some(gshadow) => { + let members = group_file.entries()[i].members.clone(); + group_file.entries_mut()[i].passwd = "x".to_string(); + set_gshadow_password(gshadow.entries_mut(), name, &members, hash); + } + None => group_file.entries_mut()[i].passwd.clone_from(hash), + } + } + + // Both files are validated before either is written, so a value that would + // corrupt one cannot leave the pair disagreeing. A commit that would write + // the same bytes writes nothing. + let mut files: Vec> = vec![Box::new(group_file)]; + if let Some(gshadow) = gshadow_file { + files.push(Box::new(gshadow)); + } + transaction::commit_all(files) + .map_err(|e| ChgpasswdError::UnexpectedFailure(format!("cannot write: {e}")))?; + + nscd::invalidate_cache("group"); + + Ok(()) +} + +/// Lock and read an account file, mapping contention to its own error. +fn open_locked(path: &Path) -> Result, ChgpasswdError> +where + T: shadow_core::transaction::Record, +{ + LockedFile::::open(path).map_err(|e| match e { + shadow_core::error::ShadowError::Lock(_) => { + ChgpasswdError::FileBusy(format!("cannot lock {}: try again later", path.display())) + } + other => { + ChgpasswdError::UnexpectedFailure(format!("cannot open {}: {other}", path.display())) + } + }) +} + +/// Set a group's password in gshadow, adding the line if it is missing. +/// +/// A group present in `/etc/group` with no gshadow line is an inconsistency +/// grpck reports; setting a password on it is a reasonable way to fix it, so +/// the line is created rather than the change refused. +fn set_gshadow_password( + entries: &mut Vec, + name: &str, + members: &[String], + hash: &str, +) { + if let Some(entry) = entries.iter_mut().find(|g| g.name == name) { + entry.passwd = hash.to_string(); + return; + } + entries.push(GshadowEntry { + name: name.to_string(), + passwd: hash.to_string(), + admins: Vec::new(), + members: members.to_vec(), + }); +} + +// --------------------------------------------------------------------------- +// Crypt method selection +// --------------------------------------------------------------------------- + +/// The hashing scheme to use, from `-c` or, absent that, from login.defs. +/// +/// The default is the system's, not a hard-coded one: Debian sets YESCRYPT, +/// and hard-coding SHA-512 would quietly write weaker hashes than the rest of +/// the host produces. +fn resolve_crypt_method( + method: Option<&str>, + root: &SysRoot, +) -> Result { + match method { + Some(name) => parse_crypt_method(name).ok_or_else(|| { + ChgpasswdError::UnexpectedFailure(match name { + // GNU accepts NONE and stores the password as clear text. A + // readable group password in /etc/gshadow is worth no more + // than no password at all, and `-e` already covers writing a + // field verbatim when that is genuinely what is wanted. + "NONE" => "NONE would store the password unhashed and is not supported; \ + use -e to write a field verbatim" + .into(), + "MD5" | "DES" => { + "MD5 and DES are insecure and not supported for plaintext hashing".into() + } + other => format!("unknown crypt method: {other}"), + }) + }), + None => Ok(default_crypt_method(root)), + } +} + +/// Map a login.defs / `-c` method name to a `CryptMethod`. +fn parse_crypt_method(name: &str) -> Option { + use shadow_core::crypt::CryptMethod; + + match name { + "SHA256" => Some(CryptMethod::Sha256), + "SHA512" => Some(CryptMethod::Sha512), + "YESCRYPT" => Some(CryptMethod::Yescrypt), + _ => None, + } +} + +/// The system's configured hashing scheme, or SHA-512 if there is none. +fn default_crypt_method(root: &SysRoot) -> shadow_core::crypt::CryptMethod { + shadow_core::login_defs::LoginDefs::load(&root.login_defs_path()) + .ok() + .and_then(|d| d.get("ENCRYPT_METHOD").and_then(parse_crypt_method)) + .unwrap_or(shadow_core::crypt::CryptMethod::Sha512) +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_app_builds() { + uu_app().debug_assert(); + } + + // ----------------------------------------------------------------------- + // Input parsing + // ----------------------------------------------------------------------- + + #[test] + fn test_parse_input_line_valid() { + let pair = parse_input_line("staff:$6$hash", 1).expect("should parse"); + assert_eq!(pair.group, "staff"); + assert_eq!(&*pair.password, "$6$hash"); + } + + /// Only the first colon separates: a hash contains them. + #[test] + fn test_parse_input_line_password_with_colons() { + let pair = parse_input_line("staff:$6$salt:hash:rest", 1).expect("should parse"); + assert_eq!(pair.group, "staff"); + assert_eq!(&*pair.password, "$6$salt:hash:rest"); + } + + /// A line with no colon supplies no password, and a blank line is that + /// same case rather than something to skip over. + #[test] + fn test_lines_without_a_password_are_refused() { + for line in ["nocolon", "", " "] { + let err = parse_input_line(line, 4).err().expect("should be refused"); + assert!( + format!("{err}").contains("line 4: missing new password"), + "unexpected message for {line:?}: {err}" + ); + } + } + + #[test] + fn test_parse_input_line_empty_group() { + let err = parse_input_line(":password", 2) + .err() + .expect("should be refused"); + assert!(format!("{err}").contains("line 2")); + } + + /// Whitespace is data: the password is everything after the first colon, + /// so trimming it would set a different password than was supplied. + #[test] + fn test_parse_input_line_preserves_whitespace() { + let pair = parse_input_line("staff:$6$hash ", 1).expect("parses"); + assert_eq!(&*pair.password, "$6$hash "); + + // CRLF input loses only the carriage return. + let pair = parse_input_line("staff:secret\r", 2).expect("parses"); + assert_eq!(&*pair.password, "secret"); + } + + #[test] + fn test_reject_empty_plaintext() { + let pairs = vec![ + parse_input_line("staff:secret", 1).expect("parses"), + parse_input_line("wheel:", 2).expect("parses"), + ]; + // -e mode: an empty field is a deliberate lock, allowed. + assert!(reject_empty_plaintext(&pairs, false).is_ok()); + let err = reject_empty_plaintext(&pairs, true).expect_err("must refuse"); + assert!( + format!("{err}").contains("line 2") && format!("{err}").contains("wheel"), + "message should name the offending line: {err}" + ); + } + + // ----------------------------------------------------------------------- + // gshadow entry handling + // ----------------------------------------------------------------------- + + #[test] + fn test_set_gshadow_password_updates_in_place() { + let mut entries = vec![GshadowEntry { + name: "staff".to_string(), + passwd: "!".to_string(), + admins: vec!["alice".to_string()], + members: vec!["bob".to_string()], + }]; + set_gshadow_password(&mut entries, "staff", &[], "$6$new"); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].passwd, "$6$new"); + // Setting a password must not disturb who administers or belongs to + // the group. + assert_eq!(entries[0].admins, vec!["alice".to_string()]); + assert_eq!(entries[0].members, vec!["bob".to_string()]); + } + + #[test] + fn test_set_gshadow_password_adds_a_missing_line() { + let mut entries = Vec::new(); + let members = vec!["carol".to_string()]; + set_gshadow_password(&mut entries, "staff", &members, "$6$new"); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].name, "staff"); + assert_eq!(entries[0].passwd, "$6$new"); + // The new line inherits the membership /etc/group already records. + assert_eq!(entries[0].members, members); + assert!(entries[0].admins.is_empty()); + } + + // ----------------------------------------------------------------------- + // Crypt method selection + // ----------------------------------------------------------------------- + + fn defs_root(contents: &str) -> (tempfile::TempDir, SysRoot) { + let dir = tempfile::tempdir().expect("tempdir"); + let etc = dir.path().join("etc"); + std::fs::create_dir_all(&etc).expect("etc"); + std::fs::write(etc.join("login.defs"), contents).expect("write"); + let root = SysRoot::new(Some(dir.path())); + (dir, root) + } + + #[test] + fn test_default_method_comes_from_login_defs() { + use shadow_core::crypt::CryptMethod; + + let (_d, root) = defs_root("ENCRYPT_METHOD YESCRYPT\n"); + assert_eq!(default_crypt_method(&root), CryptMethod::Yescrypt); + + let (_d, root) = defs_root("ENCRYPT_METHOD SHA256\n"); + assert_eq!(default_crypt_method(&root), CryptMethod::Sha256); + } + + #[test] + fn test_default_method_falls_back_to_sha512() { + use shadow_core::crypt::CryptMethod; + + for defs in ["", "ENCRYPT_METHOD MD5\n", "ENCRYPT_METHOD DES\n"] { + let (_d, root) = defs_root(defs); + assert_eq!(default_crypt_method(&root), CryptMethod::Sha512); + } + } + + /// The schemes this build refuses stay refused rather than silently + /// falling back to something else. + #[test] + fn test_insecure_methods_are_refused() { + let (_d, root) = defs_root("ENCRYPT_METHOD YESCRYPT\n"); + for bad in ["MD5", "DES", "NONE", "BCRYPT", "nonsense"] { + assert!( + resolve_crypt_method(Some(bad), &root).is_err(), + "'{bad}' should be refused" + ); + } + } + + /// NONE is refused for a different reason than the weak hashes, and the + /// message has to say which, or an operator will just try `-c MD5` next. + #[test] + fn test_none_explains_itself() { + let (_d, root) = defs_root(""); + let err = resolve_crypt_method(Some("NONE"), &root).expect_err("refused"); + assert!(format!("{err}").contains("unhashed"), "{err}"); + assert!(format!("{err}").contains("-e"), "{err}"); + } + + // ----------------------------------------------------------------------- + // Flags + // ----------------------------------------------------------------------- + + #[test] + fn test_exclusive_and_dependent_flags() { + for args in [ + vec!["chgpasswd", "-s", "5000"], + vec!["chgpasswd", "-e", "-c", "SHA512"], + vec!["chgpasswd", "-e", "-m"], + vec!["chgpasswd", "-c", "BOGUS"], + vec!["chgpasswd", "-c", "SHA512", "-s", "0"], + ] { + assert!( + uu_app().try_get_matches_from(args.clone()).is_err(), + "{args:?} should be a usage error" + ); + } + for args in [ + vec!["chgpasswd", "-c", "SHA512", "-s", "5000"], + vec!["chgpasswd", "-e"], + vec!["chgpasswd"], + ] { + assert!( + uu_app().try_get_matches_from(args.clone()).is_ok(), + "{args:?} should parse" + ); + } + } + + #[test] + fn test_sha_rounds_must_fit_a_u32() { + assert_eq!(parse_sha_rounds(None).expect("none"), None); + assert_eq!(parse_sha_rounds(Some(5000)).expect("ok"), Some(5000)); + assert!(parse_sha_rounds(Some(i64::from(u32::MAX) + 1)).is_err()); + } + + // ----------------------------------------------------------------------- + // Exit codes + // ----------------------------------------------------------------------- + + /// The codes are the interface: 1 for a failure, 3 for a chroot that + /// cannot be entered. + #[test] + fn test_exit_codes() { + use uucore::error::UError; + + assert_eq!(ChgpasswdError::PermissionDenied("x".into()).code(), 1); + assert_eq!(ChgpasswdError::UnexpectedFailure("x".into()).code(), 1); + assert_eq!(ChgpasswdError::FileBusy("x".into()).code(), 1); + assert_eq!(ChgpasswdError::InvalidInput("x".into()).code(), 1); + assert_eq!(ChgpasswdError::CantChroot("x".into()).code(), 3); + } + + #[test] + fn test_error_display_and_is_std_error() { + let err = ChgpasswdError::InvalidInput("bad line".into()); + assert_eq!(format!("{err}"), "bad line"); + let _: &dyn std::error::Error = &err; + } +} diff --git a/src/uu/chgpasswd/src/main.rs b/src/uu/chgpasswd/src/main.rs new file mode 100644 index 0000000..0438f0c --- /dev/null +++ b/src/uu/chgpasswd/src/main.rs @@ -0,0 +1,6 @@ +// 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. + +uucore::bin!(uu_chgpasswd); diff --git a/tests/by-util/test_chgpasswd.rs b/tests/by-util/test_chgpasswd.rs new file mode 100644 index 0000000..957a28b --- /dev/null +++ b/tests/by-util/test_chgpasswd.rs @@ -0,0 +1,320 @@ +// 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. +// spell-checker:ignore chgpasswd gshadow yescrypt + +//! Integration tests for the `chgpasswd` utility. +//! +//! These feed the real binary on stdin and assert on what it writes to the +//! prefix tree, which is the only way to observe the two files it has to keep +//! agreeing: the hash belongs in `/etc/gshadow`, and `/etc/group` carries the +//! `x` that says so. + +use std::io::Write as _; +use std::process::Stdio; + +use crate::common::{Output, tool}; + +/// A prefix tree with a group file and, optionally, a gshadow file. +/// +/// Passing `None` for `gshadow` builds a host that keeps group passwords in +/// `/etc/group` itself, which is the layout `chgpasswd` has to detect. +fn prefix(group: &str, gshadow: Option<&str>, encrypt_method: Option<&str>) -> tempfile::TempDir { + let dir = tempfile::tempdir().expect("failed to create temp dir"); + let etc = dir.path().join("etc"); + std::fs::create_dir_all(&etc).expect("failed to create etc dir"); + std::fs::write(etc.join("group"), group).expect("failed to write group file"); + if let Some(gshadow) = gshadow { + std::fs::write(etc.join("gshadow"), gshadow).expect("failed to write gshadow file"); + } + if let Some(method) = encrypt_method { + std::fs::write(etc.join("login.defs"), format!("ENCRYPT_METHOD {method}\n")) + .expect("failed to write login.defs"); + } + dir +} + +fn read_file(dir: &tempfile::TempDir, name: &str) -> String { + std::fs::read_to_string(dir.path().join("etc").join(name)) + .unwrap_or_else(|e| panic!("cannot read {name}: {e}")) +} + +/// The password field of one line in `group` or `gshadow`. +fn field(dir: &tempfile::TempDir, file: &str, group: &str) -> String { + read_file(dir, file) + .lines() + .find(|l| l.starts_with(&format!("{group}:"))) + .and_then(|l| l.split(':').nth(1)) + .unwrap_or_else(|| panic!("no {file} entry for {group}")) + .to_string() +} + +/// Run `chgpasswd --prefix ` with `input` on stdin. +fn chgpasswd(dir: &tempfile::TempDir, args: &[&str], input: &str) -> Output { + let mut cmd = tool("chgpasswd"); + cmd.arg("--prefix") + .arg(dir.path()) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let mut child = cmd.spawn().expect("cannot spawn chgpasswd"); + child + .stdin + .as_mut() + .expect("stdin") + .write_all(input.as_bytes()) + .expect("cannot write to chgpasswd"); + let out = child.wait_with_output().expect("chgpasswd did not finish"); + Output { + code: out.status.code().unwrap_or(1), + stdout: String::from_utf8_lossy(&out.stdout).into_owned(), + stderr: String::from_utf8_lossy(&out.stderr).into_owned(), + } +} + +const GROUP: &str = "staff:x:100:alice\nwheel:x:101:\n"; +const GSHADOW: &str = "staff:!::alice\nwheel:!::\n"; + +// --------------------------------------------------------------------------- +// Where the password lands +// --------------------------------------------------------------------------- + +/// With a gshadow file the hash goes there and `/etc/group` keeps `x`. Writing +/// the hash into a world-readable `/etc/group` instead would publish it. +#[test] +fn test_the_hash_goes_to_gshadow() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); + + assert!( + field(&dir, "gshadow", "staff").starts_with("$6$"), + "gshadow should hold the hash, got {:?}", + field(&dir, "gshadow", "staff") + ); + assert_eq!( + field(&dir, "group", "staff"), + "x", + "/etc/group must keep the placeholder, not the hash" + ); +} + +/// Without a gshadow file the hash goes into `/etc/group`, and no gshadow file +/// is conjured up: creating one changes how every other tool on the host reads +/// group passwords. +#[test] +fn test_without_gshadow_the_hash_goes_to_group() { + let dir = prefix(GROUP, None, Some("SHA512")); + chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); + + assert!( + field(&dir, "group", "staff").starts_with("$6$"), + "group should hold the hash without a gshadow file" + ); + assert!( + !dir.path().join("etc/gshadow").exists(), + "chgpasswd must not create a gshadow file" + ); +} + +/// Setting a password must not disturb who administers or belongs to a group. +#[test] +fn test_membership_and_admins_survive() { + let dir = prefix(GROUP, Some("staff:!:bob:alice\n"), Some("SHA512")); + chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); + + let line = read_file(&dir, "gshadow") + .lines() + .find(|l| l.starts_with("staff:")) + .expect("staff line") + .to_string(); + let fields: Vec<&str> = line.split(':').collect(); + assert_eq!(fields[2], "bob", "the administrator list changed"); + assert_eq!(fields[3], "alice", "the member list changed"); +} + +// --------------------------------------------------------------------------- +// Hashing +// --------------------------------------------------------------------------- + +/// `-e` stores the field verbatim; nothing is hashed a second time. +#[test] +fn test_encrypted_is_stored_verbatim() { + let dir = prefix(GROUP, Some(GSHADOW), None); + chgpasswd(&dir, &["-e"], "staff:$6$salt$alreadyhashed\n").assert_code(0); + assert_eq!(field(&dir, "gshadow", "staff"), "$6$salt$alreadyhashed"); +} + +/// `-e` may write an empty field: that is how a group password is cleared. +#[test] +fn test_encrypted_accepts_an_empty_field() { + let dir = prefix(GROUP, Some(GSHADOW), None); + chgpasswd(&dir, &["-e"], "staff:\n").assert_code(0); + assert_eq!(field(&dir, "gshadow", "staff"), ""); +} + +/// An empty plaintext password would hash to something a bare Enter matches. +#[test] +fn test_empty_plaintext_is_refused() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + chgpasswd(&dir, &[], "staff:\n") + .assert_code(1) + .assert_stderr_contains("no password supplied"); + assert_eq!(field(&dir, "gshadow", "staff"), "!", "nothing may change"); +} + +/// The default scheme is the system's, from login.defs -- not a hard-coded one. +#[test] +fn test_default_scheme_comes_from_login_defs() { + for (method, prefix_str) in [("SHA512", "$6$"), ("SHA256", "$5$")] { + let dir = prefix(GROUP, Some(GSHADOW), Some(method)); + chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); + assert!( + field(&dir, "gshadow", "staff").starts_with(prefix_str), + "{method} should produce a {prefix_str} hash" + ); + } +} + +/// `-c` overrides the configured default. +#[test] +fn test_explicit_scheme_overrides() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + chgpasswd(&dir, &["-c", "SHA256"], "staff:secret\n").assert_code(0); + assert!(field(&dir, "gshadow", "staff").starts_with("$5$")); +} + +/// GNU accepts `-c NONE` and stores the password as clear text. A readable +/// group password is worth no more than none at all, so it is refused -- and +/// the refusal must leave the file alone. +#[test] +fn test_none_is_refused() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + chgpasswd(&dir, &["-c", "NONE"], "staff:secret\n") + .assert_code(1) + .assert_stderr_contains("unhashed"); + assert_eq!(field(&dir, "gshadow", "staff"), "!"); +} + +// --------------------------------------------------------------------------- +// All or nothing +// --------------------------------------------------------------------------- + +/// The property that makes a batch tool safe: one bad line and the files are +/// untouched, including for the groups named on the good lines before it. +#[test] +fn test_an_unknown_group_changes_nothing() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + let before = read_file(&dir, "gshadow"); + + chgpasswd( + &dir, + &[], + "staff:secret\nnosuchgroup:secret\nwheel:secret\n", + ) + .assert_code(1) + .assert_stderr_contains("group 'nosuchgroup' does not exist"); + + assert_eq!( + read_file(&dir, "gshadow"), + before, + "a failed batch must leave the file exactly as it was" + ); +} + +/// The line number in the message is what makes a long batch debuggable. +#[test] +fn test_the_error_names_the_line() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + chgpasswd(&dir, &[], "staff:secret\nnosuchgroup:secret\n") + .assert_code(1) + .assert_stderr_contains("line 2"); +} + +/// Several groups in one batch all land. +#[test] +fn test_a_whole_batch_applies() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + chgpasswd(&dir, &[], "staff:one\nwheel:two\n").assert_code(0); + assert!(field(&dir, "gshadow", "staff").starts_with("$6$")); + assert!(field(&dir, "gshadow", "wheel").starts_with("$6$")); + assert_ne!( + field(&dir, "gshadow", "staff"), + field(&dir, "gshadow", "wheel"), + "each password must get its own salt" + ); +} + +// --------------------------------------------------------------------------- +// Input handling +// --------------------------------------------------------------------------- + +/// Empty input succeeds having done nothing, which is what a script driving +/// chgpasswd from a possibly-empty list depends on. +#[test] +fn test_empty_input_succeeds() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + let before = read_file(&dir, "gshadow"); + chgpasswd(&dir, &[], "").assert_code(0); + assert_eq!(read_file(&dir, "gshadow"), before); +} + +/// A line carrying no password is an error rather than something to skip: a +/// blank line in the middle of a batch is a mistake worth reporting. +#[test] +fn test_a_line_without_a_password_is_refused() { + let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); + for input in ["staff\n", "\n", "staff:secret\n\n"] { + chgpasswd(&dir, &[], input) + .assert_code(1) + .assert_stderr_contains("missing new password"); + } + assert_eq!(field(&dir, "gshadow", "staff"), "!"); +} + +/// Only the first colon separates the group from the password, so a field +/// containing colons is parsed as one value -- and then refused, because a +/// colon in a gshadow field would split the line and corrupt the file. The GNU +/// tool refuses it too, and likewise leaves the file untouched. +#[test] +fn test_a_colon_in_the_password_is_refused_without_corrupting_the_file() { + let dir = prefix(GROUP, Some(GSHADOW), None); + let before = read_file(&dir, "gshadow"); + chgpasswd(&dir, &["-e"], "staff:$6$a:b:c\n").assert_code(1); + assert_eq!( + read_file(&dir, "gshadow"), + before, + "a refused write must leave the file exactly as it was" + ); +} + +// --------------------------------------------------------------------------- +// Flags +// --------------------------------------------------------------------------- + +#[test] +fn test_help_exits_zero() { + let dir = prefix(GROUP, Some(GSHADOW), None); + chgpasswd(&dir, &["--help"], "") + .assert_code(0) + .assert_stdout_contains("Usage:"); +} + +/// An unknown scheme is a usage error, exit 2, matching the GNU tool. +#[test] +fn test_unknown_scheme_is_a_usage_error() { + let dir = prefix(GROUP, Some(GSHADOW), None); + chgpasswd(&dir, &["-c", "BOGUS"], "staff:secret\n").assert_code(2); +} + +/// MD5 is accepted by the GNU tool and refused here, deliberately. +#[test] +fn test_md5_is_refused() { + let dir = prefix(GROUP, Some(GSHADOW), None); + chgpasswd(&dir, &["-m"], "staff:secret\n") + .assert_code(1) + .assert_stderr_contains("MD5"); + assert_eq!(field(&dir, "gshadow", "staff"), "!"); +} diff --git a/tests/by-util/test_multicall.rs b/tests/by-util/test_multicall.rs index a49f929..5ce54f1 100644 --- a/tests/by-util/test_multicall.rs +++ b/tests/by-util/test_multicall.rs @@ -16,9 +16,24 @@ use std::process::Command; use crate::common::{run, run_cmd}; /// Every applet this build is expected to carry, in `--list` order. -const TOOLS: [&str; 16] = [ - "chage", "chfn", "chpasswd", "chsh", "gpasswd", "groupadd", "groupdel", "groupmod", "grpck", - "newgrp", "passwd", "pwck", "sg", "useradd", "userdel", "usermod", +const TOOLS: [&str; 17] = [ + "chage", + "chfn", + "chgpasswd", + "chpasswd", + "chsh", + "gpasswd", + "groupadd", + "groupdel", + "groupmod", + "grpck", + "newgrp", + "passwd", + "pwck", + "sg", + "useradd", + "userdel", + "usermod", ]; /// The binary with no applet argument. diff --git a/tests/e2e/deploy-test.sh b/tests/e2e/deploy-test.sh index 10c44ee..964cad0 100755 --- a/tests/e2e/deploy-test.sh +++ b/tests/e2e/deploy-test.sh @@ -100,7 +100,7 @@ hash_password() { # ── TOOLS list ────────────────────────────────────────────────────── -TOOLS="passwd pwck useradd userdel usermod chpasswd chage groupadd groupdel groupmod gpasswd grpck chfn chsh newgrp sg" +TOOLS="passwd pwck useradd userdel usermod chpasswd chgpasswd chage groupadd groupdel groupmod gpasswd grpck chfn chsh newgrp sg" SETUID_TOOLS="passwd chfn chsh newgrp gpasswd sg" # The tools an unprivileged user runs are installed in bin, the rest in sbin, @@ -869,6 +869,56 @@ test_gpasswd_group_admin() { userdel -r gp_member 2>/dev/null || true } +# ── chgpasswd: group passwords in batch ──────────────────────────── + +test_chgpasswd() { + section "chgpasswd — group passwords in batch" + + groupdel cg_one 2>/dev/null || true + groupdel cg_two 2>/dev/null || true + assert_ok "groupadd cg_one" groupadd cg_one + assert_ok "groupadd cg_two" groupadd cg_two + + assert_ok "a batch of two group passwords applies" \ + bash -c "printf 'cg_one:first\ncg_two:second\n' | chgpasswd" + assert_file_contains "cg_one got a hash in gshadow" \ + /etc/gshadow '^cg_one:\$' + assert_file_contains "cg_two got a hash in gshadow" \ + /etc/gshadow '^cg_two:\$' + assert_file_contains "/etc/group keeps the placeholder" \ + /etc/group '^cg_one:x:' + + # The password has to be the one a non-member is actually checked against, + # or the tool has written something decorative. + userdel -r cg_outsider 2>/dev/null || true + assert_ok "useradd -m cg_outsider" useradd -m cg_outsider + assert_contains "sg accepts the password chgpasswd set" "cg_one" \ + bash -c "echo first | su -s /bin/bash cg_outsider -c \"sg cg_one -c 'id -gn'\"" + assert_fail "and refuses a wrong one" \ + bash -c "echo wrong | su -s /bin/bash cg_outsider -c \"sg cg_one -c 'id -gn'\"" + + # All or nothing: one unknown group and the whole batch is discarded. + # The line is compared through a file: a hash is full of dollar signs, and + # carrying one through a shell variable expands them away. + grep '^cg_two:' /etc/gshadow > /tmp/cg_two.before + assert_fail "a batch naming an unknown group fails" \ + bash -c "printf 'cg_two:changed\nnosuchgroup:x\n' | chgpasswd" + assert_ok "and changed nothing" \ + bash -c "grep '^cg_two:' /etc/gshadow | diff -q - /tmp/cg_two.before" + + assert_ok "-e stores a field verbatim" \ + bash -c "printf 'cg_one:\$6\$salt\$hash\n' | chgpasswd -e" + assert_file_contains "the verbatim field is in gshadow" \ + /etc/gshadow '^cg_one:\$6\$salt\$hash:' + + assert_ok "empty input succeeds having done nothing" \ + bash -c "chgpasswd < /dev/null" + + userdel -r cg_outsider 2>/dev/null || true + groupdel cg_one 2>/dev/null || true + groupdel cg_two 2>/dev/null || true +} + # ── sg: running one command in another group ─────────────────────── test_sg_group_switch() { @@ -1039,6 +1089,7 @@ main() { test_self_service test_gpasswd_group_admin test_sg_group_switch + test_chgpasswd test_aging_and_input test_audit_logging test_root_option diff --git a/tests/gnu-compat.sh b/tests/gnu-compat.sh index 2e20f0e..1e96bcd 100755 --- a/tests/gnu-compat.sh +++ b/tests/gnu-compat.sh @@ -224,6 +224,7 @@ for pair in \ "groupdel:/usr/sbin/groupdel" \ "groupmod:/usr/sbin/groupmod" \ "chpasswd:/usr/sbin/chpasswd" \ + "chgpasswd:/usr/sbin/chgpasswd" \ "gpasswd:/usr/bin/gpasswd" \ "pwck:/usr/sbin/pwck" \ "grpck:/usr/sbin/grpck"; do diff --git a/tests/tests.rs b/tests/tests.rs index 7146dee..2866800 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -22,6 +22,8 @@ mod common; mod test_chage; #[path = "by-util/test_chfn.rs"] mod test_chfn; +#[path = "by-util/test_chgpasswd.rs"] +mod test_chgpasswd; #[path = "by-util/test_chpasswd.rs"] mod test_chpasswd; #[path = "by-util/test_chsh.rs"] From 56169c4f42b70116f2807b52f98ae0981aca26af Mon Sep 17 00:00:00 2001 From: Pierre Warnier Date: Sun, 6 Sep 2026 15:52:52 +0200 Subject: [PATCH 2/2] tests: run the suite as an unprivileged user too Sixteen of the chgpasswd tests assumed root. They passed locally because every container in docker-compose.yml runs as root, and failed in the one CI job that does not. Worse than the two that failed outright were the ones that passed for the wrong reason: a test asserting exit 1 got exit 1 from "permission denied" rather than from the condition it meant to check, so it was green while testing nothing. Only the two expecting exit 0 gave the game away. The guard itself is what chpasswd already does -- skip_unless_root() on every test that writes an account file. The two that only exercise argument parsing keep running everywhere, since clap answers before the root check. The gap that let this through was local, so the fix is too: `make test-unprivileged` re-runs the already-built test binaries as an ordinary user, and `make check` now does it after the root run. Removing one guard again makes it fail with "Permission denied", which is how it was verified. SHADOW_TEST_REQUIRE_ROOT is cleared for that run on purpose: it exists to turn a skip into a failure when the suite *is* root, which is the opposite case. --- CHANGELOG.md | 6 ++++ Makefile | 28 +++++++++++++++++- tests/by-util/test_chgpasswd.rs | 50 ++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1babe80..d0148bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- `make check` also runs the suite as an unprivileged user, through the new + `make test-unprivileged`. Every container in `docker-compose.yml` runs as + root, so a test that silently assumed root passed locally and failed only in + CI -- and a test asserting a failure exit code passed there for the wrong + reason, since "permission denied" is a failure too + - `newgrp` no longer refuses a passwordless group before prompting. It asked for a password only when the group had one, so whether a prompt appeared told any caller which groups have passwords set — a list of the ones worth diff --git a/Makefile b/Makefile index 715bec5..10d7d01 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,7 @@ USER_TOOLS = $(SETUID_TOOLS) chage ALL_TOOLS = $(SETUID_TOOLS) $(ROOT_TOOLS) chage -.PHONY: all build build-multicall build-arm64 dist-musl check test test-gnu-compat test-arm64 install install-multicall uninstall clean +.PHONY: all build build-multicall build-arm64 dist-musl check test test-gnu-compat test-unprivileged test-arm64 install install-multicall uninstall clean all: build @@ -71,6 +71,7 @@ check: cargo clippy --workspace --all-targets --features pam -- -D warnings cargo clippy --workspace --all-targets --all-features -- -D warnings $(MAKE) test + $(MAKE) test-unprivileged # `install` ships binaries built with pam, so the tests must cover that build # as well as the default one: the feature changes which code paths exist. @@ -102,6 +103,31 @@ build-arm64: test-arm64: build-arm64 ARM64_BIN=$(ARM64_BIN) bash tests/arm64-smoke.sh +# Run the suite as an unprivileged user. +# +# Every container in docker-compose.yml runs as root, so a test that silently +# assumes root passes here and fails in CI, where one job runs as an ordinary +# user -- and a test asserting a failure exit code passes for the wrong reason, +# because "permission denied" is also a failure. This target is that job, +# locally. The binaries are already built, so it only re-runs them. +# +# SHADOW_TEST_REQUIRE_ROOT is cleared deliberately: it exists to turn a skip +# into a failure when the suite *is* running as root, which is the opposite of +# what this target does. +UNPRIV_USER = shadowtest + +test-unprivileged: + cargo test --workspace --no-run 2>&1 \ + | sed -n 's/.*(\(target\/debug\/deps\/[^)]*\))$$/\1/p' > /tmp/test-binaries + @id -u $(UNPRIV_USER) >/dev/null 2>&1 || useradd -m $(UNPRIV_USER) + @chmod -R a+rX target + @fail=0; while read -r bin; do \ + printf '== %s\n' "$$bin"; \ + su $(UNPRIV_USER) -s /bin/sh -c "SHADOW_TEST_REQUIRE_ROOT= $(CURDIR)/$$bin" \ + || fail=1; \ + done < /tmp/test-binaries; \ + exit $$fail + # Compare our output and exit codes against the GNU tools installed alongside. # Needs root and the GNU shadow package, so it belongs in a container. test-gnu-compat: diff --git a/tests/by-util/test_chgpasswd.rs b/tests/by-util/test_chgpasswd.rs index 957a28b..07a6404 100644 --- a/tests/by-util/test_chgpasswd.rs +++ b/tests/by-util/test_chgpasswd.rs @@ -14,7 +14,7 @@ use std::io::Write as _; use std::process::Stdio; -use crate::common::{Output, tool}; +use crate::common::{Output, skip_unless_root, tool}; /// A prefix tree with a group file and, optionally, a gshadow file. /// @@ -86,6 +86,9 @@ const GSHADOW: &str = "staff:!::alice\nwheel:!::\n"; /// the hash into a world-readable `/etc/group` instead would publish it. #[test] fn test_the_hash_goes_to_gshadow() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); @@ -106,6 +109,9 @@ fn test_the_hash_goes_to_gshadow() { /// group passwords. #[test] fn test_without_gshadow_the_hash_goes_to_group() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, None, Some("SHA512")); chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); @@ -122,6 +128,9 @@ fn test_without_gshadow_the_hash_goes_to_group() { /// Setting a password must not disturb who administers or belongs to a group. #[test] fn test_membership_and_admins_survive() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some("staff:!:bob:alice\n"), Some("SHA512")); chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); @@ -142,6 +151,9 @@ fn test_membership_and_admins_survive() { /// `-e` stores the field verbatim; nothing is hashed a second time. #[test] fn test_encrypted_is_stored_verbatim() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), None); chgpasswd(&dir, &["-e"], "staff:$6$salt$alreadyhashed\n").assert_code(0); assert_eq!(field(&dir, "gshadow", "staff"), "$6$salt$alreadyhashed"); @@ -150,6 +162,9 @@ fn test_encrypted_is_stored_verbatim() { /// `-e` may write an empty field: that is how a group password is cleared. #[test] fn test_encrypted_accepts_an_empty_field() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), None); chgpasswd(&dir, &["-e"], "staff:\n").assert_code(0); assert_eq!(field(&dir, "gshadow", "staff"), ""); @@ -158,6 +173,9 @@ fn test_encrypted_accepts_an_empty_field() { /// An empty plaintext password would hash to something a bare Enter matches. #[test] fn test_empty_plaintext_is_refused() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); chgpasswd(&dir, &[], "staff:\n") .assert_code(1) @@ -168,6 +186,9 @@ fn test_empty_plaintext_is_refused() { /// The default scheme is the system's, from login.defs -- not a hard-coded one. #[test] fn test_default_scheme_comes_from_login_defs() { + if skip_unless_root() { + return; + } for (method, prefix_str) in [("SHA512", "$6$"), ("SHA256", "$5$")] { let dir = prefix(GROUP, Some(GSHADOW), Some(method)); chgpasswd(&dir, &[], "staff:secret\n").assert_code(0); @@ -181,6 +202,9 @@ fn test_default_scheme_comes_from_login_defs() { /// `-c` overrides the configured default. #[test] fn test_explicit_scheme_overrides() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); chgpasswd(&dir, &["-c", "SHA256"], "staff:secret\n").assert_code(0); assert!(field(&dir, "gshadow", "staff").starts_with("$5$")); @@ -191,6 +215,9 @@ fn test_explicit_scheme_overrides() { /// the refusal must leave the file alone. #[test] fn test_none_is_refused() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); chgpasswd(&dir, &["-c", "NONE"], "staff:secret\n") .assert_code(1) @@ -206,6 +233,9 @@ fn test_none_is_refused() { /// untouched, including for the groups named on the good lines before it. #[test] fn test_an_unknown_group_changes_nothing() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); let before = read_file(&dir, "gshadow"); @@ -227,6 +257,9 @@ fn test_an_unknown_group_changes_nothing() { /// The line number in the message is what makes a long batch debuggable. #[test] fn test_the_error_names_the_line() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); chgpasswd(&dir, &[], "staff:secret\nnosuchgroup:secret\n") .assert_code(1) @@ -236,6 +269,9 @@ fn test_the_error_names_the_line() { /// Several groups in one batch all land. #[test] fn test_a_whole_batch_applies() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); chgpasswd(&dir, &[], "staff:one\nwheel:two\n").assert_code(0); assert!(field(&dir, "gshadow", "staff").starts_with("$6$")); @@ -255,6 +291,9 @@ fn test_a_whole_batch_applies() { /// chgpasswd from a possibly-empty list depends on. #[test] fn test_empty_input_succeeds() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); let before = read_file(&dir, "gshadow"); chgpasswd(&dir, &[], "").assert_code(0); @@ -265,6 +304,9 @@ fn test_empty_input_succeeds() { /// blank line in the middle of a batch is a mistake worth reporting. #[test] fn test_a_line_without_a_password_is_refused() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), Some("SHA512")); for input in ["staff\n", "\n", "staff:secret\n\n"] { chgpasswd(&dir, &[], input) @@ -280,6 +322,9 @@ fn test_a_line_without_a_password_is_refused() { /// tool refuses it too, and likewise leaves the file untouched. #[test] fn test_a_colon_in_the_password_is_refused_without_corrupting_the_file() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), None); let before = read_file(&dir, "gshadow"); chgpasswd(&dir, &["-e"], "staff:$6$a:b:c\n").assert_code(1); @@ -312,6 +357,9 @@ fn test_unknown_scheme_is_a_usage_error() { /// MD5 is accepted by the GNU tool and refused here, deliberately. #[test] fn test_md5_is_refused() { + if skip_unless_root() { + return; + } let dir = prefix(GROUP, Some(GSHADOW), None); chgpasswd(&dir, &["-m"], "staff:secret\n") .assert_code(1)