◐ Shell
reader mode source ↗
Skip to content

Commit 314dacd

Browse files
panvaaduh95
panva
authored and
aduh95
committed
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 795db76 commit 314dacd

12 files changed

Lines changed: 201 additions & 57 deletions
Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,14 @@ async function wrapKey(format, key, wrappingKey, algorithm) {
927927
} catch {
928928
algorithm = normalizeAlgorithm(algorithm, 'encrypt');
929929
}
930+
931+
if (algorithm.name !== wrappingKey[kAlgorithm].name)
932+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
933+
934+
if (!ArrayPrototypeIncludes(wrappingKey[kKeyUsages], 'wrapKey'))
935+
throw lazyDOMException(
936+
'Unable to use this key to wrapKey', 'InvalidAccessError');
937+
930938
let keyData = await FunctionPrototypeCall(exportKey, this, format, key);
931939

932940
if (format === 'jwk') {
@@ -1005,6 +1013,13 @@ async function unwrapKey(
10051013

10061014
unwrappedKeyAlgo = normalizeAlgorithm(unwrappedKeyAlgo, 'importKey');
10071015

1016+
if (unwrapAlgo.name !== unwrappingKey[kAlgorithm].name)
1017+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1018+
1019+
if (!ArrayPrototypeIncludes(unwrappingKey[kKeyUsages], 'unwrapKey'))
1020+
throw lazyDOMException(
1021+
'Unable to use this key to unwrapKey', 'InvalidAccessError');
1022+
10081023
let keyData = await cipherOrWrap(
10091024
kWebCryptoCipherDecrypt,
10101025
unwrapAlgo,
@@ -1038,12 +1053,12 @@ async function signVerify(algorithm, key, data, signature) {
10381053
}
10391054
algorithm = normalizeAlgorithm(algorithm, usage);
10401055

1041-
if (!ArrayPrototypeIncludes(key[kKeyUsages], usage) ||
1042-
algorithm.name !== key[kAlgorithm].name) {
1056+
if (algorithm.name !== key[kAlgorithm].name)
1057+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1058+
1059+
if (!ArrayPrototypeIncludes(key[kKeyUsages], usage))
10431060
throw lazyDOMException(
1044-
`Unable to use this key to ${usage}`,
1045-
'InvalidAccessError');
1046-
}
1061+
`Unable to use this key to ${usage}`, 'InvalidAccessError');
10471062

10481063
switch (algorithm.name) {
10491064
case 'RSA-PSS':
@@ -1128,18 +1143,6 @@ async function verify(algorithm, key, signature, data) {
11281143
}
11291144

11301145
async function cipherOrWrap(mode, algorithm, key, data, op) {
1131-
// We use a Node.js style error here instead of a DOMException because
1132-
// the WebCrypto spec is not specific what kind of error is to be thrown
1133-
// in this case. Both Firefox and Chrome throw simple TypeErrors here.
1134-
// The key algorithm and cipher algorithm must match, and the
1135-
// key must have the proper usage.
1136-
if (key[kAlgorithm].name !== algorithm.name ||
1137-
!ArrayPrototypeIncludes(key[kKeyUsages], op)) {
1138-
throw lazyDOMException(
1139-
'The requested operation is not valid for the provided key',
1140-
'InvalidAccessError');
1141-
}
1142-
11431146
// While WebCrypto allows for larger input buffer sizes, we limit
11441147
// those to sizes that can fit within uint32_t because of limitations
11451148
// in the OpenSSL API.
@@ -1190,6 +1193,14 @@ async function encrypt(algorithm, key, data) {
11901193
});
11911194

11921195
algorithm = normalizeAlgorithm(algorithm, 'encrypt');
1196+
1197+
if (algorithm.name !== key[kAlgorithm].name)
1198+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1199+
1200+
if (!ArrayPrototypeIncludes(key[kKeyUsages], 'encrypt'))
1201+
throw lazyDOMException(
1202+
'Unable to use this key to encrypt', 'InvalidAccessError');
1203+
11931204
return await cipherOrWrap(
11941205
kWebCryptoCipherEncrypt,
11951206
algorithm,
@@ -1219,6 +1230,14 @@ async function decrypt(algorithm, key, data) {
12191230
});
12201231

12211232
algorithm = normalizeAlgorithm(algorithm, 'decrypt');
1233+
1234+
if (algorithm.name !== key[kAlgorithm].name)
1235+
throw lazyDOMException('Key algorithm mismatch', 'InvalidAccessError');
1236+
1237+
if (!ArrayPrototypeIncludes(key[kKeyUsages], 'decrypt'))
1238+
throw lazyDOMException(
1239+
'Unable to use this key to decrypt', 'InvalidAccessError');
1240+
12221241
return await cipherOrWrap(
12231242
kWebCryptoCipherDecrypt,
12241243
algorithm,
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) => {
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) => {
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

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

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

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) {
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)