crypto-clear.patch

From c421a5e66b6810315fe2bc713f532886002bd7cc Mon Sep 17 00:00:00 2001
From: Timothy J Fontaine <tjfontaine@gmail.com>
Date: Fri, 18 Oct 2013 08:43:52 -0700
Subject: [PATCH] crypto: clear errors from verify failure

OpenSSL will push errors onto the stack when a verify fails, which can
disrupt TLS and other routines if we don't clear the error stack

Fixes #6304
---
 src/node_crypto.cc                        | 10 ++++
 test/simple/test-crypto-verify-failure.js | 79 +++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)
 create mode 100644 test/simple/test-crypto-verify-failure.js

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index b1140b8..316f435 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -69,6 +69,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;
@@ -3406,6 +3414,8 @@ class Verify : public ObjectWrap {
   int VerifyFinal(char* key_pem, int key_pemLen, unsigned char* sig, int siglen) {
     if (!initialised_) return 0;
 
+    ClearErrorOnReturn clear_error_on_return;
+
     EVP_PKEY* pkey = NULL;
     BIO *bp = NULL;
     X509 *x509 = NULL;
diff --git a/test/simple/test-crypto-verify-failure.js b/test/simple/test-crypto-verify-failure.js
new file mode 100644
index 0000000..6ee83c3
--- /dev/null
+++ b/test/simple/test-crypto-verify-failure.js
@@ -0,0 +1,79 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+
+
+
+var common = require('../common');
+var assert = require('assert');
+
+try {
+  var crypto = require('crypto');
+  var tls = require('tls');
+} catch (e) {
+  console.log('Not compiled with OPENSSL support.');
+  process.exit();
+}
+
+crypto.DEFAULT_ENCODING = 'buffer';
+
+var fs = require('fs');
+
+var certPem = fs.readFileSync(common.fixturesDir + '/test_cert.pem', 'ascii');
+
+var options = {
+  key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
+  cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
+};
+
+var canSend = true;
+
+var server = tls.Server(options, function(socket) {
+  process.nextTick(function() {
+      console.log('sending');
+      socket.destroy();
+      verify();
+  });
+});
+
+var client;
+
+function verify() {
+  console.log('verify');
+  var verified = crypto.createVerify('RSA-SHA1')
+                     .update('Test')
+                     .verify(certPem, 'asdfasdfas', 'base64');
+}
+
+server.listen(common.PORT, function() {
+  client = tls.connect({
+    port: common.PORT,
+    rejectUnauthorized: false
+  }, function() {
+    verify();
+  }).on('data', function(data) {
+    console.log(data);
+  }).on('error', function(err) {
+    throw err;
+  }).on('close', function() {
+    server.close();
+  }).resume();
+});
-- 
1.8.3.4 (Apple Git-47)

handle-error.patch

From 78fe7d0592c53cd0a0c77f32320979eec5d7fadb Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Fri, 18 Oct 2013 10:07:49 -0700
Subject: [PATCH] crypto: clear openssl error stack when handled

Clear OpenSSL's error stack on return from Connection::HandleSSLError().
This stops stale errors from popping up later in the lifecycle of the
SSL connection where they would cause spurious failures.

This commit causes a 1-2% performance regression on `make bench-tls`.
We'll address that in follow-up commits if possible but let's ensure
correctness first.

Backport of c6e2db2
---
 src/node_crypto.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 316f435..bc22dbe 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -711,6 +711,9 @@ int Connection::HandleBIOError(BIO *bio, const char* func, int rv) {
 
 
 int Connection::HandleSSLError(const char* func, int rv) {
+  ClearErrorOnReturn clear_error_on_return;
+  (void) &clear_error_on_return;  // Silence unused variable warning.
+
   if (rv >= 0) return rv;
 
   int err = SSL_get_error(ssl_, rv);
-- 
1.8.3.4 (Apple Git-47)