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

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
PR-URL: #62527
Backport-PR-URL: #63563
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: panva

doc/api/crypto.md

@@ -4123,23 +4123,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,
@@ -51,9 +50,11 @@ const {
 } = require('internal/util');
 
 const {
-  KeyObject,
+  isKeyObject,
   kAlgorithm,
   kKeyType,
+  preparePrivateKey,
+  preparePublicOrPrivateKey,
 } = require('internal/crypto/keys');
 
 const {
@@ -281,31 +282,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();
@@ -346,10 +381,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

@@ -467,9 +467,9 @@ function parseKeyType(typeStr, required, keyType, isPublic, optionName) {
   throw new ERR_INVALID_ARG_VALUE(optionName, typeStr);
 }
 
-function option(name, objName) {
-  return objName === undefined ?
-    `options.${name}` : `options.${objName}.${name}`;
+function option(name, prefix) {
+  return prefix === undefined ?
+    `options.${name}` : `${prefix}.${name}`;
 }
 
 function parseKeyFormatAndType(enc, keyType, isPublic, objName) {
@@ -629,7 +629,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) };
@@ -640,7 +640,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;
@@ -655,7 +655,7 @@ 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') {
@@ -681,31 +681,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),
-      ...parseKeyEncoding(key, undefined, isPublic),
+      data: getArrayBufferOrView(data, `${name}.key`, encoding),
+      ...parseKeyEncoding(key, undefined, isPublic, name),
     };
   }
 
   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

@@ -130,19 +130,19 @@ function getIntOption(name, options) {
   return undefined;
 }
 
-Sign.prototype.sign = function sign(options, encoding) {
-  if (!options)
+Sign.prototype.sign = function sign(privateKey, encoding) {
+  if (!privateKey)
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
 
   const { data, format, type, passphrase, namedCurve } =
-    preparePrivateKey(options, true);
+    preparePrivateKey(privateKey, 'privateKey');
 
   // Options specific to RSA
-  const rsaPadding = getPadding(options);
-  const pssSaltLength = getSaltLength(options);
+  const rsaPadding = getPadding(privateKey);
+  const pssSaltLength = getSaltLength(privateKey);
 
   // Options specific to (EC)DSA
-  const dsaSigEnc = getDSASignatureEncoding(options);
+  const dsaSigEnc = getDSASignatureEncoding(privateKey);
 
   const ret = this[kHandle].sign(data, format, type,
                                  passphrase, namedCurve,
@@ -232,21 +232,21 @@ ObjectSetPrototypeOf(Verify, Writable);
 Verify.prototype._write = Sign.prototype._write;
 Verify.prototype.update = Sign.prototype.update;
 
-Verify.prototype.verify = function verify(options, signature, sigEncoding) {
+Verify.prototype.verify = function verify(key, signature, sigEncoding) {
   const {
     data,
     format,
     type,
     passphrase,
     namedCurve,
-  } = preparePublicOrPrivateKey(options, true);
+  } = preparePublicOrPrivateKey(key, 'key');
 
   // Options specific to RSA
-  const rsaPadding = getPadding(options);
-  const pssSaltLength = getSaltLength(options);
+  const rsaPadding = getPadding(key);
+  const pssSaltLength = getSaltLength(key);
 
   // Options specific to (EC)DSA
-  const dsaSigEnc = getDSASignatureEncoding(options);
+  const dsaSigEnc = getDSASignatureEncoding(key);
 
   signature = getArrayBufferOrView(signature, 'signature', sigEncoding);
 

src/crypto/crypto_dh.cc

@@ -505,20 +505,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();
 }