crypto: fix webcrypto HMAC "get key length" in deriveKey and generateKey

PR-URL: #44917 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs · Oct 12, 2022 · 214354f · 214354f
1 parent 40a0757
commit 214354f
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 26 deletions.
diff --git 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
@@ -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
@@ -56,6 +56,7 @@ const {
 
 const {
   getArrayBufferOrView,
+  getBlockSize,
   hasAnyNotIn,
   lazyRequire,
   normalizeAlgorithm,
@@ -200,16 +201,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
@@ -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
@@ -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;
       }
     }