Skip to content

Commit b6941b0

Browse files
lekoderMylesBorins
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. 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]>
1 parent dcbc1b4 commit b6941b0

File tree

3 files changed

+77
-1
lines changed

3 files changed

+77
-1
lines changed

‎lib/_tls_wrap.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,9 @@ TLSSocket.prototype._init = function(socket, wrap){
463463

464464
// Destroy socket if error happened before handshake's finish
465465
if(!self._secureEstablished){
466-
self.destroy(self._tlsError(err));
466+
// When handshake fails control is not yet released,
467+
// so self._tlsError will return null instead of actual error
468+
self.destroy(err);
467469
}elseif(options.isServer&&
468470
rejectUnauthorized&&
469471
/peerdidnotreturnacertificate/.test(err.message)){
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
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+
letclientErrorEmited=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('clientError',function(e){
23+
clientErrorEmited=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(clientErrorEmited,'clientError should be emited');
35+
36+
},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)