Skip to content
Permalink
Browse files

crypto: check for OpenSSL errors when signing

Errors might be injected into OpenSSL's error stack
without the return value of `PEM_read_bio_PrivateKey` being set to
`nullptr`. See the test of `test_bad_rsa_privkey.pem` for an
example.

PR-URL: #2342
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information...
pmq20 authored and indutny committed Aug 18, 2015
1 parent 102939a commit 00bffa6c758038dca039fd9114912c0430114c08
Showing with 37 additions and 1 deletion.
  1. +11 −1 src/node_crypto.cc
  2. +10 −0 test/fixtures/test_bad_rsa_privkey.pem
  3. +16 −0 test/parallel/test-crypto.js
@@ -3585,7 +3585,11 @@ SignBase::Error Sign::SignFinal(const char* key_pem,
nullptr,
CryptoPemCallback,
const_cast<char*>(passphrase));
if (pkey == nullptr)

// Errors might be injected into OpenSSL's error stack
// without `pkey` being set to nullptr;
// cf. the test of `test_bad_rsa_privkey.pem` for an example.
if (pkey == nullptr || 0 != ERR_peek_error())
goto exit;

if (EVP_SignFinal(&mdctx_, *sig, sig_len, pkey))
@@ -3633,6 +3637,9 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& args) {
md_len = 8192; // Maximum key size is 8192 bits
md_value = new unsigned char[md_len];

ClearErrorOnReturn clear_error_on_return;
(void) &clear_error_on_return; // Silence compiler warning.

Error err = sign->SignFinal(
buf,
buf_len,
@@ -3967,6 +3974,9 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
unsigned char* out_value = nullptr;
size_t out_len = 0;

ClearErrorOnReturn clear_error_on_return;
(void) &clear_error_on_return; // Silence compiler warning.

bool r = Cipher<operation, EVP_PKEY_cipher_init, EVP_PKEY_cipher>(
kbuf,
klen,
@@ -0,0 +1,10 @@
-----BEGIN RSA PRIVATE KEY-----
MIIBUwIBADANBgkqhkiG9w0BAQEFAASCAT0wggE5AgEAAkEAz0ZHmXyxQSdWk6NF
GRotTax0O94iHv843su0mOynV9QLvlAwMrUk9k4+/SwyLu0eE3iYsYgXstXi3t2u
rDSIMwIDAQABAkAH4ag/Udp7m79TBdZOygwG9BPHYv7xJstGzYAkgHssf7Yd5ZuC
hpKtBvWdPXZaAFbwF8NSisMl98Q/9zgB/q5BAiEA5zXuwMnwt4hE2YqzBDRFB4g9
I+v+l1soy6x7Wdqo9esCIQDlf15qDb26uRDurBioE3IpZstWIIvLDdKqviZXKMs8
2QIgWeC5QvA9RtsOCJLGLCg1fUwUmFYwzZ1+Kk6OVMuPSqkCIDIWFSXyL8kzoKVm
O89axxyQCaqXWcsMDkEjVLzK82gpAiB7lzdDHr7MoMWwV2wC/heEFC2p0Rw4wg9j
1V8QbL0Q0A==
-----END RSA PRIVATE KEY-----
@@ -124,5 +124,21 @@ assert.throws(function() {
crypto.createSign('RSA-SHA256').update('test').sign(priv);
}, /RSA_sign:digest too big for rsa key/);

assert.throws(function() {
// The correct header inside `test_bad_rsa_privkey.pem` should have been
// -----BEGIN PRIVATE KEY----- and -----END PRIVATE KEY-----
// instead of
// -----BEGIN RSA PRIVATE KEY----- and -----END RSA PRIVATE KEY-----
// It is generated in this way:
// $ openssl genrsa -out mykey.pem 512;
// $ openssl pkcs8 -topk8 -inform PEM -outform PEM -in mykey.pem \
// -out private_key.pem -nocrypt;
// Then open private_key.pem and change its header and footer.
var sha1_privateKey = fs.readFileSync(common.fixturesDir +
'/test_bad_rsa_privkey.pem', 'ascii');
// this would inject errors onto OpenSSL's error stack
crypto.createSign('sha1').sign(sha1_privateKey);
}, /asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag/);

// Make sure memory isn't released before being returned
console.log(crypto.randomBytes(16));

0 comments on commit 00bffa6

Please sign in to comment.
You can’t perform that action at this time.