From bc1a8b791048a46601fd3398ddee3793869b1362 Mon Sep 17 00:00:00 2001 From: Alan Hoffmeister Date: Tue, 17 Mar 2026 16:37:32 -0300 Subject: [PATCH 1/2] Load system CA bundle for verified clients Use the Zig system trust store by default when client certificate verification is enabled and no custom CA file is supplied. Keep ownership of the allocated CA bundle in the client lifecycle and cover the builder behavior with unit tests. Co-authored-by: Codex --- README.md | 2 +- SPEC/RFC5280_CHAIN_VALIDATION.md | 3 +- src/event_loop.zig | 113 +++++++++++++++++++++++-------- src/test_all.zig | 1 + 4 files changed, 87 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 7836a02..ce384dd 100644 --- a/README.md +++ b/README.md @@ -208,7 +208,7 @@ pub fn main() !void { | `port` | `4433` | Server port | | `server_name` | `"localhost"` | TLS SNI / CONNECT authority | | `path` | `"/.well-known/webtransport"` | WebTransport CONNECT path | -| `ca_cert_path` | `null` | CA certificate for TLS verification | +| `ca_cert_path` | `null` | CA certificate for TLS verification; when unset, verified clients use the system root store | | `skip_cert_verify` | `false` | Skip certificate verification (testing only) | | `max_datagram_frame_size` | `65536` | QUIC datagram frame size limit | | `ipv6` | `false` | Use IPv6 dual-stack socket | diff --git a/SPEC/RFC5280_CHAIN_VALIDATION.md b/SPEC/RFC5280_CHAIN_VALIDATION.md index e4841a0..912d331 100644 --- a/SPEC/RFC5280_CHAIN_VALIDATION.md +++ b/SPEC/RFC5280_CHAIN_VALIDATION.md @@ -19,6 +19,7 @@ #### System Root CAs - `loadSystemCaBundle()` helper wraps `Certificate.Bundle.rescan()` for OS-native roots - Supports macOS (Keychain), Linux (`/etc/ssl/certs/`), Windows (CertStore), FreeBSD, OpenBSD, etc. +- `event_loop.ClientConfig` now auto-loads the system root store when `skip_cert_verify=false` and no `ca_cert_path` is provided ### Configuration @@ -39,8 +40,6 @@ const tls_config = TlsConfig{ - **Extended Key Usage** — `id-kp-serverAuth` not checked on leaf (recommended but not required by TLS 1.3) - **Name Constraints** — RFC 5280 §4.2.1.10 - **Policy Constraints** — RFC 5280 §4.2.1.11 -- **Mandatory ca_bundle enforcement** — When `skip_cert_verify=false` and no `ca_bundle` is provided, the chain's self-signed root is accepted without trust anchor verification - ### Caveats - `skip_cert_verify` defaults to `true` for backward compatibility diff --git a/src/event_loop.zig b/src/event_loop.zig index 8be70ea..071b32e 100644 --- a/src/event_loop.zig +++ b/src/event_loop.zig @@ -167,10 +167,10 @@ pub fn Server(comptime Handler: type) type { const known = [_][]const u8{ "onConnectRequest", "onSessionReady", "onStreamData", - "onDatagram", "onSessionClosed", "onSessionDraining", - "onBidiStream", "onUniStream", "onPollComplete", - "onRequest", "onData", - "onH0Request", "onH0Data", "onH0Finished", + "onDatagram", "onSessionClosed", "onSessionDraining", + "onBidiStream", "onUniStream", "onPollComplete", + "onRequest", "onData", "onH0Request", + "onH0Data", "onH0Finished", }; for (@typeInfo(Handler).@"struct".decls) |decl| { @@ -870,6 +870,57 @@ pub const ClientConfig = struct { ipv6: bool = false, }; +const client_alpn = &[_][]const u8{"h3"}; + +const BuiltClientTlsConfig = struct { + tls_config: tls13.TlsConfig, + owned_ca_bundle: ?*Certificate.Bundle = null, + + fn deinit(self: *BuiltClientTlsConfig, alloc: std.mem.Allocator) void { + if (self.owned_ca_bundle) |bundle| { + bundle.deinit(alloc); + alloc.destroy(bundle); + self.owned_ca_bundle = null; + self.tls_config.ca_bundle = null; + } + } +}; + +fn buildClientTlsConfig(alloc: std.mem.Allocator, config: ClientConfig) !BuiltClientTlsConfig { + if (config.tls_config) |tc| { + return .{ .tls_config = tc }; + } + + var ca_bundle: ?*Certificate.Bundle = null; + if (!config.skip_cert_verify) { + const bundle_ptr = try alloc.create(Certificate.Bundle); + errdefer alloc.destroy(bundle_ptr); + + if (config.ca_cert_path) |ca_path| { + bundle_ptr.* = .{}; + errdefer bundle_ptr.deinit(alloc); + try bundle_ptr.addCertsFromFilePath(alloc, std.fs.cwd(), ca_path); + } else { + bundle_ptr.* = try tls13.loadSystemCaBundle(alloc); + errdefer bundle_ptr.deinit(alloc); + } + + ca_bundle = bundle_ptr; + } + + return .{ + .tls_config = .{ + .cert_chain_der = &.{}, + .private_key_bytes = &.{}, + .alpn = client_alpn, + .server_name = config.server_name, + .skip_cert_verify = config.skip_cert_verify, + .ca_bundle = ca_bundle, + }, + .owned_ca_bundle = ca_bundle, + }; +} + /// ClientSession wraps a single client-side connection and provides the same /// convenience methods as the server-side Session. pub const ClientSession = struct { @@ -965,9 +1016,9 @@ pub fn Client(comptime Handler: type) type { } const known = [_][]const u8{ - "onConnected", "onSessionReady", "onSessionRejected", - "onStreamData", "onDatagram", "onSessionClosed", - "onSessionDraining", "onBidiStream", "onUniStream", + "onConnected", "onSessionReady", "onSessionRejected", + "onStreamData", "onDatagram", "onSessionClosed", + "onSessionDraining", "onBidiStream", "onUniStream", "onPollComplete", }; @@ -1031,6 +1082,7 @@ pub fn Client(comptime Handler: type) type { wt_conn: ?wt.WebTransportConnection, protocol_initialized: bool, session_id: ?u64, + owned_ca_bundle: ?*Certificate.Bundle, // Config retained for protocol init server_name: []const u8, @@ -1038,27 +1090,8 @@ pub fn Client(comptime Handler: type) type { pub fn init(alloc: std.mem.Allocator, handler: *Handler, config: ClientConfig) !Self { // Build TLS config - const tls_config: tls13.TlsConfig = if (config.tls_config) |tc| tc else blk: { - const alpn = try alloc.alloc([]const u8, 1); - alpn[0] = "h3"; - - var ca_bundle: ?*Certificate.Bundle = null; - if (config.ca_cert_path) |ca_path| { - const bundle_ptr = try alloc.create(Certificate.Bundle); - bundle_ptr.* = .{}; - try bundle_ptr.addCertsFromFilePath(alloc, std.fs.cwd(), ca_path); - ca_bundle = bundle_ptr; - } - - break :blk .{ - .cert_chain_der = &.{}, - .private_key_bytes = &.{}, - .alpn = alpn, - .server_name = config.server_name, - .skip_cert_verify = config.skip_cert_verify, - .ca_bundle = ca_bundle, - }; - }; + var built_tls_config = try buildClientTlsConfig(alloc, config); + errdefer built_tls_config.deinit(alloc); // Connection config const conn_config: connection.ConnectionConfig = if (config.conn_config) |cc| cc else cc_blk: { @@ -1074,7 +1107,7 @@ pub fn Client(comptime Handler: type) type { alloc, config.server_name, conn_config, - tls_config, + built_tls_config.tls_config, null, ); // Heap-allocate so pointers remain stable @@ -1142,6 +1175,7 @@ pub fn Client(comptime Handler: type) type { .wt_conn = null, .protocol_initialized = false, .session_id = null, + .owned_ca_bundle = built_tls_config.owned_ca_bundle, .server_name = config.server_name, .path = config.path, }; @@ -1155,6 +1189,10 @@ pub fn Client(comptime Handler: type) type { posix.close(self.sockfd); self.conn.deinit(); self.allocator.destroy(self.conn); + if (self.owned_ca_bundle) |bundle| { + bundle.deinit(self.allocator); + self.allocator.destroy(bundle); + } } pub fn start(self: *Self) void { @@ -1471,3 +1509,20 @@ pub fn Client(comptime Handler: type) type { } }; } + +test "buildClientTlsConfig loads system CA bundle when verification is enabled" { + var built = try buildClientTlsConfig(std.testing.allocator, .{}); + defer built.deinit(std.testing.allocator); + + try std.testing.expect(!built.tls_config.skip_cert_verify); + try std.testing.expect(built.tls_config.ca_bundle != null); +} + +test "buildClientTlsConfig does not allocate a CA bundle when verification is skipped" { + var built = try buildClientTlsConfig(std.testing.allocator, .{ + .skip_cert_verify = true, + }); + defer built.deinit(std.testing.allocator); + + try std.testing.expect(built.tls_config.ca_bundle == null); +} diff --git a/src/test_all.zig b/src/test_all.zig index 147a726..a650a75 100644 --- a/src/test_all.zig +++ b/src/test_all.zig @@ -20,6 +20,7 @@ test { _ = @import("quic/ecn.zig"); _ = @import("quic/ecn_socket.zig"); _ = @import("quic/quic_lb.zig"); + _ = @import("event_loop.zig"); _ = @import("h3/frame.zig"); _ = @import("h3/qpack.zig"); _ = @import("h3/huffman.zig"); From 94f2b6baca8ba1a26dd2f60eb282ff44bd5d12f6 Mon Sep 17 00:00:00 2001 From: Alan Hoffmeister Date: Tue, 17 Mar 2026 16:42:58 -0300 Subject: [PATCH 2/2] Clean up client init error handling Clarify the TLS verification defaults in the RFC 5280 note and release the temporary client connection on the init error path before ownership is transferred. Co-authored-by: Codex --- SPEC/RFC5280_CHAIN_VALIDATION.md | 2 +- src/event_loop.zig | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/SPEC/RFC5280_CHAIN_VALIDATION.md b/SPEC/RFC5280_CHAIN_VALIDATION.md index 912d331..5788099 100644 --- a/SPEC/RFC5280_CHAIN_VALIDATION.md +++ b/SPEC/RFC5280_CHAIN_VALIDATION.md @@ -42,6 +42,6 @@ const tls_config = TlsConfig{ - **Policy Constraints** — RFC 5280 §4.2.1.11 ### Caveats -- `skip_cert_verify` defaults to `true` for backward compatibility +- `tls13.TlsConfig.skip_cert_verify` defaults to `true` for backward compatibility, while `event_loop.ClientConfig.skip_cert_verify` defaults to `false` - V1 certificates (no extensions) are accepted as CAs when no basicConstraints is present — this matches common practice but is less strict than RFC 5280's recommendation - The interop client always uses `skip_cert_verify=true` since interop test peers use various self-signed certs diff --git a/src/event_loop.zig b/src/event_loop.zig index 071b32e..e0053db 100644 --- a/src/event_loop.zig +++ b/src/event_loop.zig @@ -1103,13 +1103,14 @@ pub fn Client(comptime Handler: type) type { }; // Create QUIC client connection - const conn = try connection.connect( + var conn = try connection.connect( alloc, config.server_name, conn_config, built_tls_config.tls_config, null, ); + errdefer conn.deinit(); // Heap-allocate so pointers remain stable const conn_ptr = try alloc.create(connection.Connection); errdefer {