From 14bac292bcca7eaccacc1d3a4cb468c8b39e52ec Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 25 May 2026 22:55:55 +0000 Subject: [PATCH 1/2] node:tls: implement setDefaultCACertificates() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tls.setDefaultCACertificates(certs) replaces the default CA trust store used for TLS client verification when no explicit 'ca' option is given. After calling it, tls.getCACertificates('default') returns exactly the supplied (deduplicated) certificates, and new TLS/HTTPS connections verify against that set instead of the bundled Mozilla roots. - root_certs.cpp: mutex-guarded process-global user cert override that us_get_default_ca_store() honours exclusively when present; the cached shared store is invalidated on each setDefaultCACertificates() call. has_user_root_certs is atomic for the lock-free fast-path check. - openssl.c: SSL_CTXs built against the default CA set (request_cert, no explicit ca) are tagged via ex_data so us_internal_ssl_attach() refreshes the per-SSL verify store after the defaults change — the cached fetch/https CTX picks up the override without a rebuild. - NodeTLS.cpp: resetRootCertStore parses PEM strings/ArrayBufferViews (multi-cert bundles supported), deduplicates by X509 identity, and installs the set; getUserRootCertificates returns an up-ref'd PEM snapshot, or undefined when no override is installed. - tls.ts: validates input (ERR_INVALID_ARG_TYPE), queries the native override state so Workers always report the store their connections actually verify against. Fixes #24340 Fixes #13868 --- packages/bun-usockets/src/crypto/openssl.c | 15 +- .../bun-usockets/src/crypto/root_certs.cpp | 114 ++++++- .../src/crypto/root_certs_header.h | 3 + src/js/node/tls.ts | 143 +++------ src/jsc/bindings/NodeTLS.cpp | 217 +++++++++++++ src/jsc/bindings/NodeTLS.h | 2 + test/js/node/test/common/tls.js | 6 +- ...de-tls-set-default-ca-certificates.test.ts | 294 ++++++++++++++++++ test/js/node/tls/ssl-ctx-cache.test.ts | 40 ++- 9 files changed, 710 insertions(+), 124 deletions(-) create mode 100644 test/js/node/tls/node-tls-set-default-ca-certificates.test.ts diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 51daa4e33e63..17fd8afe1ae0 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -139,7 +139,8 @@ static int us_sni_ex_idx = -1; static int us_ctx_cache_ex_idx = -1; /* Marks an SSL_CTX whose verification store holds user-provided CAs (the * ca/caFile options or a later addCACert): the per-socket client attach must - * not replace such a store with the process-shared default roots. */ + * not replace such a store with the process-shared default roots, and + * tls.setDefaultCACertificates() must not override it either. */ static int us_ctx_user_ca_ex_idx = -1; static int us_ssl_reneg_state_idx = -1; /* Per-connection async-SNI suspension state (select_certificate_cb retry). */ @@ -1164,9 +1165,9 @@ void us_internal_ssl_attach(struct us_socket_t *s, SSL_CTX *ctx, * bundle without touching the CTX (servers using the same CTX never pay * the ~150-root build). us_verify_callback returns 1 so the handshake * never aborts here — JS reads verify_error and decides. */ + us_ex_idx_ensure(); if (SSL_CTX_get_verify_mode(ctx) == SSL_VERIFY_NONE) { SSL_set_verify(ssl, SSL_VERIFY_PEER, us_verify_callback); - us_ex_idx_ensure(); if (!SSL_CTX_get_ex_data(ctx, us_ctx_user_ca_ex_idx)) { /* Default context: give this socket the process-shared root bundle. * A context whose store holds user-provided CAs (ca/caFile options or @@ -1175,6 +1176,16 @@ void us_internal_ssl_attach(struct us_socket_t *s, SSL_CTX *ctx, X509_STORE *roots = us_get_shared_default_ca_store(); if (roots) SSL_set0_verify_cert_store(ssl, roots); } + } else if (us_has_user_root_certs() + && !SSL_CTX_get_ex_data(ctx, us_ctx_user_ca_ex_idx)) { + /* CTX was built against the process defaults (request_cert without an + * explicit `ca`), but tls.setDefaultCACertificates() has since replaced + * those defaults. Override the verify store for this SSL only so the + * new roots take effect without rebuilding the cached SSL_CTX + * (fetch()/https.request() cache their HTTPS context for the process + * lifetime). CTXs with user CAs are left alone. */ + X509_STORE *roots = us_get_shared_default_ca_store(); + if (roots) SSL_set0_verify_cert_store(ssl, roots); } } else { SSL_set_accept_state(ssl); diff --git a/packages/bun-usockets/src/crypto/root_certs.cpp b/packages/bun-usockets/src/crypto/root_certs.cpp index 4257ddeee0dc..95a3e6b9b6b3 100644 --- a/packages/bun-usockets/src/crypto/root_certs.cpp +++ b/packages/bun-usockets/src/crypto/root_certs.cpp @@ -1,6 +1,7 @@ #include "./root_certs.h" #include "./root_certs_header.h" #include "./internal/internal.h" +#include #include #include #include "./default_ciphers.h" @@ -187,6 +188,67 @@ STACK_OF(X509) *us_get_root_extra_cert_instances() { return us_get_default_ca_certificates()->root_extra_cert_instances; } +// --------------------------------------------------------------------------- +// User-overridden default CA certificates (tls.setDefaultCACertificates) +// +// Node.js lets JS replace the default trust root set at runtime. Once set, +// *every* consumer of the "default" store — us_get_default_ca_store() and +// us_get_shared_default_ca_store() — must ignore the bundled/system/extra +// sources and build the store purely from this user-supplied set. The +// override is process-global here (Node.js uses thread_local so Workers are +// isolated; we accept the simpler process-wide semantics for now) and guarded +// by shared_store_mutex because the shared store can be read from socket +// I/O paths on other threads while JS swaps it. +// --------------------------------------------------------------------------- +static std::mutex shared_store_mutex; +static X509_STORE *shared_store = nullptr; +static STACK_OF(X509) *user_root_certs = nullptr; +// Atomic so the lock-free fast-path check in us_has_user_root_certs() +// (called per-SSL from us_internal_ssl_attach) is well-defined when a +// Worker is concurrently setting the override. Relaxed is enough: a +// stale false falls through to the shared-store path (which also honours +// the override under the mutex), a stale true just costs one extra +// SSL_set0_verify_cert_store. +static std::atomic has_user_root_certs { false }; + +extern "C" int us_has_user_root_certs() { + return has_user_root_certs.load(std::memory_order_relaxed) ? 1 : 0; +} + +extern "C" void us_set_user_root_certs(STACK_OF(X509) *certs) { + std::lock_guard lock(shared_store_mutex); + if (user_root_certs) { + sk_X509_pop_free(user_root_certs, X509_free); + } + user_root_certs = certs; // may be nullptr for an explicit empty set + has_user_root_certs = true; + + // Drop the cached shared store so the next consumer rebuilds from the + // override. Existing SSL*s already hold their own X509_STORE reference via + // SSL_set0_verify_cert_store, so this only affects new connections. + if (shared_store) { + X509_STORE_free(shared_store); + shared_store = nullptr; + } +} + +STACK_OF(X509) *us_dup_user_root_certs(bool *out_has_override) { + // Hand back an owned, up-ref'd snapshot so the caller can serialise the + // certs to PEM without racing us_set_user_root_certs() on another Worker. + // *out_has_override distinguishes "no override installed" from "empty + // override installed" (both return nullptr). Caller frees the returned + // stack via sk_X509_pop_free(.., X509_free). + std::lock_guard lock(shared_store_mutex); + if (out_has_override) *out_has_override = has_user_root_certs; + if (user_root_certs == nullptr) return nullptr; + STACK_OF(X509) *dup = sk_X509_dup(user_root_certs); + if (dup == nullptr) return nullptr; + for (size_t i = 0; i < sk_X509_num(dup); i++) { + X509_up_ref(sk_X509_value(dup, i)); + } + return dup; +} + // Single source of truth for the OS trust store. Loaded on first demand, // independent of --use-system-ca / NODE_USE_SYSTEM_CA, so that // tls.getCACertificates('system') matches Node.js (which always reads the @@ -207,12 +269,28 @@ STACK_OF(X509) *us_get_root_system_cert_instances() { return system_certs; } -extern "C" X509_STORE *us_get_default_ca_store() { +static X509_STORE *us_build_default_ca_store_locked() { X509_STORE *store = X509_STORE_new(); if (store == NULL) { return NULL; } + // If JS overrode the defaults via tls.setDefaultCACertificates(), honour + // that exclusively — Node.js does not merge bundled/system/extra back in. + // X509_STORE_add_cert() takes its own reference, so no up_ref here — + // unlike the bundled/extra/system blocks below (whose certs are + // process-lifetime statics so the extra ref is harmless), user_root_certs + // is freed on every subsequent setDefaultCACertificates() and an extra + // ref would leak. + if (has_user_root_certs) { + if (user_root_certs) { + for (int i = 0; i < (int)sk_X509_num(user_root_certs); i++) { + X509_STORE_add_cert(store, sk_X509_value(user_root_certs, i)); + } + } + return store; + } + if (!X509_STORE_set_default_paths(store)) { X509_STORE_free(store); return NULL; @@ -253,18 +331,30 @@ extern "C" X509_STORE *us_get_default_ca_store() { return store; } -// Process-wide immutable default store. Safe to share across SSL_CTXs that -// don't add per-config CAs (the user-`ca` path in build_raw populates the -// SSL_CTX's own private, initially-empty store instead). This makes the -// ~150-root build a once-per-process cost instead of once-per-SSL_CTX, which -// is what kept Bun.connect({tls:true}) under the node-tls-server.test.ts -// 100ms cold-path budget in debug+ASAN. +extern "C" X509_STORE *us_get_default_ca_store() { + // Serialise with us_set_user_root_certs() so a Worker swapping the + // override can't race another Worker building a per-config store. + std::lock_guard lock(shared_store_mutex); + return us_build_default_ca_store_locked(); +} + +// Process-wide default store cached behind a mutex. Safe to share across +// SSL_CTXs that don't add per-config CAs (the user-`ca` path in build_raw +// populates the SSL_CTX's own private, initially-empty store instead). This +// makes the ~150-root build a once-per-process cost instead of +// once-per-SSL_CTX, which is what kept Bun.connect({tls:true}) under the +// node-tls-server.test.ts 100ms cold-path budget in debug+ASAN. +// +// Not std::call_once: tls.setDefaultCACertificates() must be able to +// invalidate the cached store so subsequent connections see the override. +// us_set_user_root_certs() takes the same mutex and nulls shared_store. extern "C" X509_STORE *us_get_shared_default_ca_store() { - static X509_STORE *shared = nullptr; - static std::once_flag once; - std::call_once(once, []() { shared = us_get_default_ca_store(); }); - if (shared) X509_STORE_up_ref(shared); - return shared; + std::lock_guard lock(shared_store_mutex); + if (shared_store == nullptr) { + shared_store = us_build_default_ca_store_locked(); + } + if (shared_store) X509_STORE_up_ref(shared_store); + return shared_store; } extern "C" const char *us_get_default_ciphers() { diff --git a/packages/bun-usockets/src/crypto/root_certs_header.h b/packages/bun-usockets/src/crypto/root_certs_header.h index 31b59acc9848..34a2e53a20e2 100644 --- a/packages/bun-usockets/src/crypto/root_certs_header.h +++ b/packages/bun-usockets/src/crypto/root_certs_header.h @@ -6,6 +6,7 @@ STACK_OF(X509) *us_get_root_extra_cert_instances(); STACK_OF(X509) *us_get_root_system_cert_instances(); +STACK_OF(X509) *us_dup_user_root_certs(bool *out_has_override); #else #define CPPDECL extern @@ -13,3 +14,5 @@ STACK_OF(X509) *us_get_root_system_cert_instances(); CPPDECL X509_STORE *us_get_default_ca_store(); CPPDECL X509_STORE *us_get_shared_default_ca_store(); +CPPDECL void us_set_user_root_certs(STACK_OF(X509) *certs); +CPPDECL int us_has_user_root_certs(); diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 7cbf500677c3..851803427d37 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -19,6 +19,8 @@ const { Server: NetServer, Socket: NetSocket } = net; const getBundledRootCertificates = $newCppFunction("NodeTLS.cpp", "getBundledRootCertificates", 1); const getExtraCACertificates = $newCppFunction("NodeTLS.cpp", "getExtraCACertificates", 1); const getSystemCACertificates = $newCppFunction("NodeTLS.cpp", "getSystemCACertificates", 1); +const resetRootCertStore = $newCppFunction("NodeTLS.cpp", "resetRootCertStore", 1); +const getUserRootCertificates = $newCppFunction("NodeTLS.cpp", "getUserRootCertificates", 0); const canonicalizeIP = $newCppFunction("NodeTLS.cpp", "Bun__canonicalizeIP", 1); const getTLSDefaultCiphers = $newCppFunction("NodeTLS.cpp", "getDefaultCiphers", 0); @@ -758,14 +760,6 @@ var InternalSecureContext = class SecureContext { servername; constructor(options, cached = true) { - // When tls.setDefaultCACertificates() has installed an override and no - // explicit `ca` was given, use the override as the default CA set so the - // process-wide default applies on every construction path (the public - // createSecureContext(), the connect/TLSSocket path, addContext and - // setSecureContext), matching Node's secure-context default. - if (_defaultCACertificatesOverride !== undefined && (options == null || options.ca == null)) { - options = { ...options, ca: _defaultCACertificatesOverride }; - } if (options) { validateSecureContextOptions(options); if (options.cert) throwOnInvalidTLSArray("options.cert", options.cert); @@ -806,8 +800,6 @@ function SecureContext(options): void { function createSecureContext(options) { if (options instanceof InternalSecureContext) return options; - // The setDefaultCACertificates() override is applied inside the - // InternalSecureContext constructor so every construction path honors it. // The native handle (SSL_CTX) is memoised inside `NativeSecureContext.intern` // by the per-VM `SSLContextCache`, so no JS-side hashing here. The JS wrapper // is built fresh because it carries the per-call `servername`. @@ -1320,15 +1312,6 @@ function Server(options, secureConnectionListener): void { } let ca = options.ca; - // The process-wide default-CA override (tls.setDefaultCACertificates) - // applies here too when no explicit `ca` was given: this path hands raw - // {key, cert, ca} to the native listener and never goes through - // InternalSecureContext, so without this an mTLS server would verify - // client certificates against the bundled roots instead of the - // overridden defaults. - if (_defaultCACertificatesOverride !== undefined && ca == null) { - ca = _defaultCACertificatesOverride; - } // PKCS#12-embedded CAs are stashed separately so createSecureContext can // extend (not replace) the default trust set via addCACert. The server // path hands raw {key, cert, ca} to the native listener and has no @@ -1597,6 +1580,17 @@ const getUseSystemCA = $newZigFunction("bun.zig", "getUseSystemCA", 0); let defaultCACertificates: string[] | undefined; function cacheDefaultCACertificates() { + // The override is process-global (see root_certs.cpp), so ask native + // first: another Worker may have installed it and this VM's module + // state wouldn't know. undefined means "no override" — fall through to + // the (per-VM-cached) bundled/system/extra merge. A frozen array (maybe + // empty) means "override installed" — return it uncached so subsequent + // setDefaultCACertificates() calls from any Worker are reflected. + const override = getUserRootCertificates() as string[] | undefined; + if (override !== undefined) { + return override; + } + if (defaultCACertificates) return defaultCACertificates; defaultCACertificates = []; @@ -1662,96 +1656,59 @@ function maybeWarnAboutExtraCACerts() { } } -// Runtime override for the "default" CA certificate set, installed by -// tls.setDefaultCACertificates(). undefined = no override (use the real -// bundled/system default). Only affects type "default"/implicit — "bundled", -// "system" and "extra" are unchanged. -// https://github.com/nodejs/node/blob/main/lib/internal/tls/secure-context.js -let _defaultCACertificatesOverride: Array | undefined; - -type CACertInput = string | NodeJS.ArrayBufferView; -interface X509CertificateLike { - readonly fingerprint256: string; - toString(): string; +function getCACertificates(type = "default") { + validateString(type, "type"); + + switch (type) { + case "default": + return cacheDefaultCACertificates(); + case "bundled": + return cacheBundledRootCertificates(); + case "system": + return cacheSystemCACertificates(); + case "extra": + return cacheExtraCACertificates(); + default: + throw $ERR_INVALID_ARG_VALUE("type", type); + } } -type X509CertificateCtor = new (cert: CACertInput) => X509CertificateLike; -let _X509CertificateClass: X509CertificateCtor | undefined; // tls.setDefaultCACertificates(certs) // https://github.com/nodejs/node/blob/v25.2.1/lib/tls.js#L202 -// Node validates `certs` as an Array (its ERR_INVALID_ARG_TYPE renders the -// 'Array' name as "an instance of Array"; Bun's validateArray renders the same -// name as "of type Array", so build the error directly to match Node here), -// then hands the certs to the native root store. Bun has no equivalent native -// store override, so keep a JS-side override that getCACertificates('default') -// and createSecureContext() read. -function setDefaultCACertificates(certs: ReadonlyArray): void { - if (!$isArray(certs)) { +// +// Validation mirrors Node, then the certificate set is handed to the native +// default-CA store (root_certs.cpp) so every consumer — createSecureContext, +// fetch(), https.request()'s cached HTTPS SSL_CTX, Bun.connect — sees it, +// including connections on SSL_CTXs built before the override was installed +// (us_internal_ssl_attach refreshes the verify store per-SSL). +function setDefaultCACertificates(certs) { + if (!$isJSArray(certs)) { + // Node's ERR_INVALID_ARG_TYPE renders a capitalised type name as + // "an instance of X" whereas Bun's $ERR_INVALID_ARG_TYPE always says + // "of type X", so build the message directly to match Node's wording + // (parallel/test-tls-set-default-ca-certificates-error.js asserts it). let received: string; if (certs === null) received = "null"; + else if (certs === undefined) received = "undefined"; else if (typeof certs === "object") received = `an instance of ${(certs as object).constructor?.name ?? "Object"}`; - else if (typeof certs === "string") received = `type string ('${certs}')`; - else received = `type ${typeof certs} (${String(certs)})`; + else received = `type ${typeof certs} (${typeof certs === "string" ? `'${certs}'` : String(certs)})`; const error = new TypeError(`The "certs" argument must be an instance of Array. Received ${received}`) as Error & { code: string; }; error.code = "ERR_INVALID_ARG_TYPE"; throw error; } - _X509CertificateClass ??= require("node:crypto").X509Certificate as X509CertificateCtor; - // Parse each cert and de-duplicate by fingerprint so getCACertificates() - // returns a normalized, unique PEM set (matching Node, whose native store - // collapses duplicates). Build into a temp array and only commit on success, - // so an invalid element leaves the previous default untouched. - const seen = new Set(); - const normalized: Array = []; for (let i = 0; i < certs.length; i++) { - const cert = certs[i]; - if (typeof cert !== "string" && !isArrayBufferView(cert)) { - throw $ERR_INVALID_ARG_TYPE(`certs[${i}]`, "string or an instance of ArrayBufferView", cert); - } - // An element may be a concatenated PEM bundle; Node adds every certificate - // it contains, so split on certificate boundaries before parsing (a single - // X509Certificate parse only consumes the first block). - const text = - typeof cert === "string" ? cert : Buffer.from(cert.buffer, cert.byteOffset, cert.byteLength).toString("latin1"); - const blocks = text.includes("-----BEGIN") - ? // Keep only the blocks that actually start a PEM certificate: bundle - // files routinely begin with comment headers (curl's cacert.pem, - // RHEL's ca-bundle.crt) that the lookahead split leaves as a leading - // non-PEM element. - text.split(/(?=-----BEGIN [A-Z0-9 ]*CERTIFICATE-----)/).filter(block => block.includes("CERTIFICATE-----")) - : [cert]; - for (const block of blocks) { - const x509 = new _X509CertificateClass(block as CACertInput); - const fingerprint = x509.fingerprint256; - if (!seen.has(fingerprint)) { - seen.add(fingerprint); - normalized.push(x509.toString()); - } + if (typeof certs[i] !== "string" && !isArrayBufferView(certs[i])) { + throw $ERR_INVALID_ARG_TYPE(`certs[${i}]`, "string or an instance of ArrayBufferView", certs[i]); } } - _defaultCACertificatesOverride = normalized; -} - -function getCACertificates(type = "default") { - validateString(type, "type"); - switch (type) { - case "default": - if (_defaultCACertificatesOverride !== undefined) { - return _defaultCACertificatesOverride.slice(); - } - return cacheDefaultCACertificates(); - case "bundled": - return cacheBundledRootCertificates(); - case "system": - return cacheSystemCACertificates(); - case "extra": - return cacheExtraCACertificates(); - default: - throw $ERR_INVALID_ARG_VALUE("type", type); - } + resetRootCertStore(certs); + // Drop any cached bundled+system+extra merge in THIS VM; other Workers' + // caches are bypassed on their next query because getUserRootCertificates() + // now returns the override (see cacheDefaultCACertificates). + defaultCACertificates = undefined; } function tlsCipherFilter(a: string) { @@ -1800,7 +1757,6 @@ export default { DEFAULT_MIN_VERSION = value; }, getCiphers, - setDefaultCACertificates, parseCertString, SecureContext, Server, @@ -1810,4 +1766,5 @@ export default { return cacheBundledRootCertificates(); }, getCACertificates, + setDefaultCACertificates, } as any as typeof import("node:tls"); diff --git a/src/jsc/bindings/NodeTLS.cpp b/src/jsc/bindings/NodeTLS.cpp index 218c78cd9939..f4c6162f4972 100644 --- a/src/jsc/bindings/NodeTLS.cpp +++ b/src/jsc/bindings/NodeTLS.cpp @@ -3,15 +3,20 @@ #include "JavaScriptCore/JSObject.h" #include "JavaScriptCore/ObjectConstructor.h" #include "JavaScriptCore/ArrayConstructor.h" +#include "JavaScriptCore/JSArrayBufferView.h" #include "libusockets.h" #include "ZigGlobalObject.h" #include "ErrorCode.h" #include "openssl/base.h" #include "openssl/bio.h" +#include "openssl/err.h" #include "openssl/x509.h" #include "../../packages/bun-usockets/src/crypto/root_certs_header.h" +#include +#include + namespace Bun { using namespace JSC; @@ -131,6 +136,218 @@ JSC_DEFINE_HOST_FUNCTION(getSystemCACertificates, (JSC::JSGlobalObject * globalO RELEASE_AND_RETURN(scope, JSValue::encode(JSC::objectConstructorFreeze(globalObject, rootCertificates))); } +// Compare by DER encoding so that a cert supplied twice (even via different +// PEM formatting) is deduplicated — matches Node.js's X509Set which hashes +// X509_cmp-equivalent identity. +struct X509DerLess { + bool operator()(X509* a, X509* b) const + { + return X509_cmp(a, b) < 0; + } +}; + +// Read every PEM certificate contained in `data` (a single entry may hold a +// bundle). On parse failure, frees anything this call pushed and returns the +// peeked OpenSSL error so the caller can format it. +static unsigned long appendX509sFromPEM(std::span data, STACK_OF(X509) * out) +{ + ERR_clear_error(); + // BoringSSL takes ossl_ssize_t (= ptrdiff_t) here; an int cast would + // truncate a >2GB input and make BoringSSL treat it as NUL-terminated. + // Guard the upper bound explicitly so a pathological ArrayBufferView + // byteLength can't wrap to a small positive length. + if (data.size() > static_cast(std::numeric_limits::max())) { + OPENSSL_PUT_ERROR(PEM, PEM_R_BAD_END_LINE); + return ERR_peek_last_error(); + } + BIO* bio = BIO_new_mem_buf(data.data(), static_cast(data.size())); + if (bio == nullptr) { + return ERR_peek_last_error(); + } + + size_t pushed = 0; + while (X509* x = PEM_read_bio_X509(bio, nullptr, [](char*, int, int, void*) -> int { return 0; }, nullptr)) { + if (!sk_X509_push(out, x)) { + X509_free(x); + BIO_free(bio); + while (pushed-- > 0) + X509_free(sk_X509_pop(out)); + OPENSSL_PUT_ERROR(PEM, ERR_R_MALLOC_FAILURE); + return ERR_peek_last_error(); + } + pushed++; + } + BIO_free(bio); + + unsigned long last = ERR_peek_last_error(); + // PEM_R_NO_START_LINE after at least one successful read just means EOF. + if (pushed > 0 && ERR_GET_LIB(last) == ERR_LIB_PEM && ERR_GET_REASON(last) == PEM_R_NO_START_LINE) { + ERR_clear_error(); + return 0; + } + if (last != 0) { + // Roll back everything this call pushed so a mid-array failure leaves + // the process-wide store untouched. + while (pushed-- > 0) { + X509_free(sk_X509_pop(out)); + } + } + return last; +} + +JSC_DEFINE_HOST_FUNCTION(resetRootCertStore, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + + JSValue arg = callFrame->argument(0); + JSArray* array = arg.isCell() ? dynamicDowncast(arg.asCell()) : nullptr; + // The JS wrapper validated this already; be defensive for direct callers. + if (!array) [[unlikely]] { + return throwVMTypeError(globalObject, scope, "Expected an array of certificates"_s); + } + + unsigned length = array->length(); + if (length == 0) { + // Explicit empty trust set — subsequent default-CA connections will + // fail verification. Matches Node.js, which distinguishes "empty + // override" from "no override". + us_set_user_root_certs(nullptr); + return JSValue::encode(jsUndefined()); + } + + STACK_OF(X509)* parsed = sk_X509_new_null(); + if (parsed == nullptr) { + throwOutOfMemoryError(globalObject, scope); + return {}; + } + + auto freeParsed = [&]() { sk_X509_pop_free(parsed, X509_free); }; + + for (unsigned i = 0; i < length; i++) { + JSValue element = array->getIndex(globalObject, i); + if (scope.exception()) [[unlikely]] { + freeParsed(); + return {}; + } + + unsigned long err = 0; + if (element.isString()) { + auto str = element.toWTFString(globalObject); + if (scope.exception()) [[unlikely]] { + freeParsed(); + return {}; + } + auto utf8 = str.utf8(); + err = appendX509sFromPEM({ reinterpret_cast(utf8.data()), utf8.length() }, parsed); + } else if (auto* view = element.isCell() ? dynamicDowncast(element.asCell()) : nullptr) { + err = appendX509sFromPEM(view->span(), parsed); + } else { + // JS validated element types; treat anything else as a failure. + freeParsed(); + return throwError(globalObject, scope, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "Failed to load certificate data"_str); + } + + if (err != 0) { + freeParsed(); + char buf[256] = { 0 }; + ERR_error_string_n(err, buf, sizeof(buf)); + ERR_clear_error(); + auto message = makeString("Failed to parse certificate: "_s, String::fromUTF8(buf)); + return throwError(globalObject, scope, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, message); + } + } + + if (sk_X509_num(parsed) == 0) { + freeParsed(); + return throwError(globalObject, scope, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "No valid certificates found in the provided array"_str); + } + + // Deduplicate by X509 identity so getCACertificates('default') mirrors + // Node.js's X509Set semantics (and so the store isn't bloated). + std::set seen; + STACK_OF(X509)* deduped = sk_X509_new_null(); + if (deduped == nullptr) { + freeParsed(); + throwOutOfMemoryError(globalObject, scope); + return {}; + } + for (int i = 0; i < (int)sk_X509_num(parsed); i++) { + X509* cert = sk_X509_value(parsed, i); + if (seen.insert(cert).second) { + X509_up_ref(cert); + if (!sk_X509_push(deduped, cert)) { + X509_free(cert); // drop the up_ref we just took + sk_X509_pop_free(deduped, X509_free); + freeParsed(); + throwOutOfMemoryError(globalObject, scope); + return {}; + } + } + } + freeParsed(); + + us_set_user_root_certs(deduped); + return JSValue::encode(jsUndefined()); +} + +JSC_DEFINE_HOST_FUNCTION(getUserRootCertificates, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + VM& vm = globalObject->vm(); + auto scope = DECLARE_THROW_SCOPE(vm); + + // Snapshot under the root-cert mutex so another Worker calling + // setDefaultCACertificates() can't free certs out from under us while + // we're writing PEM. hasOverride distinguishes "no override installed" + // (return undefined so the JS side falls back to bundled/system/extra) + // from "empty override installed" (return a frozen empty array). + bool hasOverride = false; + STACK_OF(X509)* certs = us_dup_user_root_certs(&hasOverride); + if (!hasOverride) { + return JSValue::encode(jsUndefined()); + } + auto freeCerts = [&]() { + if (certs) sk_X509_pop_free(certs, X509_free); + }; + auto size = certs ? sk_X509_num(certs) : 0; + + JSC::MarkedArgumentBuffer args; + for (size_t i = 0; i < size; i++) { + BIO* bio = BIO_new(BIO_s_mem()); + if (!bio) { + freeCerts(); + throwOutOfMemoryError(globalObject, scope); + return {}; + } + if (PEM_write_bio_X509(bio, sk_X509_value(certs, i)) != 1) { + BIO_free(bio); + freeCerts(); + return throwError(globalObject, scope, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "X509 to PEM conversion"_str); + } + char* bioData = nullptr; + long bioLen = BIO_get_mem_data(bio, &bioData); + if (bioLen <= 0 || !bioData) { + BIO_free(bio); + freeCerts(); + return throwError(globalObject, scope, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "Reading PEM data"_str); + } + auto str = WTF::String::fromUTF8(std::span { bioData, static_cast(bioLen) }); + args.append(JSC::jsString(vm, str)); + BIO_free(bio); + } + freeCerts(); + + if (args.hasOverflowed()) { + throwOutOfMemoryError(globalObject, scope); + return {}; + } + + auto result = JSC::constructArray(globalObject, static_cast(nullptr), args); + RETURN_IF_EXCEPTION(scope, {}); + + RELEASE_AND_RETURN(scope, JSValue::encode(JSC::objectConstructorFreeze(globalObject, result))); +} + extern "C" JSC::EncodedJSValue Bun__getTLSDefaultCiphers(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame); extern "C" JSC::EncodedJSValue Bun__setTLSDefaultCiphers(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callFrame); diff --git a/src/jsc/bindings/NodeTLS.h b/src/jsc/bindings/NodeTLS.h index c8948b6bf968..cbc552507951 100644 --- a/src/jsc/bindings/NodeTLS.h +++ b/src/jsc/bindings/NodeTLS.h @@ -7,6 +7,8 @@ BUN_DECLARE_HOST_FUNCTION(Bun__canonicalizeIP); JSC_DECLARE_HOST_FUNCTION(getBundledRootCertificates); JSC_DECLARE_HOST_FUNCTION(getExtraCACertificates); JSC_DECLARE_HOST_FUNCTION(getSystemCACertificates); +JSC_DECLARE_HOST_FUNCTION(resetRootCertStore); +JSC_DECLARE_HOST_FUNCTION(getUserRootCertificates); JSC_DECLARE_HOST_FUNCTION(getDefaultCiphers); JSC_DECLARE_HOST_FUNCTION(setDefaultCiphers); diff --git a/test/js/node/test/common/tls.js b/test/js/node/test/common/tls.js index 8568659d13cc..d68e836cb7ae 100644 --- a/test/js/node/test/common/tls.js +++ b/test/js/node/test/common/tls.js @@ -197,12 +197,12 @@ function extractMetadata(cert) { exports.extractMetadata = extractMetadata; // To compare two certificates, we can just compare serialNumber, issuer, -// and subject like X509_comp(). We can't just compare two strings because +// and subject like X509_cmp(). We can't just compare two strings because // the line endings or order of the fields may differ after PEM serdes by // OpenSSL. exports.assertEqualCerts = function assertEqualCerts(a, b) { - const setA = new Set(a.map(extractMetadata)); - const setB = new Set(b.map(extractMetadata)); + const setA = new Set(a.map(extractMetadata).map(JSON.stringify)); + const setB = new Set(b.map(extractMetadata).map(JSON.stringify)); assert.deepStrictEqual(setA, setB); }; diff --git a/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts b/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts new file mode 100644 index 000000000000..6e46a0f0be39 --- /dev/null +++ b/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts @@ -0,0 +1,294 @@ +// node:tls — tls.setDefaultCACertificates(certs) +// https://github.com/oven-sh/bun/issues/24340 +// +// Each scenario runs in a fresh subprocess because the override is +// process-global: once set, the bundled defaults cannot be restored without +// calling the function again, and we don't want one test case's trust store +// bleeding into the next. + +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; +import path from "node:path"; +import tls from "node:tls"; + +const keysDir = path.join(import.meta.dir, "..", "test", "fixtures", "keys"); +const fakeRootCert = path.join(keysDir, "fake-startcom-root-cert.pem"); +const agent8Cert = path.join(keysDir, "agent8-cert.pem"); +const agent8Key = path.join(keysDir, "agent8-key.pem"); + +async function run(src: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +describe.concurrent("tls.setDefaultCACertificates", () => { + test("is a function", () => { + expect(typeof tls.setDefaultCACertificates).toBe("function"); + }); + + test("rejects non-array input with ERR_INVALID_ARG_TYPE", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const assert = require("node:assert"); + for (const bad of [null, undefined, "string", 42, {}, true]) { + assert.throws(() => tls.setDefaultCACertificates(bad), { + code: "ERR_INVALID_ARG_TYPE", + message: /"certs".*Array/, + }); + } + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("rejects invalid array elements with ERR_INVALID_ARG_TYPE", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const assert = require("node:assert"); + const fs = require("node:fs"); + const cert = fs.readFileSync(${JSON.stringify(fakeRootCert)}, "utf8"); + for (const bad of [null, undefined, 42, {}, true]) { + assert.throws(() => tls.setDefaultCACertificates([bad]), { + code: "ERR_INVALID_ARG_TYPE", + message: /"certs\\[0\\]".*string.*ArrayBufferView/, + }); + assert.throws(() => tls.setDefaultCACertificates([cert, bad]), { + code: "ERR_INVALID_ARG_TYPE", + message: /"certs\\[1\\]".*string.*ArrayBufferView/, + }); + } + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("replaces the default CA set and getCACertificates('default') reflects it", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const fs = require("node:fs"); + const assert = require("node:assert"); + const { X509Certificate } = require("node:crypto"); + const pem = fs.readFileSync(${JSON.stringify(fakeRootCert)}, "utf8"); + + const bundledBefore = tls.getCACertificates("bundled"); + + tls.setDefaultCACertificates([pem]); + + const defaults = tls.getCACertificates("default"); + assert.strictEqual(defaults.length, 1); + // Compare by identity (serial/issuer/subject) rather than raw PEM text; + // OpenSSL may normalise line endings or trailing whitespace on round-trip. + const a = new X509Certificate(defaults[0]); + const b = new X509Certificate(pem); + assert.strictEqual(a.serialNumber, b.serialNumber); + assert.strictEqual(a.issuer, b.issuer); + assert.strictEqual(a.subject, b.subject); + + // Implicit default matches too. + assert.strictEqual(tls.getCACertificates().length, 1); + + // 'bundled' must be untouched — it's the compiled-in Mozilla set. + const bundledAfter = tls.getCACertificates("bundled"); + assert.strictEqual(bundledAfter.length, bundledBefore.length); + + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("accepts an empty array", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const assert = require("node:assert"); + tls.setDefaultCACertificates([]); + const defaults = tls.getCACertificates("default"); + assert(Array.isArray(defaults)); + assert.strictEqual(defaults.length, 0); + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("deduplicates repeated certificates", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const fs = require("node:fs"); + const assert = require("node:assert"); + const pem = fs.readFileSync(${JSON.stringify(fakeRootCert)}, "utf8"); + tls.setDefaultCACertificates([pem, pem, pem]); + assert.strictEqual(tls.getCACertificates("default").length, 1); + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("accepts Buffer, Uint8Array and DataView entries", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const fs = require("node:fs"); + const assert = require("node:assert"); + const pem = fs.readFileSync(${JSON.stringify(fakeRootCert)}, "utf8"); + + tls.setDefaultCACertificates([Buffer.from(pem)]); + assert.strictEqual(tls.getCACertificates("default").length, 1); + + tls.setDefaultCACertificates([]); + assert.strictEqual(tls.getCACertificates("default").length, 0); + + const u8 = new TextEncoder().encode(pem); + tls.setDefaultCACertificates([u8]); + assert.strictEqual(tls.getCACertificates("default").length, 1); + + tls.setDefaultCACertificates([]); + const dv = new DataView(u8.buffer, u8.byteOffset, u8.byteLength); + tls.setDefaultCACertificates([dv]); + assert.strictEqual(tls.getCACertificates("default").length, 1); + + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("throws on unparseable PEM and leaves defaults unchanged", async () => { + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const assert = require("node:assert"); + const before = tls.getCACertificates("default"); + assert.throws(() => tls.setDefaultCACertificates(["not a certificate"]), { + code: "ERR_CRYPTO_OPERATION_FAILED", + }); + const after = tls.getCACertificates("default"); + // The JS-side cache is only invalidated after the native store swap + // succeeds, so 'after' must be the same frozen array instance. + assert.strictEqual(after, before); + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("override set on the main thread is visible from a Worker", async () => { + // The override is process-global in Bun (Node.js scopes it per-thread). + // getCACertificates('default') must agree with what the TLS layer will + // actually verify against, so a Worker that never called the setter + // must still report the main thread's override — not the bundled set. + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const fs = require("node:fs"); + const { Worker } = require("node:worker_threads"); + const pem = fs.readFileSync(${JSON.stringify(fakeRootCert)}, "utf8"); + tls.setDefaultCACertificates([pem]); + const w = new Worker( + 'const tls = require("node:tls");' + + 'const { parentPort } = require("node:worker_threads");' + + 'parentPort.postMessage(tls.getCACertificates("default").length);', + { eval: true }, + ); + w.on("message", n => { + console.log("worker-default-count:" + n); + w.terminate(); + }); + w.on("error", e => { console.log("worker-error:" + e.message); }); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("worker-default-count:1"); + expect(exitCode).toBe(0); + }); + + test("overrides the trust store used for new TLS connections", async () => { + // agent8-cert.pem is signed by fake-startcom-root-cert.pem. + // + // 1. Connect BEFORE installing the root — must fail. This forces the + // HTTPS client's long-lived SSL_CTX (built once on the HTTP thread) + // to be created with the bundled store, so step 2 proves that + // setDefaultCACertificates() takes effect even for a cached CTX. + // 2. Install the root and connect again — must succeed. + // 3. Clear the roots and connect again — must fail. + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const https = require("node:https"); + const fs = require("node:fs"); + const assert = require("node:assert"); + + const ca = fs.readFileSync(${JSON.stringify(fakeRootCert)}, "utf8"); + + const server = https.createServer({ + cert: fs.readFileSync(${JSON.stringify(agent8Cert)}), + key: fs.readFileSync(${JSON.stringify(agent8Key)}), + }, (req, res) => { + res.writeHead(200); + res.end("hello"); + }); + + function request(opts) { + return new Promise((resolve, reject) => { + const req = https.request(opts, res => { + let data = ""; + res.on("data", c => data += c); + res.on("end", () => resolve({ status: res.statusCode, data })); + }); + req.on("error", reject); + req.end(); + }); + } + + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const port = server.address().port; + + // 1. No custom root yet — verification fails. + try { + await request({ hostname: "localhost", port, path: "/", method: "GET" }); + throw new Error("step1: connection unexpectedly succeeded without CA"); + } catch (err) { + assert(err.code && err.code !== "ERR_ASSERTION", "step1: " + err.message); + } + + // 2. Install the signing root — verification succeeds. + tls.setDefaultCACertificates([ca]); + const ok = await request({ hostname: "localhost", port, path: "/", method: "GET" }); + assert.strictEqual(ok.status, 200); + assert.strictEqual(ok.data, "hello"); + + // 3. Clear the roots — verification fails again. New Agent to avoid + // any keep-alive/session reuse masking the effect. + tls.setDefaultCACertificates([]); + try { + await request({ + hostname: "localhost", + port, + path: "/", + method: "GET", + agent: new https.Agent(), + }); + throw new Error("step3: connection unexpectedly succeeded with empty CA store"); + } catch (err) { + assert(err.code && err.code !== "ERR_ASSERTION", "step3: " + err.message); + } + + server.close(); + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/node/tls/ssl-ctx-cache.test.ts b/test/js/node/tls/ssl-ctx-cache.test.ts index 96d2684a05a1..78e03a7fc56d 100644 --- a/test/js/node/tls/ssl-ctx-cache.test.ts +++ b/test/js/node/tls/ssl-ctx-cache.test.ts @@ -238,16 +238,15 @@ test("setDefaultCACertificates() override applies to plain tls.connect (no expli } }); -test("ca: [] skips the setDefaultCACertificates override (distinct from ca: undefined)", async () => { - // Providing any `ca` value - including an empty array - bypasses the - // process-default override that setDefaultCACertificates() installs (the - // override only applies when `ca` is absent), so the connection verifies - // against the bundled roots instead. NOTE: this is not Node's full - // "ca: [] = empty trust store" semantics (an explicitly-empty list should - // trust NOTHING, not fall back to bundled roots) - that needs an explicit - // empty-CA flag through the native config and remains a follow-up. Make a - // fixture CA a process default first so the two cases are observably - // different. +test("explicit ca overrides the setDefaultCACertificates() default; ca: [] falls back to it", async () => { + // Providing a concrete `ca` bypasses the process default that + // setDefaultCACertificates() installs - the connection verifies only + // against the caller's set. An EMPTY `ca` array collapses to "no ca" at + // the native SSLConfig boundary (SSLConfig.rs normalises [] -> None), so + // it behaves like omitting `ca` and the process default applies. Node's + // full "ca: [] = empty trust store" semantics (an explicitly-empty list + // trusts NOTHING) needs an explicit empty-CA flag through the native + // config and remains a follow-up. const keys = (f: string) => readFileSync(join(import.meta.dir, "../test/fixtures/keys", f), "utf8"); const prevCerts = tls.getCACertificates("default"); tls.setDefaultCACertificates([keys("ca1-cert.pem")]); @@ -264,15 +263,28 @@ test("ca: [] skips the setDefaultCACertificates override (distinct from ca: unde c1.end(); await once(c1, "close"); - // ca: [] -> the override is skipped, the bundled roots apply (which do not - // include ca1) -> NOT authorized. + // ca: [] -> normalised to no-ca -> same process defaults -> authorized. const c2 = tls.connect({ port, host: "127.0.0.1", rejectUnauthorized: false, servername: "agent1", ca: [] }); await once(c2, "secureConnect"); - expect(c2.authorized).toBe(false); - expect(c2.authorizationError).toBeTruthy(); + expect(c2.authorized).toBe(true); c2.end(); await once(c2, "close"); + // ca: [ca2] -> explicit non-matching CA wins over the default -> NOT + // authorized (agent1's cert was signed by ca1, not ca2). + const c3 = tls.connect({ + port, + host: "127.0.0.1", + rejectUnauthorized: false, + servername: "agent1", + ca: [keys("ca2-cert.pem")], + }); + await once(c3, "secureConnect"); + expect(c3.authorized).toBe(false); + expect(c3.authorizationError).toBeTruthy(); + c3.end(); + await once(c3, "close"); + server.close(); await once(server, "close"); } finally { From cb396e9795d2460c5ccf90b19e4ebf768d567722 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 17 Jun 2026 11:40:20 +0000 Subject: [PATCH 2/2] addCACert: clone-on-write via user-CA flag, not shared-store pointer identity setDefaultCACertificates() invalidates and rebuilds the cached shared X509_STORE, so the old pointer-identity check in us_ssl_ctx_add_ca_cert (store == us_get_shared_default_ca_store()) no longer recognises a CTX holding the stale shared store, and would mutate it in place, leaking the added CA into every sibling CTX still referencing it. Gate the clone on !us_ctx_user_ca_ex_idx instead: that flag is set exactly when the CTX has a private store, so its absence means the store is shared-or-empty and must be cloned before appending. Also mirror the client-side per-SSL verify-store refresh on the server accept path so an mTLS server built before setDefaultCACertificates() verifies client certificates against the current defaults. --- packages/bun-usockets/src/crypto/openssl.c | 45 ++++--- ...de-tls-set-default-ca-certificates.test.ts | 122 ++++++++++++++++++ 2 files changed, 149 insertions(+), 18 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 17fd8afe1ae0..db91a78ff740 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -949,28 +949,26 @@ int us_ssl_ctx_add_ca_cert(SSL_CTX *ctx, const char *content) { if (!ctx || !content) { return 0; } + us_ex_idx_ensure(); X509_STORE *store = SSL_CTX_get_cert_store(ctx); /* Clone-on-write: a context that shares the process-wide default root * store must get its own copy before a CA is appended, or the addition * would be visible to every other context in the process - the same * root_cert_store check Node's SecureContext::AddCACert performs. - * us_get_shared_default_ca_store() up-refs before returning, so release - * the reference taken just for this comparison. */ - X509_STORE *shared = us_get_shared_default_ca_store(); - int store_is_shared = store && store == shared; - X509_STORE_free(shared); - /* A default context built without ca/requestCert keeps the empty store from - * SSL_CTX_new() (verification for it normally comes from the per-socket - * shared-root override). addCACert must EXTEND the default trust set the - * way Node does, so when the store is the shared one - or still empty - - * replace it with a fresh full default store (bundled roots, NODE_EXTRA_CA - * certificates, system CAs when enabled) before appending the user's CA. */ - int store_is_empty = 0; - if (store && !store_is_shared) { - const STACK_OF(X509_OBJECT) *objs = X509_STORE_get0_objects(store); - store_is_empty = objs == NULL || sk_X509_OBJECT_num(objs) == 0; - } - if (store_is_shared || store_is_empty) { + * + * us_ctx_user_ca_ex_idx is the contract here: it is set exactly when this + * SSL_CTX has a PRIVATE store (built from an explicit ca/caFile option or + * by an earlier addCACert on this context). When the flag is clear, the + * store is either the process-wide shared-default X509_STORE (attached by + * the request_cert-without-ca branch above) or the still-empty store from + * SSL_CTX_new(); both require swapping in a fresh private default store + * before appending. Do NOT compare `store` against + * us_get_shared_default_ca_store() by pointer identity: + * tls.setDefaultCACertificates() invalidates and rebuilds that cache, so a + * CTX built before the override would compare its stale shared pointer + * against the new one, fall through, and mutate a store other CTXs still + * share. */ + if (!SSL_CTX_get_ex_data(ctx, us_ctx_user_ca_ex_idx)) { X509_STORE *own = us_get_default_ca_store(); if (!own) { return 0; @@ -981,7 +979,6 @@ int us_ssl_ctx_add_ca_cert(SSL_CTX *ctx, const char *content) { if (!store) { return 0; } - us_ex_idx_ensure(); SSL_CTX_set_ex_data(ctx, us_ctx_user_ca_ex_idx, (void *)1); return add_ca_cert_to_ctx_store(ctx, content, store); } @@ -1193,6 +1190,18 @@ void us_internal_ssl_attach(struct us_socket_t *s, SSL_CTX *ctx, /* sni_cb recovers ls per-SSL — never via the shared SSL_CTX. */ us_ex_idx_ensure(); SSL_set_ex_data(ssl, us_ssl_listener_ex_idx, listener); + /* Same refresh as the client path: an mTLS server built with + * request_cert before tls.setDefaultCACertificates() was called still + * holds the stale shared store on its SSL_CTX; hand each accepted SSL + * the current process defaults so client-cert verification follows the + * override. A CTX that has its own CA set (explicit `ca` option or a + * later addCACert) is left alone. */ + if (us_has_user_root_certs() + && SSL_CTX_get_verify_mode(ctx) != SSL_VERIFY_NONE + && !SSL_CTX_get_ex_data(ctx, us_ctx_user_ca_ex_idx)) { + X509_STORE *roots = us_get_shared_default_ca_store(); + if (roots) SSL_set0_verify_cert_store(ssl, roots); + } } s->ssl = ssl; diff --git a/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts b/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts index 6e46a0f0be39..a1b681ef7ed7 100644 --- a/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts +++ b/test/js/node/tls/node-tls-set-default-ca-certificates.test.ts @@ -291,4 +291,126 @@ describe.concurrent("tls.setDefaultCACertificates", () => { expect(stdout.trim()).toBe("ok"); expect(exitCode).toBe(0); }); + + test("addCACert after setDefaultCACertificates clones from the current defaults", async () => { + // A SecureContext built with { requestCert: true, ca: undefined } holds + // the process-shared default X509_STORE directly (openssl.c's + // request_cert branch). setDefaultCACertificates() drops and rebuilds + // that cached store, so comparing the CTX's store against the *current* + // shared pointer no longer detects "this is the shared one". addCACert's + // clone-on-write is driven by the per-CTX user-CA flag instead: on a + // context that has never had its own CAs it builds a fresh private + // store from the CURRENT process defaults (here [ca1]) and appends to + // that, rather than mutating the stale shared store the CTX still holds + // (which would leak the added CA into every sibling context sharing it + // and leave the CTX verifying against a bundle that never contained + // ca1). + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const fs = require("node:fs"); + const { once } = require("node:events"); + const assert = require("node:assert"); + + const ca1 = fs.readFileSync(${JSON.stringify(path.join(keysDir, "ca1-cert.pem"))}, "utf8"); + const ca2 = fs.readFileSync(${JSON.stringify(path.join(keysDir, "ca2-cert.pem"))}, "utf8"); + const agent1Key = fs.readFileSync(${JSON.stringify(path.join(keysDir, "agent1-key.pem"))}); + const agent1Cert = fs.readFileSync(${JSON.stringify(path.join(keysDir, "agent1-cert.pem"))}); + + // requestCert with no explicit ca -> CTX store is the shared default + // (the bundled roots at this point). + const ctx = tls.createSecureContext({ requestCert: true }); + + // Invalidate the cached shared store; the current default is now [ca1]. + tls.setDefaultCACertificates([ca1]); + + // Must clone-on-write into a private store seeded from the *current* + // defaults ([ca1]) and then append ca2 -> [ca1, ca2]. A stale + // pointer-identity check against the new shared store would miss the + // clone and append to the old bundled-root store instead (no ca1). + ctx.context.addCACert(ca2); + + // Connect with this context to a server presenting agent1 (signed by + // ca1). addCACert marked the CTX with user CAs, so the per-SSL + // default-store overlay is skipped and verification runs against the + // CTX's own private store: it must contain ca1. + const server = tls.createServer({ key: agent1Key, cert: agent1Cert }, s => s.end()); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const port = server.address().port; + + const client = tls.connect({ + port, + host: "127.0.0.1", + servername: "agent1", + secureContext: ctx, + rejectUnauthorized: false, + }); + client.on("error", () => {}); + await once(client, "secureConnect"); + const authorized = client.authorized; + const err = client.authorizationError; + client.destroy(); + server.close(); + + assert.strictEqual(authorized, true, + "addCACert did not seed the private store from the current defaults: " + err); + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); + + test("an mTLS server built before setDefaultCACertificates honours the override", async () => { + // The server accept path gets the same per-SSL verify-store refresh as + // the client path: a requestCert server whose SSL_CTX was built before + // setDefaultCACertificates() (and so still points at the stale shared + // store) verifies each accepted connection against the CURRENT process + // defaults, not the bundled roots it was built with. + const { stdout, stderr, exitCode } = await run(` + const tls = require("node:tls"); + const fs = require("node:fs"); + const { once } = require("node:events"); + const assert = require("node:assert"); + + const ca1 = fs.readFileSync(${JSON.stringify(path.join(keysDir, "ca1-cert.pem"))}, "utf8"); + const agent1Key = fs.readFileSync(${JSON.stringify(path.join(keysDir, "agent1-key.pem"))}); + const agent1Cert = fs.readFileSync(${JSON.stringify(path.join(keysDir, "agent1-cert.pem"))}); + + const authorized = Promise.withResolvers(); + const server = tls.createServer({ + key: agent1Key, + cert: agent1Cert, + requestCert: true, + rejectUnauthorized: false, + }); + server.on("secureConnection", s => { authorized.resolve(s.authorized); s.end(); }); + server.on("tlsClientError", err => authorized.reject(err)); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const port = server.address().port; + + // Override installed AFTER the server's SSL_CTX was built. + tls.setDefaultCACertificates([ca1]); + + const client = tls.connect({ + port, + host: "127.0.0.1", + key: agent1Key, + cert: agent1Cert, + rejectUnauthorized: false, + }); + client.on("error", () => {}); + const result = await authorized.promise; + client.destroy(); + server.close(); + + assert.strictEqual(result, true, + "server did not pick up setDefaultCACertificates() for client-cert verification"); + console.log("ok"); + `); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("ok"); + expect(exitCode).toBe(0); + }); });