crypto: allow length=0 for HKDF and PBKDF2 in SubtleCrypto.deriveBits

PR-URL: #55866 Reviewed-By: Matthew Aitken <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jason Zhang <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Nov 17, 2024 · 1618463 · 1618463
1 parent b1ee186
commit 1618463
Show file tree

Hide file tree

Showing 10 changed files with 76 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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");
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());
+}