cluster: enable cluster transport TLS in the test and CI configurations - #37999
Draft
jasonhernandez wants to merge 8 commits into
Draft
cluster: enable cluster transport TLS in the test and CI configurations#37999jasonhernandez wants to merge 8 commits into
jasonhernandez wants to merge 8 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>
Turn on cluster transport TLS in the Rust test harness, sqllogictest, and mzcompose, so the TLS path is exercised by CI before it earns trust in production, per our convention that a new flag defaults off in production but on in test. Kept separate from the change that adds the capability so this flip is independently reviewable and revertable. It affects every test that starts a cluster, so if it proves slow or flaky, reverting it does not also revert the feature. The two are intended to land together. The mzcompose flag is version gated, as older images do not know it. 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
1 participant
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 4 of 4 in the CTP encryption stack (#37995 → #37996 → #37997 → this). #37997 adds the ability to encrypt and authenticate controller↔replica connections, defaulted off everywhere. This turns it on in the Rust test harness, sqllogictest, and mzcompose, so CI exercises the TLS path before the feature earns trust in production.
Why this is a separate PR. Our convention is that a new flag defaults off in production but on in test/CI, so the new code path gets exercised by sqllogictest, testdrive, and friends. This PR is that half of the convention. It is split out because it affects every test that starts a cluster: if it turns out to be slow or flaky, reverting it should not also revert the feature. The two are intended to land together, and #37997 is not much use without this.
What changes
TestHarness(src/environmentd/src/test_util.rs) — bootstraps a CA per test server.Materialized— passes--cluster-transport-tls, version gated since older images do not know the flag.What to watch on this PR's CI
This is the first run where CTP traffic is actually encrypted across the suites, so this build is the real signal for the whole stack. Worth looking at specifically:
COPY TOgo via persist/S3), so steady-state overhead should be small, but I have not measured it with a feature benchmark — that is an explicit gap, not a claim that it is free.For context on why real-socket coverage matters here: an earlier revision of #37997 had a bug where CTP never flushed after writing a message, which strands data in the TLS record buffer. The turmoil-simulated socket did not reproduce it; only running the real process orchestrator did. That is the class of problem this PR's CI run exists to catch.
🤖 Generated with Claude Code