From 0395543f61a8daee6b4e58afaeed20e3c65414c2 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 5 May 2023 12:44:41 +0200 Subject: [PATCH 1/2] crypto: fix webcrypto private/secret import with empty usages Refs: #47864 --- lib/internal/crypto/webcrypto.js | 29 ++++++++++++++----- .../test-webcrypto-sign-verify-ecdsa.js | 13 --------- .../test-webcrypto-sign-verify-eddsa.js | 13 --------- .../test-webcrypto-sign-verify-hmac.js | 2 +- .../test-webcrypto-sign-verify-rsa.js | 13 --------- 5 files changed, 23 insertions(+), 47 deletions(-) diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js index 4a29913d9dd8ba..9c1cf1e5d91dda 100644 --- a/lib/internal/crypto/webcrypto.js +++ b/lib/internal/crypto/webcrypto.js @@ -605,19 +605,22 @@ async function importKey( }); algorithm = normalizeAlgorithm(algorithm, 'importKey'); + let result; switch (algorithm.name) { case 'RSASSA-PKCS1-v1_5': // Fall through case 'RSA-PSS': // Fall through case 'RSA-OAEP': - return require('internal/crypto/rsa') + result = await require('internal/crypto/rsa') .rsaImportKey(format, keyData, algorithm, extractable, keyUsages); + break; case 'ECDSA': // Fall through case 'ECDH': - return require('internal/crypto/ec') + result = await require('internal/crypto/ec') .ecImportKey(format, keyData, algorithm, extractable, keyUsages); + break; case 'Ed25519': // Fall through case 'Ed448': @@ -625,11 +628,13 @@ async function importKey( case 'X25519': // Fall through case 'X448': - return require('internal/crypto/cfrg') + result = await require('internal/crypto/cfrg') .cfrgImportKey(format, keyData, algorithm, extractable, keyUsages); + break; case 'HMAC': - return require('internal/crypto/mac') + result = await require('internal/crypto/mac') .hmacImportKey(format, keyData, algorithm, extractable, keyUsages); + break; case 'AES-CTR': // Fall through case 'AES-CBC': @@ -637,20 +642,30 @@ async function importKey( case 'AES-GCM': // Fall through case 'AES-KW': - return require('internal/crypto/aes') + result = await require('internal/crypto/aes') .aesImportKey(algorithm, format, keyData, extractable, keyUsages); + break; case 'HKDF': // Fall through case 'PBKDF2': - return importGenericSecretKey( + result = await importGenericSecretKey( algorithm, format, keyData, extractable, keyUsages); + break; + default: + throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError'); } - throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError'); + if ((result.type === 'secret' || result.type === 'private') && result.usages.length === 0) { + throw lazyDOMException( + `Usages cannot be empty when importing a ${result.type} key.`, + 'SyntaxError'); + } + + return result; } // subtle.wrapKey() is essentially a subtle.exportKey() followed diff --git a/test/parallel/test-webcrypto-sign-verify-ecdsa.js b/test/parallel/test-webcrypto-sign-verify-ecdsa.js index 7794b4bb8a8115..072485cca59f7f 100644 --- a/test/parallel/test-webcrypto-sign-verify-ecdsa.js +++ b/test/parallel/test-webcrypto-sign-verify-ecdsa.js @@ -149,7 +149,6 @@ async function testSign({ name, plaintext }) { const [ publicKey, - noSignPrivateKey, privateKey, hmacKey, rsaKeys, @@ -161,12 +160,6 @@ async function testSign({ name, { name, namedCurve }, false, ['verify']), - subtle.importKey( - 'pkcs8', - privateKeyBuffer, - { name, namedCurve }, - false, - [ /* No usages */ ]), subtle.importKey( 'pkcs8', privateKeyBuffer, @@ -214,12 +207,6 @@ async function testSign({ name, message: /Unable to use this key to sign/ }); - // Test failure when no sign usage - await assert.rejects( - subtle.sign({ name, hash }, noSignPrivateKey, plaintext), { - message: /Unable to use this key to sign/ - }); - // Test failure when using the wrong algorithms await assert.rejects( subtle.sign({ name, hash }, hmacKey, plaintext), { diff --git a/test/parallel/test-webcrypto-sign-verify-eddsa.js b/test/parallel/test-webcrypto-sign-verify-eddsa.js index 7f16d62c2ea71c..4f49677b355d07 100644 --- a/test/parallel/test-webcrypto-sign-verify-eddsa.js +++ b/test/parallel/test-webcrypto-sign-verify-eddsa.js @@ -131,7 +131,6 @@ async function testSign({ name, data }) { const [ publicKey, - noSignPrivateKey, privateKey, hmacKey, rsaKeys, @@ -143,12 +142,6 @@ async function testSign({ name, { name }, false, ['verify']), - subtle.importKey( - 'pkcs8', - privateKeyBuffer, - { name }, - false, - [ /* No usages */ ]), subtle.importKey( 'pkcs8', privateKeyBuffer, @@ -197,12 +190,6 @@ async function testSign({ name, message: /Unable to use this key to sign/ }); - // Test failure when no sign usage - await assert.rejects( - subtle.sign({ name }, noSignPrivateKey, data), { - message: /Unable to use this key to sign/ - }); - // Test failure when using the wrong algorithms await assert.rejects( subtle.sign({ name }, hmacKey, data), { diff --git a/test/parallel/test-webcrypto-sign-verify-hmac.js b/test/parallel/test-webcrypto-sign-verify-hmac.js index 98cf61f5c9d179..00b742dbfea5d1 100644 --- a/test/parallel/test-webcrypto-sign-verify-hmac.js +++ b/test/parallel/test-webcrypto-sign-verify-hmac.js @@ -31,7 +31,7 @@ async function testVerify({ hash, keyBuffer, { name, hash }, false, - [ /* No usages */ ]), + ['sign']), subtle.generateKey( { name: 'RSA-PSS', diff --git a/test/parallel/test-webcrypto-sign-verify-rsa.js b/test/parallel/test-webcrypto-sign-verify-rsa.js index 6057d2f6ef9325..9074b5104efe82 100644 --- a/test/parallel/test-webcrypto-sign-verify-rsa.js +++ b/test/parallel/test-webcrypto-sign-verify-rsa.js @@ -132,7 +132,6 @@ async function testSign({ }) { const [ publicKey, - noSignPrivateKey, privateKey, hmacKey, ecdsaKeys, @@ -143,12 +142,6 @@ async function testSign({ { name: algorithm.name, hash }, false, ['verify']), - subtle.importKey( - 'pkcs8', - privateKeyBuffer, - { name: algorithm.name, hash }, - false, - [ /* No usages */ ]), subtle.importKey( 'pkcs8', privateKeyBuffer, @@ -189,12 +182,6 @@ async function testSign({ message: /Unable to use this key to sign/ }); - // Test failure when no sign usage - await assert.rejects( - subtle.sign(algorithm, noSignPrivateKey, plaintext), { - message: /Unable to use this key to sign/ - }); - // Test failure when using the wrong algorithms await assert.rejects( subtle.sign(algorithm, hmacKey, plaintext), { From 2a2d9afec069038ee2092d7b3e1bd24a8910c9b5 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 5 May 2023 18:54:25 +0200 Subject: [PATCH 2/2] fixup! crypto: fix webcrypto private/secret import with empty usages --- .../test-webcrypto-export-import-cfrg.js | 18 ++++++++++++++ .../test-webcrypto-export-import-ec.js | 18 ++++++++++++++ .../test-webcrypto-export-import-rsa.js | 18 ++++++++++++++ test/parallel/test-webcrypto-export-import.js | 24 +++++++++++++++++++ 4 files changed, 78 insertions(+) diff --git a/test/parallel/test-webcrypto-export-import-cfrg.js b/test/parallel/test-webcrypto-export-import-cfrg.js index 13634f93e4a9f4..7cec317a8347b6 100644 --- a/test/parallel/test-webcrypto-export-import-cfrg.js +++ b/test/parallel/test-webcrypto-export-import-cfrg.js @@ -164,6 +164,15 @@ async function testImportPkcs8({ name, privateUsages }, extractable) { message: /key is not extractable/ }); } + + await assert.rejects( + subtle.importKey( + 'pkcs8', + keyData[name].pkcs8, + { name }, + extractable, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); } async function testImportJwk({ name, publicUsages, privateUsages }, extractable) { @@ -311,6 +320,15 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable) publicUsages), { message: 'JWK "crv" Parameter and algorithm name mismatch' }); } + + await assert.rejects( + subtle.importKey( + 'jwk', + { ...jwk }, + { name }, + extractable, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); } async function testImportRaw({ name, publicUsages }) { diff --git a/test/parallel/test-webcrypto-export-import-ec.js b/test/parallel/test-webcrypto-export-import-ec.js index 9e551633797482..e17ff0c2d4075c 100644 --- a/test/parallel/test-webcrypto-export-import-ec.js +++ b/test/parallel/test-webcrypto-export-import-ec.js @@ -164,6 +164,15 @@ async function testImportPkcs8( message: /key is not extractable/ }); } + + await assert.rejects( + subtle.importKey( + 'pkcs8', + keyData[namedCurve].pkcs8, + { name, namedCurve }, + extractable, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); } async function testImportJwk( @@ -312,6 +321,15 @@ async function testImportJwk( privateUsages), { message: 'JWK "crv" does not match the requested algorithm' }); } + + await assert.rejects( + subtle.importKey( + 'jwk', + { ...jwk }, + { name, namedCurve }, + extractable, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); } async function testImportRaw({ name, publicUsages }, namedCurve) { diff --git a/test/parallel/test-webcrypto-export-import-rsa.js b/test/parallel/test-webcrypto-export-import-rsa.js index 218f27683b4618..c18abf9832f365 100644 --- a/test/parallel/test-webcrypto-export-import-rsa.js +++ b/test/parallel/test-webcrypto-export-import-rsa.js @@ -361,6 +361,15 @@ async function testImportPkcs8( message: /key is not extractable/ }); } + + await assert.rejects( + subtle.importKey( + 'pkcs8', + keyData[size].pkcs8, + { name, hash }, + extractable, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); } async function testImportJwk( @@ -495,6 +504,15 @@ async function testImportJwk( privateUsages), { message: 'JWK "alg" does not match the requested algorithm' }); } + + await assert.rejects( + subtle.importKey( + 'jwk', + { ...jwk }, + { name, hash }, + extractable, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a private key.' }); } // combinations to test diff --git a/test/parallel/test-webcrypto-export-import.js b/test/parallel/test-webcrypto-export-import.js index 52fbbbd3ec8d34..e7d45dbc5efeea 100644 --- a/test/parallel/test-webcrypto-export-import.js +++ b/test/parallel/test-webcrypto-export-import.js @@ -95,6 +95,18 @@ const { subtle } = globalThis.crypto; assert.deepStrictEqual( Buffer.from(jwk.k, 'base64').toString('hex'), Buffer.from(raw).toString('hex')); + + await assert.rejects( + subtle.importKey( + 'raw', + keyData, + { + name: 'HMAC', + hash: 'SHA-256' + }, + true, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a secret key.' }); } test().then(common.mustCall()); @@ -125,6 +137,18 @@ const { subtle } = globalThis.crypto; assert.deepStrictEqual( Buffer.from(jwk.k, 'base64').toString('hex'), Buffer.from(raw).toString('hex')); + + await assert.rejects( + subtle.importKey( + 'raw', + keyData, + { + name: 'AES-CTR', + length: 256, + }, + true, + [/* empty usages */]), + { name: 'SyntaxError', message: 'Usages cannot be empty when importing a secret key.' }); } test().then(common.mustCall());