src: refactor SubtleCrypto algorithm and length validations

PR-URL: https://github.com/nodejs/node/pull/57319
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

lib/internal/crypto/aes.js

@@ -4,9 +4,7 @@ const {
   ArrayBufferIsView,
   ArrayBufferPrototypeSlice,
   ArrayFrom,
-  ArrayPrototypeIncludes,
   ArrayPrototypePush,
-  MathFloor,
   PromiseReject,
   SafeSet,
   TypedArrayPrototypeSlice,
@@ -35,10 +33,7 @@ const {
 const {
   hasAnyNotIn,
   jobPromise,
-  validateByteLength,
   validateKeyOps,
-  validateMaxBufferLength,
-  kAesKeyLengths,
   kHandle,
   kKeyObject,
 } = require('internal/crypto/util');
@@ -58,7 +53,6 @@ const {
   generateKey: _generateKey,
 } = require('internal/crypto/keygen');
 
-const kTagLengths = [32, 64, 96, 104, 112, 120, 128];
 const generateKey = promisify(_generateKey);
 
 function getAlgorithmName(name, length) {
@@ -108,20 +102,7 @@ function getVariant(name, length) {
   }
 }
 
-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 (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,
@@ -132,12 +113,7 @@ function asyncAesCtrCipher(mode, key, data, algorithm) {
     algorithm.length));
 }
 
-function validateAesCbcAlgorithm(algorithm) {
-  validateByteLength(algorithm.iv, 'algorithm.iv', 16);
-}
-
 function asyncAesCbcCipher(mode, key, data, algorithm) {
-  validateAesCbcAlgorithm(algorithm);
   return jobPromise(() => new AESCipherJob(
     kCryptoJobAsync,
     mode,
@@ -156,25 +132,10 @@ function asyncAesKwCipher(mode, key, data) {
     getVariant('AES-KW', key.algorithm.length)));
 }
 
-function validateAesGcmAlgorithm(algorithm) {
-  if (!ArrayPrototypeIncludes(kTagLengths, algorithm.tagLength)) {
-    throw lazyDOMException(
-      `${algorithm.tagLength} is not a valid AES-GCM tag length`,
-      'OperationError');
-  }
-
-  validateMaxBufferLength(algorithm.iv, 'algorithm.iv');
-
-  if (algorithm.additionalData !== undefined) {
-    validateMaxBufferLength(algorithm.additionalData, 'algorithm.additionalData');
-  }
-}
-
 function asyncAesGcmCipher(mode, key, data, algorithm) {
-  algorithm.tagLength ??= 128;
-  validateAesGcmAlgorithm(algorithm);
+  const { tagLength = 128 } = algorithm;
 
-  const tagByteLength = MathFloor(algorithm.tagLength / 8);
+  const tagByteLength = tagLength / 8;
   let tag;
   switch (mode) {
     case kWebCryptoCipherDecrypt: {
@@ -220,16 +181,7 @@ function aesCipher(mode, key, data, algorithm) {
   }
 }
 
-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'];

lib/internal/crypto/cfrg.js

@@ -329,15 +329,7 @@ function cfrgImportKey(
     extractable);
 }
 
-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';
 

lib/internal/crypto/diffiehellman.js

@@ -297,22 +297,9 @@ 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 (baseKey.type !== 'private') {

lib/internal/crypto/ec.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const {
-  ObjectPrototypeHasOwnProperty,
   SafeSet,
 } = primordials;
 
@@ -76,16 +75,7 @@ function createECPublicKeyRaw(namedCurve, keyData) {
   return new PublicKeyObject(handle);
 }
 
-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);
@@ -158,7 +148,6 @@ function ecImportKey(
   extractable,
   keyUsages,
 ) {
-  validateEcKeyAlgorithm(algorithm);
   const { name, namedCurve } = algorithm;
 
   let keyObject;

lib/internal/crypto/hkdf.js

@@ -138,7 +138,7 @@ function hkdfSync(hash, key, salt, info, length) {
 }
 
 const hkdfPromise = promisify(hkdf);
-function validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length) {
+function validateHkdfDeriveBitsLength(length) {
   if (length === null)
     throw lazyDOMException('length cannot be null', 'OperationError');
   if (length % 8) {
@@ -149,7 +149,7 @@ function validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length) {
 }
 
 async function hkdfDeriveBits(algorithm, baseKey, length) {
-  validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length);
+  validateHkdfDeriveBitsLength(length);
   const { hash, salt, info } = algorithm;
 
   if (length === 0)

lib/internal/crypto/keygen.js

@@ -33,10 +33,6 @@ const {
   parsePrivateKeyEncoding,
 } = require('internal/crypto/keys');
 
-const {
-  kAesKeyLengths,
-} = require('internal/crypto/util');
-
 const {
   customPromisifyArgs,
   kEmptyObject,
@@ -355,7 +351,7 @@ function generateKeyJob(mode, keyType, options) {
       validateInteger(length, 'options.length', 8, 2 ** 31 - 1);
       break;
     case 'aes':
-      validateOneOf(length, 'options.length', kAesKeyLengths);
+      validateOneOf(length, 'options.length', [128, 192, 256]);
       break;
     default:
       throw new ERR_INVALID_ARG_VALUE(

lib/internal/crypto/mac.js

@@ -40,24 +40,7 @@ const {
 
 const generateKey = promisify(_generateKey);
 
-function validateHmacGenerateKeyAlgorithm(algorithm) {
-  if (algorithm.length !== undefined) {
-    if (algorithm.length === 0)
-      throw lazyDOMException(
-        'Zero-length key is not supported',
-        'OperationError');
-
-    // The Web Crypto spec allows for key lengths that are not multiples of 8. We don't.
-    if (algorithm.length % 8) {
-      throw lazyDOMException(
-        'Unsupported algorithm.length',
-        'NotSupportedError');
-    }
-  }
-}
-
 async function hmacGenerateKey(algorithm, extractable, keyUsages) {
-  validateHmacGenerateKeyAlgorithm(algorithm);
   const { hash, name } = algorithm;
   let { length } = algorithm;
 
@@ -96,19 +79,6 @@ function getAlgorithmName(hash) {
   }
 }
 
-function validateHmacImportKeyAlgorithm(algorithm) {
-  if (algorithm.length !== undefined) {
-    if (algorithm.length === 0) {
-      throw lazyDOMException('Zero-length key is not supported', 'DataError');
-    }
-
-    // The Web Crypto spec allows for key lengths that are not multiples of 8. We don't.
-    if (algorithm.length % 8) {
-      throw lazyDOMException('Unsupported algorithm.length', 'NotSupportedError');
-    }
-  }
-}
-
 function hmacImportKey(
   format,
   keyData,
@@ -116,7 +86,6 @@ function hmacImportKey(
   extractable,
   keyUsages,
 ) {
-  validateHmacImportKeyAlgorithm(algorithm);
   const usagesSet = new SafeSet(keyUsages);
   if (hasAnyNotIn(usagesSet, ['sign', 'verify'])) {
     throw lazyDOMException(

lib/internal/crypto/pbkdf2.js

@@ -92,12 +92,7 @@ function check(password, salt, iterations, keylen, digest) {
 }
 
 const pbkdf2Promise = promisify(pbkdf2);
-function validatePbkdf2DeriveBitsAlgorithmAndLength(algorithm, length) {
-  if (algorithm.iterations === 0)
-    throw lazyDOMException(
-      'iterations cannot be zero',
-      'OperationError');
-
+function validatePbkdf2DeriveBitsLength(length) {
   if (length === null)
     throw lazyDOMException('length cannot be null', 'OperationError');
 
@@ -109,7 +104,7 @@ function validatePbkdf2DeriveBitsAlgorithmAndLength(algorithm, length) {
 }
 
 async function pbkdf2DeriveBits(algorithm, baseKey, length) {
-  validatePbkdf2DeriveBitsAlgorithmAndLength(algorithm, length);
+  validatePbkdf2DeriveBitsLength(length);
   const { iterations, hash, salt } = algorithm;
 
   if (length === 0)

lib/internal/crypto/rsa.js

@@ -111,23 +111,17 @@ function rsaOaepCipher(mode, key, data, algorithm) {
     algorithm.label));
 }
 
-function validateRsaKeyGenerateAlgorithm(algorithm) {
+async function rsaKeyGenerate(
+  algorithm,
+  extractable,
+  keyUsages,
+) {
   const publicExponentConverted = bigIntArrayToUnsignedInt(algorithm.publicExponent);
   if (publicExponentConverted === undefined) {
     throw lazyDOMException(
       'The publicExponent must be equivalent to an unsigned 32-bit value',
       'OperationError');
   }
-
-  return publicExponentConverted;
-}
-
-async function rsaKeyGenerate(
-  algorithm,
-  extractable,
-  keyUsages,
-) {
-  const publicExponentConverted = validateRsaKeyGenerateAlgorithm(algorithm);
   const {
     name,
     modulusLength,

lib/internal/crypto/util.js

@@ -168,8 +168,6 @@ const kNamedCurveAliases = {
   'P-521': 'secp521r1',
 };
 
-const kAesKeyLengths = [128, 192, 256];
-
 const kSupportedAlgorithms = {
   'digest': {
     'SHA-1': null,
@@ -416,14 +414,6 @@ function hasAnyNotIn(set, checks) {
   return false;
 }
 
-function validateByteLength(buf, name, target) {
-  if (buf.byteLength !== target) {
-    throw lazyDOMException(
-      `${name} must contain exactly ${target} bytes`,
-      'OperationError');
-  }
-}
-
 const validateByteSource = hideStackFrames((val, name) => {
   val = toBuf(val);
 
@@ -597,11 +587,9 @@ module.exports = {
   toBuf,
 
   kNamedCurveAliases,
-  kAesKeyLengths,
   normalizeAlgorithm,
   normalizeHashName,
   hasAnyNotIn,
-  validateByteLength,
   validateByteSource,
   validateKeyOps,
   jobPromise,

lib/internal/crypto/webidl.js

@@ -13,12 +13,14 @@
 const {
   ArrayBufferIsView,
   ArrayBufferPrototype,
+  ArrayPrototypeIncludes,
   ArrayPrototypePush,
   ArrayPrototypeSort,
   MathPow,
   MathTrunc,
   Number,
   NumberIsFinite,
+  ObjectPrototypeHasOwnProperty,
   ObjectPrototypeIsPrototypeOf,
   SafeArrayIterator,
   String,
@@ -36,11 +38,16 @@ const {
 } = require('internal/webidl');
 
 const {
+  lazyDOMException,
   kEmptyObject,
   setOwnProperty,
 } = require('internal/util');
 const { CryptoKey } = require('internal/crypto/webcrypto');
-const { getDataViewOrTypedArrayBuffer } = require('internal/crypto/util');
+const {
+  getDataViewOrTypedArrayBuffer,
+  validateMaxBufferLength,
+  kNamedCurveAliases,
+} = require('internal/crypto/util');
 
 // https://tc39.es/ecma262/#sec-tonumber
 function toNumber(value, opts = kEmptyObject) {
@@ -90,6 +97,28 @@ function type(V) {
 
 const integerPart = MathTrunc;
 
+function validateByteLength(buf, name, target) {
+  if (buf.byteLength !== target) {
+    throw lazyDOMException(
+      `${name} must contain exactly ${target} bytes`,
+      'OperationError');
+  }
+}
+
+function AESLengthValidator(V, dict) {
+  if (V !== 128 && V !== 192 && V !== 256)
+    throw lazyDOMException(
+      'AES key length must be 128, 192, or 256 bits',
+      'OperationError');
+}
+
+function namedCurveValidator(V, dict) {
+  if (!ObjectPrototypeHasOwnProperty(kNamedCurveAliases, V))
+    throw lazyDOMException(
+      'Unrecognized namedCurve',
+      'NotSupportedError');
+}
+
 // This was updated to only consider bitlength up to 32 used by WebCryptoAPI
 function createIntegerConversion(bitLength) {
   const lowerBound = 0;
@@ -275,12 +304,13 @@ function createDictionaryConverter(name, dictionaries) {
         const context = `'${key}' of '${name}'${
           opts.context ? ` (${opts.context})` : ''
         }`;
-        const converter = member.converter;
+        const { converter, validator } = member;
         const idlMemberValue = converter(esMemberValue, {
           __proto__: null,
           ...opts,
           context,
         });
+        validator?.(idlMemberValue, esDict);
         setOwnProperty(idlDict, key, idlMemberValue);
       } else if (member.required) {
         throw makeException(
@@ -393,6 +423,7 @@ converters.EcKeyImportParams = createDictionaryConverter(
     {
       key: 'namedCurve',
       converter: converters.NamedCurve,
+      validator: namedCurveValidator,
       required: true,
     },
   ]);
@@ -403,6 +434,7 @@ converters.EcKeyGenParams = createDictionaryConverter(
     {
       key: 'namedCurve',
       converter: converters.NamedCurve,
+      validator: namedCurveValidator,
       required: true,
     },
   ]);
@@ -414,6 +446,7 @@ converters.AesKeyGenParams = createDictionaryConverter(
       key: 'length',
       converter: (V, opts) =>
         converters['unsigned short'](V, { ...opts, enforceRange: true }),
+      validator: AESLengthValidator,
       required: true,
     },
   ]);
@@ -430,9 +463,19 @@ converters.HmacKeyGenParams = createDictionaryConverter(
       key: 'length',
       converter: (V, opts) =>
         converters['unsigned long'](V, { ...opts, enforceRange: true }),
+      validator: (V, dict) => validateHmacKeyAlgorithm(V),
     },
   ]);
 
+function validateHmacKeyAlgorithm(length) {
+  if (length === 0)
+    throw lazyDOMException('Zero-length key is not supported', 'DataError');
+
+  // The Web Crypto spec allows for key lengths that are not multiples of 8. We don't.
+  if (length % 8)
+    throw lazyDOMException('Unsupported algorithm.length', 'NotSupportedError');
+}
+
 converters.RsaPssParams = createDictionaryConverter(
   'RsaPssParams', [
     ...new SafeArrayIterator(dictAlgorithm),
@@ -475,6 +518,7 @@ converters.HmacImportParams = createDictionaryConverter(
       key: 'length',
       converter: (V, opts) =>
         converters['unsigned long'](V, { ...opts, enforceRange: true }),
+      validator: (V, dict) => validateHmacKeyAlgorithm(V),
     },
   ]);
 
@@ -552,6 +596,10 @@ converters.Pbkdf2Params = createDictionaryConverter(
       key: 'iterations',
       converter: (V, opts) =>
         converters['unsigned long'](V, { ...opts, enforceRange: true }),
+      validator: (V, dict) => {
+        if (V === 0)
+          throw lazyDOMException('iterations cannot be zero', 'OperationError');
+      },
       required: true,
     },
     {
@@ -568,6 +616,7 @@ converters.AesDerivedKeyParams = createDictionaryConverter(
       key: 'length',
       converter: (V, opts) =>
         converters['unsigned short'](V, { ...opts, enforceRange: true }),
+      validator: AESLengthValidator,
       required: true,
     },
   ]);
@@ -578,6 +627,7 @@ converters.AesCbcParams = createDictionaryConverter(
     {
       key: 'iv',
       converter: converters.BufferSource,
+      validator: (V, dict) => validateByteLength(V, 'algorithm.iv', 16),
       required: true,
     },
   ]);
@@ -588,16 +638,25 @@ converters.AesGcmParams = createDictionaryConverter(
     {
       key: 'iv',
       converter: converters.BufferSource,
+      validator: (V, dict) => validateMaxBufferLength(V, 'algorithm.iv'),
       required: true,
     },
     {
       key: 'tagLength',
       converter: (V, opts) =>
         converters.octet(V, { ...opts, enforceRange: true }),
+      validator: (V, dict) => {
+        if (!ArrayPrototypeIncludes([32, 64, 96, 104, 112, 120, 128], V)) {
+          throw lazyDOMException(
+            `${V} is not a valid AES-GCM tag length`,
+            'OperationError');
+        }
+      },
     },
     {
       key: 'additionalData',
       converter: converters.BufferSource,
+      validator: (V, dict) => validateMaxBufferLength(V, 'algorithm.additionalData'),
     },
   ]);
 
@@ -607,12 +666,19 @@ converters.AesCtrParams = createDictionaryConverter(
     {
       key: 'counter',
       converter: converters.BufferSource,
+      validator: (V, dict) => validateByteLength(V, 'algorithm.counter', 16),
       required: true,
     },
     {
       key: 'length',
       converter: (V, opts) =>
         converters.octet(V, { ...opts, enforceRange: true }),
+      validator: (V, dict) => {
+        if (V === 0 || V > 128)
+          throw lazyDOMException(
+            'AES-CTR algorithm.length must be between 1 and 128',
+            'OperationError');
+      },
       required: true,
     },
   ]);
@@ -626,6 +692,16 @@ converters.EcdhKeyDeriveParams = createDictionaryConverter(
     {
       key: 'public',
       converter: converters.CryptoKey,
+      validator: (V, dict) => {
+        if (V.type !== 'public')
+          throw lazyDOMException(
+            'algorithm.public must be a public key', 'InvalidAccessError');
+
+        if (V.algorithm.name.toUpperCase() !== dict.name.toUpperCase())
+          throw lazyDOMException(
+            `algorithm.public must be an ${dict.name.toUpperCase()} key`,
+            'InvalidAccessError');
+      },
       required: true,
     },
   ]);
@@ -636,6 +712,11 @@ converters.Ed448Params = createDictionaryConverter(
     {
       key: 'context',
       converter: converters.BufferSource,
+      validator: (V, dict) => {
+        if (V.byteLength)
+          throw lazyDOMException(
+            'Non zero-length context is not supported.', 'NotSupportedError');
+      },
       required: false,
     },
   ]);

test/parallel/test-webcrypto-sign-verify-eddsa.js

@@ -241,10 +241,10 @@ async function testSign({ name,
       await subtle.verify({ name: 'Ed448', context: Buffer.alloc(0) }, publicKey, sig, vector.data), true);
 
     await assert.rejects(subtle.sign({ name: 'Ed448', context: Buffer.alloc(1) }, privateKey, vector.data), {
-      message: /Non zero-length context is not yet supported/
+      message: /Non zero-length context is not supported/
     });
     await assert.rejects(subtle.verify({ name: 'Ed448', context: Buffer.alloc(1) }, publicKey, sig, vector.data), {
-      message: /Non zero-length context is not yet supported/
+      message: /Non zero-length context is not supported/
     });
   }).then(common.mustCall());
 }

test/parallel/test-webcrypto-webidl.js

@@ -386,9 +386,9 @@ const opts = { prefix, context };
 
     for (const good of [
       { name: 'HMAC', hash: { name: 'SHA-1' } },
-      { name: 'HMAC', hash: { name: 'SHA-1' }, length: 20 },
+      { name: 'HMAC', hash: { name: 'SHA-1' }, length: 32 },
       { name: 'HMAC', hash: 'SHA-1' },
-      { name: 'HMAC', hash: 'SHA-1', length: 20 },
+      { name: 'HMAC', hash: 'SHA-1', length: 32 },
     ]) {
       assert.deepStrictEqual(converter({ ...good, filtered: 'out' }, opts), good);
       assert.throws(() => converter({ ...good, hash: undefined }, opts), {
@@ -452,7 +452,7 @@ const opts = { prefix, context };
 
 // AesCbcParams
 {
-  const good = { name: 'AES-CBC', iv: Buffer.alloc(0) };
+  const good = { name: 'AES-CBC', iv: Buffer.alloc(16) };
   assert.deepStrictEqual(converters.AesCbcParams({ ...good, filtered: 'out' }, opts), good);
 
   assert.throws(() => converters.AesCbcParams({ ...good, iv: undefined }, opts), {
@@ -466,8 +466,8 @@ const opts = { prefix, context };
 {
   for (const good of [
     { name: 'AES-GCM', iv: Buffer.alloc(0) },
-    { name: 'AES-GCM', iv: Buffer.alloc(0), tagLength: 16 },
-    { name: 'AES-GCM', iv: Buffer.alloc(0), tagLength: 16, additionalData: Buffer.alloc(0) },
+    { name: 'AES-GCM', iv: Buffer.alloc(0), tagLength: 64 },
+    { name: 'AES-GCM', iv: Buffer.alloc(0), tagLength: 64, additionalData: Buffer.alloc(0) },
   ]) {
     assert.deepStrictEqual(converters.AesGcmParams({ ...good, filtered: 'out' }, opts), good);
 
@@ -481,7 +481,7 @@ const opts = { prefix, context };
 
 // AesCtrParams
 {
-  const good = { name: 'AES-CTR', counter: Buffer.alloc(0), length: 20 };
+  const good = { name: 'AES-CTR', counter: Buffer.alloc(16), length: 20 };
   assert.deepStrictEqual(converters.AesCtrParams({ ...good, filtered: 'out' }, opts), good);
 
   for (const required of ['counter', 'length']) {