feat: self-describing password hashes (iterations$hex)

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

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

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

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()
+    })
+  })
 })

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)