From 248eb53a105422e93e70d8640f8ef985628e651d Mon Sep 17 00:00:00 2001 From: fylorn <249551762+fylorn@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:46:14 +0800 Subject: [PATCH] fix(redis): connect over TLS for rediss:// URLs fred was built without a TLS feature, so a rediss:// URL was dialled as plain TCP. Against a Redis that only speaks TLS - what ElastiCache with in-transit encryption, Upstash, Azure Cache and Redis Cloud require - the server exited at startup with "Failed to connect to Redis: Protocol Error: Expected string." Turning on fred's enable-rustls alone is not enough: fred then builds its connector with rustls's process-default crypto provider, and this binary compiles in two (aws-lc-rs through reqwest 0.13, ring through the reqwest 0.12 under openidconnect), so rustls panics instead of choosing. think_watch_common::redis_config builds the connector itself, with aws-lc-rs and rustls-platform-verifier - the stack reqwest already uses for upstream HTTPS - and every Redis client the server makes (the command client and the three Pub/Sub subscribers) takes its config from there. No new crates and no OpenSSL: the static musl image is unchanged, and the system roots it already ships for upstream HTTPS cover the managed services' public CAs. REDIS_CA_CERT names a PEM file for a Redis whose certificate a private CA signed; its certificates are then the only roots trusted, as with redis-cli --cacert. The Helm chart mounts it from a Secret (redis.caSecret). Client certificates are not supported. tests/redis_tls.rs runs the limit scripts, the gateway (limits, readiness, config notices) and a refused untrusted certificate against TEST_REDIS_TLS_URL, and skips without it. TEST_REDIS_CA_CERT lets the whole suite and the cluster tests run against TLS too. The config notice round-trip now resends the notice until it arrives: fred's subscribe can return before Redis holds the subscription, and a PUBLISH right after it was seen to reach no one. Co-Authored-By: Claude Opus 5.5 --- .env.example | 7 + CHANGELOG.md | 11 + Cargo.lock | 6 + Cargo.toml | 9 +- crates/common/Cargo.toml | 2 + crates/common/src/config.rs | 23 +++ crates/common/src/lib.rs | 1 + crates/common/src/redis_config.rs | 194 ++++++++++++++++++ crates/server/src/init.rs | 6 +- crates/server/src/main.rs | 15 +- crates/test-support/src/lib.rs | 67 +++++- .../tests/multi_instance_config_sync.rs | 3 +- crates/test-support/tests/redis_cluster.rs | 24 +-- crates/test-support/tests/redis_tls.rs | 188 +++++++++++++++++ deploy/helm/think-watch/README.md | 49 +++++ .../think-watch/templates/deployment.yaml | 22 ++ deploy/helm/think-watch/values.yaml | 11 +- 17 files changed, 606 insertions(+), 32 deletions(-) create mode 100644 crates/common/src/redis_config.rs create mode 100644 crates/test-support/tests/redis_tls.rs diff --git a/.env.example b/.env.example index de417fc8..988a7efb 100644 --- a/.env.example +++ b/.env.example @@ -33,6 +33,13 @@ REDIS_PASSWORD=__SECRET_HEX_16__ # prod: REDIS_URL=redis://:${REDIS_PASSWORD}@redis:6379 # Redis Cluster: REDIS_URL=redis-cluster://:${REDIS_PASSWORD}@redis-0:6379?node=redis-1:6379 # (see deploy/helm/think-watch/README.md, "Redis Cluster") +# TLS (managed Redis services usually require it): the rediss:// scheme, +# rediss-cluster:// for a cluster. The certificate is checked against the +# public CAs and the host name in the URL. +# REDIS_URL=rediss://:${REDIS_PASSWORD}@my-cache.example.com:6379 +# A Redis whose certificate a private CA signed: a PEM file with that CA, +# then trusted alone (see deploy/helm/think-watch/README.md, "Redis over TLS") +# REDIS_CA_CERT=/etc/thinkwatch/redis-ca.crt # --- Application --- JWT_SECRET=__SECRET_HEX_32__ diff --git a/CHANGELOG.md b/CHANGELOG.md index ee98abb2..675ab46a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,17 @@ target. invalidation reached one node only. Every key a script touches now shares a hash tag, pattern deletes scan every node, and the Helm chart's README describes a `redis-cluster://` URL. +- **Redis over TLS.** The server was built without TLS for Redis: a + `rediss://` URL was used as plain TCP, so against a Redis that requires + TLS — ElastiCache with in-transit encryption, Upstash, Azure Cache for + Redis and Redis Cloud among them — the server did not start + (`Failed to connect to Redis: Protocol Error: Expected string.`). + `rediss://` and `rediss-cluster://` URLs now connect over TLS and check + the certificate against the system's CAs, as upstream HTTPS does. For a + Redis whose certificate a private CA signed, `REDIS_CA_CERT` names a PEM + file with that CA, which is then trusted alone; the Helm chart sets it + from a Secret given in `redis.caSecret`. The chart's README describes + both. ## [3.1.0] — 2026-10-05 diff --git a/Cargo.lock b/Cargo.lock index 1f5b01eb..1b55c6ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1335,9 +1335,12 @@ dependencies = [ "parking_lot", "rand 0.8.5", "redis-protocol", + "rustls", + "rustls-native-certs", "semver", "socket2 0.5.10", "tokio", + "tokio-rustls", "tokio-stream", "tokio-util", "url", @@ -3220,6 +3223,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "758025cb5fccfd3bc2fd74708fd4682be41d99e5dff73c377c0646c6012c73a4" dependencies = [ "aws-lc-rs", + "log", "once_cell", "ring", "rustls-pki-types", @@ -4085,6 +4089,8 @@ dependencies = [ "rand 0.10.0", "reqwest 0.13.2", "rust_decimal", + "rustls", + "rustls-platform-verifier", "serde", "serde_json", "sha1 0.11.0", diff --git a/Cargo.toml b/Cargo.toml index 8c0b5b75..ba9ac7a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,14 @@ rust_decimal = { version = "1", features = ["serde", "serde-with-str"] } clickhouse = { version = "0.13", features = ["time", "chrono"] } # Redis -fred = { version = "10", features = ["subscriber-client", "i-scripts"] } +# `enable-rustls` gives fred its rustls connector, for `rediss://` URLs. +# The connector itself is built in `think_watch_common::redis_config` +# with the same TLS stack the HTTP client uses (below): rustls on +# aws-lc-rs, the platform's roots through rustls-platform-verifier. No +# OpenSSL anywhere, so the static musl image needs nothing new. +fred = { version = "10", features = ["subscriber-client", "i-scripts", "enable-rustls"] } +rustls = { version = "0.23", default-features = false, features = ["aws_lc_rs", "std", "tls12"] } +rustls-platform-verifier = "0.6" # Auth # Crypto-critical crates are pinned with `=` so a silent minor-release diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index b50cf489..e4d5bd7d 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -10,6 +10,8 @@ tw-bedrock = { workspace = true } axum = { workspace = true } sqlx = { workspace = true } fred = { workspace = true } +rustls = { workspace = true } +rustls-platform-verifier = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } uuid = { workspace = true } diff --git a/crates/common/src/config.rs b/crates/common/src/config.rs index 3702f021..9afd363e 100644 --- a/crates/common/src/config.rs +++ b/crates/common/src/config.rs @@ -4,6 +4,11 @@ use serde::Deserialize; pub struct AppConfig { pub database_url: String, pub redis_url: String, + /// `REDIS_CA_CERT`: a PEM file with the CA certificate(s) to trust + /// for a `rediss://` `REDIS_URL`, in place of the system's roots — + /// for a Redis whose certificate a private CA signed. See + /// [`crate::redis_config`]. + pub redis_ca_cert: Option, pub jwt_secret: String, pub encryption_key: String, pub server_host: String, @@ -51,6 +56,9 @@ impl AppConfig { .map_err(|_| anyhow::anyhow!("DATABASE_URL environment variable is required"))?, redis_url: std::env::var("REDIS_URL") .map_err(|_| anyhow::anyhow!("REDIS_URL environment variable is required"))?, + redis_ca_cert: std::env::var_os("REDIS_CA_CERT") + .filter(|s| !s.is_empty()) + .map(Into::into), jwt_secret: std::env::var("JWT_SECRET") .map_err(|_| anyhow::anyhow!("JWT_SECRET environment variable is required"))?, encryption_key: std::env::var("ENCRYPTION_KEY") @@ -134,6 +142,13 @@ impl AppConfig { } } + if self.redis_ca_cert.is_some() && !crate::redis_config::uses_tls(&self.redis_url) { + tracing::warn!( + "REDIS_CA_CERT is set, but REDIS_URL does not use TLS — the connection is \ + unencrypted and the certificate unused; use a rediss:// URL" + ); + } + // ClickHouse auth warning if self.clickhouse_url.is_some() && self.clickhouse_password.is_none() { tracing::warn!( @@ -144,6 +159,13 @@ impl AppConfig { Ok(()) } + /// The fred config every Redis client of the server is built from: + /// `REDIS_URL`, with TLS when its scheme is `rediss` and + /// `REDIS_CA_CERT`'s roots when set. + pub fn redis_config(&self) -> anyhow::Result { + crate::redis_config::client_config(&self.redis_url, self.redis_ca_cert.as_deref()) + } + pub fn gateway_addr(&self) -> String { format!("{}:{}", self.server_host, self.gateway_port) } @@ -157,6 +179,7 @@ impl AppConfig { Self { database_url: "postgres://test".into(), redis_url: "redis://test".into(), + redis_ca_cert: None, jwt_secret: jwt_secret.into(), encryption_key: encryption_key.into(), server_host: "0.0.0.0".into(), diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 5ca12adc..334950bd 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -49,6 +49,7 @@ pub mod crypto; // AES-256-GCM envelope for secrets at rest pub mod fixed_window; pub mod json_secret; // `{"$enc": ...}` — a secret nested inside a JSONB column pub mod pii; // BlobRedactor — at-rest body redaction shared by gateway + mcp-gateway +pub mod redis_config; // the fred client config for REDIS_URL, with TLS for rediss:// pub mod redis_keys; // pattern deletes that work on one node and on a Redis Cluster pub mod tasks; // supervised_spawn — panic-isolated background tasks pub mod validation; diff --git a/crates/common/src/redis_config.rs b/crates/common/src/redis_config.rs new file mode 100644 index 00000000..1c3932bf --- /dev/null +++ b/crates/common/src/redis_config.rs @@ -0,0 +1,194 @@ +//! The fred client config for a Redis URL, TLS included. +//! +//! Every Redis client the server builds — the command client and the +//! three Pub/Sub subscribers — goes through [`client_config`], so a +//! `rediss://` URL means the same thing for all of them. +//! +//! A `rediss` / `valkeys` scheme (with or without `-cluster` / +//! `-sentinel`) turns TLS on. The connector is built here rather than +//! left to fred, which would build its own on `ClientConfig::builder()`: +//! that picks the process-wide rustls crypto provider, and this binary +//! compiles in two (aws-lc-rs for reqwest 0.13, ring for the reqwest +//! 0.12 under openidconnect), so rustls cannot choose one and panics. +//! Here the provider is named — aws-lc-rs, as reqwest uses — and the +//! server certificate is checked the way the HTTP client checks an +//! upstream's: against the platform's roots, through +//! rustls-platform-verifier. Managed Redis services (ElastiCache, +//! Upstash, Azure Cache, Redis Cloud) present certificates from public +//! CAs, which those roots cover. +//! +//! A self-hosted Redis whose certificate a private CA signed needs that +//! CA: `REDIS_CA_CERT` names a PEM file holding it (one certificate or +//! several), and then only those certificates are trusted — the same as +//! `redis-cli --cacert`. + +use std::path::Path; +use std::sync::Arc; + +use anyhow::Context; +use fred::types::config::{Config, TlsConfig, TlsConnector, TlsHostMapping}; +use rustls::pki_types::CertificateDer; +use rustls::pki_types::pem::PemObject; + +/// The fred config for `url`, with a TLS connector when its scheme asks +/// for TLS. `ca_cert` is the PEM file of `REDIS_CA_CERT`; it only +/// matters for a TLS URL. +/// +/// Errors never quote `url`: it carries the Redis password. +pub fn client_config(url: &str, ca_cert: Option<&Path>) -> anyhow::Result { + let mut parsed = url::Url::parse(url).context("REDIS_URL is not a valid URL")?; + // Hand fred the plain-TCP twin of a TLS scheme, so that it parses + // the URL without building a connector of its own (see the module + // docs), and attach ours. + let tls = match plain_twin(parsed.scheme()) { + Some(plain) => { + parsed + .set_scheme(&plain) + .map_err(|()| anyhow::anyhow!("REDIS_URL has an unusable scheme"))?; + true + } + None => false, + }; + let mut config = Config::from_url(parsed.as_str()).context("REDIS_URL is not usable")?; + if tls { + config.tls = Some(TlsConfig { + connector: tls_connector(ca_cert)?, + // A cluster node is reached at the address it announces, and + // its certificate must name that address (host name or IP). + hostnames: TlsHostMapping::None, + }); + } + Ok(config) +} + +/// Whether `url` asks for TLS, by the same rule fred applies. +pub fn uses_tls(url: &str) -> bool { + url::Url::parse(url).is_ok_and(|u| plain_twin(u.scheme()).is_some()) +} + +/// `rediss` → `redis`, `rediss-cluster` → `redis-cluster`, `valkeys` → +/// `valkey`, …; `None` for a scheme without TLS. +fn plain_twin(scheme: &str) -> Option { + [("rediss", "redis"), ("valkeys", "valkey")] + .into_iter() + .find_map(|(tls, plain)| { + scheme + .strip_prefix(tls) + .map(|rest| format!("{plain}{rest}")) + }) +} + +fn tls_connector(ca_cert: Option<&Path>) -> anyhow::Result { + let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider()); + let builder = rustls::ClientConfig::builder_with_provider(provider.clone()) + .with_safe_default_protocol_versions() + .context("Redis TLS: no usable protocol version")?; + let config = match ca_cert { + Some(path) => builder.with_root_certificates(private_roots(path)?), + None => { + let verifier = rustls_platform_verifier::Verifier::new(provider) + .context("Redis TLS: could not load the system's CA certificates")?; + // `dangerous()` is only rustls's door to a verifier of one's + // own; this one verifies fully (chain and host name), as it + // does inside reqwest. + builder + .dangerous() + .with_custom_certificate_verifier(Arc::new(verifier)) + } + }; + Ok(config.with_no_client_auth().into()) +} + +/// The certificates of `REDIS_CA_CERT`, as the only roots trusted. +fn private_roots(path: &Path) -> anyhow::Result { + let shown = path.display(); + let mut roots = rustls::RootCertStore::empty(); + for cert in CertificateDer::pem_file_iter(path) + .with_context(|| format!("REDIS_CA_CERT: cannot read {shown}"))? + { + let cert = cert.with_context(|| format!("REDIS_CA_CERT: {shown} is not valid PEM"))?; + roots + .add(cert) + .with_context(|| format!("REDIS_CA_CERT: {shown} holds an unusable certificate"))?; + } + anyhow::ensure!( + !roots.is_empty(), + "REDIS_CA_CERT: {shown} holds no certificate" + ); + Ok(roots) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn plain_urls_have_no_tls() { + for url in [ + "redis://:pw@localhost:6379/1", + "redis-cluster://:pw@redis-0:6379?node=redis-1:6379", + "valkey://localhost", + ] { + let config = client_config(url, None).unwrap(); + assert!(config.tls.is_none(), "{url}"); + assert!(!uses_tls(url), "{url}"); + } + } + + #[test] + fn tls_urls_get_our_connector_and_keep_everything_else() { + let config = client_config("rediss://user:p%40ss@cache.example.com:6380/3", None).unwrap(); + assert!(config.uses_rustls()); + assert_eq!(config.username.as_deref(), Some("user")); + assert_eq!(config.password.as_deref(), Some("p@ss")); + assert_eq!(config.database, Some(3)); + let fred::types::config::ServerConfig::Centralized { server } = &config.server else { + panic!("not centralized: {:?}", config.server); + }; + assert_eq!((&*server.host, server.port), ("cache.example.com", 6380)); + + let config = client_config( + "rediss-cluster://:pw@redis-0:6379?node=redis-1:6379&node=redis-2:6379", + None, + ) + .unwrap(); + assert!(config.uses_rustls()); + assert!(config.server.is_clustered()); + assert_eq!(config.server.hosts().len(), 3); + + for url in [ + "valkeys://localhost", + "rediss-sentinel://localhost:26379/0?sentinelServiceName=m", + ] { + assert!(client_config(url, None).unwrap().uses_rustls(), "{url}"); + assert!(uses_tls(url), "{url}"); + } + } + + #[test] + fn a_ca_file_without_certificates_is_refused() { + let dir = std::env::temp_dir().join(format!("tw-redis-ca-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + let empty = dir.join("empty.pem"); + std::fs::write(&empty, "not a certificate\n").unwrap(); + let err = client_config("rediss://localhost", Some(&empty)).unwrap_err(); + assert!( + format!("{err:#}").contains("holds no certificate"), + "{err:#}" + ); + + let missing = dir.join("missing.pem"); + let err = client_config("rediss://localhost", Some(&missing)).unwrap_err(); + assert!(format!("{err:#}").contains("cannot read"), "{err:#}"); + std::fs::remove_dir_all(&dir).unwrap(); + + // A plain URL never reads it. + assert!(client_config("redis://localhost", Some(&missing)).is_ok()); + } + + #[test] + fn errors_do_not_quote_the_url() { + let err = client_config("rediss://:hunter2@", None).unwrap_err(); + assert!(!format!("{err:#}").contains("hunter2"), "{err:#}"); + } +} diff --git a/crates/server/src/init.rs b/crates/server/src/init.rs index e5f5f8fd..aac66a66 100644 --- a/crates/server/src/init.rs +++ b/crates/server/src/init.rs @@ -221,14 +221,14 @@ pub fn install_cb_listener(state: &AppState) { /// pool whenever any instance flips a setting. pub async fn spawn_config_subscriber(state: &AppState) -> anyhow::Result<()> { // Multi-instance config sync (`system_settings.value` updates → Pub/Sub). - let sub_main = fred::types::config::Config::from_url(&state.config.redis_url)?; + let sub_main = state.config.redis_config()?; let sub_main_redis: fred::clients::SubscriberClient = Builder::from_config(sub_main).build_subscriber_client()?; sub_main_redis.init().await?; dynamic_config::spawn_config_subscriber(sub_main_redis, state.dynamic_config.clone()); // Hot-reload the per-state arc-swap handles on the same channel. - let sub_filters_cfg = fred::types::config::Config::from_url(&state.config.redis_url)?; + let sub_filters_cfg = state.config.redis_config()?; let sub_filters: fred::clients::SubscriberClient = Builder::from_config(sub_filters_cfg).build_subscriber_client()?; sub_filters.init().await?; @@ -342,7 +342,7 @@ pub async fn spawn_config_subscriber(state: &AppState) -> anyhow::Result<()> { // replica's subscriber rebuilds its local `ArcSwap`. // Without this, multi-instance deployments had per-replica stale // routers between CRUD time and the next process restart. - let sub_router_cfg = fred::types::config::Config::from_url(&state.config.redis_url)?; + let sub_router_cfg = state.config.redis_config()?; let sub_router: fred::clients::SubscriberClient = Builder::from_config(sub_router_cfg).build_subscriber_client()?; sub_router.init().await?; diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 82b88575..864b2658 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -50,14 +50,19 @@ async fn main() -> anyhow::Result<()> { std::process::exit(1); } - let redis_config = Config::from_url(&config.redis_url).map_err(|e| { - tracing::error!("Invalid REDIS_URL: {e}"); - anyhow::anyhow!("Invalid REDIS_URL") - })?; + let redis_config = match config.redis_config() { + Ok(c) => c, + Err(e) => { + tracing::error!("Invalid Redis configuration: {e:#}"); + std::process::exit(1); + } + }; let redis = Builder::from_config(redis_config).build()?; if let Err(e) = redis.init().await { tracing::error!("Failed to connect to Redis: {e}"); - tracing::error!("Check REDIS_URL and ensure Redis is running"); + tracing::error!( + "Check REDIS_URL (and REDIS_CA_CERT for a rediss:// URL) and ensure Redis is running" + ); std::process::exit(1); } tracing::info!("Redis connected"); diff --git a/crates/test-support/src/lib.rs b/crates/test-support/src/lib.rs index 868fecd4..fa8db92a 100644 --- a/crates/test-support/src/lib.rs +++ b/crates/test-support/src/lib.rs @@ -8,6 +8,10 @@ //! Required env (defaults match `make infra`): //! - `TEST_DATABASE_BASE_URL` (default `postgres://thinkwatch:thinkwatch@localhost:5432`) //! - `TEST_REDIS_URL` (default `redis://localhost:6379`) +//! - `TEST_REDIS_CA_CERT` (optional): the PEM CA file for a `rediss://` +//! Redis whose certificate a private CA signed — `REDIS_CA_CERT` for +//! every Redis the tests reach (`TEST_REDIS_URL`, the cluster and TLS +//! test URLs) //! //! Tests run against the same Redis instance. Each `TestApp` FLUSHDBs //! its logical DB on spawn, so concurrent tests need separate DBs: @@ -28,7 +32,6 @@ use anyhow::Context; use fred::clients::Client as RedisClient; use fred::interfaces::ClientLike; use fred::types::Builder; -use fred::types::config::Config as RedisConfig; use sqlx::PgPool; use think_watch_common::config::AppConfig; use think_watch_server::{app, init}; @@ -157,6 +160,7 @@ impl TestApp { "redis://:225b3facaf55212ff86ad6595e6d6471@localhost:6379/1".into() }); let shared_redis = opts.redis_url.is_some(); + let redis_ca_cert = test_redis_ca_cert(); let redis_url = match &opts.redis_url { Some(url) => url.clone(), None => redis_url_for_slot(&redis_url)?, @@ -175,7 +179,7 @@ impl TestApp { // Makefile target) or under nextest, where // `redis_url_for_slot` gives each concurrently running test // its own DB. - let redis = build_redis(&redis_url).await?; + let redis = build_redis(&redis_url, redis_ca_cert.as_deref()).await?; // fred 10 doesn't expose FLUSHDB directly (only FLUSHALL), // and we don't want to nuke the dev DB. Send the raw // command so we only clear the test logical DB. @@ -220,6 +224,7 @@ impl TestApp { let config = AppConfig { database_url: db_owner.url().to_string(), redis_url, + redis_ca_cert, jwt_secret: "test-jwt-secret-with-enough-entropy-aaa".into(), // 64 hex chars = 32 bytes; valid for AES-256-GCM. encryption_key: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" @@ -442,8 +447,62 @@ fn redis_url_for_slot(redis_url: &str) -> anyhow::Result { Ok(url.to_string()) } -async fn build_redis(redis_url: &str) -> anyhow::Result { - let cfg = RedisConfig::from_url(redis_url).context("parse REDIS_URL")?; +/// `TEST_REDIS_CA_CERT`, the CA file the tests trust for a `rediss://` +/// Redis. +pub fn test_redis_ca_cert() -> Option { + std::env::var_os("TEST_REDIS_CA_CERT") + .filter(|s| !s.is_empty()) + .map(Into::into) +} + +/// The fred config for a test Redis URL, built the way the server builds +/// its own (TLS included), with `TEST_REDIS_CA_CERT`'s roots. +pub fn test_redis_config(redis_url: &str) -> fred::types::config::Config { + think_watch_common::redis_config::client_config(redis_url, test_redis_ca_cert().as_deref()) + .expect("a usable test Redis URL") +} + +/// Subscribe a fresh subscriber built from `config` to `config:changed` +/// and wait until a notice sent through `publisher` reaches it — what a +/// second instance's `init::spawn_config_subscriber` does. +/// +/// The notice is sent again until it arrives: fred's `subscribe` can +/// return while the subscription is still on its way to Redis (a +/// `PUBLISH` right after it has been seen to reach 0 receivers), and a +/// Pub/Sub message nobody is subscribed to yet is lost. +pub async fn assert_config_notice_arrives( + config: fred::types::config::Config, + publisher: &RedisClient, +) { + use fred::interfaces::{EventInterface, PubsubInterface}; + use std::time::{Duration, Instant}; + + let subscriber = Builder::from_config(config) + .build_subscriber_client() + .expect("build_subscriber_client"); + subscriber.init().await.expect("subscriber init"); + let mut rx = subscriber.message_rx(); + subscriber + .subscribe("config:changed") + .await + .expect("subscribe"); + let deadline = Instant::now() + Duration::from_secs(5); + loop { + think_watch_common::dynamic_config::notify_config_changed(publisher).await; + if let Ok(msg) = tokio::time::timeout(Duration::from_millis(200), rx.recv()).await { + assert_eq!(msg.expect("message channel").channel, "config:changed"); + return; + } + assert!(Instant::now() < deadline, "no config notice within 5 s"); + } +} + +async fn build_redis( + redis_url: &str, + ca_cert: Option<&std::path::Path>, +) -> anyhow::Result { + let cfg = think_watch_common::redis_config::client_config(redis_url, ca_cert) + .context("parse REDIS_URL")?; let client = Builder::from_config(cfg).build()?; client.init().await.context("redis init")?; Ok(client) diff --git a/crates/test-support/tests/multi_instance_config_sync.rs b/crates/test-support/tests/multi_instance_config_sync.rs index b42336d6..40e56606 100644 --- a/crates/test-support/tests/multi_instance_config_sync.rs +++ b/crates/test-support/tests/multi_instance_config_sync.rs @@ -46,8 +46,7 @@ async fn instance_b_picks_up_instance_a_setting_change_via_pubsub() { .await .expect("instance B DynamicConfig::load"), ); - let sub_cfg = fred::types::config::Config::from_url(&app.state.config.redis_url) - .expect("parse redis_url"); + let sub_cfg = app.state.config.redis_config().expect("parse redis_url"); let subscriber: fred::clients::SubscriberClient = fred::types::Builder::from_config(sub_cfg) .build_subscriber_client() .expect("build_subscriber_client"); diff --git a/crates/test-support/tests/redis_cluster.rs b/crates/test-support/tests/redis_cluster.rs index f4ab0407..94b83dc9 100644 --- a/crates/test-support/tests/redis_cluster.rs +++ b/crates/test-support/tests/redis_cluster.rs @@ -23,12 +23,16 @@ //! ``` //! //! Keys carry fresh UUIDs, so nothing is flushed and runs don't collide. +//! +//! A TLS cluster (`rediss-cluster://`) works the same way, with +//! `TEST_REDIS_CA_CERT` naming the CA of its certificates; see +//! `tests/redis_tls.rs` for how to start one. use fred::clients::Client; use fred::interfaces::{ClientLike, KeysInterface}; use fred::types::Builder; -use fred::types::config::Config; use think_watch_test_support::prelude::*; +use think_watch_test_support::test_redis_config; fn cluster_url() -> Option { let url = std::env::var("TEST_REDIS_CLUSTER_URL").ok(); @@ -39,7 +43,7 @@ fn cluster_url() -> Option { } async fn cluster(url: &str) -> Client { - let client = Builder::from_config(Config::from_url(url).unwrap()) + let client = Builder::from_config(test_redis_config(url)) .build() .unwrap(); client.init().await.unwrap(); @@ -263,20 +267,8 @@ async fn the_gateway_enforces_limits_on_a_cluster() { async fn config_change_notices_reach_a_subscriber_on_a_cluster() { // What `init::spawn_config_subscriber` does with the same URL: a // subscriber on one node hears a publish sent through another. - use fred::interfaces::{EventInterface, PubsubInterface}; let Some(url) = cluster_url() else { return }; let publisher = cluster(&url).await; - let subscriber = Builder::from_config(Config::from_url(&url).unwrap()) - .build_subscriber_client() - .unwrap(); - subscriber.init().await.unwrap(); - let mut rx = subscriber.message_rx(); - subscriber.subscribe("config:changed").await.unwrap(); - - think_watch_common::dynamic_config::notify_config_changed(&publisher).await; - let msg = tokio::time::timeout(std::time::Duration::from_secs(5), rx.recv()) - .await - .expect("a notice within 5 s") - .unwrap(); - assert_eq!(msg.channel, "config:changed"); + think_watch_test_support::assert_config_notice_arrives(test_redis_config(&url), &publisher) + .await; } diff --git a/crates/test-support/tests/redis_tls.rs b/crates/test-support/tests/redis_tls.rs new file mode 100644 index 00000000..9419ffc0 --- /dev/null +++ b/crates/test-support/tests/redis_tls.rs @@ -0,0 +1,188 @@ +//! The server against a Redis that speaks TLS only (`rediss://`). +//! +//! Managed Redis services usually require TLS. These tests point at a +//! TLS-only Redis given by `TEST_REDIS_TLS_URL` (e.g. +//! `rediss://:pw@localhost:46380`) whose certificate the CA in +//! `TEST_REDIS_CA_CERT` signed, and skip without the URL, so CI — which +//! has a plain Redis — stays green. To run them locally, make a throwaway +//! CA and a certificate for `localhost` and `127.0.0.1`, and start Redis +//! with TLS on and its plain port off: +//! +//! ```text +//! mkdir -p /tmp/tw-tls && cd /tmp/tw-tls +//! openssl req -x509 -new -nodes -newkey rsa:2048 -keyout ca.key -out ca.crt \ +//! -days 2 -subj "/CN=ThinkWatch test CA" +//! openssl req -new -nodes -newkey rsa:2048 -keyout server.key -out server.csr \ +//! -subj "/CN=localhost" +//! printf 'subjectAltName=DNS:localhost,IP:127.0.0.1\n' > san.cnf +//! openssl x509 -req -in server.csr -CA ca.crt -CAkey ca.key -CAcreateserial \ +//! -days 2 -extfile san.cnf -out server.crt +//! chmod 644 server.key # readable by the container's redis user +//! docker run -d --rm --name tw-redis-tls -p 46380:6380 -v /tmp/tw-tls:/tls:ro \ +//! redis:8-alpine redis-server --port 0 --tls-port 6380 \ +//! --tls-cert-file /tls/server.crt --tls-key-file /tls/server.key \ +//! --tls-ca-cert-file /tls/ca.crt --tls-auth-clients no --requirepass pw +//! TEST_REDIS_TLS_URL=rediss://:pw@localhost:46380 TEST_REDIS_CA_CERT=/tmp/tw-tls/ca.crt \ +//! cargo nextest run -p think-watch-test-support --test redis_tls --run-ignored only +//! ``` +//! +//! The whole suite runs over TLS the same way: `TEST_REDIS_URL` set to +//! `rediss://:pw@localhost:46380/1` with `TEST_REDIS_CA_CERT`. +//! +//! A TLS Redis Cluster (`rediss-cluster://`) runs `tests/redis_cluster.rs`. +//! Its nodes announce `127.0.0.1`, which the certificate above names. The +//! cluster bus authenticates both of its ends with that certificate, so it +//! must not be limited to server use (the commands above set no +//! `extendedKeyUsage`): +//! +//! ```text +//! docker run -d --rm --name tw-redis-cluster-tls -v /tmp/tw-tls:/tls:ro \ +//! -p 37101:37101 -p 37102:37102 -p 37103:37103 redis:8-alpine sh -c ' +//! tls="--tls-cert-file /tls/server.crt --tls-key-file /tls/server.key +//! --tls-ca-cert-file /tls/ca.crt --tls-auth-clients no" +//! for p in 37101 37102 37103; do +//! redis-server --port 0 --tls-port $p --tls-cluster yes $tls \ +//! --cluster-enabled yes --cluster-config-file n-$p.conf \ +//! --cluster-announce-ip 127.0.0.1 --protected-mode no --save "" \ +//! --daemonize yes --dir /tmp +//! done; sleep 1 +//! redis-cli --tls --cacert /tls/ca.crt --cluster create 127.0.0.1:37101 \ +//! 127.0.0.1:37102 127.0.0.1:37103 --cluster-replicas 0 --cluster-yes +//! tail -f /dev/null' +//! TEST_REDIS_CLUSTER_URL=rediss-cluster://127.0.0.1:37101 TEST_REDIS_CA_CERT=/tmp/tw-tls/ca.crt \ +//! cargo nextest run -p think-watch-test-support --test redis_cluster --run-ignored only +//! ``` +//! +//! Keys carry fresh UUIDs, so nothing is flushed and runs don't collide. + +use std::time::Duration; + +use fred::clients::Client; +use fred::interfaces::ClientLike; +use fred::types::Builder; +use think_watch_test_support::prelude::*; +use think_watch_test_support::test_redis_config; + +fn tls_url() -> Option { + let url = std::env::var("TEST_REDIS_TLS_URL").ok(); + if url.is_none() { + eprintln!("TEST_REDIS_TLS_URL not set — skipping the Redis TLS test"); + } + url +} + +async fn connect(url: &str) -> Client { + let config = test_redis_config(url); + assert!(config.uses_rustls(), "{url} is not a TLS URL"); + let client = Builder::from_config(config).build().unwrap(); + client.init().await.unwrap(); + client +} + +#[ignore = "integration test — needs TEST_REDIS_TLS_URL"] +#[tokio::test] +async fn the_limit_scripts_run_over_tls() { + let Some(url) = tls_url() else { return }; + let redis = connect(&url).await; + think_watch_test_support::redis_scripts::exercise_the_limit_scripts(&redis).await; +} + +#[ignore = "integration test — needs TEST_REDIS_TLS_URL"] +#[tokio::test] +async fn a_certificate_the_roots_do_not_cover_is_refused() { + // The test server's certificate comes from a private CA. Without + // REDIS_CA_CERT only the system's roots are trusted, so the handshake + // must fail: the certificate is checked, not merely accepted. + let Some(url) = tls_url() else { return }; + let config = think_watch_common::redis_config::client_config(&url, None).unwrap(); + let client = Builder::from_config(config).build().unwrap(); + let init = tokio::time::timeout(Duration::from_secs(10), client.init()) + .await + .expect("the handshake ends within 10 s"); + let err = init.expect_err("a certificate from an untrusted CA was accepted"); + // fred reports rustls's verdict as an I/O error; the rustls error + // inside names the certificate. + assert!(err.details().contains("InvalidCertificate"), "{err:?}"); +} + +#[ignore = "integration test — needs TEST_REDIS_TLS_URL"] +#[tokio::test] +async fn the_gateway_runs_over_tls() { + let Some(url) = tls_url() else { return }; + let app = TestApp::try_spawn_with(SpawnOptions { + redis_url: Some(url), + ..Default::default() + }) + .await + .unwrap(); + assert!(app.state.config.redis_config().unwrap().uses_rustls()); + + let user = fixtures::create_random_user(&app.db).await.unwrap(); + let mock = MockProvider::openai_chat_ok("gpt-test").await; + let provider = fixtures::create_provider( + &app.db, + &unique_name("tls-prov"), + "openai", + &mock.uri(), + None, + ) + .await + .unwrap(); + fixtures::create_model_and_route(&app.db, provider.id, "gpt-test") + .await + .unwrap(); + app.rebuild_gateway_router().await; + + let gw = app.gateway_client(); + let ready: Json = gw + .get("/health/ready") + .await + .unwrap() + .assert_ok() + .json() + .unwrap(); + assert_eq!(ready["redis"], true, "{ready}"); + + // A request limit lives in Redis: two pass, the third is refused. + fixtures::create_rate_limit_rule( + &app.db, + "user", + user.user.id, + "ai_gateway", + "requests", + 60, + 2, + ) + .await + .unwrap(); + let key = fixtures::create_api_key( + &app.db, + user.user.id, + &unique_name("tls-key"), + &["ai_gateway"], + None, + None, + ) + .await + .unwrap(); + gw.set_bearer(&key.plaintext); + let chat = json!({"model": "gpt-test", + "messages": [{"role": "user", "content": Uuid::new_v4().to_string()}]}); + for _ in 0..2 { + gw.post("/v1/chat/completions", chat.clone()) + .await + .unwrap() + .assert_ok(); + } + let r = gw.post("/v1/chat/completions", chat).await.unwrap(); + assert_eq!(r.status.as_u16(), 429, "body={}", r.text()); + + // Config change notices between instances: a subscriber built the + // way `init::spawn_config_subscriber` builds its three hears a + // publish sent through the server's own client. + think_watch_test_support::assert_config_notice_arrives( + app.state.config.redis_config().unwrap(), + &app.state.redis, + ) + .await; +} diff --git a/deploy/helm/think-watch/README.md b/deploy/helm/think-watch/README.md index 06478b71..5efc586d 100644 --- a/deploy/helm/think-watch/README.md +++ b/deploy/helm/think-watch/README.md @@ -97,6 +97,55 @@ Rate limits, budgets, route health, caches and the config change notices between instances are tested against a three-primary Redis 8 cluster (`crates/test-support/tests/redis_cluster.rs`). +### Redis over TLS + +Managed Redis services usually require TLS: ElastiCache with in-transit +encryption, Upstash, Azure Cache for Redis, Redis Cloud. Give the URL +the `rediss://` scheme — `rediss-cluster://` for a cluster: + +```yaml +redis: + bundled: false + externalUrl: rediss://:pass@master.my-cache.abc123.use1.cache.amazonaws.com:6379 +``` + +- The server checks the Redis certificate against the public CAs, the + same roots it trusts for upstream HTTPS, and against the host name in + the URL. Use the endpoint name the service gives, not an IP address, + unless the certificate names that address. +- In a cluster, each node is reached at the address it announces, and + its certificate must name that address — the host name, or the IP + address when the node announces one. A cluster whose nodes announce + IP addresses that their certificates do not name cannot be used over + TLS. +- The port is whatever the service uses for TLS (Azure Cache for Redis: + `6380`). With `networkPolicy.enabled`, the chart's egress rule lets + the server reach Redis on `6379` only. + +A self-hosted Redis whose certificate a private CA signed needs that +CA. Put its PEM certificate in a Secret and name it; the server then +trusts only the certificates in it for Redis: + +```bash +kubectl -n thinkwatch create secret generic redis-ca --from-file=ca.crt=./ca.crt +``` + +```yaml +redis: + bundled: false + externalUrl: rediss://:pass@redis.internal:6379 + caSecret: + name: redis-ca + key: ca.crt +``` + +The chart mounts the key at `/etc/thinkwatch/redis-ca/` and sets +`REDIS_CA_CERT` to it. The server reads it at start, so restart the +server pods after changing the Secret. Outside the chart, set +`REDIS_CA_CERT` to the PEM file's path yourself. Client certificates +(mutual TLS) are not supported: give such a Redis `tls-auth-clients no` +and authenticate with the password. + ## Rotating secrets `-secrets` is kept on `helm uninstall`. To rotate passwords: diff --git a/deploy/helm/think-watch/templates/deployment.yaml b/deploy/helm/think-watch/templates/deployment.yaml index c2be4980..13c47193 100644 --- a/deploy/helm/think-watch/templates/deployment.yaml +++ b/deploy/helm/think-watch/templates/deployment.yaml @@ -95,6 +95,17 @@ spec: name: {{ .Release.Name }}-config - secretRef: name: {{ .Release.Name }}-secrets + {{- with .Values.redis.caSecret }} + {{- if .name }} + env: + - name: REDIS_CA_CERT + value: /etc/thinkwatch/redis-ca/{{ .key }} + volumeMounts: + - name: redis-ca + mountPath: /etc/thinkwatch/redis-ca + readOnly: true + {{- end }} + {{- end }} startupProbe: httpGet: path: /health/live @@ -118,6 +129,17 @@ spec: failureThreshold: 3 resources: {{- toYaml .Values.resources.server | nindent 12 }} + {{- with .Values.redis.caSecret }} + {{- if .name }} + volumes: + - name: redis-ca + secret: + secretName: {{ .name }} + items: + - key: {{ .key }} + path: {{ .key }} + {{- end }} + {{- end }} --- apiVersion: apps/v1 kind: Deployment diff --git a/deploy/helm/think-watch/values.yaml b/deploy/helm/think-watch/values.yaml index e44ac6ae..38b9509a 100644 --- a/deploy/helm/think-watch/values.yaml +++ b/deploy/helm/think-watch/values.yaml @@ -91,8 +91,17 @@ postgres: redis: bundled: true image: redis:8-alpine - # Used only when bundled=false; e.g. redis://:pass@host:6379 + # Used only when bundled=false; e.g. redis://:pass@host:6379, or + # rediss://:pass@host:6379 for a Redis that requires TLS (see README.md) externalUrl: "" + # For a rediss:// externalUrl whose certificate a private CA signed: + # an existing Secret in the release namespace holding that CA's PEM + # certificate(s) under `key` (cert-manager keeps it under `ca.crt`). + # The server then trusts only those certificates for Redis. Empty = + # the public CAs, which is what managed services need. + caSecret: + name: "" + key: ca.crt storage: 2Gi storageClassName: "" resources: