mirror of
https://github.com/uutils/shadow.git
synced 2026-10-16 02:48:14 -07:00
0.3.0
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Pierre Warnier
|
acc1771bc2 |
tests: exercise the PAM stack against known permit and deny services (#281)
The only PAM coverage was the deployment image authenticating a real account. That cannot exercise a *refusal* without relying on a wrong password being wrong, and it cannot tell a refusal apart from a conversation that never reached the stack -- which is exactly the failure that broke password changes earlier this week and took three rounds to find. The glibc images now define two throwaway services, one backed by pam_permit and one by pam_deny, and shadow-core tests the wrapper against both: the permit stack authenticates and passes account management, the deny stack refuses with the stack's own answer rather than a conversation error, and a service with no configuration at all does not authenticate. The tests skip where the services are absent, which is any machine but the containers. CI also gains an all-features clippy pass. The completions feature and the shadow-core feature gates were compiled by no job, so a lint or a compile error in them could reach main unnoticed. make check runs it too. |
||
|
Pierre Warnier
|
64abc1197a |
shadow-core: one generic record reader and writer (#280)
read_*_file and write_* were the same twenty lines five times over, once per record type, differing only in which type they parsed. They delegate to records::read_entries and records::write_entries now, and keep their names because eighteen call sites read better for them. The read_*_with_layout and write_*_with_layout wrappers are gone entirely: each was a single line forwarding to records::, and after the transaction landed only one caller was left, a test that did not need the layout at all. Net: 200 lines out of shadow-core, and one place where a change to how a record file is read or written now has to be made. |
||
|
Pierre Warnier
|
d34ab6f605 |
shadow-core: commit several files together, and convert the last six tools (#279)
group and gshadow have to agree: a group present in one and absent from the other is a broken system, and every tool that touches a group touches both. Committing them one at a time leaves a window in which they disagree, and a failure in the second makes that permanent. commit_all validates every file before writing any, which closes the failure this actually hits -- a value that would corrupt a record is rejected while nothing has been touched, instead of after the first file is already on disk. A genuine I/O error partway through can still leave the set half applied; there is no journal, and a rollback that can itself fail would not be an improvement. All the locks are held until the last write finishes, so no other process sees the intermediate state. A commit that would write the same bytes now writes nothing. A rewrite is not free: it replaces the inode and moves the mtime, and for an unchanged empty file it failed outright, since the atomic writer refuses to produce a zero-length file. Every tool used to carry its own 'did anything change' flag to work around that; those are gone. Found by usermod's tests, whose fixture has an empty gshadow that -G does not touch. groupmod, useradd, usermod, userdel, pwck and grpck are converted, and there is no longer a single FileLock::acquire left in any tool. userdel loses its hand-rolled line filter, which had its own idea of which lines were comments next to the parser that already knows; one consequence is that a comment above a removed account now moves to the end of the file rather than staying in place, which is what every other tool already did. Exit codes are preserved, including the two pwck keeps for a failed write: 6 'can not sort' and 5 'can not update the files' stay distinct by matching on the file the error names. Verified against a prefix tree for each tool: useradd creating an account with a user group, supplementary groups, aging and subid; usermod renaming across four files and replacing a group list; groupmod moving a GID and following it into passwd; userdel removing an account from six files; grpck sorting group and gshadow together, keeping each comment with its group, and writing nothing on a second run. |
||
|
Pierre Warnier
|
fee24971d6 |
shadow-core: a locked-file transaction, and seven tools using it (#278)
Every tool repeats the same five steps: take the lock, read under it,
change the entries, write atomically, release. Written out by hand at
each of the thirty-odd call sites the order is easy to get wrong, and it
was -- pwck -s sorted entries it had read before taking the lock, so a
change another process made in between was silently reverted.
LockedFile makes the correct order the only one available. open() locks
and then reads; entries_mut() hands out the data; commit() validates,
writes atomically and releases; dropping without committing releases
without writing. Reading outside the lock and writing after releasing it
are not expressible. Signals are blocked for the value's lifetime, so a
SIGINT between the lock and the write cannot leave a stale lock file for
every later run to wait out.
Two variants earn their place rather than being conveniences:
- open_or_empty, for a tool that creates a record file where there was
none, such as groupadd into a fresh --prefix tree. open() stays
strict, because for passwd and chage a missing /etc/shadow is its own
error with its own exit code, and reading it as 'no accounts' would
report every login as unknown instead.
- commit_or_remove, for the files where an absent file and an empty one
mean the same thing -- group, gshadow, subid. The atomic writer
refuses zero length, since a truncated account file is a far more
common failure than a deliberately empty one, so deleting the last
group has to unlink. Not for passwd or shadow.
chfn, chsh, chage, passwd, chpasswd, groupdel and groupadd use it, and
lose 212 lines between them. The exit codes are unchanged: a locked file
is still 5 and a missing shadow file still 4, mapped explicitly where the
three cases were previously distinguished by which line failed.
groupmod, useradd, usermod, userdel, pwck and grpck keep their
hand-written sequences: each commits several files that have to agree,
which needs a transaction over more than one file. That is the rest of
this item.
|
||
|
Pierre Warnier
|
eb940c0b46 |
--root is a real chroot in every tool, not a path prefix in eight of them (#277)
* --root is a real chroot in every tool, not a path prefix in eight of them
Five tools called chroot(2) for --root. The other eight -- groupadd,
groupdel, groupmod, grpck, pwck, useradd, userdel, usermod -- folded the
argument into the same resolver --prefix uses, so it only prepended the
directory to the files the tool itself opened.
The difference shows whenever an absolute path is read out of a record
rather than being one of the tool's own files:
useradd -R /mnt/sysroot -m alice
created the home on the *host*, at /home/alice, because the path came
back from login.defs as /home/alice and nothing prefixed it. The skeleton
was copied from the host's /etc/skel. userdel -r -R had the mirror-image
problem and resolved the home to delete against /.
All thirteen now chroot, through one helper in shadow-core rather than
five copies of it, and refuse --root for anyone but root: a setuid-root
binary pointed at a tree of the caller's choosing would read and write
account files that caller controls.
pwck and grpck have no --prefix, matching GNU, so their tests move to a
spawned process: --root cannot be exercised in-process at all now, since
a chroot in the test binary takes every later test into the tree with it,
where /tmp does not exist. That is how this was caught. The other tools'
in-process tests use -P, which is what they meant.
Verified in the deployment container against a minimal target tree: the
account, its home and the skeleton all land inside it, groupadd/groupmod/
groupdel operate on the tree's group file, pwck and grpck read the tree,
userdel -r removes the home from the tree, and the host is untouched
throughout.
Closes #270.
* pwck: name the shadow file in the bad-entry test
The test passed only the passwd file, so the shadow path defaulted to the
host's /etc/shadow. Unprivileged, that cannot be read, and the test
reported 'cannot open' (exit 3) instead of the bad entry it is about
(exit 2). It passed in the containers, which run as root, and failed on
the host runner.
It was --root that hid this: as a path prefix it pointed the default at
the fixture. --root is a chroot now, so the fixture has to name both
files.
* tests: guard the pwck tests that chroot
They spawn 'pwck --root', and chroot(2) needs root: the tool refuses the
option for anyone else. The containers run as root so they passed there,
and the host runner failed them.
|
||
|
Pierre Warnier
|
cf7ecb4a0f |
tests: one binary, spawn the real tool, and assert on what it prints (#273)
* 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.
|
||
|
Pierre Warnier
|
4e539357a1 |
shadow-core: remove duplicated lookups, fix login.defs radices, allocate around NSS (#271)
* shadow-core: delete the duplicated NSS entry type and the unused surface process::PwEntry was a field-by-field twin of passwd::PasswdEntry, copied across at every NSS lookup. getpwuid now returns the parser's own type: an NSS entry and a file entry describe the same account, and a second struct for it only bought a copy per call. lookup_username goes with it, and lookup_username_by_uid becomes one line. The tty module had its own signal-mask guard beside hardening's SignalBlocker, and the PAM conversation had its own controlling-terminal lookup. There is now one of each. error::ShadowResult and ShadowError::Permission had no users at all, and display_message carried an _is_error parameter it never read. harden_process returned a sanitized environment that every one of its thirteen callers discarded, which read as though the process environment had been dealt with. It has not been and is not: the in-process PAM and NSS modules still see what the caller set. The function no longer claims otherwise, and the callers no longer bind a value to throw away. Adds gid_exists, the group-side companion to getpwuid, for the allocator. * shadow-core: read login.defs numbers in the radices the man page documents login.defs(5) writes numbers 'in decimal, or in octal with a leading 0, or in hexadecimal with a leading 0x'. get_i64 parsed decimal only, which is not a rejection but a silent misreading: UID_MIN 01000 means 512 and came back as 1000, so useradd would allocate from a range the administrator had reserved. UMASK 022 and HOME_MODE 0750 have the same shape and will be read the day they are honoured. LoginDefs also gains Default, an empty configuration, which is what a caller wants when the file is missing: every lookup returns None and the tool falls back to its own default. * useradd, groupadd: never allocate an ID the name service already knows /etc/passwd is not the only source of accounts. An ID that is free in the file may belong to an LDAP, SSSD or systemd-homed user, and handing it out gives two accounts one ID -- which the kernel cannot tell apart, so the new local user ends up owning the directory user's files. Allocation now asks NSS as well as the file. Which sources count is an explicit Scope rather than a hidden default, because a --prefix or --root run manages another system's files: the name service running here describes this one, so asking it would skip IDs that are free on the target and claim none that are taken there. SysRoot::is_prefixed decides. uid_range and gid_range were the same function twice over different keys and are now one, the system ranges being the same keys with a SYS_ prefix. The allocation tests moved from next_uid to next_free. Once next_uid consults NSS its answer depends on which accounts exist on the machine running the test, and on a typical container 1000 and 1001 do. The policy is tested where it lives, and two tests assert the property that actually matters against the live system: an allocated ID is in range, not in the entries given, and unknown to NSS. * audit: write to /dev/log instead of forking logger for every event Each account event spawned /usr/bin/logger and waited for it, to write a few dozen bytes to a socket. A setuid-root tool should spawn as little as it can: the fork inherited the process state, depended on a binary being installed at a fixed path, and blocked the caller. The record now goes straight to /dev/log as an RFC 3164 datagram, and carries the terminal it came from instead of a literal '?'. Failure stays silent in every direction. A machine with no syslog daemon is a normal machine, and a record that cannot be written must not fail the operation that produced it. The deployment suite runs rsyslogd and greps the log for the creation and deletion records, so this is covered rather than assumed. Also corrects atomic's claim that its temp-file names are unique 'across threads' in a single-threaded crate; what they separate is successive writes within one process, which usermod does four of in a row. * docs: record the hardening changes and what --root really does sysroot's module documentation said every tool offering --root performs a real chroot. Five of the thirteen do; the other eight fold it into the same resolver --prefix uses, so an absolute path read out of a record resolves against / rather than the target. That is a real divergence worth its own change and is filed as #270; the documentation now says which tools do which and points there. * cargo: only gate what changes linkage; stop forcing applets in with pam Five of shadow-core's features -- shadow, group, gshadow, login-defs, subid -- gated modules that pull in no dependency. Gating them bought nothing and cost real confusion: cargo test -p shadow-core compiled a fraction of the crate and reported a pass for it. It runs 181 tests now where it ran 20, without a single one being written. uid_alloc loses its cfg(all(group, login-defs)) with them. What remains behind a feature is what actually changes the build: pam (links libpam), crypt (links libcrypt on glibc, and now pulls subtle, which nothing else uses) and landlock. In the root manifest, pam = ["passwd/pam", ...] force-enabled the three applets that can use PAM, so asking for PAM in a build that had deliberately left them out dragged them back in. The optional-dependency form pam = ["passwd?/pam", ...] applies the feature if the applet is there and does nothing if it is not. The feat_* aliases and the feat_phase* groups are gone. Every one was a rename of a feature cargo already provides -- feat_passwd = ["passwd"] -- and nothing outside the manifest had referred to them since the phases they were named for finished. Verified against all four delivery paths: make build and make build-multicall still link libpam, the static musl archive still builds and reports all fourteen applets, and a --no-default-features --features pam build now produces a binary with no applets at all and no warnings, which is the case the ? was added for. |
||
|
Pierre Warnier
|
6442faf970 |
chage/passwd/chpasswd/newgrp: aging bounds, input rules and newgrp - (#269)
* chage, passwd: one date module, GNU's -l output, and exit 1 for an unknown login
The Hinnant calendar algorithms were written out three times. They now
live in shadow-core::date, which also gained the inverse direction and
the two display forms, with the arithmetic checked.
That matters here: 'chage -l' adds lastchg + max + inactive, three values
read from a file anyone who can write /etc/shadow chooses. Plain addition
wraps in release builds and panics in debug ones. The sum is now bounded
and an unrepresentable date shows as 'never', which is what GNU displays
for a field it cannot make sense of.
Two -l rules were wrong, both verified against GNU shadow 4.17 in the
Debian image:
- A last change of 0 is passwd -e's 'must change at next login' marker,
not a date. GNU prints 'password must be changed' on all three
password lines; we printed 1970 dates for the two derived ones.
- Expiry is disabled at a maximum age of 10000 days, not 99999. -M 9999
shows a date, -M 10000 shows never.
Both tools also exited the wrong code for an unknown login: chage 15,
which chage(1) reserves for 'can't find the shadow password file', and
passwd 3, which is for an unexpected failure. GNU exits 1 for both.
chage gains --prefix, so its behaviour can be exercised without touching
the live files. The display tests, which asserted only that a string
contained '1970' and that printing did not panic, now assert the exact
output line by line.
* chpasswd: honour ENCRYPT_METHOD, reject the flag pairs the man page forbids
The default hashing scheme was hard-coded SHA-512 while ENCRYPT_METHOD in
login.defs is how a distribution picks one for the whole system. On
Debian, which sets YESCRYPT, every password set through this tool was
weaker than the one the same user would get from passwd. -c still
overrides, and an unreadable or unusable configuration falls back to
SHA-512 rather than refusing to set a password.
Two flag combinations chpasswd(8) documents as errors were accepted and
quietly did nothing: -s without -c (a rounds count with no scheme to
apply it to) and -e with -c (a scheme for input that is already hashed).
Both are now usage errors, and usage errors exit 2 as the man page says.
The batch is also resolved before it is applied: an unknown login in the
middle of a list used to leave the first half written. Membership is
looked up through an index rather than a scan per pair, which took the
critical section -- held with signals blocked -- out of quadratic time.
Also drops the setuid(0) before the write, which raised the real uid and
made caller_is_root() answer true for the rest of the process, and owns
each input line in a zeroizing buffer: every one of them holds a
password.
Adds --prefix.
* newgrp: implement '-', and stop making every shell a login shell
newgrp(1) is 'newgrp [-] [group]'. The dash was not implemented, so
'newgrp - docker' looked for a group called '-' and reported it missing.
Worse, every invocation execd a login shell (argv[0] = -sh) whether or
not the dash was given. newgrp(1) says the current environment and
working directory are kept without it, so the profile files were being
re-read on each 'newgrp docker', in an environment they had already been
applied to.
Now: without '-', a non-login shell inheriting the environment; with '-',
a login shell, chdir to the home directory, and an environment built
explicitly (HOME, SHELL, USER, LOGNAME, PATH, plus TERM and the locale
variables a real login inherits from the terminal). That needs execve,
added to shadow-core::process, because std::env::set_var is unsafe and
process-global in edition 2024.
Membership and password sources were checked against GNU shadow 4.17
rather than changed on assumption, with four groups covering the cases:
membership comes from /etc/group and the password from /etc/gshadow, and
a user listed only in gshadow's member list is not admitted. That is what
this already did, so it stays.
The duplicated EchoGuard is gone; the prompt goes through the shared
terminal helper, which blocks interrupts for the read. The env
sanitization call that computed a value and dropped it is gone too, with
a note on why newgrp deliberately does not sanitize.
The parse tests now exercise the operand grammar instead of asserting
that clap stores a string.
* shadow-core: scrub the crypt(3) password copy, identify the caller to PAM
crypt(3) takes C strings, so a copy of the password has to exist.
CString::new made that copy on the heap and freed it without zeroing:
the caller's Zeroizing<String> scrubs its own buffer and never learns
about this one. The copy is now owned in a zeroizing buffer whose
capacity is reserved exactly, since a reallocation would leave a first
copy behind unzeroed.
The PAM response cleanup zeroed with ptr::write_bytes immediately before
free(). That is a dead store, and the optimiser is entitled to delete it,
leaving the password in the heap. Volatile writes and a compiler fence
cannot be elided.
PAM_TTY and PAM_RUSER were never set, so pam_unix and pam_faillock logged
tty=? with no requesting user, which makes their records useless for
tracing a failed authentication and stops pam_faillock's per-tty handling
from working at all. Both are now set when the handle is created, best
effort: a tool with no controlling terminal must still be able to
authenticate.
* docs: rewrite the chage, chpasswd and newgrp man pages; cover the rules end to end
None of the three explained the rules that actually decide what happens:
what a last change of 0 means in the chage output and where the 'never'
threshold sits, that chpasswd takes its scheme from ENCRYPT_METHOD and
which flag pairs are errors, or what newgrp's dash does and where
membership and the group password are each read from. All three now carry
value restrictions, configuration, exit status, files and worked
examples.
The deployment suite gains thirty-five assertions against the real
binaries: the three 'password must be changed' lines, a date at -M 9999
and 'never' at -M 10000, -1 clearing a field while -5 is refused, exit 1
for an unknown login from both chage and passwd, chpasswd following
ENCRYPT_METHOD through two changes and -c overriding it, the two usage
errors, --prefix on both tools leaving the live files untouched, and
newgrp keeping the environment without '-' and reinitializing it with.
|
||
|
Pierre Warnier
|
b6c318d1d5 |
chfn/chsh: interactive mode, and fix a setuid path oracle (#268)
* shadow-core: add a shared terminal helper
Reading a line and reading a password were open-coded in pam.rs, and
every prompt outside a PAM conversation had no helper at all.
tty.rs holds all of it: prompt_line, read_password and read_stdin_line,
with the EchoGuard that restores the terminal. Ctrl-C at a password
prompt used to leave echo disabled -- the signal terminates the process
without unwinding, so the guard's destructor never ran. The read now
blocks SIGINT, SIGQUIT and SIGTSTP for its duration, which is what
readpassphrase(3) does, and restores the mask on the way out.
The prompt prefers /dev/tty and falls back to stderr and stdin when
there is no controlling terminal (ENXIO/ENODEV), the case that broke
password changes under su -c.
* chfn: prompt for fields, and check values before asking for a password
chfn(1) with no field option prompts for each field with its current
value; ours refused with 'no flags specified'. It now prompts, and an
empty answer keeps the field.
Three fixes come with it:
- CHFN_RESTRICT is enforced on the values, not on which options were
passed. Interactive input went through unrestricted, and a field the
caller may not change is now not prompted for at all.
- Values are validated before the PAM conversation, so a rejected
value no longer costs the caller a password prompt.
- mutate_passwd no longer calls setuid(0). euid 0 is all the lock and
the atomic write need; raising the real uid made caller_is_root(),
which is deliberately real-uid based, answer true for everyone.
The five sub-fields are now one Changes value with the collect,
validate, permission and apply steps on it, which is what let uumain
shrink rather than grow.
* chsh: prompt for the shell, and stop leaking path existence
Three changes to the self-service path.
chsh(1) with no -s prompts for the new shell with the current one in
brackets; ours refused with 'no shell specified'. Pressing ENTER, or
repeating the current value, is now a successful no-op.
An empty shell is accepted. passwd(5) gives the field no meaning of its
own and login falls back to /bin/sh, so -s '' selects the system default
rather than naming a program that does not exist.
The order of the two checks on a shell now matters and is documented.
chsh is setuid-root, so Path::exists answers with root's view of the
filesystem: testing existence first made the tool an oracle for paths
the caller cannot stat, one probe at a time --
chsh -s /root/.ssh/id_ed25519
answering 'is not listed in /etc/shells' for a file that exists and
'does not exist' for one that does not. For a non-root caller the
membership test now runs first and discloses nothing, /etc/shells being
world-readable. Root still bypasses the list but not existence.
Validation also moved ahead of the PAM conversation, so a refused shell
no longer costs a password prompt, and mutate_passwd no longer calls
setuid(0): euid 0 is enough, and raising the real uid made
caller_is_root() answer true for every caller.
* docs: rewrite the chfn and chsh man pages; cover self-service end to end
The man pages documented neither interactive mode, nor CHFN_RESTRICT,
nor the restricted-account rule, nor what an empty shell field means,
and chfn's still said a field option was mandatory. Both now carry
permissions, value restrictions, configuration, files and worked
examples, and chsh explains why the two shell checks run in the order
they do.
The deployment suite gains eighteen assertions against the real setuid
binaries: interactive chfn and chsh through PAM, an empty answer as a
no-op, CHFN_RESTRICT enforced on interactive input, the empty shell
field, and the oracle itself -- an existing and a missing path under
/root must give a non-root caller the same answer, with no password
prompt before it.
|
||
|
Pierre Warnier
|
5bb637da38 |
pwck, grpck: report what the man pages say they report (#267)
grpck -q is documented as 'report errors only', but every error print was gated on !quiet along with the warnings, so -q silenced the one thing it is supposed to keep. Errors are now always shown. A gshadow file that could not be parsed was turned into an empty list and a warning, and the group/gshadow consistency check then ran only when that list was non-empty -- so a malformed or empty gshadow beside a populated group file reported nothing at all. The check is driven by the file existing, and a parse failure is an error. grpck also verifies what its man page calls 'a valid list of members and administrators': a name in either list that no account carries is now reported. pwck resolved a relative home or shell against its own working directory, so it reported on whatever happened to be there; a relative path is itself the fault and is reported as one. The exit code for a sort that cannot be written was defined and never emitted; it is now used. The home and shell checks move into their own function, which is also what keeps the entry loop readable. |
||
|
Pierre Warnier
|
90e5043fd3 |
group tools: close the groupadd race and add the missing options (#266)
groupadd read /etc/group before taking its lock, then checked the name and allocated a GID from that snapshot. Two concurrent 'groupadd -r' runs could therefore pick the same GID, and two 'groupadd www' could both conclude the name was free. The lock is now taken first and the file read under it. Four documented options were a usage error: groupadd -U and groupmod -U/-a manage a group's member list, and groupdel -f removes a group even when it is a user's primary group and succeeds when the group is already gone. groupmod -p was a silent no-op on a system without /etc/gshadow; the password now goes into the group file, which is where such a system keeps it. groupdel left the deleted group in /etc/gshadow whenever it was the only entry there -- the write was skipped for an empty result -- and could not delete the last group in /etc/group at all, because the atomic writer refuses a zero-length file. Both now unlink the file instead, an absent file meaning the same as an empty one. |
||
|
Pierre Warnier
|
a1ca5d448e |
usermod: fix the group handling and report the documented exit codes (#265)
-g accepted only a numeric GID and wrote it through unchecked, so 'usermod -g 12345' left the account with a primary group that does not exist. It now takes a name or a GID and requires the group to exist, exiting 6 as usermod(8) documents. -G '' split into one empty name and was rejected as a group that does not exist; the man page defines it as removing every supplementary membership. It does that now, -G accepts GIDs as well as names, and -a requires -G. -l combined with -G added the *old* login to the groups, after the rename had already happened -- a member naming no account. The new login is used. /etc/gshadow was never updated by -G or -l, which is precisely what grpck reports as 'members differ'. Both now keep it in step with /etc/group, and it is only written when something actually changed. -L prepended a second '!' to an already-locked password, which one -U could not undo; it is now idempotent. -U refuses to leave an account with no password instead of silently doing nothing. Group failures report 6 and 10 rather than the catch-all 1. |
||
|
Pierre Warnier
|
d48d19c749 |
useradd: honour /etc/default/useradd, add -b and -P, close the home chown race (#264)
* shadow-core: read and write /etc/default/useradd useradd's own defaults live in /etc/default/useradd as KEY=VALUE, a different format from login.defs and a different set of keys (GROUP, HOME, INACTIVE, EXPIRE, SHELL, SKEL, CREATE_MAIL_SPOOL). Nothing read it, so a site that set SHELL there had it ignored. A missing file reads as no values, and the writer replaces the file atomically. * useradd: honour /etc/default/useradd, add -b and -P, close the home chown race useradd -D with a value printed the defaults and exited 0 without saving anything, and nothing ever read /etc/default/useradd, so a site's configured SHELL, SKEL or HOME base was silently ignored. -D now persists what it is given, the file takes precedence over login.defs for the keys it holds, and a new account uses those values. -b/--base-dir and -P/--prefix are added, both documented in useradd(8) and both previously a usage error. A UID of 4294967295 is rejected: that is (uid_t)-1, the 'no change' sentinel of chown and setresuid. The new home directory is chowned through a descriptor opened with O_NOFOLLOW instead of by path. Between the mkdir and the chown, anyone able to write the parent -- a home under /tmp, or a shared base directory -- could replace the directory with a symlink and be handed its target. |
||
|
Pierre Warnier
|
968c2d8bf2 |
useradd: follow useradd(8) for system accounts, groups, ID allocation and home creation (#263)
* shadow-core: match shadow-utils on name rules and ID allocation Two behaviours verified against shadow-utils rather than assumed: Names. 'useradd Alice' and 'useradd MACHINE$' both succeed there, so rejecting upper case and a trailing dollar meant refusing accounts that exist on real systems -- the dollar is how Samba names machine accounts. Both are accepted now; the length cap and the rest of the character rules are unchanged. Allocation. Creating a, then b, deleting a, then creating c gives c the ID after b's, not a's. We handed out the lowest free ID, so a new account inherited a deleted one's ID and with it any files left behind. Allocation now starts past the highest ID in use within the range and falls back to the first gap only once the top of the range is taken. * useradd: follow useradd(8) for system accounts, groups and home creation System accounts (-r), all three verified against shadow-utils: no home directory even when CREATE_HOME is yes, no aging information in /etc/shadow (it writes 'svc:!:20700::::::'), and no subordinate UID/GID ranges. Groups: -g and -G now require the group to exist, named or numbered, and report exit 6 when it does not. A numeric GID naming no group was accepted, leaving the account with a primary group that does not exist -- id(1) then prints a bare number and new files get an unnamed group. -G also refused the numeric form the man page allows. Home directories: HOME_MODE from login.defs is honoured instead of a hardcoded 0700, and a missing parent of the home is created (0755, root-owned) instead of failing with ENOENT. |
||
|
Pierre Warnier
|
2bfeba0b50 |
shadow-core: stop deleting comments and NIS compat lines when rewriting account files (#262)
* shadow-core: keep the lines that are not entries A record file holds more than entries: comments, blank lines, and the NIS compatibility lines nsswitch.conf(5) documents for compat mode (+user, +@netgroup, -user, a bare +::::::). The parsers dropped all of them, so the first tool to rewrite a file deleted every comment in it, and the compat lines did not parse at all -- on a host using compat, every tool failed on the first line it could not read. records::read_with_layout returns the entries plus a Layout holding those lines, and write_with_layout puts them back. Each raw line is anchored to the name of the entry it preceded rather than to a line number, so it stays with that entry when the file is reordered and survives entries being added or removed elsewhere; a line whose entry disappears is written at the end rather than lost. The entry-only readers now skip raw lines instead of failing on them. * tools: preserve comments and compat lines when rewriting account files Every read-then-write path switches to the layout-preserving reader and writer, so useradd, usermod, userdel, groupadd, groupmod, groupdel, chfn, chsh, passwd, chage and chpasswd all leave the rest of the file alone. pwck and grpck additionally re-read under the lock before sorting: they were sorting entries read before it, so a concurrent change could be overwritten, and the sort now carries each comment along with the entry it described. |
||
|
Pierre Warnier
|
3f7f070ab6 |
chage, passwd: bound the aging fields and let -1 clear them (#260)
The four day-count fields accepted any negative value and wrote it through, so 'chage -M -5' left a policy no reader can make sense of. Only -1, meaning 'unset', may be negative now; anything lower exits 2 for chage and 6 for passwd, the codes their man pages give for an invalid argument to an option. The check runs before any file is opened. passwd could not even be given -1: the four options lacked allow_hyphen_values, so 'passwd -x -1' -- which passwd(1) documents as 'no maximum' -- was rejected by the parser. They accept it now, and -1 clears the field instead of being stored literally, matching what chage writes. |
||
|
Pierre Warnier
|
685c454863 |
chpasswd: refuse an empty plaintext password, keep the password verbatim (#258)
'alice:' on stdin hashed the empty string. That is a perfectly valid hash, so the account then logged in with a bare Enter. Plaintext mode now refuses an empty password and names the offending line; -e, which takes a pre-computed field, still accepts one because that is how ! and * locks are written. The input line was trimmed before the split, so 'alice:secret ' stored the hash of 'secret' -- a different password than the caller supplied, with no indication. The password is now everything after the first colon verbatim; only a trailing CR from CRLF input is removed. Hashing moves out of the critical section: a batch of yescrypt or high rounds= hashes held the /etc/shadow lock, with signals blocked, for the whole run. |
||
|
Pierre Warnier
|
eee6b34be0 |
usermod: reject a rename onto an existing login, accept dated -e (#257)
* shadow-core: one calendar implementation for the expiry fields useradd, usermod and chage each carried their own date arithmetic and their own idea of what -e accepts. Move the Gregorian conversion and the expiry parser into shadow_core::date, with the parser accepting the YYYY-MM-DD form the man pages document, an empty value or -1 for 'no expiry', and a bare integer already in days since the epoch. Impossible dates are rejected rather than normalised. useradd now calls it and drops its copies of days_since_epoch, is_leap_year and days_in_month; their tests move to the new module. * usermod: reject a rename onto an existing login, accept dated -e usermod -l never checked that the new name was free, so 'usermod -l root alice' left two entries named root in /etc/passwd and in /etc/shadow. It now exits 9, the code usermod(8) documents for a login already in use. -e only accepted days since the epoch, so 'usermod -e 2030-01-01' -- the form the man page documents and the form Ansible and shell scripts pass -- failed. It now goes through shadow_core::date, and is parsed before the passwd file is written so an invalid date no longer leaves the other fields already committed. |
||
|
Pierre Warnier
|
69d0454f46 |
chfn, chsh: enforce CHFN_RESTRICT and refuse restricted shells (#256)
chfn let any non-root user change every GECOS sub-field. login.defs(5) CHFN_RESTRICT names which fields a regular user may change (yes == rwh; an explicit letter set; unset means none -- 'only the superuser can make any changes'). chfn now reads it and refuses a field a non-root caller is not allowed to change; root stays unrestricted. GECOS values are also rejected when they contain '=' or control characters, per chfn(1), not just ':' and commas -- the shared shadow-core field validator does the character check. chsh let a restricted account change its own shell. chsh(1)/shells(5): an account whose current shell is not listed in /etc/shells is restricted and may not change it, so a confined login (e.g. /bin/rbash deliberately kept out of /etc/shells) could escape to a normal shell. A non-root caller in that state is now refused; an empty current shell means the default /bin/sh and is not restricted; an unknown user fails closed. chfn gains the login-defs feature of shadow-core it now needs. |
||
|
Pierre Warnier
|
75d70d3e38 |
pwck, grpck: never rewrite the files when a line failed to parse (#255)
-s sorts and rewrites the entries that parsed. When a line did not parse -- exactly the case these tools exist to report -- it was left out of that set, so `pwck -s`/`grpck -s` silently dropped every invalid line and every comment while returning exit 2 as if nothing had changed. They now report the errors and exit 2 without touching the files. -r and -s are rejected together, which the man pages state cannot be combined. |
||
|
Pierre Warnier
|
3a246b4e23 |
userdel: don't destroy homes it shouldn't, clean up the private group and subids (#254)
Aligns userdel with userdel(8) on the data-destroying edges: - -r refused to consider ownership or sharing: it would remove a home the account merely pointed at (a root-owned /srv/data) or one shared by another account. It now refuses a home not owned by the user, or shared with another still-present account, unless -f -- which the man page ties to exactly these cases. - the user's private group (the USERGROUPS_ENAB group named after the login) was never removed, so useradd/userdel cycles accumulated zombie groups and drifted UID/GID pairs. It is now removed when it has no other members, kept when it does or when it is another user's primary group (unless -f). - /etc/subuid and /etc/subgid rows for the user were left behind, so a later user of the same name silently inherited the ranges; they are now removed (the file is unlinked if it becomes empty, which the atomic writer's zero-length guard would otherwise forbid). - a missing user exited 1; userdel(8) says 6. With -f the absence is tolerated and the remaining cleanup still runs. -f is documented for what it now does instead of 'no effect'. |
||
|
Pierre Warnier
|
37958beaf3 |
groupmod: renumber primary-group members in /etc/passwd on -g (#253)
groupmod(8): "Users who use the group as their primary group are updated to keep the group as their primary group." We only rewrote /etc/group, so after `groupmod -g` every user whose primary group was that GID pointed at a number that no longer named a group -- id(1) showed a bare number, pwck reported the group missing, and new files got an unnamed group. The passwd lock is taken before the group lock (the tools' order is passwd < group < gshadow < shadow) so this stays deadlock-free with useradd/usermod, and only when passwd exists -- a --prefix tree may not carry one. GID 4294967295 ((gid_t)-1) is rejected as a bad argument. |
||
|
Pierre Warnier
|
4004204e3f |
shadow-core: take /etc/.pwd.lock so we exclude the system's account tools (#252)
The account files were guarded only by their own .lock files, which the rest of the system does not honour. glibc's lckpwdf(3) — used by vipw, pwconv, libuser, systemd-sysusers and pam_unix — takes an fcntl write lock on /etc/.pwd.lock instead, so a password change run by pam_unix could complete in the middle of our read-modify-write and be silently reverted. FileLock now takes that fcntl lock too for passwd/shadow/group/gshadow, reference-counted per tree so the several files one tool locks share a single lock and release it with the last one. A --prefix run locks the prefixed tree's .pwd.lock. Two long-standing lock-loop faults are fixed in passing: a stale .lock that cannot be removed (e.g. chattr +i) spun the retry loop at full CPU with no sleep and no deadline check, and any hard_link error other than 'already exists' (such as EPERM on a filesystem without hard links) was treated as 'held' and waited out the whole 15-second timeout instead of failing immediately. |
||
|
Pierre Warnier
|
382edd4dc2 |
passwd: make the interactive password change work; repair and CI the e2e suite (#251)
* passwd: make the interactive password change work Three faults kept passwd from changing a password, all found by running a real change as an unprivileged user under su -c and read from pam_unix's own debug log: - the PAM conversation only ever opened /dev/tty, which fails with ENXIO when there is no controlling terminal (su -c, a service, cron); pam_unix then reported "conversation failed ... (old) token not obtained". It now falls back to stderr for the prompt and stdin for the answer, the way getpass(3) and PAM's misc_conv do, and still prefers /dev/tty when there is one; - pam_authenticate was called against the passwd service, which defines no auth stack, so it failed before any prompt. The current password is verified by pam_unix inside pam_chauthtok, keyed on the real uid; the call is removed; - the privilege drop spanned pam_chauthtok, so pam_unix could neither run its setgid helper nor rewrite /etc/shadow. euid 0 is what the setuid bit is for; the drop is gone. Landlock is no longer applied to the PAM path: restrict_self sets no_new_privs, which strips the setgid bit from unix_chkpwd. It now wraps only the direct-file paths, with rules that let NSS, nscd and logger run. --prefix becomes root-only, like --root. * e2e: repair the deployment image and run it in CI Dockerfile.e2e ran `make install`, which since #138 installs per-tool binaries, then chmod 4755'd a shadow-rs multicall that was never installed, so the image had not built for months. It now uses install-multicall, and an `e2e` CI job builds and runs the suite on every pull request. The suite now changes a password through PAM as an unprivileged user and proves it by authenticating with the new password and failing with the old (root's own su never prompts, so the old checks proved nothing), rejects a wrong current password, has root set one without the old, checks the setuid privilege model for pwck/grpck and that /etc/shadow keeps its owner and mode. tests/pam-e2e.sh, which could not fail, is deleted. * docs: record the passwd PAM fix and the e2e CI job |
||
|
Pierre Warnier
|
881aad13c7 |
shadow-rs: drop the setuid privilege for applets that are not meant to have it (#239)
* shadow-rs: drop the setuid privilege for applets that are not meant to have it The per-tool install marks only passwd, chfn, chsh and newgrp setuid; a multicall install has to make the one binary setuid, which gave every applet euid 0. Any local user could run 'shadow-rs pwck -s' or 'grpck -s' and have /etc/passwd, /etc/shadow, /etc/group and /etc/gshadow rewritten as root. Before an applet outside that set runs, the binary now calls setuid(getuid()), so the real, effective and saved uid are all the caller's and the two layouts share one privilege model. It fails closed if the kernel does not take the privilege away. Dispatch and --list are driven by one table instead of two hand-kept lists, and an exit code outside 0..=255 no longer clamps to success. The deployment test checks that testrunner's pwck -s and grpck -s leave the files' inode and ownership untouched. * docs: the multicall binary keeps euid 0 for the four self-service tools only |