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

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed

‎lib/_tls_wrap.js‎

Lines changed: 3 additions & 1 deletion
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
}elseif(options.isServer&&
431433
rejectUnauthorized&&
432434
/peerdidnotreturnacertificate/.test(err.message)){
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
4+
if(!common.hasCrypto){
5+
common.skip('missing crypto');
6+
return;
7+
}
8+
consttls=require('tls');
9+
constnet=require('net');
10+
constassert=require('assert');
11+
12+
constbonkers=Buffer.alloc(1024,42);
13+
14+
lettlsClientErrorEmited=false;
15+
16+
constserver=tls.createServer({})
17+
.listen(0,function(){
18+
constc=net.connect({port: this.address().port},function(){
19+
c.write(bonkers);
20+
});
21+
22+
}).on('tlsClientError',function(e){
23+
tlsClientErrorEmited=true;
24+
assert.ok(einstanceofError,
25+
'Instance of Error should be passed to error handler');
26+
assert.ok(e.message.match(
27+
/SSLroutines:SSL23_GET_CLIENT_HELLO:unknownprotocol/),
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));
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
4+
if(!common.hasCrypto){
5+
common.skip('missing crypto');
6+
return;
7+
}
8+
consttls=require('tls');
9+
constnet=require('net');
10+
constassert=require('assert');
11+
12+
constbonkers=Buffer.alloc(1024,42);
13+
14+
constserver=net.createServer(function(c){
15+
setTimeout(function(){
16+
consts=newtls.TLSSocket(c,{
17+
isServer: true,
18+
server: server
19+
});
20+
21+
s.on('error',common.mustCall(function(e){
22+
assert.ok(einstanceofError,
23+
'Instance of Error should be passed to error handler');
24+
assert.ok(e.message.match(
25+
/SSLroutines:SSL23_GET_CLIENT_HELLO:unknownprotocol/),
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+
constc=net.connect({port: this.address().port},function(){
36+
c.write(bonkers);
37+
});
38+
});

0 commit comments

Comments
(0)