Proposal: generic key derivation function for crypto

ranisalt commented 1 year ago

What is the problem this feature will solve?

The crypto library currently lacks a generic key derivation function similar to OpenSSL's EVP_KDF methods and resembling WebCrypto's deriveKey. This proposal aims to introduce a generic key derivation function to the crypto library, enabling developers to derive keys for various cryptographic purposes with simplicity.

The current situation, with separate functions for each supported KDF, can lead to inconsistencies in the API. Each KDF function might have its unique set of parameters and usage patterns, making it challenging for developers to work seamlessly with different key derivation methods.

What is the feature you are proposing to solve the problem?

I propose the addition of a new function named kdf to the crypto library. This function will take the following parameters:

algorithm: An object specifying the key derivation algorithm to use. This object should contain information about the algorithm's name, mode, parameters, and any other necessary details, allowing developers to choose from a variety of key derivation algorithms, including those available in OpenSSL 3.0. This object is akin to the first parameter of Web Crypto's deriveKey.
baseKey: The base key from which the derived key will be generated (e.g. the password to be hashed). This base key can be of any type supported by the Crypto library, such as an ArrayBuffer, Buffer, or other relevant types.
salt: A Buffer containing the salt to be used in the key derivation process, enhancing the security of the derived key.
callback: An optional function to be run with the key after the derivation is complete. If undefined, run the job synchronously (as suggested by @panva).

Example Usage

Here is an example of how the crypto.kdf function might be used in Node.js:

const { kdf } = require('node:crypto');

const baseKey = 'hunter2';

const algorithm = {
  name: 'scrypt',
  cost: 32768,
};

/** 
 * Using PBKDF2 instead:
 *
 * const algorithm = {
 *   name: 'pbkdf2',
 *   iterations: 100000,
 *   digest: 'sha-512',
 * };
 */

try {
  const derivedKey = kdf(algorithm, baseKey, crypto.randomBytes(16));
  // derivedKey is a Buffer
} catch (err) {
  console.err('An error has occurred!', err);
}

What alternatives have you considered?

It is possible to keep adding functions to the crypto library as new KDF algorithms are supported, but the more options there are, the more duplication there will be as apart from the parameters available, KDF functions are quite similar in interface and behavior: you pass a key and a salt and it returns a hash.

bnoordhuis commented 1 year ago

An optional function to be run with the key after the derivation is complete. If undefined, run the job synchronously

I don't think that's a good idea - and I'm the one who introduced that pattern in the crypto module when I added randomBytes(). Mistake in hindsight, synchronous code should stand out like a sore thumb.

Also, there's a good counter example: randomFill() vs. randomFillSync().

ranisalt commented 1 year ago

@bnoordhuis would kdf and kdfSync, doing the obvious work, be a good option instead? I'm fine with either.

How about returning a Promise? It seems to be the new standard, and there's callbackify for anyone running legacy code.

panva commented 1 year ago

For the sake of consistency in the crypto module -> callbacks.

bnoordhuis commented 1 year ago

Yes. People can call util.promisify(...) to get a promise-based version.

For the record, I'm not vehemently opposed but I'm lukewarm about introducing a general multiplexing KDF API. The thing is you're almost always interested in a single concrete KDF. Once you've picked one, you don't switch lightly.

From that perspective a general API just doesn't seem that useful, only a source of potential confusion.

khaosdoctor commented 1 year ago

While I fully support the easiness of changing and creating different hashes using different KDFs, I also agree with @bnoordhuis on that it's going to create confusion. So I'm proposing a new approach, adding the generic KDF function internally only.

In this use case the idea would be to have a single internal entrypoint for all KDFs like @ranisalt mentioned, but we have different facades for which algorithm is supported or not. In my POV this would have the following pros:

It would be easier to add new KDFs to the library as needed, like #50353, this would have been an easier implementation
There is the possibility of implementing new algorithms or deprecating old ones by deprecating the facade without modifying internal behaviour (and then correct/deprecate the internal one as well)
If one day, the chance arises that we decide to change the crypto module to something generic, it's a matter of exposing the internal KDF and removing the facades
All KDFs have the same output, varying inputs

But, on the other hand, this could (and potentially will) be a big refactor in the code base since we would need to fiddle with all the KDFs for the sake of consistency as well, but it can be done one by one after the initial implementation.

What do you think?

tniessen commented 1 year ago

For the record, I'm not vehemently opposed but I'm lukewarm about introducing a general multiplexing KDF API. The thing is you're almost always interested in a single concrete KDF. Once you've picked one, you don't switch lightly.

I don't disagree @bnoordhuis. However, with the current direction OpenSSL 3 is taking, I wouldn't be surprised if there were some KDFs implemented through OpenSSL providers at some point, and a generic KDF API might allow users to interface with those in Node.js, similar to how we resolve ciphers, hash functions, etc.

github-actions[bot] commented 7 months ago

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

ranisalt commented 7 months ago

I wouldn't be surprised if there were some KDFs implemented through OpenSSL providers at some point

If I understand it correctly, that's already how they are implemented. #50353 uses generic KDF facilities

github-actions[bot] commented 1 month ago

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

ranisalt commented 3 weeks ago

This may be more interesting/consistent now that crypto.hash is a thing

nodejs / node