Skip to content

Commit

Permalink
crypto: clear openssl error stack after en/decrypt
Browse files Browse the repository at this point in the history
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: #32240

PR-URL: #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>
  • Loading branch information
bnoordhuis authored and addaleax committed Mar 30, 2020
1 parent e2b08f0 commit 2c32e59
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
3 changes: 1 addition & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 38 additions & 0 deletions test/parallel/test-crypto-private-decrypt-gh32240.js
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 2c32e59

Please sign in to comment.