From f979ef34f7f468979392e0bb0666a81834782535 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sat, 8 Oct 2022 01:05:54 +0200 Subject: [PATCH] crypto: fix webcrypto HMAC "get key length" in deriveKey and generateKey --- lib/internal/crypto/mac.js | 4 +-- lib/internal/crypto/pbkdf2.js | 24 ++++++--------- lib/internal/crypto/util.js | 12 ++++---- lib/internal/crypto/webcrypto.js | 12 ++------ test/parallel/test-webcrypto-derivekey.js | 37 ++++++++++++++++++++--- test/parallel/test-webcrypto-keygen.js | 8 ++--- 6 files changed, 57 insertions(+), 40 deletions(-) diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js index 15b3378e2eda64..c97d6ff974e27b 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/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index a9b5b1590f3b21..f7090f3e74c1cc 100644 --- a/lib/internal/crypto/pbkdf2.js +++ b/lib/internal/crypto/pbkdf2.js @@ -116,23 +116,19 @@ async function pbkdf2DeriveBits(algorithm, baseKey, length) { const raw = baseKey[kKeyObject].export(); - let byteLength = 64; // the default - if (length !== undefined) { - if (length === 0) - throw lazyDOMException('length cannot be zero', 'OperationError'); - if (length === null) - throw lazyDOMException('length cannot be null', 'OperationError'); - validateUint32(length, 'length'); - if (length % 8) { - throw lazyDOMException( - 'length must be a multiple of 8', - 'OperationError'); - } - byteLength = length / 8; + if (length === 0) + throw lazyDOMException('length cannot be zero', 'OperationError'); + if (length === null) + throw lazyDOMException('length cannot be null', 'OperationError'); + validateUint32(length, 'length'); + if (length % 8) { + throw lazyDOMException( + 'length must be a multiple of 8', + 'OperationError'); } return new Promise((resolve, reject) => { - pbkdf2(raw, salt, iterations, byteLength, hash, (err, result) => { + pbkdf2(raw, salt, iterations, length / 8, hash, (err, result) => { if (err) return reject(err); resolve(result.buffer); }); diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index dbd816cff99e9f..ec0e05390281cd 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -341,12 +341,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; } } @@ -431,8 +431,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 736137d406b7b0..72474954c47105 100644 --- 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 index f8eb996000ec89..e5c59d4ca50486 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,34 @@ 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]); + + if (derived.algorithm.name === 'HMAC') { + assert.strictEqual(derived.algorithm.length, expected); + } else { + // KDFs cannot be exportable and do not indicate their length + const secretKey = KeyObject.from(derived); + assert.strictEqual(secretKey.symmetricKeySize, expected / 8); + } + } + })().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 7d266e601b7295..8a94b09d3e2b88 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; } }