From c63e02a868790dac8863d59435b68683f0b0d9d5 Mon Sep 17 00:00:00 2001 From: Mariusz 'koder' Chwalba Date: Tue, 27 Sep 2016 08:20:10 +0200 Subject: [PATCH] tls: TLSSocket emits 'error' on handshake failure Removes branch that would make TLSSocket emit '_tlsError' event if error occured on handshake and control was not released, as it was never happening. Added test for tls.Server to ensure it still emits 'tlsClientError' as expected. Note that 'tlsClientError' does not exist in the v4.x branch so this back-port emits 'clientError' instead. See also pull request #4557. Fixes: https://github.com/nodejs/node/issues/8803 PR-URL: https://github.com/nodejs/node/pull/8805 Refs: https://github.com/nodejs/node/pull/4557 Reviewed-By: Matteo Collina Reviewed-By: Fedor Indutny --- lib/_tls_wrap.js | 4 +- ...rver-failed-handshake-emits-clienterror.js | 36 ++++++++++++++++++ ...tls-socket-failed-handshake-emits-error.js | 38 +++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-server-failed-handshake-emits-clienterror.js create mode 100644 test/parallel/test-tls-socket-failed-handshake-emits-error.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index cb435e45f26ef6..31cbe3e65edfa9 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -463,7 +463,9 @@ TLSSocket.prototype._init = function(socket, wrap) { // Destroy socket if error happened before handshake's finish if (!self._secureEstablished) { - self.destroy(self._tlsError(err)); + // When handshake fails control is not yet released, + // so self._tlsError will return null instead of actual error + self.destroy(err); } else if (options.isServer && rejectUnauthorized && /peer did not return a certificate/.test(err.message)) { diff --git a/test/parallel/test-tls-server-failed-handshake-emits-clienterror.js b/test/parallel/test-tls-server-failed-handshake-emits-clienterror.js new file mode 100644 index 00000000000000..a02912542221fb --- /dev/null +++ b/test/parallel/test-tls-server-failed-handshake-emits-clienterror.js @@ -0,0 +1,36 @@ +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) { + common.skip('missing crypto'); + return; +} +const tls = require('tls'); +const net = require('net'); +const assert = require('assert'); + +const bonkers = Buffer.alloc(1024, 42); + +let clientErrorEmited = false; + +const server = tls.createServer({}) + .listen(0, function() { + const c = net.connect({ port: this.address().port }, function() { + c.write(bonkers); + }); + + }).on('clientError', function(e) { + clientErrorEmited = true; + assert.ok(e instanceof Error, + 'Instance of Error should be passed to error handler'); + assert.ok(e.message.match( + /SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/), + 'Expecting SSL unknown protocol'); + }); + +setTimeout(function() { + server.close(); + + assert.ok(clientErrorEmited, 'clientError should be emited'); + +}, common.platformTimeout(200)); diff --git a/test/parallel/test-tls-socket-failed-handshake-emits-error.js b/test/parallel/test-tls-socket-failed-handshake-emits-error.js new file mode 100644 index 00000000000000..f655dc97b5a99b --- /dev/null +++ b/test/parallel/test-tls-socket-failed-handshake-emits-error.js @@ -0,0 +1,38 @@ +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) { + common.skip('missing crypto'); + return; +} +const tls = require('tls'); +const net = require('net'); +const assert = require('assert'); + +const bonkers = Buffer.alloc(1024, 42); + +const server = net.createServer(function(c) { + setTimeout(function() { + const s = new tls.TLSSocket(c, { + isServer: true, + server: server + }); + + s.on('error', common.mustCall(function(e) { + assert.ok(e instanceof Error, + 'Instance of Error should be passed to error handler'); + assert.ok(e.message.match( + /SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/), + 'Expecting SSL unknown protocol'); + })); + + s.on('close', function() { + server.close(); + s.destroy(); + }); + }, common.platformTimeout(200)); +}).listen(0, function() { + const c = net.connect({port: this.address().port}, function() { + c.write(bonkers); + }); +});