From 78e95536405ec4aec63c8b124b20c1617018361e Mon Sep 17 00:00:00 2001 From: Scott Alexander Date: Mon, 7 Sep 2026 22:28:41 +0100 Subject: [PATCH] server: hash passwords with PBKDF2 and migrate legacy sha256 hashes 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$$$ 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 Claude-Session: https://claude.ai/code/session_01LmPcXLvZmFC1n9vyASqy4a --- server/src/scrypted-server-main.ts | 25 ++--- server/src/services/users.ts | 151 ++++++++++++++++++++++++++- server/test/password-test.ts | 158 +++++++++++++++++++++++++++++ 3 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 server/test/password-test.ts diff --git a/server/src/scrypted-server-main.ts b/server/src/scrypted-server-main.ts index b31c836ab5..99082a1a04 100644 --- a/server/src/scrypted-server-main.ts +++ b/server/src/scrypted-server-main.ts @@ -24,7 +24,7 @@ import { createClusterServer } from './scrypted-cluster-main'; import { SCRYPTED_DEBUG_PORT, SCRYPTED_INSECURE_PORT, SCRYPTED_SECURE_PORT } from './server-settings'; import { getNpmPackageInfo } from './services/plugin'; import type { ServiceControl } from './services/service-control'; -import { setScryptedUserPassword, UsersService } from './services/users'; +import { checkScryptedUserPassword, checkScryptedUserToken, setScryptedUserPassword, UsersService } from './services/users'; import { sleep } from './sleep'; import { ONE_DAY_MILLISECONDS, UserToken } from './usertoken'; @@ -182,12 +182,13 @@ async function start(mainFilename: string, options?: { return; } - const salted = user.salt + password; - const hash = crypto.createHash('sha256'); - hash.update(salted); - const sha = hash.digest().toString('hex'); - - callback(sha === user.passwordHash || password === user.token); + try { + callback(await checkScryptedUserPassword(db, user, password)); + } + catch (e) { + console.error('basic auth password check failed', e); + callback(false); + } }); // the default http-auth will returns a WWW-Authenticate header if login fails. @@ -287,7 +288,7 @@ async function start(mainFilename: string, options?: { } for (const user of scrypted.usersService.users.values()) { - if (user.token === token) { + if (checkScryptedUserToken(user, token)) { res.locals.username = user._id; res.locals.aclId = user.aclId; break; @@ -610,11 +611,7 @@ async function start(mainFilename: string, options?: { return; } - const salted = user.salt + password; - const hash = crypto.createHash('sha256'); - hash.update(salted); - const sha = hash.digest().toString('hex'); - if (user.passwordHash !== sha && user.token !== password) { + if (!await checkScryptedUserPassword(db, user, password)) { res.send({ error: 'Incorrect password.', hasLogin, @@ -632,7 +629,7 @@ async function start(mainFilename: string, options?: { }); if (change_password) { - setScryptedUserPassword(user, change_password, timestamp); + await setScryptedUserPassword(user, change_password, timestamp); await db.upsert(user); } diff --git a/server/src/services/users.ts b/server/src/services/users.ts index 3800ee1045..312cb579f2 100644 --- a/server/src/services/users.ts +++ b/server/src/services/users.ts @@ -2,6 +2,7 @@ import { ScryptedUser } from "../db-types"; import WrappedLevel from "../level"; import { ScryptedRuntime } from "../runtime"; import crypto from 'crypto'; +import { promisify } from 'util'; export class UsersService { users = new Map(); @@ -11,8 +12,8 @@ export class UsersService { const user = new ScryptedUser(); user._id = username; user.aclId = aclId; - user.token = crypto.randomBytes(16).toString('hex'); - setScryptedUserPassword(user, password, Date.now()); + // setScryptedUserPassword assigns the token. + await setScryptedUserPassword(user, password, Date.now()); await db.upsert(user); return user; } @@ -78,9 +79,149 @@ export class UsersService { } } -export function setScryptedUserPassword(user: ScryptedUser, password: string, timestamp: number) { - user.salt = crypto.randomBytes(64).toString('base64'); - user.passwordHash = crypto.createHash('sha256').update(user.salt + password).digest().toString('hex'); +/** + * Password storage. + * + * Passwords are hashed with PBKDF2-HMAC-SHA256. The iteration count follows the + * OWASP Password Storage Cheat Sheet recommendation for PBKDF2-HMAC-SHA256. + * + * The encoded hash is self describing so the parameters can be raised later + * without invalidating existing passwords: + * + * pbkdf2$sha256$$$ + * + * Passwords created before this format was introduced were stored as a single + * uniterated sha256 of (salt + password), which is fast enough to be brute + * forced offline if the database is ever disclosed. Those hashes are still + * accepted, and are transparently upgraded to PBKDF2 the next time the user + * successfully authenticates. See checkScryptedUserPassword. + */ + +const PBKDF2_PREFIX = 'pbkdf2'; +const PBKDF2_DIGEST = 'sha256'; +const PBKDF2_KEY_LENGTH = 32; +const PBKDF2_SALT_LENGTH = 32; +export const PBKDF2_ITERATIONS = 600000; + +const pbkdf2 = promisify(crypto.pbkdf2); + +function timingSafeEqualString(a: string | undefined, b: string | undefined): boolean { + const ab = Buffer.from(a || '', 'utf8'); + const bb = Buffer.from(b || '', 'utf8'); + // lengths are not secret, and timingSafeEqual requires equal lengths. + if (ab.length !== bb.length) + return false; + return crypto.timingSafeEqual(ab, bb); +} + +interface Pbkdf2Hash { + digest: string; + iterations: number; + salt: Buffer; + hash: Buffer; +} + +function parsePbkdf2Hash(passwordHash: string | undefined): Pbkdf2Hash | undefined { + if (!passwordHash?.startsWith(`${PBKDF2_PREFIX}$`)) + return undefined; + + const parts = passwordHash.split('$'); + if (parts.length !== 5) + return undefined; + + const digest = parts[1]!; + const iterations = parseInt(parts[2]!); + if (!digest || !iterations || iterations < 1) + return undefined; + + try { + return { + digest, + iterations, + salt: Buffer.from(parts[3]!, 'base64'), + hash: Buffer.from(parts[4]!, 'base64'), + }; + } + catch (e) { + return undefined; + } +} + +async function createPbkdf2Hash(password: string, salt: Buffer, iterations: number, keyLength: number, digest: string) { + const hash = await pbkdf2(password, salt, iterations, keyLength, digest); + return `${PBKDF2_PREFIX}$${digest}$${iterations}$${salt.toString('base64')}$${hash.toString('base64')}`; +} + +/** + * Verify a password against the legacy uniterated sha256 format. + */ +function checkLegacyPassword(user: ScryptedUser, password: string): boolean { + if (!user.salt || !user.passwordHash) + return false; + const sha = crypto.createHash('sha256').update(user.salt + password).digest().toString('hex'); + return timingSafeEqualString(sha, user.passwordHash); +} + +/** + * Verify a password against the PBKDF2 format. + */ +async function checkPbkdf2Password(user: ScryptedUser, password: string): Promise { + const parsed = parsePbkdf2Hash(user.passwordHash); + if (!parsed) + return false; + const { digest, iterations, salt, hash } = parsed; + if (!hash.length) + return false; + const actual = await pbkdf2(password, salt, iterations, hash.length, digest); + return crypto.timingSafeEqual(hash, actual); +} + +/** + * Check a user's long lived api token. The token is high entropy, so it does + * not require a slow hash, but it must still be compared in constant time. + */ +export function checkScryptedUserToken(user: ScryptedUser, token: string): boolean { + if (!user.token) + return false; + return timingSafeEqualString(user.token, token); +} + +/** + * Authenticate a user with either their password or their api token. + * + * A password still stored in the legacy sha256 format is rehashed with PBKDF2 + * and persisted on success. The rehash deliberately preserves the existing + * token and passwordDate: the password itself has not changed, and rotating + * the token here would silently invalidate every existing api client. + */ +export async function checkScryptedUserPassword(db: WrappedLevel, user: ScryptedUser, password: string): Promise { + if (!password) + return false; + + if (await checkPbkdf2Password(user, password)) + return true; + + if (checkLegacyPassword(user, password)) { + try { + user.salt = ''; + user.passwordHash = await createPbkdf2Hash(password, crypto.randomBytes(PBKDF2_SALT_LENGTH), PBKDF2_ITERATIONS, PBKDF2_KEY_LENGTH, PBKDF2_DIGEST); + await db.upsert(user); + } + catch (e) { + // the password is valid even if the upgrade could not be persisted. + console.warn('Failed to upgrade password hash to PBKDF2.', e); + } + return true; + } + + return checkScryptedUserToken(user, password); +} + +export async function setScryptedUserPassword(user: ScryptedUser, password: string, timestamp: number) { + // the salt is stored inside passwordHash. it is cleared here so a stale + // legacy salt is never left behind on the user document. + user.salt = ''; + user.passwordHash = await createPbkdf2Hash(password, crypto.randomBytes(PBKDF2_SALT_LENGTH), PBKDF2_ITERATIONS, PBKDF2_KEY_LENGTH, PBKDF2_DIGEST); user.passwordDate = timestamp; user.token = crypto.randomBytes(16).toString('hex'); } diff --git a/server/test/password-test.ts b/server/test/password-test.ts new file mode 100644 index 0000000000..882fac9ee7 --- /dev/null +++ b/server/test/password-test.ts @@ -0,0 +1,158 @@ +import assert from 'assert'; +import crypto from 'crypto'; +import { ScryptedUser } from '../src/db-types'; +import type WrappedLevel from '../src/level'; +import { checkScryptedUserPassword, checkScryptedUserToken, setScryptedUserPassword } from '../src/services/users'; + +function createUpsertSpy() { + const upserted: ScryptedUser[] = []; + const db = { + async upsert(user: ScryptedUser) { + upserted.push(user); + return user; + }, + } as any as WrappedLevel; + return { db, upserted }; +} + +/** + * Create a user in the legacy uniterated sha256 format that predates PBKDF2. + */ +function createLegacyUser(username: string, password: string, timestamp: number) { + const user = new ScryptedUser(); + user._id = username; + user.salt = crypto.randomBytes(64).toString('base64'); + user.passwordHash = crypto.createHash('sha256').update(user.salt + password).digest().toString('hex'); + user.passwordDate = timestamp; + user.token = crypto.randomBytes(16).toString('hex'); + return user; +} + +async function testNewPasswordRoundTrips() { + const { db } = createUpsertSpy(); + const user = new ScryptedUser(); + user._id = 'alice'; + await setScryptedUserPassword(user, 'correct horse battery staple', Date.now()); + + assert(user.passwordHash.startsWith('pbkdf2$sha256$600000$'), 'unexpected hash format: ' + user.passwordHash); + assert.strictEqual(user.passwordHash.split('$').length, 5); + assert.strictEqual(user.salt, '', 'legacy salt field should be cleared'); + + assert.strictEqual(await checkScryptedUserPassword(db, user, 'correct horse battery staple'), true); + assert.strictEqual(await checkScryptedUserPassword(db, user, 'wrong password'), false); + assert.strictEqual(await checkScryptedUserPassword(db, user, ''), false); + console.log('ok: new password round trips'); +} + +async function testSaltIsUniquePerPassword() { + const a = new ScryptedUser(); + const b = new ScryptedUser(); + await setScryptedUserPassword(a, 'same password', Date.now()); + await setScryptedUserPassword(b, 'same password', Date.now()); + assert.notStrictEqual(a.passwordHash, b.passwordHash, 'identical passwords must not produce identical hashes'); + console.log('ok: salt is unique per password'); +} + +async function testLegacyPasswordVerifiesAndUpgrades() { + const { db, upserted } = createUpsertSpy(); + const timestamp = Date.now() - 100000; + const user = createLegacyUser('bob', 'hunter2', timestamp); + const originalToken = user.token; + const originalHash = user.passwordHash; + + assert.strictEqual(await checkScryptedUserPassword(db, user, 'hunter2'), true, 'legacy password should verify'); + + // the hash must have been upgraded in place and persisted exactly once. + assert.strictEqual(upserted.length, 1, 'upgrade should persist the user'); + assert(user.passwordHash.startsWith('pbkdf2$sha256$'), 'hash should be upgraded to pbkdf2'); + assert.notStrictEqual(user.passwordHash, originalHash); + + // an upgrade is not a password change: the token and date must survive, or + // every existing api client would break on the user's next login. + assert.strictEqual(user.token, originalToken, 'upgrade must not rotate the api token'); + assert.strictEqual(user.passwordDate, timestamp, 'upgrade must not change passwordDate'); + + // the upgraded hash still verifies, and does not re-upgrade. + assert.strictEqual(await checkScryptedUserPassword(db, user, 'hunter2'), true); + assert.strictEqual(upserted.length, 1, 'already upgraded user should not be persisted again'); + console.log('ok: legacy password verifies and upgrades in place'); +} + +async function testLegacyWrongPasswordDoesNotUpgrade() { + const { db, upserted } = createUpsertSpy(); + const user = createLegacyUser('carol', 'hunter2', Date.now()); + const originalHash = user.passwordHash; + + assert.strictEqual(await checkScryptedUserPassword(db, user, 'not the password'), false); + assert.strictEqual(user.passwordHash, originalHash, 'failed login must not modify the hash'); + assert.strictEqual(upserted.length, 0, 'failed login must not persist anything'); + console.log('ok: failed legacy login does not upgrade'); +} + +async function testTokenAuthentication() { + const { db } = createUpsertSpy(); + const user = new ScryptedUser(); + await setScryptedUserPassword(user, 'a password', Date.now()); + + // the token is accepted in place of the password, as before. + assert.strictEqual(await checkScryptedUserPassword(db, user, user.token), true); + assert.strictEqual(checkScryptedUserToken(user, user.token), true); + assert.strictEqual(checkScryptedUserToken(user, 'wrong token'), false); + assert.strictEqual(checkScryptedUserToken(user, ''), false); + + // a user with no token must never authenticate on an empty token. + const tokenless = new ScryptedUser(); + assert.strictEqual(checkScryptedUserToken(tokenless, ''), false); + assert.strictEqual(checkScryptedUserToken(tokenless, undefined as any), false); + console.log('ok: token authentication'); +} + +async function testMalformedHashesAreRejected() { + const { db } = createUpsertSpy(); + const malformed = [ + '', + 'pbkdf2', + 'pbkdf2$sha256', + 'pbkdf2$sha256$0$c2FsdA==$aGFzaA==', + 'pbkdf2$sha256$notanumber$c2FsdA==$aGFzaA==', + 'pbkdf2$sha256$600000$c2FsdA==$', + 'pbkdf2$$600000$c2FsdA==$aGFzaA==', + ]; + for (const passwordHash of malformed) { + const user = new ScryptedUser(); + user.passwordHash = passwordHash; + assert.strictEqual(await checkScryptedUserPassword(db, user, 'anything'), false, 'accepted malformed hash: ' + passwordHash); + } + + // a user with no credentials at all must never authenticate. + const empty = new ScryptedUser(); + assert.strictEqual(await checkScryptedUserPassword(db, empty, 'anything'), false); + assert.strictEqual(await checkScryptedUserPassword(db, empty, ''), false); + console.log('ok: malformed hashes are rejected'); +} + +async function testUpgradeFailureStillAuthenticates() { + // if the database write fails, the user must still be able to log in. + const db = { + async upsert() { + throw new Error('disk full'); + }, + } as any as WrappedLevel; + const user = createLegacyUser('dave', 'hunter2', Date.now()); + assert.strictEqual(await checkScryptedUserPassword(db, user, 'hunter2'), true); + console.log('ok: upgrade failure still authenticates'); +} + +async function test() { + await testNewPasswordRoundTrips(); + await testSaltIsUniquePerPassword(); + await testLegacyPasswordVerifiesAndUpgrades(); + await testLegacyWrongPasswordDoesNotUpgrade(); + await testTokenAuthentication(); + await testMalformedHashesAreRejected(); + await testUpgradeFailureStillAuthenticates(); + console.log(); + console.log('all password tests passed'); +} + +test();