crypto: accept key data in crypto.diffieHellman() and cleanup DH jobs

Signed-off-by: Filip Skokan <panva.ip@gmail.com>

Author: panva

doc/api/crypto.md

@@ -4142,23 +4142,32 @@ added:
  - v13.9.0
  - v12.17.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/62527
+    description: Accept key data in addition to KeyObject instances.
   - version: v23.11.0
     pr-url: https://github.com/nodejs/node/pull/57274
     description: Optional callback argument added.
 -->
 
 * `options` {Object}
-  * `privateKey` {KeyObject}
-  * `publicKey` {KeyObject}
+  * `privateKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject}
+  * `publicKey` {Object|string|ArrayBuffer|Buffer|TypedArray|DataView|KeyObject}
 * `callback` {Function}
   * `err` {Error}
   * `secret` {Buffer}
 * Returns: {Buffer} if the `callback` function is not provided.
 
 Computes the Diffie-Hellman shared secret based on a `privateKey` and a `publicKey`.
-Both keys must have the same `asymmetricKeyType` and must support either the DH or
+Both keys must represent the same asymmetric key type and must support either the DH or
 ECDH operation.
 
+If `options.privateKey` is not a [`KeyObject`][], this function behaves as if
+`options.privateKey` had been passed to [`crypto.createPrivateKey()`][].
+
+If `options.publicKey` is not a [`KeyObject`][], this function behaves as if
+`options.publicKey` had been passed to [`crypto.createPublicKey()`][].
+
 If the `callback` function is provided this function uses libuv's threadpool.
 
 ### `crypto.encapsulate(key[, callback])`

lib/internal/crypto/diffiehellman.js

@@ -17,7 +17,6 @@ const {
   DiffieHellman: _DiffieHellman,
   DiffieHellmanGroup: _DiffieHellmanGroup,
   ECDH: _ECDH,
-  ECDHBitsJob,
   ECDHConvertKey: _ECDHConvertKey,
   kCryptoJobAsync,
   kCryptoJobSync,
@@ -52,9 +51,11 @@ const {
 } = require('internal/util');
 
 const {
-  KeyObject,
+  isKeyObject,
   kAlgorithm,
   kKeyType,
+  preparePrivateKey,
+  preparePublicOrPrivateKey,
 } = require('internal/crypto/keys');
 
 const {
@@ -284,31 +285,65 @@ function diffieHellman(options, callback) {
     validateFunction(callback, 'callback');
 
   const { privateKey, publicKey } = options;
-  if (!(privateKey instanceof KeyObject))
+
+  // TODO(@panva): remove these non-semver-major error code preserving measures
+  // in a semver-major followup, the final state is just preparePublicOrPrivateKey
+  // and preparePrivateKey
+  if (privateKey == null)
     throw new ERR_INVALID_ARG_VALUE('options.privateKey', privateKey);
 
-  if (!(publicKey instanceof KeyObject))
+  if (publicKey == null)
     throw new ERR_INVALID_ARG_VALUE('options.publicKey', publicKey);
 
-  if (privateKey.type !== 'private')
-    throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(privateKey.type, 'private');
+  if (isKeyObject(privateKey)) {
+    if (privateKey.type !== 'private')
+      throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(privateKey.type, 'private');
+  }
 
-  if (publicKey.type !== 'public' && publicKey.type !== 'private') {
-    throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(publicKey.type,
-                                                 'private or public');
+  if (isKeyObject(publicKey)) {
+    if (publicKey.type !== 'public' && publicKey.type !== 'private') {
+      throw new ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE(publicKey.type,
+                                                   'private or public');
+    }
   }
 
-  const privateType = privateKey.asymmetricKeyType;
-  const publicType = publicKey.asymmetricKeyType;
-  if (privateType !== publicType || !dhEnabledKeyTypes.has(privateType)) {
-    throw new ERR_CRYPTO_INCOMPATIBLE_KEY('key types for Diffie-Hellman',
-                                          `${privateType} and ${publicType}`);
+  if (isKeyObject(privateKey) && isKeyObject(publicKey)) {
+    const privateType = privateKey.asymmetricKeyType;
+    const publicType = publicKey.asymmetricKeyType;
+    if (privateType !== publicType || !dhEnabledKeyTypes.has(privateType)) {
+      throw new ERR_CRYPTO_INCOMPATIBLE_KEY('key types for Diffie-Hellman',
+                                            `${privateType} and ${publicType}`);
+    }
   }
 
+  const {
+    data: pubData,
+    format: pubFormat,
+    type: pubType,
+    passphrase: pubPassphrase,
+    namedCurve: pubNamedCurve,
+  } = preparePublicOrPrivateKey(publicKey, 'options.publicKey');
+
+  const {
+    data: privData,
+    format: privFormat,
+    type: privType,
+    passphrase: privPassphrase,
+    namedCurve: privNamedCurve,
+  } = preparePrivateKey(privateKey, 'options.privateKey');
+
   const job = new DHBitsJob(
     callback ? kCryptoJobAsync : kCryptoJobSync,
-    publicKey[kHandle],
-    privateKey[kHandle]);
+    pubData,
+    pubFormat,
+    pubType,
+    pubPassphrase,
+    pubNamedCurve,
+    privData,
+    privFormat,
+    privType,
+    privPassphrase,
+    privNamedCurve);
 
   if (!callback) {
     const { 0: err, 1: secret } = job.run();
@@ -349,10 +384,18 @@ async function ecdhDeriveBits(algorithm, baseKey, length) {
     throw lazyDOMException('Named curve mismatch', 'InvalidAccessError');
   }
 
-  const bits = await jobPromise(() => new ECDHBitsJob(
+  const bits = await jobPromise(() => new DHBitsJob(
     kCryptoJobAsync,
     key[kKeyObject][kHandle],
-    baseKey[kKeyObject][kHandle]));
+    undefined,
+    undefined,
+    undefined,
+    undefined,
+    baseKey[kKeyObject][kHandle],
+    undefined,
+    undefined,
+    undefined,
+    undefined));
 
   // If a length is not specified, return the full derived secret
   if (length === null)

lib/internal/crypto/keys.js

@@ -628,7 +628,7 @@ function getKeyTypes(allowKeyObject, bufferOnly = false) {
 }
 
 
-function prepareAsymmetricKey(key, ctx) {
+function prepareAsymmetricKey(key, ctx, name = 'key') {
   if (isKeyObject(key)) {
     // Best case: A key object, as simple as that.
     return { data: getKeyObjectHandle(key, ctx) };
@@ -639,7 +639,7 @@ function prepareAsymmetricKey(key, ctx) {
   }
   if (isStringOrBuffer(key)) {
     // Expect PEM by default, mostly for backward compatibility.
-    return { format: kKeyFormatPEM, data: getArrayBufferOrView(key, 'key') };
+    return { format: kKeyFormatPEM, data: getArrayBufferOrView(key, name) };
   }
   if (typeof key === 'object') {
     const { key: data, encoding, format } = key;
@@ -654,23 +654,23 @@ function prepareAsymmetricKey(key, ctx) {
       return { data: getKeyObjectHandle(data[kKeyObject], ctx) };
     }
     if (format === 'jwk') {
-      validateObject(data, 'key.key');
+      validateObject(data, `${name}.key`);
       return { data, format: kKeyFormatJWK };
     } else if (format === 'raw-public' || format === 'raw-private' ||
                format === 'raw-seed') {
       if (!isStringOrBuffer(data)) {
         throw new ERR_INVALID_ARG_TYPE(
-          'key.key',
+          `${name}.key`,
           ['ArrayBuffer', 'Buffer', 'TypedArray', 'DataView'],
           data);
       }
-      validateString(key.asymmetricKeyType, 'key.asymmetricKeyType');
+      validateString(key.asymmetricKeyType, `${name}.asymmetricKeyType`);
       if (key.asymmetricKeyType === 'ec') {
-        validateString(key.namedCurve, 'key.namedCurve');
+        validateString(key.namedCurve, `${name}.namedCurve`);
       }
       const rawFormat = parseKeyFormat(format, undefined, 'options.format');
       return {
-        data: getArrayBufferOrView(data, 'key.key'),
+        data: getArrayBufferOrView(data, `${name}.key`),
         format: rawFormat,
         type: key.asymmetricKeyType,
         namedCurve: key.namedCurve ?? null,
@@ -680,31 +680,31 @@ function prepareAsymmetricKey(key, ctx) {
     // Either PEM or DER using PKCS#1 or SPKI.
     if (!isStringOrBuffer(data)) {
       throw new ERR_INVALID_ARG_TYPE(
-        'key.key',
+        `${name}.key`,
         getKeyTypes(ctx !== kCreatePrivate),
         data);
     }
 
     const isPublic =
       (ctx === kConsumePrivate || ctx === kCreatePrivate) ? false : undefined;
     return {
-      data: getArrayBufferOrView(data, 'key', encoding),
+      data: getArrayBufferOrView(data, `${name}.key`, encoding),
       ...parseKeyEncoding(key, undefined, isPublic),
     };
   }
 
   throw new ERR_INVALID_ARG_TYPE(
-    'key',
+    name,
     getKeyTypes(ctx !== kCreatePrivate),
     key);
 }
 
-function preparePrivateKey(key) {
-  return prepareAsymmetricKey(key, kConsumePrivate);
+function preparePrivateKey(key, name) {
+  return prepareAsymmetricKey(key, kConsumePrivate, name);
 }
 
-function preparePublicOrPrivateKey(key) {
-  return prepareAsymmetricKey(key, kConsumePublic);
+function preparePublicOrPrivateKey(key, name) {
+  return prepareAsymmetricKey(key, kConsumePublic, name);
 }
 
 function prepareSecretKey(key, encoding, bufferOnly = false) {

lib/internal/crypto/sig.js

@@ -135,7 +135,7 @@ Sign.prototype.sign = function sign(options, encoding) {
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
 
   const { data, format, type, passphrase, namedCurve } =
-    preparePrivateKey(options, true);
+    preparePrivateKey(options);
 
   // Options specific to RSA
   const rsaPadding = getPadding(options);
@@ -239,7 +239,7 @@ Verify.prototype.verify = function verify(options, signature, sigEncoding) {
     type,
     passphrase,
     namedCurve,
-  } = preparePublicOrPrivateKey(options, true);
+  } = preparePublicOrPrivateKey(options);
 
   // Options specific to RSA
   const rsaPadding = getPadding(options);

src/crypto/crypto_dh.cc

@@ -477,20 +477,16 @@ Maybe<void> DHBitsTraits::AdditionalConfig(
     const FunctionCallbackInfo<Value>& args,
     unsigned int offset,
     DHBitsConfig* params) {
-  CHECK(args[offset]->IsObject());  // public key
-  CHECK(args[offset + 1]->IsObject());  // private key
+  auto public_key = KeyObjectData::GetPublicOrPrivateKeyFromJs(args, &offset);
+  if (!public_key) [[unlikely]]
+    return Nothing<void>();
 
-  KeyObjectHandle* private_key;
-  KeyObjectHandle* public_key;
+  auto private_key = KeyObjectData::GetPrivateKeyFromJs(args, &offset, true);
+  if (!private_key) [[unlikely]]
+    return Nothing<void>();
 
-  ASSIGN_OR_RETURN_UNWRAP(&public_key, args[offset], Nothing<void>());
-  ASSIGN_OR_RETURN_UNWRAP(&private_key, args[offset + 1], Nothing<void>());
-
-  CHECK(private_key->Data().GetKeyType() == kKeyTypePrivate);
-  CHECK(public_key->Data().GetKeyType() != kKeyTypeSecret);
-
-  params->public_key = public_key->Data().addRef();
-  params->private_key = private_key->Data().addRef();
+  params->public_key = std::move(public_key);
+  params->private_key = std::move(private_key);
 
   return JustVoid();
 }