crypto: improve SubtleCrypto.supports() accuracy · nodejs/node@501f816 · GitHub
Skip to content

Commit 501f816

Browse files
panvaaduh95
panva
authored and
aduh95
committed
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 fbec4eb commit 501f816

16 files changed

Lines changed: 264 additions & 93 deletions

‎lib/internal/crypto/hkdf.js‎

Lines changed: 8 additions & 2 deletions

‎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
@@ -341,20 +341,23 @@ const kAlgorithmDefinitions = {
341341
'importKey': null,
342342
'encapsulate': null,
343343
'decapsulate': null,
344+
'get shared key length': null,
344345
},
345346
'ML-KEM-768': {
346347
'generateKey': null,
347348
'exportKey': null,
348349
'importKey': null,
349350
'encapsulate': null,
350351
'decapsulate': null,
352+
'get shared key length': null,
351353
},
352354
'ML-KEM-1024': {
353355
'generateKey': null,
354356
'exportKey': null,
355357
'importKey': null,
356358
'encapsulate': null,
357359
'decapsulate': null,
360+
'get shared key length': null,
358361
},
359362
'PBKDF2': {
360363
'importKey': null,
@@ -886,15 +889,18 @@ function jobPromiseThen(promise, onFulfilled, onRejected) {
886889
// an unsigned int from a Buffer are not adequate. The implementation
887890
// here is adapted from the chromium implementation here:
888891
// https://github.com/chromium/chromium/blob/HEAD/third_party/blink/public/platform/web_crypto_algorithm_params.h, but ported to JavaScript
889-
// Returns undefined if the conversion was unsuccessful.
892+
// Throws an OperationError if the value does not fit in an unsigned 32-bit integer.
890893
function bigIntArrayToUnsignedInt(input) {
891894
let result = 0;
892895
const length = TypedArrayPrototypeGetLength(input);
893896

894897
for (let n = 0; n < length; ++n) {
895898
const n_reversed = length - n - 1;
896-
if (n_reversed >= 4 && input[n])
897-
return; // Too large
899+
if (n_reversed >= 4 && input[n]) {
900+
throw lazyDOMException(
901+
'algorithm.publicExponent must fit in an unsigned 32-bit integer',
902+
'OperationError');
903+
}
898904
result |= input[n] << 8 * n_reversed;
899905
}
900906

‎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
@@ -24,6 +24,7 @@ const {
2424
getCryptoKeyType,
2525
} = require('internal/crypto/keys');
2626
const {
27+
bigIntArrayToUnsignedInt,
2728
validateMaxBufferLength,
2829
getBufferSourceByteLength,
2930
getBufferSourceBytes,
@@ -42,6 +43,8 @@ const {
4243
type,
4344
} = require('internal/webidl');
4445

46+
const kRsaKeyGenMinimumModulusLength = isFips ? 2048 : 512;
47+
4548
function validateByteLength(buf, name, target) {
4649
if (getBufferSourceByteLength(buf) !== target) {
4750
throw lazyDOMException(
@@ -176,11 +179,33 @@ const dictRsaKeyGenParams = [
176179
key: 'modulusLength',
177180
converter: (V, opts) =>
178181
converters['unsigned long'](V, enforceRangeOptions(opts)),
182+
validator: (modulusLength) => {
183+
if (modulusLength < kRsaKeyGenMinimumModulusLength) {
184+
throw lazyDOMException(
185+
`algorithm.modulusLength must be at least ${kRsaKeyGenMinimumModulusLength}`,
186+
'OperationError');
187+
}
188+
},
179189
required: true,
180190
},
181191
{
182192
key: 'publicExponent',
183193
converter: converters.BigInteger,
194+
validator: (publicExponent) => {
195+
const converted = bigIntArrayToUnsignedInt(publicExponent);
196+
197+
if (converted < 3) {
198+
throw lazyDOMException(
199+
'algorithm.publicExponent must be at least 3',
200+
'OperationError');
201+
}
202+
203+
if (converted % 2 === 0) {
204+
throw lazyDOMException(
205+
'algorithm.publicExponent must be odd',
206+
'OperationError');
207+
}
208+
},
184209
required: true,
185210
},
186211
];
@@ -649,20 +674,27 @@ converters.ContextParams = createDictionaryConverter(
649674
key: 'context',
650675
converter: converters.BufferSource,
651676
validator(V, dict) {
677+
const validateLength = (V) =>
678+
validateMaxBufferLength(V, 'ContextParams.context', 255);
679+
652680
if (process.features.openssl_is_boringssl) {
653-
this.validator = undefined;
681+
this.validator = validateLength;
654682
} else {
655683
let { 0: major, 1: minor } =
656684
StringPrototypeSplit(process.versions.openssl, '.');
657685
major = NumberParseInt(major, 10);
658686
minor = NumberParseInt(minor, 10);
659687
if (major > 3 || (major === 3 && minor >= 2)) {
660-
this.validator = undefined;
688+
this.validator = validateLength;
661689
} else {
662-
this.validator = validateZeroLength('ContextParams.context');
663-
this.validator(V, dict);
690+
const validateEmpty = validateZeroLength('ContextParams.context');
691+
this.validator = (V, dict) => {
692+
validateLength(V);
693+
validateEmpty(V, dict);
694+
};
664695
}
665696
}
697+
this.validator(V, dict);
666698
},
667699
},
668700
],

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

Lines changed: 32 additions & 0 deletions

0 commit comments

Comments
 (0)