From 4ab1190d7151306614ca92b12b2cbc31601d601b 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 7efe42ab46ca22..4ade824bd6ea95 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -455,7 +455,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); + }); +});