Page MenuHomeFreeBSD

pam_unix: Reliably disable empty password auth
Needs ReviewPublic

Authored by zirias on Nov 23 2023, 11:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 8:19 PM
Unknown Object (File)
Fri, Apr 26, 2:23 AM
Unknown Object (File)
Feb 14 2024, 2:38 PM
Unknown Object (File)
Dec 25 2023, 5:16 AM
Unknown Object (File)
Dec 23 2023, 3:03 AM
Unknown Object (File)
Dec 21 2023, 3:46 AM
Unknown Object (File)
Nov 30 2023, 6:27 PM
Unknown Object (File)
Nov 29 2023, 5:19 AM
Subscribers

Details

Reviewers
des
Summary
There are two possibilities to store an empty password in the passwd(5)
database. We can either store an empty hash, or a hash of the empty
string.

pam_unix checked for both cases separately. The code checking for an
empty hash succeeds without prompt when empty hashes ("nullok") are
allowed, otherwise changes the hash to "*" to force authentication to
fail later. The code checking for a hash of the empty password also
immediately succeeded when empty passwords ("emptyok") are allowed, but
missed the part forcing a failure otherwise. As a result, with a hash of
the empty password stored, you could still successfully authenticate by
just hitting enter on the password prompt that appears without the
"nullok" or "emptyok" option.

Unify both code blocks to make sure they do exactly the same, enforcing
authentication with an empty password to always fail without the
"nullok" option. Remove the now redundant "emptyok" option. Update the
manpage accordingly, and also remove a warning that "nullok" could allow
authentication with any password when invoked unprivileged: This won't
happen because getpwnam(3) puts "*" in the password field in that case.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 54604
Build 51493: arc lint + arc unit

Event Timeline

I just noticed I broke the already existing emptyok option with this. I'm not sure whether it's a good idea to distinguish them at all, but they're certainly used in an inconsistent way: During password change, whether an empty password will be allowed depends on nullok (line 390), nevertheless a hash will be stored, matching the "empty" password.

I'll invest some more thought and update this review, sorry for the noise for now.

Remove the emptyok option to really treat both possible ways of storing an "empty" password the same. I think everything else would be confusing and bear the risk of misconfiguration.

As an example, consider "AllowEmptyPasswords" is set in sshd (maybe by accident). When neither "nullok" nor "emptyok" is set, authentication via SSH would work when there's a hash of the empty password stored, but not when there's *no* hash stored. Furthermore when changing the password and supplying an empty one, a hash of the empty string will be stored, although the "nullok" option is checked to decide whether to allow the empty password at all – I try to address this inconsistency in the linked child revision.

Out of curiosity, I also checked the behavior of a Linux system (with LinuxPAM and shadowtools). It suffers from the same problem, but makes it very hard to store a hash of an empty password at all. The passwd tool just rejects an empty password without even consulting PAM (which is all the wrong way, but okay ...). So at least the risk of misconfiguration is lower, the only way I found to store a hash of the empty password was to supply the hash itself to "usermod".

Of course it would also be possible to address the risk/confusion *keeping* the "emptyok" option, but this would IMHO require more complex code and I don't see the benefit ... please correct me if you think I'm wrong here.

edit: Added the protagonists of https://reviews.freebsd.org/D27569 as it introduced the "emptyok" option in the first place, hope that's ok...

zirias added subscribers: trasz, markj.