crypto: use WebIDL converters in WebCryptoAPI

WebCryptoAPI functions' arguments are now coersed and validated as per their WebIDL definitions like in other Web Crypto API implementations. This further improves interoperability with other implementations of Web Crypto API. PR-URL: nodejs#46067 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Backport-PR-URL: nodejs#47383
panva · Apr 18, 2023 · 52e3bf8 · 52e3bf8
1 parent 8570ffa
commit 52e3bf8
Show file tree

Hide file tree

Showing 33 changed files with 2,061 additions and 440 deletions.
diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md
@@ -2,6 +2,10 @@
 
 <!-- YAML
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/46067
+    description: Arguments are now coerced and validated as per their WebIDL
+      definitions like in other Web Crypto API implementations.
   - version: v16.17.0
     pr-url: https://github.com/nodejs/node/pull/43310
     description: Removed proprietary `'node.keyObject'` import/export format.

diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js
@@ -32,7 +32,6 @@ const {
 } = internalBinding('crypto');
 
 const {
-  getArrayBufferOrView,
   hasAnyNotIn,
   jobPromise,
   validateByteLength,
@@ -112,13 +111,10 @@ function getVariant(name, length) {
 }
 
 function asyncAesCtrCipher(mode, key, data, { counter, length }) {
-  counter = getArrayBufferOrView(counter, 'algorithm.counter');
   validateByteLength(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 (typeof length !== 'number' ||
-      length <= 0 ||
-      length > kMaxCounterLength) {
+  if (length === 0 || length > kMaxCounterLength) {
     throw lazyDOMException(
       'AES-CTR algorithm.length must be between 1 and 128',
       'OperationError');
@@ -135,7 +131,6 @@ function asyncAesCtrCipher(mode, key, data, { counter, length }) {
 }
 
 function asyncAesCbcCipher(mode, key, data, { iv }) {
-  iv = getArrayBufferOrView(iv, 'algorithm.iv');
   validateByteLength(iv, 'algorithm.iv', 16);
   return jobPromise(() => new AESCipherJob(
     kCryptoJobAsync,
@@ -166,12 +161,9 @@ function asyncAesGcmCipher(
       'OperationError'));
   }
 
-  iv = getArrayBufferOrView(iv, 'algorithm.iv');
   validateMaxBufferLength(iv, 'algorithm.iv');
 
   if (additionalData !== undefined) {
-    additionalData =
-      getArrayBufferOrView(additionalData, 'algorithm.additionalData');
     validateMaxBufferLength(additionalData, 'algorithm.additionalData');
   }
 
@@ -282,24 +274,26 @@ async function aesImportKey(
       break;
     }
     case 'jwk': {
-      if (keyData == null || typeof keyData !== 'object')
-        throw lazyDOMException('Invalid JWK keyData', 'DataError');
+      if (!keyData.kty)
+        throw lazyDOMException('Invalid keyData', 'DataError');
 
       if (keyData.kty !== 'oct')
-        throw lazyDOMException('Invalid key type', 'DataError');
+        throw lazyDOMException('Invalid JWK "kty" Parameter', 'DataError');
 
       if (usagesSet.size > 0 &&
           keyData.use !== undefined &&
           keyData.use !== 'enc') {
-        throw lazyDOMException('Invalid use type', 'DataError');
+        throw lazyDOMException('Invalid JWK "use" Parameter', 'DataError');
       }
 
       validateKeyOps(keyData.key_ops, usagesSet);
 
       if (keyData.ext !== undefined &&
           keyData.ext === false &&
           extractable === true) {
-        throw lazyDOMException('JWK is not extractable', 'DataError');
+        throw lazyDOMException(
+          'JWK "ext" Parameter and extractable mismatch',
+          'DataError');
       }
 
       const handle = new KeyObjectHandle();
@@ -309,10 +303,10 @@ async function aesImportKey(
       validateKeyLength(length);
 
       if (keyData.alg !== undefined) {
-        if (typeof keyData.alg !== 'string')
-          throw lazyDOMException('Invalid alg', 'DataError');
         if (keyData.alg !== getAlgorithmName(algorithm.name, length))
-          throw lazyDOMException('Algorithm mismatch', 'DataError');
+          throw lazyDOMException(
+            'JWK "alg" does not match the requested algorithm',
+            'DataError');
       }
 
       keyObject = new SecretKeyObject(handle);

diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js
@@ -18,7 +18,6 @@ const {
 } = internalBinding('crypto');
 
 const {
-  getArrayBufferOrView,
   getUsagesUnion,
   hasAnyNotIn,
   jobPromise,
@@ -83,7 +82,6 @@ function verifyAcceptableCfrgKeyUse(name, type, usages) {
 
 function createCFRGRawKey(name, keyData, isPublic) {
   const handle = new KeyObjectHandle();
-  keyData = getArrayBufferOrView(keyData, 'keyData');
 
   switch (name) {
     case 'Ed25519':
@@ -248,12 +246,13 @@ async function cfrgImportKey(
       break;
     }
     case 'jwk': {
-      if (keyData == null || typeof keyData !== 'object')
-        throw lazyDOMException('Invalid JWK keyData', 'DataError');
+      if (!keyData.kty)
+        throw lazyDOMException('Invalid keyData', 'DataError');
       if (keyData.kty !== 'OKP')
-        throw lazyDOMException('Invalid key type', 'DataError');
+        throw lazyDOMException('Invalid JWK "kty" Parameter', 'DataError');
       if (keyData.crv !== name)
-        throw lazyDOMException('Subtype mismatch', 'DataError');
+        throw lazyDOMException(
+          'JWK "crv" Parameter and algorithm name mismatch', 'DataError');
       const isPublic = keyData.d === undefined;
 
       if (usagesSet.size > 0 && keyData.use !== undefined) {
@@ -271,30 +270,32 @@ async function cfrgImportKey(
             break;
         }
         if (keyData.use !== checkUse)
-          throw lazyDOMException('Invalid use type', 'DataError');
+          throw lazyDOMException('Invalid JWK "use" Parameter', 'DataError');
       }
 
       validateKeyOps(keyData.key_ops, usagesSet);
 
       if (keyData.ext !== undefined &&
           keyData.ext === false &&
           extractable === true) {
-        throw lazyDOMException('JWK is not extractable', 'DataError');
+        throw lazyDOMException(
+          'JWK "ext" Parameter and extractable mismatch',
+          'DataError');
       }
 
       if (keyData.alg !== undefined) {
-        if (typeof keyData.alg !== 'string')
-          throw lazyDOMException('Invalid alg', 'DataError');
         if (
           (name === 'Ed25519' || name === 'Ed448') &&
           keyData.alg !== 'EdDSA'
         ) {
-          throw lazyDOMException('Invalid alg', 'DataError');
+          throw lazyDOMException(
+            'JWK "alg" does not match the requested algorithm',
+            'DataError');
         }
       }
 
       if (!isPublic && typeof keyData.x !== 'string') {
-        throw lazyDOMException('Invalid JWK keyData', 'DataError');
+        throw lazyDOMException('Invalid JWK', 'DataError');
       }
 
       verifyAcceptableCfrgKeyUse(
@@ -316,7 +317,7 @@ async function cfrgImportKey(
           false);
 
         if (!createPublicKey(keyObject).equals(publicKeyObject)) {
-          throw lazyDOMException('Invalid JWK keyData', 'DataError');
+          throw lazyDOMException('Invalid JWK', 'DataError');
         }
       }
       break;
@@ -347,13 +348,9 @@ function eddsaSignVerify(key, data, { name, context }, signature) {
   if (key.type !== type)
     throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError');
 
-  if (name === 'Ed448' && context !== undefined) {
-    context =
-      getArrayBufferOrView(context, 'algorithm.context');
-    if (context.byteLength !== 0) {
-      throw lazyDOMException(
-        'Non zero-length context is not yet supported.', 'NotSupportedError');
-    }
+  if (name === 'Ed448' && context?.byteLength) {
+    throw lazyDOMException(
+      'Non zero-length context is not yet supported.', 'NotSupportedError');
   }
 
   return jobPromise(() => new SignJob(

diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js
@@ -37,7 +37,6 @@ const {
   validateInt32,
   validateObject,
   validateString,
-  validateUint32,
 } = require('internal/validators');
 
 const {
@@ -51,7 +50,6 @@ const {
 
 const {
   KeyObject,
-  isCryptoKey,
 } = require('internal/crypto/keys');
 
 const {
@@ -335,13 +333,6 @@ function deriveBitsECDH(name, publicKey, privateKey, callback) {
 async function asyncDeriveBitsECDH(algorithm, baseKey, length) {
   const { 'public': key } = algorithm;
 
-  // Null means that we're not asking for a specific number of bits, just
-  // give us everything that is generated.
-  if (length !== null)
-    validateUint32(length, 'length');
-  if (!isCryptoKey(key))
-    throw new ERR_INVALID_ARG_TYPE('algorithm.public', 'CryptoKey', key);
-
   if (key.type !== 'public') {
     throw lazyDOMException(
       'algorithm.public must be a public key', 'InvalidAccessError');

diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
@@ -18,13 +18,6 @@ const {
 } = internalBinding('crypto');
 
 const {
-  codes: {
-    ERR_MISSING_OPTION,
-  }
-} = require('internal/errors');
-
-const {
-  getArrayBufferOrView,
   getUsagesUnion,
   hasAnyNotIn,
   jobPromise,
@@ -86,7 +79,6 @@ function verifyAcceptableEcKeyUse(name, type, usages) {
 
 function createECPublicKeyRaw(namedCurve, keyData) {
   const handle = new KeyObjectHandle();
-  keyData = getArrayBufferOrView(keyData, 'keyData');
 
   if (!handle.initECRaw(kNamedCurveAliases[namedCurve], keyData)) {
     throw lazyDOMException('Invalid keyData', 'DataError');
@@ -215,12 +207,14 @@ async function ecImportKey(
       break;
     }
     case 'jwk': {
-      if (keyData == null || typeof keyData !== 'object')
-        throw lazyDOMException('Invalid JWK keyData', 'DataError');
+      if (!keyData.kty)
+        throw lazyDOMException('Invalid keyData', 'DataError');
       if (keyData.kty !== 'EC')
-        throw lazyDOMException('Invalid key type', 'DataError');
+        throw lazyDOMException('Invalid JWK "kty" Parameter', 'DataError');
       if (keyData.crv !== namedCurve)
-        throw lazyDOMException('Named curve mismatch', 'DataError');
+        throw lazyDOMException(
+          'JWK "crv" does not match the requested algorithm',
+          'DataError');
 
       if (keyData.d !== undefined) {
         verifyAcceptableEcKeyUse(name, 'private', usagesSet);
@@ -229,37 +223,38 @@ async function ecImportKey(
       }
 
       if (usagesSet.size > 0 && keyData.use !== undefined) {
-        if (algorithm.name === 'ECDSA' && keyData.use !== 'sig')
-          throw lazyDOMException('Invalid use type', 'DataError');
-        if (algorithm.name === 'ECDH' && keyData.use !== 'enc')
-          throw lazyDOMException('Invalid use type', 'DataError');
+        const checkUse = name === 'ECDH' ? 'enc' : 'sig';
+        if (keyData.use !== checkUse)
+          throw lazyDOMException('Invalid JWK "use" Parameter', 'DataError');
       }
 
       validateKeyOps(keyData.key_ops, usagesSet);
 
       if (keyData.ext !== undefined &&
           keyData.ext === false &&
           extractable === true) {
-        throw lazyDOMException('JWK is not extractable', 'DataError');
+        throw lazyDOMException(
+          'JWK "ext" Parameter and extractable mismatch',
+          'DataError');
       }
 
       if (algorithm.name === 'ECDSA' && keyData.alg !== undefined) {
-        if (typeof keyData.alg !== 'string')
-          throw lazyDOMException('Invalid alg', 'DataError');
         let algNamedCurve;
         switch (keyData.alg) {
           case 'ES256': algNamedCurve = 'P-256'; break;
           case 'ES384': algNamedCurve = 'P-384'; break;
           case 'ES512': algNamedCurve = 'P-521'; break;
         }
         if (algNamedCurve !== namedCurve)
-          throw lazyDOMException('Named curve mismatch', 'DataError');
+          throw lazyDOMException(
+            'JWK "alg" does not match the requested algorithm',
+            'DataError');
       }
 
       const handle = new KeyObjectHandle();
       const type = handle.initJwk(keyData, namedCurve);
       if (type === undefined)
-        throw lazyDOMException('Invalid JWK keyData', 'DataError');
+        throw lazyDOMException('Invalid JWK', 'DataError');
       keyObject = type === kKeyTypePrivate ?
         new PrivateKeyObject(handle) :
         new PublicKeyObject(handle);
@@ -301,8 +296,6 @@ function ecdsaSignVerify(key, data, { name, hash }, signature) {
   if (key.type !== type)
     throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError');
 
-  if (hash === undefined)
-    throw new ERR_MISSING_OPTION('algorithm.hash');
   const hashname = normalizeHashName(hash.name);
 
   return jobPromise(() => new SignJob(

diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js
@@ -14,11 +14,9 @@ const {
 } = internalBinding('crypto');
 
 const {
-  getArrayBufferOrView,
   getDefaultEncoding,
   getStringOption,
   jobPromise,
-  normalizeAlgorithm,
   normalizeHashName,
   validateMaxBufferLength,
   kHandle,
@@ -168,13 +166,8 @@ Hmac.prototype._transform = Hash.prototype._transform;
 // Implementation for WebCrypto subtle.digest()
 
 async function asyncDigest(algorithm, data) {
-  algorithm = normalizeAlgorithm(algorithm);
-  data = getArrayBufferOrView(data, 'data');
   validateMaxBufferLength(data, 'data');
 
-  if (algorithm.length !== undefined)
-    validateUint32(algorithm.length, 'algorithm.length');
-
   switch (algorithm.name) {
     case 'SHA-1':
       // Fall through
@@ -186,11 +179,10 @@ async function asyncDigest(algorithm, data) {
       return jobPromise(() => new HashJob(
         kCryptoJobAsync,
         normalizeHashName(algorithm.name),
-        data,
-        algorithm.length));
+        data));
   }
 
-  throw lazyDOMException('Unrecognized name.', 'NotSupportedError');
+  throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError');
 }
 
 module.exports = {

diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js
@@ -21,7 +21,6 @@ const {
 const { kMaxLength } = require('buffer');
 
 const {
-  getArrayBufferOrView,
   normalizeHashName,
   toBuf,
   validateByteSource,
@@ -46,7 +45,6 @@ const {
   codes: {
     ERR_INVALID_ARG_TYPE,
     ERR_OUT_OF_RANGE,
-    ERR_MISSING_OPTION,
   },
   hideStackFrames,
 } = require('internal/errors');
@@ -140,11 +138,7 @@ function hkdfSync(hash, key, salt, info, length) {
 }
 
 async function hkdfDeriveBits(algorithm, baseKey, length) {
-  const { hash } = algorithm;
-  const salt = getArrayBufferOrView(algorithm.salt, 'algorithm.salt');
-  const info = getArrayBufferOrView(algorithm.info, 'algorithm.info');
-  if (hash === undefined)
-    throw new ERR_MISSING_OPTION('algorithm.hash');
+  const { hash, salt, info } = algorithm;
 
   let byteLength = 512 / 8;
   if (length !== undefined) {