From 31aceae61993604808cdfd97b5b60b41fa63f07a Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Fri, 13 Mar 2020 10:34:59 +0100
Subject: [PATCH] crypto: clear openssl error stack after en/decrypt

The publicEncrypt/privateDecrypt/etc. family of functions didn't clear
OpenSSL's error stack on early return.

Notably, trying to use an encrypted key with the wrong passphrase left
an error on the stack that made subsequent encrypt or decrypt operations
fail, even with an unencrypted key.

Fixes: https://github.com/nodejs/node/issues/32240
---
 src/node_crypto.cc                            |  3 +-
 .../test-crypto-private-decrypt-gh32240.js    | 38 +++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-crypto-private-decrypt-gh32240.js

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index e136903989ec51..403a4e413c2a55 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5026,6 +5026,7 @@ template <PublicKeyCipher::Operation operation,
           PublicKeyCipher::EVP_PKEY_cipher_init_t EVP_PKEY_cipher_init,
           PublicKeyCipher::EVP_PKEY_cipher_t EVP_PKEY_cipher>
 void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
+  MarkPopErrorOnReturn mark_pop_error_on_return;
   Environment* env = Environment::GetCurrent(args);
 
   unsigned int offset = 0;
@@ -5056,8 +5057,6 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
 
   AllocatedBuffer out;
 
-  ClearErrorOnReturn clear_error_on_return;
-
   bool r = Cipher<operation, EVP_PKEY_cipher_init, EVP_PKEY_cipher>(
       env,
       pkey,
diff --git a/test/parallel/test-crypto-private-decrypt-gh32240.js b/test/parallel/test-crypto-private-decrypt-gh32240.js
new file mode 100644
index 00000000000000..4b48774145a3f8
--- /dev/null
+++ b/test/parallel/test-crypto-private-decrypt-gh32240.js
@@ -0,0 +1,38 @@
+'use strict';
+
+// Verify that privateDecrypt() does not leave an error on the
+// openssl error stack that is visible to subsequent operations.
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const {
+  generateKeyPairSync,
+  publicEncrypt,
+  privateDecrypt,
+} = require('crypto');
+
+const pair = generateKeyPairSync('rsa', { modulusLength: 512 });
+
+const expected = Buffer.from('shibboleth');
+const encrypted = publicEncrypt(pair.publicKey, expected);
+
+const pkey = pair.privateKey.export({ type: 'pkcs1', format: 'pem' });
+const pkeyEncrypted =
+  pair.privateKey.export({
+    type: 'pkcs1',
+    format: 'pem',
+    cipher: 'aes128',
+    passphrase: 'secret',
+  });
+
+function decrypt(key) {
+  const decrypted = privateDecrypt(key, encrypted);
+  assert.deepStrictEqual(decrypted, expected);
+}
+
+decrypt(pkey);
+assert.throws(() => decrypt(pkeyEncrypted), { code: 'ERR_MISSING_PASSPHRASE' });
+decrypt(pkey);  // Should not throw.