Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/js/node/tls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,9 @@ var InternalSecureContext = class SecureContext {
};

function SecureContext(options): void {
return new InternalSecureContext(options) as never;
// Same contract as createSecureContext(): user-constructed contexts own
// their SSL_CTX exclusively (see the note there), so delegate to it.
return createSecureContext(options) as never;
Comment thread
alii marked this conversation as resolved.
}

function createSecureContext(options) {
Expand Down Expand Up @@ -914,8 +916,8 @@ function TLSSocket(socket?, options?) {
// server-upgrade method below; leaving it unset until then means a synchronous
// teardown during upgradeTLS won't call close() on the bare net.Socket.
}
// Internal path: keep the per-digest cache (only the user-facing
// tls.createSecureContext() owns its SSL_CTX exclusively).
// Internal path: keep the per-digest cache (the user-facing constructors,
// createSecureContext() and new tls.SecureContext(), own theirs exclusively).
this[ksecureContext] = options.secureContext || new InternalSecureContext(options);
this.authorized = false;
this.secureConnecting = true;
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/api/SecureContext.classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ export default [
// digest so identical configs return the same JS cell. Replaces the
// old SHA-256/WeakRef cache that lived in `tls.ts`.
intern: { fn: "intern", length: 1 },
// `tls.createSecureContext()` — exclusive-ownership variant: no digest
// memoisation at either cache level, so addCACert on one context can
// never affect another. The connect/listen paths keep using `intern`.
// The user-facing constructors (`tls.createSecureContext()` and
// `new tls.SecureContext()`) — exclusive ownership: no digest memoisation
// at either cache level, so addCACert on one context can never affect
// another. The internal connect/listen paths keep using `intern`.
createPrivate: { fn: "create_private", length: 1 },
// Parses a PKCS#12 (`pfx`) blob into { key, cert, ca } PEM strings so
// the regular key/cert/ca option plumbing can consume it.
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/api/bun/SecureContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl SecureContext {
Ok(result)
}

/// `tls.createSecureContext()` entry - builds a context that owns its
/// `tls.createSecureContext()` / `new tls.SecureContext()` entry - builds a context that owns its
/// SSL_CTX exclusively: no digest memoisation at either the JS-wrapper
/// cache or the native SSLContextCache level, so prototype mutators like
/// `addCACert` can never affect another context (or the cached
Expand Down
34 changes: 34 additions & 0 deletions test/js/node/tls/ssl-ctx-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,40 @@ test("addCACert on one user-facing context does not affect another with identica
expect(a.context).not.toBe(b.context);
});

// The exported constructor is user-facing too: it must never hand out the
// digest-interned SSL_CTX, or addCACert on one instance would silently extend
// the trust store of every context sharing that digest.
test("new tls.SecureContext() owns its native handle exclusively, like createSecureContext()", () => {
const a = new (tls as any).SecureContext({ ca: tlsCerts.cert });
const b = new (tls as any).SecureContext({ ca: tlsCerts.cert });
// The interned cache would hand both the same native cell.
expect(a.context).not.toBe(b.context);
});

test("addCACert on one exported SecureContext instance does not change what another verifies", async () => {
// agent6's chain roots at ca1, which is not in the default roots: a client
// using `b` must keep rejecting it after `a` starts trusting ca1 (Node
// isolates the contexts and fails this connection closed).
const fx = (n: string) => readFileSync(join(import.meta.dir, "fixtures", n), "utf8");
const server = tls.createServer({ key: fx("agent6-key.pem"), cert: fx("agent6-cert.pem") }, s => s.end());
server.listen(0);
await once(server, "listening");
const { port } = server.address() as import("net").AddressInfo;
const a = new (tls as any).SecureContext({ ca: fx("ca2-cert.pem") });
const b = new (tls as any).SecureContext({ ca: fx("ca2-cert.pem") });
a.context.addCACert(fx("ca1-cert.pem"));
const outcome = Promise.withResolvers<string>();
const socket = tls.connect({ port, secureContext: b, checkServerIdentity: () => undefined });
socket.on("secureConnect", () => outcome.resolve(`secureConnect authorized=${socket.authorized}`));
Comment thread
alii marked this conversation as resolved.
socket.on("error", error => outcome.resolve(`error ${(error as NodeJS.ErrnoException).code}`));
try {
expect(await outcome.promise).toBe("error UNABLE_TO_GET_ISSUER_CERT_LOCALLY");
} finally {
socket.destroy();
server.close();
}
});
Comment thread
claude[bot] marked this conversation as resolved.

test("setDefaultCACertificates() override applies to plain tls.connect (no explicit ca)", async () => {
const keys = (f: string) => readFileSync(join(import.meta.dir, "../test/fixtures/keys", f));
const prev = tls.getCACertificates("default");
Expand Down
Loading