Skip to content

Commit 51cdf20

Browse files
panvapanva
committed
lib,crypto: add early structural JWK validation
Tightens `validateJwk` to require the per-`kty` string members up front (before switching to C++), short-circuiting with the same `Invalid keyData` message and `DataError` when the JWK is missing required fields or passes a non-string value for one. In theory non-strings are already rejected by WebIDL's JWK converter but this doesn't hurt. - RSA: requires `n`, `e`; if `d` is present, also requires `p`, `q`, `dp`, `dq`, and `qi`. - EC: requires `crv`, `x`, `y`; optional `d`. - OKP: requires `crv`, `x`; optional `d`. - oct: requires `k`. - AKP: requires `alg`, `pub`; optional `priv`. Four export/import negative tests update their expected error text from the later "Invalid JWK … Parameter and algorithm name mismatch" to the new short-circuit "Invalid keyData" (for the case where `crv`/`alg` is missing entirely). A new `{ kty: 'oct' }` missing-`k` negative is added for ChaCha20-Poly1305. The tests check error messages but the error class (DataError/DOMException) is the same everywhere. Signed-off-by: Filip Skokan <[email protected]>
1 parent fc5b670 commit 51cdf20

6 files changed

Lines changed: 53 additions & 9 deletions

‎lib/internal/crypto/webcrypto_util.js‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,53 @@ function importDerKey(keyData, isPublic) {
3333
}
3434

3535
function validateJwk(keyData, kty, extractable, usagesSet, expectedUse) {
36-
if (!keyData.kty)
36+
if (typeof keyData.kty !== 'string')
3737
throw lazyDOMException('Invalid keyData', 'DataError');
3838
if (keyData.kty !== kty)
3939
throw lazyDOMException('Invalid JWK "kty" Parameter', 'DataError');
40+
switch (kty) {
41+
case 'RSA':
42+
if (typeof keyData.n !== 'string' ||
43+
typeof keyData.e !== 'string' ||
44+
(keyData.d !== undefined && typeof keyData.d !== 'string'))
45+
throw lazyDOMException('Invalid keyData', 'DataError');
46+
if (typeof keyData.d === 'string' &&
47+
(typeof keyData.p !== 'string' ||
48+
typeof keyData.q !== 'string' ||
49+
typeof keyData.dp !== 'string' ||
50+
typeof keyData.dq !== 'string' ||
51+
typeof keyData.qi !== 'string'))
52+
throw lazyDOMException('Invalid keyData', 'DataError');
53+
break;
54+
case 'EC':
55+
if (typeof keyData.crv !== 'string' ||
56+
typeof keyData.x !== 'string' ||
57+
typeof keyData.y !== 'string' ||
58+
(keyData.d !== undefined && typeof keyData.d !== 'string'))
59+
throw lazyDOMException('Invalid keyData', 'DataError');
60+
break;
61+
case 'OKP':
62+
if (typeof keyData.crv !== 'string' ||
63+
typeof keyData.x !== 'string' ||
64+
(keyData.d !== undefined && typeof keyData.d !== 'string'))
65+
throw lazyDOMException('Invalid keyData', 'DataError');
66+
break;
67+
case 'oct':
68+
if (typeof keyData.k !== 'string')
69+
throw lazyDOMException('Invalid keyData', 'DataError');
70+
break;
71+
case 'AKP':
72+
if (typeof keyData.alg !== 'string' ||
73+
typeof keyData.pub !== 'string' ||
74+
(keyData.priv !== undefined && typeof keyData.priv !== 'string'))
75+
throw lazyDOMException('Invalid keyData', 'DataError');
76+
break;
77+
default: {
78+
// It is not possible to get here because all possible cases are handled above.
79+
const assert = require('internal/assert');
80+
assert.fail('Unreachable code');
81+
}
82+
}
4083
if (usagesSet.size > 0 && keyData.use !== undefined) {
4184
if (keyData.use !== expectedUse)
4285
throw lazyDOMException('Invalid JWK "use" Parameter', 'DataError');

‎test/parallel/test-webcrypto-encrypt-decrypt-chacha20-poly1305.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) {
220220

221221
// JWK error conditions
222222
const jwkTests = [
223+
[{ kty: 'oct' }, /Invalid keyData/],
223224
[{ k: baseJwk.k }, /Invalid keyData/],
224225
[{ ...baseJwk, kty: 'RSA' }, /Invalid JWK "kty" Parameter/],
225226
[{ ...baseJwk, use: 'sig' }, /Invalid JWK "use" Parameter/],

‎test/parallel/test-webcrypto-export-import-cfrg.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
355355
{ name },
356356
extractable,
357357
publicUsages),
358-
{ message: 'JWK "crv" Parameter and algorithm name mismatch' });
358+
{ message: crv ? 'JWK "crv" Parameter and algorithm name mismatch' : 'Invalid keyData' });
359359

360360
await assert.rejects(
361361
subtle.importKey(
@@ -364,7 +364,7 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
364364
{ name },
365365
extractable,
366366
privateUsages),
367-
{ message: 'JWK "crv" Parameter and algorithm name mismatch' });
367+
{ message: crv ? 'JWK "crv" Parameter and algorithm name mismatch' : 'Invalid keyData' });
368368
}
369369

370370
await assert.rejects(

‎test/parallel/test-webcrypto-export-import-ec.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ async function testImportJwk(
322322
{ name, namedCurve },
323323
extractable,
324324
publicUsages),
325-
{ message: 'JWK "crv" does not match the requested algorithm' });
325+
{ message: crv ? 'JWK "crv" does not match the requested algorithm' : 'Invalid keyData' });
326326

327327
await assert.rejects(
328328
subtle.importKey(
@@ -331,7 +331,7 @@ async function testImportJwk(
331331
{ name, namedCurve },
332332
extractable,
333333
privateUsages),
334-
{ message: 'JWK "crv" does not match the requested algorithm' });
334+
{ message: crv ? 'JWK "crv" does not match the requested algorithm' : 'Invalid keyData' });
335335
}
336336

337337
await assert.rejects(

‎test/parallel/test-webcrypto-export-import-ml-dsa.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
347347
{ name },
348348
extractable,
349349
publicUsages),
350-
{ message: 'JWK "alg" Parameter and algorithm name mismatch' });
350+
{ message: alg ? 'JWK "alg" Parameter and algorithm name mismatch' : 'Invalid keyData' });
351351

352352
await assert.rejects(
353353
subtle.importKey(
@@ -356,7 +356,7 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
356356
{ name },
357357
extractable,
358358
privateUsages),
359-
{ message: 'JWK "alg" Parameter and algorithm name mismatch' });
359+
{ message: alg ? 'JWK "alg" Parameter and algorithm name mismatch' : 'Invalid keyData' });
360360
}
361361

362362
await assert.rejects(

‎test/parallel/test-webcrypto-export-import-ml-kem.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
423423
{ name },
424424
extractable,
425425
publicUsages),
426-
{ message: 'JWK "alg" Parameter and algorithm name mismatch' });
426+
{ message: alg ? 'JWK "alg" Parameter and algorithm name mismatch' : 'Invalid keyData' });
427427

428428
await assert.rejects(
429429
subtle.importKey(
@@ -432,7 +432,7 @@ async function testImportJwk({ name, publicUsages, privateUsages }, extractable)
432432
{ name },
433433
extractable,
434434
privateUsages),
435-
{ message: 'JWK "alg" Parameter and algorithm name mismatch' });
435+
{ message: alg ? 'JWK "alg" Parameter and algorithm name mismatch' : 'Invalid keyData' });
436436
}
437437

438438
await assert.rejects(

0 commit comments

Comments
 (0)