chgpasswd: set group passwords in batch - #292
Open
pierre-warnier wants to merge 2 commits into
Open
Conversation
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
chgpasswd, the seventeenth tool:chpasswdfor groups.Stacked on #291 — the diff against that branch is
chgpasswdalone. Base retargets tomainonce #291 merges.It reads
group:passwordlines from stdin and applies them, following the rule that makes a batch tool safe to run unattended: every line is parsed, every group resolved and every password hashed before any file is written. 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
Established by running the GNU tool, not by reading it:
/etc/group/etc/gshadowpresentx/etc/gshadowabsent/etc/group/etc/groupis world-readable, which is the whole reason for the split. And no gshadow file is created where none exists: conjuring one up would change how every other tool on the host reads group passwords.Deliberate divergences
Three, all refusals:
-c NONE. GNU honours it and stores the password as clear text in/etc/gshadow. A readable group password is worth no more than none at all.-ealready writes a field verbatim when that is genuinely what is wanted, and the error message says so — otherwise an operator just tries-c MD5next.-m,-c MD5,-c DES— aschpasswdalready refuses them.""produces a valid hash that a bare Enter matches: a group anyone can enter, not a group with no password.-ewith an empty field remains the way to clear it.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 — agreement, not divergence.
Verification
cargo test --workspacemake checkpam— exit 0make test-gnu-compatThe e2e section checks the tools against each other rather than only against file shapes: the password
chgpasswdsets is the onesgthen accepts from a non-member, and a wrong one is refused. A hash written in a format nothing on the host can verify would pass agrepassertion and fail that one.