Skip to content

Commit e1cdcec

Browse files
authored
crypto: improve SubtleCrypto.supports() accuracy
Constrain context parameters, ML-KEM derived-key imports, HKDF output lengths, and RSA key generation. Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #65222 Refs: https://redirect.github.com/w3c/webcrypto/pull/558 Refs: https://redirect.github.com/WICG/webcrypto-modern-algos/pull/76 Refs: https://redirect.github.com/WICG/webcrypto-modern-algos/pull/77 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent f914e45 commit e1cdcec

16 files changed

Lines changed: 264 additions & 93 deletions

‎lib/internal/crypto/hkdf.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const {
2222
const { kMaxLength } = require('buffer');
2323

2424
const {
25+
getDigestSizeInBytes,
2526
jobPromise,
2627
normalizeHashName,
2728
toBuf,
@@ -141,19 +142,24 @@ function hkdfSync(hash, key, salt, info, length) {
141142
return bits;
142143
}
143144

144-
function validateHkdfDeriveBitsLength(length) {
145+
function validateHkdfDeriveBitsLength(length, hash) {
145146
if (length === null)
146147
throw lazyDOMException('length cannot be null', 'OperationError');
147148
if (length % 8) {
148149
throw lazyDOMException(
149150
'length must be a multiple of 8',
150151
'OperationError');
151152
}
153+
if (length > 255 * getDigestSizeInBytes(hash.name) * 8) {
154+
throw lazyDOMException(
155+
'length exceeds the maximum derived bit length',
156+
'OperationError');
157+
}
152158
}
153159

154160
function hkdfDeriveBits(algorithm, baseKey, length) {
155-
validateHkdfDeriveBitsLength(length);
156161
const { hash, salt, info } = algorithm;
162+
validateHkdfDeriveBitsLength(length, hash);
157163

158164
if (length === 0)
159165
return PromiseResolve(new ArrayBuffer(0));

‎lib/internal/crypto/rsa.js‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,6 @@ function rsaKeyGenerate(
103103
extractable,
104104
usages,
105105
) {
106-
const publicExponentConverted = bigIntArrayToUnsignedInt(algorithm.publicExponent);
107-
if (publicExponentConverted === undefined) {
108-
throw lazyDOMException(
109-
'The publicExponent must be equivalent to an unsigned 32-bit value',
110-
'OperationError');
111-
}
112106
const {
113107
name,
114108
modulusLength,
@@ -118,6 +112,7 @@ function rsaKeyGenerate(
118112

119113
const allowedUsages = kUsages[name];
120114
const usagesSet = validateKeyUsages(usages, allowedUsages.keygen, name);
115+
const publicExponentConverted = bigIntArrayToUnsignedInt(publicExponent);
121116

122117
const keyAlgorithm = {
123118
name,
@@ -126,12 +121,6 @@ function rsaKeyGenerate(
126121
hash,
127122
};
128123

129-
if (publicExponentConverted < 3 || publicExponentConverted % 2 === 0) {
130-
throw lazyDOMException(
131-
'The operation failed for an operation-specific reason',
132-
'OperationError');
133-
}
134-
135124
const keyUsages = getKeyPairUsages(usagesSet, allowedUsages);
136125
validateUsagesNotEmpty(keyUsages.private);
137126

‎lib/internal/crypto/util.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,20 +349,23 @@ const kAlgorithmDefinitions = {
349349
'importKey': null,
350350
'encapsulate': null,
351351
'decapsulate': null,
352+
'get shared key length': null,
352353
},
353354
'ML-KEM-768': {
354355
'generateKey': null,
355356
'exportKey': null,
356357
'importKey': null,
357358
'encapsulate': null,
358359
'decapsulate': null,
360+
'get shared key length': null,
359361
},
360362
'ML-KEM-1024': {
361363
'generateKey': null,
362364
'exportKey': null,
363365
'importKey': null,
364366
'encapsulate': null,
365367
'decapsulate': null,
368+
'get shared key length': null,
366369
},
367370
'PBKDF2': {
368371
'importKey': null,
@@ -894,15 +897,18 @@ function jobPromiseThen(promise, onFulfilled, onRejected) {
894897
// an unsigned int from a Buffer are not adequate. The implementation
895898
// here is adapted from the chromium implementation here:
896899
// https://github.com/chromium/chromium/blob/HEAD/third_party/blink/public/platform/web_crypto_algorithm_params.h, but ported to JavaScript
897-
// Returns undefined if the conversion was unsuccessful.
900+
// Throws an OperationError if the value does not fit in an unsigned 32-bit integer.
898901
function bigIntArrayToUnsignedInt(input) {
899902
let result = 0;
900903
const length = TypedArrayPrototypeGetLength(input);
901904

902905
for (let n = 0; n < length; ++n) {
903906
const n_reversed = length - n - 1;
904-
if (n_reversed >= 4 && input[n])
905-
return; // Too large
907+
if (n_reversed >= 4 && input[n]) {
908+
throw lazyDOMException(
909+
'algorithm.publicExponent must fit in an unsigned 32-bit integer',
910+
'OperationError');
911+
}
906912
result |= input[n] << 8 * n_reversed;
907913
}
908914

‎lib/internal/crypto/webcrypto.js‎

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,54 @@ function getKeyLength({ name, length, hash }) {
348348
}
349349
}
350350

351+
function getSharedKeyLength({ name }) {
352+
switch (name) {
353+
case 'ML-KEM-512':
354+
// Fall through
355+
case 'ML-KEM-768':
356+
// Fall through
357+
case 'ML-KEM-1024':
358+
return 256;
359+
/* c8 ignore start */
360+
default: {
361+
const assert = require('internal/assert');
362+
assert.fail('Unreachable code');
363+
}
364+
/* c8 ignore stop */
365+
}
366+
}
367+
368+
function canImportRawSecret(algorithm, sharedKeyLength) {
369+
switch (algorithm.name) {
370+
case 'AES-OCB':
371+
case 'AES-KW':
372+
case 'AES-GCM':
373+
case 'AES-CTR':
374+
case 'AES-CBC':
375+
return sharedKeyLength === 128 ||
376+
sharedKeyLength === 192 ||
377+
sharedKeyLength === 256;
378+
case 'ChaCha20-Poly1305':
379+
return sharedKeyLength === 256;
380+
case 'HKDF':
381+
case 'PBKDF2':
382+
case 'Argon2i':
383+
case 'Argon2d':
384+
case 'Argon2id':
385+
return true;
386+
case 'HMAC':
387+
if (sharedKeyLength === 0)
388+
return false;
389+
// Fall through
390+
case 'KMAC128':
391+
case 'KMAC256':
392+
return algorithm.length === undefined ||
393+
numBitsToBytes(algorithm.length) * 8 === sharedKeyLength;
394+
default:
395+
return false;
396+
}
397+
}
398+
351399
function deriveKey(
352400
algorithm,
353401
baseKey,
@@ -1741,37 +1789,19 @@ class SubtleCrypto {
17411789
},
17421790
);
17431791

1792+
let sharedKeyLength;
17441793
let normalizedAdditionalAlgorithm;
17451794
try {
1795+
const normalizedAlgorithm =
1796+
normalizeAlgorithm(algorithm, 'get shared key length');
1797+
sharedKeyLength = getSharedKeyLength(normalizedAlgorithm);
17461798
normalizedAdditionalAlgorithm = normalizeAlgorithm(additionalAlgorithm, 'importKey');
17471799
} catch {
17481800
return false;
17491801
}
17501802

1751-
switch (normalizedAdditionalAlgorithm.name) {
1752-
case 'AES-OCB':
1753-
case 'AES-KW':
1754-
case 'AES-GCM':
1755-
case 'AES-CTR':
1756-
case 'AES-CBC':
1757-
case 'ChaCha20-Poly1305':
1758-
case 'HKDF':
1759-
case 'PBKDF2':
1760-
case 'Argon2i':
1761-
case 'Argon2d':
1762-
case 'Argon2id':
1763-
break;
1764-
case 'HMAC':
1765-
case 'KMAC128':
1766-
case 'KMAC256':
1767-
if (normalizedAdditionalAlgorithm.length === undefined ||
1768-
numBitsToBytes(normalizedAdditionalAlgorithm.length) === 32) {
1769-
break;
1770-
}
1771-
return false;
1772-
default:
1773-
return false;
1774-
}
1803+
if (!canImportRawSecret(normalizedAdditionalAlgorithm, sharedKeyLength))
1804+
return false;
17751805
}
17761806

17771807
try {
@@ -1807,8 +1837,6 @@ function check(op, alg, length) {
18071837
}
18081838

18091839
switch (op) {
1810-
case 'decapsulate':
1811-
case 'decrypt':
18121840
case 'digest': {
18131841
if ((normalizedAlgorithm.name === 'cSHAKE128' ||
18141842
normalizedAlgorithm.name === 'cSHAKE256') &&
@@ -1818,6 +1846,8 @@ function check(op, alg, length) {
18181846
}
18191847
return true;
18201848
}
1849+
case 'decapsulate':
1850+
case 'decrypt':
18211851
case 'encapsulate':
18221852
case 'encrypt':
18231853
case 'exportKey':
@@ -1829,7 +1859,8 @@ function check(op, alg, length) {
18291859
return true;
18301860
case 'deriveBits': {
18311861
if (normalizedAlgorithm.name === 'HKDF') {
1832-
require('internal/crypto/hkdf').validateHkdfDeriveBitsLength(length);
1862+
require('internal/crypto/hkdf')
1863+
.validateHkdfDeriveBitsLength(length, normalizedAlgorithm.hash);
18331864
}
18341865

18351866
if (normalizedAlgorithm.name === 'PBKDF2') {

‎lib/internal/crypto/webidl.js‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const {
2323
getCryptoKeyType,
2424
} = require('internal/crypto/keys');
2525
const {
26+
bigIntArrayToUnsignedInt,
2627
validateMaxBufferLength,
2728
getBufferSourceByteLength,
2829
getBufferSourceBytes,
@@ -41,6 +42,8 @@ const {
4142
type,
4243
} = require('internal/webidl');
4344

45+
const kRsaKeyGenMinimumModulusLength = isFips ? 2048 : 512;
46+
4447
function validateByteLength(buf, name, target) {
4548
if (getBufferSourceByteLength(buf) !== target) {
4649
throw lazyDOMException(
@@ -152,11 +155,33 @@ const dictRsaKeyGenParams = [
152155
key: 'modulusLength',
153156
converter: (V, opts) =>
154157
converters['unsigned long'](V, enforceRangeOptions(opts)),
158+
validator: (modulusLength) => {
159+
if (modulusLength < kRsaKeyGenMinimumModulusLength) {
160+
throw lazyDOMException(
161+
`algorithm.modulusLength must be at least ${kRsaKeyGenMinimumModulusLength}`,
162+
'OperationError');
163+
}
164+
},
155165
required: true,
156166
},
157167
{
158168
key: 'publicExponent',
159169
converter: converters.BigInteger,
170+
validator: (publicExponent) => {
171+
const converted = bigIntArrayToUnsignedInt(publicExponent);
172+
173+
if (converted < 3) {
174+
throw lazyDOMException(
175+
'algorithm.publicExponent must be at least 3',
176+
'OperationError');
177+
}
178+
179+
if (converted % 2 === 0) {
180+
throw lazyDOMException(
181+
'algorithm.publicExponent must be odd',
182+
'OperationError');
183+
}
184+
},
160185
required: true,
161186
},
162187
];
@@ -625,20 +650,27 @@ converters.ContextParams = createDictionaryConverter(
625650
key: 'context',
626651
converter: converters.BufferSource,
627652
validator(V, dict) {
653+
const validateLength = (V) =>
654+
validateMaxBufferLength(V, 'ContextParams.context', 255);
655+
628656
if (process.features.openssl_is_boringssl) {
629-
this.validator = undefined;
657+
this.validator = validateLength;
630658
} else {
631659
let { 0: major, 1: minor } =
632660
StringPrototypeSplit(process.versions.openssl, '.');
633661
major = NumberParseInt(major, 10);
634662
minor = NumberParseInt(minor, 10);
635663
if (major > 3 || (major === 3 && minor >= 2)) {
636-
this.validator = undefined;
664+
this.validator = validateLength;
637665
} else {
638-
this.validator = validateZeroLength('ContextParams.context');
639-
this.validator(V, dict);
666+
const validateEmpty = validateZeroLength('ContextParams.context');
667+
this.validator = (V, dict) => {
668+
validateLength(V);
669+
validateEmpty(V, dict);
670+
};
640671
}
641672
}
673+
this.validator(V, dict);
642674
},
643675
},
644676
],

‎test/fixtures/webcrypto/supports-level-2.mjs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
import { getFips } from 'node:crypto';
2+
13
const { subtle } = globalThis.crypto;
4+
const RSA_MINIMUM_MODULUS_LENGTH = getFips() === 1 ? 2048 : 512;
25

36
const RSA_KEY_GEN = {
47
modulusLength: 2048,
@@ -66,6 +69,30 @@ export const vectors = {
6669
[true, { name: 'RSASSA-PKCS1-v1_5', hash: 'SHA-256', ...RSA_KEY_GEN }],
6770
[true, { name: 'RSA-PSS', hash: 'SHA-256', ...RSA_KEY_GEN }],
6871
[true, { name: 'RSA-OAEP', hash: 'SHA-256', ...RSA_KEY_GEN }],
72+
[true, {
73+
name: 'RSA-PSS',
74+
hash: 'SHA-256',
75+
modulusLength: RSA_MINIMUM_MODULUS_LENGTH,
76+
publicExponent: new Uint8Array([1, 0, 1]),
77+
}],
78+
[false, {
79+
name: 'RSASSA-PKCS1-v1_5',
80+
hash: 'SHA-256',
81+
modulusLength: RSA_MINIMUM_MODULUS_LENGTH - 1,
82+
publicExponent: new Uint8Array([1, 0, 1]),
83+
}],
84+
[false, {
85+
name: 'RSA-PSS',
86+
hash: 'SHA-256',
87+
...RSA_KEY_GEN,
88+
publicExponent: new Uint8Array([2]),
89+
}],
90+
[false, {
91+
name: 'RSA-OAEP',
92+
hash: 'SHA-256',
93+
...RSA_KEY_GEN,
94+
publicExponent: new Uint8Array([1, 0, 0, 0, 1]),
95+
}],
6996
[true, { name: 'ECDSA', namedCurve: 'P-256' }],
7097
[false, { name: 'ECDSA', namedCurve: 'X25519' }],
7198
[true, { name: 'AES-CTR', length: 128 }],
@@ -146,6 +173,8 @@ export const vectors = {
146173
'deriveBits': [
147174
[true, { name: 'HKDF', hash: 'SHA-256', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, 8],
148175
[true, { name: 'HKDF', hash: 'SHA-256', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, 0],
176+
[true, { name: 'HKDF', hash: 'SHA-256', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, 65280],
177+
[false, { name: 'HKDF', hash: 'SHA-256', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, 65288],
149178
[false, { name: 'HKDF', hash: 'SHA-256', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, null],
150179
[false, { name: 'HKDF', hash: 'SHA-256', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, 7],
151180
[false, { name: 'HKDF', hash: 'Invalid', salt: Buffer.alloc(0), info: Buffer.alloc(0) }, 8],
@@ -234,4 +263,7 @@ export const vectors = {
234263
'get key length': [
235264
[false, { name: 'HMAC', hash: 'SHA-256' }],
236265
],
266+
'get shared key length': [
267+
[false, 'ML-KEM-768'],
268+
],
237269
};

0 commit comments

Comments
 (0)