From 291d9e9936b4051977b30e75914f585102b27c27 Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Wed, 10 Feb 2021 12:22:52 +0100
Subject: [PATCH] crypto: check ed/x webcrypto key import algorithm names

PR-URL: https://github.com/nodejs/node/pull/37305
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/webcrypto.md                          |  4 +-
 lib/internal/crypto/ec.js                     | 54 ++++++++++++++++---
 test/parallel/test-webcrypto-ed25519-ed448.js | 54 ++++++++++++++++++-
 test/parallel/test-webcrypto-x25519-x448.js   | 27 ++++++++--
 4 files changed, 125 insertions(+), 14 deletions(-)

diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md
index 79bf1c75c5152f..e3ed1405355ad5 100644
--- a/doc/api/webcrypto.md
+++ b/doc/api/webcrypto.md
@@ -1697,8 +1697,8 @@ added: v15.8.0
 
 * Type: {boolean}
 
-The `public` parameter is used to specify that the key is to be interpreted
-as a public key.
+The `public` parameter is used to specify that the `'raw'` format key is to be
+interpreted as a public key. **Default:** `false`.
 
 ### `NODE-SCRYPT` Algorithm
 <!-- YAML
diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
index 68ef854d7eb4f8..a12ec3e505a757 100644
--- a/lib/internal/crypto/ec.js
+++ b/lib/internal/crypto/ec.js
@@ -253,10 +253,6 @@ async function ecImportKey(
     namedCurve,
     'algorithm.namedCurve',
     ObjectKeys(kNamedCurveAliases));
-  // Only used for NODE-EDnnnn key variants to distinguish between
-  // importing a raw public key or raw private key.
-  if (algorithm.public !== undefined)
-    validateBoolean(algorithm.public, 'algorithm.public');
   let keyObject;
   const usagesSet = new SafeSet(keyUsages);
   let checkNamedCurve = true;
@@ -266,6 +262,24 @@ async function ecImportKey(
         throw new ERR_INVALID_ARG_TYPE('keyData', 'KeyObject', keyData);
       if (keyData.type === 'secret')
         throw lazyDOMException('Invalid key type', 'InvalidAccessException');
+
+      switch (namedCurve) {
+        case 'NODE-X25519':
+          // Fall through
+        case 'NODE-X448':
+          checkNamedCurve = false;
+          if (algorithm.name !== 'ECDH')
+            throw lazyDOMException('Invalid algorithm name.', 'DataError');
+          break;
+        case 'NODE-ED25519':
+          // Fall through
+        case 'NODE-ED448':
+          checkNamedCurve = false;
+          if (algorithm.name !== namedCurve)
+            throw lazyDOMException('Invalid algorithm name.', 'DataError');
+          break;
+      }
+
       verifyAcceptableEcKeyUse(name, keyData.type, usagesSet);
       keyObject = keyData;
       break;
@@ -296,8 +310,24 @@ async function ecImportKey(
         case 'OKP': {
           checkNamedCurve = false;
           const isPublic = keyData.d === undefined;
-          const type =
-            namedCurve === 'NODE-X25519' || 'NODE-X448' ? 'ECDH' : 'ECDSA';
+
+          let type;
+          switch (namedCurve) {
+            case 'NODE-ED25519':
+              // Fall through
+            case 'NODE-ED448':
+              type = `NODE-${keyData.crv.toUpperCase()}`;
+              break;
+            case 'NODE-X25519':
+              // Fall through
+            case 'NODE-X448':
+              type = 'ECDH';
+              break;
+          }
+
+          if (algorithm.name !== type)
+            throw lazyDOMException('Invalid algorithm name.', 'DataError');
+
           verifyAcceptableEcKeyUse(
             type,
             isPublic ? 'public' : 'private',
@@ -364,8 +394,12 @@ async function ecImportKey(
           // Fall through
         case 'NODE-X448':
           checkNamedCurve = false;
+          if (algorithm.public !== undefined)
+            validateBoolean(algorithm.public, 'algorithm.public');
+          if (algorithm.name !== 'ECDH')
+            throw lazyDOMException('Invalid algorithm name.', 'DataError');
           verifyAcceptableEcKeyUse(
-            'ECDH',
+            algorithm.name,
             algorithm.public === true ? 'public' : 'private',
             usagesSet);
           keyObject = createECRawKey(namedCurve, keyData, algorithm.public);
@@ -374,8 +408,12 @@ async function ecImportKey(
           // Fall through
         case 'NODE-ED448':
           checkNamedCurve = false;
+          if (algorithm.public !== undefined)
+            validateBoolean(algorithm.public, 'algorithm.public');
+          if (algorithm.name !== namedCurve)
+            throw lazyDOMException('Invalid algorithm name.', 'DataError');
           verifyAcceptableEcKeyUse(
-            'ECDSA',
+            algorithm.name,
             algorithm.public === true ? 'public' : 'private',
             usagesSet);
           keyObject = createECRawKey(namedCurve, keyData, algorithm.public);
diff --git a/test/parallel/test-webcrypto-ed25519-ed448.js b/test/parallel/test-webcrypto-ed25519-ed448.js
index b06de24c78f682..c13bf05d5cee9c 100644
--- a/test/parallel/test-webcrypto-ed25519-ed448.js
+++ b/test/parallel/test-webcrypto-ed25519-ed448.js
@@ -6,7 +6,10 @@ if (!common.hasCrypto)
   common.skip('missing crypto');
 
 const assert = require('assert');
-const { subtle } = require('crypto').webcrypto;
+const {
+  generateKeyPairSync,
+  webcrypto: { subtle }
+} = require('crypto');
 
 async function generateKey(namedCurve) {
   return subtle.generateKey(
@@ -365,3 +368,52 @@ assert.rejects(
   {
     message: /Unsupported named curves for ECDSA/
   });
+
+{
+  for (const asymmetricKeyType of ['ed25519', 'ed448']) {
+    const { publicKey, privateKey } = generateKeyPairSync(asymmetricKeyType);
+    for (const keyObject of [publicKey, privateKey]) {
+      const namedCurve = `NODE-${asymmetricKeyType.toUpperCase()}`;
+      subtle.importKey(
+        'node.keyObject',
+        keyObject,
+        { name: namedCurve, namedCurve },
+        true,
+        keyObject.type === 'private' ? ['sign'] : ['verify'],
+      ).then((cryptoKey) => {
+        assert.strictEqual(cryptoKey.type, keyObject.type);
+        assert.strictEqual(cryptoKey.algorithm.name, namedCurve);
+      }, common.mustNotCall());
+
+      assert.rejects(
+        subtle.importKey(
+          'node.keyObject',
+          keyObject,
+          {
+            name: 'ECDSA',
+            namedCurve,
+          },
+          true,
+          keyObject.type === 'private' ? ['sign'] : ['verify']
+        ),
+        {
+          message: /Invalid algorithm name/
+        });
+
+      assert.rejects(
+        subtle.importKey(
+          'node.keyObject',
+          keyObject,
+          {
+            name: 'ECDH',
+            namedCurve,
+          },
+          true,
+          keyObject.type === 'private' ? ['deriveBits', 'deriveKey'] : [],
+        ),
+        {
+          message: /Invalid algorithm name/
+        });
+    }
+  }
+}
diff --git a/test/parallel/test-webcrypto-x25519-x448.js b/test/parallel/test-webcrypto-x25519-x448.js
index 322c6b7b013b21..84f8b88f254261 100644
--- a/test/parallel/test-webcrypto-x25519-x448.js
+++ b/test/parallel/test-webcrypto-x25519-x448.js
@@ -6,7 +6,11 @@ if (!common.hasCrypto)
   common.skip('missing crypto');
 
 const assert = require('assert');
-const { subtle } = require('crypto').webcrypto;
+
+const {
+  generateKeyPairSync,
+  webcrypto: { subtle }
+} = require('crypto');
 
 // X25519 and X448 are ECDH named curves that should work
 // with the existing ECDH mechanisms with no additional
@@ -260,7 +264,7 @@ assert.rejects(
       namedCurve: 'NODE-X25519'
     },
     true,
-    ['deriveBits']).then(common.mustCall());
+    ['deriveBits']).then(common.mustCall(), common.mustNotCall());
 
   // Public JWK import
   subtle.importKey(
@@ -275,5 +279,22 @@ assert.rejects(
       namedCurve: 'NODE-X25519'
     },
     true,
-    []).then(common.mustCall());
+    []).then(common.mustCall(), common.mustNotCall());
+
+  for (const asymmetricKeyType of ['x25519', 'x448']) {
+    const { publicKey, privateKey } = generateKeyPairSync(asymmetricKeyType);
+    for (const keyObject of [publicKey, privateKey]) {
+      const namedCurve = `NODE-${asymmetricKeyType.toUpperCase()}`;
+      subtle.importKey(
+        'node.keyObject',
+        keyObject,
+        { name: 'ECDH', namedCurve },
+        true,
+        keyObject.type === 'private' ? ['deriveBits', 'deriveKey'] : [],
+      ).then((cryptoKey) => {
+        assert.strictEqual(cryptoKey.type, keyObject.type);
+        assert.strictEqual(cryptoKey.algorithm.name, 'ECDH');
+      }, common.mustNotCall());
+    }
+  }
 }