unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
denoland/deno
senior8/10
securityerror-handlingbackwards-compatibility

fix(ext/node): do not resume client TLS sessions unless requested

Project: denoland/deno | Language: Rust, JavaScript | Concepts: security, error-handling, backwards-compatibility | Difficulty: senior
Link: https://github.com/denoland/deno/pull/36592

The problem

TLS client connections in Deno's Node.js compatibility layer were always offering session resumption to servers, even when the user did not explicitly request it (via options.session or setSession()). This broke compatibility with servers (notably SQL Server via mssql/tedious) that do not support TLS session resumption, causing subsequent pooled connections to fail with handshake timeouts.

Why it's non-trivial

A naive fix would simply disable session caching or resumption, but that would break Node.js compatibility: Node only resumes sessions when explicitly requested, but still emits 'session' events and supports getSession(). The fix must:

  • Only offer session resumption when requested, but continue to store session tickets for later use.
  • Preserve correct behavior across connection fallback/re-initialization (e.g., Happy Eyeballs).
  • Maintain compatibility with Node's APIs and event semantics.
  • Handle subtle cases like process-level custom CA certificates, which affect Rustls's session cache identity.

The approach

1. Add a per-connection resumption flag and wrapper

A new allow_resumption flag is added to each TLS connection, and a wrapper is introduced to gate session resumption:

+  allow_resumption: Arc<AtomicBool>,

This flag tracks whether session resumption should be allowed for the current connection.

The NodeClientSessionStoreWrapper struct wraps the underlying session store. It always records new sessions, but only offers them for resumption if allow_resumption is true:

+struct NodeClientSessionStoreWrapper {
+  inner: Arc<dyn rustls::client::ClientSessionStore>,
+  allow_resumption: Arc<AtomicBool>,
+}
+
+impl rustls::client::ClientSessionStore for NodeClientSessionStoreWrapper {
+  // ... (other methods)
+  fn tls12_session(
+    &self,
+    server_name: &rustls::pki_types::ServerName<'_>,
+  ) -> Option<rustls::client::Tls12ClientSessionValue> {
+    if self.allow_resumption.load(Ordering::Relaxed) {
+      self.inner.tls12_session(server_name)
+    } else {
+      None
+    }
+  }
+  fn take_tls13_ticket(
+    &self,
+    server_name: &rustls::pki_types::ServerName<'_>,
+  ) -> Option<rustls::client::Tls13ClientSessionValue> {
+    if self.allow_resumption.load(Ordering::Relaxed) {
+      self.inner.take_tls13_ticket(server_name)
+    } else {
+      None
+    }
+  }
+}

Annotation: This ensures session resumption is opt-in, but session tickets are still stored for API compatibility.

2. Expose a fast op to toggle resumption from JS

A new fast op is exposed to JS to toggle the flag:

+  #[fast]
+  fn set_session_allowed(&self, allowed: bool) {
+    let inner = unsafe { &*self.inner.as_ptr() };
+    inner.allow_resumption.store(allowed, Ordering::Relaxed);
+  }

Annotation: This allows the JS layer to control when session resumption is permitted, matching Node.js semantics.

3. Update JS layer to control resumption

In the JS polyfill, the session is only applied (and thus resumption allowed) if it has been validated and matches the destination:

+  if (this._session && this._sessionReused) {
+    this._handle.setSessionAllowed(true);
+  }

Annotation: This prevents accidental resumption with mismatched or invalid session data, especially during connection fallback.

4. Ensure correct session cache identity with custom CAs

The Rust config builder is updated so that process-level custom CAs do not break the session cache identity, which would otherwise prevent resumption from working after setDefaultCACertificates():

-  let is_default_path = ca_certs.is_empty()
+  let is_default_path = !has_explicit_ca
     && use_default_ca
-    && op_state
-      .try_borrow::<NodeTlsState>()
-      .is_none_or(|s| s.custom_ca_certs.is_none())
     && matches!(maybe_cert_chain_and_key, TlsKeys::Null);

Annotation: This ensures that opt-in session resumption still works if the user sets process-level custom CAs.

5. Regression tests

A comprehensive regression test is added to verify all cases:

+Deno.test("tls client session resumption is opt-in per connection", async (t) => {
+  // ... start server ...
+  await t.step("initial connection is a full handshake", async () => {
+    const res = await connectTlsClient({ port, host: "127.0.0.1" });
+    assertEquals(res.isReused, false);
+    assert(res.session !== null, "expected session data from connection 1");
+    firstSession = res.session;
+  });
+  await t.step(
+    "connection without options.session does not resume",
+    async () => {
+      const res = await connectTlsClient({ port, host: "127.0.0.1" });
+      assertEquals(res.isReused, false);
+    },
+  );
+  await t.step("options.session enables resumption", async () => {
+    const res = await connectTlsClient({
+      port,
+      host: "127.0.0.1",
+      session: firstSession!,
+    });
+    assertEquals(res.isReused, true);
+  });
+  await t.step("setSession() enables resumption", async () => {
+    const res = await connectTlsClient(
+      { port, host: "127.0.0.1" },
+      firstSession,
+    );
+    assertEquals(res.isReused, true);
+  });
+  // ...
+});

Annotation: This validates that session resumption is only used when explicitly requested and remains compatible with Node.js behavior.

From the review thread

bartlomieju:
This is a second, distinct fix bundled into the PR (commit e5ba4bf) and it isn't mentioned in the description. I traced it and I agree it's correct — op_set_default_ca_certificates clears both cached_default_verifier and cached_insecure_verifier on every change (ext/node/ops/tls.rs:236-238), so the verifier Arc identity that rustls keys compatible_config on stays consistent, and without this change opt-in resumption would silently never work after tls.setDefaultCACertificates().

Two asks:

  1. Please describe it in the PR body — it's a behavior change reviewers shouldn't have to reverse-engineer from the diff.
  2. It has no test. Please add one: setDefaultCACertificates(), then two tls.connect()s where the second passes options.session, and assert isSessionReused() === true. That's exactly the regression this line guards.

bartlomieju:
P2: Revalidate sessions when reinitializing the handle

When autoSelectFamily falls back to another address after an invalid or host/port-mismatched session was supplied, setSession() retains that buffer in this._session but marks _sessionReused false. This path passes the nonempty buffer directly to the new native handle, enabling the shared cache and allowing an unrelated cached session to be resumed despite the earlier validation failure. Please only reapply the session when _sessionReused is true.

Takeaways

  • Session resumption should be opt-in, not opt-out, for compatibility and security: Only offer TLS session resumption when the user explicitly requests it, as some servers do not support it and may fail if it is attempted automatically.
  • Always store session tickets, but gate their use: To match Node.js semantics, always record new session tickets for later use, but only offer them for resumption when requested.
  • Preserve cache identity and handle fallback cases: When working with shared caches (e.g., for TLS sessions), ensure that cache identity is preserved across configuration changes (such as custom CAs) and that session resumption is not accidentally enabled during connection fallback or re-initialization.