From 3a2b5030ae1cd200e92eaf3928bd20a8deda50c6 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 17 May 2013 17:19:12 -0700 Subject: [PATCH] crypto: Clear error after DiffieHellman key errors Fixes #5499 --- src/node_crypto.cc | 17 ++++++++++------- test/simple/test-crypto.js | 16 +++++++++++----- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index d5915101638..93f88202f73 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -67,6 +67,14 @@ namespace crypto { using namespace v8; +// Forcibly clear OpenSSL's error stack on return. This stops stale errors +// from popping up later in the lifecycle of crypto operations where they +// would cause spurious failures. It's a rather blunt method, though. +// ERR_clear_error() isn't necessarily cheap either. +struct ClearErrorOnReturn { + ~ClearErrorOnReturn() { ERR_clear_error(); } +}; + static Persistent errno_symbol; static Persistent syscall_symbol; static Persistent subject_symbol; @@ -908,13 +916,6 @@ int Connection::HandleBIOError(BIO *bio, const char* func, int rv) { int Connection::HandleSSLError(const char* func, int rv, ZeroStatus zs) { - // Forcibly clear OpenSSL's error stack on return. This stops stale errors - // from popping up later in the lifecycle of the SSL connection where they - // would cause spurious failures. It's a rather blunt method, though. - // ERR_clear_error() isn't necessarily cheap either. - struct ClearErrorOnReturn { - ~ClearErrorOnReturn() { ERR_clear_error(); } - }; ClearErrorOnReturn clear_error_on_return; (void) &clear_error_on_return; // Silence unused variable warning. @@ -3603,6 +3604,8 @@ class DiffieHellman : public ObjectWrap { return ThrowException(Exception::Error(String::New("Not initialized"))); } + ClearErrorOnReturn clear_error_on_return; + (void) &clear_error_on_return; // Silence compiler warning. BIGNUM* key = 0; if (args.Length() == 0) { diff --git a/test/simple/test-crypto.js b/test/simple/test-crypto.js index 01487aca88c..1d913fe0b54 100644 --- a/test/simple/test-crypto.js +++ b/test/simple/test-crypto.js @@ -698,10 +698,21 @@ var secret3 = dh3.computeSecret(key2, 'hex', 'base64'); assert.equal(secret1, secret3); +// Run this one twice to make sure that the dh3 clears its error properly +(function() { + var c = crypto.createDecipher('aes-128-ecb', ''); + assert.throws(function() { c.final('utf8') }, /wrong final block length/); +})(); + assert.throws(function() { dh3.computeSecret(''); }, /key is too small/i); +(function() { + var c = crypto.createDecipher('aes-128-ecb', ''); + assert.throws(function() { c.final('utf8') }, /wrong final block length/); +})(); + // Create a shared using a DH group. var alice = crypto.createDiffieHellmanGroup('modp5'); var bob = crypto.createDiffieHellmanGroup('modp5'); @@ -858,11 +869,6 @@ assert.equal(-1, crypto.getHashes().indexOf('SHA1')); assert.equal(-1, crypto.getHashes().indexOf('SHA')); assertSorted(crypto.getHashes()); -(function() { - var c = crypto.createDecipher('aes-128-ecb', ''); - assert.throws(function() { c.final('utf8') }, /invalid public key/); -})(); - // Base64 padding regression test, see #4837. (function() { var c = crypto.createCipher('aes-256-cbc', 'secret');