Skip to content

Commit c7bc9bc

Browse files
lekodermcollina
authored andcommitted
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. Addedd test for tls.Server to ensure it still emits 'tlsClientError' as expected. Fixes: #8803 PR-URL: #8805 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
1 parent 7542bdd commit c7bc9bc

3 files changed

+78
-1
lines changed

lib/_tls_wrap.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,9 @@ TLSSocket.prototype._init = function(socket, wrap) {
426426

427427
// Destroy socket if error happened before handshake's finish
428428
if (!self._secureEstablished) {
429-
self.destroy(self._tlsError(err));
429+
// When handshake fails control is not yet released,
430+
// so self._tlsError will return null instead of actual error
431+
self.destroy(err);
430432
} else if (options.isServer &&
431433
rejectUnauthorized &&
432434
/peer did not return a certificate/.test(err.message)) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto) {
5+
common.skip('missing crypto');
6+
return;
7+
}
8+
const tls = require('tls');
9+
const net = require('net');
10+
const assert = require('assert');
11+
12+
const bonkers = Buffer.alloc(1024, 42);
13+
14+
let tlsClientErrorEmited = false;
15+
16+
const server = tls.createServer({})
17+
.listen(0, function() {
18+
const c = net.connect({ port: this.address().port }, function() {
19+
c.write(bonkers);
20+
});
21+
22+
}).on('tlsClientError', function(e) {
23+
tlsClientErrorEmited = true;
24+
assert.ok(e instanceof Error,
25+
'Instance of Error should be passed to error handler');
26+
assert.ok(e.message.match(
27+
/SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/),
28+
'Expecting SSL unknown protocol');
29+
});
30+
31+
setTimeout(function() {
32+
server.close();
33+
34+
assert.ok(tlsClientErrorEmited,
35+
'tlsClientError should be emited');
36+
37+
}, common.platformTimeout(200));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
if (!common.hasCrypto) {
5+
common.skip('missing crypto');
6+
return;
7+
}
8+
const tls = require('tls');
9+
const net = require('net');
10+
const assert = require('assert');
11+
12+
const bonkers = Buffer.alloc(1024, 42);
13+
14+
const server = net.createServer(function(c) {
15+
setTimeout(function() {
16+
const s = new tls.TLSSocket(c, {
17+
isServer: true,
18+
server: server
19+
});
20+
21+
s.on('error', common.mustCall(function(e) {
22+
assert.ok(e instanceof Error,
23+
'Instance of Error should be passed to error handler');
24+
assert.ok(e.message.match(
25+
/SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/),
26+
'Expecting SSL unknown protocol');
27+
}));
28+
29+
s.on('close', function() {
30+
server.close();
31+
s.destroy();
32+
});
33+
}, common.platformTimeout(200));
34+
}).listen(0, function() {
35+
const c = net.connect({port: this.address().port}, function() {
36+
c.write(bonkers);
37+
});
38+
});

0 commit comments

Comments
 (0)