From 57f507f1ddb8b9d61a6c3e37ad7aabf0e3fc3d3f Mon Sep 17 00:00:00 2001
From: Filip Skokan <panva.ip@gmail.com>
Date: Wed, 12 Oct 2022 21:18:38 +0200
Subject: [PATCH] crypto: fix webcrypto HMAC "get key length" in deriveKey and
 generateKey

PR-URL: https://github.com/nodejs/node/pull/44917
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 lib/internal/crypto/mac.js                |  4 +--
 lib/internal/crypto/util.js               | 12 ++++-----
 lib/internal/crypto/webcrypto.js          | 12 ++-------
 test/parallel/test-webcrypto-derivekey.js | 31 ++++++++++++++++++++---
 test/parallel/test-webcrypto-keygen.js    |  8 +++---
 5 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js
index 81bdd5f770c5c5..0d83691ba55548 100644
--- a/lib/internal/crypto/mac.js
+++ b/lib/internal/crypto/mac.js
@@ -14,7 +14,7 @@ const {
 } = internalBinding('crypto');
 
 const {
-  getHashLength,
+  getBlockSize,
   hasAnyNotIn,
   jobPromise,
   normalizeHashName,
@@ -54,7 +54,7 @@ async function hmacGenerateKey(algorithm, extractable, keyUsages) {
     throw new ERR_MISSING_OPTION('algorithm.hash');
 
   if (length === undefined)
-    length = getHashLength(hash.name);
+    length = getBlockSize(hash.name);
 
   validateBitLength(length, 'algorithm.length', true);
 
diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js
index f4e2b1ad6fc123..c94725dd82ddcb 100644
--- a/lib/internal/crypto/util.js
+++ b/lib/internal/crypto/util.js
@@ -340,12 +340,12 @@ function getUsagesUnion(usageSet, ...usages) {
   return newset;
 }
 
-function getHashLength(name) {
+function getBlockSize(name) {
   switch (name) {
-    case 'SHA-1': return 160;
-    case 'SHA-256': return 256;
-    case 'SHA-384': return 384;
-    case 'SHA-512': return 512;
+    case 'SHA-1': return 512;
+    case 'SHA-256': return 512;
+    case 'SHA-384': return 1024;
+    case 'SHA-512': return 1024;
   }
 }
 
@@ -430,8 +430,8 @@ module.exports = {
   validateMaxBufferLength,
   bigIntArrayToUnsignedBigInt,
   bigIntArrayToUnsignedInt,
+  getBlockSize,
   getStringOption,
   getUsagesUnion,
-  getHashLength,
   secureHeapUsed,
 };
diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js
index f6e4bd70081c89..f8af06b1adf14f 100644
--- a/lib/internal/crypto/webcrypto.js
+++ b/lib/internal/crypto/webcrypto.js
@@ -51,6 +51,7 @@ const {
 
 const {
   getArrayBufferOrView,
+  getBlockSize,
   hasAnyNotIn,
   lazyRequire,
   normalizeAlgorithm,
@@ -195,16 +196,7 @@ function getKeyLength({ name, length, hash }) {
       return length;
     case 'HMAC':
       if (length === undefined) {
-        switch (hash?.name) {
-          case 'SHA-1':
-            return 160;
-          case 'SHA-256':
-            return 256;
-          case 'SHA-384':
-            return 384;
-          case 'SHA-512':
-            return 512;
-        }
+        return getBlockSize(hash?.name);
       }
 
       if (typeof length === 'number' && length !== 0) {
diff --git a/test/parallel/test-webcrypto-derivekey.js b/test/parallel/test-webcrypto-derivekey.js
index f8eb996000ec89..b819b998d217e0 100644
--- a/test/parallel/test-webcrypto-derivekey.js
+++ b/test/parallel/test-webcrypto-derivekey.js
@@ -124,10 +124,11 @@ const { webcrypto: { subtle }, KeyObject } = require('crypto');
   const vectors = [
     ['PBKDF2', 'deriveKey', 528],
     ['HKDF', 'deriveKey', 528],
-    [{ name: 'HMAC', hash: 'SHA-1' }, 'sign', 160],
-    [{ name: 'HMAC', hash: 'SHA-256' }, 'sign', 256],
-    [{ name: 'HMAC', hash: 'SHA-384' }, 'sign', 384],
-    [{ name: 'HMAC', hash: 'SHA-512' }, 'sign', 512],
+    [{ name: 'HMAC', hash: 'SHA-1' }, 'sign', 512],
+    [{ name: 'HMAC', hash: 'SHA-256' }, 'sign', 512],
+    // Not long enough secret generated by ECDH
+    // [{ name: 'HMAC', hash: 'SHA-384' }, 'sign', 1024],
+    // [{ name: 'HMAC', hash: 'SHA-512' }, 'sign', 1024],
   ];
 
   (async () => {
@@ -151,6 +152,28 @@ const { webcrypto: { subtle }, KeyObject } = require('crypto');
   })().then(common.mustCall());
 }
 
+{
+  const vectors = [
+    [{ name: 'HMAC', hash: 'SHA-1' }, 'sign', 512],
+    [{ name: 'HMAC', hash: 'SHA-256' }, 'sign', 512],
+    [{ name: 'HMAC', hash: 'SHA-384' }, 'sign', 1024],
+    [{ name: 'HMAC', hash: 'SHA-512' }, 'sign', 1024],
+  ];
+
+  (async () => {
+    for (const [derivedKeyAlgorithm, usage, expected] of vectors) {
+      const derived = await subtle.deriveKey(
+        { name: 'PBKDF2', salt: new Uint8Array([]), hash: 'SHA-256', iterations: 20 },
+        await subtle.importKey('raw', new Uint8Array([]), { name: 'PBKDF2' }, false, ['deriveKey']),
+        derivedKeyAlgorithm,
+        false,
+        [usage]);
+
+      assert.strictEqual(derived.algorithm.length, expected);
+    }
+  })().then(common.mustCall());
+}
+
 // Test X25519 and X448 key derivation
 {
   async function test(name) {
diff --git a/test/parallel/test-webcrypto-keygen.js b/test/parallel/test-webcrypto-keygen.js
index 5acea2debdd292..b0977d5935082e 100644
--- a/test/parallel/test-webcrypto-keygen.js
+++ b/test/parallel/test-webcrypto-keygen.js
@@ -551,10 +551,10 @@ const vectors = {
 
     if (length === undefined) {
       switch (hash) {
-        case 'SHA-1': length = 160; break;
-        case 'SHA-256': length = 256; break;
-        case 'SHA-384': length = 384; break;
-        case 'SHA-512': length = 512; break;
+        case 'SHA-1': length = 512; break;
+        case 'SHA-256': length = 512; break;
+        case 'SHA-384': length = 1024; break;
+        case 'SHA-512': length = 1024; break;
       }
     }