fix(pam_faillock): write deny profile to the filename that is enabled - #360
fix(pam_faillock): write deny profile to the filename that is enabled#360brennanneoh wants to merge 1 commit into
Conversation
apply() writes the pam_faillock "authfail" deny profile to /usr/share/pam-configs/failock (one "l"), but then runs `pam-auth-update --force --enable faillock` using the name declared in pam_update_profile. The names do not match, so the deny profile is never enabled. Only faillock_notify is applied, which supplies `pam_faillock.so preauth` for auth and `pam_faillock.so` for account. Lockout after N failed attempts -- the actual point of the control -- never takes effect. audit() does not catch this because it only requires some uncommented pam_faillock.so line in common-auth and common-account, and faillock_notify satisfies both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Correcting myself before anyone spends review time on the original claim. I originally described this as a security defect — that the misnamed file meant the I've rewritten the PR description accordingly. This is a naming/robustness fix — the The other two reports I opened are unaffected by this and were verified independently: #361 and #362. |
Problem
In
bin/hardening.sh'spam_faillock_enabled.sh,apply()declares the profile names it intends to enable:pam_update_profile="faillock faillock_notify"but writes the first profile to a filename spelled with one
l:pam-auth-update --force --enable faillocktherefore names a profile that does not exist.Actual impact (measured, not assumed)
On a default run the lockout does still take effect, for two reasons I verified in
tests/docker/Dockerfile.debian13:pam-auth-update --force --enable <unknown-name>exits0and silently ignores the unknown profile.pam-auth-updateenables every profile in/usr/share/pam-configscarryingDefault: yes— which both generated profiles do — regardless of what--enablenames.So the misnamed file is still picked up. Measured on
master:Deleting
/usr/share/pam-configs/failockand re-runningpam-auth-update --forcedropsauthfailto0, confirming it is the file, not the--enableargument, doing the work.Why fix it anyway
The behaviour is correct today by coincidence rather than by construction:
--enable faillockcall is a silent no-op. The loop over$pam_update_profilecurrently achieves nothing for that entry, which is misleading to anyone reading or maintaining this.Default: yes. A profile intended to be opt-in (Default: no) would depend entirely on--enablematching the filename, and would never activate.failockfile on every hardened host, which does not match the name in the source and is easy to mistake for cruft.pam-auth-updatesilently ignoring unknown names means this class of typo cannot surface as an error.The change
One character: write the profile to the name that is enabled.
Verification
tests/docker_build_and_run_tests.sh debian13 pam_faillock_enabled.sh— all tests succeed, stderr empty, root/sudo consistency checks pass.shellcheck --exclude=SC2317 --shell=bash -x --source-path=SCRIPTDIRandshfmt -l -i 4 -dare both clean.Before and after the change,
common-authends up with bothpreauthandauthfaillines; the difference is that/usr/share/pam-configs/faillockis now named consistently with the source and the--enablecall actually refers to it.Happy to close this if you consider it not worth the churn.
🤖 Generated with Claude Code