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.
Addedd test for tls.Server to ensure it still emits 'tlsClientError' as expected.

Fixes: #8803
PR-URL: #8805
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
lekoder authored and Fishrock123 committed Oct 11, 2016
1 parent c4f0bb2 commit cf92be6
Show file tree
Hide file tree
Showing 3 changed files with 78 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 @@ -426,7 +426,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,37 @@
'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 tlsClientErrorEmited = false;

const server = tls.createServer({})
.listen(0, function() {
const c = net.connect({ port: this.address().port }, function() {
c.write(bonkers);
});

}).on('tlsClientError', function(e) {
tlsClientErrorEmited = 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(tlsClientErrorEmited,
'tlsClientError 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 cf92be6

Please sign in to comment.