Skip to content

Commit bfa7cbe

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. Format: `iterations$hexHash` (validated by /^\d+\$[0-9a-f]{64}$/). Legacy formats (plain text, SHA-1, raw PBKDF2-5000) are detected and silently upgraded to self-describing format on login. validPassword() returns { valid, needsUpgrade } instead of setting mutable instance state — callers destructure the result directly. PBKDF2_ITERATIONS env var (default 220000) is validated at module load. Future iteration bumps 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 eec83cc commit bfa7cbe

5 files changed

Lines changed: 209 additions & 32 deletions

File tree

lib/user/store/base.js

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

3+
const PBKDF2_ITERATIONS = (() => {
4+
const raw = process.env.PBKDF2_ITERATIONS
5+
if (raw === undefined) return 220000
6+
const parsed = parseInt(raw, 10)
7+
if (Number.isNaN(parsed) || parsed < 1) {
8+
console.warn(`PBKDF2_ITERATIONS="${raw}" is not a valid positive integer, using 220000`)
9+
return 220000
10+
}
11+
return parsed
12+
})()
13+
const LEGACY_ITERATIONS = 5000
14+
const SELF_DESCRIBING_RE = /^(\d+)\$([0-9a-f]{64})$/
15+
316
/**
417
* User domain class – pure attributes and password business logic.
518
*
@@ -60,33 +73,58 @@ class UserBase {
6073
return salt
6174
}
6275

63-
async hashAuthPbkdf2(pass, salt) {
76+
async hashAuthPbkdf2(pass, salt, iterations = PBKDF2_ITERATIONS) {
6477
return new Promise((resolve, reject) => {
65-
// match the defaults for NicTool 2.x
66-
crypto.pbkdf2(pass, salt, 5000, 32, 'sha512', (err, derivedKey) => {
78+
crypto.pbkdf2(pass, salt, iterations, 32, 'sha512', (err, derivedKey) => {
6779
if (err) return reject(err)
6880
resolve(derivedKey.toString('hex'))
6981
})
7082
})
7183
}
7284

85+
async hashForStorage(pass, salt, iterations = PBKDF2_ITERATIONS) {
86+
const hex = await this.hashAuthPbkdf2(pass, salt, iterations)
87+
return `${iterations}$${hex}`
88+
}
89+
7390
async validPassword(passTry, passDb, username, salt) {
74-
if (!salt && passTry === passDb) return true // plain pass, TODO, encrypt!
91+
if (!salt && passTry === passDb) {
92+
return { valid: true, needsUpgrade: true }
93+
}
7594

7695
if (salt) {
77-
const hashed = await this.hashAuthPbkdf2(passTry, salt)
78-
if (this.debug) console.log(`hashed: (${hashed === passDb}) ${hashed}`)
79-
return hashed === passDb
96+
// Self-describing format: "iterations$hexHash" — single hash, no fallback
97+
const m = SELF_DESCRIBING_RE.exec(passDb)
98+
if (m) {
99+
const storedIters = parseInt(m[1], 10)
100+
const storedHashHex = m[2]
101+
const hashed = await this.hashAuthPbkdf2(passTry, salt, storedIters)
102+
if (this.debug) console.log(`self-describing: (${hashed === storedHashHex}) ${hashed}`)
103+
if (hashed === storedHashHex) {
104+
return { valid: true, needsUpgrade: storedIters < PBKDF2_ITERATIONS }
105+
}
106+
return { valid: false, needsUpgrade: false }
107+
}
108+
109+
// Raw hex (legacy NicTool 2 format, implicitly 5000 iterations)
110+
const legacy = await this.hashAuthPbkdf2(passTry, salt, LEGACY_ITERATIONS)
111+
if (this.debug) console.log(`legacy: (${legacy === passDb}) ${legacy}`)
112+
if (legacy === passDb) {
113+
return { valid: true, needsUpgrade: true }
114+
}
115+
return { valid: false, needsUpgrade: false }
80116
}
81117

82118
// Check for HMAC SHA-1 password
83119
if (/^[0-9a-f]{40}$/.test(passDb)) {
84120
const digest = crypto.createHmac('sha1', username.toLowerCase()).update(passTry).digest('hex')
85121
if (this.debug) console.log(`digest: (${digest === passDb}) ${digest}`)
86-
return digest === passDb
122+
if (digest === passDb) {
123+
return { valid: true, needsUpgrade: true }
124+
}
87125
}
88126

89-
return false
127+
return { valid: false, needsUpgrade: false }
90128
}
91129
}
92130

lib/user/store/mysql.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,18 @@ class UserRepoMySQL extends UserBase {
3636
AND g.name = ?`
3737

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

6677
if (args.password) {
6778
if (!args.pass_salt) args.pass_salt = this.generateSalt()
68-
args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt)
79+
args.password = await this.hashForStorage(args.password, args.pass_salt)
6980
}
7081

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

lib/user/store/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/test.js

Lines changed: 147 additions & 19 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' }
@@ -77,27 +79,35 @@ describe('user', function () {
7779
describe('validPassword', function () {
7880
it('auths user with plain text password', async () => {
7981
const r = await User.validPassword('test', 'test', 'demo', '')
80-
assert.equal(r, true)
82+
assert.deepEqual(r, { valid: true, needsUpgrade: 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-
)
90-
assert.equal(r, true)
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)
89+
assert.deepEqual(r, { valid: true, needsUpgrade: false })
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-
)
100-
assert.equal(r, false)
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.deepEqual(r, { valid: false, needsUpgrade: 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.deepEqual(r, { valid: true, needsUpgrade: 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)
110+
assert.deepEqual(r, { valid: false, needsUpgrade: false })
101111
})
102112

103113
it('auths valid SHA1 password', async () => {
@@ -106,7 +116,7 @@ describe('user', function () {
106116
'083007777a5241d01abba70c938c60d80be60027',
107117
'unit-test',
108118
)
109-
assert.equal(r, true)
119+
assert.deepEqual(r, { valid: true, needsUpgrade: true })
110120
})
111121

112122
it('rejects invalid SHA1 password', async () => {
@@ -115,7 +125,7 @@ describe('user', function () {
115125
'083007777a5241d01abba7Oc938c60d80be60027',
116126
'unit-test',
117127
)
118-
assert.equal(r, false)
128+
assert.deepEqual(r, { valid: false, needsUpgrade: false })
119129
})
120130
})
121131

@@ -144,4 +154,122 @@ 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+
// Raw SQL so we can insert specific legacy password formats (plain text,
183+
// SHA-1, PBKDF2-5000) that User.create() would hash on the way in.
184+
async function insertUser(password, passSalt) {
185+
await Mysql.execute(
186+
'INSERT INTO nt_user (nt_user_id, nt_group_id, username, email, first_name, last_name, password, pass_salt) VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
187+
[upgradeUserId, upgradeUser.nt_group_id, upgradeUser.username, upgradeUser.email, upgradeUser.first_name, upgradeUser.last_name, password, passSalt],
188+
)
189+
}
190+
191+
async function cleanup() {
192+
await Mysql.execute(
193+
'DELETE FROM nt_user WHERE nt_user_id = ?',
194+
[upgradeUserId],
195+
)
196+
}
197+
198+
before(cleanup)
199+
after(cleanup)
200+
201+
it('upgrades plain text password to self-describing PBKDF2 on login', async () => {
202+
await cleanup()
203+
await insertUser(testPass, null)
204+
205+
const result = await User.authenticate(authCreds)
206+
assert.ok(result, 'login should succeed')
207+
208+
const row = await getDbPassword()
209+
assert.ok(row.pass_salt, 'pass_salt should be set after upgrade')
210+
assert.notEqual(row.password, testPass, 'password should be hashed')
211+
assert.ok(row.password.includes('$'), 'password should be in self-describing format')
212+
213+
// verify round-trip: can still log in with the upgraded hash
214+
const again = await User.authenticate(authCreds)
215+
assert.ok(again, 'login should succeed after upgrade')
216+
await cleanup()
217+
})
218+
219+
it('upgrades SHA1 password to self-describing PBKDF2 on login', async () => {
220+
// authenticate() passes the full authTry.username (including @group) to
221+
// validPassword(), so the HMAC key must match that full string
222+
const sha1Hash = crypto
223+
.createHmac('sha1', authCreds.username.toLowerCase())
224+
.update(testPass)
225+
.digest('hex')
226+
await cleanup()
227+
await insertUser(sha1Hash, null)
228+
229+
const result = await User.authenticate(authCreds)
230+
assert.ok(result, 'login should succeed with SHA1 hash')
231+
232+
const row = await getDbPassword()
233+
assert.ok(row.pass_salt, 'pass_salt should be set after upgrade')
234+
assert.notEqual(row.password, sha1Hash, 'password should be re-hashed')
235+
assert.ok(row.password.includes('$'), 'password should be in self-describing format')
236+
237+
const again = await User.authenticate(authCreds)
238+
assert.ok(again, 'login should succeed after upgrade')
239+
await cleanup()
240+
})
241+
242+
it('upgrades PBKDF2-5000 to self-describing format on login', async () => {
243+
const legacySalt = User.generateSalt()
244+
const legacyHash = await User.hashAuthPbkdf2(testPass, legacySalt, 5000)
245+
await cleanup()
246+
await insertUser(legacyHash, legacySalt)
247+
248+
const result = await User.authenticate(authCreds)
249+
assert.ok(result, 'login should succeed with legacy PBKDF2')
250+
251+
const row = await getDbPassword()
252+
assert.notEqual(row.password, legacyHash, 'password should be re-hashed')
253+
assert.notEqual(row.pass_salt, legacySalt, 'salt should be regenerated')
254+
assert.ok(row.password.includes('$'), 'password should be in self-describing format')
255+
256+
const again = await User.authenticate(authCreds)
257+
assert.ok(again, 'login should succeed after upgrade')
258+
await cleanup()
259+
})
260+
261+
it('does not re-hash password already in self-describing format', async () => {
262+
const salt = User.generateSalt()
263+
const hash = await User.hashForStorage(testPass, salt)
264+
await cleanup()
265+
await insertUser(hash, salt)
266+
267+
await User.authenticate(authCreds)
268+
269+
const row = await getDbPassword()
270+
assert.equal(row.password, hash, 'password should be unchanged')
271+
assert.equal(row.pass_salt, salt, 'salt should be unchanged')
272+
await cleanup()
273+
})
274+
})
147275
})

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)