tls: propagate singleUse to the secure context

tls.connect() sets options.singleUse, and both TLSWrap.prototype.close()
and TLSSocket.prototype._destroySSL() check _secureContext.singleUse
before closing the context. Nothing ever assigned the flag to the
context, so the check never fired and each connection's SSL_CTX stayed
alive until the JS wrapper was garbage collected. The assignment was
lost when the context configuration was extracted into
configSecureContext().

Restore it in createSecureContext(), which owns the JS wrapper the flag
is read from, and make the close idempotent: a socket that swaps its
handle, as autoSelectFamily retries do, shares a single context between
the successive TLSWraps and each of them reaches the same teardown.

Signed-off-by: Carlos Vinicius <viniciusdev.26@gmail.com>
PR-URL: #66025
Fixes: #66002
Refs: #38116
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>

Author: ViniciusDev26

lib/internal/tls/common.js

@@ -100,6 +100,7 @@ function createSecureContext(options) {
     minVersion,
     maxVersion,
     secureProtocol,
+    singleUse,
   } = options;
 
   let { secureOptions } = options;
@@ -112,6 +113,13 @@ function createSecureContext(options) {
 
   configSecureContext(c.context, options);
 
+  // A single-use context belongs to exactly one connection, so the underlying
+  // SSL_CTX can be freed as soon as that connection closes instead of waiting
+  // for the JS wrapper to be garbage collected. TLSSocket relies on this flag
+  // to know that it may close the context itself.
+  if (singleUse)
+    c.singleUse = true;
+
   return c;
 }
 

lib/internal/tls/wrap.js

@@ -104,6 +104,7 @@ const kDisableRenegotiation = Symbol('disable-renegotiation');
 const kErrorEmitted = Symbol('error-emitted');
 const kHandshakeTimeout = Symbol('handshake-timeout');
 const kRes = Symbol('res');
+const kSecureContext = Symbol('secureContext');
 const kSNICallback = Symbol('snicallback');
 const kALPNCallback = Symbol('alpncallback');
 const kEnableTrace = Symbol('enableTrace');
@@ -651,6 +652,7 @@ function TLSSocket(socket, opts) {
   this.authorized = false;
   this.authorizationError = null;
   this[kRes] = null;
+  this[kSecureContext] = null;
   this[kIsVerified] = false;
   this[kPendingSession] = null;
 
@@ -749,6 +751,18 @@ for (const proxiedMethod of proxiedMethods) {
     makeMethodProxy(proxiedMethod);
 }
 
+// A single-use context is owned by the socket that created it, so its SSL_CTX
+// can be released as soon as the socket is gone instead of waiting for the JS
+// wrapper to be garbage collected. It is shared by every handle the socket
+// goes through, so it must outlive any individual TLSWrap.
+function closeSingleUseContext(secureContext) {
+  if (secureContext !== null && secureContext.singleUse &&
+      secureContext.context !== null) {
+    secureContext.context.close();
+    secureContext.context = null;
+  }
+}
+
 tls_wrap.TLSWrap.prototype.close = function close(cb) {
   let ssl;
   if (this[owner_symbol]) {
@@ -761,10 +775,6 @@ tls_wrap.TLSWrap.prototype.close = function close(cb) {
   const done = () => {
     if (ssl) {
       ssl.destroySSL();
-      if (ssl._secureContext.singleUse) {
-        ssl._secureContext.context.close();
-        ssl._secureContext.context = null;
-      }
     }
     if (cb)
       cb();
@@ -822,6 +832,7 @@ TLSSocket.prototype._wrapHandle = function(wrap, handle, wrapHasActiveWriteFromP
   res._parent = handle;  // C++ "wrap" object: TCPWrap, JSStream, ...
   res._parentWrap = wrap;  // JS object: net.Socket, JSStreamSocket, ...
   res._secureContext = context;
+  this[kSecureContext] = context;
   res.reading = handle.reading;
   this[kRes] = res;
   defineHandleReading(this, handle);
@@ -887,13 +898,16 @@ function destroySSL(self) {
 }
 
 TLSSocket.prototype._destroySSL = function _destroySSL() {
-  if (!this.ssl) return;
-  this.ssl.destroySSL();
-  if (this.ssl._secureContext.singleUse) {
-    this.ssl._secureContext.context.close();
-    this.ssl._secureContext.context = null;
-  }
-  this.ssl = null;
+  if (this.ssl) {
+    this.ssl.destroySSL();
+    this.ssl = null;
+  }
+  // Release the context here rather than when a handle closes: a socket that
+  // swaps handles, as autoSelectFamily retries do, shares one context across
+  // the successive TLSWraps and still needs it after the old handle is gone.
+  // TLSWrap.close() may already have cleared `ssl` on the way here.
+  closeSingleUseContext(this[kSecureContext]);
+  this[kSecureContext] = null;
   this[kPendingSession] = null;
   this[kIsVerified] = false;
 };

test/parallel/test-tls-connect-single-use-context.js

@@ -0,0 +1,132 @@
+'use strict';
+
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// tls.connect() builds a SecureContext per connection and marks it as single
+// use, so that the underlying SSL_CTX is released as soon as the socket
+// closes instead of when the JS wrapper happens to be garbage collected.
+// Regression test for https://github.com/nodejs/node/issues/66002, where the
+// flag was lost on its way to the context and nothing was ever closed.
+
+const assert = require('assert');
+const tls = require('tls');
+const fixtures = require('../common/fixtures');
+const { createMockedLookup } = require('../common/dns');
+
+const key = fixtures.readKey('agent1-key.pem');
+const cert = fixtures.readKey('agent1-cert.pem');
+
+const server = tls.createServer({ key, cert }, (conn) => conn.end());
+
+// Bound to a single address so that the addresses the handle-swap case
+// retries through are refused rather than answered by this server.
+server.listen(0, '127.0.0.1', common.mustCall(() => {
+  connectWithOwnContext(common.mustCall(() => {
+    connectWithSharedContext(common.mustCall(() => {
+      connectAcrossHandleSwaps(common.mustCall(() => {
+        connectWithKeepAlive(common.mustCall(() => server.close()));
+      }));
+    }));
+  }));
+}));
+
+// _destroySSL() runs from the immediate queue, after the 'close' event.
+function afterDestroySSL(socket, fn) {
+  socket.on('close', common.mustCall(() => setImmediate(fn)));
+}
+
+function connectWithOwnContext(done) {
+  const socket = tls.connect({
+    host: '127.0.0.1',
+    port: server.address().port,
+    rejectUnauthorized: false,
+  }, common.mustCall(() => {
+    const secureContext = socket.ssl._secureContext;
+    assert.strictEqual(secureContext.singleUse, true);
+    assert.notStrictEqual(secureContext.context, null);
+
+    afterDestroySSL(socket, common.mustCall(() => {
+      assert.strictEqual(secureContext.context, null);
+      done();
+    }));
+  }));
+}
+
+function connectWithSharedContext(done) {
+  // A context passed in by the user may outlive the connection, so it must
+  // not be marked single use, and it must still work for the next socket.
+  const secureContext = tls.createSecureContext();
+  let remaining = 2;
+
+  (function connectOnce() {
+    const socket = tls.connect({
+      host: '127.0.0.1',
+      port: server.address().port,
+      rejectUnauthorized: false,
+      secureContext,
+    }, common.mustCall(() => {
+      assert.strictEqual(socket.ssl._secureContext, secureContext);
+      assert.strictEqual(secureContext.singleUse, undefined);
+
+      afterDestroySSL(socket, common.mustCall(() => {
+        assert.notStrictEqual(secureContext.context, null);
+        if (--remaining === 0) done();
+        else connectOnce();
+      }));
+    }));
+  })();
+}
+
+function connectAcrossHandleSwaps(done) {
+  // autoSelectFamily reinitializes the handle on every failed attempt, and the
+  // successive TLSWraps share the socket's context. Releasing it with the old
+  // handle leaves the next attempt without a context. Two failing addresses
+  // are needed: the close happens on the first swap, and the next swap is what
+  // trips over it.
+  const socket = tls.connect({
+    host: 'example.org',
+    port: server.address().port,
+    rejectUnauthorized: false,
+    autoSelectFamily: true,
+    autoSelectFamilyAttemptTimeout:
+      common.defaultAutoSelectFamilyAttemptTimeout,
+    lookup: createMockedLookup('::1', '127.0.0.2', '127.0.0.1'),
+  }, common.mustCall(() => {
+    // `ssl` is cleared while the handle is swapped, so read the context from
+    // the handle the socket ended up with.
+    const secureContext = socket._handle._secureContext;
+    assert.strictEqual(secureContext.singleUse, true);
+    assert.notStrictEqual(secureContext.context, null);
+
+    afterDestroySSL(socket, common.mustCall(() => {
+      assert.strictEqual(secureContext.context, null);
+      done();
+    }));
+  }));
+}
+
+function connectWithKeepAlive(done) {
+  // tls.connect() only asks for a single-use context when TCP keepalive is
+  // off, so a client that enables it leaves its context to the garbage
+  // collector. Pinned here as the current behaviour: whether the context
+  // should be released early in this case too is the open question in the
+  // issue, and is deliberately not decided by this change.
+  const socket = tls.connect({
+    host: '127.0.0.1',
+    port: server.address().port,
+    rejectUnauthorized: false,
+    keepAlive: true,
+  }, common.mustCall(() => {
+    const secureContext = socket.ssl._secureContext;
+    assert.strictEqual(secureContext.singleUse, undefined);
+    assert.notStrictEqual(secureContext.context, null);
+
+    afterDestroySSL(socket, common.mustCall(() => {
+      assert.notStrictEqual(secureContext.context, null);
+      done();
+    }));
+  }));
+}