Add mutual TLS client certificate authentication on HTTPS listeners

[Shine-neko]

Closes #1299.

What

Adds mutual TLS (client certificate authentication) to HTTPS listeners. Until now lib/src/https.rs hardcoded .with_no_client_auth(), so verifying a client certificate was impossible.

Configuration

A per-listener client_auth mode drives the rustls WebPkiClientVerifier:

[[listeners]]
protocol = "https"
address = "0.0.0.0:443"
client_auth = "required"                     # none (default) | optional | required
client_ca_certificates = ["/etc/sozu/ca.pem"]
client_ca_crls = ["/etc/sozu/crl.pem"]       # optional

• none — current behavior, no CertificateRequest is sent
• optional — requests a certificate, accepts clients presenting none; a presented certificate must still validate
• required — aborts the handshake unless the client presents a certificate chaining to a trusted CA

Wire compatibility

Per your note on the issue, protobuf ordering is untouched. Three fields are appended to HttpsListenerConfig (48, 49, 50) plus a new ClientAuthMode enum whose zero value is CLIENT_AUTH_NONE, so an absent field decodes to today's behavior. The two new repeated fields get #[serde(default)] via command/build.rs so state files written before this change still load.

Fail-closed behavior

Configuration is rejected rather than silently degraded when:

• client_auth carries an unknown enum value (never folded to NONE)
• a non-none mode has no trusted CA
• a configured CA or CRL file cannot be read
• a CA or CRL entry parses to zero certificates / revocation lists (would otherwise reduce the trust set or disable revocation silently)
• an mTLS field is set on a non-HTTPS listener (to_http / to_tcp / to_udp would discard it)

CRL expiration is enforced (enforce_revocation_expiration); rustls defaults to ExpirationPolicy::Ignore, which would keep trusting a CRL past its nextUpdate. In none mode CA/CRL paths are not read at all, so a stale path never blocks config loading.

The verifier is built with the same explicitly selected CryptoProvider as the server config, so it does not depend on a process-default provider (absent in crypto-openssl-only builds, ambiguous under --all-features).

Tests

8 verifier tests in lib/src/https.rs (each mode, unknown value, missing/malformed CA, empty CA entry among valid ones, empty CRL entry) and 4 in command/src/config.rs (old state file without the new fields, lowercase mode names, non-HTTPS rejection, none ignoring stale paths).

cargo clippy --all-targets -- -D warnings and cargo fmt --check are clean.

Known limitation

An invalid mTLS config is caught worker-side in create_rustls_context, after the master has already recorded the listener in ConfigState. Recreating it then hits StateError::Exists. This mirrors how every other invalid HTTPS listener config behaves today (bad certificate, bad key, unbuildable rustls context) since add_https_listener does no config validation, so it is left as-is rather than changed here. Happy to open a separate issue if you want master-side validation addressed across the board.

[FlorentinDUBOIS]

Cross-review (5 lenses + Codex cross-check)

Reviewed 1518d93e across /review, /review-code, /security-review, /guidelines, /simplify, plus an independent OpenAI Codex pass. 9 raw findings → 4 after dedupe → 4/4 survived adversarial refutation, 0 dropped.

What checked out clean

The security core of this feature is correct and fails closed — verified against the vendored rustls 0.23.42 source, not from memory:

• client_cert_verifier rejects unknown/garbage client_auth enum values via ClientAuthMode::try_from rather than silently degrading to NONE.
• A non-NONE mode with an empty CA bundle is rejected; a CA/CRL PEM entry parsing to zero objects is rejected rather than silently dropped (the before/after .len() check is sound — RootCertStore::add does not dedupe).
• CRL expiration is unconditionally enforced (enforce_revocation_expiration()), and WebPkiClientVerifier's default UnknownStatusPolicy::Deny is left intact.
• allow_unauthenticated() (OPTIONAL) matches its rustls contract: a presented certificate is still fully chain-validated — no silent bypass.
• try_new/create_rustls_context return Result, so a listener that fails to build its verifier is never activated — no fallback to an unauthenticated listener.
• UpdateHttpsListenerConfig carries no mTLS fields, so a running listener's policy cannot be downgraded by a runtime patch; the ALPN-change rebuild path reuses the existing config.
• Protobuf/serde back-compat is sound: field numbers 48–50 don't collide, Option<i32> defaults to None without #[serde(default)], and the repeated fields correctly do carry it.

Codex additionally scripted a live TLS handshake exercising OPTIONAL/REQUIRED with and without a client certificate and confirmed correct accept/reject behavior; its verdict was "no actionable defect attributable to this commit". cargo check --workspace --all-features, cargo clippy -p sozu-lib -p sozu-command-lib -- -D warnings, and the 12 new unit tests all pass. No correctness or security defect was found.

What's left (4 inline comments)

Severity	Finding
🟡 medium	doc/configure.md not updated for the three new TOML keys (CONTRIBUTING.md "docs are code")
🔵 low	HttpsListenerConfig's hand-written Debug impl omits the new mTLS fields
🔵 low	New helper orphans to_http's doc comment
🔵 low	No end-to-end handshake test, despite the existing tcp_sni_tests.rs mTLS pattern

Nothing here is a defect in the shipped logic — the blocking item is the documentation rule. Details inline.

_{Grounded at head 1518d93eba16072d6da2e53b3f6decc198abde97.}

[FlorentinDUBOIS]

🟡 medium — New client_auth / client_ca_certificates / client_ca_crls TOML keys are undocumented

doc/configure.md is the canonical [[listeners]] TOML reference and is untouched by this PR. Verified: git show 1518d93e:doc/configure.md | grep -n client_auth → no hits; the only mTLS mentions are pre-existing rows in the handshake-error tables (lines 1197, 2292, 2294) that assume the feature exists elsewhere.

CONTRIBUTING.md:74 says "Docs are code: a change to a public metric, config key, or CLI flag updates its documentation in the same changeset." Every comparable listener knob — hsts, sni_preread_max_bytes, the h2_* family, cipher suites, key-exchange groups — has a subsection under Options specific to HTTPS listeners (line 208) / Options specific to Rustls based HTTPS listeners (line 375) with an example TOML block and, for HSTS, a validation matrix. These three keys are the most security-sensitive of the set and an operator has no way to discover the accepted values (none/optional/required), that CA/CRL entries are filesystem paths resolved at config-materialization time, or the fail-closed failure modes (ConfigError::ClientAuthOnNonHttps, empty-CA rejection, unknown-enum rejection, CRL expiration enforcement).

Please add a mTLS subsection mirroring the HSTS treatment. Also stale in the same changeset: doc/upgrade/1.x-to-2.0.md:400 still lists "Frontend mTLS termination" ([#663]) as follow-up release, which this PR implements.

---

_{lenses: /review, /review-code, /guidelines, /codex • confidence: high • adversarially verified at 1518d93e}

[FlorentinDUBOIS]

🔵 low — HttpsListenerConfig's hand-written Debug impl doesn't surface the new mTLS fields

HttpsListenerConfig is in the skip_debug list on the very next line, so prost's derive is replaced by the hand-written impl std::fmt::Debug for command::HttpsListenerConfig at command/src/proto/mod.rs:409. That impl enumerates every field explicitly — sensitive ones redacted (certificate, key), bulky ones summarised (_len/_count) — and ends at .finish() with hsts then h2_max_header_fields as the last entries. client_auth, client_ca_certificates, and client_ca_crls were not added, so a listener's entire mTLS configuration is invisible in Debug/log output, not even as a count.

Not a leak (omission, not over-exposure), but it breaks the established exhaustive pattern and removes the one observability handle for triaging mTLS config issues. Add .field("client_auth", &self.client_auth) plus client_ca_certificates_count/client_ca_crls_count (and total_string_len for the PEM bodies), matching the cipher_list/groups_list treatment in the same impl.

---

_{lenses: /codex • confidence: high • adversarially verified at 1518d93e}

[FlorentinDUBOIS]

🔵 low — New helper was inserted between an existing doc comment and its target, orphaning to_http's docs

Verified at head: config.rs:1047 is the pre-existing /// build an HTTP listener with config timeouts, using defaults if no config is provided, which documented pub fn to_http. This diff inserts reject_client_auth_fields (with its own doc block at 1048-1051) between the two, so rustdoc now renders that stray line as the first paragraph of reject_client_auth_fields's docs, while to_http at config.rs:1070 ends up with no doc comment at all — unlike its siblings to_tls/to_tcp/to_udp, which each keep theirs.

Fix: delete line 1047 and re-add it directly above pub fn to_http at 1070, leaving reject_client_auth_fields with only its own four-line block.

---

_{lenses: /review, /review-code, /simplify • confidence: high • adversarially verified at 1518d93e}

[FlorentinDUBOIS]

🔵 low — No end-to-end handshake test for the new frontend client_auth feature

CLAUDE.md lists lib/src/https.rs under "Security-sensitive areas — conservative changes + tests required". The 8 new unit tests here only exercise client_cert_verifier in isolation (does the builder succeed or return ListenerError::ClientAuth for a given config); none drives an actual TLS handshake through a live HttpsListener to prove that required really aborts a cert-less connection and that optional/required really accept a valid client identity.

That gap matters because the interesting failure mode is an auth bypass at handshake time, not at builder time — e.g. allow_unauthenticated() applied on the wrong branch would still build a verifier and pass every test here.

The repo already has the pattern and the assets: e2e/src/tests/tcp_sni_tests.rs:702 (try_tcp_sni_mtls_client_cert_reaches_backend) and :798 (try_tcp_sni_mtls_rejects_client_without_cert) drive real rustls::ClientConnections with and without a client identity against a live listener and assert accept/reject, reusing e2e/assets/tcp_sni/mtls-client-{cert,key}.pem. Worth adding the frontend-termination equivalent (required rejects no-cert, optional accepts no-cert, both accept a valid cert) before merge.

---

_{lenses: /review • confidence: medium • adversarially verified at 1518d93e}

[Shine-neko]

Thanks for the thorough pass, especially for grounding the rustls behaviour against the vendored source rather than from memory. All four findings are addressed.

Heads-up on history: the branch was force-pushed, so 1518d93e is now 2c3c092f and your four inline comments are likely marked outdated. The delta against your reviewed commit is exactly the five files below, nothing else.

🟡 medium — undocumented TOML keys

Added a #### mTLS — client certificate authentication section to doc/configure.md, under Options specific to HTTPS listeners and mirroring the HSTS treatment: a commented TOML block, a mode table (CertificateRequest sent / behaviour with and without a client certificate), a validation matrix covering every fail-closed path, and notes on scope.

The matrix distinguishes the two rejection stages, which the description in this PR conflated: reading the CA/CRL files and rejecting mTLS keys on a non-HTTPS listener happen at config-load in the master, while PEM parsing and verifier construction happen worker-side.

Also updated, as you flagged: doc/upgrade/1.x-to-2.0.md:400 now reads shipped via #1300 instead of follow-up release. I deliberately left the version out rather than guessing a tag.

One thing I did not carry over from your comment: the UnknownStatusPolicy::Deny row. You verified it against the vendored rustls sources; I did not, and https.rs has no explicit mention of it, so I left the claim out of the docs rather than restate something I hadn't checked. Happy to add it if you confirm.

🔵 low — Debug impl

command/src/proto/mod.rs now emits client_auth, client_ca_certificates_count/_len and client_ca_crls_count/_len, following the cipher_list / groups_list treatment in the same impl.

🔵 low — orphaned doc comment

/// build an HTTP listener with config timeouts... moved back directly above pub fn to_http; reject_client_auth_fields keeps only its own block.

🔵 low — no end-to-end handshake test

Added three tests in e2e/src/tests/tls_tests.rs, driving real rustls::ClientConnection handshakes against a live HTTPS listener:

Test	Asserts
test_mtls_required_rejects_client_without_cert	no response and requests_received == 0
test_mtls_required_accepts_trusted_client_cert	HTTP/1.1 200 + backend reached
test_mtls_optional_accepts_client_without_cert	HTTP/1.1 200 + backend reached

The reject test asserts an untouched backend, not just an absent response: absence alone would also pass if Sōzu admitted the session and answered a 4xx, which is not what required means.

They reuse the e2e/assets/tcp_sni/ CA and client identity (mtls-client-cert.pem chains to ca-cert.pem and carries the TLS Web Client Authentication EKU). Since HttpsListenerConfig carries inlined PEM rather than paths, the tests mutate the generated config in place, same approach as the existing disable_http11 test.

Mutation-checked against exactly the bypass you described: flipping allow_unauthenticated() from the OPTIONAL arm to the REQUIRED one makes required_rejects and optional_accepts both fail, while every unit test in lib/src/https.rs still passes. That pair is what pins the mode boundary.

Verification

cargo clippy --all-targets -- -D warnings and cargo +nightly fmt --check clean; tls_tests 12/12; the 8 verifier unit tests and the sozu-command-lib suite green. CI is green on 2c3c092f except Docker build and push to Docker Hub, which fails identically on 1518d93e (fork credentials, unrelated to this change).

The master-side validation limitation from the PR description is unchanged and still looks like a separate issue to me.

[FlorentinDUBOIS]

Thx, I will take another look tomorrow

[FlorentinDUBOIS]

Automated cross-review (review / review-code / Rust guidelines / security-review + Codex, adversarially verified). Three findings are inline and were submitted as one batched review.

[FlorentinDUBOIS]

[security, high] Older workers silently ignore required mTLS and acknowledge the listener

UpgradeMain deliberately hands pre-upgrade workers to the new master, with worker replacement happening afterwards (bin/src/command/LIFECYCLE.md:188-203). Because this extends the existing AddHttpsListener payload rather than using a capability-distinguishable verb, a pre-PR worker recognizes the outer request while prost skips unknown nested tags 48-50. It then builds the listener through the historical .with_no_client_auth() path and returns OK, so client_auth = "required" can accept unauthenticated traffic until every old worker is gone. A failed worker upgrade can leave this state persistent because the CLI logs individual upgrade failures but still returns success. Please make mTLS listener creation fail closed on older workers—for example through a new request variant that old workers reject, or a verified per-worker capability gate before fan-out—and add a new-master/old-worker encoding regression test.

[FlorentinDUBOIS]

[docs, medium] Document rustls's fail-closed CRL coverage requirements

The implementation calls with_crls(...).enforce_revocation_expiration() while retaining rustls 0.23.42's defaults of chain-wide revocation checking and UnknownStatusPolicy::Deny. Consequently, a handshake is rejected not only when a certificate is listed as revoked, but also when revocation status cannot be established for an applicable certificate in the chain or when the CRL is expired. The current text only promises rejection of certificates listed as revoked, so an operator supplying a partial CRL set can unexpectedly lock out otherwise valid clients. Please document the full-chain, complete/current-CRL requirement and the unknown-status fail-closed behavior.

[FlorentinDUBOIS]

[regression, low] The default listener listing hides the active client-authentication policy

The normal ListListeners output formats HttpsListenerConfig through command/src/proto/display.rs:1257-1334, whose table has no row for client_auth or safe CA/CRL metadata. An operator can therefore apply required authentication successfully and then see output indistinguishable from a listener with client authentication disabled. The new Debug fields do not cover this default CLI path. Please display the mode plus non-secret CA/CRL counts or byte lengths, without rendering PEM contents, and cover that formatter.