cluster: encrypt and authenticate controller to replica connections - #37997
Draft
jasonhernandez wants to merge 7 commits into
Draft
cluster: encrypt and authenticate controller to replica connections#37997jasonhernandez wants to merge 7 commits into
jasonhernandez wants to merge 7 commits 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>
Extend SecretsController and SecretsReader with internal secrets, keyed by name rather than by CatalogItemId. These hold system-generated credentials, such as the upcoming CTP transport keys, that have no corresponding catalog item. The two namespaces are disjoint in every backend: Kubernetes uses an "internal-" infix next to the existing "user-managed-" one, the process orchestrator prefixes secret files with "internal-", and AWS Secrets Manager gets an "internal-" infix after the deployment prefix. Internal secrets are excluded from list(), whose parsers only recognize user secret names. Names are validated against a conservative alphabet (lowercase alphanumerics and dashes) that is valid in all backends and cannot traverse paths. The process orchestrator's list() previously errored on any file in the secrets directory whose name did not parse as a CatalogItemId. It now skips such files, matching the documented behavior of the other backends. The caching reader deliberately does not cache internal secrets. They are read rarely, typically once at process startup, so caching would only extend the lifetime of key material in memory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…agement Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Callers bootstrapping credentials must distinguish a secret that does not exist yet (generate and persist a new one) from a transient read failure (fail and retry). Conflating the two could cause a caller to regenerate and overwrite live key material. Return Option instead of an untyped error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Wire the CTP mutual TLS support end to end, so controller and replica authenticate each other and their traffic is encrypted. Off by default everywhere, including in tests. Enabling it in the test and CI configurations is a separate change. environmentd, behind the new --cluster-transport-tls flag, bootstraps a per-environment certificate authority into an internal secret at startup and mints its own controller certificate. The cluster controller then mints a certificate per replica when it provisions the replica, writes it to the replica's internal secret, and passes the secret name to clusterd. clusterd reads the credentials through the secrets reader it already has and requires mutual TLS on both controller listeners. Dropping a replica deletes its secret, and so does cleaning up replicas left behind by a past deploy generation. Peer identity is the replica's service name, carried in the certificate SAN. The controller verifies it connected to the intended replica, and the replica verifies the peer holds a controller certificate from the same environment. Credential distribution rides the machinery that already exists for user secrets, so this needs no cert-manager, no operator changes, and no new RBAC. The credential lifecycle needs no operator intervention and no shell in either image. Replica certificates are re-minted on every replica process creation, so rotation rides process lifecycle rather than expiry, and a missing or corrupt credential secret is repaired by the controller re-ensuring it. Handshake failures are diagnosable from logs and metrics alone: a per-server failure counter and a certificate-expiry gauge, plus logs naming the peer and the reason. A new client now displaces the established one only after passing transport security, so with TLS enabled an unauthenticated peer that reaches the listen port can no longer evict the controller. The handshake runs in its own task, bounded by a 30s timeout, so a stalled peer blocks neither the accept loop nor the live connection. Also flush after writing each CTP message. A raw socket write reaches the peer on its own, but a TLS writer buffers plaintext into records and need not emit them until flushed, which would strand messages indefinitely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jasonhernandez
force-pushed
the
jason/ctp-tls-wiring
branch
from
August 2, 2026 17:54
5bd5bd6 to
36b6343
Compare
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
PR 3 of 4. Turns #37995 (CTP mutual TLS support) and #37996 (internal secrets) into a working feature: controller↔replica traffic can be encrypted, with both ends authenticating each other.
Today CTP is plaintext bincode with no authentication. It carries user rows (peek results,
SUBSCRIBEbatches), and because a CTP server serves one client and cancels the incumbent on any new connect, anyone who can reach a replica's*ctlport can evict the controller and take over the command stream. The trust model is "the pod network is isolated," which stops holding once an operator can mirror ENI traffic.Off by default everywhere, including in tests. Enabling it in the test and CI configurations is #37999, split out so that risky flip is independently revertable. The two are intended to land together.
Stacked on #37996 (which is stacked on #37995). Review those first; this branch merges both.
How it works
--cluster-transport-tls, bootstraps a per-environment CA into an internal secret at startup and mints its own controller certificate.--ctp-tls-secret=<name>to clusterd.SecretsReaderit already links and requires mutual TLS on both controller listeners.Identity is the replica's service name in the certificate SAN. The controller verifies it reached the intended replica; the replica verifies the peer holds a controller certificate from the same environment. Distribution rides the machinery that already exists for user secrets: no cert-manager, no operator changes, no new RBAC.
No shell required, by construction
This is meant to hold up the other half of the distroless bargain, so the credential lifecycle never needs an operator inside a container:
opensslCLI, no scripts — including for rotation.mz_cluster_server_tls_handshake_failures_totalcounter, anmz_ctp_tls_cert_not_aftergauge, and handshake-failure logs naming the peer and reason.kubectl logsplus scraping is enough — noopenssl s_clientneeded.Behavior changes
Credential cleanup
Past-generation replica cleanup (
try_remove_past_generation_replicas) drops services directly rather than going throughdeprovision_replica, so an earlier revision of this branch leaked one credential secret per replica per deploy generation — unbounded growth across upgrades. It now deletes credentials too, and because that path is already async it awaits the deletion, so a transient failure propagates into the existing retry loop instead of being logged and dropped.The
DROP CLUSTERpath (deprovision_replica) is synchronous and so still spawns the deletion and logs failures, matching the existing posture for user secrets, where orphaned-secret GC is already noted as future work. Worth a reviewer's opinion on whether that is good enough or should be made reliable too.Testing
mz-environmentdintegration test to pass locally —test_empty_subscribe_noticetimes out at 240s. A/B'ing it withcluster_tlsforced toNonetimes out identically, so the timeout is environmental (local debug build against a containerized CockroachDB), not caused by this change. Because TLS is off by default here, this PR's CI does not exercise the TLS path at all; cluster: enable cluster transport TLS in the test and CI configurations #37999 is where that gets tested.🤖 Generated with Claude Code