From 16184633f6cb539045fc6e03d80afcd00889c327 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 17 Nov 2024 14:13:47 +0100 Subject: [PATCH] crypto: allow length=0 for HKDF and PBKDF2 in SubtleCrypto.deriveBits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/55866 Reviewed-By: Matthew Aitken Reviewed-By: Yagiz Nizipli Reviewed-By: Jason Zhang Reviewed-By: Luigi Pinca Reviewed-By: Tobias Nießen --- lib/internal/crypto/hkdf.js | 3 ++- lib/internal/crypto/pbkdf2.js | 7 +++--- test/fixtures/wpt/README.md | 2 +- ...gorithm_returns_cached_object.https.any.js | 24 +++++++++++++++++++ .../derived_bits_length_testcases.js | 4 ++-- .../wpt/WebCryptoAPI/derive_bits_keys/hkdf.js | 4 ++-- .../WebCryptoAPI/derive_bits_keys/pbkdf2.js | 20 ++++++++-------- test/fixtures/wpt/versions.json | 2 +- .../test-webcrypto-derivebits-hkdf.js | 20 ++++++++++++---- .../test-webcrypto-derivebits-pbkdf2.js | 21 ++++++++++++---- 10 files changed, 76 insertions(+), 31 deletions(-) create mode 100644 test/fixtures/wpt/WebCryptoAPI/cryptokey_algorithm_returns_cached_object.https.any.js diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js index 757a2391a01..849e593e440 100644 --- a/lib/internal/crypto/hkdf.js +++ b/lib/internal/crypto/hkdf.js @@ -1,6 +1,7 @@ 'use strict'; const { + ArrayBuffer, FunctionPrototypeCall, } = primordials; @@ -141,7 +142,7 @@ async function hkdfDeriveBits(algorithm, baseKey, length) { const { hash, salt, info } = algorithm; if (length === 0) - throw lazyDOMException('length cannot be zero', 'OperationError'); + return new ArrayBuffer(0); if (length === null) throw lazyDOMException('length cannot be null', 'OperationError'); if (length % 8) { diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index 697ceffa542..4148725d034 100644 --- a/lib/internal/crypto/pbkdf2.js +++ b/lib/internal/crypto/pbkdf2.js @@ -1,6 +1,7 @@ 'use strict'; const { + ArrayBuffer, FunctionPrototypeCall, } = primordials; @@ -98,10 +99,8 @@ async function pbkdf2DeriveBits(algorithm, baseKey, length) { 'iterations cannot be zero', 'OperationError'); - const raw = baseKey[kKeyObject].export(); - if (length === 0) - throw lazyDOMException('length cannot be zero', 'OperationError'); + return new ArrayBuffer(0); if (length === null) throw lazyDOMException('length cannot be null', 'OperationError'); if (length % 8) { @@ -113,7 +112,7 @@ async function pbkdf2DeriveBits(algorithm, baseKey, length) { let result; try { result = await pbkdf2Promise( - raw, salt, iterations, length / 8, normalizeHashName(hash.name), + baseKey[kKeyObject].export(), salt, iterations, length / 8, normalizeHashName(hash.name), ); } catch (err) { throw lazyDOMException( diff --git a/test/fixtures/wpt/README.md b/test/fixtures/wpt/README.md index 3c7b7fb845d..3f684bf1b32 100644 --- a/test/fixtures/wpt/README.md +++ b/test/fixtures/wpt/README.md @@ -32,7 +32,7 @@ Last update: - user-timing: https://github.com/web-platform-tests/wpt/tree/5ae85bf826/user-timing - wasm/jsapi: https://github.com/web-platform-tests/wpt/tree/cde25e7e3c/wasm/jsapi - wasm/webapi: https://github.com/web-platform-tests/wpt/tree/fd1b23eeaa/wasm/webapi -- WebCryptoAPI: https://github.com/web-platform-tests/wpt/tree/5f0f4ac1af/WebCryptoAPI +- WebCryptoAPI: https://github.com/web-platform-tests/wpt/tree/b81831169b/WebCryptoAPI - webidl/ecmascript-binding/es-exceptions: https://github.com/web-platform-tests/wpt/tree/a370aad338/webidl/ecmascript-binding/es-exceptions - webmessaging/broadcastchannel: https://github.com/web-platform-tests/wpt/tree/6495c91853/webmessaging/broadcastchannel - webstorage: https://github.com/web-platform-tests/wpt/tree/9dafa89214/webstorage diff --git a/test/fixtures/wpt/WebCryptoAPI/cryptokey_algorithm_returns_cached_object.https.any.js b/test/fixtures/wpt/WebCryptoAPI/cryptokey_algorithm_returns_cached_object.https.any.js new file mode 100644 index 00000000000..b2d73fbab78 --- /dev/null +++ b/test/fixtures/wpt/WebCryptoAPI/cryptokey_algorithm_returns_cached_object.https.any.js @@ -0,0 +1,24 @@ +// META: title=WebCryptoAPI: CryptoKey.algorithm getter returns cached object + +// https://w3c.github.io/webcrypto/#dom-cryptokey-algorithm +// https://github.com/servo/servo/issues/33908 + +promise_test(function() { + return self.crypto.subtle.generateKey( + { + name: "AES-CTR", + length: 256, + }, + true, + ["encrypt"], + ).then( + function(key) { + let a = key.algorithm; + let b = key.algorithm; + assert_true(a === b); + }, + function(err) { + assert_unreached("generateKey threw an unexpected error: " + err.toString()); + } + ); +}, "CryptoKey.algorithm getter returns cached object"); \ No newline at end of file diff --git a/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js b/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js index 518c781d9f1..2679fa79e2a 100644 --- a/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js +++ b/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/derived_bits_length_testcases.js @@ -3,7 +3,7 @@ var testCases = { {length: 256, expected: algorithms["HKDF"].derivation}, {length: 384, expected: algorithms["HKDF"].derivation384}, {length: 230, expected: undefined}, // should throw an exception, not multiple of 8 - {length: 0, expected: undefined}, // explicitly disallowed, so should throw + {length: 0, expected: emptyArray}, {length: null, expected: undefined }, // should throw an exception {length: undefined, expected: undefined }, // should throw an exception {length: "omitted", expected: undefined }, // default value is null, so should throw @@ -12,7 +12,7 @@ var testCases = { {length: 256, expected: algorithms["PBKDF2"].derivation}, {length: 384, expected: algorithms["PBKDF2"].derivation384}, {length: 230, expected: undefined}, // should throw an exception, not multiple of 8 - {length: 0, expected: undefined}, // explicitly disallowed, so should throw + {length: 0, expected: emptyArray}, {length: null, expected: undefined }, // should throw an exception {length: undefined, expected: undefined }, // should throw an exception {length: "omitted", expected: undefined }, // default value is null, so should throw diff --git a/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/hkdf.js b/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/hkdf.js index b2dfda0257b..0384f88ec73 100644 --- a/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/hkdf.js +++ b/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/hkdf.js @@ -45,13 +45,13 @@ function define_tests() { }); }, testName); - // 0 length (OperationError) + // 0 length subsetTest(promise_test, function(test) { return subtle.deriveBits(algorithm, baseKeys[derivedKeySize], 0) .then(function(derivation) { assert_equals(derivation.byteLength, 0, "Derived correctly empty key"); }, function(err) { - assert_equals(err.name, "OperationError", "deriveBits with 0 length correctly threw OperationError: " + err.message); + assert_unreached("deriveBits failed with error " + err.name + ": " + err.message); }); }, testName + " with 0 length"); diff --git a/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/pbkdf2.js b/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/pbkdf2.js index 090806ceb6b..38cf3b1bfe9 100644 --- a/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/pbkdf2.js +++ b/test/fixtures/wpt/WebCryptoAPI/derive_bits_keys/pbkdf2.js @@ -42,6 +42,16 @@ function define_tests() { }); }, testName); + // 0 length + subsetTest(promise_test, function(test) { + return subtle.deriveBits({name: "PBKDF2", salt: salts[saltSize], hash: hashName, iterations: parseInt(iterations)}, baseKeys[passwordSize], 0) + .then(function(derivation) { + assert_true(equalBuffers(derivation.byteLength, 0, "Derived correctly empty key")); + }, function(err) { + assert_unreached("deriveBits failed with error " + err.name + ": " + err.message); + }); + }, testName + " with 0 length"); + // Check for correct deriveKey results for every kind of // key that can be created by the deriveKeys operation. derivedKeyTypes.forEach(function(derivedKeyType) { @@ -103,16 +113,6 @@ function define_tests() { }); - // 0 length (OperationError) - subsetTest(promise_test, function(test) { - return subtle.deriveBits({name: "PBKDF2", salt: salts[saltSize], hash: hashName, iterations: parseInt(iterations)}, baseKeys[passwordSize], 0) - .then(function(derivation) { - assert_unreached("0 length should have thrown an OperationError"); - }, function(err) { - assert_equals(err.name, "OperationError", "deriveBits with 0 length correctly threw OperationError: " + err.message); - }); - }, testName + " with 0 length"); - // length not multiple of 8 (OperationError) subsetTest(promise_test, function(test) { return subtle.deriveBits({name: "PBKDF2", salt: salts[saltSize], hash: hashName, iterations: parseInt(iterations)}, baseKeys[passwordSize], 44) diff --git a/test/fixtures/wpt/versions.json b/test/fixtures/wpt/versions.json index 6d521f98a4a..59a69635489 100644 --- a/test/fixtures/wpt/versions.json +++ b/test/fixtures/wpt/versions.json @@ -88,7 +88,7 @@ "path": "wasm/webapi" }, "WebCryptoAPI": { - "commit": "5f0f4ac1af4848480406621fac99163c8ba0e242", + "commit": "b81831169b8527a6c569a4ad92cf8a1baf4a7118", "path": "WebCryptoAPI" }, "webidl/ecmascript-binding/es-exceptions": { diff --git a/test/parallel/test-webcrypto-derivebits-hkdf.js b/test/parallel/test-webcrypto-derivebits-hkdf.js index bef6abdc19d..0e3b2992e1c 100644 --- a/test/parallel/test-webcrypto-derivebits-hkdf.js +++ b/test/parallel/test-webcrypto-derivebits-hkdf.js @@ -261,11 +261,6 @@ async function testDeriveBitsBadLengths( subtle.deriveBits(algorithm, baseKeys[size], undefined), { name: 'OperationError', }), - assert.rejects( - subtle.deriveBits(algorithm, baseKeys[size], 0), { - message: /length cannot be zero/, - name: 'OperationError', - }), assert.rejects( subtle.deriveBits(algorithm, baseKeys[size], null), { message: 'length cannot be null', @@ -562,3 +557,18 @@ async function testWrongKeyType( await Promise.all(variations); })().then(common.mustCall()); + +// https://github.com/w3c/webcrypto/pull/380 +{ + crypto.subtle.importKey('raw', new Uint8Array(0), 'HKDF', false, ['deriveBits']).then((key) => { + return crypto.subtle.deriveBits({ + name: 'HKDF', + hash: { name: 'SHA-256' }, + info: new Uint8Array(0), + salt: new Uint8Array(0), + }, key, 0); + }).then((bits) => { + assert.deepStrictEqual(bits, new ArrayBuffer(0)); + }) + .then(common.mustCall()); +} diff --git a/test/pummel/test-webcrypto-derivebits-pbkdf2.js b/test/pummel/test-webcrypto-derivebits-pbkdf2.js index 382dadf1b35..242bb080d82 100644 --- a/test/pummel/test-webcrypto-derivebits-pbkdf2.js +++ b/test/pummel/test-webcrypto-derivebits-pbkdf2.js @@ -449,11 +449,6 @@ async function testDeriveBitsBadLengths( subtle.deriveBits(algorithm, baseKeys[size], undefined), { name: 'OperationError', }), - assert.rejects( - subtle.deriveBits(algorithm, baseKeys[size], 0), { - message: /length cannot be zero/, - name: 'OperationError', - }), assert.rejects( subtle.deriveBits(algorithm, baseKeys[size], null), { message: 'length cannot be null', @@ -693,3 +688,19 @@ async function testWrongKeyType( await Promise.all(variations); })().then(common.mustCall()); + + +// https://github.com/w3c/webcrypto/pull/380 +{ + crypto.subtle.importKey('raw', new Uint8Array(0), 'PBKDF2', false, ['deriveBits']).then((key) => { + return crypto.subtle.deriveBits({ + name: 'PBKDF2', + hash: { name: 'SHA-256' }, + iterations: 10, + salt: new Uint8Array(0), + }, key, 0); + }).then((bits) => { + assert.deepStrictEqual(bits, new ArrayBuffer(0)); + }) + .then(common.mustCall()); +}