feat: self-describing password hashes (iterations$hex)#41
Open
aberoham wants to merge 1 commit intoNicTool:mainfrom
Open
feat: self-describing password hashes (iterations$hex)#41aberoham wants to merge 1 commit intoNicTool:mainfrom
aberoham wants to merge 1 commit intoNicTool:mainfrom
Conversation
aberoham
force-pushed
the
feat/password-upgrade
branch
2 times, most recently
from
13d2dae to
3d4ef53
Compare
aberoham
changed the title
aberoham
force-pushed
the
feat/password-upgrade
branch
from
3d4ef53 to
1d023e3
Compare
Contributor
|
hmmm, needs to be rebased. I like the idea and description. |
Store PBKDF2 iteration count alongside the hash to eliminate the
linear fallback chain that tried current then legacy iterations on
every login attempt. A wrong password now costs exactly one PBKDF2
computation regardless of how many iteration baselines have existed.
Format: `iterations$hexHash` (validated by /^\d+\$[0-9a-f]{64}$/).
Legacy formats (plain text, SHA-1, raw PBKDF2-5000) are detected
and silently upgraded to self-describing format on login.
validPassword() returns { valid, needsUpgrade } instead of setting
mutable instance state — callers destructure the result directly.
PBKDF2_ITERATIONS env var (default 220000) is validated at module
load. Future iteration bumps require zero code changes — users at
older iterations upgrade on next login.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
aberoham
force-pushed
the
feat/password-upgrade
branch
from
7eb6edd to
bfa7cbe
Compare
msimerson
approved these changes
Apr 12, 2026
Contributor
msimerson
left a comment
msimerson
left a comment
There was a problem hiding this comment.
Sorry, my PR collided with this. Self-merge once you've rebased.
Contributor
|
Also, in the PR description you should be saying 'fixes #30', right? |
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
2 participants
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.
Matt -- this one tackles the password storage situation. Right now the DB has a mix of plain text, SHA-1, and PBKDF2-5000 passwords with no way to tell which is which. The approach here is what I'd call "self-describing" hashes -- the stored format becomes
iterations$hexHash, sovalidPassword()always knows exactly how to verify without guessing.The thing that bugged me about the naive approach (try current iterations, fall back to 5000, fall back to SHA-1...) is that every failed login pays the cost of all those tiers. That's basically a free DoS amplifier. With self-describing hashes, a wrong password costs exactly one PBKDF2 computation no matter how many legacy formats have existed.
The upgrade path is transparent -- when someone logs in successfully with any legacy format, their hash gets silently rewritten to
220000$hexwith a fresh salt. Future iteration bumps just need aPBKDF2_ITERATIONSenv var change, zero code touches, and users upgrade on next login.The interesting bits to look at are in
userBase.js--hashForStorage()wraps the existinghashAuthPbkdf2()to produce the self-describing format, andvalidPassword()now returns{ valid, needsUpgrade }instead of a bare boolean (cleaner than the mutable state approach claude tried first). All the storage sites (create, authenticate, PUT /user) go throughhashForStorage()now.No new dependencies, just
node:crypto.All 345 tests pass in docker. The upgrade tests cover all four paths -- plain text, SHA-1, PBKDF2-5000, and already-current (should not re-hash). Each one does a round-trip to verify login still works after the upgrade.
Let me know if the format choice makes sense or if you'd rather store it differently.
Abe