From 2c32e59d8d83caa92155664f3e3fc0687a624c33 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
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

PR-URL: https://github.com/nodejs/node/pull/32248
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
---
 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 c5af0924846c0b..8f673ee6e34772 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.