From 84db3e7b06979a388a65d8ebce2571554c2dadd6 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sat, 25 Jun 2022 08:22:35 +0300 Subject: [PATCH] crypto: handle webcrypto generateKey() usages edge case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/43454 Reviewed-By: Tobias Nießen --- lib/internal/crypto/webcrypto.js | 40 +++++++++---- .../test-webcrypto-derivebits-ecdh.js | 2 +- .../parallel/test-webcrypto-derivekey-ecdh.js | 2 +- test/parallel/test-webcrypto-keygen.js | 59 +++++++++++-------- 4 files changed, 65 insertions(+), 38 deletions(-) diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js index dc2e94aae7770e..b7760efaa234c8 100644 --- a/lib/internal/crypto/webcrypto.js +++ b/lib/internal/crypto/webcrypto.js @@ -89,19 +89,18 @@ 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'); - } + let result; + let resultType; switch (algorithm.name) { case 'RSASSA-PKCS1-v1_5': // Fall through case 'RSA-PSS': // Fall through case 'RSA-OAEP': - return lazyRequire('internal/crypto/rsa') + resultType = 'CryptoKeyPair'; + result = await lazyRequire('internal/crypto/rsa') .rsaKeyGenerate(algorithm, extractable, keyUsages); + break; case 'Ed25519': // Fall through case 'Ed448': @@ -109,16 +108,22 @@ async function generateKey( case 'X25519': // Fall through case 'X448': - return lazyRequire('internal/crypto/cfrg') + resultType = 'CryptoKeyPair'; + result = await lazyRequire('internal/crypto/cfrg') .cfrgGenerateKey(algorithm, extractable, keyUsages); + break; case 'ECDSA': // Fall through case 'ECDH': - return lazyRequire('internal/crypto/ec') + resultType = 'CryptoKeyPair'; + result = await lazyRequire('internal/crypto/ec') .ecGenerateKey(algorithm, extractable, keyUsages); + break; case 'HMAC': - return lazyRequire('internal/crypto/mac') + resultType = 'CryptoKey'; + result = await lazyRequire('internal/crypto/mac') .hmacGenerateKey(algorithm, extractable, keyUsages); + break; case 'AES-CTR': // Fall through case 'AES-CBC': @@ -126,11 +131,26 @@ async function generateKey( case 'AES-GCM': // Fall through case 'AES-KW': - return lazyRequire('internal/crypto/aes') + resultType = 'CryptoKey'; + result = await lazyRequire('internal/crypto/aes') .aesGenerateKey(algorithm, extractable, keyUsages); + break; default: throw lazyDOMException('Unrecognized name.'); } + + if ( + (resultType === 'CryptoKey' && + (result.type === 'secret' || result.type === 'private') && + result.usages.length === 0) || + (resultType === 'CryptoKeyPair' && result.privateKey.usages.length === 0) + ) { + throw lazyDOMException( + 'Usages cannot be empty when creating a key.', + 'SyntaxError'); + } + + return result; } async function deriveBits(algorithm, baseKey, length) { diff --git a/test/parallel/test-webcrypto-derivebits-ecdh.js b/test/parallel/test-webcrypto-derivebits-ecdh.js index 826b591d37ca65..739155fba47818 100644 --- a/test/parallel/test-webcrypto-derivebits-ecdh.js +++ b/test/parallel/test-webcrypto-derivebits-ecdh.js @@ -201,7 +201,7 @@ async function prepareKeys() { { name: 'ECDSA', namedCurve: 'P-521' - }, false, ['verify']); + }, false, ['sign', 'verify']); await assert.rejects(subtle.deriveBits({ name: 'ECDH', diff --git a/test/parallel/test-webcrypto-derivekey-ecdh.js b/test/parallel/test-webcrypto-derivekey-ecdh.js index 203a0c5677fbc0..2ca54957a55304 100644 --- a/test/parallel/test-webcrypto-derivekey-ecdh.js +++ b/test/parallel/test-webcrypto-derivekey-ecdh.js @@ -165,7 +165,7 @@ async function prepareKeys() { namedCurve: 'P-521' }, false, - ['verify']); + ['sign', 'verify']); await assert.rejects( subtle.deriveKey( diff --git a/test/parallel/test-webcrypto-keygen.js b/test/parallel/test-webcrypto-keygen.js index 5d039fdf036344..e61d7c9b913abf 100644 --- a/test/parallel/test-webcrypto-keygen.js +++ b/test/parallel/test-webcrypto-keygen.js @@ -29,49 +29,49 @@ const allUsages = [ const vectors = { 'AES-CTR': { algorithm: { length: 256 }, + result: 'CryptoKey', usages: [ 'encrypt', 'decrypt', 'wrapKey', 'unwrapKey', ], - mandatoryUsages: [] }, 'AES-CBC': { algorithm: { length: 256 }, + result: 'CryptoKey', usages: [ 'encrypt', 'decrypt', 'wrapKey', 'unwrapKey', ], - mandatoryUsages: [] }, 'AES-GCM': { algorithm: { length: 256 }, + result: 'CryptoKey', usages: [ 'encrypt', 'decrypt', 'wrapKey', 'unwrapKey', ], - mandatoryUsages: [] }, 'AES-KW': { algorithm: { length: 256 }, + result: 'CryptoKey', usages: [ 'wrapKey', 'unwrapKey', ], - mandatoryUsages: [] }, 'HMAC': { algorithm: { length: 256, hash: 'SHA-256' }, + result: 'CryptoKey', usages: [ 'sign', 'verify', ], - mandatoryUsages: [] }, 'RSASSA-PKCS1-v1_5': { algorithm: { @@ -79,11 +79,11 @@ const vectors = { publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256' }, + result: 'CryptoKeyPair', usages: [ 'sign', 'verify', ], - mandatoryUsages: ['sign'], }, 'RSA-PSS': { algorithm: { @@ -91,11 +91,11 @@ const vectors = { publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256' }, + result: 'CryptoKeyPair', usages: [ 'sign', 'verify', ], - mandatoryUsages: ['sign'] }, 'RSA-OAEP': { algorithm: { @@ -103,69 +103,57 @@ const vectors = { publicExponent: new Uint8Array([1, 0, 1]), hash: 'SHA-256' }, + result: 'CryptoKeyPair', usages: [ 'encrypt', 'decrypt', 'wrapKey', 'unwrapKey', ], - mandatoryUsages: [ - 'decrypt', - 'unwrapKey', - ] }, 'ECDSA': { algorithm: { namedCurve: 'P-521' }, + result: 'CryptoKeyPair', usages: [ 'sign', 'verify', ], - mandatoryUsages: ['sign'] }, 'ECDH': { algorithm: { namedCurve: 'P-521' }, + result: 'CryptoKeyPair', usages: [ 'deriveKey', 'deriveBits', ], - mandatoryUsages: [ - 'deriveKey', - 'deriveBits', - ] }, 'Ed25519': { + result: 'CryptoKeyPair', usages: [ 'sign', 'verify', ], - mandatoryUsages: ['sign'] }, 'Ed448': { + result: 'CryptoKeyPair', usages: [ 'sign', 'verify', ], - mandatoryUsages: ['sign'] }, 'X25519': { + result: 'CryptoKeyPair', usages: [ 'deriveKey', 'deriveBits', ], - mandatoryUsages: [ - 'deriveKey', - 'deriveBits', - ] }, 'X448': { + result: 'CryptoKeyPair', usages: [ 'deriveKey', 'deriveBits', ], - mandatoryUsages: [ - 'deriveKey', - 'deriveBits', - ] }, }; @@ -219,6 +207,25 @@ const vectors = { []), { message: /Usages cannot be empty/ }); + // For CryptoKeyPair results the private key + // usages must not be empty. + // - ECDH(-like) algorithm key pairs only have private key usages + // - Signing algorithm key pairs may pass a non-empty array but + // with only a public key usage + if ( + vectors[name].result === 'CryptoKeyPair' && + vectors[name].usages.includes('verify') + ) { + await assert.rejects( + subtle.generateKey( + { + name, ...vectors[name].algorithm + }, + true, + ['verify']), + { message: /Usages cannot be empty/ }); + } + const invalidUsages = []; allUsages.forEach((usage) => { if (!vectors[name].usages.includes(usage))