crypto: allow non-multiple of 8 in SubtleCrypto.deriveBits

panva · ruyadorno · commit 9ee9f524a70e · 2025-01-05T12:57:25.000-05:00
PR-URL: #55296 Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/webcrypto.md b/doc/api/webcrypto.md
@@ -598,9 +598,6 @@ 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 `length`, when a number, is a multiple
-of `8`.
-
 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.
diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js
@@ -5,6 +5,7 @@ const {
   MathCeil,
   ObjectDefineProperty,
   SafeSet,
+  Uint8Array,
 } = primordials;
 
 const { Buffer } = require('buffer');
@@ -295,6 +296,8 @@ function diffieHellman(options) {
   return statelessDH(privateKey[kHandle], publicKey[kHandle]);
 }
 
+let masks;
+
 // The ecdhDeriveBits function is part of the Web Crypto API and serves both
 // deriveKeys and deriveBits functions.
 async function ecdhDeriveBits(algorithm, baseKey, length) {
@@ -341,18 +344,25 @@ async function ecdhDeriveBits(algorithm, baseKey, length) {
 
   // If the length is not a multiple of 8 the nearest ceiled
   // multiple of 8 is sliced.
-  length = MathCeil(length / 8);
-  const { byteLength } = bits;
+  const sliceLength = MathCeil(length / 8);
 
+  const { byteLength } = bits;
   // If the length is larger than the derived secret, throw.
-  // Otherwise, we either return the secret or a truncated
-  // slice.
-  if (byteLength < length)
+  if (byteLength < sliceLength)
     throw lazyDOMException('derived bit length is too small', 'OperationError');
 
-  return length === byteLength ?
-    bits :
-    ArrayBufferPrototypeSlice(bits, 0, length);
+  const slice = ArrayBufferPrototypeSlice(bits, 0, sliceLength);
+
+  const mod = length % 8;
+  if (mod === 0)
+    return slice;
+
+  // eslint-disable-next-line no-sparse-arrays
+  masks ||= [, 0b10000000, 0b11000000, 0b11100000, 0b11110000, 0b11111000, 0b11111100, 0b11111110];
+
+  const masked = new Uint8Array(slice);
+  masked[sliceLength - 1] = masked[sliceLength - 1] & masks[mod];
+  return masked.buffer;
 }
 
 module.exports = {
diff --git a/test/parallel/test-webcrypto-derivebits-cfrg.js b/test/parallel/test-webcrypto-derivebits-cfrg.js
@@ -140,9 +140,11 @@ async function prepareKeys() {
           public: publicKey
         }, privateKey, 8 * size - 11);
 
-        assert.strictEqual(
-          Buffer.from(bits).toString('hex'),
-          result.slice(0, -2));
+        const expected = Buffer.from(result.slice(0, -2), 'hex');
+        expected[size - 2] = expected[size - 2] & 0b11111000;
+        assert.deepStrictEqual(
+          Buffer.from(bits),
+          expected);
       }
     }));
 
diff --git a/test/parallel/test-webcrypto-derivebits-ecdh.js b/test/parallel/test-webcrypto-derivebits-ecdh.js
@@ -161,9 +161,11 @@ async function prepareKeys() {
           public: publicKey
         }, privateKey, 8 * size - 11);
 
-        assert.strictEqual(
-          Buffer.from(bits).toString('hex'),
-          result.slice(0, -2));
+        const expected = Buffer.from(result.slice(0, -2), 'hex');
+        expected[size - 2] = expected[size - 2] & 0b11111000;
+        assert.deepStrictEqual(
+          Buffer.from(bits),
+          expected);
       }
     }));
 
diff --git a/test/wpt/status/WebCryptoAPI.cjs b/test/wpt/status/WebCryptoAPI.cjs
@@ -8,16 +8,6 @@ module.exports = {
   'algorithm-discards-context.https.window.js': {
     'skip': 'Not relevant in Node.js context',
   },
-  'derive_bits_keys/derived_bits_length.https.any.js': {
-    'fail': {
-      // See https://github.com/nodejs/node/pull/55296
-      // The fix is pending a decision whether truncation in ECDH/X* will be removed from the spec entirely
-      'expected': [
-        "ECDH derivation with 230 as 'length' parameter",
-        "X25519 derivation with 230 as 'length' parameter",
-      ],
-    },
-  },
   'historical.any.js': {
     'skip': 'Not relevant in Node.js context',
   },
