Skip to content

Commit

Permalink
crypto: allow non-multiple of 8 in SubtleCrypto.deriveBits
Browse files Browse the repository at this point in the history
PR-URL: nodejs#55296
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
panva authored and Ceres6 committed Nov 26, 2024
1 parent 4c82a25 commit 15e8901
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 27 deletions.
3 changes: 0 additions & 3 deletions doc/api/webcrypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -601,9 +601,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.
Expand Down
26 changes: 18 additions & 8 deletions lib/internal/crypto/diffiehellman.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
MathCeil,
ObjectDefineProperty,
SafeSet,
Uint8Array,
} = primordials;

const { Buffer } = require('buffer');
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 = {
Expand Down
8 changes: 5 additions & 3 deletions test/parallel/test-webcrypto-derivebits-cfrg.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}));

Expand Down
8 changes: 5 additions & 3 deletions test/parallel/test-webcrypto-derivebits-ecdh.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}));

Expand Down
10 changes: 0 additions & 10 deletions test/wpt/status/WebCryptoAPI.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
Expand Down

0 comments on commit 15e8901

Please sign in to comment.