Skip to content

Commit b10ac9a

Browse files
panvaaduh95
panva
authored and
aduh95
committed
src: refactor SubtleCrypto algorithm and length validations
PR-URL: #57273 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Mattias Buelens <mattias@buelens.com>
1 parent 90cd780 commit b10ac9a

17 files changed

+208
-180
lines changed

‎lib/internal/crypto/aes.js

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ const {
5858
generateKey: _generateKey,
5959
} = require('internal/crypto/keygen');
6060

61-
const kMaxCounterLength = 128;
6261
const kTagLengths = [32, 64, 96, 104, 112, 120, 128];
6362
const generateKey = promisify(_generateKey);
6463

@@ -109,35 +108,43 @@ function getVariant(name, length) {
109108
}
110109
}
111110

112-
function asyncAesCtrCipher(mode, key, data, { counter, length }) {
113-
validateByteLength(counter, 'algorithm.counter', 16);
111+
function validateAesCtrAlgorithm(algorithm) {
112+
validateByteLength(algorithm.counter, 'algorithm.counter', 16);
114113
// The length must specify an integer between 1 and 128. While
115114
// there is no default, this should typically be 64.
116-
if (length === 0 || length > kMaxCounterLength) {
115+
if (algorithm.length === 0 || algorithm.length > 128) {
117116
throw lazyDOMException(
118117
'AES-CTR algorithm.length must be between 1 and 128',
119118
'OperationError');
120119
}
120+
}
121+
122+
function asyncAesCtrCipher(mode, key, data, algorithm) {
123+
validateAesCtrAlgorithm(algorithm);
121124

122125
return jobPromise(() => new AESCipherJob(
123126
kCryptoJobAsync,
124127
mode,
125128
key[kKeyObject][kHandle],
126129
data,
127130
getVariant('AES-CTR', key.algorithm.length),
128-
counter,
129-
length));
131+
algorithm.counter,
132+
algorithm.length));
133+
}
134+
135+
function validateAesCbcAlgorithm(algorithm) {
136+
validateByteLength(algorithm.iv, 'algorithm.iv', 16);
130137
}
131138

132-
function asyncAesCbcCipher(mode, key, data, { iv }) {
133-
validateByteLength(iv, 'algorithm.iv', 16);
139+
function asyncAesCbcCipher(mode, key, data, algorithm) {
140+
validateAesCbcAlgorithm(algorithm);
134141
return jobPromise(() => new AESCipherJob(
135142
kCryptoJobAsync,
136143
mode,
137144
key[kKeyObject][kHandle],
138145
data,
139146
getVariant('AES-CBC', key.algorithm.length),
140-
iv));
147+
algorithm.iv));
141148
}
142149

143150
function asyncAesKwCipher(mode, key, data) {
@@ -149,24 +156,25 @@ function asyncAesKwCipher(mode, key, data) {
149156
getVariant('AES-KW', key.algorithm.length)));
150157
}
151158

152-
function asyncAesGcmCipher(
153-
mode,
154-
key,
155-
data,
156-
{ iv, additionalData, tagLength = 128 }) {
157-
if (!ArrayPrototypeIncludes(kTagLengths, tagLength)) {
158-
return PromiseReject(lazyDOMException(
159-
`${tagLength} is not a valid AES-GCM tag length`,
160-
'OperationError'));
159+
function validateAesGcmAlgorithm(algorithm) {
160+
if (!ArrayPrototypeIncludes(kTagLengths, algorithm.tagLength)) {
161+
throw lazyDOMException(
162+
`${algorithm.tagLength} is not a valid AES-GCM tag length`,
163+
'OperationError');
161164
}
162165

163-
validateMaxBufferLength(iv, 'algorithm.iv');
166+
validateMaxBufferLength(algorithm.iv, 'algorithm.iv');
164167

165-
if (additionalData !== undefined) {
166-
validateMaxBufferLength(additionalData, 'algorithm.additionalData');
168+
if (algorithm.additionalData !== undefined) {
169+
validateMaxBufferLength(algorithm.additionalData, 'algorithm.additionalData');
167170
}
171+
}
168172

169-
const tagByteLength = MathFloor(tagLength / 8);
173+
function asyncAesGcmCipher(mode, key, data, algorithm) {
174+
algorithm.tagLength ??= 128;
175+
validateAesGcmAlgorithm(algorithm);
176+
177+
const tagByteLength = MathFloor(algorithm.tagLength / 8);
170178
let tag;
171179
switch (mode) {
172180
case kWebCryptoCipherDecrypt: {
@@ -198,9 +206,9 @@ function asyncAesGcmCipher(
198206
key[kKeyObject][kHandle],
199207
data,
200208
getVariant('AES-GCM', key.algorithm.length),
201-
iv,
209+
algorithm.iv,
202210
tag,
203-
additionalData));
211+
algorithm.additionalData));
204212
}
205213

206214
function aesCipher(mode, key, data, algorithm) {
@@ -212,13 +220,17 @@ function aesCipher(mode, key, data, algorithm) {
212220
}
213221
}
214222

215-
async function aesGenerateKey(algorithm, extractable, keyUsages) {
216-
const { name, length } = algorithm;
217-
if (!ArrayPrototypeIncludes(kAesKeyLengths, length)) {
223+
function validateAesGenerateKeyAlgorithm(algorithm) {
224+
if (!ArrayPrototypeIncludes(kAesKeyLengths, algorithm.length)) {
218225
throw lazyDOMException(
219226
'AES key length must be 128, 192, or 256 bits',
220227
'OperationError');
221228
}
229+
}
230+
231+
async function aesGenerateKey(algorithm, extractable, keyUsages) {
232+
validateAesGenerateKeyAlgorithm(algorithm);
233+
const { name, length } = algorithm;
222234

223235
const checkUsages = ['wrapKey', 'unwrapKey'];
224236
if (name !== 'AES-KW')

‎lib/internal/crypto/cfrg.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -329,18 +329,21 @@ function cfrgImportKey(
329329
extractable);
330330
}
331331

332-
function eddsaSignVerify(key, data, { name, context }, signature) {
332+
function validateEdDSASignVerifyAlgorithm(algorithm) {
333+
if (algorithm.name === 'Ed448' && algorithm.context?.byteLength) {
334+
throw lazyDOMException(
335+
'Non zero-length context is not yet supported.', 'NotSupportedError');
336+
}
337+
}
338+
339+
function eddsaSignVerify(key, data, algorithm, signature) {
340+
validateEdDSASignVerifyAlgorithm(algorithm);
333341
const mode = signature === undefined ? kSignJobModeSign : kSignJobModeVerify;
334342
const type = mode === kSignJobModeSign ? 'private' : 'public';
335343

336344
if (key.type !== type)
337345
throw lazyDOMException(`Key must be a ${type} key`, 'InvalidAccessError');
338346

339-
if (name === 'Ed448' && context?.byteLength) {
340-
throw lazyDOMException(
341-
'Non zero-length context is not yet supported.', 'NotSupportedError');
342-
}
343-
344347
return jobPromise(() => new SignJob(
345348
kCryptoJobAsync,
346349
mode,

‎lib/internal/crypto/diffiehellman.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -298,28 +298,28 @@ function diffieHellman(options) {
298298

299299
let masks;
300300

301+
function validateEcdhDeriveBitsAlgorithmAndLength(algorithm, length) {
302+
if (algorithm.public.type !== 'public') {
303+
throw lazyDOMException(
304+
'algorithm.public must be a public key', 'InvalidAccessError');
305+
}
306+
307+
if (algorithm.name !== algorithm.public.algorithm.name) {
308+
throw lazyDOMException(`algorithm.public must be an ${algorithm.name} key`, 'InvalidAccessError');
309+
}
310+
}
311+
301312
// The ecdhDeriveBits function is part of the Web Crypto API and serves both
302313
// deriveKeys and deriveBits functions.
303314
async function ecdhDeriveBits(algorithm, baseKey, length) {
315+
validateEcdhDeriveBitsAlgorithmAndLength(algorithm, length);
304316
const { 'public': key } = algorithm;
305317

306-
if (key.type !== 'public') {
307-
throw lazyDOMException(
308-
'algorithm.public must be a public key', 'InvalidAccessError');
309-
}
310318
if (baseKey.type !== 'private') {
311319
throw lazyDOMException(
312320
'baseKey must be a private key', 'InvalidAccessError');
313321
}
314322

315-
if (
316-
key.algorithm.name !== 'ECDH' &&
317-
key.algorithm.name !== 'X25519' &&
318-
key.algorithm.name !== 'X448'
319-
) {
320-
throw lazyDOMException('Keys must be ECDH, X25519, or X448 keys', 'InvalidAccessError');
321-
}
322-
323323
if (key.algorithm.name !== baseKey.algorithm.name) {
324324
throw lazyDOMException(
325325
'The public and private keys must be of the same type',

‎lib/internal/crypto/ec.js

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
'use strict';
22

33
const {
4-
ArrayPrototypeIncludes,
5-
ObjectKeys,
4+
ObjectPrototypeHasOwnProperty,
65
SafeSet,
76
} = primordials;
87

@@ -77,14 +76,17 @@ function createECPublicKeyRaw(namedCurve, keyData) {
7776
return new PublicKeyObject(handle);
7877
}
7978

80-
async function ecGenerateKey(algorithm, extractable, keyUsages) {
81-
const { name, namedCurve } = algorithm;
82-
83-
if (!ArrayPrototypeIncludes(ObjectKeys(kNamedCurveAliases), namedCurve)) {
79+
function validateEcKeyAlgorithm(algorithm) {
80+
if (!ObjectPrototypeHasOwnProperty(kNamedCurveAliases, algorithm.namedCurve)) {
8481
throw lazyDOMException(
8582
'Unrecognized namedCurve',
8683
'NotSupportedError');
8784
}
85+
}
86+
87+
async function ecGenerateKey(algorithm, extractable, keyUsages) {
88+
validateEcKeyAlgorithm(algorithm);
89+
const { name, namedCurve } = algorithm;
8890

8991
const usageSet = new SafeSet(keyUsages);
9092
switch (name) {
@@ -154,16 +156,11 @@ function ecImportKey(
154156
keyData,
155157
algorithm,
156158
extractable,
157-
keyUsages) {
158-
159+
keyUsages,
160+
) {
161+
validateEcKeyAlgorithm(algorithm);
159162
const { name, namedCurve } = algorithm;
160163

161-
if (!ArrayPrototypeIncludes(ObjectKeys(kNamedCurveAliases), namedCurve)) {
162-
throw lazyDOMException(
163-
'Unrecognized namedCurve',
164-
'NotSupportedError');
165-
}
166-
167164
let keyObject;
168165
const usagesSet = new SafeSet(keyUsages);
169166
switch (format) {

‎lib/internal/crypto/hkdf.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,22 @@ function hkdfSync(hash, key, salt, info, length) {
138138
}
139139

140140
const hkdfPromise = promisify(hkdf);
141-
async function hkdfDeriveBits(algorithm, baseKey, length) {
142-
const { hash, salt, info } = algorithm;
143-
144-
if (length === 0)
145-
return new ArrayBuffer(0);
141+
function validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length) {
146142
if (length === null)
147143
throw lazyDOMException('length cannot be null', 'OperationError');
148144
if (length % 8) {
149145
throw lazyDOMException(
150146
'length must be a multiple of 8',
151147
'OperationError');
152148
}
149+
}
150+
151+
async function hkdfDeriveBits(algorithm, baseKey, length) {
152+
validateHkdfDeriveBitsAlgorithmAndLength(algorithm, length);
153+
const { hash, salt, info } = algorithm;
154+
155+
if (length === 0)
156+
return new ArrayBuffer(0);
153157

154158
try {
155159
return await hkdfPromise(

‎lib/internal/crypto/keys.js

Lines changed: 13 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -895,12 +895,14 @@ function isCryptoKey(obj) {
895895
}
896896

897897
function importGenericSecretKey(
898-
{ name, length },
898+
algorithm,
899899
format,
900900
keyData,
901901
extractable,
902-
keyUsages) {
902+
keyUsages,
903+
) {
903904
const usagesSet = new SafeSet(keyUsages);
905+
const { name } = algorithm;
904906
if (extractable)
905907
throw lazyDOMException(`${name} keys are not extractable`, 'SyntaxError');
906908

@@ -910,47 +912,22 @@ function importGenericSecretKey(
910912
'SyntaxError');
911913
}
912914

915+
let keyObject;
913916
switch (format) {
914917
case 'KeyObject': {
915-
if (hasAnyNotIn(usagesSet, ['deriveKey', 'deriveBits'])) {
916-
throw lazyDOMException(
917-
`Unsupported key usage for a ${name} key`,
918-
'SyntaxError');
919-
}
920-
921-
const checkLength = keyData.symmetricKeySize * 8;
922-
923-
// The Web Crypto spec allows for key lengths that are not multiples of
924-
// 8. We don't. Our check here is stricter than that defined by the spec
925-
// in that we require that algorithm.length match keyData.length * 8 if
926-
// algorithm.length is specified.
927-
if (length !== undefined && length !== checkLength) {
928-
throw lazyDOMException('Invalid key length', 'DataError');
929-
}
930-
return new InternalCryptoKey(keyData, { name }, keyUsages, false);
918+
keyObject = keyData;
919+
break;
931920
}
932921
case 'raw': {
933-
if (hasAnyNotIn(usagesSet, ['deriveKey', 'deriveBits'])) {
934-
throw lazyDOMException(
935-
`Unsupported key usage for a ${name} key`,
936-
'SyntaxError');
937-
}
938-
939-
const checkLength = keyData.byteLength * 8;
940-
941-
// The Web Crypto spec allows for key lengths that are not multiples of
942-
// 8. We don't. Our check here is stricter than that defined by the spec
943-
// in that we require that algorithm.length match keyData.length * 8 if
944-
// algorithm.length is specified.
945-
if (length !== undefined && length !== checkLength) {
946-
throw lazyDOMException('Invalid key length', 'DataError');
947-
}
948-
949-
const keyObject = createSecretKey(keyData);
950-
return new InternalCryptoKey(keyObject, { name }, keyUsages, false);
922+
keyObject = createSecretKey(keyData);
923+
break;
951924
}
952925
}
953926

927+
if (keyObject) {
928+
return new InternalCryptoKey(keyObject, { name }, keyUsages, false);
929+
}
930+
954931
throw lazyDOMException(
955932
`Unable to import ${name} key with format ${format}`,
956933
'NotSupportedError');

0 commit comments

Comments
 (0)