Skip to content

Commit 683ea01

Browse files
authored
self describing password hashes (#56)
1 parent 49e1b6c commit 683ea01

7 files changed

Lines changed: 352 additions & 76 deletions

File tree

lib/user/credentials.js

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import crypto from 'node:crypto'
2+
3+
const DEFAULT_ITERATIONS = 220000
4+
const LEGACY_ITERATIONS = 5000
5+
const SELF_DESCRIBING_RE = /^(\d+)\$([0-9a-f]{64})$/
6+
const LEGACY_PBKDF2_RE = /^[0-9a-f]{64}$/
7+
const LEGACY_SHA1_RE = /^[0-9a-f]{40}$/
8+
9+
// PBKDF2-SHA512 costs ~40ms at the 220k default. This ceiling bounds both a
10+
// misconfigured env var and a tampered DB row: a stored count is untrusted
11+
// input to a CPU bound function, so cap the amplification at roughly 4x.
12+
const MAX_ITERATIONS = 1_000_000
13+
14+
function validIterations(n) {
15+
return Number.isSafeInteger(n) && n >= 1 && n <= MAX_ITERATIONS
16+
}
17+
18+
const PBKDF2_ITERATIONS = (() => {
19+
const raw = process.env.PBKDF2_ITERATIONS
20+
if (raw === undefined) return DEFAULT_ITERATIONS
21+
22+
// Number() rather than parseInt(), which would read "220000zzz" as 220000
23+
const parsed = Number(raw)
24+
if (!validIterations(parsed)) {
25+
console.warn(
26+
`PBKDF2_ITERATIONS="${raw}" is not an integer in 1..${MAX_ITERATIONS}, using ${DEFAULT_ITERATIONS}`,
27+
)
28+
return DEFAULT_ITERATIONS
29+
}
30+
return parsed
31+
})()
32+
33+
function equalsConstantTime(a, b) {
34+
const bufA = Buffer.from(a, 'hex')
35+
const bufB = Buffer.from(b, 'hex')
36+
if (bufA.length !== bufB.length) return false
37+
return crypto.timingSafeEqual(bufA, bufB)
38+
}
39+
40+
/**
41+
* Password hashing and verification, independent of how users are stored.
42+
*
43+
* Hashes are stored self-describing as `iterations$hexHash`, so verification
44+
* never has to guess a format and a wrong password costs exactly one PBKDF2
45+
* computation no matter how many legacy formats have existed.
46+
*/
47+
class Credentials {
48+
constructor(args = {}) {
49+
this.debug = args?.debug ?? false
50+
}
51+
52+
generateSalt(length = 16) {
53+
const chars = Array.from({ length: 87 }, (_, i) => String.fromCharCode(i + 40)) // ASCII 40–126
54+
let salt = ''
55+
for (let i = 0; i < length; i++) {
56+
salt += chars[Math.floor(Math.random() * 87)]
57+
}
58+
return salt
59+
}
60+
61+
async hashAuthPbkdf2(pass, salt, iterations = PBKDF2_ITERATIONS) {
62+
return new Promise((resolve, reject) => {
63+
crypto.pbkdf2(pass, salt, iterations, 32, 'sha512', (err, derivedKey) => {
64+
if (err) return reject(err)
65+
resolve(derivedKey.toString('hex'))
66+
})
67+
})
68+
}
69+
70+
async hashForStorage(pass, salt, iterations = PBKDF2_ITERATIONS) {
71+
const hex = await this.hashAuthPbkdf2(pass, salt, iterations)
72+
return `${iterations}$${hex}`
73+
}
74+
75+
/**
76+
* The password/pass_salt pair to persist for a plain text password.
77+
*
78+
* Omit `salt` to get a fresh one, which is what a password change and a
79+
* hash upgrade both want. Pass an existing salt only to preserve a caller
80+
* supplied one, as create() does for fixtures and imports.
81+
*/
82+
async forStorage(pass, salt = this.generateSalt()) {
83+
return { password: await this.hashForStorage(pass, salt), pass_salt: salt }
84+
}
85+
86+
async validPassword(passTry, passDb, username, salt) {
87+
const invalid = { valid: false, needsUpgrade: false }
88+
89+
if (!salt && passTry === passDb) {
90+
return { valid: true, needsUpgrade: true }
91+
}
92+
93+
if (salt) {
94+
// Self-describing format: "iterations$hexHash" — single hash, no fallback
95+
const m = SELF_DESCRIBING_RE.exec(passDb)
96+
if (m) {
97+
const storedIters = parseInt(m[1], 10)
98+
const storedHashHex = m[2]
99+
if (!validIterations(storedIters)) {
100+
console.warn(`refusing stored PBKDF2 iteration count ${storedIters} for user ${username}`)
101+
return invalid
102+
}
103+
104+
let hashed
105+
try {
106+
hashed = await this.hashAuthPbkdf2(passTry, salt, storedIters)
107+
} catch (err) {
108+
console.error(`PBKDF2 failed for user ${username}`, err)
109+
return invalid
110+
}
111+
112+
if (this.debug) console.log(`self-describing: ${hashed}`)
113+
if (equalsConstantTime(hashed, storedHashHex)) {
114+
return { valid: true, needsUpgrade: storedIters < PBKDF2_ITERATIONS }
115+
}
116+
return invalid
117+
}
118+
119+
// Raw hex (legacy NicTool 2 format, implicitly 5000 iterations). Reject
120+
// on shape before hashing, so an unrecognized stored value still costs
121+
// an attempt exactly zero PBKDF2 computations.
122+
if (!LEGACY_PBKDF2_RE.test(passDb)) return invalid
123+
124+
const legacy = await this.hashAuthPbkdf2(passTry, salt, LEGACY_ITERATIONS)
125+
if (this.debug) console.log(`legacy: ${legacy}`)
126+
if (equalsConstantTime(legacy, passDb)) {
127+
return { valid: true, needsUpgrade: true }
128+
}
129+
return invalid
130+
}
131+
132+
// HMAC SHA-1 (NicTool 2). Verified, never written: a successful match
133+
// returns needsUpgrade so the caller rewrites it as PBKDF2 immediately.
134+
// CodeQL js/insufficient-password-hash flags this; dismissed as won't fix.
135+
if (LEGACY_SHA1_RE.test(passDb)) {
136+
const digest = crypto.createHmac('sha1', username.toLowerCase()).update(passTry).digest('hex')
137+
if (this.debug) console.log(`digest: ${digest}`)
138+
if (equalsConstantTime(digest, passDb)) {
139+
return { valid: true, needsUpgrade: true }
140+
}
141+
}
142+
143+
return invalid
144+
}
145+
}
146+
147+
export default new Credentials()
148+
export { Credentials }

lib/user/store/base.js

Lines changed: 3 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
import crypto from 'node:crypto'
2-
31
/**
4-
* User domain classpure attributes and password business logic.
2+
* Base user repositorythe persistence contract
53
*
64
* Has zero knowledge of how users are persisted. All user repository classes
7-
* must extend this class and implement the repo contract methods.
5+
* must extend this class and implement the repo contract methods. Password
6+
* hashing and verification live in ../credentials.js.
87
*
98
* Repo contract:
109
* authenticate(authTry) → { user, group } | undefined
@@ -50,48 +49,6 @@ class UserBase {
5049
async destroy(_args) {
5150
throw new Error('destroy() not implemented by this repo')
5251
}
53-
54-
// -------------------------------------------------------------------------
55-
// Password business logic
56-
// -------------------------------------------------------------------------
57-
58-
generateSalt(length = 16) {
59-
const chars = Array.from({ length: 87 }, (_, i) => String.fromCharCode(i + 40)) // ASCII 40–126
60-
let salt = ''
61-
for (let i = 0; i < length; i++) {
62-
salt += chars[Math.floor(Math.random() * 87)]
63-
}
64-
return salt
65-
}
66-
67-
async hashAuthPbkdf2(pass, salt) {
68-
return new Promise((resolve, reject) => {
69-
// match the defaults for NicTool 2.x
70-
crypto.pbkdf2(pass, salt, 5000, 32, 'sha512', (err, derivedKey) => {
71-
if (err) return reject(err)
72-
resolve(derivedKey.toString('hex'))
73-
})
74-
})
75-
}
76-
77-
async validPassword(passTry, passDb, username, salt) {
78-
if (!salt && passTry === passDb) return true // plain pass, TODO, encrypt!
79-
80-
if (salt) {
81-
const hashed = await this.hashAuthPbkdf2(passTry, salt)
82-
if (this.debug) console.log(`hashed: (${hashed === passDb}) ${hashed}`)
83-
return hashed === passDb
84-
}
85-
86-
// Check for HMAC SHA-1 password
87-
if (/^[0-9a-f]{40}$/.test(passDb)) {
88-
const digest = crypto.createHmac('sha1', username.toLowerCase()).update(passTry).digest('hex')
89-
if (this.debug) console.log(`digest: (${digest === passDb}) ${digest}`)
90-
return digest === passDb
91-
}
92-
93-
return false
94-
}
9552
}
9653

9754
export default UserBase

lib/user/store/mysql.js

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Mysql from '../../mysql.js'
22
import Config from '../../config.js'
3+
import Credentials from '../credentials.js'
34
import UserBase from './base.js'
45
import Permission from '../../permission/index.js'
56
import { mapToDbColumn } from '../../util.js'
@@ -36,7 +37,23 @@ class UserRepoMySQL extends UserBase {
3637
AND g.name = ?`
3738

3839
for (const u of await Mysql.execute(query, [username, groupName])) {
39-
if (await this.validPassword(authTry.password, u.password, authTry.username, u.pass_salt)) {
40+
const { valid, needsUpgrade } = await Credentials.validPassword(
41+
authTry.password,
42+
u.password,
43+
authTry.username,
44+
u.pass_salt,
45+
)
46+
if (valid) {
47+
// best effort: a failed rewrite must not cost the user their login,
48+
// they simply get upgraded on a later attempt
49+
if (needsUpgrade) {
50+
try {
51+
const creds = await Credentials.forStorage(authTry.password)
52+
await Mysql.execute(...Mysql.update('nt_user', `nt_user_id=${u.id}`, creds))
53+
} catch (err) {
54+
console.warn(`password hash upgrade failed for user ${authTry.username}`, err)
55+
}
56+
}
4057
for (const f of ['password', 'pass_salt']) {
4158
delete u[f] // SECURITY: no longer needed
4259
}
@@ -64,8 +81,7 @@ class UserRepoMySQL extends UserBase {
6481
delete args.inherit_group_permissions
6582

6683
if (args.password) {
67-
if (!args.pass_salt) args.pass_salt = this.generateSalt()
68-
args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt)
84+
Object.assign(args, await Credentials.forStorage(args.password, args.pass_salt))
6985
}
7086

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

lib/user/store/toml.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { fileURLToPath } from 'node:url'
55
import { parse, stringify } from 'smol-toml'
66

77
import Config from '../../config.js'
8+
import Credentials from '../credentials.js'
89
import UserBase from './base.js'
910

1011
const __dirname = path.dirname(fileURLToPath(import.meta.url))
@@ -86,7 +87,24 @@ class UserRepoTOML extends UserBase {
8687
if (u.username !== username) continue
8788
if (u.deleted) continue
8889

89-
if (await this.validPassword(authTry.password, u.password, authTry.username, u.pass_salt)) {
90+
const { valid, needsUpgrade } = await Credentials.validPassword(
91+
authTry.password,
92+
u.password,
93+
authTry.username,
94+
u.pass_salt,
95+
)
96+
if (valid) {
97+
// best effort, as in the MySQL repo: a failed rewrite must not cost
98+
// the user their login
99+
if (needsUpgrade) {
100+
try {
101+
Object.assign(u, await Credentials.forStorage(authTry.password))
102+
await this._save(users)
103+
} catch (err) {
104+
console.warn(`password hash upgrade failed for user ${authTry.username}`, err)
105+
}
106+
}
107+
90108
const result = { ...u }
91109
for (const f of ['password', 'pass_salt', 'permissions']) delete result[f]
92110
const g = { id: result.gid, name: groupName }
@@ -151,8 +169,7 @@ class UserRepoTOML extends UserBase {
151169
delete args.inherit_group_permissions
152170

153171
if (args.password) {
154-
if (!args.pass_salt) args.pass_salt = this.generateSalt()
155-
args.password = await this.hashAuthPbkdf2(args.password, args.pass_salt)
172+
Object.assign(args, await Credentials.forStorage(args.password, args.pass_salt))
156173
}
157174

158175
if (inherit === false) {

lib/user/test/index.js

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { describe, it, after, before } from 'node:test'
33

44
import User from '../index.js'
55
import Group from '../../group/index.js'
6+
import Credentials from '../credentials.js'
67

78
import userJson from './user.json' with { type: 'json' }
89
import groupJson from '../../group/test/group.json' with { type: 'json' }
@@ -90,46 +91,54 @@ describe('user', function () {
9091

9192
describe('validPassword', function () {
9293
it('auths user with plain text password', async () => {
93-
const r = await User.validPassword('test', 'test', 'demo', '')
94-
assert.equal(r, true)
94+
const r = await Credentials.validPassword('test', 'test', 'demo', '')
95+
assert.deepEqual(r, { valid: true, needsUpgrade: true })
9596
})
9697

97-
it('auths valid pbkdb2 password', async () => {
98-
const r = await User.validPassword(
99-
'YouGuessedIt!',
100-
'050cfa70c3582be0d5bfae25138a8486dc2e6790f39bc0c4e111223ba6034432',
101-
'unit-test',
102-
'(ICzAm2.QfCa6.MN',
103-
)
104-
assert.equal(r, true)
98+
it('auths valid self-describing PBKDF2 password', async () => {
99+
const salt = '(ICzAm2.QfCa6.MN'
100+
const hash = await Credentials.hashForStorage('YouGuessedIt!', salt)
101+
const r = await Credentials.validPassword('YouGuessedIt!', hash, 'unit-test', salt)
102+
assert.deepEqual(r, { valid: true, needsUpgrade: false })
105103
})
106104

107-
it('rejects invalid pbkdb2 password', async () => {
108-
const r = await User.validPassword(
109-
'YouMissedIt!',
110-
'050cfa70c3582be0d5bfae25138a8486dc2e6790f39bc0c4e111223ba6034432',
111-
'unit-test',
112-
'(ICzAm2.QfCa6.MN',
113-
)
114-
assert.equal(r, false)
105+
it('rejects invalid self-describing PBKDF2 password', async () => {
106+
const salt = '(ICzAm2.QfCa6.MN'
107+
const hash = await Credentials.hashForStorage('YouGuessedIt!', salt)
108+
const r = await Credentials.validPassword('YouMissedIt!', hash, 'unit-test', salt)
109+
assert.deepEqual(r, { valid: false, needsUpgrade: false })
110+
})
111+
112+
it('auths valid legacy PBKDF2-5000 password', async () => {
113+
const salt = '(ICzAm2.QfCa6.MN'
114+
const hash = await Credentials.hashAuthPbkdf2('YouGuessedIt!', salt, 5000)
115+
const r = await Credentials.validPassword('YouGuessedIt!', hash, 'unit-test', salt)
116+
assert.deepEqual(r, { valid: true, needsUpgrade: true })
117+
})
118+
119+
it('rejects invalid legacy PBKDF2-5000 password', async () => {
120+
const salt = '(ICzAm2.QfCa6.MN'
121+
const hash = await Credentials.hashAuthPbkdf2('YouGuessedIt!', salt, 5000)
122+
const r = await Credentials.validPassword('YouMissedIt!', hash, 'unit-test', salt)
123+
assert.deepEqual(r, { valid: false, needsUpgrade: false })
115124
})
116125

117126
it('auths valid SHA1 password', async () => {
118-
const r = await User.validPassword(
127+
const r = await Credentials.validPassword(
119128
'OhNoYouDont',
120129
'083007777a5241d01abba70c938c60d80be60027',
121130
'unit-test',
122131
)
123-
assert.equal(r, true)
132+
assert.deepEqual(r, { valid: true, needsUpgrade: true })
124133
})
125134

126135
it('rejects invalid SHA1 password', async () => {
127-
const r = await User.validPassword(
136+
const r = await Credentials.validPassword(
128137
'OhNoYouDont',
129138
'083007777a5241d01abba7Oc938c60d80be60027',
130139
'unit-test',
131140
)
132-
assert.equal(r, false)
141+
assert.deepEqual(r, { valid: false, needsUpgrade: false })
133142
})
134143
})
135144

0 commit comments

Comments
 (0)