crypto: fix webcrypto private/secret import with empty usages

Refs: #47864 PR-URL: #47877 Refs: #47864 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · May 12, 2023 · 09fb74a · 09fb74a
1 parent 7e34f77
commit 09fb74a
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 47 deletions.
diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js
@@ -605,52 +605,67 @@ async function importKey(
   });
 
   algorithm = normalizeAlgorithm(algorithm, 'importKey');
+  let result;
   switch (algorithm.name) {
     case 'RSASSA-PKCS1-v1_5':
       // Fall through
     case 'RSA-PSS':
       // Fall through
     case 'RSA-OAEP':
-      return require('internal/crypto/rsa')
+      result = await require('internal/crypto/rsa')
         .rsaImportKey(format, keyData, algorithm, extractable, keyUsages);
+      break;
     case 'ECDSA':
       // Fall through
     case 'ECDH':
-      return require('internal/crypto/ec')
+      result = await require('internal/crypto/ec')
         .ecImportKey(format, keyData, algorithm, extractable, keyUsages);
+      break;
     case 'Ed25519':
       // Fall through
     case 'Ed448':
       // Fall through
     case 'X25519':
       // Fall through
     case 'X448':
-      return require('internal/crypto/cfrg')
+      result = await require('internal/crypto/cfrg')
         .cfrgImportKey(format, keyData, algorithm, extractable, keyUsages);
+      break;
     case 'HMAC':
-      return require('internal/crypto/mac')
+      result = await require('internal/crypto/mac')
         .hmacImportKey(format, keyData, algorithm, extractable, keyUsages);
+      break;
     case 'AES-CTR':
       // Fall through
     case 'AES-CBC':
       // Fall through
     case 'AES-GCM':
       // Fall through
     case 'AES-KW':
-      return require('internal/crypto/aes')
+      result = await require('internal/crypto/aes')
         .aesImportKey(algorithm, format, keyData, extractable, keyUsages);
+      break;
     case 'HKDF':
       // Fall through
     case 'PBKDF2':
-      return importGenericSecretKey(
+      result = await importGenericSecretKey(
         algorithm,
         format,
         keyData,
         extractable,
         keyUsages);
+      break;
+    default:
+      throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError');
   }
 
-  throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError');
+  if ((result.type === 'secret' || result.type === 'private') && result.usages.length === 0) {
+    throw lazyDOMException(
+      `Usages cannot be empty when importing a ${result.type} key.`,
+      'SyntaxError');
+  }
+
+  return result;
 }
 
 // subtle.wrapKey() is essentially a subtle.exportKey() followed

diff --git a/test/parallel/test-webcrypto-export-import-cfrg.js b/test/parallel/test-webcrypto-export-import-cfrg.js
@@ -164,6 +164,15 @@ async function testImportPkcs8({ name, privateUsages }, extractable) {
         message: /key is not extractable/
       });
   }
+
+  await assert.rejects(
+    subtle.importKey(
+      'pkcs8',
+      keyData[name].pkcs8,
+      { name },
+      extractable,
+      [/* empty usages */]),
+    { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
 }
 
 async function testImportJwk({ name, publicUsages, privateUsages }, extractable) {
@@ -311,6 +320,15 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
         publicUsages),
       { message: 'JWK "crv" Parameter and algorithm name mismatch' });
   }
+
+  await assert.rejects(
+    subtle.importKey(
+      'jwk',
+      { ...jwk },
+      { name },
+      extractable,
+      [/* empty usages */]),
+    { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
 }
 
 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
@@ -164,6 +164,15 @@ async function testImportPkcs8(
         message: /key is not extractable/
       });
   }
+
+  await assert.rejects(
+    subtle.importKey(
+      'pkcs8',
+      keyData[namedCurve].pkcs8,
+      { name, namedCurve },
+      extractable,
+      [/* empty usages */]),
+    { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
 }
 
 async function testImportJwk(
@@ -312,6 +321,15 @@ async function testImportJwk(
         privateUsages),
       { message: 'JWK "crv" does not match the requested algorithm' });
   }
+
+  await assert.rejects(
+    subtle.importKey(
+      'jwk',
+      { ...jwk },
+      { name, namedCurve },
+      extractable,
+      [/* empty usages */]),
+    { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
 }
 
 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
@@ -361,6 +361,15 @@ async function testImportPkcs8(
         message: /key is not extractable/
       });
   }
+
+  await assert.rejects(
+    subtle.importKey(
+      'pkcs8',
+      keyData[size].pkcs8,
+      { name, hash },
+      extractable,
+      [/* empty usages */]),
+    { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
 }
 
 async function testImportJwk(
@@ -495,6 +504,15 @@ async function testImportJwk(
         privateUsages),
       { message: 'JWK "alg" does not match the requested algorithm' });
   }
+
+  await assert.rejects(
+    subtle.importKey(
+      'jwk',
+      { ...jwk },
+      { name, hash },
+      extractable,
+      [/* empty usages */]),
+    { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' });
 }
 
 // combinations to test

diff --git a/test/parallel/test-webcrypto-export-import.js b/test/parallel/test-webcrypto-export-import.js
@@ -95,6 +95,18 @@ const { subtle } = globalThis.crypto;
     assert.deepStrictEqual(
       Buffer.from(jwk.k, 'base64').toString('hex'),
       Buffer.from(raw).toString('hex'));
+
+    await assert.rejects(
+      subtle.importKey(
+        'raw',
+        keyData,
+        {
+          name: 'HMAC',
+          hash: 'SHA-256'
+        },
+        true,
+        [/* empty usages */]),
+      { name: 'SyntaxError', message: 'Usages cannot be empty when importing a secret key.' });
   }
 
   test().then(common.mustCall());
@@ -125,6 +137,18 @@ const { subtle } = globalThis.crypto;
     assert.deepStrictEqual(
       Buffer.from(jwk.k, 'base64').toString('hex'),
       Buffer.from(raw).toString('hex'));
+
+    await assert.rejects(
+      subtle.importKey(
+        'raw',
+        keyData,
+        {
+          name: 'AES-CTR',
+          length: 256,
+        },
+        true,
+        [/* empty usages */]),
+      { name: 'SyntaxError', message: 'Usages cannot be empty when importing a secret key.' });
   }
 
   test().then(common.mustCall());

diff --git a/test/parallel/test-webcrypto-sign-verify-ecdsa.js b/test/parallel/test-webcrypto-sign-verify-ecdsa.js
@@ -149,7 +149,6 @@ async function testSign({ name,
                           plaintext }) {
   const [
     publicKey,
-    noSignPrivateKey,
     privateKey,
     hmacKey,
     rsaKeys,
@@ -161,12 +160,6 @@ async function testSign({ name,
       { name, namedCurve },
       false,
       ['verify']),
-    subtle.importKey(
-      'pkcs8',
-      privateKeyBuffer,
-      { name, namedCurve },
-      false,
-      [ /* No usages */ ]),
     subtle.importKey(
       'pkcs8',
       privateKeyBuffer,
@@ -214,12 +207,6 @@ async function testSign({ name,
       message: /Unable to use this key to sign/
     });
 
-  // Test failure when no sign usage
-  await assert.rejects(
-    subtle.sign({ name, hash }, noSignPrivateKey, plaintext), {
-      message: /Unable to use this key to sign/
-    });
-
   // Test failure when using the wrong algorithms
   await assert.rejects(
     subtle.sign({ name, hash }, hmacKey, plaintext), {

diff --git a/test/parallel/test-webcrypto-sign-verify-eddsa.js b/test/parallel/test-webcrypto-sign-verify-eddsa.js
@@ -131,7 +131,6 @@ async function testSign({ name,
                           data }) {
   const [
     publicKey,
-    noSignPrivateKey,
     privateKey,
     hmacKey,
     rsaKeys,
@@ -143,12 +142,6 @@ async function testSign({ name,
       { name },
       false,
       ['verify']),
-    subtle.importKey(
-      'pkcs8',
-      privateKeyBuffer,
-      { name },
-      false,
-      [ /* No usages */ ]),
     subtle.importKey(
       'pkcs8',
       privateKeyBuffer,
@@ -197,12 +190,6 @@ async function testSign({ name,
       message: /Unable to use this key to sign/
     });
 
-  // Test failure when no sign usage
-  await assert.rejects(
-    subtle.sign({ name }, noSignPrivateKey, data), {
-      message: /Unable to use this key to sign/
-    });
-
   // Test failure when using the wrong algorithms
   await assert.rejects(
     subtle.sign({ name }, hmacKey, data), {

diff --git a/test/parallel/test-webcrypto-sign-verify-hmac.js b/test/parallel/test-webcrypto-sign-verify-hmac.js
@@ -31,7 +31,7 @@ async function testVerify({ hash,
       keyBuffer,
       { name, hash },
       false,
-      [ /* No usages */ ]),
+      ['sign']),
     subtle.generateKey(
       {
         name: 'RSA-PSS',

diff --git a/test/parallel/test-webcrypto-sign-verify-rsa.js b/test/parallel/test-webcrypto-sign-verify-rsa.js
@@ -132,7 +132,6 @@ async function testSign({
 }) {
   const [
     publicKey,
-    noSignPrivateKey,
     privateKey,
     hmacKey,
     ecdsaKeys,
@@ -143,12 +142,6 @@ async function testSign({
       { name: algorithm.name, hash },
       false,
       ['verify']),
-    subtle.importKey(
-      'pkcs8',
-      privateKeyBuffer,
-      { name: algorithm.name, hash },
-      false,
-      [ /* No usages */ ]),
     subtle.importKey(
       'pkcs8',
       privateKeyBuffer,
@@ -189,12 +182,6 @@ async function testSign({
       message: /Unable to use this key to sign/
     });
 
-  // Test failure when no sign usage
-  await assert.rejects(
-    subtle.sign(algorithm, noSignPrivateKey, plaintext), {
-      message: /Unable to use this key to sign/
-    });
-
   // Test failure when using the wrong algorithms
   await assert.rejects(
     subtle.sign(algorithm, hmacKey, plaintext), {