Skip to content

Commit 3d4ef53

Browse files
aberohamclaude
andcommitted
feat: self-describing password hashes (iterations$hex)
Store PBKDF2 iteration count alongside the hash to eliminate the linear fallback chain that tried current then legacy iterations on every login attempt. A wrong password now costs exactly one PBKDF2 computation regardless of how many iteration baselines have existed. Legacy formats (plain text, SHA-1, raw PBKDF2-5000) are detected and silently upgraded to self-describing format on login. Future iteration bumps (via PBKDF2_ITERATIONS env var) require zero code changes — users at older iterations upgrade on next login. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1bcf896 commit 3d4ef53

5 files changed

Lines changed: 194 additions & 25 deletions

File tree

lib/user/mysql.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ class UserRepoMySQL extends UserBase {
3737

3838
for (const u of await Mysql.execute(query, [username, groupName])) {
3939
if (await this.validPassword(authTry.password, u.password, authTry.username, u.pass_salt)) {
40+
if (this._needsUpgrade) {
41+
const salt = this.generateSalt()
42+
const hash = await this.hashForStorage(authTry.password, salt)
43+
await Mysql.execute(
44+
...Mysql.update('nt_user', `nt_user_id=${u.id}`, {
45+
password: hash,
46+
pass_salt: salt,
47+
}),
48+
)
49+
}
4050
for (const f of ['password', 'pass_salt']) {
4151
delete u[f] // SECURITY: no longer needed
4252
}
@@ -65,7 +75,7 @@ class UserRepoMySQL extends UserBase {
6575

6676
if (args.password) {
6777
if (!args.pass_salt) args.pass_salt = this.generateSalt()
68-
args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt)
78+
args.password = await this.hashForStorage(args.password, args.pass_salt)
6979
}
7080

7181
const userId = await Mysql.execute(...Mysql.insert(`nt_user`, mapToDbColumn(args, userDbMap)))

lib/user/test.js

Lines changed: 140 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import assert from 'node:assert/strict'
2+
import crypto from 'node:crypto'
23
import { describe, it, after, before } from 'node:test'
34

45
import User from './index.js'
56
import Group from '../group/index.js'
7+
import Mysql from '../mysql.js'
68

79
import userCase from '../test/user.json' with { type: 'json' }
810
import groupCase from '../test/group.json' with { type: 'json' }
@@ -80,23 +82,31 @@ describe('user', function () {
8082
assert.equal(r, true)
8183
})
8284

83-
it('auths valid pbkdb2 password', async () => {
84-
const r = await User.validPassword(
85-
'YouGuessedIt!',
86-
'050cfa70c3582be0d5bfae25138a8486dc2e6790f39bc0c4e111223ba6034432',
87-
'unit-test',
88-
'(ICzAm2.QfCa6.MN',
89-
)
85+
it('auths valid self-describing PBKDF2 password', async () => {
86+
const salt = '(ICzAm2.QfCa6.MN'
87+
const hash = await User.hashForStorage('YouGuessedIt!', salt)
88+
const r = await User.validPassword('YouGuessedIt!', hash, 'unit-test', salt)
9089
assert.equal(r, true)
9190
})
9291

93-
it('rejects invalid pbkdb2 password', async () => {
94-
const r = await User.validPassword(
95-
'YouMissedIt!',
96-
'050cfa70c3582be0d5bfae25138a8486dc2e6790f39bc0c4e111223ba6034432',
97-
'unit-test',
98-
'(ICzAm2.QfCa6.MN',
99-
)
92+
it('rejects invalid self-describing PBKDF2 password', async () => {
93+
const salt = '(ICzAm2.QfCa6.MN'
94+
const hash = await User.hashForStorage('YouGuessedIt!', salt)
95+
const r = await User.validPassword('YouMissedIt!', hash, 'unit-test', salt)
96+
assert.equal(r, false)
97+
})
98+
99+
it('auths valid legacy PBKDF2-5000 password', async () => {
100+
const salt = '(ICzAm2.QfCa6.MN'
101+
const hash = await User.hashAuthPbkdf2('YouGuessedIt!', salt, 5000)
102+
const r = await User.validPassword('YouGuessedIt!', hash, 'unit-test', salt)
103+
assert.equal(r, true)
104+
})
105+
106+
it('rejects invalid legacy PBKDF2-5000 password', async () => {
107+
const salt = '(ICzAm2.QfCa6.MN'
108+
const hash = await User.hashAuthPbkdf2('YouGuessedIt!', salt, 5000)
109+
const r = await User.validPassword('YouMissedIt!', hash, 'unit-test', salt)
100110
assert.equal(r, false)
101111
})
102112

@@ -144,4 +154,120 @@ describe('user', function () {
144154
assert.ok(u)
145155
})
146156
})
157+
158+
describe('password upgrade on login', () => {
159+
const upgradeUserId = 4200
160+
const upgradeUser = {
161+
nt_user_id: upgradeUserId,
162+
nt_group_id: groupCase.id,
163+
username: 'upgrade-test',
164+
email: 'upgrade-test@example.com',
165+
first_name: 'Upgrade',
166+
last_name: 'Test',
167+
}
168+
const testPass = 'UpgradeMe!123'
169+
const authCreds = {
170+
username: `${upgradeUser.username}@${groupCase.name}`,
171+
password: testPass,
172+
}
173+
174+
async function getDbPassword() {
175+
const rows = await Mysql.execute(
176+
'SELECT password, pass_salt FROM nt_user WHERE nt_user_id = ?',
177+
[upgradeUserId],
178+
)
179+
return rows[0]
180+
}
181+
182+
async function insertUser(password, passSalt) {
183+
await Mysql.execute(
184+
'INSERT INTO nt_user (nt_user_id, nt_group_id, username, email, first_name, last_name, password, pass_salt) VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
185+
[upgradeUserId, upgradeUser.nt_group_id, upgradeUser.username, upgradeUser.email, upgradeUser.first_name, upgradeUser.last_name, password, passSalt],
186+
)
187+
}
188+
189+
async function cleanup() {
190+
await Mysql.execute(
191+
'DELETE FROM nt_user WHERE nt_user_id = ?',
192+
[upgradeUserId],
193+
)
194+
}
195+
196+
before(cleanup)
197+
after(cleanup)
198+
199+
it('upgrades plain text password to self-describing PBKDF2 on login', async () => {
200+
await cleanup()
201+
await insertUser(testPass, null)
202+
203+
const result = await User.authenticate(authCreds)
204+
assert.ok(result, 'login should succeed')
205+
206+
const row = await getDbPassword()
207+
assert.ok(row.pass_salt, 'pass_salt should be set after upgrade')
208+
assert.notEqual(row.password, testPass, 'password should be hashed')
209+
assert.ok(row.password.includes('$'), 'password should be in self-describing format')
210+
211+
// verify round-trip: can still log in with the upgraded hash
212+
const again = await User.authenticate(authCreds)
213+
assert.ok(again, 'login should succeed after upgrade')
214+
await cleanup()
215+
})
216+
217+
it('upgrades SHA1 password to self-describing PBKDF2 on login', async () => {
218+
// authenticate() passes the full authTry.username (including @group) to
219+
// validPassword(), so the HMAC key must match that full string
220+
const sha1Hash = crypto
221+
.createHmac('sha1', authCreds.username.toLowerCase())
222+
.update(testPass)
223+
.digest('hex')
224+
await cleanup()
225+
await insertUser(sha1Hash, null)
226+
227+
const result = await User.authenticate(authCreds)
228+
assert.ok(result, 'login should succeed with SHA1 hash')
229+
230+
const row = await getDbPassword()
231+
assert.ok(row.pass_salt, 'pass_salt should be set after upgrade')
232+
assert.notEqual(row.password, sha1Hash, 'password should be re-hashed')
233+
assert.ok(row.password.includes('$'), 'password should be in self-describing format')
234+
235+
const again = await User.authenticate(authCreds)
236+
assert.ok(again, 'login should succeed after upgrade')
237+
await cleanup()
238+
})
239+
240+
it('upgrades PBKDF2-5000 to self-describing format on login', async () => {
241+
const legacySalt = User.generateSalt()
242+
const legacyHash = await User.hashAuthPbkdf2(testPass, legacySalt, 5000)
243+
await cleanup()
244+
await insertUser(legacyHash, legacySalt)
245+
246+
const result = await User.authenticate(authCreds)
247+
assert.ok(result, 'login should succeed with legacy PBKDF2')
248+
249+
const row = await getDbPassword()
250+
assert.notEqual(row.password, legacyHash, 'password should be re-hashed')
251+
assert.notEqual(row.pass_salt, legacySalt, 'salt should be regenerated')
252+
assert.ok(row.password.includes('$'), 'password should be in self-describing format')
253+
254+
const again = await User.authenticate(authCreds)
255+
assert.ok(again, 'login should succeed after upgrade')
256+
await cleanup()
257+
})
258+
259+
it('does not re-hash password already in self-describing format', async () => {
260+
const salt = User.generateSalt()
261+
const hash = await User.hashForStorage(testPass, salt)
262+
await cleanup()
263+
await insertUser(hash, salt)
264+
265+
await User.authenticate(authCreds)
266+
267+
const row = await getDbPassword()
268+
assert.equal(row.password, hash, 'password should be unchanged')
269+
assert.equal(row.pass_salt, salt, 'salt should be unchanged')
270+
await cleanup()
271+
})
272+
})
147273
})

lib/user/toml.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class UserRepoTOML extends UserBase {
8686
args = JSON.parse(JSON.stringify(args))
8787
if (args.password) {
8888
if (!args.pass_salt) args.pass_salt = this.generateSalt()
89-
args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt)
89+
args.password = await this.hashForStorage(args.password, args.pass_salt)
9090
}
9191

9292
const users = await this._load()

lib/user/userBase.js

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import crypto from 'node:crypto'
22

3+
const PBKDF2_ITERATIONS = parseInt(process.env.PBKDF2_ITERATIONS) || 220000
4+
const LEGACY_ITERATIONS = 5000
5+
36
/**
47
* User domain class – pure attributes and password business logic.
58
*
@@ -60,30 +63,60 @@ class UserBase {
6063
return salt
6164
}
6265

63-
async hashAuthPbkdf2(pass, salt) {
66+
async hashAuthPbkdf2(pass, salt, iterations = PBKDF2_ITERATIONS) {
6467
return new Promise((resolve, reject) => {
65-
// match the defaults for NicTool 2.x
66-
crypto.pbkdf2(pass, salt, 5000, 32, 'sha512', (err, derivedKey) => {
68+
crypto.pbkdf2(pass, salt, iterations, 32, 'sha512', (err, derivedKey) => {
6769
if (err) return reject(err)
6870
resolve(derivedKey.toString('hex'))
6971
})
7072
})
7173
}
7274

75+
async hashForStorage(pass, salt, iterations = PBKDF2_ITERATIONS) {
76+
const hex = await this.hashAuthPbkdf2(pass, salt, iterations)
77+
return `${iterations}$${hex}`
78+
}
79+
7380
async validPassword(passTry, passDb, username, salt) {
74-
if (!salt && passTry === passDb) return true // plain pass, TODO, encrypt!
81+
this._needsUpgrade = false
82+
83+
if (!salt && passTry === passDb) {
84+
this._needsUpgrade = true
85+
return true
86+
}
7587

7688
if (salt) {
77-
const hashed = await this.hashAuthPbkdf2(passTry, salt)
78-
if (this.debug) console.log(`hashed: (${hashed === passDb}) ${hashed}`)
79-
return hashed === passDb
89+
// Self-describing format: "iterations$hexHash" — single hash, no fallback
90+
if (passDb.includes('$')) {
91+
const [storedItersStr, storedHashHex] = passDb.split('$')
92+
const storedIters = parseInt(storedItersStr, 10)
93+
const hashed = await this.hashAuthPbkdf2(passTry, salt, storedIters)
94+
if (this.debug) console.log(`self-describing: (${hashed === storedHashHex}) ${hashed}`)
95+
if (hashed === storedHashHex) {
96+
if (storedIters < PBKDF2_ITERATIONS) this._needsUpgrade = true
97+
return true
98+
}
99+
return false
100+
}
101+
102+
// Raw hex (legacy NicTool 2 format, implicitly 5000 iterations)
103+
const legacy = await this.hashAuthPbkdf2(passTry, salt, LEGACY_ITERATIONS)
104+
if (this.debug) console.log(`legacy: (${legacy === passDb}) ${legacy}`)
105+
if (legacy === passDb) {
106+
this._needsUpgrade = true
107+
return true
108+
}
109+
return false
80110
}
81111

82112
// Check for HMAC SHA-1 password
83113
if (/^[0-9a-f]{40}$/.test(passDb)) {
84114
const digest = crypto.createHmac('sha1', username.toLowerCase()).update(passTry).digest('hex')
85115
if (this.debug) console.log(`digest: (${digest === passDb}) ${digest}`)
86-
return digest === passDb
116+
if (digest === passDb) {
117+
this._needsUpgrade = true
118+
return true
119+
}
87120
}
88121

89122
return false

routes/user.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ function UserRoutes(server) {
136136

137137
if (args.password) {
138138
args.pass_salt = User.generateSalt()
139-
args.password = await User.hashAuthPbkdf2(args.password, args.pass_salt)
139+
args.password = await User.hashForStorage(args.password, args.pass_salt)
140140
}
141141

142142
await User.put(args)

0 commit comments

Comments
 (0)