From bc175683b309a25275f7e5ca56951666d75827d0 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 18 Jan 2023 19:07:26 +0100 Subject: [PATCH 01/12] crypto: add CryptoKey Symbol.toStringTag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #45987 PR-URL: https://github.com/nodejs/node/pull/46042 Fixes: https://github.com/nodejs/node/issues/45987 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Reviewed-By: Juan José Arboleda Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/keys.js | 7 +++++++ test/parallel/test-webcrypto-keygen.js | 8 ++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 4303dc44fe60d7..a6612e3b2a74ee 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -6,6 +6,7 @@ const { ObjectDefineProperty, ObjectSetPrototypeOf, Symbol, + SymbolToStringTag, Uint8Array, } = primordials; @@ -697,6 +698,12 @@ class CryptoKey extends JSTransferable { } } +ObjectDefineProperty(CryptoKey.prototype, SymbolToStringTag, { + __proto__: null, + configurable: true, + value: 'CryptoKey', +}); + // All internal code must use new InternalCryptoKey to create // CryptoKey instances. The CryptoKey class is exposed to end // user code but is not permitted to be constructed directly. diff --git a/test/parallel/test-webcrypto-keygen.js b/test/parallel/test-webcrypto-keygen.js index 5acea2debdd292..a18ab84f89ba63 100644 --- a/test/parallel/test-webcrypto-keygen.js +++ b/test/parallel/test-webcrypto-keygen.js @@ -277,6 +277,8 @@ const vectors = { assert.strictEqual(publicKey.type, 'public'); assert.strictEqual(privateKey.type, 'private'); + assert.strictEqual(publicKey.toString(), '[object CryptoKey]'); + assert.strictEqual(privateKey.toString(), '[object CryptoKey]'); assert.strictEqual(publicKey.extractable, true); assert.strictEqual(privateKey.extractable, true); assert.deepStrictEqual(publicKey.usages, publicUsages); @@ -439,6 +441,8 @@ const vectors = { assert.strictEqual(publicKey.type, 'public'); assert.strictEqual(privateKey.type, 'private'); + assert.strictEqual(publicKey.toString(), '[object CryptoKey]'); + assert.strictEqual(privateKey.toString(), '[object CryptoKey]'); assert.strictEqual(publicKey.extractable, true); assert.strictEqual(privateKey.extractable, true); assert.deepStrictEqual(publicKey.usages, publicUsages); @@ -503,6 +507,7 @@ const vectors = { assert(isCryptoKey(key)); assert.strictEqual(key.type, 'secret'); + assert.strictEqual(key.toString(), '[object CryptoKey]'); assert.strictEqual(key.extractable, true); assert.deepStrictEqual(key.usages, usages); assert.strictEqual(key.algorithm.name, name); @@ -562,6 +567,7 @@ const vectors = { assert(isCryptoKey(key)); assert.strictEqual(key.type, 'secret'); + assert.strictEqual(key.toString(), '[object CryptoKey]'); assert.strictEqual(key.extractable, true); assert.deepStrictEqual(key.usages, usages); assert.strictEqual(key.algorithm.name, 'HMAC'); @@ -629,6 +635,8 @@ assert.throws(() => new CryptoKey(), { code: 'ERR_ILLEGAL_CONSTRUCTOR' }); assert.strictEqual(publicKey.type, 'public'); assert.strictEqual(privateKey.type, 'private'); + assert.strictEqual(publicKey.toString(), '[object CryptoKey]'); + assert.strictEqual(privateKey.toString(), '[object CryptoKey]'); assert.strictEqual(publicKey.extractable, true); assert.strictEqual(privateKey.extractable, true); assert.deepStrictEqual(publicKey.usages, publicUsages); From cca5940b35675820caaacf90a0dbc989697c7597 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 3 Jan 2023 17:34:41 +0100 Subject: [PATCH 02/12] crypto: add KeyObject Symbol.toStringTag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/46043 Reviewed-By: Antoine du Hamel Reviewed-By: Yagiz Nizipli Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/keys.js | 6 ++++++ test/parallel/test-crypto-key-objects.js | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index a6612e3b2a74ee..b26c85c1b59b8d 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -137,6 +137,12 @@ const { } } + ObjectDefineProperty(KeyObject.prototype, SymbolToStringTag, { + __proto__: null, + configurable: true, + value: 'KeyObject', + }); + class SecretKeyObject extends KeyObject { constructor(handle) { super('secret', handle); diff --git a/test/parallel/test-crypto-key-objects.js b/test/parallel/test-crypto-key-objects.js index 93e7cca3fbbdbd..84e32de6ba1ae9 100644 --- a/test/parallel/test-crypto-key-objects.js +++ b/test/parallel/test-crypto-key-objects.js @@ -69,6 +69,7 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem', const keybuf = randomBytes(32); const key = createSecretKey(keybuf); assert.strictEqual(key.type, 'secret'); + assert.strictEqual(key.toString(), '[object KeyObject]'); assert.strictEqual(key.symmetricKeySize, 32); assert.strictEqual(key.asymmetricKeyType, undefined); assert.strictEqual(key.asymmetricKeyDetails, undefined); @@ -150,27 +151,32 @@ const privateDsa = fixtures.readKey('dsa_private_encrypted_1025.pem', const publicKey = createPublicKey(publicPem); assert.strictEqual(publicKey.type, 'public'); + assert.strictEqual(publicKey.toString(), '[object KeyObject]'); assert.strictEqual(publicKey.asymmetricKeyType, 'rsa'); assert.strictEqual(publicKey.symmetricKeySize, undefined); const privateKey = createPrivateKey(privatePem); assert.strictEqual(privateKey.type, 'private'); + assert.strictEqual(privateKey.toString(), '[object KeyObject]'); assert.strictEqual(privateKey.asymmetricKeyType, 'rsa'); assert.strictEqual(privateKey.symmetricKeySize, undefined); // It should be possible to derive a public key from a private key. const derivedPublicKey = createPublicKey(privateKey); assert.strictEqual(derivedPublicKey.type, 'public'); + assert.strictEqual(derivedPublicKey.toString(), '[object KeyObject]'); assert.strictEqual(derivedPublicKey.asymmetricKeyType, 'rsa'); assert.strictEqual(derivedPublicKey.symmetricKeySize, undefined); const publicKeyFromJwk = createPublicKey({ key: publicJwk, format: 'jwk' }); assert.strictEqual(publicKey.type, 'public'); + assert.strictEqual(publicKey.toString(), '[object KeyObject]'); assert.strictEqual(publicKey.asymmetricKeyType, 'rsa'); assert.strictEqual(publicKey.symmetricKeySize, undefined); const privateKeyFromJwk = createPrivateKey({ key: jwk, format: 'jwk' }); assert.strictEqual(privateKey.type, 'private'); + assert.strictEqual(privateKey.toString(), '[object KeyObject]'); assert.strictEqual(privateKey.asymmetricKeyType, 'rsa'); assert.strictEqual(privateKey.symmetricKeySize, undefined); From c7c5011a75b2c293394155077bde26c846400b1f Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 7 Nov 2022 07:35:35 +0100 Subject: [PATCH 03/12] crypto: handle more webcrypto errors with OperationError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45320 Reviewed-By: Yagiz Nizipli Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/aes.js | 8 ++++---- lib/internal/crypto/cfrg.js | 4 ++-- lib/internal/crypto/diffiehellman.js | 6 +++++- lib/internal/crypto/ec.js | 4 ++-- lib/internal/crypto/hash.js | 2 +- lib/internal/crypto/mac.js | 2 +- lib/internal/crypto/rsa.js | 6 +++--- lib/internal/crypto/util.js | 11 ++++++++--- test/parallel/test-webcrypto-digest.js | 2 +- 9 files changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js index 79ca61c5ff2097..e716caedd050de 100644 --- a/lib/internal/crypto/aes.js +++ b/lib/internal/crypto/aes.js @@ -124,7 +124,7 @@ function asyncAesCtrCipher(mode, key, data, { counter, length }) { 'OperationError'); } - return jobPromise(new AESCipherJob( + return jobPromise(() => new AESCipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], @@ -137,7 +137,7 @@ function asyncAesCtrCipher(mode, key, data, { counter, length }) { function asyncAesCbcCipher(mode, key, data, { iv }) { iv = getArrayBufferOrView(iv, 'algorithm.iv'); validateByteLength(iv, 'algorithm.iv', 16); - return jobPromise(new AESCipherJob( + return jobPromise(() => new AESCipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], @@ -147,7 +147,7 @@ function asyncAesCbcCipher(mode, key, data, { iv }) { } function asyncAesKwCipher(mode, key, data) { - return jobPromise(new AESCipherJob( + return jobPromise(() => new AESCipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], @@ -201,7 +201,7 @@ function asyncAesGcmCipher( break; } - return jobPromise(new AESCipherJob( + return jobPromise(() => new AESCipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index 17358c7ccca23d..98e236052f555e 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -194,7 +194,7 @@ async function cfrgGenerateKey(algorithm, extractable, keyUsages) { function cfrgExportKey(key, format) { emitExperimentalWarning(`The ${key.algorithm.name} Web Crypto API algorithm`); - return jobPromise(new ECKeyExportJob( + return jobPromise(() => new ECKeyExportJob( kCryptoJobAsync, format, key[kKeyObject][kHandle])); @@ -323,7 +323,7 @@ function eddsaSignVerify(key, data, { name, context }, signature) { } } - return jobPromise(new SignJob( + return jobPromise(() => new SignJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js index 62107a31a4f9e8..ac37e45a8b6b99 100644 --- a/lib/internal/crypto/diffiehellman.js +++ b/lib/internal/crypto/diffiehellman.js @@ -377,7 +377,11 @@ async function asyncDeriveBitsECDH(algorithm, baseKey, length) { key.algorithm.name === 'ECDH' ? baseKey.algorithm.namedCurve : baseKey.algorithm.name, key[kKeyObject][kHandle], baseKey[kKeyObject][kHandle], (err, bits) => { - if (err) return reject(err); + if (err) { + return reject(lazyDOMException( + 'The operation failed for an operation-specific reason', + 'OperationError')); + } resolve(bits); }); }); diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index ed7484dbbb596e..e39d443941dba7 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -151,7 +151,7 @@ async function ecGenerateKey(algorithm, extractable, keyUsages) { } function ecExportKey(key, format) { - return jobPromise(new ECKeyExportJob( + return jobPromise(() => new ECKeyExportJob( kCryptoJobAsync, format, key[kKeyObject][kHandle])); @@ -286,7 +286,7 @@ function ecdsaSignVerify(key, data, { name, hash }, signature) { throw new ERR_MISSING_OPTION('algorithm.hash'); const hashname = normalizeHashName(hash.name); - return jobPromise(new SignJob( + return jobPromise(() => new SignJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], diff --git a/lib/internal/crypto/hash.js b/lib/internal/crypto/hash.js index 956053af0a8c2c..72d127b37cbeb6 100644 --- a/lib/internal/crypto/hash.js +++ b/lib/internal/crypto/hash.js @@ -183,7 +183,7 @@ async function asyncDigest(algorithm, data) { case 'SHA-384': // Fall through case 'SHA-512': - return jobPromise(new HashJob( + return jobPromise(() => new HashJob( kCryptoJobAsync, normalizeHashName(algorithm.name), data, diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js index 15b3378e2eda64..f08ed611142bc0 100644 --- a/lib/internal/crypto/mac.js +++ b/lib/internal/crypto/mac.js @@ -184,7 +184,7 @@ async function hmacImportKey( function hmacSignVerify(key, data, algorithm, signature) { const mode = signature === undefined ? kSignJobModeSign : kSignJobModeVerify; - return jobPromise(new HmacJob( + return jobPromise(() => new HmacJob( kCryptoJobAsync, mode, normalizeHashName(key.algorithm.hash.name), diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index fa6928e4d78f06..3fe7f4fef4aef3 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -116,7 +116,7 @@ function rsaOaepCipher(mode, key, data, { label }) { validateMaxBufferLength(label, 'algorithm.label'); } - return jobPromise(new RSACipherJob( + return jobPromise(() => new RSACipherJob( kCryptoJobAsync, mode, key[kKeyObject][kHandle], @@ -224,7 +224,7 @@ async function rsaKeyGenerate( } function rsaExportKey(key, format) { - return jobPromise(new RSAKeyExportJob( + return jobPromise(() => new RSAKeyExportJob( kCryptoJobAsync, format, key[kKeyObject][kHandle], @@ -347,7 +347,7 @@ function rsaSignVerify(key, data, { saltLength }, signature) { if (key.type !== type) throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError'); - return jobPromise(new SignJob( + return jobPromise(() => new SignJob( kCryptoJobAsync, signature === undefined ? kSignJobModeSign : kSignJobModeVerify, key[kKeyObject][kHandle], diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js index dbd816cff99e9f..b6b84c441ad742 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -287,10 +287,15 @@ function onDone(resolve, reject, err, result) { resolve(result); } -function jobPromise(job) { +function jobPromise(getJob) { return new Promise((resolve, reject) => { - job.ondone = FunctionPrototypeBind(onDone, job, resolve, reject); - job.run(); + try { + const job = getJob(); + job.ondone = FunctionPrototypeBind(onDone, job, resolve, reject); + job.run(); + } catch (err) { + onDone(resolve, reject, err); + } }); } diff --git a/test/parallel/test-webcrypto-digest.js b/test/parallel/test-webcrypto-digest.js index b8680564d1a1a3..25b78500b7d346 100644 --- a/test/parallel/test-webcrypto-digest.js +++ b/test/parallel/test-webcrypto-digest.js @@ -83,7 +83,7 @@ Promise.all([1, [], {}, null, undefined].map((i) => // addition to the API, and is added as a support for future additional // hash algorithms that support variable digest output lengths. assert.rejects(subtle.digest({ name: 'SHA-512', length: 510 }, kData), { - message: /Digest method not supported/ + name: 'OperationError', }).then(common.mustCall()); const kSourceData = { From 5b9738aee676e9254a823e8f1c56d28a66b35ad0 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 31 Mar 2023 13:52:31 +0200 Subject: [PATCH 04/12] test: mock structuredClone for WebCryptoAPI WPTs --- test/wpt/test-webcrypto.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/wpt/test-webcrypto.js b/test/wpt/test-webcrypto.js index f90ffa9ad5316b..7c8469fc9e413f 100644 --- a/test/wpt/test-webcrypto.js +++ b/test/wpt/test-webcrypto.js @@ -13,6 +13,8 @@ runner.setFlags(['--experimental-global-webcrypto']); runner.setInitScript(` global.location = {}; + const { serialize, deserialize } = require('v8'); + global.structuredClone = (value) => deserialize(serialize(value)); `); runner.runJsTests(); From ae600b9bcfada99f576bb937e4e6446c6af7af95 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Wed, 12 Oct 2022 21:18:38 +0200 Subject: [PATCH 05/12] crypto: fix webcrypto HMAC "get key length" in deriveKey and generateKey PR-URL: https://github.com/nodejs/node/pull/44917 Reviewed-By: James M Snell Reviewed-By: Antoine du Hamel Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/mac.js | 4 +-- lib/internal/crypto/util.js | 12 ++++----- lib/internal/crypto/webcrypto.js | 12 ++------- test/parallel/test-webcrypto-derivekey.js | 31 ++++++++++++++++++++--- test/parallel/test-webcrypto-keygen.js | 8 +++--- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/lib/internal/crypto/mac.js b/lib/internal/crypto/mac.js index f08ed611142bc0..074e3ba535db72 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/util.js b/lib/internal/crypto/util.js index b6b84c441ad742..831fdd597eeede 100644 --- a/lib/internal/crypto/util.js +++ b/lib/internal/crypto/util.js @@ -346,12 +346,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; } } @@ -436,8 +436,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 df4bb072043def..bfc36bbb20a98b 100644 --- a/lib/internal/crypto/webcrypto.js +++ b/lib/internal/crypto/webcrypto.js @@ -51,6 +51,7 @@ const { const { getArrayBufferOrView, + getBlockSize, hasAnyNotIn, lazyRequire, normalizeAlgorithm, @@ -195,16 +196,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..b819b998d217e0 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,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 index a18ab84f89ba63..946a1e58889122 100644 --- a/test/parallel/test-webcrypto-keygen.js +++ b/test/parallel/test-webcrypto-keygen.js @@ -556,10 +556,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; } } From 252839a8e01bd5e113dd13f0c1153fc955f3903f Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 21 Nov 2022 23:09:57 +0100 Subject: [PATCH 06/12] crypto: ensure "x" is present when importing private CFRG webcrypto keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45569 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/cfrg.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index 98e236052f555e..7b825fc93e0669 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -276,6 +276,10 @@ async function cfrgImportKey( } } + if (!isPublic && typeof keyData.x !== 'string') { + throw lazyDOMException('Invalid JWK keyData', 'DataError'); + } + verifyAcceptableCfrgKeyUse( name, isPublic ? 'public' : 'private', From a3212380a873d2ccbad25cf2f86dad2da9fd3eed Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 21 Nov 2022 23:14:06 +0100 Subject: [PATCH 07/12] crypto: fix X25519 and X448 webcrypto public CryptoKey usages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45569 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/cfrg.js | 9 ++++++++- test/parallel/test-webcrypto-derivebits-cfrg.js | 8 ++++---- test/parallel/test-webcrypto-derivekey-cfrg.js | 12 ++++++------ test/parallel/test-webcrypto-export-import-cfrg.js | 12 ++++++------ 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index 7b825fc93e0669..37fc5c5fcf2b8e 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -53,7 +53,14 @@ function verifyAcceptableCfrgKeyUse(name, type, usages) { case 'X25519': // Fall through case 'X448': - checkSet = ['deriveKey', 'deriveBits']; + switch (type) { + case 'private': + checkSet = ['deriveKey', 'deriveBits']; + break; + case 'public': + checkSet = []; + break; + } break; case 'Ed25519': // Fall through diff --git a/test/parallel/test-webcrypto-derivebits-cfrg.js b/test/parallel/test-webcrypto-derivebits-cfrg.js index 2233d1a2d274c7..1324a5fecc4c87 100644 --- a/test/parallel/test-webcrypto-derivebits-cfrg.js +++ b/test/parallel/test-webcrypto-derivebits-cfrg.js @@ -52,7 +52,7 @@ async function prepareKeys() { Buffer.from(spki, 'hex'), { name }, true, - ['deriveKey', 'deriveBits']), + []), ]); keys[name] = { privateKey, @@ -180,7 +180,7 @@ async function prepareKeys() { name: 'X448', public: keys.X448.publicKey }, keys.X448.publicKey, null), { - message: /baseKey must be a private key/ + name: 'InvalidAccessError' }); } @@ -190,7 +190,7 @@ async function prepareKeys() { name: 'X448', public: keys.X448.privateKey }, keys.X448.publicKey, null), { - message: /algorithm\.public must be a public key/ + name: 'InvalidAccessError' }); } @@ -207,7 +207,7 @@ async function prepareKeys() { name: 'X448', public: key }, keys.X448.publicKey, null), { - message: /algorithm\.public must be a public key/ + name: 'InvalidAccessError' }); } })().then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-derivekey-cfrg.js b/test/parallel/test-webcrypto-derivekey-cfrg.js index ddf51626a89b4b..90289c98efd5d5 100644 --- a/test/parallel/test-webcrypto-derivekey-cfrg.js +++ b/test/parallel/test-webcrypto-derivekey-cfrg.js @@ -51,7 +51,7 @@ async function prepareKeys() { Buffer.from(spki, 'hex'), { name }, true, - ['deriveKey', 'deriveBits']), + []), ]); keys[name] = { privateKey, @@ -150,20 +150,20 @@ async function prepareKeys() { }, keys.X448.publicKey, ...otherArgs), - { message: /baseKey must be a private key/ }); + { name: 'InvalidAccessError' }); } { - // Base key is not a private key + // Public is not a public key await assert.rejects( subtle.deriveKey( { name: 'X448', public: keys.X448.privateKey }, - keys.X448.publicKey, + keys.X448.privateKey, ...otherArgs), - { message: /algorithm\.public must be a public key/ }); + { name: 'InvalidAccessError' }); } { @@ -183,6 +183,6 @@ async function prepareKeys() { }, keys.X448.publicKey, ...otherArgs), - { message: /algorithm\.public must be a public key/ }); + { name: 'InvalidAccessError' }); } })().then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-export-import-cfrg.js b/test/parallel/test-webcrypto-export-import-cfrg.js index 6d162ac61c2e30..af6a7956e6716a 100644 --- a/test/parallel/test-webcrypto-export-import-cfrg.js +++ b/test/parallel/test-webcrypto-export-import-cfrg.js @@ -315,19 +315,19 @@ async function testImportRaw({ name, publicUsages }) { const rsaPrivate = crypto.createPrivateKey( fixtures.readKey('rsa_private_2048.pem')); - for (const [name, [publicUsage, privateUsage]] of Object.entries({ - 'Ed25519': ['verify', 'sign'], - 'X448': ['deriveBits', 'deriveBits'], - })) { + for (const [name, publicUsages, privateUsages] of [ + ['Ed25519', ['verify'], ['sign']], + ['X448', [], ['deriveBits']], + ]) { assert.rejects(subtle.importKey( 'spki', rsaPublic.export({ format: 'der', type: 'spki' }), { name }, - true, [publicUsage]), { message: /Invalid key type/ }); + true, publicUsages), { message: /Invalid key type/ }); assert.rejects(subtle.importKey( 'pkcs8', rsaPrivate.export({ format: 'der', type: 'pkcs8' }), { name }, - true, [privateUsage]), { message: /Invalid key type/ }); + true, privateUsages), { message: /Invalid key type/ }); } } From b72552c052f0933d808ff3694565cab83b38a639 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 21 Nov 2022 23:43:57 +0100 Subject: [PATCH 08/12] crypto: use DataError for webcrypto keyData import failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45569 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/cfrg.js | 38 ++++++++++++++++++++++------------- lib/internal/crypto/ec.js | 40 ++++++++++++++++++++++++------------- lib/internal/crypto/rsa.js | 30 ++++++++++++++++++---------- 3 files changed, 70 insertions(+), 38 deletions(-) diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index 37fc5c5fcf2b8e..6e9066d2cdb8fe 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -90,26 +90,26 @@ function createCFRGRawKey(name, keyData, isPublic) { case 'X25519': if (keyData.byteLength !== 32) { throw lazyDOMException( - `${name} raw keys must be exactly 32-bytes`); + `${name} raw keys must be exactly 32-bytes`, 'DataError'); } break; case 'Ed448': if (keyData.byteLength !== 57) { throw lazyDOMException( - `${name} raw keys must be exactly 57-bytes`); + `${name} raw keys must be exactly 57-bytes`, 'DataError'); } break; case 'X448': if (keyData.byteLength !== 56) { throw lazyDOMException( - `${name} raw keys must be exactly 56-bytes`); + `${name} raw keys must be exactly 56-bytes`, 'DataError'); } break; } const keyType = isPublic ? kKeyTypePublic : kKeyTypePrivate; if (!handle.initEDRaw(name, keyData, keyType)) { - throw lazyDOMException('Failure to generate key object'); + throw lazyDOMException('Invalid keyData', 'DataError'); } return isPublic ? new PublicKeyObject(handle) : new PrivateKeyObject(handle); @@ -221,20 +221,30 @@ async function cfrgImportKey( switch (format) { case 'spki': { verifyAcceptableCfrgKeyUse(name, 'public', usagesSet); - keyObject = createPublicKey({ - key: keyData, - format: 'der', - type: 'spki' - }); + try { + keyObject = createPublicKey({ + key: keyData, + format: 'der', + type: 'spki' + }); + } catch { + throw lazyDOMException( + 'Invalid keyData', 'DataError'); + } break; } case 'pkcs8': { verifyAcceptableCfrgKeyUse(name, 'private', usagesSet); - keyObject = createPrivateKey({ - key: keyData, - format: 'der', - type: 'pkcs8' - }); + try { + keyObject = createPrivateKey({ + key: keyData, + format: 'der', + type: 'pkcs8' + }); + } catch { + throw lazyDOMException( + 'Invalid keyData', 'DataError'); + } break; } case 'jwk': { diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index e39d443941dba7..aea9bc9410d4ec 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -80,8 +80,12 @@ function verifyAcceptableEcKeyUse(name, type, usages) { function createECPublicKeyRaw(namedCurve, keyData) { const handle = new KeyObjectHandle(); keyData = getArrayBufferOrView(keyData, 'keyData'); - if (handle.initECRaw(kNamedCurveAliases[namedCurve], keyData)) - return new PublicKeyObject(handle); + + if (!handle.initECRaw(kNamedCurveAliases[namedCurve], keyData)) { + throw lazyDOMException('Invalid keyData', 'DataError'); + } + + return new PublicKeyObject(handle); } async function ecGenerateKey(algorithm, extractable, keyUsages) { @@ -177,20 +181,30 @@ async function ecImportKey( switch (format) { case 'spki': { verifyAcceptableEcKeyUse(name, 'public', usagesSet); - keyObject = createPublicKey({ - key: keyData, - format: 'der', - type: 'spki' - }); + try { + keyObject = createPublicKey({ + key: keyData, + format: 'der', + type: 'spki' + }); + } catch { + throw lazyDOMException( + 'Invalid keyData', 'DataError'); + } break; } case 'pkcs8': { verifyAcceptableEcKeyUse(name, 'private', usagesSet); - keyObject = createPrivateKey({ - key: keyData, - format: 'der', - type: 'pkcs8' - }); + try { + keyObject = createPrivateKey({ + key: keyData, + format: 'der', + type: 'pkcs8' + }); + } catch { + throw lazyDOMException( + 'Invalid keyData', 'DataError'); + } break; } case 'jwk': { @@ -247,8 +261,6 @@ async function ecImportKey( case 'raw': { verifyAcceptableEcKeyUse(name, 'public', usagesSet); keyObject = createECPublicKeyRaw(namedCurve, keyData); - if (keyObject === undefined) - throw lazyDOMException('Unable to import EC key', 'OperationError'); break; } } diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js index 3fe7f4fef4aef3..dad158a264e7cd 100644 --- a/lib/internal/crypto/rsa.js +++ b/lib/internal/crypto/rsa.js @@ -246,20 +246,30 @@ async function rsaImportKey( switch (format) { case 'spki': { verifyAcceptableRsaKeyUse(algorithm.name, 'public', usagesSet); - keyObject = createPublicKey({ - key: keyData, - format: 'der', - type: 'spki' - }); + try { + keyObject = createPublicKey({ + key: keyData, + format: 'der', + type: 'spki' + }); + } catch { + throw lazyDOMException( + 'Invalid keyData', 'DataError'); + } break; } case 'pkcs8': { verifyAcceptableRsaKeyUse(algorithm.name, 'private', usagesSet); - keyObject = createPrivateKey({ - key: keyData, - format: 'der', - type: 'pkcs8' - }); + try { + keyObject = createPrivateKey({ + key: keyData, + format: 'der', + type: 'pkcs8' + }); + } catch { + throw lazyDOMException( + 'Invalid keyData', 'DataError'); + } break; } case 'jwk': { From adca91a495a1c73c4e053217ee9de08487ca13ba Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Mon, 21 Nov 2022 23:30:21 +0100 Subject: [PATCH 09/12] crypto: validate CFRG webcrypto JWK import "d" and "x" are a pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45569 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/cfrg.js | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/internal/crypto/cfrg.js b/lib/internal/crypto/cfrg.js index 6e9066d2cdb8fe..6d5572a3f99988 100644 --- a/lib/internal/crypto/cfrg.js +++ b/lib/internal/crypto/cfrg.js @@ -301,12 +301,24 @@ async function cfrgImportKey( name, isPublic ? 'public' : 'private', usagesSet); - keyObject = createCFRGRawKey( + + const publicKeyObject = createCFRGRawKey( name, - Buffer.from( - isPublic ? keyData.x : keyData.d, - 'base64'), - isPublic); + Buffer.from(keyData.x, 'base64'), + true); + + if (isPublic) { + keyObject = publicKeyObject; + } else { + keyObject = createCFRGRawKey( + name, + Buffer.from(keyData.d, 'base64'), + false); + + if (!createPublicKey(keyObject).equals(publicKeyObject)) { + throw lazyDOMException('Invalid JWK keyData', 'DataError'); + } + } break; } case 'raw': { From 8570ffab8b331693ebc43b75ecad6618445dc5ca Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 22 Nov 2022 00:22:08 +0100 Subject: [PATCH 10/12] crypto: fix ECDH webcrypto public CryptoKey usages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/45569 Reviewed-By: Antoine du Hamel Reviewed-By: Tobias Nießen Backport-PR-URL: https://github.com/nodejs/node/pull/47336 --- lib/internal/crypto/ec.js | 9 ++++++++- test/parallel/test-webcrypto-derivebits-ecdh.js | 12 ++++++------ test/parallel/test-webcrypto-derivekey-ecdh.js | 8 ++++---- test/parallel/test-webcrypto-export-import-ec.js | 12 ++++++------ 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js index aea9bc9410d4ec..83f691bcabc35c 100644 --- a/lib/internal/crypto/ec.js +++ b/lib/internal/crypto/ec.js @@ -58,7 +58,14 @@ function verifyAcceptableEcKeyUse(name, type, usages) { let checkSet; switch (name) { case 'ECDH': - checkSet = ['deriveKey', 'deriveBits']; + switch (type) { + case 'private': + checkSet = ['deriveKey', 'deriveBits']; + break; + case 'public': + checkSet = []; + break; + } break; case 'ECDSA': switch (type) { diff --git a/test/parallel/test-webcrypto-derivebits-ecdh.js b/test/parallel/test-webcrypto-derivebits-ecdh.js index 739155fba47818..2ab152ba5a7615 100644 --- a/test/parallel/test-webcrypto-derivebits-ecdh.js +++ b/test/parallel/test-webcrypto-derivebits-ecdh.js @@ -73,7 +73,7 @@ async function prepareKeys() { namedCurve }, true, - ['deriveKey', 'deriveBits']), + []), ]); keys[namedCurve] = { privateKey, @@ -235,17 +235,17 @@ async function prepareKeys() { name: 'ECDH', public: keys['P-521'].publicKey }, keys['P-521'].publicKey, null), { - message: /baseKey must be a private key/ + name: 'InvalidAccessError' }); } { - // Base key is not a private key + // Public is not a public key await assert.rejects(subtle.deriveBits({ name: 'ECDH', public: keys['P-521'].privateKey - }, keys['P-521'].publicKey, null), { - message: /algorithm\.public must be a public key/ + }, keys['P-521'].privateKey, null), { + name: 'InvalidAccessError' }); } @@ -262,7 +262,7 @@ async function prepareKeys() { name: 'ECDH', public: key }, keys['P-521'].publicKey, null), { - message: /algorithm\.public must be a public key/ + name: 'InvalidAccessError' }); } })().then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-derivekey-ecdh.js b/test/parallel/test-webcrypto-derivekey-ecdh.js index 2ca54957a55304..d3ba660fde6d5b 100644 --- a/test/parallel/test-webcrypto-derivekey-ecdh.js +++ b/test/parallel/test-webcrypto-derivekey-ecdh.js @@ -68,7 +68,7 @@ async function prepareKeys() { namedCurve }, true, - ['deriveKey', 'deriveBits']), + []), ]); keys[namedCurve] = { privateKey, @@ -209,7 +209,7 @@ async function prepareKeys() { }, keys['P-521'].publicKey, ...otherArgs), - { message: /baseKey must be a private key/ }); + { name: 'InvalidAccessError' }); } { @@ -222,7 +222,7 @@ async function prepareKeys() { }, keys['P-521'].publicKey, ...otherArgs), - { message: /algorithm\.public must be a public key/ }); + { name: 'InvalidAccessError' }); } { @@ -242,6 +242,6 @@ async function prepareKeys() { }, keys['P-521'].publicKey, ...otherArgs), - { message: /algorithm\.public must be a public key/ }); + { name: 'InvalidAccessError' }); } })().then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-export-import-ec.js b/test/parallel/test-webcrypto-export-import-ec.js index 79f82a3d4adc26..ec5615b6582f88 100644 --- a/test/parallel/test-webcrypto-export-import-ec.js +++ b/test/parallel/test-webcrypto-export-import-ec.js @@ -333,19 +333,19 @@ async function testImportRaw({ name, publicUsages }, namedCurve) { const rsaPrivate = crypto.createPrivateKey( fixtures.readKey('rsa_private_2048.pem')); - for (const [name, [publicUsage, privateUsage]] of Object.entries({ - 'ECDSA': ['verify', 'sign'], - 'ECDH': ['deriveBits', 'deriveBits'], - })) { + for (const [name, publicUsages, privateUsages] of [ + ['ECDSA', ['verify'], ['sign']], + ['ECDH', [], ['deriveBits', 'deriveBits']], + ]) { assert.rejects(subtle.importKey( 'spki', rsaPublic.export({ format: 'der', type: 'spki' }), { name, hash: 'SHA-256', namedCurve: 'P-256' }, - true, [publicUsage]), { message: /Invalid key type/ }); + true, publicUsages), { message: /Invalid key type/ }); assert.rejects(subtle.importKey( 'pkcs8', rsaPrivate.export({ format: 'der', type: 'pkcs8' }), { name, hash: 'SHA-256', namedCurve: 'P-256' }, - true, [privateUsage]), { message: /Invalid key type/ }); + true, privateUsages), { message: /Invalid key type/ }); } } From 52e3bf8bf3205dfbe22cbc910becb7ee087cd74d Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 17 Jan 2023 09:57:58 +0100 Subject: [PATCH 11/12] crypto: use WebIDL converters in WebCryptoAPI WebCryptoAPI functions' arguments are now coersed and validated as per their WebIDL definitions like in other Web Crypto API implementations. This further improves interoperability with other implementations of Web Crypto API. PR-URL: https://github.com/nodejs/node/pull/46067 Reviewed-By: Antoine du Hamel Reviewed-By: Chengzhong Wu Backport-PR-URL: https://github.com/nodejs/node/pull/47383 --- doc/api/webcrypto.md | 4 + lib/internal/crypto/aes.js | 28 +- lib/internal/crypto/cfrg.js | 37 +- lib/internal/crypto/diffiehellman.js | 9 - lib/internal/crypto/ec.js | 39 +- lib/internal/crypto/hash.js | 12 +- lib/internal/crypto/hkdf.js | 8 +- lib/internal/crypto/mac.js | 62 +- lib/internal/crypto/pbkdf2.js | 55 +- lib/internal/crypto/rsa.js | 52 +- lib/internal/crypto/util.js | 282 +++++-- lib/internal/crypto/webcrypto.js | 347 +++++++-- lib/internal/crypto/webidl.js | 707 ++++++++++++++++++ test/fixtures/crypto/aes_gcm.js | 2 +- .../test-webcrypto-cryptokey-workers.js | 2 +- .../test-webcrypto-derivebits-cfrg.js | 2 +- .../test-webcrypto-derivebits-ecdh.js | 2 +- .../test-webcrypto-derivebits-hkdf.js | 20 +- .../parallel/test-webcrypto-derivekey-cfrg.js | 2 +- .../parallel/test-webcrypto-derivekey-ecdh.js | 2 +- test/parallel/test-webcrypto-digest.js | 20 +- .../test-webcrypto-export-import-cfrg.js | 38 +- .../test-webcrypto-export-import-ec.js | 40 +- .../test-webcrypto-export-import-rsa.js | 51 ++ test/parallel/test-webcrypto-export-import.js | 14 +- test/parallel/test-webcrypto-keygen.js | 59 +- .../test-webcrypto-sign-verify-ecdsa.js | 3 +- .../test-webcrypto-sign-verify-hmac.js | 17 +- .../test-webcrypto-sign-verify-rsa.js | 19 +- test/parallel/test-webcrypto-util.js | 12 +- test/parallel/test-webcrypto-webidl.js | 519 +++++++++++++ .../test-webcrypto-derivebits-pbkdf2.js | 22 +- test/wpt/status/WebCryptoAPI.json | 13 + 33 files changed, 2061 insertions(+), 440 deletions(-) create mode 100644 lib/internal/crypto/webidl.js create mode 100644 test/parallel/test-webcrypto-webidl.js diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md index 2188edd8523154..9f0275a8affa90 100644 --- a/doc/api/webcrypto.md +++ b/doc/api/webcrypto.md @@ -2,6 +2,10 @@