Skip to content

Commit

Permalink
https: evict cached sessions on error
Browse files Browse the repository at this point in the history
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 <evanlucas@me.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
  • Loading branch information
indutny authored and rvagg committed Feb 8, 2016
1 parent 54e8845 commit 101de9d
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 0 deletions.
16 changes: 16 additions & 0 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ function createConnection(port, host, options) {

self._cacheSession(options._agentKey, socket.getSession());
});

// Evict session on error
socket.once('close', (err) => {
if (err)
this._evictSession(options._agentKey);
});

return socket;
}

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

Agent.prototype._evictSession = function _evictSession(key) {
const index = this._sessionCache.list.indexOf(key);
if (index === -1)
return;

this._sessionCache.list.splice(index, 1);
delete this._sessionCache.map[key];
};

const globalAgent = new Agent();

exports.globalAgent = globalAgent;
Expand Down
88 changes: 88 additions & 0 deletions test/parallel/test-https-agent-session-eviction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
'use strict';

const common = require('../common');

if (!common.hasCrypto) {
console.log('1..0 # Skipped: missing crypto');
return;
}

const assert = require('assert');
const https = require('https');
const fs = require('fs');
const constants = require('constants');

const options = {
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'),
secureOptions: constants.SSL_OP_NO_TICKET
};

// Create TLS1.2 server
https.createServer(options, function(req, res) {
res.end('ohai');
}).listen(common.PORT, function() {
first(this);
});

// Do request and let agent cache the session
function first(server) {
const req = https.request({
port: common.PORT,
rejectUnauthorized: false
}, function(res) {
res.resume();

server.close(function() {
faultyServer();
});
});
req.end();
}

// Create TLS1 server
function faultyServer() {
options.secureProtocol = 'TLSv1_method';
https.createServer(options, function(req, res) {
res.end('hello faulty');
}).listen(common.PORT, function() {
second(this);
});
}

// Attempt to request using cached session
function second(server, session) {
const req = https.request({
port: common.PORT,
rejectUnauthorized: false
}, function(res) {
res.resume();
});

// Let it fail
req.on('error', common.mustCall(function(err) {
assert(/wrong version number/.test(err.message));

req.on('close', function() {
third(server);
});
}));
req.end();
}

// Try on more time - session should be evicted!
function third(server) {
const req = https.request({
port: common.PORT,
rejectUnauthorized: false
}, function(res) {
res.resume();
assert(!req.socket.isSessionReused());
server.close();
});
req.on('error', function(err) {
// never called
assert(false);
});
req.end();
}

0 comments on commit 101de9d

Please sign in to comment.