From fab9180ee571378cd271e971502e5e2ae7af46d5 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Tue, 22 Sep 2026 22:13:37 +0200 Subject: [PATCH] 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 Assisted-by: Codex PR-URL: https://github.com/nodejs/node/pull/66237 Reviewed-By: James M Snell Reviewed-By: Aviv Keller --- src/crypto/crypto_cipher.h | 7 +-- .../test-webcrypto-cipher-unrelated-errors.js | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-webcrypto-cipher-unrelated-errors.js diff --git a/src/crypto/crypto_cipher.h b/src/crypto/crypto_cipher.h index f6deb774ea3..8817171a2be 100644 --- a/src/crypto/crypto_cipher.h +++ b/src/crypto/crypto_cipher.h @@ -219,6 +219,7 @@ class CipherJob final : public CryptoJob { WebCryptoCipherMode cipher_mode() const { return cipher_mode_; } void DoThreadPoolWork() override { + ncrypto::ClearErrorOnReturn clear_error_on_return; const WebCryptoCipherStatus status = CipherTraits::DoCipher( AsyncWrap::env(), @@ -253,11 +254,7 @@ class CipherJob final : public CryptoJob { Environment* env = AsyncWrap::env(); CryptoErrorStore* errors = CryptoJob::errors(); - if (errors->Empty()) - errors->Capture(); - - if (out_.size() > 0 || errors->Empty()) { - CHECK(errors->Empty()); + if (errors->Empty()) { *err = v8::Undefined(env->isolate()); *result = out_.ToArrayBuffer(env); if (result->IsEmpty()) { diff --git a/test/parallel/test-webcrypto-cipher-unrelated-errors.js b/test/parallel/test-webcrypto-cipher-unrelated-errors.js new file mode 100644 index 00000000000..258733b0440 --- /dev/null +++ b/test/parallel/test-webcrypto-cipher-unrelated-errors.js @@ -0,0 +1,45 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const { scrypt, scryptSync } = require('crypto'); +const { subtle } = globalThis.crypto; + +if (typeof scryptSync !== 'function') + common.skip('no scrypt support'); + +function failScrypt(sync) { + // The failed parameter check leaves an error on the main thread's + // OpenSSL error queue. Cipher jobs must only use their own errors. + assert.throws(() => { + if (sync) { + scryptSync('password', 'salt', 64, { N: 2 ** 17 }); + } else { + scrypt('password', 'salt', 64, { N: 2 ** 17 }, common.mustNotCall()); + } + }, { code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS' }); +} + +(async () => { + const key = await subtle.importKey( + 'raw', new Uint8Array(16), 'AES-GCM', false, ['encrypt', 'decrypt']); + const algorithm = { name: 'AES-GCM', iv: new Uint8Array(12) }; + + for (const sync of [true, false]) { + for (const byteLength of [0, 16]) { + const plaintext = new Uint8Array(byteLength); + failScrypt(sync); + const ciphertext = await subtle.encrypt(algorithm, key, plaintext); + assert.strictEqual(ciphertext.byteLength, byteLength + 16); + + failScrypt(sync); + assert.deepStrictEqual( + new Uint8Array(await subtle.decrypt(algorithm, key, ciphertext)), + plaintext); + } + } +})().then(common.mustCall());