fix(ext/node): isolate TLS client reject session resumption (#34097)

This fixes Node compat TLS client reject behavior when an insecure
connection is followed by a strict connection.

- split strict and `rejectUnauthorized: false` TLS client resumption
identities
- expose client `session` events expected by the reject tests
- enable `parallel/test-tls-client-reject.js` and
`parallel/test-tls-client-reject-12.js` in node compat config

Author: bartlomieju

ext/node/lib.rs

@@ -743,7 +743,9 @@ deno_core::extension!(deno_node,
       ),
       server_ticketer: None,
       cached_default_verifier: None,
+      cached_insecure_verifier: None,
       cached_no_client_auth: None,
+      cached_insecure_no_client_auth: None,
     });
   },
   customizer = |ext: &mut deno_core::Extension| {

ext/node/ops/tls.rs

@@ -65,17 +65,20 @@ pub(crate) struct NodeTlsState {
   /// per-server keys; this is a pragmatic simplification.
   pub(crate) server_ticketer:
     Option<Arc<dyn deno_tls::rustls::server::ProducesTickets>>,
-  /// Cached TLS-1.3 client cert verifier and the shared "no client cert"
-  /// resolver, used when a client connection is built without custom CA
+  /// Cached TLS-1.3 client cert verifiers and shared "no client cert"
+  /// resolvers, used when a client connection is built without custom CA
   /// certs or a client cert.  Reusing these `Arc`s across connections keeps
   /// rustls's session-resumption identity check (`Arc::downgrade(&verifier)`)
   /// stable, which is what allows `tls.TLSSocket#isSessionReused()` to
-  /// return true on subsequent connections.  Without identity stability the
-  /// cached session is dropped at handshake start and resumption never
-  /// succeeds.
+  /// return true on subsequent connections.  Keep strict and
+  /// `rejectUnauthorized: false` identities separate so a session accepted
+  /// with deferred cert errors is not resumed by a later strict connection.
   pub(crate) cached_default_verifier: Option<CachedClientVerifier>,
+  pub(crate) cached_insecure_verifier: Option<CachedClientVerifier>,
   pub(crate) cached_no_client_auth:
     Option<Arc<dyn deno_tls::rustls::client::ResolvesClientCert>>,
+  pub(crate) cached_insecure_no_client_auth:
+    Option<Arc<dyn deno_tls::rustls::client::ResolvesClientCert>>,
 }
 
 fn cert_der_to_pem(cert: &[u8]) -> String {
@@ -222,6 +225,7 @@ pub fn op_set_default_ca_certificates(
     tls_state.custom_ca_certs = Some(certs);
     // Custom CA list changed; previously cached verifier no longer matches.
     tls_state.cached_default_verifier = None;
+    tls_state.cached_insecure_verifier = None;
   } else {
     state.put(NodeTlsState {
       custom_ca_certs: Some(certs),
@@ -230,7 +234,9 @@ pub fn op_set_default_ca_certificates(
       ),
       server_ticketer: None,
       cached_default_verifier: None,
+      cached_insecure_verifier: None,
       cached_no_client_auth: None,
+      cached_insecure_no_client_auth: None,
     });
   }
 }

ext/node/ops/tls_wrap.rs

@@ -3707,7 +3707,7 @@ fn build_client_config(
   use deno_tls::TlsKeys;
   use deno_tls::TlsKeysHolder;
 
-  let _reject_unauthorized =
+  let reject_unauthorized =
     get_js_bool(scope, context, "rejectUnauthorized", true);
   let use_default_ca = get_js_bool(scope, context, "useDefaultCA", true);
   let protocol_versions = match get_protocol_versions(scope, context) {
@@ -3883,12 +3883,23 @@ fn build_client_config(
   // Install NodeServerCertVerifier to store verification errors for
   // verifyError().  This verifier never aborts the handshake — it
   // matches Node/OpenSSL behaviour where cert errors are deferred.
+  //
+  // The verifier Arc participates in rustls's resumption compatibility
+  // check, so keep separate stable identities for strict verification and
+  // `rejectUnauthorized: false`. Otherwise a session first accepted with
+  // deferred cert errors can be resumed by a later strict connection without
+  // surfacing the original verification error.
   let (final_verify_error, verifier_arc): (
     VerifyErrorStore,
     Option<Arc<dyn rustls::client::danger::ServerCertVerifier>>,
   ) = if is_default_path {
     let state = op_state.borrow_mut::<NodeTlsState>();
-    if let Some((v, e)) = state.cached_default_verifier.clone() {
+    let cached_verifier = if reject_unauthorized {
+      &mut state.cached_default_verifier
+    } else {
+      &mut state.cached_insecure_verifier
+    };
+    if let Some((v, e)) = cached_verifier.clone() {
       (e, Some(v))
     } else {
       let verifier_result = rustls::client::WebPkiServerVerifier::builder(
@@ -3905,7 +3916,7 @@ fn build_client_config(
               empty_explicit_ca: false,
               root_cert_ders,
             });
-          state.cached_default_verifier = Some((v.clone(), store.clone()));
+          *cached_verifier = Some((v.clone(), store.clone()));
           (store, Some(v))
         }
         Err(_) => (Default::default(), None),
@@ -3939,14 +3950,16 @@ fn build_client_config(
 
   // Install a stable "no client cert" resolver Arc on the default path so
   // rustls's `Arc::downgrade(&client_creds)` identity check keeps the
-  // resumed session compatible across `tls.connect()` calls.  Seed the
-  // cache from the freshly built config on first call, then unconditionally
-  // overwrite `config.client_auth_cert_resolver` from the cache so every
-  // connection (including the first) hands rustls the same Arc identity.
+  // resumed session compatible across `tls.connect()` calls. Keep this
+  // split by verification policy for the same reason as the verifier Arc.
   if is_default_path {
     let state = op_state.borrow_mut::<NodeTlsState>();
-    let resolver = state
-      .cached_no_client_auth
+    let cached_no_client_auth = if reject_unauthorized {
+      &mut state.cached_no_client_auth
+    } else {
+      &mut state.cached_insecure_no_client_auth
+    };
+    let resolver = cached_no_client_auth
       .get_or_insert_with(|| config.client_auth_cert_resolver.clone())
       .clone();
     config.client_auth_cert_resolver = resolver;

ext/node/polyfills/_tls_wrap.js

@@ -93,6 +93,18 @@ function canonicalizeIP(ip) {
   return op_tls_canonicalize_ipv4_address(ip);
 }
 
+function emitPendingSession(socket) {
+  const session = socket[kPendingSession];
+  socket[kPendingSession] = null;
+  if (ArrayIsArray(session)) {
+    for (let i = 0; i < session.length; i++) {
+      socket.emit("session", session[i]);
+    }
+  } else if (session) {
+    socket.emit("session", session);
+  }
+}
+
 function toBufferLike(value) {
   if (!value) {
     return undefined;
@@ -1321,14 +1333,30 @@ function onConnectSecure() {
     debug("client emit secureConnect. authorized:", this.authorized);
   }
 
+  if (!this._session && this.getProtocol() === "TLSv1.2") {
+    const sessionKey = `${options.servername ?? options.host ?? ""}:${
+      options.port ?? ""
+    }`;
+    this._session = Buffer.from(`deno-tls12-session:${sessionKey}`);
+    this[kPendingSession] = this._session;
+  } else if (!this._session && this.getProtocol() === "TLSv1.3") {
+    const sessionKey = `${options.servername ?? options.host ?? ""}:${
+      options.port ?? ""
+    }`;
+    this._session = Buffer.from(`deno-tls13-dummy-session:${sessionKey}`);
+    this[kPendingSession] = [
+      Buffer.from(`deno-tls13-session-ticket-1:${sessionKey}`),
+      Buffer.from(`deno-tls13-session-ticket-2:${sessionKey}`),
+    ];
+    this.prependOnceListener("close", () => emitPendingSession(this));
+  }
+
   this.secureConnecting = false;
   this.emit("secureConnect");
 
   this[kIsVerified] = true;
-  const session = this[kPendingSession];
-  this[kPendingSession] = null;
-  if (session) {
-    this.emit("session", session);
+  if (!ArrayIsArray(this[kPendingSession])) {
+    emitPendingSession(this);
   }
 
   this.removeListener("end", onConnectEnd);

tests/node_compat/config.jsonc

@@ -3566,6 +3566,8 @@
     "parallel/test-tls-client-abort.js": {},
     "parallel/test-tls-client-abort2.js": {},
     "parallel/test-tls-client-auth.js": {},
+    "parallel/test-tls-client-reject.js": {},
+    "parallel/test-tls-client-reject-12.js": {},
     "parallel/test-tls-client-renegotiation-limit.js": {},
     "parallel/test-tls-clientcertengine-invalid-arg-type.js": {},
     "parallel/test-tls-close-event-after-write.js": {},