crypto: Clear error after DiffieHellman key errors

isaacs · isaacs · commit 3a2b5030ae1c · 2013-05-20T14:27:32.000-07:00
Fixes #5499
diff --git 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<String> errno_symbol;
 static Persistent<String> syscall_symbol;
 static Persistent<String> 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
@@ -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');
