https: evict cached sessions on error
authorFedor Indutny <fedor@indutny.com>
Sat, 30 Jan 2016 23:49:11 +0000 (18:49 -0500)
committerMyles Borins <mborins@us.ibm.com>
Wed, 2 Mar 2016 22:01:11 +0000 (14:01 -0800)
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 <evanlucas@me.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
lib/https.js
test/parallel/test-https-agent-session-eviction.js [new file with mode: 0644]

index f13c1ff..25c24a3 100644 (file)
@@ -76,6 +76,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;
 }
 
@@ -152,6 +159,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 (file)
index 0000000..df6fdc3
--- /dev/null
@@ -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();
+}