tls_wrap: fix error cb when fatal TLS Alert recvd

SSL_read() returns 0 when fatal TLS Alert is received.
Fix to invoke ssl error callback in this case.

PR-URL: https://github.com/nodejs/io.js/pull/1661
Reviewed-By: Fedor Indutny <fedor@indutny.com>

diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc
index fd337d7..3076864 100644 (file)
--- a/src/tls_wrap.cc
+++ b/src/tls_wrap.cc
@@ -352,6 +352,10 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
 Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
   EscapableHandleScope scope(env()->isolate());
 
+  // ssl_ is already destroyed in reading EOF by close notify alert.
+  if (ssl_ == nullptr)
+    return Local<Value>();
+
   *err = SSL_get_error(ssl_, status);
   switch (*err) {
     case SSL_ERROR_NONE:
@@ -432,7 +436,10 @@ void TLSWrap::ClearOut() {
     OnRead(UV_EOF, nullptr);
   }
 
-  if (read == -1) {
+  // We need to check whether an error occurred or the connection was
+  // shutdown cleanly (SSL_ERROR_ZERO_RETURN) even when read == 0.
+  // See iojs#1642 and SSL_read(3SSL) for details.
+  if (read <= 0) {
     int err;
     Local<Value> arg = GetSSLError(read, &err, nullptr);
 

diff --git a/test/parallel/test-tls-alert-handling.js b/test/parallel/test-tls-alert-handling.js
new file mode 100644 (file)
index 0000000..045e69b
--- /dev/null
+++ b/test/parallel/test-tls-alert-handling.js
@@ -0,0 +1,90 @@
+var common = require('../common');
+var assert = require('assert');
+
+if (!common.opensslCli) {
+  console.error('Skipping because node compiled without OpenSSL CLI.');
+  process.exit(0);
+}
+
+if (!common.hasCrypto) {
+  console.log('1..0 # Skipped: missing crypto');
+  process.exit();
+}
+
+var tls = require('tls');
+var net = require('net');
+var fs = require('fs');
+
+var success = false;
+
+function filenamePEM(n) {
+  return require('path').join(common.fixturesDir, 'keys', n + '.pem');
+}
+
+function loadPEM(n) {
+  return fs.readFileSync(filenamePEM(n));
+}
+
+var opts = {
+  key: loadPEM('agent2-key'),
+  cert: loadPEM('agent2-cert')
+};
+
+var max_iter = 20;
+var iter = 0;
+
+var server = tls.createServer(opts, function(s) {
+  s.pipe(s);
+  s.on('error', function(e) {
+    // ignore error
+  });
+});
+
+server.listen(common.PORT, function() {
+  sendClient();
+});
+
+
+function sendClient() {
+  var client = tls.connect(common.PORT, {
+    rejectUnauthorized: false
+  });
+  client.on('data', function(chunk) {
+    if (iter++ === 2) sendBADTLSRecord();
+    if (iter < max_iter) {
+      client.write('a');
+      return;
+    }
+    client.end();
+    server.close();
+    success = true;
+  });
+  client.write('a');
+  client.on('error', function(e) {
+    // ignore error
+  });
+  client.on('close', function() {
+    server.close();
+  });
+}
+
+
+function sendBADTLSRecord() {
+  var BAD_RECORD = new Buffer([0xff, 0xff, 0xff, 0xff, 0xff, 0xff]);
+  var socket = net.connect(common.PORT);
+  var client = tls.connect({
+    socket: socket,
+    rejectUnauthorized: false
+  }, function() {
+    socket.write(BAD_RECORD);
+    socket.end();
+  });
+  client.on('error', function(e) {
+    // ignore error
+  });
+}
+
+process.on('exit', function() {
+  assert(iter === max_iter);
+  assert(success);
+});