Skip to content

Commit

Permalink
tls: TLSSocket emits 'error' on handshake failure
Browse files Browse the repository at this point in the history
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: #8803
PR-URL: #8805
Refs: #4557
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Fedor Indutny <[email protected]>
  • Loading branch information
lekoder authored and MylesBorins committed Apr 19, 2017
1 parent ee7552e commit 6d3aaa7
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
38 changes: 38 additions & 0 deletions test/parallel/test-tls-socket-failed-handshake-emits-error.js
Original file line number Diff line number Diff line change
@@ -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);
});
});

0 comments on commit 6d3aaa7

Please sign in to comment.