crypto: make deriveBits length parameter optional and nullable

panva · aduh95 · commit 3cc01aa3143b · 2024-07-16T12:01:05.000+02:00
PR-URL: #53601 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md
@@ -569,11 +569,15 @@ The algorithms currently supported include:
 * `'AES-CBC'`
 * `'AES-GCM`'
 
-### `subtle.deriveBits(algorithm, baseKey, length)`
+### `subtle.deriveBits(algorithm, baseKey[, length])`
 
 <!-- YAML
 added: v15.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/53601
+    description: The length parameter is now optional for `'ECDH'`, `'X25519'`,
+                 and `'X448'`.
   - version:
     - v18.4.0
     - v16.17.0
@@ -585,7 +589,7 @@ changes:
 
 * `algorithm`: {AlgorithmIdentifier|EcdhKeyDeriveParams|HkdfParams|Pbkdf2Params}
 * `baseKey`: {CryptoKey}
-* `length`: {number|null}
+* `length`: {number|null} **Default:** `null`
 * Returns: {Promise} Fulfills with an {ArrayBuffer}
 
 <!--lint enable maximum-line-length remark-lint-->
@@ -594,12 +598,12 @@ Using the method and parameters specified in `algorithm` and the keying
 material provided by `baseKey`, `subtle.deriveBits()` attempts to generate
 `length` bits.
 
-The Node.js implementation requires that when `length` is a
-number it must be multiple of `8`.
+The Node.js implementation requires that `length`, when a number, is a multiple
+of `8`.
 
-When `length` is `null` the maximum number of bits for a given algorithm is
-generated. This is allowed for the `'ECDH'`, `'X25519'`, and `'X448'`
-algorithms.
+When `length` is not provided or `null` the maximum number of bits for a given
+algorithm is generated. This is allowed for the `'ECDH'`, `'X25519'`, and `'X448'`
+algorithms, for other algorithms `length` is required to be a number.
 
 If successful, the returned promise will be resolved with an {ArrayBuffer}
 containing the generated data.
diff --git a/lib/internal/crypto/webcrypto.js b/lib/internal/crypto/webcrypto.js
@@ -178,12 +178,12 @@ async function generateKey(
   return result;
 }
 
-async function deriveBits(algorithm, baseKey, length) {
+async function deriveBits(algorithm, baseKey, length = null) {
   if (this !== subtle) throw new ERR_INVALID_THIS('SubtleCrypto');
 
   webidl ??= require('internal/crypto/webidl');
   const prefix = "Failed to execute 'deriveBits' on 'SubtleCrypto'";
-  webidl.requiredArguments(arguments.length, 3, { prefix });
+  webidl.requiredArguments(arguments.length, 2, { prefix });
   algorithm = webidl.converters.AlgorithmIdentifier(algorithm, {
     prefix,
     context: '1st argument',
diff --git a/test/parallel/test-webcrypto-derivebits-cfrg.js b/test/parallel/test-webcrypto-derivebits-cfrg.js
@@ -101,6 +101,16 @@ async function prepareKeys() {
         assert.strictEqual(Buffer.from(bits).toString('hex'), result);
       }
 
+      {
+        // Default length
+        const bits = await subtle.deriveBits({
+          name,
+          public: publicKey
+        }, privateKey);
+
+        assert.strictEqual(Buffer.from(bits).toString('hex'), result);
+      }
+
       {
         // Short Result
         const bits = await subtle.deriveBits({
diff --git a/test/parallel/test-webcrypto-derivebits-ecdh.js b/test/parallel/test-webcrypto-derivebits-ecdh.js
@@ -122,6 +122,16 @@ async function prepareKeys() {
         assert.strictEqual(Buffer.from(bits).toString('hex'), result);
       }
 
+      {
+        // Default length
+        const bits = await subtle.deriveBits({
+          name: 'ECDH',
+          public: publicKey
+        }, privateKey);
+
+        assert.strictEqual(Buffer.from(bits).toString('hex'), result);
+      }
+
       {
         // Short Result
         const bits = await subtle.deriveBits({
diff --git a/test/parallel/test-webcrypto-derivebits-hkdf.js b/test/parallel/test-webcrypto-derivebits-hkdf.js
@@ -271,6 +271,11 @@ async function testDeriveBitsBadLengths(
         message: 'length cannot be null',
         name: 'OperationError',
       }),
+    assert.rejects(
+      subtle.deriveBits(algorithm, baseKeys[size]), {
+        message: 'length cannot be null',
+        name: 'OperationError',
+      }),
     assert.rejects(
       subtle.deriveBits(algorithm, baseKeys[size], 15), {
         message: /length must be a multiple of 8/,
diff --git a/test/pummel/test-webcrypto-derivebits-pbkdf2.js b/test/pummel/test-webcrypto-derivebits-pbkdf2.js
@@ -459,6 +459,11 @@ async function testDeriveBitsBadLengths(
         message: 'length cannot be null',
         name: 'OperationError',
       }),
+    assert.rejects(
+      subtle.deriveBits(algorithm, baseKeys[size]), {
+        message: 'length cannot be null',
+        name: 'OperationError',
+      }),
     assert.rejects(
       subtle.deriveBits(algorithm, baseKeys[size], 15), {
         message: /length must be a multiple of 8/,
diff --git a/test/wpt/status/WebCryptoAPI.json b/test/wpt/status/WebCryptoAPI.json
@@ -4,5 +4,13 @@
   },
   "historical.any.js": {
     "skip": "Not relevant in Node.js context"
+  },
+  "idlharness.https.any.js": {
+    "fail": {
+      "note": "WPT not updated for https://github.com/w3c/webcrypto/pull/345 yet",
+      "expected": [
+        "SubtleCrypto interface: operation deriveBits(AlgorithmIdentifier, CryptoKey, unsigned long)"
+      ]
+    }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -4,5 +4,13 @@`
`4`	`4`	`},`
`5`	`5`	`"historical.any.js": {`
`6`	`6`	`"skip": "Not relevant in Node.js context"`
	`7`	`+ },`
	`8`	`+ "idlharness.https.any.js": {`
	`9`	`+ "fail": {`
	`10`	`+ "note": "WPT not updated for https://github.com/w3c/webcrypto/pull/345 yet",`
	`11`	`+ "expected": [`
	`12`	`+ "SubtleCrypto interface: operation deriveBits(AlgorithmIdentifier, CryptoKey, unsigned long)"`
	`13`	`+ ]`
	`14`	`+ }`
`7`	`15`	`}`
`8`	`16`	`}`