tls: propagate singleUse to the secure context · nodejs/node@ec37f93 · GitHub
Skip to content

Commit ec37f93

Browse files
ViniciusDev26aduh95
authored andcommitted
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>
1 parent 0f62f2b commit ec37f93

3 files changed

Lines changed: 165 additions & 11 deletions

File tree

‎lib/internal/tls/common.js‎

Lines changed: 8 additions & 0 deletions

‎lib/internal/tls/wrap.js‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ const kDisableRenegotiation = Symbol('disable-renegotiation');
104104
const kErrorEmitted = Symbol('error-emitted');
105105
const kHandshakeTimeout = Symbol('handshake-timeout');
106106
const kRes = Symbol('res');
107+
const kSecureContext = Symbol('secureContext');
107108
const kSNICallback = Symbol('snicallback');
108109
const kALPNCallback = Symbol('alpncallback');
109110
const kEnableTrace = Symbol('enableTrace');
@@ -651,6 +652,7 @@ function TLSSocket(socket, opts) {
651652
this.authorized = false;
652653
this.authorizationError = null;
653654
this[kRes] = null;
655+
this[kSecureContext] = null;
654656
this[kIsVerified] = false;
655657
this[kPendingSession] = null;
656658

@@ -749,6 +751,18 @@ for (const proxiedMethod of proxiedMethods) {
749751
makeMethodProxy(proxiedMethod);
750752
}
751753

754+
// A single-use context is owned by the socket that created it, so its SSL_CTX
755+
// can be released as soon as the socket is gone instead of waiting for the JS
756+
// wrapper to be garbage collected. It is shared by every handle the socket
757+
// goes through, so it must outlive any individual TLSWrap.
758+
function closeSingleUseContext(secureContext) {
759+
if (secureContext !== null && secureContext.singleUse &&
760+
secureContext.context !== null) {
761+
secureContext.context.close();
762+
secureContext.context = null;
763+
}
764+
}
765+
752766
tls_wrap.TLSWrap.prototype.close = function close(cb) {
753767
let ssl;
754768
if (this[owner_symbol]) {
@@ -761,10 +775,6 @@ tls_wrap.TLSWrap.prototype.close = function close(cb) {
761775
const done = () => {
762776
if (ssl) {
763777
ssl.destroySSL();
764-
if (ssl._secureContext.singleUse) {
765-
ssl._secureContext.context.close();
766-
ssl._secureContext.context = null;
767-
}
768778
}
769779
if (cb)
770780
cb();
@@ -822,6 +832,7 @@ TLSSocket.prototype._wrapHandle = function(wrap, handle, wrapHasActiveWriteFromP
822832
res._parent = handle; // C++ "wrap" object: TCPWrap, JSStream, ...
823833
res._parentWrap = wrap; // JS object: net.Socket, JSStreamSocket, ...
824834
res._secureContext = context;
835+
this[kSecureContext] = context;
825836
res.reading = handle.reading;
826837
this[kRes] = res;
827838
defineHandleReading(this, handle);
@@ -887,13 +898,16 @@ function destroySSL(self) {
887898
}
888899

889900
TLSSocket.prototype._destroySSL = function _destroySSL() {
890-
if (!this.ssl) return;
891-
this.ssl.destroySSL();
892-
if (this.ssl._secureContext.singleUse) {
893-
this.ssl._secureContext.context.close();
894-
this.ssl._secureContext.context = null;
895-
}
896-
this.ssl = null;
901+
if (this.ssl) {
902+
this.ssl.destroySSL();
903+
this.ssl = null;
904+
}
905+
// Release the context here rather than when a handle closes: a socket that
906+
// swaps handles, as autoSelectFamily retries do, shares one context across
907+
// the successive TLSWraps and still needs it after the old handle is gone.
908+
// TLSWrap.close() may already have cleared `ssl` on the way here.
909+
closeSingleUseContext(this[kSecureContext]);
910+
this[kSecureContext] = null;
897911
this[kPendingSession] = null;
898912
this[kIsVerified] = false;
899913
};
Lines changed: 132 additions & 0 deletions

0 commit comments

Comments
 (0)