diff --git a/lib/user/store/base.js b/lib/user/store/base.js index a9989e4..aee57bd 100644 --- a/lib/user/store/base.js +++ b/lib/user/store/base.js @@ -1,5 +1,18 @@ import crypto from 'node:crypto' +const PBKDF2_ITERATIONS = (() => { + const raw = process.env.PBKDF2_ITERATIONS + if (raw === undefined) return 220000 + const parsed = parseInt(raw, 10) + if (Number.isNaN(parsed) || parsed < 1) { + console.warn(`PBKDF2_ITERATIONS="${raw}" is not a valid positive integer, using 220000`) + return 220000 + } + return parsed +})() +const LEGACY_ITERATIONS = 5000 +const SELF_DESCRIBING_RE = /^(\d+)\$([0-9a-f]{64})$/ + /** * User domain class – pure attributes and password business logic. * @@ -60,33 +73,58 @@ class UserBase { return salt } - async hashAuthPbkdf2(pass, salt) { + async hashAuthPbkdf2(pass, salt, iterations = PBKDF2_ITERATIONS) { return new Promise((resolve, reject) => { - // match the defaults for NicTool 2.x - crypto.pbkdf2(pass, salt, 5000, 32, 'sha512', (err, derivedKey) => { + crypto.pbkdf2(pass, salt, iterations, 32, 'sha512', (err, derivedKey) => { if (err) return reject(err) resolve(derivedKey.toString('hex')) }) }) } + async hashForStorage(pass, salt, iterations = PBKDF2_ITERATIONS) { + const hex = await this.hashAuthPbkdf2(pass, salt, iterations) + return `${iterations}$${hex}` + } + async validPassword(passTry, passDb, username, salt) { - if (!salt && passTry === passDb) return true // plain pass, TODO, encrypt! + if (!salt && passTry === passDb) { + return { valid: true, needsUpgrade: true } + } if (salt) { - const hashed = await this.hashAuthPbkdf2(passTry, salt) - if (this.debug) console.log(`hashed: (${hashed === passDb}) ${hashed}`) - return hashed === passDb + // Self-describing format: "iterations$hexHash" — single hash, no fallback + const m = SELF_DESCRIBING_RE.exec(passDb) + if (m) { + const storedIters = parseInt(m[1], 10) + const storedHashHex = m[2] + const hashed = await this.hashAuthPbkdf2(passTry, salt, storedIters) + if (this.debug) console.log(`self-describing: (${hashed === storedHashHex}) ${hashed}`) + if (hashed === storedHashHex) { + return { valid: true, needsUpgrade: storedIters < PBKDF2_ITERATIONS } + } + return { valid: false, needsUpgrade: false } + } + + // Raw hex (legacy NicTool 2 format, implicitly 5000 iterations) + const legacy = await this.hashAuthPbkdf2(passTry, salt, LEGACY_ITERATIONS) + if (this.debug) console.log(`legacy: (${legacy === passDb}) ${legacy}`) + if (legacy === passDb) { + return { valid: true, needsUpgrade: true } + } + return { valid: false, needsUpgrade: false } } // Check for HMAC SHA-1 password if (/^[0-9a-f]{40}$/.test(passDb)) { const digest = crypto.createHmac('sha1', username.toLowerCase()).update(passTry).digest('hex') if (this.debug) console.log(`digest: (${digest === passDb}) ${digest}`) - return digest === passDb + if (digest === passDb) { + return { valid: true, needsUpgrade: true } + } } - return false + return { valid: false, needsUpgrade: false } } } diff --git a/lib/user/store/mysql.js b/lib/user/store/mysql.js index 8fb243f..445b851 100644 --- a/lib/user/store/mysql.js +++ b/lib/user/store/mysql.js @@ -36,7 +36,18 @@ class UserRepoMySQL extends UserBase { AND g.name = ?` for (const u of await Mysql.execute(query, [username, groupName])) { - if (await this.validPassword(authTry.password, u.password, authTry.username, u.pass_salt)) { + const { valid, needsUpgrade } = await this.validPassword(authTry.password, u.password, authTry.username, u.pass_salt) + if (valid) { + if (needsUpgrade) { + const salt = this.generateSalt() + const hash = await this.hashForStorage(authTry.password, salt) + await Mysql.execute( + ...Mysql.update('nt_user', `nt_user_id=${u.id}`, { + password: hash, + pass_salt: salt, + }), + ) + } for (const f of ['password', 'pass_salt']) { delete u[f] // SECURITY: no longer needed } @@ -65,7 +76,7 @@ class UserRepoMySQL extends UserBase { if (args.password) { if (!args.pass_salt) args.pass_salt = this.generateSalt() - args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt) + args.password = await this.hashForStorage(args.password, args.pass_salt) } const userId = await Mysql.execute(...Mysql.insert(`nt_user`, mapToDbColumn(args, userDbMap))) diff --git a/lib/user/store/toml.js b/lib/user/store/toml.js index 01a2832..d724b00 100644 --- a/lib/user/store/toml.js +++ b/lib/user/store/toml.js @@ -86,7 +86,7 @@ class UserRepoTOML extends UserBase { args = JSON.parse(JSON.stringify(args)) if (args.password) { if (!args.pass_salt) args.pass_salt = this.generateSalt() - args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt) + args.password = await this.hashForStorage(args.password, args.pass_salt) } const users = await this._load() diff --git a/lib/user/test.js b/lib/user/test.js index 2adb1ec..8faca45 100644 --- a/lib/user/test.js +++ b/lib/user/test.js @@ -1,8 +1,10 @@ import assert from 'node:assert/strict' +import crypto from 'node:crypto' import { describe, it, after, before } from 'node:test' import User from './index.js' import Group from '../group/index.js' +import Mysql from '../mysql.js' import userCase from '../test/user.json' with { type: 'json' } import groupCase from '../test/group.json' with { type: 'json' } @@ -77,27 +79,35 @@ describe('user', function () { describe('validPassword', function () { it('auths user with plain text password', async () => { const r = await User.validPassword('test', 'test', 'demo', '') - assert.equal(r, true) + assert.deepEqual(r, { valid: true, needsUpgrade: true }) }) - it('auths valid pbkdb2 password', async () => { - const r = await User.validPassword( - 'YouGuessedIt!', - '050cfa70c3582be0d5bfae25138a8486dc2e6790f39bc0c4e111223ba6034432', - 'unit-test', - '(ICzAm2.QfCa6.MN', - ) - assert.equal(r, true) + it('auths valid self-describing PBKDF2 password', async () => { + const salt = '(ICzAm2.QfCa6.MN' + const hash = await User.hashForStorage('YouGuessedIt!', salt) + const r = await User.validPassword('YouGuessedIt!', hash, 'unit-test', salt) + assert.deepEqual(r, { valid: true, needsUpgrade: false }) }) - it('rejects invalid pbkdb2 password', async () => { - const r = await User.validPassword( - 'YouMissedIt!', - '050cfa70c3582be0d5bfae25138a8486dc2e6790f39bc0c4e111223ba6034432', - 'unit-test', - '(ICzAm2.QfCa6.MN', - ) - assert.equal(r, false) + it('rejects invalid self-describing PBKDF2 password', async () => { + const salt = '(ICzAm2.QfCa6.MN' + const hash = await User.hashForStorage('YouGuessedIt!', salt) + const r = await User.validPassword('YouMissedIt!', hash, 'unit-test', salt) + assert.deepEqual(r, { valid: false, needsUpgrade: false }) + }) + + it('auths valid legacy PBKDF2-5000 password', async () => { + const salt = '(ICzAm2.QfCa6.MN' + const hash = await User.hashAuthPbkdf2('YouGuessedIt!', salt, 5000) + const r = await User.validPassword('YouGuessedIt!', hash, 'unit-test', salt) + assert.deepEqual(r, { valid: true, needsUpgrade: true }) + }) + + it('rejects invalid legacy PBKDF2-5000 password', async () => { + const salt = '(ICzAm2.QfCa6.MN' + const hash = await User.hashAuthPbkdf2('YouGuessedIt!', salt, 5000) + const r = await User.validPassword('YouMissedIt!', hash, 'unit-test', salt) + assert.deepEqual(r, { valid: false, needsUpgrade: false }) }) it('auths valid SHA1 password', async () => { @@ -106,7 +116,7 @@ describe('user', function () { '083007777a5241d01abba70c938c60d80be60027', 'unit-test', ) - assert.equal(r, true) + assert.deepEqual(r, { valid: true, needsUpgrade: true }) }) it('rejects invalid SHA1 password', async () => { @@ -115,7 +125,7 @@ describe('user', function () { '083007777a5241d01abba7Oc938c60d80be60027', 'unit-test', ) - assert.equal(r, false) + assert.deepEqual(r, { valid: false, needsUpgrade: false }) }) }) @@ -144,4 +154,122 @@ describe('user', function () { assert.ok(u) }) }) + + describe('password upgrade on login', () => { + const upgradeUserId = 4200 + const upgradeUser = { + nt_user_id: upgradeUserId, + nt_group_id: groupCase.id, + username: 'upgrade-test', + email: 'upgrade-test@example.com', + first_name: 'Upgrade', + last_name: 'Test', + } + const testPass = 'UpgradeMe!123' + const authCreds = { + username: `${upgradeUser.username}@${groupCase.name}`, + password: testPass, + } + + async function getDbPassword() { + const rows = await Mysql.execute( + 'SELECT password, pass_salt FROM nt_user WHERE nt_user_id = ?', + [upgradeUserId], + ) + return rows[0] + } + + // Raw SQL so we can insert specific legacy password formats (plain text, + // SHA-1, PBKDF2-5000) that User.create() would hash on the way in. + async function insertUser(password, passSalt) { + await Mysql.execute( + 'INSERT INTO nt_user (nt_user_id, nt_group_id, username, email, first_name, last_name, password, pass_salt) VALUES (?, ?, ?, ?, ?, ?, ?, ?)', + [upgradeUserId, upgradeUser.nt_group_id, upgradeUser.username, upgradeUser.email, upgradeUser.first_name, upgradeUser.last_name, password, passSalt], + ) + } + + async function cleanup() { + await Mysql.execute( + 'DELETE FROM nt_user WHERE nt_user_id = ?', + [upgradeUserId], + ) + } + + before(cleanup) + after(cleanup) + + it('upgrades plain text password to self-describing PBKDF2 on login', async () => { + await cleanup() + await insertUser(testPass, null) + + const result = await User.authenticate(authCreds) + assert.ok(result, 'login should succeed') + + const row = await getDbPassword() + assert.ok(row.pass_salt, 'pass_salt should be set after upgrade') + assert.notEqual(row.password, testPass, 'password should be hashed') + assert.ok(row.password.includes('$'), 'password should be in self-describing format') + + // verify round-trip: can still log in with the upgraded hash + const again = await User.authenticate(authCreds) + assert.ok(again, 'login should succeed after upgrade') + await cleanup() + }) + + it('upgrades SHA1 password to self-describing PBKDF2 on login', async () => { + // authenticate() passes the full authTry.username (including @group) to + // validPassword(), so the HMAC key must match that full string + const sha1Hash = crypto + .createHmac('sha1', authCreds.username.toLowerCase()) + .update(testPass) + .digest('hex') + await cleanup() + await insertUser(sha1Hash, null) + + const result = await User.authenticate(authCreds) + assert.ok(result, 'login should succeed with SHA1 hash') + + const row = await getDbPassword() + assert.ok(row.pass_salt, 'pass_salt should be set after upgrade') + assert.notEqual(row.password, sha1Hash, 'password should be re-hashed') + assert.ok(row.password.includes('$'), 'password should be in self-describing format') + + const again = await User.authenticate(authCreds) + assert.ok(again, 'login should succeed after upgrade') + await cleanup() + }) + + it('upgrades PBKDF2-5000 to self-describing format on login', async () => { + const legacySalt = User.generateSalt() + const legacyHash = await User.hashAuthPbkdf2(testPass, legacySalt, 5000) + await cleanup() + await insertUser(legacyHash, legacySalt) + + const result = await User.authenticate(authCreds) + assert.ok(result, 'login should succeed with legacy PBKDF2') + + const row = await getDbPassword() + assert.notEqual(row.password, legacyHash, 'password should be re-hashed') + assert.notEqual(row.pass_salt, legacySalt, 'salt should be regenerated') + assert.ok(row.password.includes('$'), 'password should be in self-describing format') + + const again = await User.authenticate(authCreds) + assert.ok(again, 'login should succeed after upgrade') + await cleanup() + }) + + it('does not re-hash password already in self-describing format', async () => { + const salt = User.generateSalt() + const hash = await User.hashForStorage(testPass, salt) + await cleanup() + await insertUser(hash, salt) + + await User.authenticate(authCreds) + + const row = await getDbPassword() + assert.equal(row.password, hash, 'password should be unchanged') + assert.equal(row.pass_salt, salt, 'salt should be unchanged') + await cleanup() + }) + }) }) diff --git a/routes/user.js b/routes/user.js index 04311ea..df27081 100644 --- a/routes/user.js +++ b/routes/user.js @@ -136,7 +136,7 @@ function UserRoutes(server) { if (args.password) { args.pass_salt = User.generateSalt() - args.password = await User.hashAuthPbkdf2(args.password, args.pass_salt) + args.password = await User.hashForStorage(args.password, args.pass_salt) } await User.put(args)