server: hash passwords with PBKDF2 and migrate legacy sha256 hashes

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);
             }
 

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<string, ScryptedUser>();
@@ -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$<iterations>$<salt-base64>$<hash-base64>
+ *
+ * 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<boolean> {
+    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<boolean> {
+    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');
 }

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();