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: nodejs#32240 PR-URL: nodejs#32248 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
jyxinye · Mar 30, 2020 · c37d4cc · c37d4cc
1 parent 2d88129
commit c37d4cc
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 2 deletions.
diff --git 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
@@ -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.