Skip to content

Commit 101de9d

Browse files
indutnyrvagg
authored andcommitted
https: evict cached sessions on error
Instead of using the same session over and over, evict it when the socket emits error. This could be used as a mitigation of #3692, until OpenSSL fix will be merged/released. See: #3692 PR-URL: #4982 Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]>
1 parent 54e8845 commit 101de9d

File tree

2 files changed

+104
-0
lines changed

2 files changed

+104
-0
lines changed

‎lib/https.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,13 @@ function createConnection(port, host, options){
8383

8484
self._cacheSession(options._agentKey,socket.getSession());
8585
});
86+
87+
// Evict session on error
88+
socket.once('close',(err)=>{
89+
if(err)
90+
this._evictSession(options._agentKey);
91+
});
92+
8693
returnsocket;
8794
}
8895

@@ -163,6 +170,15 @@ Agent.prototype._cacheSession = function _cacheSession(key, session){
163170
this._sessionCache.map[key]=session;
164171
};
165172

173+
Agent.prototype._evictSession=function_evictSession(key){
174+
constindex=this._sessionCache.list.indexOf(key);
175+
if(index===-1)
176+
return;
177+
178+
this._sessionCache.list.splice(index,1);
179+
deletethis._sessionCache.map[key];
180+
};
181+
166182
constglobalAgent=newAgent();
167183

168184
exports.globalAgent=globalAgent;
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
5+
if(!common.hasCrypto){
6+
console.log('1..0 # Skipped: missing crypto');
7+
return;
8+
}
9+
10+
constassert=require('assert');
11+
consthttps=require('https');
12+
constfs=require('fs');
13+
constconstants=require('constants');
14+
15+
constoptions={
16+
key: fs.readFileSync(common.fixturesDir+'/keys/agent1-key.pem'),
17+
cert: fs.readFileSync(common.fixturesDir+'/keys/agent1-cert.pem'),
18+
secureOptions: constants.SSL_OP_NO_TICKET
19+
};
20+
21+
// Create TLS1.2 server
22+
https.createServer(options,function(req,res){
23+
res.end('ohai');
24+
}).listen(common.PORT,function(){
25+
first(this);
26+
});
27+
28+
// Do request and let agent cache the session
29+
functionfirst(server){
30+
constreq=https.request({
31+
port: common.PORT,
32+
rejectUnauthorized: false
33+
},function(res){
34+
res.resume();
35+
36+
server.close(function(){
37+
faultyServer();
38+
});
39+
});
40+
req.end();
41+
}
42+
43+
// Create TLS1 server
44+
functionfaultyServer(){
45+
options.secureProtocol='TLSv1_method';
46+
https.createServer(options,function(req,res){
47+
res.end('hello faulty');
48+
}).listen(common.PORT,function(){
49+
second(this);
50+
});
51+
}
52+
53+
// Attempt to request using cached session
54+
functionsecond(server,session){
55+
constreq=https.request({
56+
port: common.PORT,
57+
rejectUnauthorized: false
58+
},function(res){
59+
res.resume();
60+
});
61+
62+
// Let it fail
63+
req.on('error',common.mustCall(function(err){
64+
assert(/wrongversionnumber/.test(err.message));
65+
66+
req.on('close',function(){
67+
third(server);
68+
});
69+
}));
70+
req.end();
71+
}
72+
73+
// Try on more time - session should be evicted!
74+
functionthird(server){
75+
constreq=https.request({
76+
port: common.PORT,
77+
rejectUnauthorized: false
78+
},function(res){
79+
res.resume();
80+
assert(!req.socket.isSessionReused());
81+
server.close();
82+
});
83+
req.on('error',function(err){
84+
// never called
85+
assert(false);
86+
});
87+
req.end();
88+
}

0 commit comments

Comments
(0)