Add password scanner - #424
Conversation
Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
jonbartels
left a comment
There was a problem hiding this comment.
I think the approach is wrong. This take a known password list and checks against it like an actual login.
Since we control the DB and can read the salts I think it would be more efficient to compute a rainbow table using large known password list like this example. Then compute the hash from that list for each users salt. Then do a lookup by hash.
Also consider my usual comment - is there a library or utility that does this for us? John the Ripper was the tool to use back in the day, but I haven't kept pace.
This is the rub. Salts are used to prevent rainbow tables from being used. We can't run a large password list against each user since computing the hash is expensive by design. A small wordlist is the best that can be done in a background scanner. To check a large table, we would have to do it when we have the cleartext in hand (e.g.: during set or during login). Doing it without the cleartext is not possible to do securely, since any weakened hash (even if not used during login) can also be used to crack passwords. |
|
Good direction overall. One concern worth flagging before this merges: the warning log includes the username of the affected account. Anyone with access to the server logs, whether via a SIEM, a central log management platform, or direct file access, can see which accounts have weak passwords and correlate that against the bundled wordlist. That is information they should not have, and the daily cadence means it persists in archives long after the password has been changed. A cleaner approach: keep the background scanner for detection, but log only an aggregate count without account names, for example: "2 account(s) have passwords matching known-weak credentials, administrator action required." Combine this with a forced rotation at next login when the stored hash matches. This way dormant accounts are still detected and flagged, but log readers cannot determine which accounts are affected. This also covers existing installations that PR #421 does not reach, and aligns with NIST SP 800-63B §5.1.1.2. The mgaffigan/jonbartels debate about salts vs. rainbow tables is largely moot, mgaffigan is correct that salts make precomputed lookups ineffective against expensive hashes. |
Making it known to log-readers is the cost for keeping the alert actionable. The vulnerability is the weak password, not the warning; an attacker can trivially re-create this information.
Change after login is addressed in #427
NIST SP 800-63B §5.1.1.2 password requirement is addressed in #426. |
Add a password scanner for trivial passwords. Logs to server event log.