mirror of
https://github.com/uutils/shadow.git
synced 2026-10-16 02:48:14 -07:00
Pierre Warnier
cf7ecb4a0f
* tests: one binary, spawn the real tool, assert on what it prints
Fourteen [[test]] targets each linked all fourteen tool crates and
carried its own copy of the helpers through a #[path] include, so the
suite was compiled and linked fourteen times over. It is one target now,
the layout the rest of uutils uses.
More importantly the tests called uumain in-process and asserted on an
exit code and nothing else. Not one assertion in the suite looked at a
byte of output, so 'bit-for-bit identical to GNU' was never checked. A
shared process also let harden_process, setuid, Landlock and umask
changes leak from one test into the next.
common now offers a spawned-binary helper with assertions on the exit
code and both streams, and the tests use it. Three findings came out of
that within the hour:
- chage exited 3 for an invalid date where GNU exits 2; 3 is for an
unexpected failure, not for an argument the caller got wrong.
- passwd -l was not idempotent. Eight concurrent locks left eight
markers, and since passwd -u refuses to unlock an account that would
stay locked, an account locked twice could not be unlocked with the
tool at all. GNU leaves a single marker however many times it is
asked; two unit tests that encoded the old behaviour are rewritten
to the verified one.
- pwck's duplicate-UID test accepted either 0 or 2, so it asserted
nothing. Two accounts may deliberately share a UID (useradd -o), and
GNU reports nothing; a duplicate *name* is the fault, and that is now
what is checked, message included.
test_chage and test_chpasswd never invoked their tool at all -- they set
a field through shadow-core and read it back, testing the parser twice.
Both now drive the binary through --prefix, which those tools have since
gained, and cover the aging-field writes, the -l wording and thresholds,
the batch rules, ENCRYPT_METHOD and the flag pairs the man page forbids.
The concurrency test raced four threads through uumain against a lock
keyed on the process id, which same-process threads never contend for,
and discarded all eight return codes. It spawns eight processes now and
checks every one.
New: the multicall binary is tested for the property one table now
guarantees -- every applet listed is reachable, and the help text is
identical through the argument and the symlink.
* passwd: make -l idempotent
Locking prepended '!' unconditionally, so locking an account twice left
'!!'. One unlock then removed one marker and left the account locked --
and passwd -u refuses to unlock an account that would stay locked, so
the tool could not undo its own second lock.
GNU shadow 4.17 leaves a single marker however many times it is asked:
three locks give one '!', which one unlock removes. Unlocking a field
that already carries two markers still removes exactly one, which is
also what GNU does; that case is reachable only from a file some other
tool wrote.
Found by running eight concurrent passwd -l processes against one
account and looking at the result, which is a thing the suite could not
do while it called uumain in-process. Two unit tests asserted the old
behaviour and now assert the verified one, and the deployment suite
locks twice and checks the marker did not double.
* ci: make root-only skips visible, and test root together with pam
Roughly three quarters of the integration suite is root-only, and a
skipped test is reported ok, so a run that exercised almost nothing
looked exactly like a run that exercised everything.
SHADOW_TEST_REQUIRE_ROOT turns a skip into a failure. The Docker images
set it, since they run as root: 27 useradd tests that reported ok off-root
fail loudly there now if the suite ever stops running as root.
The matrix also runs with pam. The feature changes which code paths
exist -- chfn and chsh refuse every non-root call without it, and passwd
loses its interactive path -- so root and PAM together had never been
exercised by any job.
* fuzz: assert a real invariant, add a subid target, build in CI
The targets asserted only that parsing does not panic. For a record file
the property that matters is that a parsed entry survives being written
back: if Display can emit a line that parses differently, a tool
rewriting the file silently changes an account. Each target now checks
that the rendered line carries exactly the separators its format has --
one more would shift every following field on the next read, which is
how an injected account appears -- and that parsing it again renders
identically. /etc/subuid had no target at all.
The crate is a separate workspace nothing else builds, so it had rotted:
its manifest asked shadow-core for features that no longer exist. A
stable 'cargo check --bins' in CI catches that without nightly.
The corpus was both gitignored and tracked. It is kept: 667 accumulated
inputs are worth carrying between runs, and the main suite now replays
them through the same two properties on every pull request, so a
regression a fuzzer once found is caught on stable without a fuzzing
session.
* chage: an invalid date is a bad argument, not an unexpected failure
'chage -E 2025-02-29' exited 3, which chage(1) uses for something going
wrong that the caller did not ask for. A date the caller typed wrongly is
exit 2, which is what GNU shadow 4.17 answers -- 'chage -d -5 u' prints
"invalid date '-5'" and exits 2.
Found by the integration tests once they ran the real binary and looked
at its exit code.
* tests: guard the chage tests that need root, and let an empty flag mean off
The chage tests drive the tool against a --prefix tree, but 'chage -l' on
an account other than the caller's, and every modification, are refused
for a non-root caller before any file is opened. The host runner is not
root, so those tests failed there.
SHADOW_TEST_REQUIRE_ROOT also treated an empty value as set, so
'-e SHADOW_TEST_REQUIRE_ROOT=' turned the requirement mysteriously on
rather than off. An empty value now counts as unset, which is the
conventional reading.
* ci: do not ask Alpine for a pam build
Alpine's Rust image targets musl with crt-static, where shadow-core
refuses the pam feature at compile time on purpose: Linux-PAM loads its
modules with dlopen(3), which a static binary cannot do, so a build that
compiled would produce an authentication path that never works.
The new 'with pam' matrix step ran on all three distros and so hit that
guard. It runs on the glibc images now. The static musl job already
asserts that requesting pam there is rejected, and Alpine is in the
matrix for musl's libc behaviour, not for PAM.
9 lines
131 B
Plaintext
9 lines
131 B
Plaintext
/target
|
|
/fuzz/target
|
|
/fuzz/artifacts
|
|
/fuzz/Cargo.lock
|
|
CLAUDE.md
|
|
PLAN-shadow-rs.md
|
|
docs/claude-review-*.md
|
|
docs/gemini-review-*.txt
|