From 101de9de3f4095efd9cd12bbb01fd773b1c6f0ef Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 30 Jan 2016 18:49:11 -0500 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/3692 PR-URL: https://github.com/nodejs/node/pull/4982 Reviewed-By: Evan Lucas Reviewed-By: Shigeki Ohtsu --- lib/https.js | 16 ++++ .../test-https-agent-session-eviction.js | 88 +++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 test/parallel/test-https-agent-session-eviction.js diff --git a/lib/https.js b/lib/https.js index 8e79d295bf5ea6..75beb01ebf8fd2 100644 --- a/lib/https.js +++ b/lib/https.js @@ -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; } @@ -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; diff --git a/test/parallel/test-https-agent-session-eviction.js b/test/parallel/test-https-agent-session-eviction.js new file mode 100644 index 00000000000000..df6fdc3658c2c9 --- /dev/null +++ b/test/parallel/test-https-agent-session-eviction.js @@ -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(); +}