Skip to content

Commit 4cb1f28

Browse files
panvapanva
authored
lib: improve Web Cryptography key validation ordering
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: #62749 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent b178842 commit 4cb1f28

12 files changed

+201
-57
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+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
925+
926+
if (!ArrayPrototypeIncludes(wrappingKey[kKeyUsages], 'wrapKey'))
927+
throw lazyDOMException(
928+
'Unable to use this key to wrapKey', 'InvalidAccessError');
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+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1010+
1011+
if (!ArrayPrototypeIncludes(unwrappingKey[kKeyUsages], 'unwrapKey'))
1012+
throw lazyDOMException(
1013+
'Unable to use this key to unwrapKey', 'InvalidAccessError');
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+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1191+
1192+
if (!ArrayPrototypeIncludes(key[kKeyUsages], 'encrypt'))
1193+
throw lazyDOMException(
1194+
'Unable to use this key to encrypt', 'InvalidAccessError');
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+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1228+
1229+
if (!ArrayPrototypeIncludes(key[kKeyUsages], 'decrypt'))
1230+
throw lazyDOMException(
1231+
'Unable to use this key to decrypt', 'InvalidAccessError');
1232+
12141233
return await cipherOrWrap(
12151234
kWebCryptoCipherDecrypt,
12161235
algorithm,

‎test/parallel/test-webcrypto-encrypt-decrypt-aes.js‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ async function testEncryptNoEncrypt({ keyBuffer, algorithm, plaintext }) {
4949
['decrypt']);
5050

5151
return assert.rejects(subtle.encrypt(algorithm, key, plaintext), {
52-
message: /The requested operation is not valid for the provided key/
52+
message: /Unable to use this key to encrypt/
5353
});
5454
}
5555

@@ -65,7 +65,7 @@ async function testEncryptNoDecrypt({ keyBuffer, algorithm, plaintext }) {
6565
const output = await subtle.encrypt(algorithm, key, plaintext);
6666

6767
return assert.rejects(subtle.decrypt(algorithm, key, output), {
68-
message: /The requested operation is not valid for the provided key/
68+
message: /Unable to use this key to decrypt/
6969
});
7070
}
7171

@@ -80,7 +80,23 @@ async function testEncryptWrongAlg({ keyBuffer, algorithm, plaintext }, alg) {
8080
['encrypt']);
8181

8282
return assert.rejects(subtle.encrypt(algorithm, key, plaintext), {
83-
message: /The requested operation is not valid for the provided key/
83+
message: /Key algorithm mismatch/
84+
});
85+
}
86+
87+
async function testDecryptWrongAlg({ keyBuffer, algorithm, result }, alg) {
88+
if (result === undefined) return;
89+
assert.notStrictEqual(algorithm.name, alg);
90+
const keyFormat = alg === 'AES-OCB' ? 'raw-secret' : 'raw';
91+
const key = await subtle.importKey(
92+
keyFormat,
93+
keyBuffer,
94+
{ name: alg },
95+
false,
96+
['decrypt']);
97+
98+
return assert.rejects(subtle.decrypt(algorithm, key, result), {
99+
message: /Key algorithm mismatch/
84100
});
85101
}
86102

@@ -112,6 +128,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) {
112128
variations.push(testEncryptNoEncrypt(vector));
113129
variations.push(testEncryptNoDecrypt(vector));
114130
variations.push(testEncryptWrongAlg(vector, 'AES-CTR'));
131+
variations.push(testDecryptWrongAlg(vector, 'AES-CTR'));
115132
});
116133

117134
failing.forEach((vector) => {
@@ -149,6 +166,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) {
149166
variations.push(testEncryptNoEncrypt(vector));
150167
variations.push(testEncryptNoDecrypt(vector));
151168
variations.push(testEncryptWrongAlg(vector, 'AES-CBC'));
169+
variations.push(testDecryptWrongAlg(vector, 'AES-CBC'));
152170
});
153171

154172
// TODO(@jasnell): These fail for different reasons. Need to
@@ -188,6 +206,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) {
188206
variations.push(testEncryptNoEncrypt(vector));
189207
variations.push(testEncryptNoDecrypt(vector));
190208
variations.push(testEncryptWrongAlg(vector, 'AES-CBC'));
209+
variations.push(testDecryptWrongAlg(vector, 'AES-CBC'));
191210
});
192211

193212
failing.forEach((vector) => {
@@ -225,6 +244,7 @@ if (hasOpenSSL(3)) {
225244
variations.push(testEncryptNoEncrypt(vector));
226245
variations.push(testEncryptNoDecrypt(vector));
227246
variations.push(testEncryptWrongAlg(vector, 'AES-GCM'));
247+
variations.push(testDecryptWrongAlg(vector, 'AES-GCM'));
228248
});
229249

230250
failing.forEach((vector) => {

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ async function testEncryptNoEncrypt({ keyBuffer, algorithm, plaintext }) {
4848
['decrypt']);
4949

5050
return assert.rejects(subtle.encrypt(algorithm, key, plaintext), {
51-
message: /The requested operation is not valid for the provided key/
51+
message: /Unable to use this key to encrypt/
5252
});
5353
}
5454

@@ -63,7 +63,7 @@ async function testEncryptNoDecrypt({ keyBuffer, algorithm, plaintext }) {
6363
const output = await subtle.encrypt(algorithm, key, plaintext);
6464

6565
return assert.rejects(subtle.decrypt(algorithm, key, output), {
66-
message: /The requested operation is not valid for the provided key/
66+
message: /Unable to use this key to decrypt/
6767
});
6868
}
6969

@@ -77,7 +77,22 @@ async function testEncryptWrongAlg({ keyBuffer, algorithm, plaintext }, alg) {
7777
['encrypt']);
7878

7979
return assert.rejects(subtle.encrypt(algorithm, key, plaintext), {
80-
message: /The requested operation is not valid for the provided key/
80+
message: /Key algorithm mismatch/
81+
});
82+
}
83+
84+
async function testDecryptWrongAlg({ keyBuffer, algorithm, result }, alg) {
85+
if (result === undefined) return;
86+
assert.notStrictEqual(algorithm.name, alg);
87+
const key = await subtle.importKey(
88+
'raw-secret',
89+
keyBuffer,
90+
{ name: alg },
91+
false,
92+
['decrypt']);
93+
94+
return assert.rejects(subtle.decrypt(algorithm, key, result), {
95+
message: /Key algorithm mismatch/
8196
});
8297
}
8398

@@ -107,6 +122,7 @@ async function testDecrypt({ keyBuffer, algorithm, result }) {
107122
variations.push(testEncryptNoEncrypt(vector));
108123
variations.push(testEncryptNoDecrypt(vector));
109124
variations.push(testEncryptWrongAlg(vector, 'AES-GCM'));
125+
variations.push(testDecryptWrongAlg(vector, 'AES-GCM'));
110126
});
111127

112128
failing.forEach((vector) => {

‎test/parallel/test-webcrypto-encrypt-decrypt-rsa.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ async function testEncryptionWrongKey({ algorithm,
147147
['decrypt']);
148148
return assert.rejects(
149149
subtle.encrypt(algorithm, privateKey, plaintext), {
150-
message: /The requested operation is not valid/
150+
message: /Unable to use this key to encrypt/
151151
});
152152
}
153153

@@ -167,7 +167,7 @@ async function testEncryptionBadUsage({ algorithm,
167167
['decrypt']);
168168
return assert.rejects(
169169
subtle.encrypt(algorithm, publicKey, plaintext), {
170-
message: /The requested operation is not valid/
170+
message: /Unable to use this key to encrypt/
171171
});
172172
}
173173

@@ -191,7 +191,7 @@ async function testDecryptionWrongKey({ ciphertext,
191191

192192
return assert.rejects(
193193
subtle.decrypt(algorithm, publicKey, ciphertext), {
194-
message: /The requested operation is not valid/
194+
message: /Unable to use this key to decrypt/
195195
});
196196
}
197197

@@ -215,7 +215,7 @@ async function testDecryptionBadUsage({ ciphertext,
215215

216216
return assert.rejects(
217217
subtle.decrypt(algorithm, publicKey, ciphertext), {
218-
message: /The requested operation is not valid/
218+
message: /Unable to use this key to decrypt/
219219
});
220220
}
221221

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@ const { subtle } = globalThis.crypto;
4343
name: 'RSA-OAEP',
4444
}, privateKey, buf), {
4545
name: 'InvalidAccessError',
46-
message: 'The requested operation is not valid for the provided key'
46+
message: 'Unable to use this key to encrypt'
4747
});
4848

4949
await assert.rejects(() => subtle.decrypt({
5050
name: 'RSA-OAEP',
5151
}, publicKey, ciphertext), {
5252
name: 'InvalidAccessError',
53-
message: 'The requested operation is not valid for the provided key'
53+
message: 'Unable to use this key to decrypt'
5454
});
5555
}
5656

@@ -88,14 +88,14 @@ if (!process.features.openssl_is_boringssl) {
8888
name: 'RSA-OAEP',
8989
}, privateKey, buf), {
9090
name: 'InvalidAccessError',
91-
message: 'The requested operation is not valid for the provided key'
91+
message: 'Unable to use this key to encrypt'
9292
});
9393

9494
await assert.rejects(() => subtle.decrypt({
9595
name: 'RSA-OAEP',
9696
}, publicKey, ciphertext), {
9797
name: 'InvalidAccessError',
98-
message: 'The requested operation is not valid for the provided key'
98+
message: 'Unable to use this key to decrypt'
9999
});
100100
}
101101

‎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

0 commit comments

Comments
 (0)