crypto: ensure invalid SubtleCrypto JWK data import results in DataError

PR-URL: #55041 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs · Oct 4, 2024 · d3f8c35 · d3f8c35
1 parent 452bc9b
commit d3f8c35
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 20 deletions.
diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js
@@ -295,7 +295,12 @@ async function aesImportKey(
       }
 
       const handle = new KeyObjectHandle();
-      handle.initJwk(keyData);
+      try {
+        handle.initJwk(keyData);
+      } catch (err) {
+        throw lazyDOMException(
+          'Invalid keyData', { name: 'DataError', cause: err });
+      }
 
       ({ length } = handle.keyDetail({ }));
       validateKeyLength(length);

diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js
@@ -17,6 +17,12 @@ const {
   kSignJobModeVerify,
 } = internalBinding('crypto');
 
+const {
+  codes: {
+    ERR_CRYPTO_INVALID_JWK,
+  },
+} = require('internal/errors');
+
 const {
   getUsagesUnion,
   hasAnyNotIn,
@@ -277,22 +283,26 @@ async function cfrgImportKey(
         isPublic,
         usagesSet);
 
-      const publicKeyObject = createCFRGRawKey(
-        name,
-        Buffer.from(keyData.x, 'base64'),
-        true);
-
-      if (isPublic) {
-        keyObject = publicKeyObject;
-      } else {
-        keyObject = createCFRGRawKey(
+      try {
+        const publicKeyObject = createCFRGRawKey(
           name,
-          Buffer.from(keyData.d, 'base64'),
-          false);
-
-        if (!createPublicKey(keyObject).equals(publicKeyObject)) {
-          throw lazyDOMException('Invalid JWK', 'DataError');
+          Buffer.from(keyData.x, 'base64'),
+          true);
+
+        if (isPublic) {
+          keyObject = publicKeyObject;
+        } else {
+          keyObject = createCFRGRawKey(
+            name,
+            Buffer.from(keyData.d, 'base64'),
+            false);
+
+          if (!createPublicKey(keyObject).equals(publicKeyObject)) {
+            throw new ERR_CRYPTO_INVALID_JWK();
+          }
         }
+      } catch (err) {
+        throw lazyDOMException('Invalid keyData', { name: 'DataError', cause: err });
       }
       break;
     }

diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
@@ -240,9 +240,15 @@ async function ecImportKey(
       }
 
       const handle = new KeyObjectHandle();
-      const type = handle.initJwk(keyData, namedCurve);
+      let type;
+      try {
+        type = handle.initJwk(keyData, namedCurve);
+      } catch (err) {
+        throw lazyDOMException(
+          'Invalid keyData', { name: 'DataError', cause: err });
+      }
       if (type === undefined)
-        throw lazyDOMException('Invalid JWK', 'DataError');
+        throw lazyDOMException('Invalid keyData', 'DataError');
       keyObject = type === kKeyTypePrivate ?
         new PrivateKeyObject(handle) :
         new PublicKeyObject(handle);

diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js
@@ -145,7 +145,12 @@ async function hmacImportKey(
       }
 
       const handle = new KeyObjectHandle();
-      handle.initJwk(keyData);
+      try {
+        handle.initJwk(keyData);
+      } catch (err) {
+        throw lazyDOMException(
+          'Invalid keyData', { name: 'DataError', cause: err });
+      }
       keyObject = new SecretKeyObject(handle);
       break;
     }

diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js
@@ -275,9 +275,15 @@ async function rsaImportKey(
       }
 
       const handle = new KeyObjectHandle();
-      const type = handle.initJwk(keyData);
+      let type;
+      try {
+        type = handle.initJwk(keyData);
+      } catch (err) {
+        throw lazyDOMException(
+          'Invalid keyData', { name: 'DataError', cause: err });
+      }
       if (type === undefined)
-        throw lazyDOMException('Invalid JWK', 'DataError');
+        throw lazyDOMException('Invalid keyData', 'DataError');
 
       keyObject = type === kKeyTypePrivate ?
         new PrivateKeyObject(handle) :

diff --git a/test/parallel/test-webcrypto-export-import-cfrg.js b/test/parallel/test-webcrypto-export-import-cfrg.js
@@ -322,6 +322,15 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
       extractable,
       [/* empty usages */]),
     { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
+
+  await assert.rejects(
+    subtle.importKey(
+      'jwk',
+      { kty: jwk.kty, /* missing x */ crv: jwk.crv },
+      { name },
+      extractable,
+      publicUsages),
+    { name: 'DataError', message: 'Invalid keyData' });
 }
 
 async function testImportRaw({ name, publicUsages }) {

diff --git a/test/parallel/test-webcrypto-export-import-ec.js b/test/parallel/test-webcrypto-export-import-ec.js
@@ -330,6 +330,15 @@ async function testImportJwk(
       extractable,
       [/* empty usages */]),
     { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
+
+  await assert.rejects(
+    subtle.importKey(
+      'jwk',
+      { kty: jwk.kty, /* missing x */ y: jwk.y, crv: jwk.crv },
+      { name, namedCurve },
+      extractable,
+      publicUsages),
+    { name: 'DataError', message: 'Invalid keyData' });
 }
 
 async function testImportRaw({ name, publicUsages }, namedCurve) {

diff --git a/test/parallel/test-webcrypto-export-import-rsa.js b/test/parallel/test-webcrypto-export-import-rsa.js
@@ -513,6 +513,15 @@ async function testImportJwk(
       extractable,
       [/* empty usages */]),
     { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
+
+  await assert.rejects(
+    subtle.importKey(
+      'jwk',
+      { kty: jwk.kty, /* missing e */ n: jwk.n },
+      { name, hash },
+      extractable,
+      publicUsages),
+    { name: 'DataError', message: 'Invalid keyData' });
 }
 
 // combinations to test