src: refactor SubtleCrypto algorithm and length validations

PR-URL: #57273 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jason Zhang <[email protected]> Reviewed-By: Mattias Buelens <[email protected]>
nodejs · Mar 4, 2025 · 6fdd4e6 · 6fdd4e6
1 parent 52ac448
commit 6fdd4e6
Show file tree

Hide file tree

Showing 17 changed files with 208 additions and 180 deletions.
diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js
@@ -58,7 +58,6 @@ const {
   generateKey: _generateKey,
 } = require('internal/crypto/keygen');
 
-const kMaxCounterLength = 128;
 const kTagLengths = [32, 64, 96, 104, 112, 120, 128];
 const generateKey = promisify(_generateKey);
 
@@ -109,35 +108,43 @@ function getVariant(name, length) {
   }
 }
 
-function asyncAesCtrCipher(mode, key, data, { counter, length }) {
-  validateByteLength(counter, 'algorithm.counter', 16);
+function validateAesCtrAlgorithm(algorithm) {
+  validateByteLength(algorithm.counter, 'algorithm.counter', 16);
   // The length must specify an integer between 1 and 128. While
   // there is no default, this should typically be 64.
-  if (length === 0 || length > kMaxCounterLength) {
+  if (algorithm.length === 0 || algorithm.length > 128) {
     throw lazyDOMException(
       'AES-CTR algorithm.length must be between 1 and 128',
       'OperationError');
   }
+}
+
+function asyncAesCtrCipher(mode, key, data, algorithm) {
+  validateAesCtrAlgorithm(algorithm);
 
   return jobPromise(() => new AESCipherJob(
     kCryptoJobAsync,
     mode,
     key[kKeyObject][kHandle],
     data,
     getVariant('AES-CTR', key.algorithm.length),
-    counter,
-    length));
+    algorithm.counter,
+    algorithm.length));
+}
+
+function validateAesCbcAlgorithm(algorithm) {
+  validateByteLength(algorithm.iv, 'algorithm.iv', 16);
 }
 
-function asyncAesCbcCipher(mode, key, data, { iv }) {
-  validateByteLength(iv, 'algorithm.iv', 16);
+function asyncAesCbcCipher(mode, key, data, algorithm) {
+  validateAesCbcAlgorithm(algorithm);
   return jobPromise(() => new AESCipherJob(
     kCryptoJobAsync,
     mode,
     key[kKeyObject][kHandle],
     data,
     getVariant('AES-CBC', key.algorithm.length),
-    iv));
+    algorithm.iv));
 }
 
 function asyncAesKwCipher(mode, key, data) {
@@ -149,24 +156,25 @@ function asyncAesKwCipher(mode, key, data) {
     getVariant('AES-KW', key.algorithm.length)));
 }
 
-function asyncAesGcmCipher(
-  mode,
-  key,
-  data,
-  { iv, additionalData, tagLength = 128 }) {
-  if (!ArrayPrototypeIncludes(kTagLengths, tagLength)) {
-    return PromiseReject(lazyDOMException(
-      `${tagLength} is not a valid AES-GCM tag length`,
-      'OperationError'));
+function validateAesGcmAlgorithm(algorithm) {
+  if (!ArrayPrototypeIncludes(kTagLengths, algorithm.tagLength)) {
+    throw lazyDOMException(
+      `${algorithm.tagLength} is not a valid AES-GCM tag length`,
+      'OperationError');
   }
 
-  validateMaxBufferLength(iv, 'algorithm.iv');
+  validateMaxBufferLength(algorithm.iv, 'algorithm.iv');
 
-  if (additionalData !== undefined) {
-    validateMaxBufferLength(additionalData, 'algorithm.additionalData');
+  if (algorithm.additionalData !== undefined) {
+    validateMaxBufferLength(algorithm.additionalData, 'algorithm.additionalData');
   }
+}
 
-  const tagByteLength = MathFloor(tagLength / 8);
+function asyncAesGcmCipher(mode, key, data, algorithm) {
+  algorithm.tagLength ??= 128;
+  validateAesGcmAlgorithm(algorithm);
+
+  const tagByteLength = MathFloor(algorithm.tagLength / 8);
   let tag;
   switch (mode) {
     case kWebCryptoCipherDecrypt: {
@@ -198,9 +206,9 @@ function asyncAesGcmCipher(
     key[kKeyObject][kHandle],
     data,
     getVariant('AES-GCM', key.algorithm.length),
-    iv,
+    algorithm.iv,
     tag,
-    additionalData));
+    algorithm.additionalData));
 }
 
 function aesCipher(mode, key, data, algorithm) {
@@ -212,13 +220,17 @@ function aesCipher(mode, key, data, algorithm) {
   }
 }
 
-async function aesGenerateKey(algorithm, extractable, keyUsages) {
-  const { name, length } = algorithm;
-  if (!ArrayPrototypeIncludes(kAesKeyLengths, length)) {
+function validateAesGenerateKeyAlgorithm(algorithm) {
+  if (!ArrayPrototypeIncludes(kAesKeyLengths, algorithm.length)) {
     throw lazyDOMException(
       'AES key length must be 128, 192, or 256 bits',
       'OperationError');
   }
+}
+
+async function aesGenerateKey(algorithm, extractable, keyUsages) {
+  validateAesGenerateKeyAlgorithm(algorithm);
+  const { name, length } = algorithm;
 
   const checkUsages = ['wrapKey', 'unwrapKey'];
   if (name !== 'AES-KW')

diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js
@@ -329,18 +329,21 @@ function cfrgImportKey(
     extractable);
 }
 
-function eddsaSignVerify(key, data, { name, context }, signature) {
+function validateEdDSASignVerifyAlgorithm(algorithm) {
+  if (algorithm.name === 'Ed448' && algorithm.context?.byteLength) {
+    throw lazyDOMException(
+      'Non zero-length context is not yet supported.', 'NotSupportedError');
+  }
+}
+
+function eddsaSignVerify(key, data, algorithm, signature) {
+  validateEdDSASignVerifyAlgorithm(algorithm);
   const mode = signature === undefined ? kSignJobModeSign : kSignJobModeVerify;
   const type = mode === kSignJobModeSign ? 'private' : 'public';
 
   if (key.type !== type)
     throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError');
 
-  if (name === 'Ed448' && context?.byteLength) {
-    throw lazyDOMException(
-      'Non zero-length context is not yet supported.', 'NotSupportedError');
-  }
-
   return jobPromise(() => new SignJob(
     kCryptoJobAsync,
     mode,

diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js
@@ -298,28 +298,28 @@ function diffieHellman(options) {
 
 let masks;
 
+function validateEcdhDeriveBitsAlgorithmAndLength(algorithm, length) {
+  if (algorithm.public.type !== 'public') {
+    throw lazyDOMException(
+      'algorithm.public must be a public key', 'InvalidAccessError');
+  }
+
+  if (algorithm.name !== algorithm.public.algorithm.name) {
+    throw lazyDOMException(`algorithm.public must be an ${algorithm.name} key`, 'InvalidAccessError');
+  }
+}
+
 // The ecdhDeriveBits function is part of the Web Crypto API and serves both
 // deriveKeys and deriveBits functions.
 async function ecdhDeriveBits(algorithm, baseKey, length) {
+  validateEcdhDeriveBitsAlgorithmAndLength(algorithm, length);
   const { 'public': key } = algorithm;
 
-  if (key.type !== 'public') {
-    throw lazyDOMException(
-      'algorithm.public must be a public key', 'InvalidAccessError');
-  }
   if (baseKey.type !== 'private') {
     throw lazyDOMException(
       'baseKey must be a private key', 'InvalidAccessError');
   }
 
-  if (
-    key.algorithm.name !== 'ECDH' &&
-    key.algorithm.name !== 'X25519' &&
-    key.algorithm.name !== 'X448'
-  ) {
-    throw lazyDOMException('Keys must be ECDH, X25519, or X448 keys', 'InvalidAccessError');
-  }
-
   if (key.algorithm.name !== baseKey.algorithm.name) {
     throw lazyDOMException(
       'The public and private keys must be of the same type',

diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
@@ -1,8 +1,7 @@
 'use strict';
 
 const {
-  ArrayPrototypeIncludes,
-  ObjectKeys,
+  ObjectPrototypeHasOwnProperty,
   SafeSet,
 } = primordials;
 
@@ -77,14 +76,17 @@ function createECPublicKeyRaw(namedCurve, keyData) {
   return new PublicKeyObject(handle);
 }
 
-async function ecGenerateKey(algorithm, extractable, keyUsages) {
-  const { name, namedCurve } = algorithm;
-
-  if (!ArrayPrototypeIncludes(ObjectKeys(kNamedCurveAliases), namedCurve)) {
+function validateEcKeyAlgorithm(algorithm) {
+  if (!ObjectPrototypeHasOwnProperty(kNamedCurveAliases, algorithm.namedCurve)) {
     throw lazyDOMException(
       'Unrecognized namedCurve',
       'NotSupportedError');
   }
+}
+
+async function ecGenerateKey(algorithm, extractable, keyUsages) {
+  validateEcKeyAlgorithm(algorithm);
+  const { name, namedCurve } = algorithm;
 
   const usageSet = new SafeSet(keyUsages);
   switch (name) {
@@ -154,16 +156,11 @@ function ecImportKey(
   keyData,
   algorithm,
   extractable,
-  keyUsages) {
-
+  keyUsages,
+) {
+  validateEcKeyAlgorithm(algorithm);
   const { name, namedCurve } = algorithm;
 
-  if (!ArrayPrototypeIncludes(ObjectKeys(kNamedCurveAliases), namedCurve)) {
-    throw lazyDOMException(
-      'Unrecognized namedCurve',
-      'NotSupportedError');
-  }
-
   let keyObject;
   const usagesSet = new SafeSet(keyUsages);
   switch (format) {

diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js
@@ -138,18 +138,22 @@ function hkdfSync(hash, key, salt, info, length) {
 }
 
 const hkdfPromise = promisify(hkdf);
-async function hkdfDeriveBits(algorithm, baseKey, length) {
-  const { hash, salt, info } = algorithm;
-
-  if (length === 0)
-    return new ArrayBuffer(0);
+function validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length) {
   if (length === null)
     throw lazyDOMException('length cannot be null', 'OperationError');
   if (length % 8) {
     throw lazyDOMException(
       'length must be a multiple of 8',
       'OperationError');
   }
+}
+
+async function hkdfDeriveBits(algorithm, baseKey, length) {
+  validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length);
+  const { hash, salt, info } = algorithm;
+
+  if (length === 0)
+    return new ArrayBuffer(0);
 
   try {
     return await hkdfPromise(

diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
@@ -895,12 +895,14 @@ function isCryptoKey(obj) {
 }
 
 function importGenericSecretKey(
-  { name, length },
+  algorithm,
   format,
   keyData,
   extractable,
-  keyUsages) {
+  keyUsages,
+) {
   const usagesSet = new SafeSet(keyUsages);
+  const { name } = algorithm;
   if (extractable)
     throw lazyDOMException(`${name} keys are not extractable`, 'SyntaxError');
 
@@ -910,47 +912,22 @@ function importGenericSecretKey(
       'SyntaxError');
   }
 
+  let keyObject;
   switch (format) {
     case 'KeyObject': {
-      if (hasAnyNotIn(usagesSet, ['deriveKey', 'deriveBits'])) {
-        throw lazyDOMException(
-          `Unsupported key usage for a ${name} key`,
-          'SyntaxError');
-      }
-
-      const checkLength = keyData.symmetricKeySize * 8;
-
-      // The Web Crypto spec allows for key lengths that are not multiples of
-      // 8. We don't. Our check here is stricter than that defined by the spec
-      // in that we require that algorithm.length match keyData.length * 8 if
-      // algorithm.length is specified.
-      if (length !== undefined && length !== checkLength) {
-        throw lazyDOMException('Invalid key length', 'DataError');
-      }
-      return new InternalCryptoKey(keyData, { name }, keyUsages, false);
+      keyObject = keyData;
+      break;
     }
     case 'raw': {
-      if (hasAnyNotIn(usagesSet, ['deriveKey', 'deriveBits'])) {
-        throw lazyDOMException(
-          `Unsupported key usage for a ${name} key`,
-          'SyntaxError');
-      }
-
-      const checkLength = keyData.byteLength * 8;
-
-      // The Web Crypto spec allows for key lengths that are not multiples of
-      // 8. We don't. Our check here is stricter than that defined by the spec
-      // in that we require that algorithm.length match keyData.length * 8 if
-      // algorithm.length is specified.
-      if (length !== undefined && length !== checkLength) {
-        throw lazyDOMException('Invalid key length', 'DataError');
-      }
-
-      const keyObject = createSecretKey(keyData);
-      return new InternalCryptoKey(keyObject, { name }, keyUsages, false);
+      keyObject = createSecretKey(keyData);
+      break;
     }
   }
 
+  if (keyObject) {
+    return new InternalCryptoKey(keyObject, { name }, keyUsages, false);
+  }
+
   throw lazyDOMException(
     `Unable to import ${name} key with format ${format}`,
     'NotSupportedError');