Description
Node.js supports three KDFs:
const password = '123';
const salt = '123';
const keyLength = 10;
const digest = 'sha256';
const iterations = 1000;
const info = 'info';
const pbkdf2 = crypto.pbkdf2Sync(password, salt, iterations, keyLength, digest);
const scrypt = crypto.scryptSync(password, salt, keyLength);
const hkdf = crypto.hkdfSync(digest, password, salt, info, keyLength);
console.log({ pbkdf2, scrypt, hkdf });
Output:
{
pbkdf2: <Buffer 00 1c 95 97 43 5c 16 7a 03 6d>,
scrypt: <Buffer 92 8a 6a ad 7d 2b 8c f8 49 20>,
hkdf: ArrayBuffer {
[Uint8Contents]: <bb b5 51 4b 6b c0 57 34 ea e0>,
byteLength: 10
}
}
Inconsistencies:
- Unlike the two more established key derivation functions, HKDF produces an
ArrayBuffer
, not a Node.jsBuffer
. In the async case, the callback even has the same signature and same argument names(err, derivedKey)
, but they derived key is not aBuffer
when using HKDF. - The arguments are in a different order than the arguments to
pbkdf2
.
And this can absolutely lead to security issues. The output is computationally indistinguishable from a random bit sequence and all three KDFs are collision-resistant and preimage-resistant, therefore, the comparison of outputs does not necessarily need to be timing-safe. While not advisable, it is possible (depending on the application) to compare the outputs in a timing-unsafe manner without compromising the password
a.toString('hex') === b.toString('hex')
to compare the output of the KDF to a known correct value, e.g., as part of a login procedure. Now change a
and b
to ArrayBuffer
each, and suddenly, the condition is always true, since
hkdf.toString('hex') === '[object ArrayBuffer]'
When scrypt was added, we had a lengthy discussion about designing KDF APIs. Quoting #21766 (comment):
cryptography API design is very important to application security in a lot of ways that aren't always immediately obvious. Node.js cryptography API security is, therefore, emphatically a security issue that affects Node.js.
Is there anything we can do about it now? Or simply label it wontfix
and close the issue?