hkdf API is inconsistent with other supported KDFs

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.js Buffer. In the async case, the callback even has the same signature and same argument names (err, derivedKey), but they derived key is not a Buffer 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?

Refs: #21766
Refs: #35093
Refs: #39453

The only non-breaking way out of it I see is

deprecate now and later remove hkdf
expose 'crypto/kdf' module => { hkdf, scrypt, pbkdf2 } with scrypt and pbhkf2 being aliases? while hkdf having Buffer as a return type and well-ordered arguments

KDFs would having their own module is then the remaining inconsistency with the rest of the crypto module?

I disagree with it being an actual issue but, I won't block if someone wants to change things so long as hkdf is not removed.

That said, there's no reason to expose a different module or make any breaking changes to the existing APIs. We can simply expose a single crypto.kdf(options) function...

const pbkdf2 = crypto.kdfSync({
  type: 'pbkdf2',
  password,
  salt,
  iterations,
  keyLength,
  digest,
  arrayBuffer: [true|false],
});

const scrypt = crypto.kdfSync({
  type: 'scrypt',
  password,
  salt,
  keyLength,
  arrayBuffer: [true|false],
});

const hkdf = crypto.kdfSync({
  type: 'hkdf',
  digest,
  password,
  salt,
  info,
  keyLength,
  arrayBuffer: [true|false],
);

I don't have a strong opinion. I do believe that the current API is not ideal and should have aimed for consistency with Node.js and not with WebCrypto, but I don't see a way to solve that problem now without breaking existing code. Adding more APIs makes things more complicated and does not actually solve the problem at hand. I opened this issue mostly so that, when someone does come across this problem, they will find a resolution, even if that resolution is wontfix.

Adding more APIs makes things more complicated and does not actually solve the problem at hand.

Why does replacing the current API not solve the problem at hand?

doc,lib,test: rename HKDF 'key' argument

I still think it's not ideal that we are being inconsistent in when we return Buffer, ArrayBuffer, or Uint8Array, but I don't see this improving, so I am closing the issue.

Navigation Menu

Repository navigation

hkdf API is inconsistent with other supported KDFs #39471

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

hkdf API is inconsistent with other supported KDFs #39471

Description

Activity

panva commented on Jul 20, 2021

jasnell commented on Jul 20, 2021

tniessen commented on Jul 20, 2021

panva commented on Jul 21, 2021

tniessen commented on Apr 19, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions