Skip to content

Commit fab9180

Browse files
panvanodejs-github-bot
authored andcommitted
crypto: isolate cipher job errors
Use only errors captured by the cipher job when converting its result. Unrelated errors on the event loop thread must not turn a successful operation into a failure or trigger an assertion. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex PR-URL: #66237 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent 8c7346a commit fab9180

2 files changed

Lines changed: 47 additions & 5 deletions

File tree

‎src/crypto/crypto_cipher.h‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ class CipherJob final : public CryptoJob<CipherTraits> {
219219
WebCryptoCipherMode cipher_mode() const { return cipher_mode_; }
220220

221221
void DoThreadPoolWork() override {
222+
ncrypto::ClearErrorOnReturn clear_error_on_return;
222223
const WebCryptoCipherStatus status =
223224
CipherTraits::DoCipher(
224225
AsyncWrap::env(),
@@ -253,11 +254,7 @@ class CipherJob final : public CryptoJob<CipherTraits> {
253254
Environment* env = AsyncWrap::env();
254255
CryptoErrorStore* errors = CryptoJob<CipherTraits>::errors();
255256

256-
if (errors->Empty())
257-
errors->Capture();
258-
259-
if (out_.size() > 0 || errors->Empty()) {
260-
CHECK(errors->Empty());
257+
if (errors->Empty()) {
261258
*err = v8::Undefined(env->isolate());
262259
*result = out_.ToArrayBuffer(env);
263260
if (result->IsEmpty()) {
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const assert = require('assert');
9+
const { scrypt, scryptSync } = require('crypto');
10+
const { subtle } = globalThis.crypto;
11+
12+
if (typeof scryptSync !== 'function')
13+
common.skip('no scrypt support');
14+
15+
function failScrypt(sync) {
16+
// The failed parameter check leaves an error on the main thread's
17+
// OpenSSL error queue. Cipher jobs must only use their own errors.
18+
assert.throws(() => {
19+
if (sync) {
20+
scryptSync('password', 'salt', 64, { N: 2 ** 17 });
21+
} else {
22+
scrypt('password', 'salt', 64, { N: 2 ** 17 }, common.mustNotCall());
23+
}
24+
}, { code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS' });
25+
}
26+
27+
(async () => {
28+
const key = await subtle.importKey(
29+
'raw', new Uint8Array(16), 'AES-GCM', false, ['encrypt', 'decrypt']);
30+
const algorithm = { name: 'AES-GCM', iv: new Uint8Array(12) };
31+
32+
for (const sync of [true, false]) {
33+
for (const byteLength of [0, 16]) {
34+
const plaintext = new Uint8Array(byteLength);
35+
failScrypt(sync);
36+
const ciphertext = await subtle.encrypt(algorithm, key, plaintext);
37+
assert.strictEqual(ciphertext.byteLength, byteLength + 16);
38+
39+
failScrypt(sync);
40+
assert.deepStrictEqual(
41+
new Uint8Array(await subtle.decrypt(algorithm, key, ciphertext)),
42+
plaintext);
43+
}
44+
}
45+
})().then(common.mustCall());

0 commit comments

Comments
 (0)