service: add mutual TLS support to the cluster transport protocol - #37995
Draft
jasonhernandez wants to merge 1 commit into
Draft
service: add mutual TLS support to the cluster transport protocol#37995jasonhernandez wants to merge 1 commit into
jasonhernandez wants to merge 1 commit into
Conversation
Add optional mutual TLS to CTP connections. When configured, both endpoints present an X.509 certificate signed by a deployment-internal CA and verify the peer's certificate chain and identity (a DNS-shaped name in the SAN) before any CTP bytes are exchanged. Nothing enables TLS yet. All callers pass None, so behavior is unchanged. Wiring the configuration through environmentd, the controllers, and clusterd is follow-up work. The new transport::tls module provides: * ClientTlsConfig / ServerTlsConfig: rustls-based endpoint configs. TLS 1.3 only, explicit aws-lc-rs provider. The server requires and verifies client certificates, then checks the client's identity against the expected name. The client verifies the server's identity through standard server-name verification. * CertificateAuthority: rcgen-based issuance of a path-length-zero CA and per-endpoint leaf certificates, with PEM persistence and reconstruction, for the controller to mint replica credentials. Connection::start now takes pre-split stream halves so it can run on either a plain stream or a TLS stream wrapped around one. The CTP handshake is unchanged and runs inside the TLS channel. The server bounds the TLS handshake with a 30s timeout so a stalled or non-TLS peer cannot occupy the single connection slot indefinitely. Private key material is wrapped in mz_ore::secure::SecureString, so it is zeroed on drop and redacted from debug output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
CTP (the controller↔replica transport) is plaintext bincode with no authentication. It carries user rows (peek results,
SUBSCRIBEbatches), and its single-client-server design means anyone who can reach a replica's*ctlport can displace the controller and take over the command stream. The trust model today is "the pod network is isolated", which breaks down once an operator can mirror ENI traffic, and it is a gap we want to close as part of removing shells and other operator access from the data plane.This PR adds the transport-level building block: optional mutual TLS on CTP connections, with identity pinning on both sides. It is PR 1 of a planned stack of 3:
mz_service::transport, plus CA/leaf issuance helpers. Nothing enables it; all callers passNone, so behavior is unchanged.SecretsController/SecretsReader, so the controller can distribute per-replica credentials through the existing secrets machinery.ensure_replica_locationtime, clusterd requires TLS on its controller listeners, plus observability and rollout gating.Design notes
builder_with_provider, so this neither depends on nor installs a process-default provider, and stays compatible with the FIPS path.WebPkiClientVerifierand checks the client's SAN against the expected identity after the handshake. The CTPHellohandshake is unchanged and runs inside the TLS channel, demoted to a compatibility check.CertificateAuthorityissues a path-length-zero CA and per-endpoint leaves (ECDSA P-256), with PEM persistence and reconstruction. Reconstruction rebuilds signing state from the same deterministic params helper as generation, keyed by the common name, rather than parsing the certificate. This avoids rcgen'sx509-parserfeature (which drags in a bannedlazy_staticand a duplicatesyn), and a name mismatch fails closed: issued leaves carry the wrong issuer name and cannot verify. A test proves the fail-closed behavior.Connection::startnow takes pre-split stream halves so the same code runs on plain and TLS streams. The server bounds the TLS handshake at 30s so a stalled or non-TLS peer cannot occupy the single connection slot indefinitely.mz_ore::secure::SecureString(zeroed on drop, redacted in debug output, non-Clone).Cargo.lock note
Cargo.lockgains entries forx509-parserand its dependency chain even though the rcgen feature is not enabled: rcgen references it as a weak dependency feature (x509-parser?/verify-aws), and cargo's version resolver locks optional dependencies regardless of activation.cargo tree -i x509-parserconfirms nothing depends on it and it is never built.cargo deny check licenses banspasses.Testing
Extends the existing turmoil-based CTP test suite (
src/service/tests/transport.rs) with nine TLS tests, all running under the same deterministic simulation as the existing tests:expect_err.The suite was run 20+ times locally with random simulation seeds with no flakes. Existing tests updated mechanically for the new
tlsparameter.🤖 Generated with Claude Code