From 4df7bc5cb019a44ed3fdc817be58f9d0f76b9fea Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 14 Jun 2022 15:07:50 +0200 Subject: [PATCH 1/5] crypto: fix webcrypto deriveBits for non-byte lengths --- lib/internal/crypto/diffiehellman.js | 8 ++++---- test/parallel/test-webcrypto-derivebits-cfrg.js | 2 +- test/parallel/test-webcrypto-derivebits-ecdh.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js index 85f2be16007c74..036f661fad6be2 100644 --- a/lib/internal/crypto/diffiehellman.js +++ b/lib/internal/crypto/diffiehellman.js @@ -3,7 +3,7 @@ const { ArrayBufferPrototypeSlice, FunctionPrototypeCall, - MathFloor, + MathCeil, ObjectDefineProperty, Promise, SafeSet, @@ -386,9 +386,9 @@ async function asyncDeriveBitsECDH(algorithm, baseKey, length) { if (length === null) return bits; - // If the length is not a multiple of 8, it will be truncated - // down to the nearest multiple of 8. - length = MathFloor(length / 8); + // If the length is not a multiple of 8 the nearest ceiled + // multiple of 8 is sliced. + length = MathCeil(length / 8); const { byteLength } = bits; // If the length is larger than the derived secret, throw. diff --git a/test/parallel/test-webcrypto-derivebits-cfrg.js b/test/parallel/test-webcrypto-derivebits-cfrg.js index ff3e4ef7de5470..2233d1a2d274c7 100644 --- a/test/parallel/test-webcrypto-derivebits-cfrg.js +++ b/test/parallel/test-webcrypto-derivebits-cfrg.js @@ -133,7 +133,7 @@ async function prepareKeys() { assert.strictEqual( Buffer.from(bits).toString('hex'), - result.slice(0, -4)); + result.slice(0, -2)); } })); diff --git a/test/parallel/test-webcrypto-derivebits-ecdh.js b/test/parallel/test-webcrypto-derivebits-ecdh.js index de19ac1cd4e415..826b591d37ca65 100644 --- a/test/parallel/test-webcrypto-derivebits-ecdh.js +++ b/test/parallel/test-webcrypto-derivebits-ecdh.js @@ -154,7 +154,7 @@ async function prepareKeys() { assert.strictEqual( Buffer.from(bits).toString('hex'), - result.slice(0, -4)); + result.slice(0, -2)); } })); From 7bb49887a55744ca4ed96a79611c8c3b8859dd1d Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 14 Jun 2022 16:26:28 +0200 Subject: [PATCH 2/5] crypto: fix webcrypto AES-KW keys accepting encrypt/decrypt usages --- lib/internal/crypto/aes.js | 10 +++++++--- test/parallel/test-webcrypto-keygen.js | 18 ++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js index 2c74a49139a70a..324662e1f8b1b4 100644 --- a/lib/internal/crypto/aes.js +++ b/lib/internal/crypto/aes.js @@ -230,13 +230,17 @@ async function aesGenerateKey(algorithm, extractable, keyUsages) { validateInteger(length, 'algorithm.length'); validateOneOf(length, 'algorithm.length', kAesKeyLengths); - const usageSet = new SafeSet(keyUsages); + const checkUsages = ['wrapKey', 'unwrapKey']; + if (name !== 'AES-KW') + ArrayPrototypePush(checkUsages, 'encrypt', 'decrypt'); - if (hasAnyNotIn(usageSet, ['encrypt', 'decrypt', 'wrapKey', 'unwrapKey'])) { + const usagesSet = new SafeSet(keyUsages); + if (hasAnyNotIn(usagesSet, checkUsages)) { throw lazyDOMException( 'Unsupported key usage for an AES key', 'SyntaxError'); } + return new Promise((resolve, reject) => { generateKey('aes', { length }, (err, key) => { if (err) { @@ -249,7 +253,7 @@ async function aesGenerateKey(algorithm, extractable, keyUsages) { resolve(new InternalCryptoKey( key, { name, length }, - ArrayFrom(usageSet), + ArrayFrom(usagesSet), extractable)); }); }); diff --git a/test/parallel/test-webcrypto-keygen.js b/test/parallel/test-webcrypto-keygen.js index 948c755a9114bc..c3429af99e66a7 100644 --- a/test/parallel/test-webcrypto-keygen.js +++ b/test/parallel/test-webcrypto-keygen.js @@ -211,14 +211,16 @@ const vectors = { if (!vectors[name].usages.includes(usage)) invalidUsages.push(usage); }); - return assert.rejects( - subtle.generateKey( - { - name, ...vectors[name].algorithm - }, - true, - invalidUsages), - { message: /Unsupported key usage/ }); + for (const invalidUsage of invalidUsages) { + await assert.rejects( + subtle.generateKey( + { + name, ...vectors[name].algorithm + }, + true, + [...vectors[name].usages, invalidUsage]), + { message: /Unsupported key usage/ }); + } } const tests = Object.keys(vectors).map(test); From 514e8a5ecffdf5aa7cad432c9dea6856774aabb5 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 14 Jun 2022 14:17:21 +0200 Subject: [PATCH 3/5] crypto: fix webcrypto RSA generateKey() use of publicExponent --- lib/internal/crypto/rsa.js | 2 +- test/parallel/test-webcrypto-keygen.js | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index 27b5803db4f77d..728e4bd951cdc5 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -176,7 +176,7 @@ async function rsaKeyGenerate( return new Promise((resolve, reject) => { generateKeyPair('rsa', { modulusLength, - publicExponentConverted, + publicExponent: publicExponentConverted, }, (err, pubKey, privKey) => { if (err) { return reject(lazyDOMException( diff --git a/test/parallel/test-webcrypto-keygen.js b/test/parallel/test-webcrypto-keygen.js index c3429af99e66a7..32e8fc39ee6d15 100644 --- a/test/parallel/test-webcrypto-keygen.js +++ b/test/parallel/test-webcrypto-keygen.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); @@ -10,8 +11,11 @@ const { types: { isCryptoKey } } = require('util'); const { webcrypto: { subtle, CryptoKey }, createSecretKey, + KeyObject, } = require('crypto'); +const { bigIntArrayToUnsignedBigInt } = require('internal/crypto/util'); + const allUsages = [ 'encrypt', 'decrypt', @@ -264,10 +268,16 @@ const vectors = { assert.strictEqual(publicKey.algorithm.name, name); assert.strictEqual(publicKey.algorithm.modulusLength, modulusLength); assert.deepStrictEqual(publicKey.algorithm.publicExponent, publicExponent); + assert.strictEqual( + KeyObject.from(publicKey).asymmetricKeyDetails.publicExponent, + bigIntArrayToUnsignedBigInt(publicExponent)); assert.strictEqual(publicKey.algorithm.hash.name, hash); assert.strictEqual(privateKey.algorithm.name, name); assert.strictEqual(privateKey.algorithm.modulusLength, modulusLength); assert.deepStrictEqual(privateKey.algorithm.publicExponent, publicExponent); + assert.strictEqual( + KeyObject.from(privateKey).asymmetricKeyDetails.publicExponent, + bigIntArrayToUnsignedBigInt(publicExponent)); assert.strictEqual(privateKey.algorithm.hash.name, hash); // Missing parameters @@ -344,6 +354,17 @@ const vectors = { code: 'ERR_INVALID_ARG_TYPE' }); })); + + await Promise.all([[1], [1, 0, 0]].map((publicExponent) => { + return assert.rejects(subtle.generateKey({ + name, + modulusLength, + publicExponent: new Uint8Array(publicExponent), + hash + }, true, usages), { + name: 'OperationError', + }); + })); } const kTests = [ From 55be639273c0577d9de5a427fa469b45e3a1317d Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 14 Jun 2022 21:28:59 +0200 Subject: [PATCH 4/5] crypto: fix webcrypto digest() invalid algorithm --- lib/internal/crypto/hash.js | 25 ++++++++++++++++++++----- test/parallel/test-webcrypto-digest.js | 6 ++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js index 6c502f52b66503..956053af0a8c2c 100644 --- a/lib/internal/crypto/hash.js +++ b/lib/internal/crypto/hash.js @@ -28,6 +28,10 @@ const { prepareSecretKey, } = require('internal/crypto/keys'); +const { + lazyDOMException, +} = require('internal/util'); + const { Buffer, } = require('buffer'); @@ -171,11 +175,22 @@ async function asyncDigest(algorithm, data) { if (algorithm.length !== undefined) validateUint32(algorithm.length, 'algorithm.length'); - return jobPromise(new HashJob( - kCryptoJobAsync, - normalizeHashName(algorithm.name), - data, - algorithm.length)); + switch (algorithm.name) { + case 'SHA-1': + // Fall through + case 'SHA-256': + // Fall through + case 'SHA-384': + // Fall through + case 'SHA-512': + return jobPromise(new HashJob( + kCryptoJobAsync, + normalizeHashName(algorithm.name), + data, + algorithm.length)); + } + + throw lazyDOMException('Unrecognized name.', 'NotSupportedError'); } module.exports = { diff --git a/test/parallel/test-webcrypto-digest.js b/test/parallel/test-webcrypto-digest.js index 78b12ea6a5574c..b8680564d1a1a3 100644 --- a/test/parallel/test-webcrypto-digest.js +++ b/test/parallel/test-webcrypto-digest.js @@ -168,3 +168,9 @@ async function testDigest(size, name) { await Promise.all(variations); })().then(common.mustCall()); + +(async () => { + await assert.rejects(subtle.digest('RSA-OAEP', Buffer.alloc(1)), { + name: 'NotSupportedError', + }); +})().then(common.mustCall()); From 6de6ec9e996ff78a8b70770aed4755cd1267fa6c Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 14 Jun 2022 21:29:17 +0200 Subject: [PATCH 5/5] crypto: fix webcrypto generateKey() with empty usages --- lib/internal/crypto/webcrypto.js | 5 +++++ test/parallel/test-webcrypto-keygen.js | 9 +++++++++ test/parallel/test-webcrypto-sign-verify-hmac.js | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js index a5f442b0e57cfb..dc2e94aae7770e 100644 --- a/lib/internal/crypto/webcrypto.js +++ b/lib/internal/crypto/webcrypto.js @@ -89,6 +89,11 @@ async function generateKey( algorithm = normalizeAlgorithm(algorithm); validateBoolean(extractable, 'extractable'); validateArray(keyUsages, 'keyUsages'); + if (keyUsages.length === 0) { + throw lazyDOMException( + 'Usages cannot be empty when creating a key', + 'SyntaxError'); + } switch (algorithm.name) { case 'RSASSA-PKCS1-v1_5': // Fall through diff --git a/test/parallel/test-webcrypto-keygen.js b/test/parallel/test-webcrypto-keygen.js index 32e8fc39ee6d15..5d039fdf036344 100644 --- a/test/parallel/test-webcrypto-keygen.js +++ b/test/parallel/test-webcrypto-keygen.js @@ -210,6 +210,15 @@ const vectors = { // Test bad usages { async function test(name) { + await assert.rejects( + subtle.generateKey( + { + name, ...vectors[name].algorithm + }, + true, + []), + { message: /Usages cannot be empty/ }); + const invalidUsages = []; allUsages.forEach((usage) => { if (!vectors[name].usages.includes(usage)) diff --git a/test/parallel/test-webcrypto-sign-verify-hmac.js b/test/parallel/test-webcrypto-sign-verify-hmac.js index 3028816054ca76..0962773b132768 100644 --- a/test/parallel/test-webcrypto-sign-verify-hmac.js +++ b/test/parallel/test-webcrypto-sign-verify-hmac.js @@ -153,7 +153,7 @@ async function testSign({ hash, } await assert.rejects( - subtle.generateKey({ name }, false, []), { + subtle.generateKey({ name }, false, ['sign', 'verify']), { name: 'TypeError', code: 'ERR_MISSING_OPTION', message: 'algorithm.hash is required'