tls: fix legacy SecurePair clienthello race window

bnoordhuis · BethGriggs · commit 7573b55a1505 · 2019-03-20T23:53:42.000Z
There is a time window between the first and the last step of processing the clienthello event and the SecurePair may have been destroyed during that interval. Fixes: #26428 PR-URL: #26452 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
diff --git a/lib/_tls_legacy.js b/lib/_tls_legacy.js
@@ -659,6 +659,10 @@ function onclienthello(hello) {
     if (err) return self.socket.destroy(err);
 
     setImmediate(function() {
+      // SecurePair might have been destroyed in the time window
+      // between callback() and this function.
+      if (!self.ssl) return;
+
       self.ssl.loadSession(session);
       self.ssl.endParser();
 
diff --git a/test/parallel/test-tls-async-cb-after-socket-end-securepair.js b/test/parallel/test-tls-async-cb-after-socket-end-securepair.js
@@ -0,0 +1,80 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const fixtures = require('../common/fixtures');
+const SSL_OP_NO_TICKET = require('crypto').constants.SSL_OP_NO_TICKET;
+const assert = require('assert');
+const net = require('net');
+const tls = require('tls');
+
+const options = {
+  secureOptions: SSL_OP_NO_TICKET,
+  key: fixtures.readSync('test_key.pem'),
+  cert: fixtures.readSync('test_cert.pem')
+};
+
+const server = net.createServer(function(socket) {
+  const sslcontext = tls.createSecureContext(options);
+  sslcontext.context.setCiphers('RC4-SHA:AES128-SHA:AES256-SHA');
+
+  const pair = tls.createSecurePair(sslcontext, true, false, false, { server });
+
+  assert.ok(pair.encrypted.writable);
+  assert.ok(pair.cleartext.writable);
+
+  pair.encrypted.pipe(socket);
+  socket.pipe(pair.encrypted);
+
+  pair.on('error', () => {});  // Expected, client s1 closes connection.
+});
+
+let sessionCb = null;
+let client = null;
+
+server.on('newSession', common.mustCall(function(key, session, done) {
+  done();
+}));
+
+server.on('resumeSession', common.mustCall(function(id, cb) {
+  sessionCb = cb;
+
+  next();
+}));
+
+server.listen(0, function() {
+  const clientOpts = {
+    port: this.address().port,
+    rejectUnauthorized: false,
+    session: false
+  };
+
+  const s1 = tls.connect(clientOpts, function() {
+    clientOpts.session = s1.getSession();
+    console.log('1st secure');
+
+    s1.destroy();
+    const s2 = tls.connect(clientOpts, function(s) {
+      console.log('2nd secure');
+
+      s2.destroy();
+    }).on('connect', function() {
+      console.log('2nd connected');
+      client = s2;
+
+      next();
+    });
+  });
+});
+
+function next() {
+  if (!client || !sessionCb)
+    return;
+
+  client.destroy();
+  setTimeout(function() {
+    sessionCb();
+    server.close();
+  }, 100);
+}
