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

Commit 5a46a9a

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. PR-URL: #65222 Backport-PR-URL: #66128 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: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 4396c9b commit 5a46a9a

15 files changed

Lines changed: 266 additions & 81 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
@@ -338,20 +338,23 @@ const kAlgorithmDefinitions = {
338338
'importKey': null,
339339
'encapsulate': null,
340340
'decapsulate': null,
341+
'get shared key length': null,
341342
},
342343
'ML-KEM-768': {
343344
'generateKey': null,
344345
'exportKey': null,
345346
'importKey': null,
346347
'encapsulate': null,
347348
'decapsulate': null,
349+
'get shared key length': null,
348350
},
349351
'ML-KEM-1024': {
350352
'generateKey': null,
351353
'exportKey': null,
352354
'importKey': null,
353355
'encapsulate': null,
354356
'decapsulate': null,
357+
'get shared key length': null,
355358
},
356359
'PBKDF2': {
357360
'importKey': null,
@@ -874,15 +877,18 @@ function jobPromiseThen(promise, onFulfilled, onRejected) {
874877
// an unsigned int from a Buffer are not adequate. The implementation
875878
// here is adapted from the chromium implementation here:
876879
// https://github.com/chromium/chromium/blob/HEAD/third_party/blink/public/platform/web_crypto_algorithm_params.h, but ported to JavaScript
877-
// Returns undefined if the conversion was unsuccessful.
880+
// Throws an OperationError if the value does not fit in an unsigned 32-bit integer.
878881
function bigIntArrayToUnsignedInt(input) {
879882
let result = 0;
880883
const length = TypedArrayPrototypeGetLength(input);
881884

882885
for (let n = 0; n < length; ++n) {
883886
const n_reversed = length - n - 1;
884-
if (n_reversed >= 4 && input[n])
885-
return; // Too large
887+
if (n_reversed >= 4 && input[n]) {
888+
throw lazyDOMException(
889+
'algorithm.publicExponent must fit in an unsigned 32-bit integer',
890+
'OperationError');
891+
}
886892
result |= input[n] << 8 * n_reversed;
887893
}
888894

‎lib/internal/crypto/webcrypto.js‎

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

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

1800+
let sharedKeyLength;
17521801
let normalizedAdditionalAlgorithm;
17531802
try {
1803+
const normalizedAlgorithm =
1804+
normalizeAlgorithm(algorithm, 'get shared key length');
1805+
sharedKeyLength = getSharedKeyLength(normalizedAlgorithm);
17541806
normalizedAdditionalAlgorithm = normalizeAlgorithm(additionalAlgorithm, 'importKey');
17551807
} catch {
17561808
return false;
17571809
}
17581810

1759-
switch (normalizedAdditionalAlgorithm.name) {
1760-
case 'AES-OCB':
1761-
case 'AES-KW':
1762-
case 'AES-GCM':
1763-
case 'AES-CTR':
1764-
case 'AES-CBC':
1765-
case 'ChaCha20-Poly1305':
1766-
case 'HKDF':
1767-
case 'PBKDF2':
1768-
case 'Argon2i':
1769-
case 'Argon2d':
1770-
case 'Argon2id':
1771-
break;
1772-
case 'HMAC':
1773-
case 'KMAC128':
1774-
case 'KMAC256':
1775-
if (normalizedAdditionalAlgorithm.length === undefined ||
1776-
numBitsToBytes(normalizedAdditionalAlgorithm.length) === 32) {
1777-
break;
1778-
}
1779-
return false;
1780-
default:
1781-
return false;
1782-
}
1811+
if (!canImportRawSecret(normalizedAdditionalAlgorithm, sharedKeyLength))
1812+
return false;
17831813
}
17841814

17851815
try {
@@ -1815,8 +1845,6 @@ function check(op, alg, length) {
18151845
}
18161846

18171847
switch (op) {
1818-
case 'decapsulate':
1819-
case 'decrypt':
18201848
case 'digest': {
18211849
if ((normalizedAlgorithm.name === 'cSHAKE128' ||
18221850
normalizedAlgorithm.name === 'cSHAKE256') &&
@@ -1826,6 +1854,8 @@ function check(op, alg, length) {
18261854
}
18271855
return true;
18281856
}
1857+
case 'decapsulate':
1858+
case 'decrypt':
18291859
case 'encapsulate':
18301860
case 'encrypt':
18311861
case 'exportKey':
@@ -1837,7 +1867,8 @@ function check(op, alg, length) {
18371867
return true;
18381868
case 'deriveBits': {
18391869
if (normalizedAlgorithm.name === 'HKDF') {
1840-
require('internal/crypto/hkdf').validateHkdfDeriveBitsLength(length);
1870+
require('internal/crypto/hkdf')
1871+
.validateHkdfDeriveBitsLength(length, normalizedAlgorithm.hash);
18411872
}
18421873

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

‎lib/internal/crypto/webidl.js‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525
getCryptoKeyType,
2626
} = require('internal/crypto/keys');
2727
const {
28+
bigIntArrayToUnsignedInt,
2829
validateMaxBufferLength,
2930
getBufferSourceByteLength,
3031
getBufferSourceBytes,
@@ -41,6 +42,9 @@ const {
4142
type,
4243
} = require('internal/webidl');
4344

45+
const { getFipsCrypto } = internalBinding('crypto');
46+
const kRsaKeyGenMinimumModulusLength = getFipsCrypto() === 1 ? 2048 : 512;
47+
4448
function validateByteLength(buf, name, target) {
4549
if (getBufferSourceByteLength(buf) !== target) {
4650
throw lazyDOMException(
@@ -175,11 +179,33 @@ const dictRsaKeyGenParams = [
175179
key: 'modulusLength',
176180
converter: (V, opts) =>
177181
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+
},
178189
required: true,
179190
},
180191
{
181192
key: 'publicExponent',
182193
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+
},
183209
required: true,
184210
},
185211
];
@@ -639,20 +665,27 @@ converters.ContextParams = createDictionaryConverter(
639665
key: 'context',
640666
converter: converters.BufferSource,
641667
validator(V, dict) {
668+
const validateLength = (V) =>
669+
validateMaxBufferLength(V, 'ContextParams.context', 255);
670+
642671
if (process.features.openssl_is_boringssl) {
643-
this.validator = undefined;
672+
this.validator = validateLength;
644673
} else {
645674
let { 0: major, 1: minor } =
646675
StringPrototypeSplit(process.versions.openssl, '.');
647676
major = NumberParseInt(major, 10);
648677
minor = NumberParseInt(minor, 10);
649678
if (major > 3 || (major === 3 && minor >= 2)) {
650-
this.validator = undefined;
679+
this.validator = validateLength;
651680
} else {
652-
this.validator = validateZeroLength('ContextParams.context');
653-
this.validator(V, dict);
681+
const validateEmpty = validateZeroLength('ContextParams.context');
682+
this.validator = (V, dict) => {
683+
validateLength(V);
684+
validateEmpty(V, dict);
685+
};
654686
}
655687
}
688+
this.validator(V, dict);
656689
},
657690
},
658691
],

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

Lines changed: 32 additions & 0 deletions

0 commit comments

Comments
 (0)