server: hash passwords with PBKDF2 and migrate legacy sha256 hashes - #2140
Open
Gooshy wants to merge 1 commit into
Open
server: hash passwords with PBKDF2 and migrate legacy sha256 hashes#2140Gooshy wants to merge 1 commit into
Gooshy wants to merge 1 commit into
Conversation
Passwords were stored as a single uniterated sha256 of (salt + password).
The per user salt rules out rainbow tables and cross user amortization, but
there is no work factor, so a disclosed database can be attacked offline at
GPU speed. On a single current GPU an 8 character mixed alphanumeric password
falls in about 3 hours, and a wordlist run against a human chosen password
finishes far sooner.
Hash with PBKDF2-HMAC-SHA256 at 600,000 iterations, the OWASP Password Storage
Cheat Sheet recommendation for this construction. The encoded hash is self
describing:
pbkdf2$sha256$<iterations>$<salt-base64>$<hash-base64>
so the digest and iteration count can be raised later without invalidating
existing passwords.
Legacy hashes remain valid and are transparently upgraded the next time the
user authenticates successfully, since rehashing requires the plaintext. The
upgrade deliberately preserves the existing token and passwordDate: the
password itself has not changed, and rotating the token there would silently
invalidate every existing api client. A failed upgrade write is logged but
still authenticates the user rather than locking them out.
Password and token comparisons now use crypto.timingSafeEqual. The token is
128 bits of randomness so it does not need a slow hash, but it is a bearer
credential and should not be compared with ===.
Adds server/test/password-test.ts covering round trip, per password salt
uniqueness, legacy verification and in place upgrade, token preservation
across upgrade, no upgrade on failed login, token authentication, malformed
hash rejection, and authentication surviving a failed upgrade write.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LmPcXLvZmFC1n9vyASqy4a
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
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.
Problem
setScryptedUserPasswordstores passwords as a single uniterated sha256:The per-user 64-byte salt is good — it rules out rainbow tables and stops one crack being amortized across users. The problem is that there's no work factor. sha256 is the primitive GPUs are built to accelerate, so a disclosed database can be attacked offline at full GPU speed.
Full-keyspace exhaustion on a single RTX 4090 (~20 GH/s, hashcat
-m 1400):A wordlist-plus-rules run against a human-chosen password finishes far sooner. The same 8-char mixed alphanumeric under a proper KDF takes millennia.
The realistic harm is password reuse: anyone who obtains the database (the volume directory, or a
/web/component/backupzip, whichservices/backup.tswrites as a plain unencrypted copy of the whole datastore) recovers the plaintext and tries it against the user's other accounts.Change
Hash with PBKDF2-HMAC-SHA256 at 600,000 iterations — the OWASP Password Storage Cheat Sheet recommendation for this construction, and available in the Node standard library so this adds no dependency. Measured at ~70ms per verification on an M-series laptop.
The encoded hash is self-describing, so the digest and iteration count can be raised later without invalidating existing passwords:
Migration
Rehashing requires the plaintext, so it can't be done as a batch job. Legacy hashes stay valid and are transparently upgraded the next time the user authenticates successfully, at both verification sites (
POST /loginand the http basic auth checker).Two details worth calling out in review:
tokenandpasswordDate. It deliberately does not callsetScryptedUserPassword, which regeneratesuser.token. An upgrade is not a password change, and rotating the token there would silently invalidate every existing API client on the user's next login.db.upsertthrows, the password was still correct; the error is logged and login proceeds rather than locking the user out.Also
Password and token comparisons now use
crypto.timingSafeEqual. The token is 128 bits of randomness so it doesn't need a slow hash, but it is a bearer credential accepted asAuthorization: Bearerand in place of the password at/login, so it shouldn't be compared with===.Compatibility
setScryptedUserPasswordis nowasync; both call sites are updated.user.tokenstill works as a password on old code, and thereset-loginvolume file remains as a recovery path.Tests
Adds
server/test/password-test.ts, following the existing standalone-script style inserver/test:tsc --noEmitis clean acrossserver/srcand the new test.🤖 Generated with Claude Code
https://claude.ai/code/session_01LmPcXLvZmFC1n9vyASqy4a