Skip to content

Commit 9ce84b8

Browse files
panvapanva
committed
lib: improve Web Cryptography key validation ordering
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent ed05549 commit 9ce84b8

8 files changed

+127
-43
lines changed

‎lib/internal/crypto/webcrypto.js‎

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,14 @@ async function wrapKey(format, key, wrappingKey, algorithm) {
919919
} catch {
920920
algorithm = normalizeAlgorithm(algorithm, 'encrypt');
921921
}
922+
923+
if (algorithm.name !== wrappingKey[kAlgorithm].name ||
924+
!ArrayPrototypeIncludes(wrappingKey[kKeyUsages], 'wrapKey')) {
925+
throw lazyDOMException(
926+
'The requested operation is not valid for the provided key',
927+
'InvalidAccessError');
928+
}
929+
922930
let keyData = await FunctionPrototypeCall(exportKey, this, format, key);
923931

924932
if (format === 'jwk') {
@@ -997,6 +1005,13 @@ async function unwrapKey(
9971005

9981006
unwrappedKeyAlgo = normalizeAlgorithm(unwrappedKeyAlgo, 'importKey');
9991007

1008+
if (unwrapAlgo.name !== unwrappingKey[kAlgorithm].name ||
1009+
!ArrayPrototypeIncludes(unwrappingKey[kKeyUsages], 'unwrapKey')) {
1010+
throw lazyDOMException(
1011+
'The requested operation is not valid for the provided key',
1012+
'InvalidAccessError');
1013+
}
1014+
10001015
let keyData = await cipherOrWrap(
10011016
kWebCryptoCipherDecrypt,
10021017
unwrapAlgo,
@@ -1030,12 +1045,12 @@ async function signVerify(algorithm, key, data, signature) {
10301045
}
10311046
algorithm = normalizeAlgorithm(algorithm, usage);
10321047

1033-
if (!ArrayPrototypeIncludes(key[kKeyUsages], usage) ||
1034-
algorithm.name !== key[kAlgorithm].name) {
1048+
if (algorithm.name !== key[kAlgorithm].name)
1049+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1050+
1051+
if (!ArrayPrototypeIncludes(key[kKeyUsages], usage))
10351052
throw lazyDOMException(
1036-
`Unable to use this key to ${usage}`,
1037-
'InvalidAccessError');
1038-
}
1053+
`Unable to use this key to ${usage}`, 'InvalidAccessError');
10391054

10401055
switch (algorithm.name) {
10411056
case 'RSA-PSS':
@@ -1120,18 +1135,6 @@ async function verify(algorithm, key, signature, data) {
11201135
}
11211136

11221137
async function cipherOrWrap(mode, algorithm, key, data, op) {
1123-
// We use a Node.js style error here instead of a DOMException because
1124-
// the WebCrypto spec is not specific what kind of error is to be thrown
1125-
// in this case. Both Firefox and Chrome throw simple TypeErrors here.
1126-
// The key algorithm and cipher algorithm must match, and the
1127-
// key must have the proper usage.
1128-
if (key[kAlgorithm].name !== algorithm.name ||
1129-
!ArrayPrototypeIncludes(key[kKeyUsages], op)) {
1130-
throw lazyDOMException(
1131-
'The requested operation is not valid for the provided key',
1132-
'InvalidAccessError');
1133-
}
1134-
11351138
// While WebCrypto allows for larger input buffer sizes, we limit
11361139
// those to sizes that can fit within uint32_t because of limitations
11371140
// in the OpenSSL API.
@@ -1182,6 +1185,14 @@ async function encrypt(algorithm, key, data) {
11821185
});
11831186

11841187
algorithm = normalizeAlgorithm(algorithm, 'encrypt');
1188+
1189+
if (algorithm.name !== key[kAlgorithm].name ||
1190+
!ArrayPrototypeIncludes(key[kKeyUsages], 'encrypt')) {
1191+
throw lazyDOMException(
1192+
'The requested operation is not valid for the provided key',
1193+
'InvalidAccessError');
1194+
}
1195+
11851196
return await cipherOrWrap(
11861197
kWebCryptoCipherEncrypt,
11871198
algorithm,
@@ -1211,6 +1222,14 @@ async function decrypt(algorithm, key, data) {
12111222
});
12121223

12131224
algorithm = normalizeAlgorithm(algorithm, 'decrypt');
1225+
1226+
if (algorithm.name !== key[kAlgorithm].name ||
1227+
!ArrayPrototypeIncludes(key[kKeyUsages], 'decrypt')) {
1228+
throw lazyDOMException(
1229+
'The requested operation is not valid for the provided key',
1230+
'InvalidAccessError');
1231+
}
1232+
12141233
return await cipherOrWrap(
12151234
kWebCryptoCipherDecrypt,
12161235
algorithm,

‎test/parallel/test-webcrypto-sign-verify-ecdsa.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,17 @@ async function testVerify({ name,
8888
// Test failure when using the wrong algorithms
8989
await assert.rejects(
9090
subtle.verify({ name, hash }, hmacKey, signature, plaintext), {
91-
message: /Unable to use this key to verify/
91+
message: /Key algorithm mismatch/
9292
});
9393

9494
await assert.rejects(
9595
subtle.verify({ name, hash }, rsaKeys.publicKey, signature, plaintext), {
96-
message: /Unable to use this key to verify/
96+
message: /Key algorithm mismatch/
9797
});
9898

9999
await assert.rejects(
100100
subtle.verify({ name, hash }, okpKeys.publicKey, signature, plaintext), {
101-
message: /Unable to use this key to verify/
101+
message: /Key algorithm mismatch/
102102
});
103103

104104
// Test failure when signature is altered
@@ -210,17 +210,17 @@ async function testSign({ name,
210210
// Test failure when using the wrong algorithms
211211
await assert.rejects(
212212
subtle.sign({ name, hash }, hmacKey, plaintext), {
213-
message: /Unable to use this key to sign/
213+
message: /Key algorithm mismatch/
214214
});
215215

216216
await assert.rejects(
217217
subtle.sign({ name, hash }, rsaKeys.privateKey, plaintext), {
218-
message: /Unable to use this key to sign/
218+
message: /Key algorithm mismatch/
219219
});
220220

221221
await assert.rejects(
222222
subtle.sign({ name, hash }, okpKeys.privateKey, plaintext), {
223-
message: /Unable to use this key to sign/
223+
message: /Key algorithm mismatch/
224224
});
225225
}
226226

‎test/parallel/test-webcrypto-sign-verify-eddsa.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,17 +101,17 @@ async function testVerify({ name,
101101
// Test failure when using the wrong algorithms
102102
await assert.rejects(
103103
subtle.verify({ name, context }, hmacKey, signature, data), {
104-
message: /Unable to use this key to verify/
104+
message: /Key algorithm mismatch/
105105
});
106106

107107
await assert.rejects(
108108
subtle.verify({ name, context }, rsaKeys.publicKey, signature, data), {
109-
message: /Unable to use this key to verify/
109+
message: /Key algorithm mismatch/
110110
});
111111

112112
await assert.rejects(
113113
subtle.verify({ name, context }, ecKeys.publicKey, signature, data), {
114-
message: /Unable to use this key to verify/
114+
message: /Key algorithm mismatch/
115115
});
116116

117117
if (name === 'Ed448' && supportsContext) {
@@ -227,17 +227,17 @@ async function testSign({ name,
227227
// Test failure when using the wrong algorithms
228228
await assert.rejects(
229229
subtle.sign({ name, context }, hmacKey, data), {
230-
message: /Unable to use this key to sign/
230+
message: /Key algorithm mismatch/
231231
});
232232

233233
await assert.rejects(
234234
subtle.sign({ name, context }, rsaKeys.privateKey, data), {
235-
message: /Unable to use this key to sign/
235+
message: /Key algorithm mismatch/
236236
});
237237

238238
await assert.rejects(
239239
subtle.sign({ name, context }, ecKeys.privateKey, data), {
240-
message: /Unable to use this key to sign/
240+
message: /Key algorithm mismatch/
241241
});
242242

243243
if (name === 'Ed448' && supportsContext) {

‎test/parallel/test-webcrypto-sign-verify-hmac.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ async function testVerify({ hash,
6262
// Test failure when using the wrong algorithms
6363
await assert.rejects(
6464
subtle.verify({ name, hash }, rsaKeys.publicKey, signature, plaintext), {
65-
message: /Unable to use this key to verify/
65+
message: /Key algorithm mismatch/
6666
});
6767

6868
// Test failure when signature is altered
@@ -165,7 +165,7 @@ async function testSign({ hash,
165165
// Test failure when using the wrong algorithms
166166
await assert.rejects(
167167
subtle.sign({ name, hash }, rsaKeys.privateKey, plaintext), {
168-
message: /Unable to use this key to sign/
168+
message: /Key algorithm mismatch/
169169
});
170170
}
171171

‎test/parallel/test-webcrypto-sign-verify-kmac.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ async function testVerify({ algorithm,
6969
// Test failure when using the wrong algorithms
7070
await assert.rejects(
7171
subtle.verify(signParams, keyPair.publicKey, expected, data), {
72-
message: /Unable to use this key to verify/
72+
message: /Key algorithm mismatch/
7373
});
7474

7575
// Test failure when signature is altered
@@ -177,7 +177,7 @@ async function testSign({ algorithm,
177177
// Test failure when using the wrong algorithms
178178
await assert.rejects(
179179
subtle.sign(signParams, keyPair.privateKey, data), {
180-
message: /Unable to use this key to sign/
180+
message: /Key algorithm mismatch/
181181
});
182182
}
183183

‎test/parallel/test-webcrypto-sign-verify-ml-dsa.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,17 @@ async function testVerify({ name,
8686
// Test failure when using the wrong algorithms
8787
await assert.rejects(
8888
subtle.verify({ name, context }, hmacKey, signature, data), {
89-
message: /Unable to use this key to verify/
89+
message: /Key algorithm mismatch/
9090
});
9191

9292
await assert.rejects(
9393
subtle.verify({ name, context }, rsaKeys.publicKey, signature, data), {
94-
message: /Unable to use this key to verify/
94+
message: /Key algorithm mismatch/
9595
});
9696

9797
await assert.rejects(
9898
subtle.verify({ name, context }, ecKeys.publicKey, signature, data), {
99-
message: /Unable to use this key to verify/
99+
message: /Key algorithm mismatch/
100100
});
101101

102102
// Test failure when too long context
@@ -194,17 +194,17 @@ async function testSign({ name,
194194
// Test failure when using the wrong algorithms
195195
await assert.rejects(
196196
subtle.sign({ name, context }, hmacKey, data), {
197-
message: /Unable to use this key to sign/
197+
message: /Key algorithm mismatch/
198198
});
199199

200200
await assert.rejects(
201201
subtle.sign({ name, context }, rsaKeys.privateKey, data), {
202-
message: /Unable to use this key to sign/
202+
message: /Key algorithm mismatch/
203203
});
204204

205205
await assert.rejects(
206206
subtle.sign({ name, context }, ecKeys.privateKey, data), {
207-
message: /Unable to use this key to sign/
207+
message: /Key algorithm mismatch/
208208
});
209209

210210
// Test failure when too long context

‎test/parallel/test-webcrypto-sign-verify-rsa.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ async function testVerify({
8282
// Test failure when using the wrong algorithms
8383
await assert.rejects(
8484
subtle.verify(algorithm, hmacKey, signature, plaintext), {
85-
message: /Unable to use this key to verify/
85+
message: /Key algorithm mismatch/
8686
});
8787

8888
await assert.rejects(
8989
subtle.verify(algorithm, ecdsaKeys.publicKey, signature, plaintext), {
90-
message: /Unable to use this key to verify/
90+
message: /Key algorithm mismatch/
9191
});
9292

9393
// Test failure when signature is altered
@@ -185,12 +185,12 @@ async function testSign({
185185
// Test failure when using the wrong algorithms
186186
await assert.rejects(
187187
subtle.sign(algorithm, hmacKey, plaintext), {
188-
message: /Unable to use this key to sign/
188+
message: /Key algorithm mismatch/
189189
});
190190

191191
await assert.rejects(
192192
subtle.sign(algorithm, ecdsaKeys.privateKey, plaintext), {
193-
message: /Unable to use this key to sign/
193+
message: /Key algorithm mismatch/
194194
});
195195
}
196196

‎test/parallel/test-webcrypto-wrap-unwrap.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,3 +378,68 @@ function testWrapping(name, keys) {
378378
});
379379
await Promise.all(variations);
380380
})().then(common.mustCall());
381+
382+
// Test that wrapKey validates the wrapping key's algorithm and usage
383+
// before attempting to export the key to be wrapped.
384+
// Spec: https://w3c.github.io/webcrypto/#SubtleCrypto-method-wrapKey
385+
// Steps 9-10 (wrapping key checks) must precede step 12 (exportKey).
386+
(async function() {
387+
const hmacKey = await subtle.generateKey(
388+
{ name: 'HMAC', hash: 'SHA-256' },
389+
true,
390+
['sign', 'verify'],
391+
);
392+
393+
const ecKey = await subtle.generateKey(
394+
{ name: 'ECDSA', namedCurve: 'P-256' },
395+
true,
396+
['sign', 'verify'],
397+
);
398+
399+
// Wrong algorithm: wrapping key is HMAC but algorithm says AES-GCM.
400+
// Even though exporting ecKey.privateKey as 'spki' would also fail
401+
// (wrong key type for spki), the wrapping key check must come first.
402+
await assert.rejects(
403+
subtle.wrapKey('spki', ecKey.privateKey, hmacKey, {
404+
name: 'AES-GCM',
405+
iv: new Uint8Array(12),
406+
}), {
407+
name: 'InvalidAccessError',
408+
message: 'The requested operation is not valid for the provided key',
409+
});
410+
411+
// Missing wrapKey usage: aesKey only has encrypt/decrypt, not wrapKey.
412+
// Even though exporting ecKey.privateKey as 'spki' would also fail,
413+
// the usage check must come first.
414+
const aesKey = await subtle.generateKey(
415+
{ name: 'AES-GCM', length: 128 },
416+
true,
417+
['encrypt', 'decrypt'],
418+
);
419+
420+
await assert.rejects(
421+
subtle.wrapKey('spki', ecKey.privateKey, aesKey, {
422+
name: 'AES-GCM',
423+
iv: new Uint8Array(12),
424+
}), {
425+
name: 'InvalidAccessError',
426+
message: 'The requested operation is not valid for the provided key',
427+
});
428+
429+
// Correct wrapping key algorithm and usage results in the expected
430+
// exportKey error (not the wrapping key validation error).
431+
const wrapKey = await subtle.generateKey(
432+
{ name: 'AES-GCM', length: 128 },
433+
true,
434+
['wrapKey'],
435+
);
436+
437+
await assert.rejects(
438+
subtle.wrapKey('spki', ecKey.privateKey, wrapKey, {
439+
name: 'AES-GCM',
440+
iv: new Uint8Array(12),
441+
}), {
442+
// exportKey('spki', privateKey) throws NotSupportedError
443+
name: 'NotSupportedError',
444+
});
445+
})().then(common.mustCall());

0 commit comments

Comments
 (0)