diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a6f2a6ea9..3e2b82d44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -152,12 +152,21 @@ jobs: # These tests are ignored in the default local lane because they require # a redis-server executable. CI installs Redis above and invokes them - # explicitly so the Lua scripts, delete fencing, NOSCRIPT reload, and - # connection recovery are verified against a real server on every PR. - - name: Redis compression state (Lua fencing + recovery) + # explicitly so secure DSN semantics, Lua scripts, delete fencing, + # NOSCRIPT reload, and connection recovery are verified against real + # server processes on every PR. + - name: Redis live state (secure DSN + Lua fencing + recovery) if: steps.filter.outputs.code == 'true' run: | redis-server --version + tls_probe_output="$(redis-server --port 0 --tls-port 1 2>&1 || true)" + if ! grep -q 'No tls-cert-file configured' <<<"${tls_probe_output}"; then + echo "::error::redis-server lacks the TLS support required by secure Redis tests" + exit 1 + fi + unset tls_probe_output + cargo test -p sbproxy-platform --test redis_secure -- \ + --ignored --nocapture --test-threads=1 cargo test -p sbproxy-core --locked --lib \ 'compression_store::redis::tests::live_redis_' -- \ --ignored --test-threads=1 diff --git a/Cargo.lock b/Cargo.lock index ce6cfc60f..112ba5ef3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7652,13 +7652,14 @@ dependencies = [ [[package]] name = "redis" -version = "0.28.2" +version = "0.31.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e37ec3fd44bea2ec947ba6cc7634d7999a6590aca7c35827c250bc0de502bda6" +checksum = "0bc1ea653e0b2e097db3ebb5b7f678be339620b8041f66b30a308c1d45d36a7f" dependencies = [ "arc-swap", "backon", "bytes", + "cfg-if", "combine", "futures-channel", "futures-util", @@ -7667,10 +7668,9 @@ dependencies = [ "percent-encoding", "pin-project-lite", "rustls", - "rustls-native-certs 0.7.3", - "rustls-pemfile", - "rustls-pki-types", + "rustls-native-certs 0.8.3", "ryu", + "socket2 0.5.10", "tokio", "tokio-rustls", "tokio-util", @@ -8508,6 +8508,7 @@ dependencies = [ "bytes", "compact_str 0.8.1", "http 1.4.0", + "rcgen", "sbproxy-capability", "sbproxy-observe", "sbproxy-platform", @@ -8557,6 +8558,7 @@ dependencies = [ "pingora-proxy", "prometheus 0.14.0", "rand 0.8.6", + "rcgen", "regex", "reqwest 0.12.28", "rustls", @@ -8984,9 +8986,13 @@ dependencies = [ "dashmap", "hex", "parking_lot", + "prometheus 0.14.0", + "rcgen", "redb", "redis", "rusqlite", + "rustls", + "rustls-pki-types", "sbproxy-plugin", "serde", "serde_json", @@ -8994,6 +9000,7 @@ dependencies = [ "tokio", "tracing", "ureq", + "url", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 5ba1c5f31..8e508fe91 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -232,7 +232,7 @@ tokio-util = { version = "0.7", features = ["rt"] } # dep so sbproxy-modules can drive XREAD directly for feature feeds (WAF # rule subscription) without going through the KVStore facade. # `streams` enables the XREAD typed reply parser (StreamReadReply). -redis = { version = "0.28.1", default-features = false, features = ["tokio-comp", "streams"] } +redis = { version = "0.31.0", default-features = false, features = ["tokio-comp", "streams"] } # Bloom filter bloomfilter = "1" diff --git a/NOTICE b/NOTICE index 8eb7b8cde..6a0a90bde 100644 --- a/NOTICE +++ b/NOTICE @@ -214,6 +214,13 @@ Crates: ciborium, ciborium-io, ciborium-ll. =========================================================================== +backon +Copyright the backon contributors +Licensed under the Apache License, Version 2.0 +https://github.com/Xuanwo/backon + +=========================================================================== + The following crates are pulled in only by the embedded model-host engine (mistral.rs, behind the off-by-default `embedded` cargo feature) and are licensed under the Apache License, Version 2.0. Where a crate is diff --git a/crates/sbproxy-config/Cargo.toml b/crates/sbproxy-config/Cargo.toml index 7c5d3239a..a1a9d05c1 100644 --- a/crates/sbproxy-config/Cargo.toml +++ b/crates/sbproxy-config/Cargo.toml @@ -32,6 +32,7 @@ tempfile = "3" schemars = { workspace = true } [dev-dependencies] +rcgen = { workspace = true } serde_json = { workspace = true } tempfile = "3" tokio = { workspace = true } diff --git a/crates/sbproxy-config/src/compiler.rs b/crates/sbproxy-config/src/compiler.rs index 832c9ae22..f02d81fdf 100644 --- a/crates/sbproxy-config/src/compiler.rs +++ b/crates/sbproxy-config/src/compiler.rs @@ -4,6 +4,8 @@ //! performance-optimized `CompiledConfig` / `CompiledOrigin` types that //! the proxy runtime works with. +use std::fs::File; +use std::io::Read; use std::sync::Arc; use anyhow::{Context, Result}; @@ -14,18 +16,22 @@ use sbproxy_platform::messenger::redis::RedisMessengerConfig; use sbproxy_platform::messenger::{ GcpPubSubMessenger, MemoryMessenger, Messenger, RedisMessenger, SqsMessenger, }; -use sbproxy_platform::storage::{KVStore, RedisConfig, RedisKVStore}; +use sbproxy_platform::storage::{ + KVStore, RedisConfig, RedisKVStore, RedisTlsConfig, ValidatedRedisConnection, +}; use smallvec::SmallVec; use crate::snapshot::{CompiledConfig, CompiledOrigin}; -use crate::types::{ConfigFile, L2CacheConfig, MessengerSettings, RawOriginConfig}; +use crate::types::{ConfigFile, L2CacheConfig, L2CacheParams, MessengerSettings, RawOriginConfig}; + +const MAX_REDIS_TLS_FILE_BYTES: u64 = 1_048_576; /// Extract the Redis host:port pair from a DSN like `redis://host:6379/0`. /// /// Accepts either a bare `host:port` form (as used by the raw RESP client) /// or a `redis://[user[:pass]@]host:port[/db]` URL. The database index is /// ignored since the single-connection RESP client does not issue SELECT. -fn parse_redis_addr(dsn: &str) -> Result { +fn parse_redis_messenger_addr(dsn: &str) -> Result { let s = dsn.trim(); let without_scheme = s .strip_prefix("redis://") @@ -56,22 +62,62 @@ fn parse_redis_addr(dsn: &str) -> Result { } } +fn read_redis_tls_file(path: &str, error: &'static str) -> Result> { + let file = File::open(path).map_err(|_| anyhow::anyhow!(error))?; + let mut bytes = Vec::new(); + file.take(MAX_REDIS_TLS_FILE_BYTES + 1) + .read_to_end(&mut bytes) + .map_err(|_| anyhow::anyhow!(error))?; + if bytes.is_empty() || bytes.len() as u64 > MAX_REDIS_TLS_FILE_BYTES { + return Err(anyhow::anyhow!(error)); + } + Ok(bytes) +} + +fn read_optional_redis_tls_file( + path: Option<&str>, + error: &'static str, +) -> Result>> { + path.map(|path| read_redis_tls_file(path, error)) + .transpose() +} + +/// Compile the Redis connection shared by the blocking L2 store and async +/// compression state without opening a network connection. +/// +/// # Errors +/// +/// Returns a static, redacted error when the DSN, TLS field combination, file +/// contents, or client identity is invalid. +pub fn build_l2_redis_connection(params: &L2CacheParams) -> Result { + let tls = RedisTlsConfig { + root_cert: read_optional_redis_tls_file( + params.ca_file.as_deref(), + "invalid Redis ca_file configuration", + )?, + client_cert: read_optional_redis_tls_file( + params.cert_file.as_deref(), + "invalid Redis cert_file configuration", + )?, + client_key: read_optional_redis_tls_file( + params.key_file.as_deref(), + "invalid Redis key_file configuration", + )?, + }; + ValidatedRedisConnection::new(¶ms.dsn, tls) +} + /// Build a concrete `KVStore` for the given L2 cache config. /// /// # Errors /// -/// Returns an error if the configured `driver` is not recognized, or if -/// the `redis` driver is selected and its DSN cannot be parsed into a -/// `host:port` address. +/// Returns an error if the configured `driver` is not recognized or if the +/// Redis connection and TLS configuration is invalid. pub fn build_l2_store(cfg: &L2CacheConfig) -> Result> { match cfg.driver.as_str() { "redis" => { - let addr = parse_redis_addr(&cfg.params.dsn) - .with_context(|| format!("invalid redis DSN '{}'", cfg.params.dsn))?; - Ok(Arc::new(RedisKVStore::new(RedisConfig { - addr, - ..RedisConfig::default() - }))) + let connection = build_l2_redis_connection(&cfg.params)?; + Ok(Arc::new(RedisKVStore::new(RedisConfig::new(connection)))) } other => anyhow::bail!("unsupported l2_cache driver: '{}'", other), } @@ -115,7 +161,7 @@ pub fn build_messenger(settings: &MessengerSettings) -> Result Result { } } - // Instantiate the L2 cache backend (Redis) if configured. The store is - // created lazily, so this call just records the target address without - // opening a connection yet. Any concrete failure surfaces the first - // time a request tries to use it. + // Instantiate the L2 cache backend (Redis) if configured. DSN semantics, + // TLS field combinations, and local PEM material are validated here at + // startup. Only the network connection is lazy; reachability, TLS handshake, + // authentication, and database-selection failures surface on first use. let l2_store = match &config_file.proxy.l2_cache { Some(cfg) => Some(build_l2_store(cfg)?), None => None, diff --git a/crates/sbproxy-config/src/types.rs b/crates/sbproxy-config/src/types.rs index 0a65b08c6..c2cc0c008 100644 --- a/crates/sbproxy-config/src/types.rs +++ b/crates/sbproxy-config/src/types.rs @@ -2150,12 +2150,35 @@ pub struct L2CacheConfig { /// /// Kept separate from `L2CacheConfig` so future drivers can add fields /// (auth, pool size) without churning the parent struct. -#[derive(Debug, Clone, Deserialize, Serialize, Default, schemars::JsonSchema)] +#[derive(Clone, Deserialize, Serialize, Default, schemars::JsonSchema)] pub struct L2CacheParams { - /// Connection DSN. For `redis` drivers this is a `redis://host:port[/db]` - /// URL. Only the host:port portion is parsed today; the DB index is ignored. + /// Redis connection DSN. Supports `redis://`, `rediss://`, credentials, + /// bracketed IPv6 addresses, and a non-negative logical database. #[serde(default)] pub dsn: String, + /// Optional path to PEM-encoded Redis trust anchors for a private CA. + #[serde(default)] + pub ca_file: Option, + /// Optional path to a PEM-encoded Redis client certificate chain. + /// Must be configured together with `key_file` and requires `rediss://`. + #[serde(default)] + pub cert_file: Option, + /// Optional path to the PEM-encoded Redis client private key. + /// Must be configured together with `cert_file` and requires `rediss://`. + #[serde(default)] + pub key_file: Option, +} + +impl std::fmt::Debug for L2CacheParams { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("L2CacheParams") + .field("dsn_configured", &!self.dsn.is_empty()) + .field("ca_file_configured", &self.ca_file.is_some()) + .field("cert_file_configured", &self.cert_file.is_some()) + .field("key_file_configured", &self.key_file.is_some()) + .finish() + } } // --- Cache Reserve Config --- diff --git a/crates/sbproxy-config/tests/l2_redis.rs b/crates/sbproxy-config/tests/l2_redis.rs new file mode 100644 index 000000000..2de343c33 --- /dev/null +++ b/crates/sbproxy-config/tests/l2_redis.rs @@ -0,0 +1,341 @@ +use std::path::Path; + +use anyhow::Error; +use sbproxy_config::{build_l2_redis_connection, L2CacheConfig, L2CacheParams, ProxyServerConfig}; + +const MAX_REDIS_TLS_FILE_BYTES: usize = 1_048_576; + +fn certificate_and_key(name: &str) -> (Vec, Vec) { + let key = rcgen::KeyPair::generate().expect("generate test key"); + let params = rcgen::CertificateParams::new(vec![name.to_string()]) + .expect("create test certificate parameters"); + let certificate = params + .self_signed(&key) + .expect("self-sign test certificate"); + ( + certificate.pem().into_bytes(), + key.serialize_pem().into_bytes(), + ) +} + +fn write_file(path: &Path, bytes: &[u8]) -> String { + std::fs::write(path, bytes).expect("write test fixture"); + path.to_string_lossy().into_owned() +} + +fn params(dsn: &str) -> L2CacheParams { + L2CacheParams { + dsn: dsn.to_string(), + ..L2CacheParams::default() + } +} + +fn assert_safe_error(error: &Error, expected: &str, forbidden: &[&str]) { + assert_eq!(error.to_string(), expected); + let chain = format!("{error:#}"); + for value in forbidden { + assert!( + !chain.contains(value), + "error chain exposed forbidden Redis configuration material: {chain}" + ); + } +} + +#[test] +fn l2_redis_params_debug_exposes_only_configuration_presence() { + let params = L2CacheParams { + dsn: "rediss://sentinel-user:sentinel-password@sentinel-host.invalid:6380/7".to_string(), + ca_file: Some("/sentinel/tls/ca.pem".to_string()), + cert_file: Some("/sentinel/tls/client.pem".to_string()), + key_file: Some("/sentinel/tls/client-key.pem".to_string()), + }; + + assert_eq!( + format!("{params:?}"), + "L2CacheParams { dsn_configured: true, ca_file_configured: true, cert_file_configured: true, key_file_configured: true }" + ); +} + +#[test] +fn l2_redis_enclosing_config_debug_uses_redacted_params() { + let config = L2CacheConfig { + driver: "redis".to_string(), + params: L2CacheParams { + dsn: "rediss://sentinel-user:sentinel-password@sentinel-host.invalid:6380/7" + .to_string(), + ca_file: Some("/sentinel/tls/ca.pem".to_string()), + cert_file: Some("/sentinel/tls/client.pem".to_string()), + key_file: Some("/sentinel/tls/client-key.pem".to_string()), + }, + }; + + let debug = format!("{config:?}"); + assert_eq!( + debug, + "L2CacheConfig { driver: \"redis\", params: L2CacheParams { dsn_configured: true, ca_file_configured: true, cert_file_configured: true, key_file_configured: true } }" + ); + for forbidden in [ + "sentinel-user", + "sentinel-password", + "sentinel-host", + "/7", + "/sentinel/tls", + ] { + assert!( + !debug.contains(forbidden), + "enclosing config Debug exposed Redis material: {debug}" + ); + } +} + +#[test] +fn l2_redis_deserializes_tls_file_fields() { + let yaml = r#" +proxy: + l2_cache_settings: + driver: redis + params: + dsn: rediss://default:p%40ss@[::1]:6380/7 + ca_file: /tmp/redis-ca.pem + cert_file: /tmp/redis-client.pem + key_file: /tmp/redis-client-key.pem +"#; + + let config: sbproxy_config::ConfigFile = serde_yaml::from_str(yaml).expect("parse config"); + let l2 = config.proxy.l2_cache.expect("L2 cache settings"); + assert_eq!(l2.params.dsn, "rediss://default:p%40ss@[::1]:6380/7"); + assert_eq!(l2.params.ca_file.as_deref(), Some("/tmp/redis-ca.pem")); + assert_eq!( + l2.params.cert_file.as_deref(), + Some("/tmp/redis-client.pem") + ); + assert_eq!( + l2.params.key_file.as_deref(), + Some("/tmp/redis-client-key.pem") + ); +} + +#[test] +fn l2_redis_schema_exposes_tls_file_fields() { + let schema = schemars::schema_for!(ProxyServerConfig); + let json = serde_json::to_string(&schema).expect("serialize schema"); + + for field in ["ca_file", "cert_file", "key_file"] { + assert!(json.contains(&format!("\"{field}\"")), "missing {field}"); + } +} + +#[test] +fn l2_redis_compiles_private_ca_and_mtls_without_network_io() { + let directory = tempfile::tempdir().expect("create test directory"); + let (certificate, key) = certificate_and_key("redis-client.example"); + let mut config = params("rediss://default:p%40ss@[::1]:6380/7"); + config.ca_file = Some(write_file(&directory.path().join("ca.pem"), &certificate)); + config.cert_file = Some(write_file( + &directory.path().join("client.pem"), + &certificate, + )); + config.key_file = Some(write_file(&directory.path().join("client-key.pem"), &key)); + + let connection = build_l2_redis_connection(&config) + .expect("valid private CA and client identity must compile"); + + assert!(connection.uses_tls()); +} + +#[test] +fn l2_redis_rejects_tls_files_for_plaintext_connections_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let (certificate, _) = certificate_and_key("sentinel-plaintext-certificate.example"); + let path = directory.path().join("sentinel-plaintext-ca.pem"); + let dsn = "redis://default:sentinel-plaintext-password@sentinel-plaintext-host.invalid:6379/7"; + let mut config = params(dsn); + config.ca_file = Some(write_file(&path, &certificate)); + + let error = build_l2_redis_connection(&config).expect_err("plaintext TLS files must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-plaintext", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_certificate_without_key_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let (certificate, _) = certificate_and_key("sentinel-cert-only.example"); + let path = directory.path().join("sentinel-cert-only.pem"); + let dsn = "rediss://default:sentinel-cert-only-password@sentinel-cert-only-host.invalid:6380/7"; + let mut config = params(dsn); + config.cert_file = Some(write_file(&path, &certificate)); + + let error = build_l2_redis_connection(&config).expect_err("one-sided identity must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-cert-only", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_key_without_certificate_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let (_, key) = certificate_and_key("sentinel-key-only.example"); + let path = directory.path().join("sentinel-key-only.pem"); + let dsn = "rediss://default:sentinel-key-only-password@sentinel-key-only-host.invalid:6380/7"; + let mut config = params(dsn); + config.key_file = Some(write_file(&path, &key)); + + let error = build_l2_redis_connection(&config).expect_err("one-sided identity must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-key-only", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_missing_file_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let path = directory.path().join("sentinel-missing-ca.pem"); + let dsn = "rediss://default:sentinel-missing-password@sentinel-missing-host.invalid:6380/7"; + let mut config = params(dsn); + config.ca_file = Some(path.to_string_lossy().into_owned()); + + let error = build_l2_redis_connection(&config).expect_err("missing TLS file must fail"); + + assert_safe_error( + &error, + "invalid Redis ca_file configuration", + &[dsn, "sentinel-missing", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_empty_file_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let path = directory.path().join("sentinel-empty-ca.pem"); + let dsn = "rediss://default:sentinel-empty-password@sentinel-empty-host.invalid:6380/7"; + let mut config = params(dsn); + config.ca_file = Some(write_file(&path, b"")); + + let error = build_l2_redis_connection(&config).expect_err("empty TLS file must fail"); + + assert_safe_error( + &error, + "invalid Redis ca_file configuration", + &[dsn, "sentinel-empty", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_oversized_file_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let path = directory.path().join("sentinel-oversized-ca.pem"); + let dsn = "rediss://default:sentinel-oversized-password@sentinel-oversized-host.invalid:6380/7"; + let mut config = params(dsn); + let oversized = vec![b'x'; MAX_REDIS_TLS_FILE_BYTES + 1]; + config.ca_file = Some(write_file(&path, &oversized)); + + let error = build_l2_redis_connection(&config).expect_err("oversized TLS file must fail"); + + assert_safe_error( + &error, + "invalid Redis ca_file configuration", + &[dsn, "sentinel-oversized", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_malformed_file_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let path = directory.path().join("sentinel-malformed-ca.pem"); + let dsn = "rediss://default:sentinel-malformed-password@sentinel-malformed-host.invalid:6380/7"; + let mut config = params(dsn); + config.ca_file = Some(write_file( + &path, + b"-----BEGIN CERTIFICATE-----\nsentinel-malformed-content\n", + )); + + let error = build_l2_redis_connection(&config).expect_err("malformed TLS file must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-malformed", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_mismatched_identity_without_disclosure() { + let directory = tempfile::tempdir().expect("create test directory"); + let (certificate, _) = certificate_and_key("sentinel-mismatched-client.example"); + let (_, other_key) = certificate_and_key("sentinel-mismatched-other.example"); + let cert_path = directory.path().join("sentinel-mismatched-cert.pem"); + let key_path = directory.path().join("sentinel-mismatched-key.pem"); + let dsn = + "rediss://default:sentinel-mismatched-password@sentinel-mismatched-host.invalid:6380/7"; + let mut config = params(dsn); + config.cert_file = Some(write_file(&cert_path, &certificate)); + config.key_file = Some(write_file(&key_path, &other_key)); + + let error = build_l2_redis_connection(&config).expect_err("mismatched identity must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-mismatched", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_query_without_disclosure() { + let dsn = "redis://default:sentinel-query-password@sentinel-query-host.invalid:6379/7?sentinel-query=true"; + let error = build_l2_redis_connection(¶ms(dsn)).expect_err("query must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-query", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_fragment_without_disclosure() { + let dsn = "rediss://default:sentinel-fragment-password@sentinel-fragment-host.invalid:6380/7#sentinel-fragment"; + let error = build_l2_redis_connection(¶ms(dsn)).expect_err("fragment must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-fragment", "/7"], + ); +} + +#[test] +fn l2_redis_rejects_negative_database_without_disclosure() { + let dsn = "redis://default:sentinel-negative-password@sentinel-negative-host.invalid:6379/-1"; + let error = build_l2_redis_connection(¶ms(dsn)).expect_err("negative database must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-negative", "/-1"], + ); +} + +#[test] +fn l2_redis_rejects_username_without_password_without_disclosure() { + let dsn = "redis://sentinel-username@sentinel-username-host.invalid:6379/7"; + let error = + build_l2_redis_connection(¶ms(dsn)).expect_err("username without password must fail"); + + assert_safe_error( + &error, + "invalid Redis connection configuration", + &[dsn, "sentinel-username", "/7"], + ); +} diff --git a/crates/sbproxy-config/tests/validate_examples.rs b/crates/sbproxy-config/tests/validate_examples.rs index 1b66edd84..a774a724a 100644 --- a/crates/sbproxy-config/tests/validate_examples.rs +++ b/crates/sbproxy-config/tests/validate_examples.rs @@ -4,6 +4,41 @@ //! test catches that on every CI run. use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +struct RedisTlsExampleFixtures { + _directory: tempfile::TempDir, + ca_file: String, + cert_file: String, + key_file: String, +} + +fn redis_tls_example_fixtures() -> &'static RedisTlsExampleFixtures { + static FIXTURES: OnceLock = OnceLock::new(); + FIXTURES.get_or_init(|| { + let directory = tempfile::tempdir().expect("create Redis TLS example fixture directory"); + let key = rcgen::KeyPair::generate().expect("generate Redis TLS example key"); + let certificate = rcgen::CertificateParams::new(vec!["redis-client.example".to_string()]) + .expect("create Redis TLS example certificate parameters") + .self_signed(&key) + .expect("self-sign Redis TLS example certificate"); + + let ca_file = directory.path().join("ca.pem"); + let cert_file = directory.path().join("client.pem"); + let key_file = directory.path().join("client.key"); + std::fs::write(&ca_file, certificate.pem()).expect("write Redis TLS example CA"); + std::fs::write(&cert_file, certificate.pem()) + .expect("write Redis TLS example client certificate"); + std::fs::write(&key_file, key.serialize_pem()).expect("write Redis TLS example client key"); + + RedisTlsExampleFixtures { + _directory: directory, + ca_file: ca_file.to_string_lossy().into_owned(), + cert_file: cert_file.to_string_lossy().into_owned(), + key_file: key_file.to_string_lossy().into_owned(), + } + }) +} fn examples_root() -> PathBuf { // sbproxy-config lives at crates/sbproxy-config/ inside the workspace. @@ -73,10 +108,16 @@ fn export_example_env_dummies() { ), ("ENV_VAR", "dummy"), ("VAR", "dummy"), + ("REDIS_PASSWORD", "redis-example-dummy"), ]; for (k, v) in DUMMIES { std::env::set_var(k, v); } + + let redis = redis_tls_example_fixtures(); + std::env::set_var("REDIS_CA_FILE", &redis.ca_file); + std::env::set_var("REDIS_CLIENT_CERT_FILE", &redis.cert_file); + std::env::set_var("REDIS_CLIENT_KEY_FILE", &redis.key_file); } #[test] diff --git a/crates/sbproxy-core/Cargo.toml b/crates/sbproxy-core/Cargo.toml index aa7187d7d..e0ba8406e 100644 --- a/crates/sbproxy-core/Cargo.toml +++ b/crates/sbproxy-core/Cargo.toml @@ -198,5 +198,6 @@ base64.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["full", "test-util"] } tracing-subscriber.workspace = true +rcgen.workspace = true tempfile = "3" loom = "0.7" diff --git a/crates/sbproxy-core/src/admin_compression.rs b/crates/sbproxy-core/src/admin_compression.rs index e5e1bced6..e7fbff510 100644 --- a/crates/sbproxy-core/src/admin_compression.rs +++ b/crates/sbproxy-core/src/admin_compression.rs @@ -128,9 +128,10 @@ impl CompressionAdminRegistry { .iter() .any(|entry| entry.backend == CompressionBackend::Redis) { - if let Some(store) = - crate::compression_runtime::redis_admin_store(&pipeline.config.server) - { + if let Some(store) = crate::compression_runtime::redis_admin_store( + &pipeline.config.server, + pipeline.config.l2_store.as_deref(), + ) { stores.push(AdminStore { backend: CompressionBackend::Redis, store, @@ -1080,12 +1081,16 @@ mod tests { #[test] fn disabled_summary_policy_keeps_the_configured_redis_admin_store() { let mut pipeline = crate::pipeline::CompiledPipeline::default(); - pipeline.config.server.l2_cache = Some(sbproxy_config::L2CacheConfig { + let l2_config = sbproxy_config::L2CacheConfig { driver: "redis".to_string(), params: sbproxy_config::L2CacheParams { dsn: "redis://redis.internal:6379/0".to_string(), + ..sbproxy_config::L2CacheParams::default() }, - }); + }; + pipeline.config.l2_store = + Some(sbproxy_config::build_l2_store(&l2_config).expect("compile general L2 store")); + pipeline.config.server.l2_cache = Some(l2_config); let registry = CompressionAdminRegistry::from_pipeline(&pipeline); assert_eq!( diff --git a/crates/sbproxy-core/src/compression_runtime.rs b/crates/sbproxy-core/src/compression_runtime.rs index 81f1b9ede..2c9cf1e97 100644 --- a/crates/sbproxy-core/src/compression_runtime.rs +++ b/crates/sbproxy-core/src/compression_runtime.rs @@ -10,7 +10,7 @@ use sbproxy_ai::compression::{ SummarizerError, SummaryBufferLever, WindowFitLever, }; use sbproxy_ai::{AiClient, AiHandlerConfig, ProviderConfig}; -use sbproxy_platform::storage::{AsyncRedisConfig, AsyncRedisKVStore}; +use sbproxy_platform::storage::{AsyncRedisConfig, AsyncRedisKVStore, KVStore}; use std::fmt; use std::sync::Arc; use std::time::Duration; @@ -25,9 +25,10 @@ struct RuntimeDependencies { impl RuntimeDependencies { fn from_process( server: &sbproxy_config::ProxyServerConfig, + l2_store: Option<&dyn KVStore>, redis_required: bool, ) -> anyhow::Result { - let redis = redis_dependency(server, redis_required)?; + let redis = redis_dependency(server, l2_store, redis_required)?; let cluster = crate::cluster::current_cluster_handle(); let writer_node = cluster .as_ref() @@ -42,9 +43,10 @@ impl RuntimeDependencies { fn for_validation( server: &sbproxy_config::ProxyServerConfig, + l2_store: Option<&dyn KVStore>, redis_required: bool, ) -> anyhow::Result { - let redis = redis_dependency(server, redis_required)?; + let redis = redis_dependency(server, l2_store, redis_required)?; let writer_node = server .cluster .as_ref() @@ -69,6 +71,7 @@ impl RuntimeDependencies { fn redis_dependency( server: &sbproxy_config::ProxyServerConfig, + l2_store: Option<&dyn KVStore>, required: bool, ) -> anyhow::Result>> { if !required { @@ -76,10 +79,13 @@ fn redis_dependency( } match server.l2_cache.as_ref() { Some(config) if config.driver == "redis" => { - validate_redis_dsn(&config.params.dsn)?; - Ok(Some(AsyncRedisKVStore::new(AsyncRedisConfig::new( - &config.params.dsn, - )))) + let connection = l2_store + .and_then(KVStore::validated_redis_connection) + .ok_or_else(|| { + anyhow::anyhow!("Redis compression state has invalid connection configuration") + })?; + let async_config = AsyncRedisConfig::from_connection(connection); + Ok(Some(AsyncRedisKVStore::new(async_config))) } _ => Ok(None), } @@ -89,26 +95,13 @@ fn redis_dependency( /// no active origin currently enables `summary_buffer`. pub(crate) fn redis_admin_store( server: &sbproxy_config::ProxyServerConfig, + l2_store: Option<&dyn KVStore>, ) -> Option> { - let redis = redis_dependency(server, true).ok().flatten()?; + let redis = redis_dependency(server, l2_store, true).ok().flatten()?; let store = RedisCompressionStore::new(redis, RedisCompressionStoreConfig::default()).ok()?; Some(Arc::new(store)) } -fn validate_redis_dsn(dsn: &str) -> anyhow::Result<()> { - let parsed = url::Url::parse(dsn).context("Redis compression state has an invalid DSN")?; - if !matches!(parsed.scheme(), "redis" | "rediss") - || parsed.host_str().is_none() - || parsed.fragment().is_some() - { - bail!("Redis compression state has an invalid DSN"); - } - AsyncRedisConfig::new(dsn) - .validate() - .map_err(|_| anyhow::anyhow!("Redis compression state has an invalid DSN"))?; - Ok(()) -} - /// Immutable per-origin compression dependencies held by a pipeline snapshot. pub struct CompressionRuntime { policy: CompressionPolicy, @@ -156,10 +149,11 @@ impl CompressionRuntimeRegistry { /// Bind every non-empty effective AI policy to current process dependencies. pub fn from_process( server: &sbproxy_config::ProxyServerConfig, + l2_store: Option<&dyn KVStore>, actions: &[sbproxy_modules::Action], ) -> anyhow::Result { let dependencies = - RuntimeDependencies::from_process(server, actions_require_redis(actions))?; + RuntimeDependencies::from_process(server, l2_store, actions_require_redis(actions))?; Self::with_dependencies(actions, dependencies) } @@ -167,10 +161,11 @@ impl CompressionRuntimeRegistry { /// process-global cluster state. The returned registry is discard-only. pub(crate) fn for_validation( server: &sbproxy_config::ProxyServerConfig, + l2_store: Option<&dyn KVStore>, actions: &[sbproxy_modules::Action], ) -> anyhow::Result { let dependencies = - RuntimeDependencies::for_validation(server, actions_require_redis(actions))?; + RuntimeDependencies::for_validation(server, l2_store, actions_require_redis(actions))?; Self::with_dependencies(actions, dependencies) } @@ -574,10 +569,11 @@ fn destination_allowed(value: &str, allowed: &[String], blocked: &[String]) -> b #[cfg(test)] mod tests { use super::{ - validate_redis_dsn, CompressionExecution, CompressionRuntime, CompressionRuntimeRegistry, + redis_dependency, CompressionExecution, CompressionRuntime, CompressionRuntimeRegistry, RuntimeDependencies, }; use async_trait::async_trait; + use rcgen::{CertificateParams, KeyPair}; use sbproxy_ai::budget::{BudgetLimit, BudgetScope}; use sbproxy_ai::compression::{ CommitError, CompressionBackend, CompressionConsistency, CompressionRecordId, @@ -589,6 +585,7 @@ mod tests { use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::time::Duration; + use tempfile::TempDir; #[derive(Default)] struct TestStore { @@ -800,37 +797,144 @@ mod tests { .expect("handler fixture") } - #[test] - fn compression_redis_dsn_accepts_plaintext_and_tls_transports() { - assert!(validate_redis_dsn("redis://redis.internal:6379/0").is_ok()); - assert!(validate_redis_dsn("rediss://redis.internal:6380/0").is_ok()); + struct GeneratedRedisIdentity { + _directory: TempDir, + cert_file: String, + key_file: String, + } + + fn generated_redis_identity() -> GeneratedRedisIdentity { + let directory = tempfile::tempdir().expect("create Redis identity fixture directory"); + let key = KeyPair::generate().expect("generate Redis identity fixture key"); + let certificate = CertificateParams::new(vec!["redis-client.example".to_string()]) + .expect("configure Redis identity fixture certificate") + .self_signed(&key) + .expect("self-sign Redis identity fixture certificate"); + let cert_file = directory.path().join("client.pem"); + let key_file = directory.path().join("client-key.pem"); + std::fs::write(&cert_file, certificate.pem()) + .expect("write Redis identity fixture certificate"); + std::fs::write(&key_file, key.serialize_pem()) + .expect("write Redis identity fixture private key"); + + GeneratedRedisIdentity { + _directory: directory, + cert_file: cert_file.to_string_lossy().into_owned(), + key_file: key_file.to_string_lossy().into_owned(), + } + } + + fn server_with_l2(params: sbproxy_config::L2CacheParams) -> sbproxy_config::ProxyServerConfig { + sbproxy_config::ProxyServerConfig { + l2_cache: Some(sbproxy_config::L2CacheConfig { + driver: "redis".to_string(), + params, + }), + ..sbproxy_config::ProxyServerConfig::default() + } } #[test] - fn compression_redis_dsn_rejects_invalid_database_paths_before_startup() { - let error = validate_redis_dsn("redis://redis.internal/not-a-database") - .expect_err("Redis database selection must be parsed during validation"); + fn compression_reuses_compiled_l2_tls_snapshot_after_source_files_are_removed() { + let identity = generated_redis_identity(); + let cert_file = &identity.cert_file; + let key_file = &identity.key_file; + let yaml = format!( + r#" +proxy: + l2_cache_settings: + driver: redis + params: + dsn: rediss://default:p%40ss@[::1]:6380/7 + ca_file: '{cert_file}' + cert_file: '{cert_file}' + key_file: '{key_file}' +origins: + "ai.example.com": + action: + type: ai_proxy + providers: + - name: summary-provider + api_key: test-key + models: [summary-model] + compression: + state: + backend: redis + ttl: 1h + levers: + - type: summary_buffer + min_tokens: 100 + retain_recent_messages: 2 + target_summary_tokens: 20 + summarizer: + provider: summary-provider + model: summary-model + timeout: 2s +"# + ); + let compiled = sbproxy_config::compile_config(&yaml) + .expect("general L2 must compile and snapshot its TLS material"); + assert!(compiled.l2_store.is_some()); - assert!(error.to_string().contains("invalid DSN")); - assert!(!format!("{error:#}").contains("redis://")); + std::fs::remove_file(cert_file).expect("remove compiled certificate source"); + std::fs::remove_file(key_file).expect("remove compiled private-key source"); + + let pipeline = crate::pipeline::CompiledPipeline::from_config_for_validation(compiled) + .expect("compression must reuse the compiled L2 snapshot without rereading files"); + assert!(pipeline.compression_runtimes.get(0).is_some()); } #[test] - fn compression_redis_dsn_rejects_negative_database_before_startup() { - let error = validate_redis_dsn("redis://redis.internal/-1") - .expect_err("Redis database selection must be non-negative"); + fn compression_redis_reuses_private_ca_and_mtls_without_network_io() { + let identity = generated_redis_identity(); + let server = server_with_l2(sbproxy_config::L2CacheParams { + dsn: "rediss://default:p%40ss@[::1]:6380/7".to_string(), + ca_file: Some(identity.cert_file.clone()), + cert_file: Some(identity.cert_file.clone()), + key_file: Some(identity.key_file.clone()), + }); + let l2_store = + sbproxy_config::build_l2_store(server.l2_cache.as_ref().expect("L2 configuration")) + .expect("compile general L2 store"); - assert!(error.to_string().contains("invalid DSN")); - assert!(!format!("{error:#}").contains("redis://")); + let dependency = redis_dependency(&server, Some(l2_store.as_ref()), true) + .expect("valid Redis TLS configuration must compile without connecting"); + + assert!(dependency.is_some()); } #[test] - fn compression_redis_dsn_rejects_insecure_tls_escape_hatch() { - let error = validate_redis_dsn("rediss://redis.internal:6380/0#insecure") - .expect_err("compression state must never disable TLS certificate verification"); + fn compression_redis_rejects_the_same_tls_mismatch_as_l2_without_disclosure() { + let identity = generated_redis_identity(); + let dsn = "redis://default:sentinel-compression-password@sentinel-compression-host.invalid:6379/7"; + let server = server_with_l2(sbproxy_config::L2CacheParams { + dsn: dsn.to_string(), + ca_file: Some(identity.cert_file.clone()), + ..sbproxy_config::L2CacheParams::default() + }); + let l2_config = server.l2_cache.as_ref().expect("L2 config"); - assert!(error.to_string().contains("invalid DSN")); - assert!(!format!("{error:#}").contains("rediss://")); + let l2_error = match sbproxy_config::build_l2_store(l2_config) { + Ok(_) => panic!("blocking L2 store accepted plaintext TLS material"), + Err(error) => error, + }; + let compression_error = match redis_dependency(&server, None, true) { + Ok(_) => panic!("compression state accepted plaintext TLS material"), + Err(error) => error, + }; + + assert_eq!( + compression_error.to_string(), + "Redis compression state has invalid connection configuration" + ); + for chain in [format!("{l2_error:#}"), format!("{compression_error:#}")] { + for forbidden in [dsn, "sentinel-compression", "/7"] { + assert!( + !chain.contains(forbidden), + "Redis configuration error exposed forbidden material: {chain}" + ); + } + } } #[test] @@ -840,12 +944,13 @@ mod tests { driver: "redis".to_string(), params: sbproxy_config::L2CacheParams { dsn: "redis.internal:6379".to_string(), + ..sbproxy_config::L2CacheParams::default() }, }), ..sbproxy_config::ProxyServerConfig::default() }; - CompressionRuntimeRegistry::for_validation(&server, &[]) + CompressionRuntimeRegistry::for_validation(&server, None, &[]) .expect("unused compression runtime must not narrow the general L2 contract"); } diff --git a/crates/sbproxy-core/src/pipeline.rs b/crates/sbproxy-core/src/pipeline.rs index 6424364fb..a25d42a02 100644 --- a/crates/sbproxy-core/src/pipeline.rs +++ b/crates/sbproxy-core/src/pipeline.rs @@ -1314,12 +1314,14 @@ impl CompiledPipeline { PipelineConstructionMode::Runtime => { crate::compression_runtime::CompressionRuntimeRegistry::from_process( &config.server, + config.l2_store.as_deref(), &actions, )? } PipelineConstructionMode::Validation => { crate::compression_runtime::CompressionRuntimeRegistry::for_validation( &config.server, + config.l2_store.as_deref(), &actions, )? } diff --git a/crates/sbproxy-core/src/server/request_phase.rs b/crates/sbproxy-core/src/server/request_phase.rs index 901cd8fbd..169f54a6f 100644 --- a/crates/sbproxy-core/src/server/request_phase.rs +++ b/crates/sbproxy-core/src/server/request_phase.rs @@ -3474,22 +3474,10 @@ fn get_or_init_revocation_store( } sbproxy_config::OlpRevocationStoreConfig::Redis { url } => { // WOR-808 PR11: open the operator-declared Redis store. - // RedisConfig wants a `host:port` address, not a - // `redis://` URL; tolerate either by stripping the - // scheme prefix. Bad URL / unreachable Redis returns - // None so the handler 503s (introspect MUST NOT - // silently allow when the revocation store is down). - let addr = url - .strip_prefix("redis://") - .or_else(|| url.strip_prefix("rediss://")) - .unwrap_or(url.as_str()) - .trim_end_matches('/') - .to_string(); - let redis_cfg = sbproxy_platform::storage::RedisConfig { - addr, - pool_size: 8, - acquire_timeout: std::time::Duration::from_secs(5), - }; + // Invalid connection configuration returns None so the handler + // 503s. Introspection must not silently allow when the revocation + // store is unavailable. + let redis_cfg = sbproxy_platform::storage::RedisConfig::from_dsn(url).ok()?; std::sync::Arc::new(sbproxy_platform::storage::RedisKVStore::new(redis_cfg)) } }; @@ -3497,6 +3485,46 @@ fn get_or_init_revocation_store( Some(new_store) } +#[cfg(test)] +mod redis_revocation_store_tests { + use super::get_or_init_revocation_store; + use sbproxy_config::OlpRevocationStoreConfig; + + #[test] + fn redis_revocation_store_accepts_full_secure_dsn_without_network_io() { + let config = OlpRevocationStoreConfig::Redis { + url: "rediss://default:p%40ss@[::1]:6380/7".to_string(), + }; + + assert!(get_or_init_revocation_store("secure-dsn.example", &config).is_some()); + } + + #[test] + fn redis_revocation_store_rejects_invalid_dsn_before_network_io() { + let config = OlpRevocationStoreConfig::Redis { + url: "rediss://default:sentinel-olp-password@sentinel-olp-host.invalid:6380/-1" + .to_string(), + }; + + assert!(get_or_init_revocation_store("invalid-dsn.example", &config).is_none()); + } + + #[test] + fn invalid_revocation_store_log_uses_static_unavailable_message() { + let source = include_str!("request_phase.rs"); + let obsolete = ["revocation store backend not yet", " implemented"].concat(); + + assert!( + source.contains("warn!(\"olp introspect: revocation store unavailable\");"), + "invalid revocation-store construction must report unavailability" + ); + assert!( + !source.contains(&obsolete), + "shipped Redis support must not be described as unimplemented" + ); + } +} + /// WOR-808 PR10: per-IP token-bucket rate limiter for the /// `active: false` path on `/.well-known/olp/introspect`. RFC 7662 /// ยง2.1 calls out token-scanning attacks against introspect @@ -3678,7 +3706,7 @@ async fn handle_olp_introspect_or_revoke( let store = match get_or_init_revocation_store(aud_hint, &introspect_cfg.revocation_store) { Some(s) => s, None => { - warn!("olp introspect: revocation store backend not yet implemented (PR10/PR11)"); + warn!("olp introspect: revocation store unavailable"); let body = br#"{"error":"temporarily_unavailable"}"#; send_response(session, 503, "application/json", body).await?; return Ok(()); diff --git a/crates/sbproxy-core/tests/construct_examples.rs b/crates/sbproxy-core/tests/construct_examples.rs index 8616e9fda..5f6e7402d 100644 --- a/crates/sbproxy-core/tests/construct_examples.rs +++ b/crates/sbproxy-core/tests/construct_examples.rs @@ -10,6 +10,41 @@ //! that sweep and refused to boot; this test closes the gap. use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +struct RedisTlsExampleFixtures { + _directory: tempfile::TempDir, + ca_file: String, + cert_file: String, + key_file: String, +} + +fn redis_tls_example_fixtures() -> &'static RedisTlsExampleFixtures { + static FIXTURES: OnceLock = OnceLock::new(); + FIXTURES.get_or_init(|| { + let directory = tempfile::tempdir().expect("create Redis TLS example fixture directory"); + let key = rcgen::KeyPair::generate().expect("generate Redis TLS example key"); + let certificate = rcgen::CertificateParams::new(vec!["redis-client.example".to_string()]) + .expect("create Redis TLS example certificate parameters") + .self_signed(&key) + .expect("self-sign Redis TLS example certificate"); + + let ca_file = directory.path().join("ca.pem"); + let cert_file = directory.path().join("client.pem"); + let key_file = directory.path().join("client.key"); + std::fs::write(&ca_file, certificate.pem()).expect("write Redis TLS example CA"); + std::fs::write(&cert_file, certificate.pem()) + .expect("write Redis TLS example client certificate"); + std::fs::write(&key_file, key.serialize_pem()).expect("write Redis TLS example client key"); + + RedisTlsExampleFixtures { + _directory: directory, + ca_file: ca_file.to_string_lossy().into_owned(), + cert_file: cert_file.to_string_lossy().into_owned(), + key_file: key_file.to_string_lossy().into_owned(), + } + }) +} fn workspace_root() -> PathBuf { // sbproxy-core lives at crates/sbproxy-core/ inside the workspace. @@ -72,10 +107,16 @@ fn export_example_env_dummies() { ), ("ENV_VAR", "dummy"), ("VAR", "dummy"), + ("REDIS_PASSWORD", "redis-example-dummy"), ]; for (k, v) in DUMMIES { std::env::set_var(k, v); } + + let redis = redis_tls_example_fixtures(); + std::env::set_var("REDIS_CA_FILE", &redis.ca_file); + std::env::set_var("REDIS_CLIENT_CERT_FILE", &redis.cert_file); + std::env::set_var("REDIS_CLIENT_KEY_FILE", &redis.key_file); } #[test] diff --git a/crates/sbproxy-observe/src/metric_registry.rs b/crates/sbproxy-observe/src/metric_registry.rs index 69e82449a..c1744748b 100644 --- a/crates/sbproxy-observe/src/metric_registry.rs +++ b/crates/sbproxy-observe/src/metric_registry.rs @@ -1861,6 +1861,39 @@ pub const METRICS: &[MetricCapability] = &[ description: "Workspace rate-limit budget outcomes by workspace and result (soft/throttle).", dead_reason: None, }, + MetricCapability { + name: "sbproxy_redis_kv_connections_total", + kind: MetricKind::Counter, + writer: Writer::Recorder("redis_connection_results"), + support: SupportLevel::Stable, + compat: CompatTier::Beta, + registry: Registry::Default, + labels: &["result"], + description: "Redis KV connection attempts by result.", + dead_reason: None, + }, + MetricCapability { + name: "sbproxy_redis_kv_operation_duration_seconds", + kind: MetricKind::Histogram, + writer: Writer::Recorder("redis_operation_duration"), + support: SupportLevel::Stable, + compat: CompatTier::Beta, + registry: Registry::Default, + labels: &["operation"], + description: "Redis KV operation duration in seconds.", + dead_reason: None, + }, + MetricCapability { + name: "sbproxy_redis_kv_operation_errors_total", + kind: MetricKind::Counter, + writer: Writer::Recorder("redis_operation_errors"), + support: SupportLevel::Stable, + compat: CompatTier::Beta, + registry: Registry::Default, + labels: &["operation", "reason"], + description: "Redis KV operation failures by operation and reason.", + dead_reason: None, + }, MetricCapability { name: "sbproxy_request_duration_seconds", kind: MetricKind::Histogram, diff --git a/crates/sbproxy-platform/Cargo.toml b/crates/sbproxy-platform/Cargo.toml index f14b0591a..9d6ae5356 100644 --- a/crates/sbproxy-platform/Cargo.toml +++ b/crates/sbproxy-platform/Cargo.toml @@ -27,14 +27,20 @@ ureq = { version = "2", features = ["json"] } # Async runtime bridge. `rt` supports spawn_blocking for sync backends; # `macros`+`net` are required by the async Redis client (AsyncRedisKVStore). tokio = { workspace = true, features = ["rt", "macros", "sync", "net"] } -# Async Redis client for AsyncRedisKVStore, including rustls for `rediss://`. -redis = { version = "0.28.1", default-features = false, features = ["connection-manager", "tokio-rustls-comp"] } +# Redis client for validated connection config and AsyncRedisKVStore, including +# rustls for `rediss://`. +redis = { workspace = true, features = ["connection-manager", "tokio-rustls-comp"] } async-trait = { workspace = true } +prometheus = { workspace = true } +rustls = { workspace = true } +rustls-pki-types = { workspace = true } tracing = { workspace = true } +url = { workspace = true } # Optional storage backends redb = { workspace = true, optional = true } rusqlite = { workspace = true, optional = true } [dev-dependencies] +rcgen = { workspace = true } tempfile = "3" diff --git a/crates/sbproxy-platform/src/storage/async_redis.rs b/crates/sbproxy-platform/src/storage/async_redis.rs index 9076197d4..a284bf777 100644 --- a/crates/sbproxy-platform/src/storage/async_redis.rs +++ b/crates/sbproxy-platform/src/storage/async_redis.rs @@ -14,12 +14,13 @@ use std::{ collections::HashMap, + fmt, future::Future, sync::{atomic::AtomicU64, atomic::Ordering, Arc}, time::Duration, }; -use anyhow::{Context, Result}; +use anyhow::Result; use async_trait::async_trait; use bytes::Bytes; use redis::{ @@ -28,7 +29,10 @@ use redis::{ }; use tokio::sync::Mutex; -use super::async_kv::AsyncKVStore; +use super::{ + async_kv::AsyncKVStore, + redis_connection::{RedisTlsConfig, ValidatedRedisConnection}, +}; const DEFAULT_CONNECT_TIMEOUT: Duration = Duration::from_millis(500); const DEFAULT_RESPONSE_TIMEOUT: Duration = Duration::from_secs(1); @@ -37,6 +41,108 @@ const RECONNECT_EXPONENT_BASE: u64 = 2; const RECONNECT_DELAY_FACTOR_MS: u64 = 25; const RECONNECT_RETRIES: usize = 2; +#[derive(Debug, Clone, Copy)] +enum RedisErrorAction { + Connect, + ScriptLoad, + EvalSha, + EvalShaAfterReload, + Scan, + Get, + Set, + SetWithExpiry, + Incr, + Expire, + IncrBy, + Delete, +} + +impl RedisErrorAction { + fn message(self) -> &'static str { + match self { + Self::Connect => "connecting to Redis failed", + Self::ScriptLoad => "redis SCRIPT LOAD failed", + Self::EvalSha => "redis EVALSHA failed", + Self::EvalShaAfterReload => "redis EVALSHA failed after NOSCRIPT reload", + Self::Scan => "redis SCAN failed", + Self::Get => "redis GET failed", + Self::Set => "redis SET failed", + Self::SetWithExpiry => "redis SET EX failed", + Self::Incr => "redis INCR failed", + Self::Expire => "redis EXPIRE failed", + Self::IncrBy => "redis INCRBY failed", + Self::Delete => "redis DEL failed", + } + } +} + +#[derive(Debug, Clone, Copy)] +enum RedisErrorReason { + Timeout, + Transport, + Authentication, + Configuration, + ScriptUnavailable, + Command, +} + +impl RedisErrorReason { + fn from_error(error: &redis::RedisError) -> Self { + if error.is_timeout() { + return Self::Timeout; + } + match error.kind() { + ErrorKind::IoError + | ErrorKind::Moved + | ErrorKind::Ask + | ErrorKind::TryAgain + | ErrorKind::ClusterDown + | ErrorKind::MasterDown + | ErrorKind::MasterNameNotFoundBySentinel + | ErrorKind::NoValidReplicasFoundBySentinel + | ErrorKind::EmptySentinelList + | ErrorKind::ClusterConnectionNotFound => Self::Transport, + ErrorKind::AuthenticationFailed => Self::Authentication, + ErrorKind::InvalidClientConfig => Self::Configuration, + ErrorKind::NoScriptError => Self::ScriptUnavailable, + _ => Self::Command, + } + } + + fn message(self) -> &'static str { + match self { + Self::Timeout => "Redis transport timed out", + Self::Transport => "Redis transport failed", + Self::Authentication => "Redis authentication failed", + Self::Configuration => "Redis client configuration failed", + Self::ScriptUnavailable => "Redis script unavailable", + Self::Command => "Redis command failed", + } + } +} + +fn sanitize_redis_error(action: RedisErrorAction, error: &redis::RedisError) -> anyhow::Error { + anyhow::anyhow!( + "{}: {}", + action.message(), + RedisErrorReason::from_error(error).message() + ) +} + +enum RedisQueryError { + Connection(anyhow::Error), + Command(redis::RedisError), +} + +impl RedisQueryError { + fn into_public_error(self, action: RedisErrorAction) -> anyhow::Error { + match self { + Self::Connection(error) => error, + Self::Command(error) => sanitize_redis_error(action, &error), + } + } +} + #[derive(Debug, Clone, Copy)] struct RedisTimeouts { connect: Duration, @@ -55,10 +161,28 @@ impl Default for RedisTimeouts { } /// Configuration for [`AsyncRedisKVStore`]. -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct AsyncRedisConfig { - /// Connection URL (`redis://host:6379/0` or `rediss://host:6380/0`). - pub url: String, + source: AsyncRedisClientSource, +} + +#[derive(Clone)] +enum AsyncRedisClientSource { + Dsn(String), + Validated(ValidatedRedisConnection), +} + +impl fmt::Debug for AsyncRedisConfig { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + let source = match &self.source { + AsyncRedisClientSource::Dsn(_) => "dsn", + AsyncRedisClientSource::Validated(_) => "validated", + }; + formatter + .debug_struct("AsyncRedisConfig") + .field("source", &source) + .finish() + } } /// One bounded Redis `SCAN` response. @@ -71,21 +195,33 @@ pub struct RedisScanPage { } impl AsyncRedisConfig { - /// Construct a new config from a Redis connection URL. + /// Construct a new config from a Redis connection string. pub fn new(url: &str) -> Self { Self { - url: url.to_string(), + source: AsyncRedisClientSource::Dsn(url.to_string()), + } + } + + /// Construct a config from an already validated Redis connection. + pub fn from_connection(connection: ValidatedRedisConnection) -> Self { + Self { + source: AsyncRedisClientSource::Validated(connection), } } - /// Validate Redis-specific URL semantics without opening a connection. + /// Validate Redis connection semantics without opening a connection. pub fn validate(&self) -> Result<()> { - let client = Client::open(self.url.as_str()).context("invalid Redis connection URL")?; - anyhow::ensure!( - client.get_connection_info().redis.db >= 0, - "invalid Redis database selection" - ); - Ok(()) + self.client().map(|_| ()) + } + + fn client(&self) -> Result { + match &self.source { + AsyncRedisClientSource::Dsn(dsn) => { + ValidatedRedisConnection::new(dsn, RedisTlsConfig::default()) + .map(|connection| connection.client()) + } + AsyncRedisClientSource::Validated(connection) => Ok(connection.client()), + } } } @@ -155,8 +291,7 @@ impl AsyncRedisKVStore { } } // Slow path: establish the connection without holding the lock. - let client = - Client::open(self.config.url.as_str()).context("invalid Redis connection URL")?; + let client = self.config.client()?; let manager_config = ConnectionManagerConfig::new() .set_exponent_base(RECONNECT_EXPONENT_BASE) .set_factor(RECONNECT_DELAY_FACTOR_MS) @@ -174,7 +309,7 @@ impl AsyncRedisKVStore { self.timeouts.connect.as_millis() ) })? - .context("connecting to Redis")?; + .map_err(|error| sanitize_redis_error(RedisErrorAction::Connect, &error))?; let candidate = CachedConnection { generation: self .next_connection_generation @@ -202,17 +337,14 @@ impl AsyncRedisKVStore { } } - async fn query_redis(&self, command: &mut redis::Cmd) -> redis::RedisResult + async fn query_redis( + &self, + command: &mut redis::Cmd, + ) -> std::result::Result where T: FromRedisValue, { - let mut cached = self.conn().await.map_err(|error| { - redis::RedisError::from(( - ErrorKind::IoError, - "opening managed Redis connection", - error.to_string(), - )) - })?; + let mut cached = self.conn().await.map_err(RedisQueryError::Connection)?; let result = command.query_async(&mut cached.manager).await; if result .as_ref() @@ -220,14 +352,16 @@ impl AsyncRedisKVStore { { self.invalidate_connection(cached.generation).await; } - result + result.map_err(RedisQueryError::Command) } - async fn query(&self, command: &mut redis::Cmd, context: &'static str) -> Result + async fn query(&self, command: &mut redis::Cmd, action: RedisErrorAction) -> Result where T: FromRedisValue, { - self.query_redis(command).await.context(context) + self.query_redis(command) + .await + .map_err(|error| error.into_public_error(action)) } /// Execute a Lua script through `EVALSHA`, loading or reloading it as needed. @@ -267,13 +401,13 @@ impl AsyncRedisKVStore { match self.evalsha(&script_hash, keys, args).await { Ok(value) => Ok(value), - Err(error) if error.kind() == ErrorKind::NoScriptError => { + Err(RedisQueryError::Command(error)) if error.kind() == ErrorKind::NoScriptError => { let reloaded_hash = self.load_script(script_source).await?; self.evalsha(&reloaded_hash, keys, args) .await - .context("redis EVALSHA failed after NOSCRIPT reload") + .map_err(|error| error.into_public_error(RedisErrorAction::EvalShaAfterReload)) } - Err(error) => Err(error).context("redis EVALSHA failed"), + Err(error) => Err(error.into_public_error(RedisErrorAction::EvalSha)), } } @@ -281,7 +415,7 @@ impl AsyncRedisKVStore { let script_hash: String = self .query( redis::cmd("SCRIPT").arg("LOAD").arg(script_source), - "redis SCRIPT LOAD failed", + RedisErrorAction::ScriptLoad, ) .await?; self.script_hashes @@ -296,7 +430,7 @@ impl AsyncRedisKVStore { script_hash: &str, keys: &[String], args: &[String], - ) -> redis::RedisResult> { + ) -> std::result::Result, RedisQueryError> { let mut command = redis::cmd("EVALSHA"); command.arg(script_hash).arg(keys.len()); for key in keys { @@ -326,7 +460,7 @@ impl AsyncRedisKVStore { .arg(pattern) .arg("COUNT") .arg(count), - "redis SCAN failed", + RedisErrorAction::Scan, ) .await?; Ok(RedisScanPage { next_cursor, keys }) @@ -340,7 +474,7 @@ impl AsyncKVStore for AsyncRedisKVStore { async fn get(&self, key: &[u8]) -> Result> { self.with_operation_deadline("GET", async { let value: Option> = self - .query(redis::cmd("GET").arg(key), "redis GET failed") + .query(redis::cmd("GET").arg(key), RedisErrorAction::Get) .await?; Ok(value.map(Bytes::from)) }) @@ -349,7 +483,7 @@ impl AsyncKVStore for AsyncRedisKVStore { async fn put(&self, key: &[u8], value: &[u8]) -> Result<()> { self.with_operation_deadline("SET", async { - self.query::<()>(redis::cmd("SET").arg(key).arg(value), "redis SET failed") + self.query::<()>(redis::cmd("SET").arg(key).arg(value), RedisErrorAction::Set) .await?; Ok(()) }) @@ -359,7 +493,7 @@ impl AsyncKVStore for AsyncRedisKVStore { async fn put_with_ttl(&self, key: &[u8], value: &[u8], ttl_secs: u64) -> Result<()> { self.with_operation_deadline("SET with TTL", async { if ttl_secs == 0 { - self.query::<()>(redis::cmd("SET").arg(key).arg(value), "redis SET failed") + self.query::<()>(redis::cmd("SET").arg(key).arg(value), RedisErrorAction::Set) .await?; } else { self.query::<()>( @@ -368,7 +502,7 @@ impl AsyncKVStore for AsyncRedisKVStore { .arg(value) .arg("EX") .arg(ttl_secs), - "redis SET EX failed", + RedisErrorAction::SetWithExpiry, ) .await?; } @@ -386,12 +520,12 @@ impl AsyncKVStore for AsyncRedisKVStore { // TTL. If stricter atomicity is needed later, switch to a Lua // script via EVAL. let value: i64 = self - .query(redis::cmd("INCR").arg(key), "redis INCR failed") + .query(redis::cmd("INCR").arg(key), RedisErrorAction::Incr) .await?; if ttl_secs > 0 { self.query::( redis::cmd("EXPIRE").arg(key).arg(ttl_secs), - "redis EXPIRE failed", + RedisErrorAction::Expire, ) .await?; } @@ -408,13 +542,13 @@ impl AsyncKVStore for AsyncRedisKVStore { let value: i64 = self .query( redis::cmd("INCRBY").arg(key).arg(amount), - "redis INCRBY failed", + RedisErrorAction::IncrBy, ) .await?; if ttl_secs > 0 { self.query::( redis::cmd("EXPIRE").arg(key).arg(ttl_secs), - "redis EXPIRE failed", + RedisErrorAction::Expire, ) .await?; } @@ -425,7 +559,7 @@ impl AsyncKVStore for AsyncRedisKVStore { async fn delete(&self, key: &[u8]) -> Result<()> { self.with_operation_deadline("DEL", async { - self.query::(redis::cmd("DEL").arg(key), "redis DEL failed") + self.query::(redis::cmd("DEL").arg(key), RedisErrorAction::Delete) .await?; Ok(()) }) @@ -436,6 +570,7 @@ impl AsyncKVStore for AsyncRedisKVStore { #[cfg(test)] mod tests { use super::*; + use crate::storage::{RedisTlsConfig, ValidatedRedisConnection}; use std::process::{Child, Command, Stdio}; use std::time::{Duration, Instant}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -458,7 +593,6 @@ mod tests { #[test] fn config_constructs() { let cfg = AsyncRedisConfig::new("redis://127.0.0.1:6379/0"); - assert_eq!(cfg.url, "redis://127.0.0.1:6379/0"); cfg.validate().unwrap(); assert!(AsyncRedisConfig::new("redis://127.0.0.1/not-a-database") .validate() @@ -475,6 +609,73 @@ mod tests { .unwrap(); } + #[test] + fn validated_connection_config_debug_redacts_dsn_and_private_ca() { + let key = rcgen::KeyPair::generate().unwrap(); + let params = + rcgen::CertificateParams::new(vec!["sentinel-private-ca.invalid".to_string()]).unwrap(); + let certificate = params.self_signed(&key).unwrap().pem(); + let dsn = "rediss://sentinel-user:sentinel-password@sentinel-host.invalid:6380/7"; + let connection = ValidatedRedisConnection::new( + dsn, + RedisTlsConfig { + root_cert: Some(certificate.as_bytes().to_vec()), + ..RedisTlsConfig::default() + }, + ) + .unwrap(); + + let config = AsyncRedisConfig::from_connection(connection); + let rendered = format!("{config:?}"); + for forbidden in [ + dsn, + "sentinel-user", + "sentinel-password", + "sentinel-host", + certificate.as_str(), + ] { + assert!(!rendered.contains(forbidden), "{rendered}"); + } + } + + #[test] + fn legacy_constructor_still_validates_database_selection() { + AsyncRedisConfig::new("redis://localhost:6379/7") + .validate() + .unwrap(); + } + + #[test] + fn redis_error_sanitization_drops_endpoint_bearing_source_chains() { + let source = redis::RedisError::from(( + ErrorKind::IoError, + "TLS connection failed", + "rediss://sentinel-user:sentinel-password@sentinel-host.invalid:6380/7: \ + certificate is not valid for 203.0.113.77; key sentinel-key; value sentinel-value" + .to_string(), + )); + assert!(source.to_string().contains("sentinel-host.invalid")); + + let error = sanitize_redis_error(RedisErrorAction::Get, &source); + let rendered = format!("{error:#}"); + + assert_eq!(rendered, "redis GET failed: Redis transport failed"); + assert_eq!(error.chain().count(), 1); + for forbidden in [ + "sentinel-user", + "sentinel-password", + "sentinel-host", + "203.0.113.77", + "6380", + "/7", + "certificate", + "sentinel-key", + "sentinel-value", + ] { + assert!(!rendered.contains(forbidden), "{rendered}"); + } + } + #[test] fn new_defers_connection() { // Bad URL is fine until we actually try to connect. diff --git a/crates/sbproxy-platform/src/storage/mod.rs b/crates/sbproxy-platform/src/storage/mod.rs index 30fd27a16..ce7b10ae8 100644 --- a/crates/sbproxy-platform/src/storage/mod.rs +++ b/crates/sbproxy-platform/src/storage/mod.rs @@ -6,6 +6,7 @@ mod file; mod memory; mod redb_store; mod redis; +mod redis_connection; mod sqlite; pub use async_kv::AsyncKVStore; @@ -14,6 +15,7 @@ pub use file::FileKVStore; pub use memory::MemoryKVStore; pub use redb_store::RedbKVStore; pub use redis::{RedisConfig, RedisKVStore}; +pub use redis_connection::{RedisTlsConfig, ValidatedRedisConnection}; pub use sqlite::SqliteKVStore; use anyhow::Result; @@ -21,6 +23,17 @@ use bytes::Bytes; /// Low-level key-value storage. All implementations must be thread-safe. pub trait KVStore: Send + Sync + 'static { + /// Clone the already-validated Redis connection snapshot, when this store + /// is Redis-backed. + /// + /// Runtime consumers use this internal seam to share one compiled DSN and + /// TLS identity without reopening configuration files. Other backends + /// retain the default `None` implementation. + #[doc(hidden)] + fn validated_redis_connection(&self) -> Option { + None + } + /// Get a value by key. Returns None if the key does not exist. fn get(&self, key: &[u8]) -> Result>; diff --git a/crates/sbproxy-platform/src/storage/redis.rs b/crates/sbproxy-platform/src/storage/redis.rs index 91bc47c79..c2c81af5e 100644 --- a/crates/sbproxy-platform/src/storage/redis.rs +++ b/crates/sbproxy-platform/src/storage/redis.rs @@ -1,77 +1,77 @@ -//! Redis KVStore backend using raw RESP protocol over a small connection pool. +//! Redis KVStore backend using a small blocking connection pool. //! -//! This implementation speaks the Redis Serialization Protocol (RESP2) directly -//! over blocking `TcpStream`s. A pool of connections (default 8) is maintained -//! so that concurrent operations do not serialize on a single connection. +//! A pool of validated Redis connections (default 8) is maintained so that +//! concurrent operations do not serialize on a single connection. //! //! Connections are lazily opened, up to `pool_size` total, and returned to the -//! idle list on drop. Broken connections (those that return an error during -//! use) are discarded rather than returned to the pool so a subsequent checkout -//! will open a fresh one. +//! idle list on drop. Connections with transport, timeout, or protocol failures +//! are discarded so a subsequent checkout opens a fresh one. //! //! Supported commands: GET, SET, DEL, SCAN (with MATCH + COUNT). -//! -//! # Limitations -//! - Only RESP2 simple strings, bulk strings, integers, and arrays are decoded. -//! - Pool size is fixed after construction. -use std::io::BufReader; -use std::net::TcpStream; -use std::sync::{Condvar, Mutex}; +use std::error::Error as StdError; +use std::fmt; +use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::{Condvar, Mutex, OnceLock}; use std::time::{Duration, Instant}; -use anyhow::{bail, Context, Result}; +use anyhow::Result; use bytes::Bytes; +use prometheus::{HistogramVec, IntCounterVec}; +use redis::{Connection, ErrorKind, RedisError}; -use super::KVStore; -use crate::resp::{read_resp, write_command, RespValue}; - -// --- Connection management --- - -struct Connection { - reader: BufReader, - writer: TcpStream, -} - -impl Connection { - fn connect(addr: &str) -> Result { - let stream = - TcpStream::connect(addr).with_context(|| format!("connect to Redis at {}", addr))?; - stream.set_read_timeout(Some(Duration::from_secs(5)))?; - stream.set_write_timeout(Some(Duration::from_secs(5)))?; - let writer = stream.try_clone()?; - let reader = BufReader::new(stream); - Ok(Self { reader, writer }) - } +use super::{ + redis_connection::{RedisTlsConfig, ValidatedRedisConnection}, + KVStore, +}; - fn call(&mut self, args: &[&[u8]]) -> Result { - write_command(&mut self.writer, args)?; - read_resp(&mut self.reader) - } -} +const HEALTH_UNKNOWN: u8 = 0; +const HEALTH_HEALTHY: u8 = 1; +const HEALTH_FAILED: u8 = 2; // --- RedisKVStore --- /// Configuration for [`RedisKVStore`]. pub struct RedisConfig { - /// Redis server address, e.g. `"127.0.0.1:6379"`. - pub addr: String, + /// Prevalidated Redis client configuration. + pub connection: ValidatedRedisConnection, /// Maximum number of connections held in the pool. Connections are opened /// lazily up to this bound. Default: 8. pub pool_size: usize, /// Timeout when acquiring a connection from the pool before erroring out. /// Default: 5 seconds. pub acquire_timeout: Duration, + /// Timeout for establishing a connection, including AUTH and SELECT. + /// Default: 5 seconds. + pub connect_timeout: Duration, + /// Read and write timeout for Redis commands. Default: 5 seconds. + pub command_timeout: Duration, } -impl Default for RedisConfig { - fn default() -> Self { +impl RedisConfig { + /// Build a blocking-store configuration from a validated connection. + pub fn new(connection: ValidatedRedisConnection) -> Self { Self { - addr: "127.0.0.1:6379".into(), + connection, pool_size: 8, acquire_timeout: Duration::from_secs(5), + connect_timeout: Duration::from_secs(5), + command_timeout: Duration::from_secs(5), } } + + /// Validate a Redis DSN without opening a network connection. + pub fn from_dsn(dsn: &str) -> Result { + ValidatedRedisConnection::new(dsn, RedisTlsConfig::default()).map(Self::new) + } +} + +impl Default for RedisConfig { + fn default() -> Self { + let connection = ValidatedRedisConnection::new("127.0.0.1:6379", RedisTlsConfig::default()) + .expect("default Redis connection configuration must be valid"); + Self::new(connection) + } } // --- Pool internals --- @@ -88,32 +88,28 @@ struct PoolState { /// strings. Keys are hex-encoded before being sent to Redis to avoid /// characters that are problematic with SCAN MATCH glob patterns. pub struct RedisKVStore { - addr: String, + connection: ValidatedRedisConnection, pool_size: usize, acquire_timeout: Duration, + connect_timeout: Duration, + command_timeout: Duration, // Pool state: a Mutex holding idle connections and the in-use count. state: Mutex, // Signaled when a connection is returned to idle or when in_use drops, // so a waiting checkout can make progress. available: Condvar, + health: AtomicU8, } -/// RAII guard returned by [`RedisKVStore::checkout`]. Holds a live connection -/// and returns it to the pool on drop. If the caller observes an I/O error -/// while using the connection, they should call [`PooledConnection::invalidate`] -/// to prevent a broken connection from being returned to the pool. -pub struct PooledConnection<'a> { +/// RAII guard that returns a healthy connection to the pool on drop. +struct PooledConnection<'a> { pool: &'a RedisKVStore, conn: Option, } impl PooledConnection<'_> { - /// Execute a RESP command on the borrowed connection. - fn call(&mut self, args: &[&[u8]]) -> Result { - self.conn - .as_mut() - .expect("connection already invalidated") - .call(args) + fn connection(&mut self) -> &mut Connection { + self.conn.as_mut().expect("connection already invalidated") } /// Mark the connection as broken so it is discarded on drop instead of @@ -144,21 +140,24 @@ impl RedisKVStore { pub fn new(config: RedisConfig) -> Self { let pool_size = config.pool_size.max(1); Self { - addr: config.addr, + connection: config.connection, pool_size, acquire_timeout: config.acquire_timeout, + connect_timeout: config.connect_timeout, + command_timeout: config.command_timeout, state: Mutex::new(PoolState { idle: Vec::with_capacity(pool_size), in_use: 0, }), available: Condvar::new(), + health: AtomicU8::new(HEALTH_UNKNOWN), } } /// Borrow a connection from the pool, opening a new one if the pool has /// spare capacity. Blocks up to `acquire_timeout` waiting for a connection /// to become available; returns an error on timeout. - fn checkout(&self) -> Result> { + fn checkout(&self, operation: RedisOperation) -> Result> { let deadline = Instant::now() + self.acquire_timeout; let mut guard = self.state.lock().expect("lock poisoned"); loop { @@ -177,21 +176,36 @@ impl RedisKVStore { // released before propagating the error. if guard.in_use < self.pool_size { guard.in_use += 1; + let client = self.connection.client(); drop(guard); - match Connection::connect(&self.addr) { + let connection = client + .get_connection_with_timeout(self.connect_timeout) + .and_then(|connection| { + connection.set_read_timeout(Some(self.command_timeout))?; + connection.set_write_timeout(Some(self.command_timeout))?; + Ok(connection) + }); + match connection { Ok(conn) => { + redis_connection_results() + .with_label_values(&["success"]) + .inc(); return Ok(PooledConnection { pool: self, conn: Some(conn), }); } Err(err) => { - // Connect failed; release the reserved slot and wake a - // waiter so they can retry. - let mut g = self.state.lock().expect("lock poisoned"); - g.in_use = g.in_use.saturating_sub(1); - self.available.notify_one(); - return Err(err); + redis_connection_results() + .with_label_values(&["error"]) + .inc(); + self.release_reserved_slot(); + let reason = classify_redis_error( + &err, + ErrorPhase::Connect, + self.connection.uses_tls(), + ); + return Err(safe_error(operation, reason)); } } } @@ -199,7 +213,7 @@ impl RedisKVStore { // Pool saturated: wait for a connection to be returned. let now = Instant::now(); if now >= deadline { - bail!("timed out acquiring Redis connection (pool exhausted)"); + return Err(safe_error(operation, RedisFailureReason::PoolTimeout)); } let remaining = deadline - now; let (g, timeout) = self @@ -208,27 +222,89 @@ impl RedisKVStore { .expect("condvar wait failed"); guard = g; if timeout.timed_out() { - bail!("timed out acquiring Redis connection (pool exhausted)"); + return Err(safe_error(operation, RedisFailureReason::PoolTimeout)); } } } + fn release_reserved_slot(&self) { + let mut guard = self.state.lock().expect("lock poisoned"); + guard.in_use = guard.in_use.saturating_sub(1); + self.available.notify_one(); + } + /// Checkout a connection, run `f` against it, and invalidate the /// connection if `f` returns an error so a broken conn is not returned /// to the pool. - fn with_conn(&self, mut f: F) -> Result + fn with_conn(&self, operation: RedisOperation, f: F) -> Result where - F: FnMut(&mut PooledConnection<'_>) -> Result, + F: FnOnce(&mut Connection) -> redis::RedisResult, { - let mut conn = self.checkout()?; - match f(&mut conn) { + let mut conn = self.checkout(operation)?; + match f(conn.connection()) { Ok(v) => Ok(v), - Err(e) => { - // Connection may be in an inconsistent state; discard it. - conn.invalidate(); - Err(e) + Err(error) => { + if should_invalidate(&error) { + conn.invalidate(); + } + let reason = + classify_redis_error(&error, ErrorPhase::Command, self.connection.uses_tls()); + Err(safe_error(operation, reason)) + } + } + } + + fn execute( + &self, + operation: RedisOperation, + function: impl FnOnce() -> Result, + ) -> Result { + let started_at = Instant::now(); + let result = function(); + redis_operation_duration() + .with_label_values(&[operation.as_str()]) + .observe(started_at.elapsed().as_secs_f64()); + + match &result { + Ok(_) => self.record_success(operation), + Err(error) => { + let reason = error + .downcast_ref::() + .map_or(RedisFailureReason::Protocol, |error| error.reason); + redis_operation_errors() + .with_label_values(&[operation.as_str(), reason.as_str()]) + .inc(); + self.record_failure(operation, reason); } } + result + } + + fn record_success(&self, operation: RedisOperation) { + let previous = self.health.swap(HEALTH_HEALTHY, Ordering::AcqRel); + if previous == HEALTH_FAILED { + tracing::info!( + operation = operation.as_str(), + "redis store health recovered" + ); + } + } + + fn record_failure(&self, operation: RedisOperation, reason: RedisFailureReason) { + let previous = self.health.swap(HEALTH_FAILED, Ordering::AcqRel); + if matches!(previous, HEALTH_UNKNOWN | HEALTH_HEALTHY) { + tracing::warn!( + operation = operation.as_str(), + reason = reason.as_str(), + "redis store health failed" + ); + } else { + tracing::debug!( + operation = operation.as_str(), + reason = reason.as_str(), + "redis store health remains failed" + ); + } } /// Hex-encode the raw key for safe use in Redis key names and SCAN patterns. @@ -238,339 +314,1390 @@ impl RedisKVStore { } impl KVStore for RedisKVStore { + fn validated_redis_connection(&self) -> Option { + Some(self.connection.clone()) + } + fn get(&self, key: &[u8]) -> Result> { - let encoded = Self::encode_key(key); - self.with_conn(|c| match c.call(&[b"GET", encoded.as_bytes()])? { - RespValue::Nil => Ok(None), - RespValue::Bytes(b) => Ok(Some(Bytes::from(b))), - other => bail!("unexpected GET response: {:?}", other), + self.execute(RedisOperation::Get, || { + let encoded = Self::encode_key(key); + self.with_conn(RedisOperation::Get, |connection| { + let value: Option> = redis::cmd("GET").arg(&encoded).query(connection)?; + Ok(value.map(Bytes::from)) + }) }) } fn put(&self, key: &[u8], value: &[u8]) -> Result<()> { - let encoded = Self::encode_key(key); - self.with_conn(|c| { - c.call(&[b"SET", encoded.as_bytes(), value])?; - Ok(()) + self.execute(RedisOperation::Set, || { + let encoded = Self::encode_key(key); + self.with_conn(RedisOperation::Set, |connection| { + redis::cmd("SET") + .arg(&encoded) + .arg(value) + .query::<()>(connection)?; + Ok(()) + }) }) } fn delete(&self, key: &[u8]) -> Result<()> { - let encoded = Self::encode_key(key); - self.with_conn(|c| { - c.call(&[b"DEL", encoded.as_bytes()])?; - Ok(()) + self.execute(RedisOperation::Delete, || { + let encoded = Self::encode_key(key); + self.with_conn(RedisOperation::Delete, |connection| { + redis::cmd("DEL").arg(&encoded).query::(connection)?; + Ok(()) + }) }) } fn put_with_ttl(&self, key: &[u8], value: &[u8], ttl_secs: u64) -> Result<()> { - // SET EX - let encoded = Self::encode_key(key); - let ttl_str = ttl_secs.to_string(); - self.with_conn(|c| { - c.call(&[b"SET", encoded.as_bytes(), value, b"EX", ttl_str.as_bytes()])?; - Ok(()) + self.execute(RedisOperation::SetTtl, || { + // SET EX + let encoded = Self::encode_key(key); + self.with_conn(RedisOperation::SetTtl, |connection| { + redis::cmd("SET") + .arg(&encoded) + .arg(value) + .arg("EX") + .arg(ttl_secs) + .query::<()>(connection)?; + Ok(()) + }) }) } fn incr_with_ttl(&self, key: &[u8], ttl_secs: u64) -> Result { - // Use MULTI / INCR / EXPIRE / EXEC so both commands land atomically. - // The EXEC reply is an array whose first element is the INCR result - // (new counter value) and second is the EXPIRE reply (1 when applied). - let encoded = Self::encode_key(key); - let ttl_str = ttl_secs.to_string(); - - self.with_conn(|c| { - // Start transaction. - match c.call(&[b"MULTI"])? { - RespValue::Bytes(b) if b == b"OK" => {} - other => bail!("unexpected MULTI response: {:?}", other), - } + self.execute(RedisOperation::Increment, || { + // Use MULTI / INCR / EXPIRE / EXEC so both commands land atomically. + // The EXEC reply is an array whose first element is the INCR result + // (new counter value) and second is the EXPIRE reply (1 when applied). + let encoded = Self::encode_key(key); + self.with_conn(RedisOperation::Increment, |connection| { + let (counter, _expiry_applied): (i64, bool) = redis::pipe() + .atomic() + .cmd("INCR") + .arg(&encoded) + .cmd("EXPIRE") + .arg(&encoded) + .arg(ttl_secs) + .query(connection)?; + Ok(counter) + }) + }) + } - // Queued INCR. - match c.call(&[b"INCR", encoded.as_bytes()])? { - RespValue::Bytes(b) if b == b"QUEUED" => {} - other => bail!("unexpected INCR-queue response: {:?}", other), - } + fn try_lock(&self, key: &[u8], token: &[u8], ttl_secs: u64) -> Result { + self.execute(RedisOperation::Lock, || { + // Atomic lease: SET NX PX . The reply is "OK" + // when the key was set (lock acquired) or nil when it already + // exists (another holder has it). WOR-1774. + let encoded = Self::encode_key(key); + let ttl_ms = ttl_secs.saturating_mul(1000); + self.with_conn(RedisOperation::Lock, |connection| { + let response: Option = redis::cmd("SET") + .arg(&encoded) + .arg(token) + .arg("NX") + .arg("PX") + .arg(ttl_ms) + .query(connection)?; + match response.as_deref() { + Some("OK") => Ok(true), + None => Ok(false), + Some(_) => Err((ErrorKind::TypeError, "unexpected SET NX response").into()), + } + }) + }) + } - // Queued EXPIRE. - match c.call(&[b"EXPIRE", encoded.as_bytes(), ttl_str.as_bytes()])? { - RespValue::Bytes(b) if b == b"QUEUED" => {} - other => bail!("unexpected EXPIRE-queue response: {:?}", other), - } + fn unlock(&self, key: &[u8], token: &[u8]) -> Result<()> { + self.execute(RedisOperation::Unlock, || { + // Compare-and-delete via EVAL so we only delete the lock while it is + // still ours: a bare DEL could remove a lock a different node + // acquired after this one's lease had expired. + const RELEASE: &[u8] = b"if redis.call('get', KEYS[1]) == ARGV[1] then return redis.call('del', KEYS[1]) else return 0 end"; + let encoded = Self::encode_key(key); + self.with_conn(RedisOperation::Unlock, |connection| { + redis::cmd("EVAL") + .arg(RELEASE) + .arg(1) + .arg(&encoded) + .arg(token) + .query::(connection)?; + Ok(()) + }) + }) + } - // Execute. Reply is an array of per-command replies. - match c.call(&[b"EXEC"])? { - RespValue::Array(results) => { - // First reply is the INCR integer. - match results.into_iter().next() { - Some(RespValue::Integer(n)) => Ok(n), - Some(other) => { - bail!("unexpected INCR reply inside EXEC: {:?}", other) - } - None => bail!("empty EXEC reply"), + fn scan_prefix(&self, prefix: &[u8]) -> Result> { + self.execute(RedisOperation::Scan, || { + // Build a SCAN MATCH glob: hex(prefix)* (safe because hex output + // contains only [0-9a-f] which has no glob special characters). + let pattern = format!("{}*", Self::encode_key(prefix)); + + let mut results = Vec::new(); + let mut cursor = 0_u64; + + loop { + let (next_cursor, keys): (u64, Vec>) = + self.with_conn(RedisOperation::Scan, |connection| { + redis::cmd("SCAN") + .arg(cursor) + .arg("MATCH") + .arg(&pattern) + .arg("COUNT") + .arg(100) + .query(connection) + })?; + + for encoded_key in keys { + let raw_key = hex::decode(&encoded_key).map_err(|_| { + safe_error(RedisOperation::Scan, RedisFailureReason::Protocol) + })?; + + // Fetch one value for each key returned by SCAN. + let value = self.with_conn(RedisOperation::Scan, |connection| { + let value: Option> = + redis::cmd("GET").arg(&encoded_key).query(connection)?; + Ok(value.map(Bytes::from)) + })?; + + if let Some(value) = value { + results.push((Bytes::from(raw_key), value)); } } - other => bail!("unexpected EXEC response: {:?}", other), + + if next_cursor == 0 { + break; + } + cursor = next_cursor; } + + Ok(results) }) } +} - fn try_lock(&self, key: &[u8], token: &[u8], ttl_secs: u64) -> Result { - // Atomic lease: SET NX PX . The reply is "OK" - // when the key was set (lock acquired) or nil when it already - // exists (another holder has it). WOR-1774. - let encoded = Self::encode_key(key); - let ttl_ms = ttl_secs.saturating_mul(1000).to_string(); - self.with_conn(|c| { - match c.call(&[ - b"SET", - encoded.as_bytes(), - token, - b"NX", - b"PX", - ttl_ms.as_bytes(), - ])? { - RespValue::Bytes(b) if b == b"OK" => Ok(true), - RespValue::Nil => Ok(false), - other => bail!("unexpected SET NX response: {:?}", other), - } - }) +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RedisOperation { + Get, + Set, + SetTtl, + Delete, + Increment, + Lock, + Unlock, + Scan, +} + +impl RedisOperation { + const fn as_str(self) -> &'static str { + match self { + Self::Get => "get", + Self::Set => "set", + Self::SetTtl => "set_ttl", + Self::Delete => "delete", + Self::Increment => "increment", + Self::Lock => "lock", + Self::Unlock => "unlock", + Self::Scan => "scan", + } } +} - fn unlock(&self, key: &[u8], token: &[u8]) -> Result<()> { - // Compare-and-delete via EVAL so we only delete the lock while it is - // still ours: a bare DEL could remove a lock a different node - // acquired after this one's lease had expired. - const RELEASE: &[u8] = b"if redis.call('get', KEYS[1]) == ARGV[1] then return redis.call('del', KEYS[1]) else return 0 end"; - let encoded = Self::encode_key(key); - self.with_conn(|c| { - c.call(&[b"EVAL", RELEASE, b"1", encoded.as_bytes(), token])?; - Ok(()) - }) +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RedisFailureReason { + PoolTimeout, + ConnectTimeout, + CommandTimeout, + Tls, + Auth, + Transport, + Server, + Protocol, +} + +impl RedisFailureReason { + const fn as_str(self) -> &'static str { + match self { + Self::PoolTimeout => "pool_timeout", + Self::ConnectTimeout => "connect_timeout", + Self::CommandTimeout => "command_timeout", + Self::Tls => "tls", + Self::Auth => "auth", + Self::Transport => "transport", + Self::Server => "server", + Self::Protocol => "protocol", + } } +} - fn scan_prefix(&self, prefix: &[u8]) -> Result> { - // Build a SCAN MATCH glob: hex(prefix)* (safe because hex output - // contains only [0-9a-f] which has no glob special characters). - let pattern = format!("{}*", Self::encode_key(prefix)); +fn redis_connection_results() -> &'static IntCounterVec { + static COUNTER: OnceLock = OnceLock::new(); + COUNTER.get_or_init(|| { + prometheus::register_int_counter_vec!( + "sbproxy_redis_kv_connections_total", + "Redis KV connection attempts by result.", + &["result"] + ) + .expect("Redis connection metric must register") + }) +} - let mut results = Vec::new(); - let mut cursor = b"0".to_vec(); +fn redis_operation_duration() -> &'static HistogramVec { + static HISTOGRAM: OnceLock = OnceLock::new(); + HISTOGRAM.get_or_init(|| { + prometheus::register_histogram_vec!( + "sbproxy_redis_kv_operation_duration_seconds", + "Redis KV operation duration in seconds.", + &["operation"] + ) + .expect("Redis operation duration metric must register") + }) +} - loop { - let (next_cursor, keys) = self.with_conn(|c| { - let resp = c.call(&[ - b"SCAN", - &cursor, - b"MATCH", - pattern.as_bytes(), - b"COUNT", - b"100", - ])?; - match resp { - RespValue::Array(mut elems) if elems.len() == 2 => { - let keys_resp = elems.pop().ok_or_else(|| { - anyhow::anyhow!("redis SCAN returned malformed response") - })?; - let cursor_resp = elems.pop().ok_or_else(|| { - anyhow::anyhow!("redis SCAN returned malformed response") - })?; - - let next_cursor = match cursor_resp { - RespValue::Bytes(b) => b, - _ => bail!("unexpected cursor type"), - }; - - let keys = match keys_resp { - RespValue::Array(items) => items - .into_iter() - .filter_map(|v| match v { - RespValue::Bytes(b) => Some(b), - _ => None, - }) - .collect::>(), - _ => bail!("unexpected keys type"), - }; - - Ok((next_cursor, keys)) - } - _ => bail!("unexpected SCAN response format"), - } - })?; +fn redis_operation_errors() -> &'static IntCounterVec { + static COUNTER: OnceLock = OnceLock::new(); + COUNTER.get_or_init(|| { + prometheus::register_int_counter_vec!( + "sbproxy_redis_kv_operation_errors_total", + "Redis KV operation failures by operation and reason.", + &["operation", "reason"] + ) + .expect("Redis operation error metric must register") + }) +} + +#[derive(Debug)] +struct SafeRedisError { + operation: RedisOperation, + reason: RedisFailureReason, +} - for hex_key in keys { - let raw_key = hex::decode(&hex_key).with_context(|| "decode hex key")?; +impl fmt::Display for SafeRedisError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + formatter, + "redis {} failed: {}", + self.operation.as_str(), + self.reason.as_str() + ) + } +} - // Fetch the value. - let value = self.with_conn(|c| match c.call(&[b"GET", &hex_key])? { - RespValue::Bytes(b) => Ok(Some(Bytes::from(b))), - RespValue::Nil => Ok(None), - other => bail!("unexpected GET response: {:?}", other), - })?; +impl StdError for SafeRedisError {} - if let Some(value) = value { - results.push((Bytes::from(raw_key), value)); - } - } +fn safe_error(operation: RedisOperation, reason: RedisFailureReason) -> anyhow::Error { + anyhow::Error::new(SafeRedisError { operation, reason }) +} - if next_cursor == b"0" { - break; +#[derive(Clone, Copy)] +enum ErrorPhase { + Connect, + Command, +} + +fn classify_redis_error( + error: &RedisError, + phase: ErrorPhase, + uses_tls: bool, +) -> RedisFailureReason { + if error.is_timeout() { + return match phase { + ErrorPhase::Connect => RedisFailureReason::ConnectTimeout, + ErrorPhase::Command => RedisFailureReason::CommandTimeout, + }; + } + + match error.kind() { + ErrorKind::AuthenticationFailed => RedisFailureReason::Auth, + ErrorKind::IoError => { + if uses_tls && has_tls_error_source(error, phase) { + RedisFailureReason::Tls + } else { + RedisFailureReason::Transport } - cursor = next_cursor; } + ErrorKind::InvalidClientConfig + | ErrorKind::ParseError + | ErrorKind::TypeError + | ErrorKind::ClientError + | ErrorKind::RESP3NotSupported => RedisFailureReason::Protocol, + _ => RedisFailureReason::Server, + } +} - Ok(results) +fn has_tls_error_source(error: &RedisError, phase: ErrorPhase) -> bool { + let mut source = StdError::source(error); + while let Some(cause) = source { + if cause.is::() { + return true; + } + if cause + .downcast_ref::() + .and_then(std::io::Error::get_ref) + .is_some_and(|inner| inner.is::()) + { + return true; + } + source = cause.source(); } + + // Redis converts direct rustls setup failures to detail-backed IoError + // values before a connection exists. Presence is sufficient here; never + // inspect or expose the potentially sensitive detail text. + matches!(phase, ErrorPhase::Connect) && error.detail().is_some() +} + +fn should_invalidate(error: &RedisError) -> bool { + error.is_timeout() + || error.is_io_error() + || error.is_connection_dropped() + || error.is_unrecoverable_error() + || matches!( + error.kind(), + ErrorKind::InvalidClientConfig + | ErrorKind::ParseError + | ErrorKind::TypeError + | ErrorKind::ClientError + | ErrorKind::RESP3NotSupported + ) } #[cfg(test)] mod tests { use super::*; - use std::sync::Arc; + use std::collections::VecDeque; + use std::io::{self, BufRead, BufReader, Read, Write}; + use std::net::{Shutdown, TcpListener, TcpStream}; + use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + use std::sync::{Arc, MutexGuard}; + use std::thread::{self, JoinHandle}; + + use crate::storage::{RedisTlsConfig, ValidatedRedisConnection}; + use tracing::field::{Field, Visit}; + use tracing::span::{Attributes, Id, Record}; + use tracing::{Event, Level, Metadata, Subscriber}; + + static TEST_LOCK: Mutex<()> = Mutex::new(()); + + const RELEASE_LOCK_SCRIPT: &[u8] = b"if redis.call('get', KEYS[1]) == ARGV[1] then return redis.call('del', KEYS[1]) else return 0 end"; + + #[derive(Debug)] + enum ScriptedReply { + Resp(Vec), + Close, + } - fn store() -> RedisKVStore { - RedisKVStore::new(RedisConfig::default()) + #[derive(Default)] + struct ScriptedState { + accepts: usize, + commands: Vec>>, + replies: VecDeque, + problems: Vec, } - #[test] - #[ignore = "requires a running Redis instance on 127.0.0.1:6379"] - fn test_get_put_delete_roundtrip() { - let s = store(); - s.delete(b"redis:test:k1").unwrap(); // clean up if leftover + struct ScriptedRedis { + address: String, + state: Arc>, + stop: Arc, + thread: Option>, + } - assert!(s.get(b"redis:test:k1").unwrap().is_none()); + impl ScriptedRedis { + fn start(replies: Vec) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + listener.set_nonblocking(true).unwrap(); + let address = listener.local_addr().unwrap().to_string(); + let state = Arc::new(Mutex::new(ScriptedState { + replies: replies.into(), + ..ScriptedState::default() + })); + let stop = Arc::new(AtomicBool::new(false)); + let thread_state = Arc::clone(&state); + let thread_stop = Arc::clone(&stop); + let thread = thread::spawn(move || { + while !thread_stop.load(Ordering::Acquire) { + match listener.accept() { + Ok((stream, _)) => { + thread_state.lock().unwrap().accepts += 1; + serve_connection(stream, &thread_state, &thread_stop); + } + Err(error) if error.kind() == io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(2)); + } + Err(error) => { + thread_state + .lock() + .unwrap() + .problems + .push(format!("accept failed: {error}")); + break; + } + } + } + }); - s.put(b"redis:test:k1", b"hello").unwrap(); - assert_eq!(s.get(b"redis:test:k1").unwrap().unwrap(), &b"hello"[..]); + Self { + address, + state, + stop, + thread: Some(thread), + } + } - s.put(b"redis:test:k1", b"world").unwrap(); - assert_eq!(s.get(b"redis:test:k1").unwrap().unwrap(), &b"world"[..]); + fn address(&self) -> &str { + &self.address + } - s.delete(b"redis:test:k1").unwrap(); - assert!(s.get(b"redis:test:k1").unwrap().is_none()); + fn accepts(&self) -> usize { + self.state.lock().unwrap().accepts + } - s.delete(b"redis:test:k1").unwrap(); // no-op - } + fn commands(&self) -> Vec>> { + self.state.lock().unwrap().commands.clone() + } - #[test] - #[ignore = "requires a running Redis instance on 127.0.0.1:6379"] - fn test_scan_prefix() { - let s = store(); - let keys: &[&[u8]] = &[b"redis:scan:a", b"redis:scan:b", b"redis:other:c"]; - for k in keys { - s.delete(k).unwrap(); + fn enqueue(&self, replies: Vec) { + self.state.lock().unwrap().replies.extend(replies); } - s.put(b"redis:scan:a", b"1").unwrap(); - s.put(b"redis:scan:b", b"2").unwrap(); - s.put(b"redis:other:c", b"3").unwrap(); + fn application_commands(&self) -> Vec>> { + self.commands() + .into_iter() + .filter(|command| { + !matches!( + command.first().map(Vec::as_slice), + Some(b"AUTH" | b"SELECT" | b"CLIENT" | b"HELLO") + ) + }) + .collect() + } - let results = s.scan_prefix(b"redis:scan:").unwrap(); - assert_eq!(results.len(), 2); + fn assert_finished(&self) { + let state = self.state.lock().unwrap(); + assert!( + state.replies.is_empty(), + "unused replies: {:?}", + state.replies + ); + assert!( + state.problems.is_empty(), + "server problems: {:?}", + state.problems + ); + } + } - for k in keys { - s.delete(k).unwrap(); + impl Drop for ScriptedRedis { + fn drop(&mut self) { + self.stop.store(true, Ordering::Release); + let _ = TcpStream::connect(&self.address); + if let Some(thread) = self.thread.take() { + thread.join().unwrap(); + } } } - #[test] - #[ignore = "requires a running Redis instance on 127.0.0.1:6379"] - fn concurrent_ops_use_pool() { - // Many threads hitting the store concurrently should succeed without - // deadlocking or serializing on a single connection. - let s = Arc::new(store()); - let mut handles = Vec::new(); - for i in 0..32 { - let s = s.clone(); - handles.push(std::thread::spawn(move || { - let k = format!("redis:pool:k{}", i); - s.put(k.as_bytes(), b"v").unwrap(); - assert_eq!( - s.get(k.as_bytes()).unwrap().unwrap(), - Bytes::from_static(b"v") - ); - s.delete(k.as_bytes()).unwrap(); - })); + fn serve_connection(stream: TcpStream, state: &Arc>, stop: &AtomicBool) { + stream + .set_read_timeout(Some(Duration::from_millis(50))) + .unwrap(); + let mut reader = BufReader::new(stream); + loop { + match read_command(&mut reader) { + Ok(Some(command)) => { + let is_setup = matches!( + command.first().map(Vec::as_slice), + Some(b"AUTH" | b"SELECT" | b"CLIENT" | b"HELLO") + ); + let reply = { + let mut state = state.lock().unwrap(); + state.commands.push(command); + if is_setup { + ScriptedReply::Resp(b"+OK\r\n".to_vec()) + } else { + match state.replies.pop_front() { + Some(reply) => reply, + None => { + state.problems.push("missing scripted reply".to_string()); + ScriptedReply::Resp(b"-ERR unscripted command\r\n".to_vec()) + } + } + } + }; + match reply { + ScriptedReply::Resp(bytes) => { + if let Err(error) = reader.get_mut().write_all(&bytes) { + state + .lock() + .unwrap() + .problems + .push(format!("reply failed: {error}")); + return; + } + } + ScriptedReply::Close => { + let _ = reader.get_mut().shutdown(Shutdown::Both); + return; + } + } + } + Ok(None) => return, + Err(error) + if matches!( + error.kind(), + io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut + ) => + { + if stop.load(Ordering::Acquire) { + return; + } + } + Err(error) => { + state + .lock() + .unwrap() + .problems + .push(format!("command read failed: {error}")); + return; + } + } + } + } + + fn read_command(reader: &mut BufReader) -> io::Result>>> { + let mut marker = [0_u8; 1]; + match reader.read_exact(&mut marker) { + Ok(()) => {} + Err(error) if error.kind() == io::ErrorKind::UnexpectedEof => return Ok(None), + Err(error) => return Err(error), } - for h in handles { - h.join().unwrap(); + if marker != *b"*" { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "expected RESP array", + )); } + let count = read_resp_len(reader)?; + let mut command = Vec::with_capacity(count); + for _ in 0..count { + reader.read_exact(&mut marker)?; + if marker != *b"$" { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "expected RESP bulk string", + )); + } + let length = read_resp_len(reader)?; + let mut argument = vec![0_u8; length]; + reader.read_exact(&mut argument)?; + let mut ending = [0_u8; 2]; + reader.read_exact(&mut ending)?; + if ending != *b"\r\n" { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "invalid RESP bulk ending", + )); + } + command.push(argument); + } + Ok(Some(command)) + } + + fn read_resp_len(reader: &mut BufReader) -> io::Result { + let mut line = Vec::new(); + reader.read_until(b'\n', &mut line)?; + let digits = line + .strip_suffix(b"\r\n") + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "invalid RESP line"))?; + std::str::from_utf8(digits) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "non-UTF8 RESP length"))? + .parse() + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "invalid RESP length")) + } + + fn reply(bytes: &[u8]) -> ScriptedReply { + ScriptedReply::Resp(bytes.to_vec()) + } + + fn bulk_reply(bytes: &[u8]) -> ScriptedReply { + let mut response = format!("${}\r\n", bytes.len()).into_bytes(); + response.extend_from_slice(bytes); + response.extend_from_slice(b"\r\n"); + ScriptedReply::Resp(response) + } + + fn scan_reply(cursor: &[u8], keys: &[&[u8]]) -> ScriptedReply { + let mut response = format!("*2\r\n${}\r\n", cursor.len()).into_bytes(); + response.extend_from_slice(cursor); + response.extend_from_slice(b"\r\n"); + response.extend_from_slice(format!("*{}\r\n", keys.len()).as_bytes()); + for key in keys { + response.extend_from_slice(format!("${}\r\n", key.len()).as_bytes()); + response.extend_from_slice(key); + response.extend_from_slice(b"\r\n"); + } + ScriptedReply::Resp(response) + } + + fn test_guard() -> MutexGuard<'static, ()> { + TEST_LOCK.lock().unwrap_or_else(|error| error.into_inner()) + } + + fn store_for(server: &ScriptedRedis) -> RedisKVStore { + let connection = ValidatedRedisConnection::new( + &format!("redis://{}/0", server.address()), + RedisTlsConfig::default(), + ) + .unwrap(); + RedisKVStore::new(RedisConfig::new(connection)) } #[test] - fn pool_exhaustion_times_out() { - // A pool of size 1 with a held checkout must time out any second - // checkout after `acquire_timeout`. This test does not need a running - // Redis because we never execute a command on the held connection. - // - // However, `checkout()` does call `Connection::connect()` which does - // require a reachable TCP listener. If Redis is not running locally, - // the first `checkout()` itself will fail with a connect error, which - // is still a valid exhaustion-prevention behavior but not the path we - // want to exercise. Skip gracefully in that case. - let cfg = RedisConfig { - pool_size: 1, - acquire_timeout: Duration::from_millis(100), - ..Default::default() - }; - let s = RedisKVStore::new(cfg); + fn classifies_structural_tls_io_shapes_without_relabeling_plain_transport() { + let invalid_transport_record = RedisError::from(io::Error::new( + io::ErrorKind::InvalidData, + "sentinel invalid transport data", + )); + assert_eq!( + classify_redis_error(&invalid_transport_record, ErrorPhase::Connect, true), + RedisFailureReason::Transport + ); - let held = match s.checkout() { - Ok(c) => c, - Err(_) => { - // No Redis running; nothing to exhaust. - return; - } - }; + let nested_tls_record = RedisError::from(io::Error::new( + io::ErrorKind::InvalidData, + rustls::Error::General("sentinel TLS record failure".to_string()), + )); + let redis_source = StdError::source(&nested_tls_record).expect("redis io source"); + let io_source = redis_source + .downcast_ref::() + .expect("redis source is io error"); + assert!(io_source + .get_ref() + .is_some_and(|source| source.is::())); + assert_eq!( + classify_redis_error(&nested_tls_record, ErrorPhase::Connect, true), + RedisFailureReason::Tls + ); + assert_eq!( + classify_redis_error(&nested_tls_record, ErrorPhase::Connect, false), + RedisFailureReason::Transport + ); + + let other_transport = RedisError::from(io::Error::other("sentinel transport failure")); + assert_eq!( + classify_redis_error(&other_transport, ErrorPhase::Connect, true), + RedisFailureReason::Transport + ); - let start = Instant::now(); - let err = s.checkout(); - let elapsed = start.elapsed(); - assert!(err.is_err(), "second checkout should fail"); - assert!( - elapsed >= Duration::from_millis(90), - "expected to wait ~100ms, waited {:?}", - elapsed + let cause_less_tls = RedisError::from(( + ErrorKind::IoError, + "opaque handshake failure", + "sentinel detail".to_string(), + )); + assert_eq!( + classify_redis_error(&cause_less_tls, ErrorPhase::Connect, true), + RedisFailureReason::Tls ); - assert!( - elapsed < Duration::from_millis(500), - "waited too long: {:?}", - elapsed + assert_eq!( + classify_redis_error(&cause_less_tls, ErrorPhase::Command, true), + RedisFailureReason::Transport ); - // Dropping the held guard returns the connection to the pool so a - // subsequent checkout succeeds. + for kind in [ + io::ErrorKind::ConnectionRefused, + io::ErrorKind::ConnectionReset, + io::ErrorKind::NotConnected, + io::ErrorKind::UnexpectedEof, + io::ErrorKind::NotFound, + io::ErrorKind::NetworkUnreachable, + io::ErrorKind::HostUnreachable, + io::ErrorKind::AddrNotAvailable, + io::ErrorKind::PermissionDenied, + io::ErrorKind::Unsupported, + ] { + let transport = RedisError::from(io::Error::new(kind, "sentinel transport failure")); + assert_eq!( + classify_redis_error(&transport, ErrorPhase::Connect, true), + RedisFailureReason::Transport + ); + } + + let timeout = RedisError::from(io::Error::new( + io::ErrorKind::TimedOut, + "sentinel connect timeout", + )); + assert_eq!( + classify_redis_error(&timeout, ErrorPhase::Connect, true), + RedisFailureReason::ConnectTimeout + ); + + let invalid_client_config = RedisError::from(( + ErrorKind::InvalidClientConfig, + "sentinel client configuration failure", + )); + assert_eq!( + classify_redis_error(&invalid_client_config, ErrorPhase::Connect, true), + RedisFailureReason::Protocol + ); + } + + #[test] + fn construction_is_lazy_and_first_operation_uses_validated_client() { + let _guard = test_guard(); + let server = ScriptedRedis::start(vec![reply(b"$-1\r\n")]); + let dsn = format!( + "redis://sentinel-user:sentinel-password@{}/7", + server.address() + ); + let store = RedisKVStore::new(RedisConfig::from_dsn(&dsn).unwrap()); + + assert_eq!(server.accepts(), 0, "construction must not open a socket"); + assert_eq!(store.get(b"sentinel-first-key").unwrap(), None); + assert_eq!(server.accepts(), 1); + assert_eq!( + server.application_commands(), + vec![vec![ + b"GET".to_vec(), + hex::encode(b"sentinel-first-key").into_bytes() + ]] + ); + + let setup = server.commands(); + assert!(setup.iter().any(|command| command[0] == b"AUTH")); + assert!(setup + .iter() + .any(|command| command == &[b"SELECT".to_vec(), b"7".to_vec()])); + server.assert_finished(); + } + + #[test] + fn pool_exhaustion_uses_the_bounded_acquisition_deadline() { + let _guard = test_guard(); + let server = ScriptedRedis::start(Vec::new()); + let connection = ValidatedRedisConnection::new( + &format!("redis://{}/0", server.address()), + RedisTlsConfig::default(), + ) + .unwrap(); + let mut config = RedisConfig::new(connection); + config.pool_size = 1; + config.acquire_timeout = Duration::from_millis(50); + let store = RedisKVStore::new(config); + let held = store.checkout(RedisOperation::Get).unwrap(); + + let started_at = Instant::now(); + let error = store + .checkout(RedisOperation::Set) + .err() + .expect("second checkout must time out"); + let elapsed = started_at.elapsed(); + assert_eq!(error.to_string(), "redis set failed: pool_timeout"); + assert!(elapsed >= Duration::from_millis(40), "elapsed: {elapsed:?}"); + assert!(elapsed < Duration::from_millis(500), "elapsed: {elapsed:?}"); + assert!(!format_error_chain(&error).contains(server.address())); + drop(held); - let _ = s.checkout().expect("checkout after release"); + drop(store.checkout(RedisOperation::Get).unwrap()); + assert_eq!(server.accepts(), 1); + server.assert_finished(); + } + + #[test] + fn kv_commands_keep_hex_keys_and_typed_results() { + let _guard = test_guard(); + let server = ScriptedRedis::start(vec![ + bulk_reply(b"typed-value"), + reply(b"+OK\r\n"), + reply(b":1\r\n"), + reply(b"+OK\r\n"), + ]); + let store = store_for(&server); + let raw_key = b"sentinel:key:\x00\xff"; + let encoded = hex::encode(raw_key).into_bytes(); + + assert_eq!( + store.get(raw_key).unwrap(), + Some(Bytes::from_static(b"typed-value")) + ); + store.put(raw_key, b"sentinel-value").unwrap(); + store.delete(raw_key).unwrap(); + store + .put_with_ttl(raw_key, b"sentinel-ttl-value", 73) + .unwrap(); + + assert_eq!( + server.application_commands(), + vec![ + vec![b"GET".to_vec(), encoded.clone()], + vec![b"SET".to_vec(), encoded.clone(), b"sentinel-value".to_vec()], + vec![b"DEL".to_vec(), encoded.clone()], + vec![ + b"SET".to_vec(), + encoded, + b"sentinel-ttl-value".to_vec(), + b"EX".to_vec(), + b"73".to_vec(), + ], + ] + ); + server.assert_finished(); + } + + #[test] + fn increment_remains_one_atomic_multi_exec_pipeline() { + let _guard = test_guard(); + let server = ScriptedRedis::start(vec![ + reply(b"+OK\r\n"), + reply(b"+QUEUED\r\n"), + reply(b"+QUEUED\r\n"), + reply(b"*2\r\n:41\r\n:1\r\n"), + ]); + let store = store_for(&server); + let raw_key = b"sentinel-increment-key"; + let encoded = hex::encode(raw_key).into_bytes(); + + assert_eq!(store.incr_with_ttl(raw_key, 29).unwrap(), 41); + assert_eq!( + server.application_commands(), + vec![ + vec![b"MULTI".to_vec()], + vec![b"INCR".to_vec(), encoded.clone()], + vec![b"EXPIRE".to_vec(), encoded, b"29".to_vec()], + vec![b"EXEC".to_vec()], + ] + ); + server.assert_finished(); + } + + #[test] + fn locks_and_scan_keep_existing_wire_contract() { + let _guard = test_guard(); + let prefix = b"sentinel-scan-prefix:"; + let raw_key_a = b"sentinel-scan-prefix:a"; + let raw_key_b = b"sentinel-scan-prefix:b"; + let encoded_a = hex::encode(raw_key_a).into_bytes(); + let encoded_b = hex::encode(raw_key_b).into_bytes(); + let server = ScriptedRedis::start(vec![ + reply(b"+OK\r\n"), + reply(b"$-1\r\n"), + reply(b":1\r\n"), + scan_reply(b"0", &[&encoded_a, &encoded_b]), + bulk_reply(b"sentinel-scan-value"), + reply(b"$-1\r\n"), + ]); + let store = store_for(&server); + let lock_key = b"sentinel-lock-key"; + let encoded_lock = hex::encode(lock_key).into_bytes(); + + assert!(store + .try_lock(lock_key, b"sentinel-lock-token-a", 11) + .unwrap()); + assert!(!store + .try_lock(lock_key, b"sentinel-lock-token-b", 11) + .unwrap()); + store.unlock(lock_key, b"sentinel-lock-token-a").unwrap(); + assert_eq!( + store.scan_prefix(prefix).unwrap(), + vec![( + Bytes::from_static(raw_key_a), + Bytes::from_static(b"sentinel-scan-value") + )] + ); + + assert_eq!( + server.application_commands(), + vec![ + vec![ + b"SET".to_vec(), + encoded_lock.clone(), + b"sentinel-lock-token-a".to_vec(), + b"NX".to_vec(), + b"PX".to_vec(), + b"11000".to_vec(), + ], + vec![ + b"SET".to_vec(), + encoded_lock.clone(), + b"sentinel-lock-token-b".to_vec(), + b"NX".to_vec(), + b"PX".to_vec(), + b"11000".to_vec(), + ], + vec![ + b"EVAL".to_vec(), + RELEASE_LOCK_SCRIPT.to_vec(), + b"1".to_vec(), + encoded_lock, + b"sentinel-lock-token-a".to_vec(), + ], + vec![ + b"SCAN".to_vec(), + b"0".to_vec(), + b"MATCH".to_vec(), + format!("{}*", hex::encode(prefix)).into_bytes(), + b"COUNT".to_vec(), + b"100".to_vec(), + ], + vec![b"GET".to_vec(), encoded_a], + vec![b"GET".to_vec(), encoded_b], + ] + ); + server.assert_finished(); + } + + #[test] + fn transport_failure_discards_connection_and_next_operation_reconnects() { + let _guard = test_guard(); + let server = ScriptedRedis::start(vec![ScriptedReply::Close, reply(b"$-1\r\n")]); + let store = store_for(&server); + let key = b"sentinel-reconnect-key"; + + let error = store.get(key).unwrap_err(); + let rendered = format_error_chain(&error); + for forbidden in [server.address(), "sentinel-reconnect-key"] { + assert!( + !rendered.contains(forbidden), + "leaked {forbidden}: {rendered}" + ); + } + assert_eq!(store.get(key).unwrap(), None); + assert_eq!(server.accepts(), 2); + assert_eq!(server.application_commands().len(), 2); + server.assert_finished(); + } + + #[test] + fn server_error_is_sanitized_and_does_not_expose_command_or_key() { + let _guard = test_guard(); + let raw_key = b"sentinel-server-error-key"; + let encoded = hex::encode(raw_key); + let server_error = format!( + "-ERR sentinel-server-error sentinel-command sentinel-server-error-key {encoded}\r\n" + ); + let server = ScriptedRedis::start(vec![ScriptedReply::Resp(server_error.into_bytes())]); + let dsn = format!( + "redis://sentinel-user:sentinel-password@{}/0", + server.address() + ); + let store = RedisKVStore::new(RedisConfig::from_dsn(&dsn).unwrap()); + + let error = store.get(raw_key).unwrap_err(); + let rendered = format_error_chain(&error); + assert_eq!(error.to_string(), "redis get failed: server"); + assert_eq!(error.chain().count(), 1, "must not retain a source error"); + for forbidden in [ + dsn.as_str(), + server.address(), + "sentinel-user", + "sentinel-password", + "sentinel-command", + "sentinel-server-error", + "sentinel-server-error-key", + encoded.as_str(), + ] { + assert!( + !rendered.contains(forbidden), + "leaked {forbidden}: {rendered}" + ); + } + server.assert_finished(); } #[test] - #[ignore = "requires a running Redis instance on 127.0.0.1:6379"] - fn try_lock_is_exclusive_and_release_is_token_scoped() { - // WOR-1774: the distributed issuance lock. Exercises SET NX PX + - // the Lua compare-and-delete release against a real Redis. - let s = RedisKVStore::new(RedisConfig::default()); - let key = b"test:wor1774:issue-lock"; - s.delete(key).ok(); - - // First holder acquires; a different token cannot while it is held. - assert!(s.try_lock(key, b"token-A", 30).unwrap(), "A acquires"); - assert!(!s.try_lock(key, b"token-B", 30).unwrap(), "B blocked"); - - // A non-owner release is a no-op (token mismatch): still held. - s.unlock(key, b"token-B").unwrap(); - assert!( - !s.try_lock(key, b"token-C", 30).unwrap(), - "still held after non-owner release" + fn records_only_closed_connection_and_operation_labels() { + let _guard = test_guard(); + let server = ScriptedRedis::start(Vec::new()); + let key = b"sentinel-metric-key"; + let encoded_key = hex::encode(key); + let endpoint = server.address().to_string(); + let dsn = format!("redis://sentinel-metric-user:sentinel-metric-password@{endpoint}/7"); + let sentinel_error = "sentinel-metric-server-error"; + server.enqueue(vec![ + reply(b"$-1\r\n"), + ScriptedReply::Resp( + format!( + "-ERR {sentinel_error} {endpoint} sentinel-metric-key sentinel-metric-value\r\n" + ) + .into_bytes(), + ), + ]); + let store = RedisKVStore::new(RedisConfig::from_dsn(&dsn).unwrap()); + let connection_before = metric_counter( + "sbproxy_redis_kv_connections_total", + &[("result", "success")], + ); + let get_before = metric_histogram_count( + "sbproxy_redis_kv_operation_duration_seconds", + &[("operation", "get")], + ); + let set_before = metric_histogram_count( + "sbproxy_redis_kv_operation_duration_seconds", + &[("operation", "set")], + ); + let error_before = metric_counter( + "sbproxy_redis_kv_operation_errors_total", + &[("operation", "set"), ("reason", "server")], + ); + + let (error, events) = capture_events(|| { + assert_eq!(store.get(key).unwrap(), None); + store.put(key, b"sentinel-metric-value").unwrap_err() + }); + + assert_eq!( + metric_counter( + "sbproxy_redis_kv_connections_total", + &[("result", "success")], + ), + connection_before + 1.0 + ); + assert_eq!( + metric_histogram_count( + "sbproxy_redis_kv_operation_duration_seconds", + &[("operation", "get")], + ), + get_before + 1 + ); + assert_eq!( + metric_histogram_count( + "sbproxy_redis_kv_operation_duration_seconds", + &[("operation", "set")], + ), + set_before + 1 ); + assert_eq!( + metric_counter( + "sbproxy_redis_kv_operation_errors_total", + &[("operation", "set"), ("reason", "server")], + ), + error_before + 1.0 + ); + + let forbidden = [ + dsn.as_str(), + endpoint.as_str(), + "127.0.0.1", + "sentinel-metric-user", + "sentinel-metric-password", + "sentinel-metric-key", + encoded_key.as_str(), + "sentinel-metric-value", + sentinel_error, + ]; + assert_closed_metric_labels(&forbidden); + assert_private_observations(&[error], &events, &forbidden); + server.assert_finished(); + } - // The owner releases; the lock is now free to acquire again. - s.unlock(key, b"token-A").unwrap(); - assert!( - s.try_lock(key, b"token-D", 30).unwrap(), - "free after owner release" + #[test] + fn repeated_failure_does_not_repeat_warning_transition() { + let _guard = test_guard(); + let server = ScriptedRedis::start(Vec::new()); + let endpoint = server.address().to_string(); + let dsn = format!("redis://sentinel-repeated-user:sentinel-repeated-password@{endpoint}/7"); + let key = b"sentinel-repeated-failure-key"; + let encoded_key = hex::encode(key); + let sentinel_error = "sentinel-repeated-server-error"; + let error_reply = format!( + "-ERR {sentinel_error} {endpoint} sentinel-repeated-failure-key {encoded_key}\r\n" + ) + .into_bytes(); + server.enqueue(vec![ + ScriptedReply::Resp(error_reply.clone()), + ScriptedReply::Resp(error_reply), + ]); + let store = RedisKVStore::new(RedisConfig::from_dsn(&dsn).unwrap()); + + let (errors, events) = + capture_events(|| vec![store.get(key).unwrap_err(), store.get(key).unwrap_err()]); + let redis_events = redis_events(&events); + assert_eq!(count_level(&redis_events, Level::WARN), 1); + assert_eq!(count_level(&redis_events, Level::DEBUG), 1); + assert_eq!(count_level(&redis_events, Level::INFO), 0); + assert_eq!( + server.accepts(), + 1, + "server errors keep the connection idle" ); - s.unlock(key, b"token-D").unwrap(); + + let forbidden = [ + dsn.as_str(), + endpoint.as_str(), + "127.0.0.1", + "sentinel-repeated-user", + "sentinel-repeated-password", + "sentinel-repeated-failure-key", + encoded_key.as_str(), + sentinel_error, + ]; + assert_private_observations(&errors, &events, &forbidden); + server.assert_finished(); + } + + #[test] + fn recovery_moves_failed_health_back_to_healthy_once() { + let _guard = test_guard(); + let server = ScriptedRedis::start(Vec::new()); + let endpoint = server.address().to_string(); + let dsn = format!("redis://sentinel-recovery-user:sentinel-recovery-password@{endpoint}/7"); + let key = b"sentinel-recovery-key"; + let encoded_key = hex::encode(key); + let sentinel_error = "sentinel-recovery-server-error"; + server.enqueue(vec![ + ScriptedReply::Resp( + format!("-ERR {sentinel_error} {endpoint} sentinel-recovery-key {encoded_key}\r\n") + .into_bytes(), + ), + reply(b"$-1\r\n"), + reply(b"$-1\r\n"), + ]); + let store = RedisKVStore::new(RedisConfig::from_dsn(&dsn).unwrap()); + + let (errors, events) = capture_events(|| { + let error = store.get(key).unwrap_err(); + assert_eq!(store.get(key).unwrap(), None); + assert_eq!(store.get(key).unwrap(), None); + vec![error] + }); + let redis_events = redis_events(&events); + assert_eq!(count_level(&redis_events, Level::WARN), 1); + assert_eq!(count_level(&redis_events, Level::INFO), 1); + assert_eq!(count_level(&redis_events, Level::DEBUG), 0); + + let forbidden = [ + dsn.as_str(), + endpoint.as_str(), + "127.0.0.1", + "sentinel-recovery-user", + "sentinel-recovery-password", + "sentinel-recovery-key", + encoded_key.as_str(), + sentinel_error, + ]; + assert_private_observations(&errors, &events, &forbidden); + server.assert_finished(); + } + + #[derive(Clone, Debug)] + struct CapturedEvent { + level: Level, + target: String, + fields: String, + } + + struct CaptureSubscriber { + events: Arc>>, + next_span: AtomicU64, + } + + impl Subscriber for CaptureSubscriber { + fn enabled(&self, _metadata: &Metadata<'_>) -> bool { + true + } + + fn new_span(&self, _span: &Attributes<'_>) -> Id { + Id::from_u64(self.next_span.fetch_add(1, Ordering::Relaxed) + 1) + } + + fn record(&self, _span: &Id, _values: &Record<'_>) {} + + fn record_follows_from(&self, _span: &Id, _follows: &Id) {} + + fn event(&self, event: &Event<'_>) { + let mut visitor = FieldVisitor::default(); + event.record(&mut visitor); + self.events.lock().unwrap().push(CapturedEvent { + level: *event.metadata().level(), + target: event.metadata().target().to_string(), + fields: visitor.fields, + }); + } + + fn enter(&self, _span: &Id) {} + + fn exit(&self, _span: &Id) {} + + fn max_level_hint(&self) -> Option { + Some(tracing::metadata::LevelFilter::TRACE) + } + } + + #[derive(Default)] + struct FieldVisitor { + fields: String, + } + + impl Visit for FieldVisitor { + fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) { + use fmt::Write as _; + let _ = write!(&mut self.fields, "{}={value:?};", field.name()); + } + } + + fn capture_events(function: impl FnOnce() -> T) -> (T, Vec) { + let events = Arc::new(Mutex::new(Vec::new())); + let subscriber = CaptureSubscriber { + events: Arc::clone(&events), + next_span: AtomicU64::new(0), + }; + let value = tracing::subscriber::with_default(subscriber, function); + let captured = events.lock().unwrap().clone(); + (value, captured) + } + + fn redis_events(events: &[CapturedEvent]) -> Vec<&CapturedEvent> { + events + .iter() + .filter(|event| event.target.ends_with("storage::redis")) + .collect() + } + + fn count_level(events: &[&CapturedEvent], level: Level) -> usize { + events.iter().filter(|event| event.level == level).count() + } + + fn assert_private_observations( + errors: &[anyhow::Error], + events: &[CapturedEvent], + forbidden: &[&str], + ) { + let errors = errors + .iter() + .map(format_error_chain) + .collect::>() + .join(" | "); + let logs = events + .iter() + .map(|event| format!("{} {} {}", event.level, event.target, event.fields)) + .collect::>() + .join(" | "); + for sentinel in forbidden { + assert!( + !errors.contains(sentinel), + "error leaked {sentinel}: {errors}" + ); + assert!(!logs.contains(sentinel), "log leaked {sentinel}: {logs}"); + } + } + + fn assert_closed_metric_labels(forbidden: &[&str]) { + let expected = [ + ("sbproxy_redis_kv_connections_total", &["result"][..]), + ( + "sbproxy_redis_kv_operation_duration_seconds", + &["operation"][..], + ), + ( + "sbproxy_redis_kv_operation_errors_total", + &["operation", "reason"][..], + ), + ]; + let families = prometheus::gather(); + for (family_name, expected_names) in expected { + let family = families + .iter() + .find(|family| family.name() == family_name) + .unwrap_or_else(|| panic!("missing metric family {family_name}")); + assert!(!family.get_metric().is_empty()); + for metric in family.get_metric() { + let mut names = metric + .get_label() + .iter() + .map(|label| label.name()) + .collect::>(); + names.sort_unstable(); + let mut expected_names = expected_names.to_vec(); + expected_names.sort_unstable(); + assert_eq!(names, expected_names, "labels for {family_name}"); + for label in metric.get_label() { + let allowed = match label.name() { + "result" => matches!(label.value(), "success" | "error"), + "operation" => matches!( + label.value(), + "get" + | "set" + | "set_ttl" + | "delete" + | "increment" + | "lock" + | "unlock" + | "scan" + ), + "reason" => matches!( + label.value(), + "pool_timeout" + | "connect_timeout" + | "command_timeout" + | "tls" + | "auth" + | "transport" + | "server" + | "protocol" + ), + _ => false, + }; + assert!( + allowed, + "unbounded label {}={}", + label.name(), + label.value() + ); + for sentinel in forbidden { + assert!( + !label.name().contains(sentinel) && !label.value().contains(sentinel), + "metric label leaked {sentinel}" + ); + } + } + } + } + } + + fn metric_counter(name: &str, labels: &[(&str, &str)]) -> f64 { + prometheus::gather() + .into_iter() + .find(|family| family.name() == name) + .and_then(|family| { + family + .get_metric() + .iter() + .find(|metric| metric_has_labels(metric, labels)) + .map(|metric| metric.get_counter().value()) + }) + .unwrap_or(0.0) + } + + fn metric_histogram_count(name: &str, labels: &[(&str, &str)]) -> u64 { + prometheus::gather() + .into_iter() + .find(|family| family.name() == name) + .and_then(|family| { + family + .get_metric() + .iter() + .find(|metric| metric_has_labels(metric, labels)) + .map(|metric| metric.get_histogram().get_sample_count()) + }) + .unwrap_or(0) + } + + fn metric_has_labels(metric: &prometheus::proto::Metric, labels: &[(&str, &str)]) -> bool { + labels.iter().all(|(name, value)| { + metric + .get_label() + .iter() + .any(|label| label.name() == *name && label.value() == *value) + }) + } + + fn format_error_chain(error: &anyhow::Error) -> String { + error + .chain() + .map(ToString::to_string) + .collect::>() + .join(" | ") } } diff --git a/crates/sbproxy-platform/src/storage/redis_connection.rs b/crates/sbproxy-platform/src/storage/redis_connection.rs new file mode 100644 index 000000000..d2b4e5fb1 --- /dev/null +++ b/crates/sbproxy-platform/src/storage/redis_connection.rs @@ -0,0 +1,445 @@ +use std::fmt; + +use anyhow::{anyhow, Result}; +use rustls::sign::CertifiedKey; +use rustls_pki_types::{pem::PemObject, CertificateDer, PrivateKeyDer}; +use url::Url; + +const INVALID_CONNECTION: &str = "invalid Redis connection configuration"; + +/// Optional certificate material for a Redis TLS connection. +#[derive(Clone, Default)] +pub struct RedisTlsConfig { + /// Additional PEM-encoded root certificates. + pub root_cert: Option>, + /// PEM-encoded client certificate chain for mutual TLS. + pub client_cert: Option>, + /// PEM-encoded client private key for mutual TLS. + pub client_key: Option>, +} + +impl fmt::Debug for RedisTlsConfig { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("RedisTlsConfig") + .field("root_cert", &self.root_cert.is_some()) + .field("client_cert", &self.client_cert.is_some()) + .field("client_key", &self.client_key.is_some()) + .finish() + } +} + +/// A parsed Redis client whose connection material is redacted from debug output. +#[derive(Clone)] +pub struct ValidatedRedisConnection { + client: redis::Client, + uses_tls: bool, +} + +impl fmt::Debug for ValidatedRedisConnection { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("ValidatedRedisConnection") + .field("uses_tls", &self.uses_tls) + .finish() + } +} + +impl ValidatedRedisConnection { + /// Parse and validate a Redis DSN without opening a network connection. + pub fn new(dsn: &str, tls: RedisTlsConfig) -> Result { + Self::build(dsn, tls).map_err(|_| anyhow!(INVALID_CONNECTION)) + } + + fn build(dsn: &str, tls: RedisTlsConfig) -> Result { + let trimmed = dsn.trim(); + let has_scheme = trimmed.contains("://"); + anyhow::ensure!(has_scheme || !trimmed.contains('@')); + let is_legacy = !trimmed.contains("://") + && !trimmed.contains('/') + && !trimmed.contains('?') + && !trimmed.contains('#'); + let normalized = if is_legacy { + validate_legacy_authority(trimmed)?; + format!("redis://{trimmed}") + } else { + trimmed.to_string() + }; + + validate_url_authority(&normalized)?; + let parsed = Url::parse(&normalized)?; + anyhow::ensure!(matches!(parsed.scheme(), "redis" | "rediss")); + anyhow::ensure!(parsed.host().is_some()); + anyhow::ensure!(parsed.query().is_none()); + anyhow::ensure!(parsed.fragment().is_none()); + anyhow::ensure!(parsed.username().is_empty() || parsed.password().is_some()); + + let uses_tls = parsed.scheme() == "rediss"; + let parsed_client = redis::Client::open(normalized.as_str())?; + anyhow::ensure!(parsed_client.get_connection_info().redis.db >= 0); + + let RedisTlsConfig { + root_cert, + client_cert, + client_key, + } = tls; + let has_tls_material = root_cert.is_some() || client_cert.is_some() || client_key.is_some(); + anyhow::ensure!(uses_tls || !has_tls_material); + + if let Some(root_cert) = root_cert.as_deref() { + let certificates = CertificateDer::pem_slice_iter(root_cert) + .collect::, _>>()?; + anyhow::ensure!(!certificates.is_empty()); + } + + let client_tls = match (client_cert, client_key) { + (None, None) => None, + (Some(client_cert), Some(client_key)) => { + let certificate_chain = CertificateDer::pem_slice_iter(&client_cert) + .collect::, _>>()?; + anyhow::ensure!(!certificate_chain.is_empty()); + let private_key = PrivateKeyDer::from_pem_slice(&client_key)?; + CertifiedKey::from_der( + certificate_chain, + private_key, + &rustls::crypto::ring::default_provider(), + )?; + Some(redis::ClientTlsConfig { + client_cert, + client_key, + }) + } + _ => anyhow::bail!(INVALID_CONNECTION), + }; + + let client = if has_tls_material { + redis::Client::build_with_tls( + normalized.as_str(), + redis::TlsCertificates { + client_tls, + root_cert, + }, + )? + } else { + parsed_client + }; + + Ok(Self { client, uses_tls }) + } + + /// Return a clone of the validated Redis client. + pub(crate) fn client(&self) -> redis::Client { + self.client.clone() + } + + /// Return whether the Redis DSN uses TLS. + pub fn uses_tls(&self) -> bool { + self.uses_tls + } +} + +fn validate_legacy_authority(authority: &str) -> Result<()> { + anyhow::ensure!(!authority.is_empty()); + + if let Some(bracketed) = authority.strip_prefix('[') { + let closing_bracket = bracketed + .find(']') + .ok_or_else(|| anyhow!(INVALID_CONNECTION))?; + anyhow::ensure!(closing_bracket > 0); + let suffix = &bracketed[closing_bracket + 1..]; + if !suffix.is_empty() { + let port = suffix + .strip_prefix(':') + .ok_or_else(|| anyhow!(INVALID_CONNECTION))?; + validate_explicit_port(port)?; + } + return Ok(()); + } + + anyhow::ensure!(!authority.contains(['[', ']'])); + match authority.split_once(':') { + Some((host, port)) => { + anyhow::ensure!(!host.is_empty() && !port.contains(':')); + validate_explicit_port(port) + } + None => Ok(()), + } +} + +fn validate_url_authority(dsn: &str) -> Result<()> { + let (_, remainder) = dsn + .split_once("://") + .ok_or_else(|| anyhow!(INVALID_CONNECTION))?; + let authority_end = remainder.find(['/', '?', '#']).unwrap_or(remainder.len()); + let authority = &remainder[..authority_end]; + anyhow::ensure!(!authority.is_empty()); + let host_and_port = authority + .rsplit_once('@') + .map_or(authority, |(_, host_and_port)| host_and_port); + anyhow::ensure!(!host_and_port.is_empty()); + + if let Some(bracketed) = host_and_port.strip_prefix('[') { + let closing_bracket = bracketed + .find(']') + .ok_or_else(|| anyhow!(INVALID_CONNECTION))?; + anyhow::ensure!(closing_bracket > 0); + let suffix = &bracketed[closing_bracket + 1..]; + if !suffix.is_empty() { + let port = suffix + .strip_prefix(':') + .ok_or_else(|| anyhow!(INVALID_CONNECTION))?; + validate_explicit_port(port)?; + } + } else if let Some((host, port)) = host_and_port.rsplit_once(':') { + anyhow::ensure!(!host.is_empty()); + validate_explicit_port(port)?; + } + + Ok(()) +} + +fn validate_explicit_port(port: &str) -> Result<()> { + anyhow::ensure!(!port.is_empty() && port.bytes().all(|byte| byte.is_ascii_digit())); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn certificate_and_key(name: &str) -> (Vec, Vec) { + let key = rcgen::KeyPair::generate().unwrap(); + let params = rcgen::CertificateParams::new(vec![name.to_string()]).unwrap(); + let certificate = params.self_signed(&key).unwrap(); + ( + certificate.pem().into_bytes(), + key.serialize_pem().into_bytes(), + ) + } + + #[test] + fn normalizes_legacy_addresses_without_network_io() { + for input in [ + "localhost", + "localhost:6380", + "127.0.0.1:6379", + "[::1]", + "[::1]:6379", + ] { + let config = ValidatedRedisConnection::new(input, RedisTlsConfig::default()).unwrap(); + assert!(!config.uses_tls()); + } + } + + #[test] + fn rejects_schemeless_credentials_and_malformed_legacy_authorities() { + for input in [ + "user:password@host", + ":password@host", + "@host", + "host@", + "[::1]suffix", + "[::1]suffix:6379", + "host:6379:6380", + "host::6379", + ] { + let error = + ValidatedRedisConnection::new(input, RedisTlsConfig::default()).unwrap_err(); + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + assert_eq!(error.chain().count(), 1); + } + } + + #[test] + fn rejects_empty_or_non_numeric_explicit_ports() { + for input in [ + "localhost:", + "localhost:not-a-port", + "[::1]:", + "[::1]:not-a-port", + "redis://localhost:/7", + "redis://localhost:not-a-port/7", + "redis://default:secret@localhost:/7", + "redis://[::1]:/7", + "redis://[::1]:not-a-port/7", + ] { + let error = + ValidatedRedisConnection::new(input, RedisTlsConfig::default()).unwrap_err(); + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + assert_eq!(error.chain().count(), 1); + } + } + + #[test] + fn cloned_client_retains_auth_ipv6_port_and_database_selection() { + let connection = ValidatedRedisConnection::new( + "redis://acl-user:p%40ss%2Fword@[::1]:6380/7", + RedisTlsConfig::default(), + ) + .unwrap(); + let client = connection.client(); + let info = client.get_connection_info(); + + let (host, port) = match &info.addr { + redis::ConnectionAddr::Tcp(host, port) => (host.as_str(), *port), + _ => panic!("expected a plaintext TCP Redis address"), + }; + assert_eq!(host, "::1"); + assert_eq!(port, 6380); + assert_eq!(info.redis.username.as_deref(), Some("acl-user")); + assert_eq!(info.redis.password.as_deref(), Some("p@ss/word")); + assert_eq!(info.redis.db, 7); + } + + #[test] + fn accepts_full_safe_url_semantics() { + for input in [ + "redis://:p%40ss@localhost:6379/7", + "redis://default:p%2Fss@[::1]:6379/4", + "rediss://default:secret@localhost:6380/2", + ] { + ValidatedRedisConnection::new(input, RedisTlsConfig::default()).unwrap(); + } + } + + #[test] + fn rejects_semantics_that_must_not_be_discarded() { + for input in [ + "", + "http://localhost:6379", + "redis://", + "redis://user@localhost:6379", + "redis://localhost:6379/-1", + "redis://localhost:6379/0?x=1", + "rediss://localhost:6380/0#insecure", + "::1:6379", + ] { + let error = + ValidatedRedisConnection::new(input, RedisTlsConfig::default()).unwrap_err(); + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + } + } + + #[test] + fn debug_and_errors_never_expose_connection_material() { + let sentinel = "user:sentinel-password@secret-host.invalid:6380/7"; + let config = ValidatedRedisConnection::new( + &format!("rediss://{sentinel}"), + RedisTlsConfig::default(), + ) + .unwrap(); + let rendered = format!("{config:?}"); + for forbidden in ["sentinel-password", "secret-host", "user", "/7"] { + assert!(!rendered.contains(forbidden)); + } + } + + #[test] + fn validates_complete_tls_material_without_network_io() { + let (certificate, key) = certificate_and_key("client.example"); + let connection = ValidatedRedisConnection::new( + "rediss://localhost:6380/3", + RedisTlsConfig { + root_cert: Some(certificate.clone()), + client_cert: Some(certificate), + client_key: Some(key), + }, + ) + .unwrap(); + + assert!(connection.uses_tls()); + } + + #[test] + fn rejects_tls_material_for_plaintext_connections() { + let (certificate, _) = certificate_and_key("root.example"); + let error = ValidatedRedisConnection::new( + "redis://localhost:6379/0", + RedisTlsConfig { + root_cert: Some(certificate), + ..RedisTlsConfig::default() + }, + ) + .unwrap_err(); + + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + } + + #[test] + fn rejects_incomplete_client_identity_pairs() { + let (certificate, key) = certificate_and_key("client.example"); + for tls in [ + RedisTlsConfig { + client_cert: Some(certificate), + ..RedisTlsConfig::default() + }, + RedisTlsConfig { + client_key: Some(key), + ..RedisTlsConfig::default() + }, + ] { + let error = + ValidatedRedisConnection::new("rediss://localhost:6380/0", tls).unwrap_err(); + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + } + } + + #[test] + fn rejects_empty_or_invalid_certificate_material() { + for tls in [ + RedisTlsConfig { + root_cert: Some(Vec::new()), + ..RedisTlsConfig::default() + }, + RedisTlsConfig { + root_cert: Some(b"not a PEM certificate".to_vec()), + ..RedisTlsConfig::default() + }, + RedisTlsConfig { + client_cert: Some(Vec::new()), + client_key: Some(b"not a PEM private key".to_vec()), + ..RedisTlsConfig::default() + }, + ] { + let error = + ValidatedRedisConnection::new("rediss://localhost:6380/0", tls).unwrap_err(); + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + } + } + + #[test] + fn rejects_mismatched_client_certificate_and_key() { + let (certificate, _) = certificate_and_key("client.example"); + let (_, other_key) = certificate_and_key("other.example"); + let error = ValidatedRedisConnection::new( + "rediss://localhost:6380/0", + RedisTlsConfig { + client_cert: Some(certificate), + client_key: Some(other_key), + ..RedisTlsConfig::default() + }, + ) + .unwrap_err(); + + assert_eq!(error.to_string(), "invalid Redis connection configuration"); + } + + #[test] + fn tls_config_debug_shows_only_material_presence() { + let sentinel = b"sentinel-private-certificate-material".to_vec(); + let rendered = format!( + "{:?}", + RedisTlsConfig { + root_cert: Some(sentinel.clone()), + client_cert: Some(sentinel.clone()), + client_key: Some(sentinel), + } + ); + + assert!(!rendered.contains("sentinel")); + assert_eq!( + rendered, + "RedisTlsConfig { root_cert: true, client_cert: true, client_key: true }" + ); + } +} diff --git a/crates/sbproxy-platform/tests/redis_secure.rs b/crates/sbproxy-platform/tests/redis_secure.rs new file mode 100644 index 000000000..fd4a115f9 --- /dev/null +++ b/crates/sbproxy-platform/tests/redis_secure.rs @@ -0,0 +1,567 @@ +mod support; + +use std::fs; +use std::net::{IpAddr, Ipv4Addr}; +use std::path::PathBuf; +use std::sync::{Arc, Barrier}; +use std::thread; +use std::time::{Duration, Instant}; + +use rcgen::{ + BasicConstraints, Certificate, CertificateParams, DistinguishedName, DnType, + ExtendedKeyUsagePurpose, IsCa, KeyPair, KeyUsagePurpose, SanType, +}; +use sbproxy_platform::storage::{ + KVStore, RedisConfig, RedisKVStore, RedisTlsConfig, ValidatedRedisConnection, +}; +use support::redis_server::{RedisProtocolAuditProxy, RedisServer, RedisTlsServerConfig}; +use tempfile::TempDir; + +const PASSWORD: &str = "p@ss:/?#[]"; +const ENCODED_PASSWORD: &str = "p%40ss%3A%2F%3F%23%5B%5D"; + +fn authenticated_dsn(port: u16, password: &str, database: u8) -> String { + format!("redis://default:{password}@127.0.0.1:{port}/{database}") +} + +fn store_for(dsn: &str, tls: RedisTlsConfig) -> RedisKVStore { + let connection = ValidatedRedisConnection::new(dsn, tls) + .unwrap_or_else(|_| panic!("secure Redis test connection configuration was rejected")); + let mut config = RedisConfig::new(connection); + config.pool_size = 1; + config.acquire_timeout = Duration::from_millis(500); + config.connect_timeout = Duration::from_millis(500); + config.command_timeout = Duration::from_millis(500); + RedisKVStore::new(config) +} + +fn pooled_store_for(dsn: &str, tls: RedisTlsConfig, pool_size: usize) -> RedisKVStore { + let connection = ValidatedRedisConnection::new(dsn, tls) + .unwrap_or_else(|_| panic!("secure Redis test connection configuration was rejected")); + let mut config = RedisConfig::new(connection); + config.pool_size = pool_size; + config.acquire_timeout = Duration::from_secs(2); + config.connect_timeout = Duration::from_secs(2); + config.command_timeout = Duration::from_secs(2); + RedisKVStore::new(config) +} + +struct TestPki { + _directory: TempDir, + server_cert_file: PathBuf, + server_key_file: PathBuf, + ca_cert_file: PathBuf, + ca_pem: Vec, + wrong_ca_pem: Vec, + client_cert_pem: Vec, + client_key_pem: Vec, +} + +impl TestPki { + fn generate() -> Self { + let directory = tempfile::tempdir() + .unwrap_or_else(|_| panic!("failed to create the temporary Redis PKI directory")); + let (ca, ca_key) = generate_ca("WOR-1946 Redis test CA"); + let (wrong_ca, _wrong_ca_key) = generate_ca("WOR-1946 wrong Redis test CA"); + + let server_key = KeyPair::generate() + .unwrap_or_else(|_| panic!("failed to generate the Redis server test key")); + let mut server_params = CertificateParams::new(Vec::::new()) + .unwrap_or_else(|_| panic!("failed to configure the Redis server test certificate")); + server_params.distinguished_name = DistinguishedName::new(); + server_params + .distinguished_name + .push(DnType::CommonName, "WOR-1946 Redis test server"); + server_params.subject_alt_names = vec![SanType::IpAddress(IpAddr::V4(Ipv4Addr::LOCALHOST))]; + server_params.key_usages = vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyEncipherment, + ]; + server_params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ServerAuth]; + let server_cert = server_params + .signed_by(&server_key, &ca, &ca_key) + .unwrap_or_else(|_| panic!("failed to sign the Redis server test certificate")); + + let client_key = KeyPair::generate() + .unwrap_or_else(|_| panic!("failed to generate the Redis client test key")); + let mut client_params = CertificateParams::new(Vec::::new()) + .unwrap_or_else(|_| panic!("failed to configure the Redis client test certificate")); + client_params.distinguished_name = DistinguishedName::new(); + client_params + .distinguished_name + .push(DnType::CommonName, "WOR-1946 Redis test client"); + client_params.key_usages = vec![KeyUsagePurpose::DigitalSignature]; + client_params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ClientAuth]; + let client_cert = client_params + .signed_by(&client_key, &ca, &ca_key) + .unwrap_or_else(|_| panic!("failed to sign the Redis client test certificate")); + + let server_cert_file = directory.path().join("redis-server.pem"); + let server_key_file = directory.path().join("redis-server-key.pem"); + let ca_cert_file = directory.path().join("redis-ca.pem"); + write_private_fixture(&server_cert_file, server_cert.pem().as_bytes()); + write_private_fixture(&server_key_file, server_key.serialize_pem().as_bytes()); + write_private_fixture(&ca_cert_file, ca.pem().as_bytes()); + + Self { + _directory: directory, + server_cert_file, + server_key_file, + ca_cert_file, + ca_pem: ca.pem().into_bytes(), + wrong_ca_pem: wrong_ca.pem().into_bytes(), + client_cert_pem: client_cert.pem().into_bytes(), + client_key_pem: client_key.serialize_pem().into_bytes(), + } + } + + fn server_config(&self) -> RedisTlsServerConfig { + RedisTlsServerConfig { + server_cert_file: self.server_cert_file.clone(), + server_key_file: self.server_key_file.clone(), + ca_cert_file: self.ca_cert_file.clone(), + readiness_root_cert: self.ca_pem.clone(), + readiness_client_cert: self.client_cert_pem.clone(), + readiness_client_key: self.client_key_pem.clone(), + } + } + + fn client_tls(&self) -> RedisTlsConfig { + RedisTlsConfig { + root_cert: Some(self.ca_pem.clone()), + client_cert: Some(self.client_cert_pem.clone()), + client_key: Some(self.client_key_pem.clone()), + } + } +} + +fn generate_ca(common_name: &str) -> (Certificate, KeyPair) { + let key = + KeyPair::generate().unwrap_or_else(|_| panic!("failed to generate a Redis test CA key")); + let mut params = CertificateParams::new(vec![common_name.to_string()]) + .unwrap_or_else(|_| panic!("failed to configure a Redis test CA")); + params.distinguished_name = DistinguishedName::new(); + params + .distinguished_name + .push(DnType::CommonName, common_name); + params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained); + params.key_usages = vec![ + KeyUsagePurpose::DigitalSignature, + KeyUsagePurpose::KeyCertSign, + KeyUsagePurpose::CrlSign, + ]; + let certificate = params + .self_signed(&key) + .unwrap_or_else(|_| panic!("failed to create a Redis test CA")); + (certificate, key) +} + +fn write_private_fixture(path: &std::path::Path, contents: &[u8]) { + fs::write(path, contents) + .unwrap_or_else(|_| panic!("failed to write an ephemeral Redis TLS fixture")); +} + +fn tls_dsn(port: u16) -> String { + format!("rediss://127.0.0.1:{port}/0") +} + +fn assert_safe_failure(error: anyhow::Error, expected: &str) { + assert_eq!(error.to_string(), expected); + assert_eq!(error.chain().count(), 1); + for forbidden in [PASSWORD, ENCODED_PASSWORD, "PRIVATE KEY", "CERTIFICATE"] { + assert!(!error.to_string().contains(forbidden)); + } +} + +fn raw_get(dsn: &str, key: &str, database: u8) -> Option> { + let client = redis::Client::open(dsn) + .unwrap_or_else(|_| panic!("failed to build the independent Redis verification client")); + let mut connection = client + .get_connection_with_timeout(Duration::from_millis(500)) + .unwrap_or_else(|_| panic!("independent Redis verification client failed to connect")); + redis::cmd("SELECT") + .arg(database) + .query::<()>(&mut connection) + .unwrap_or_else(|_| panic!("independent Redis verification client failed to select DB")); + redis::cmd("GET") + .arg(key) + .query(&mut connection) + .unwrap_or_else(|_| panic!("independent Redis verification client failed to read")) +} + +fn raw_put(dsn: &str, key: &str, value: &[u8], database: u8) { + let client = redis::Client::open(dsn) + .unwrap_or_else(|_| panic!("failed to build the independent Redis verification client")); + let mut connection = client + .get_connection_with_timeout(Duration::from_millis(500)) + .unwrap_or_else(|_| panic!("independent Redis verification client failed to connect")); + redis::cmd("SELECT") + .arg(database) + .query::<()>(&mut connection) + .unwrap_or_else(|_| panic!("independent Redis verification client failed to select DB")); + redis::cmd("SET") + .arg(key) + .arg(value) + .query::<()>(&mut connection) + .unwrap_or_else(|_| panic!("independent Redis verification client failed to write")); +} + +#[test] +#[ignore = "requires redis-server executable on PATH"] +fn auth_and_db7_preserve_percent_encoded_credentials_and_isolate_data() { + let server = RedisServer::spawn_authenticated(PASSWORD); + let dsn = authenticated_dsn(server.port(), ENCODED_PASSWORD, 7); + let store = store_for(&dsn, RedisTlsConfig::default()); + let key = b"wor-1946-db7-isolation"; + let value = b"stored-only-in-db7"; + + store + .put(key, value) + .unwrap_or_else(|_| panic!("authenticated DB 7 write failed")); + + let encoded_key = hex::encode(key); + assert_eq!(raw_get(&dsn, &encoded_key, 7), Some(value.to_vec())); + assert_eq!(raw_get(&dsn, &encoded_key, 0), None); +} + +#[test] +#[ignore = "requires redis-server executable on PATH"] +fn wrong_password_is_safe_auth_failure_without_anonymous_retry() { + let server = RedisServer::spawn_acl_fallback_trap("wor1946-test-user", PASSWORD); + let anonymous_dsn = format!("redis://127.0.0.1:{}/7", server.port()); + let key = b"must-not-run-anonymously"; + let encoded_key = hex::encode(key); + raw_put( + &anonymous_dsn, + &encoded_key, + b"anonymous-fallback-would-read-this", + 7, + ); + assert_eq!( + raw_get(&anonymous_dsn, &encoded_key, 7), + Some(b"anonymous-fallback-would-read-this".to_vec()) + ); + + let wrong_dsn = format!( + "redis://wor1946-test-user:definitely-wrong@127.0.0.1:{}/7", + server.port() + ); + let store = store_for(&wrong_dsn, RedisTlsConfig::default()); + + let error = store.get(key).expect_err("wrong Redis password must fail"); + + assert_eq!(error.to_string(), "redis get failed: auth"); + assert_eq!(error.chain().count(), 1); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn private_ca_and_required_client_identity_succeed() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = store_for(&tls_dsn(server.port()), pki.client_tls()); + + store + .put(b"mtls-key", b"mtls-value") + .unwrap_or_else(|_| panic!("private-CA Redis mTLS write failed")); + assert_eq!( + store + .get(b"mtls-key") + .unwrap_or_else(|_| panic!("private-CA Redis mTLS read failed")) + .as_deref(), + Some(&b"mtls-value"[..]) + ); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn private_ca_mtls_ttl_value_expires_within_a_bounded_window() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = store_for(&tls_dsn(server.port()), pki.client_tls()); + let key = b"wor-1946-live-ttl"; + + store + .put_with_ttl(key, b"expires", 1) + .unwrap_or_else(|_| panic!("private-CA Redis TTL write failed")); + assert_eq!( + store + .get(key) + .unwrap_or_else(|_| panic!("private-CA Redis TTL read failed")) + .as_deref(), + Some(&b"expires"[..]) + ); + + let deadline = Instant::now() + Duration::from_secs(3); + loop { + match store + .get(key) + .unwrap_or_else(|_| panic!("private-CA Redis TTL polling read failed")) + { + None => break, + Some(_) if Instant::now() < deadline => thread::sleep(Duration::from_millis(20)), + Some(_) => panic!("private-CA Redis TTL did not expire within the bounded window"), + } + } +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn private_ca_mtls_increment_is_atomic_across_concurrent_connections() { + const WORKERS: usize = 4; + + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = Arc::new(pooled_store_for( + &tls_dsn(server.port()), + pki.client_tls(), + WORKERS, + )); + let barrier = Arc::new(Barrier::new(WORKERS)); + let mut workers = Vec::with_capacity(WORKERS); + + for _ in 0..WORKERS { + let worker_store = Arc::clone(&store); + let worker_barrier = Arc::clone(&barrier); + workers.push(thread::spawn(move || { + worker_barrier.wait(); + worker_store + .incr_with_ttl(b"wor-1946-live-counter", 30) + .unwrap_or_else(|_| panic!("private-CA Redis concurrent increment failed")) + })); + } + + let mut observed = workers + .into_iter() + .map(|worker| { + worker + .join() + .unwrap_or_else(|_| panic!("private-CA Redis increment worker panicked")) + }) + .collect::>(); + observed.sort_unstable(); + assert_eq!(observed, (1..=WORKERS as i64).collect::>()); + assert_eq!( + store + .get(b"wor-1946-live-counter") + .unwrap_or_else(|_| panic!("private-CA Redis counter read failed")) + .map(|value| value.to_vec()), + Some(WORKERS.to_string().into_bytes()) + ); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn private_ca_mtls_unlock_is_scoped_to_the_lock_owner() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = store_for(&tls_dsn(server.port()), pki.client_tls()); + let key = b"wor-1946-live-lock"; + + assert!(store + .try_lock(key, b"owner-a", 30) + .unwrap_or_else(|_| panic!("private-CA Redis lock acquisition failed"))); + assert!(!store + .try_lock(key, b"owner-b", 30) + .unwrap_or_else(|_| panic!("private-CA Redis lock contention failed"))); + + store + .unlock(key, b"owner-b") + .unwrap_or_else(|_| panic!("private-CA Redis non-owner unlock failed")); + assert!(!store + .try_lock(key, b"owner-c", 30) + .unwrap_or_else(|_| panic!("private-CA Redis lock fencing check failed"))); + + store + .unlock(key, b"owner-a") + .unwrap_or_else(|_| panic!("private-CA Redis owner unlock failed")); + assert!(store + .try_lock(key, b"owner-b", 30) + .unwrap_or_else(|_| panic!("private-CA Redis lock handoff failed"))); + + store + .unlock(key, b"owner-a") + .unwrap_or_else(|_| panic!("private-CA Redis stale-owner unlock failed")); + assert!(!store + .try_lock(key, b"owner-c", 30) + .unwrap_or_else(|_| panic!("private-CA Redis stale-owner fencing check failed"))); + + store + .unlock(key, b"owner-b") + .unwrap_or_else(|_| panic!("private-CA Redis final owner unlock failed")); + assert!(store + .try_lock(key, b"owner-c", 30) + .unwrap_or_else(|_| panic!("private-CA Redis final lock handoff failed"))); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn private_ca_mtls_scan_returns_only_the_requested_prefix() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = store_for(&tls_dsn(server.port()), pki.client_tls()); + + for (key, value) in [ + (&b"wor-1946-live-scan:a"[..], &b"value-a"[..]), + (&b"wor-1946-live-scan:b"[..], &b"value-b"[..]), + (&b"wor-1946-live-other"[..], &b"not-returned"[..]), + ] { + store + .put(key, value) + .unwrap_or_else(|_| panic!("private-CA Redis scan fixture write failed")); + } + + let mut actual = store + .scan_prefix(b"wor-1946-live-scan:") + .unwrap_or_else(|_| panic!("private-CA Redis prefix scan failed")) + .into_iter() + .map(|(key, value)| (key.to_vec(), value.to_vec())) + .collect::>(); + actual.sort(); + assert_eq!( + actual, + vec![ + (b"wor-1946-live-scan:a".to_vec(), b"value-a".to_vec()), + (b"wor-1946-live-scan:b".to_vec(), b"value-b".to_vec()), + ] + ); + assert!(store + .scan_prefix(b"wor-1946-live-missing:") + .unwrap_or_else(|_| panic!("private-CA Redis empty prefix scan failed")) + .is_empty()); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn missing_client_identity_is_rejected_by_mtls_server() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = store_for( + &tls_dsn(server.port()), + RedisTlsConfig { + root_cert: Some(pki.ca_pem.clone()), + ..RedisTlsConfig::default() + }, + ); + + assert_safe_failure( + store + .get(b"missing-client-identity") + .expect_err("Redis mTLS must require a client identity"), + "redis get failed: tls", + ); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn wrong_private_ca_is_rejected_without_plaintext_fallback() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let audit_proxy = RedisProtocolAuditProxy::spawn(server.port()); + let store = store_for( + &tls_dsn(audit_proxy.port()), + RedisTlsConfig { + root_cert: Some(pki.wrong_ca_pem.clone()), + client_cert: Some(pki.client_cert_pem.clone()), + client_key: Some(pki.client_key_pem.clone()), + }, + ); + + assert_safe_failure( + store + .get(b"wrong-private-ca") + .expect_err("an untrusted Redis server certificate must fail"), + "redis get failed: tls", + ); + + let counts = audit_proxy.protocol_counts(); + assert!( + counts.tls > 0, + "failed rediss must make an observable TLS attempt" + ); + assert_eq!( + counts.plaintext, 0, + "failed rediss must never retry with RESP plaintext" + ); + assert_eq!( + counts.other, 0, + "failed rediss emitted an unexpected protocol prefix" + ); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn omitted_private_ca_is_rejected() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let store = store_for( + &tls_dsn(server.port()), + RedisTlsConfig { + client_cert: Some(pki.client_cert_pem.clone()), + client_key: Some(pki.client_key_pem.clone()), + ..RedisTlsConfig::default() + }, + ); + + assert_safe_failure( + store + .get(b"omitted-private-ca") + .expect_err("the private Redis CA must not be optional"), + "redis get failed: tls", + ); +} + +#[test] +#[ignore = "requires redis-server executable on PATH with TLS support"] +fn plaintext_never_succeeds_on_tls_only_port() { + let pki = TestPki::generate(); + let server = RedisServer::spawn_tls(pki.server_config()); + let plaintext = store_for( + &format!("redis://127.0.0.1:{}/0", server.port()), + RedisTlsConfig::default(), + ); + + for _ in 0..2 { + assert_safe_failure( + plaintext + .get(b"plaintext-must-fail") + .expect_err("a TLS-only Redis port must reject plaintext"), + "redis get failed: transport", + ); + } + + let tls = store_for(&tls_dsn(server.port()), pki.client_tls()); + tls.put(b"still-tls-only", b"ok") + .unwrap_or_else(|_| panic!("valid mTLS stopped working after plaintext probes")); +} + +#[test] +#[ignore = "requires redis-server executable on PATH"] +fn pooled_connection_recovers_after_redis_kill_and_restart() { + let mut server = RedisServer::spawn_authenticated(PASSWORD); + let dsn = authenticated_dsn(server.port(), ENCODED_PASSWORD, 0); + let store = store_for(&dsn, RedisTlsConfig::default()); + store + .put(b"before-restart", b"present") + .unwrap_or_else(|_| panic!("initial Redis write failed")); + + server.stop(); + server.restart(); + + assert_safe_failure( + store + .get(b"before-restart") + .expect_err("the pooled connection must observe Redis process replacement"), + "redis get failed: transport", + ); + store + .put(b"after-restart", b"reconnected") + .unwrap_or_else(|_| panic!("Redis store did not reconnect after invalidation")); + assert_eq!( + store + .get(b"after-restart") + .unwrap_or_else(|_| panic!("Redis store could not use the replacement connection")) + .as_deref(), + Some(&b"reconnected"[..]) + ); +} diff --git a/crates/sbproxy-platform/tests/support/mod.rs b/crates/sbproxy-platform/tests/support/mod.rs new file mode 100644 index 000000000..a416ac218 --- /dev/null +++ b/crates/sbproxy-platform/tests/support/mod.rs @@ -0,0 +1 @@ +pub mod redis_server; diff --git a/crates/sbproxy-platform/tests/support/redis_server.rs b/crates/sbproxy-platform/tests/support/redis_server.rs new file mode 100644 index 000000000..f37b7874c --- /dev/null +++ b/crates/sbproxy-platform/tests/support/redis_server.rs @@ -0,0 +1,444 @@ +use std::io::{self, Read, Write}; +use std::net::{Shutdown, SocketAddr, TcpListener, TcpStream}; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::Arc; +use std::thread; +use std::time::{Duration, Instant}; + +use tempfile::TempDir; +use url::Url; + +const STARTUP_TIMEOUT: Duration = Duration::from_secs(5); +const PROBE_TIMEOUT: Duration = Duration::from_millis(100); +const PROXY_IO_TIMEOUT: Duration = Duration::from_secs(1); +const PROXY_MAX_CONNECTION_LIFETIME: Duration = Duration::from_secs(2); +const PROXY_ACCEPT_POLL: Duration = Duration::from_millis(5); + +pub struct RedisServer { + child: Option, + _directory: TempDir, + port: u16, + mode: RedisServerMode, +} + +pub struct RedisTlsServerConfig { + pub server_cert_file: PathBuf, + pub server_key_file: PathBuf, + pub ca_cert_file: PathBuf, + pub readiness_root_cert: Vec, + pub readiness_client_cert: Vec, + pub readiness_client_key: Vec, +} + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub struct RedisProtocolCounts { + pub tls: usize, + pub plaintext: usize, + pub other: usize, +} + +pub struct RedisProtocolAuditProxy { + port: u16, + counts: Arc, + shutdown: Arc, + worker: Option>, +} + +#[derive(Default)] +struct ProtocolCounters { + tls: AtomicUsize, + plaintext: AtomicUsize, + other: AtomicUsize, +} + +enum RedisServerMode { + Authenticated { password: String }, + AclFallbackTrap { username: String, password: String }, + Tls(RedisTlsServerConfig), +} + +impl RedisProtocolAuditProxy { + pub fn spawn(upstream_port: u16) -> Self { + let listener = TcpListener::bind("127.0.0.1:0") + .unwrap_or_else(|_| panic!("failed to bind the Redis protocol audit proxy")); + listener + .set_nonblocking(true) + .unwrap_or_else(|_| panic!("failed to configure the Redis protocol audit proxy")); + let port = listener + .local_addr() + .unwrap_or_else(|_| panic!("failed to inspect the Redis protocol audit proxy")) + .port(); + let counts = Arc::new(ProtocolCounters::default()); + let shutdown = Arc::new(AtomicBool::new(false)); + let worker_counts = Arc::clone(&counts); + let worker_shutdown = Arc::clone(&shutdown); + let worker = thread::spawn(move || { + run_protocol_audit_proxy(listener, upstream_port, worker_counts, worker_shutdown); + }); + + Self { + port, + counts, + shutdown, + worker: Some(worker), + } + } + + pub const fn port(&self) -> u16 { + self.port + } + + pub fn protocol_counts(&self) -> RedisProtocolCounts { + RedisProtocolCounts { + tls: self.counts.tls.load(Ordering::Acquire), + plaintext: self.counts.plaintext.load(Ordering::Acquire), + other: self.counts.other.load(Ordering::Acquire), + } + } +} + +impl Drop for RedisProtocolAuditProxy { + fn drop(&mut self) { + self.shutdown.store(true, Ordering::Release); + if let Some(worker) = self.worker.take() { + let _ = worker.join(); + } + } +} + +fn run_protocol_audit_proxy( + listener: TcpListener, + upstream_port: u16, + counts: Arc, + shutdown: Arc, +) { + let mut handlers = Vec::new(); + while !shutdown.load(Ordering::Acquire) { + match listener.accept() { + Ok((client, _)) => { + let connection_counts = Arc::clone(&counts); + handlers.push(thread::spawn(move || { + proxy_redis_connection(client, upstream_port, &connection_counts); + })); + } + Err(error) if error.kind() == io::ErrorKind::WouldBlock => { + thread::sleep(PROXY_ACCEPT_POLL); + } + Err(_) => break, + } + } + + for handler in handlers { + let _ = handler.join(); + } +} + +fn proxy_redis_connection(client: TcpStream, upstream_port: u16, counts: &ProtocolCounters) { + let _ = client.set_read_timeout(Some(PROXY_IO_TIMEOUT)); + let _ = client.set_write_timeout(Some(PROXY_IO_TIMEOUT)); + + let mut prefix = [0_u8; 1]; + match client.peek(&mut prefix) { + Ok(1) if prefix[0] == 0x16 => { + counts.tls.fetch_add(1, Ordering::AcqRel); + } + Ok(1) if prefix[0] == b'*' => { + counts.plaintext.fetch_add(1, Ordering::AcqRel); + } + _ => { + counts.other.fetch_add(1, Ordering::AcqRel); + return; + } + } + + let upstream_addr = SocketAddr::from(([127, 0, 0, 1], upstream_port)); + let Ok(upstream) = TcpStream::connect_timeout(&upstream_addr, PROXY_IO_TIMEOUT) else { + return; + }; + let _ = upstream.set_read_timeout(Some(PROXY_IO_TIMEOUT)); + let _ = upstream.set_write_timeout(Some(PROXY_IO_TIMEOUT)); + + let Ok(mut client_reader) = client.try_clone() else { + return; + }; + let Ok(mut upstream_writer) = upstream.try_clone() else { + return; + }; + let client_to_upstream = thread::spawn(move || { + forward_bounded(&mut client_reader, &mut upstream_writer); + }); + + let mut upstream_reader = upstream; + let mut client_writer = client; + forward_bounded(&mut upstream_reader, &mut client_writer); + let _ = client_to_upstream.join(); +} + +fn forward_bounded(reader: &mut TcpStream, writer: &mut TcpStream) { + let deadline = Instant::now() + PROXY_MAX_CONNECTION_LIFETIME; + let mut buffer = [0_u8; 4096]; + while Instant::now() < deadline { + match reader.read(&mut buffer) { + Ok(0) => break, + Ok(length) => { + if writer.write_all(&buffer[..length]).is_err() { + break; + } + } + Err(error) if error.kind() == io::ErrorKind::Interrupted => continue, + Err(_) => break, + } + } + let _ = writer.shutdown(Shutdown::Write); +} + +impl RedisServer { + pub fn spawn_authenticated(password: &str) -> Self { + let reservation = TcpListener::bind("127.0.0.1:0") + .unwrap_or_else(|_| panic!("failed to reserve a loopback port for redis-server")); + let port = reservation + .local_addr() + .unwrap_or_else(|_| panic!("failed to inspect the reserved redis-server port")) + .port(); + let directory = tempfile::tempdir() + .unwrap_or_else(|_| panic!("failed to create the disposable redis-server directory")); + drop(reservation); + + let mode = RedisServerMode::Authenticated { + password: password.to_string(), + }; + let child = spawn_child(port, directory.path(), &mode); + let mut server = Self { + child: Some(child), + _directory: directory, + port, + mode, + }; + server.wait_until_ready(); + server + } + + pub fn spawn_acl_fallback_trap(username: &str, password: &str) -> Self { + let reservation = TcpListener::bind("127.0.0.1:0") + .unwrap_or_else(|_| panic!("failed to reserve a loopback port for redis-server")); + let port = reservation + .local_addr() + .unwrap_or_else(|_| panic!("failed to inspect the reserved redis-server port")) + .port(); + let directory = tempfile::tempdir() + .unwrap_or_else(|_| panic!("failed to create the disposable redis-server directory")); + drop(reservation); + + let mode = RedisServerMode::AclFallbackTrap { + username: username.to_string(), + password: password.to_string(), + }; + let child = spawn_child(port, directory.path(), &mode); + let mut server = Self { + child: Some(child), + _directory: directory, + port, + mode, + }; + server.wait_until_ready(); + server + } + + pub fn spawn_tls(config: RedisTlsServerConfig) -> Self { + let reservation = TcpListener::bind("127.0.0.1:0") + .unwrap_or_else(|_| panic!("failed to reserve a loopback port for TLS redis-server")); + let port = reservation + .local_addr() + .unwrap_or_else(|_| panic!("failed to inspect the reserved TLS redis-server port")) + .port(); + let directory = tempfile::tempdir().unwrap_or_else(|_| { + panic!("failed to create the disposable TLS redis-server directory") + }); + drop(reservation); + + let mode = RedisServerMode::Tls(config); + let child = spawn_child(port, directory.path(), &mode); + let mut server = Self { + child: Some(child), + _directory: directory, + port, + mode, + }; + server.wait_until_ready(); + server + } + + pub const fn port(&self) -> u16 { + self.port + } + + fn wait_until_ready(&mut self) { + let client = readiness_client(self.port, &self.mode); + let deadline = Instant::now() + STARTUP_TIMEOUT; + loop { + if self + .child + .as_mut() + .expect("redis-server child must exist while starting") + .try_wait() + .unwrap_or_else(|_| panic!("failed to inspect the redis-server child process")) + .is_some() + { + panic!("redis-server exited before secure readiness"); + } + let last_error_kind = match readiness_ping(&client) { + Ok(()) => break, + Err(error) => format!("{:?}", error.kind()), + }; + if Instant::now() >= deadline { + panic!( + "redis-server did not become ready for secure PING ({})", + last_error_kind + ); + } + thread::sleep(Duration::from_millis(20)); + } + + if let RedisServerMode::AclFallbackTrap { username, password } = &self.mode { + configure_acl_user(self.port, username, password); + } + } + + pub fn stop(&mut self) { + if let Some(mut child) = self.child.take() { + let _ = child.kill(); + let _ = child.wait(); + } + } + + pub fn restart(&mut self) { + self.stop(); + self.child = Some(spawn_child(self.port, self._directory.path(), &self.mode)); + self.wait_until_ready(); + } +} + +impl Drop for RedisServer { + fn drop(&mut self) { + self.stop(); + } +} + +fn spawn_child(port: u16, directory: &Path, mode: &RedisServerMode) -> Child { + let mut command = Command::new("redis-server"); + command + .args([ + "--bind", + "127.0.0.1", + "--protected-mode", + "no", + "--save", + "", + "--appendonly", + "no", + "--daemonize", + "no", + ]) + .arg("--dir") + .arg(directory); + match mode { + RedisServerMode::Authenticated { password } => { + command + .arg("--port") + .arg(port.to_string()) + .arg("--requirepass") + .arg(password); + } + RedisServerMode::AclFallbackTrap { .. } => { + command.arg("--port").arg(port.to_string()); + } + RedisServerMode::Tls(config) => { + command + .args(["--port", "0", "--tls-port"]) + .arg(port.to_string()) + .arg("--tls-cert-file") + .arg(&config.server_cert_file) + .arg("--tls-key-file") + .arg(&config.server_key_file) + .arg("--tls-ca-cert-file") + .arg(&config.ca_cert_file) + .args(["--tls-auth-clients", "yes"]); + } + } + command + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap_or_else(|error| { + if error.kind() == std::io::ErrorKind::NotFound { + panic!("redis-server prerequisite is missing from PATH"); + } + panic!( + "failed to spawn the redis-server prerequisite ({:?})", + error.kind() + ); + }) +} + +fn readiness_client(port: u16, mode: &RedisServerMode) -> redis::Client { + match mode { + RedisServerMode::Authenticated { password } => authenticated_client(port, password), + RedisServerMode::AclFallbackTrap { .. } => anonymous_client(port), + RedisServerMode::Tls(config) => redis::Client::build_with_tls( + format!("rediss://127.0.0.1:{port}/0"), + redis::TlsCertificates { + client_tls: Some(redis::ClientTlsConfig { + client_cert: config.readiness_client_cert.clone(), + client_key: config.readiness_client_key.clone(), + }), + root_cert: Some(config.readiness_root_cert.clone()), + }, + ) + .unwrap_or_else(|_| panic!("failed to build the TLS Redis readiness client")), + } +} + +fn anonymous_client(port: u16) -> redis::Client { + redis::Client::open(format!("redis://127.0.0.1:{port}/0")) + .unwrap_or_else(|_| panic!("failed to build the anonymous Redis probe client")) +} + +fn configure_acl_user(port: u16, username: &str, password: &str) { + let client = anonymous_client(port); + let mut connection = client + .get_connection_with_timeout(PROBE_TIMEOUT) + .unwrap_or_else(|_| panic!("failed to connect while configuring the Redis ACL trap")); + redis::cmd("ACL") + .arg("SETUSER") + .arg(username) + .arg("reset") + .arg("on") + .arg(format!(">{password}")) + .arg("~*") + .arg("+@all") + .query::<()>(&mut connection) + .unwrap_or_else(|_| panic!("failed to configure the Redis ACL trap")); +} + +fn authenticated_client(port: u16, password: &str) -> redis::Client { + let mut url = Url::parse(&format!("redis://127.0.0.1:{port}/0")) + .expect("the loopback Redis probe URL must be valid"); + url.set_username("default") + .expect("the Redis probe username must be valid"); + url.set_password(Some(password)) + .expect("the Redis probe password must be valid"); + redis::Client::open(url.as_str()) + .unwrap_or_else(|_| panic!("failed to build the authenticated Redis readiness client")) +} + +fn readiness_ping(client: &redis::Client) -> redis::RedisResult<()> { + let mut connection = client.get_connection_with_timeout(PROBE_TIMEOUT)?; + connection.set_read_timeout(Some(PROBE_TIMEOUT))?; + connection.set_write_timeout(Some(PROBE_TIMEOUT))?; + redis::cmd("PING") + .query::(&mut connection) + .map(|_| ()) +} diff --git a/crates/sbproxy-tls/src/lib.rs b/crates/sbproxy-tls/src/lib.rs index 81b2a590d..a84e82ba1 100644 --- a/crates/sbproxy-tls/src/lib.rs +++ b/crates/sbproxy-tls/src/lib.rs @@ -120,15 +120,19 @@ fn open_cert_backend(acme: Option<&sbproxy_config::AcmeConfig>) -> Arc { - // storage_path holds the redis address (host:port) for the - // shared cert store; connections open lazily. The distributed - // issuance lock (SET NX PX) makes a fleet issue a cert once - // instead of stampeding the CA (WOR-1774). - let cfg = sbproxy_platform::storage::RedisConfig { - addr: acme.storage_path.clone(), - ..Default::default() + // Connections open lazily. The distributed issuance lock + // (SET NX PX) makes a fleet issue a cert once instead of + // stampeding the CA (WOR-1774). + let cfg = match sbproxy_platform::storage::RedisConfig::from_dsn(&acme.storage_path) { + Ok(cfg) => cfg, + Err(_) => { + warn!( + "cert store: invalid Redis connection configuration; certs will NOT persist (in-memory fallback)" + ); + return Arc::new(MemoryKVStore::new(0)); + } }; - info!(addr = %acme.storage_path, "cert store backend: redis (shared, cluster-safe)"); + info!("cert store backend: redis (shared, cluster-safe)"); Arc::new(sbproxy_platform::storage::RedisKVStore::new(cfg)) } "file" => { @@ -649,6 +653,37 @@ mod tests { use super::*; use cert_store::CertMeta; + fn acme_with_storage(backend: &str, path: &str) -> sbproxy_config::AcmeConfig { + sbproxy_config::AcmeConfig { + enabled: true, + email: "operator@example.com".to_string(), + directory_url: "https://acme.invalid/directory".to_string(), + challenge_types: vec!["http-01".to_string()], + storage_backend: backend.to_string(), + storage_path: path.to_string(), + renew_before_days: 30, + } + } + + #[test] + fn redis_cert_backend_rejects_invalid_full_dsn_without_network_io() { + let sentinel = "rediss://default:sentinel-acme-password@sentinel-acme-host.invalid:6380/-1"; + let acme = acme_with_storage("redis", sentinel); + + let backend = open_cert_backend(Some(&acme)); + + backend + .put(b"certificate-key", b"certificate-value") + .expect("invalid Redis config must retain the in-memory fallback posture"); + assert_eq!( + backend + .get(b"certificate-key") + .expect("read fallback certificate state") + .as_deref(), + Some(b"certificate-value".as_slice()) + ); + } + // Regression: the OCSP refresh task and the maintenance handle are started // from the synchronous proxy-setup path, before Pingora installs a runtime. // A bare `tokio::spawn` there panics with "there is no reactor running", diff --git a/crates/sbproxy/tests/models_lifecycle_cli.rs b/crates/sbproxy/tests/models_lifecycle_cli.rs index 3018db92b..2dc8b29d4 100644 --- a/crates/sbproxy/tests/models_lifecycle_cli.rs +++ b/crates/sbproxy/tests/models_lifecycle_cli.rs @@ -6,7 +6,9 @@ use std::net::TcpListener; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; use std::thread; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +const ADMIN_FIXTURE_TIMEOUT: Duration = Duration::from_secs(30); fn binary() -> &'static str { env!("CARGO_BIN_EXE_sbproxy") @@ -74,22 +76,23 @@ fn fixture_admin( listener.set_nonblocking(true).unwrap(); let address = listener.local_addr().unwrap(); let handle = thread::spawn(move || { - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + let deadline = Instant::now() + ADMIN_FIXTURE_TIMEOUT; let (mut stream, _) = loop { match listener.accept() { Ok(accepted) => break accepted, Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { assert!( - std::time::Instant::now() < deadline, + Instant::now() < deadline, "timed out waiting for admin request" ); - thread::sleep(std::time::Duration::from_millis(10)); + thread::sleep(Duration::from_millis(10)); } Err(error) => panic!("accept admin request: {error}"), } }; + stream.set_nonblocking(false).unwrap(); stream - .set_read_timeout(Some(std::time::Duration::from_secs(5))) + .set_read_timeout(Some(ADMIN_FIXTURE_TIMEOUT)) .unwrap(); let mut request = Vec::new(); let mut buffer = [0u8; 4096]; diff --git a/docs/ai-context-compression.md b/docs/ai-context-compression.md index 6d1b26cfa..440893983 100644 --- a/docs/ai-context-compression.md +++ b/docs/ai-context-compression.md @@ -196,8 +196,12 @@ reported output count and a conservative local estimate must both fit ## Redis state `backend: redis` reuses the process-wide Redis L2 configuration and Redis -service. The compression runtime opens its own lazy multiplexed connection from -that configured DSN; the compression block does not accept a separate DSN. +service. It inherits all four connection fields: `dsn`, `ca_file`, `cert_file`, +and `key_file`. The compression runtime clones the same validated Redis client +and opens its own lazy multiplexed connection. The compression block does not +accept a separate DSN, CA, or client identity, so it cannot silently lose the +L2 trust or mTLS configuration. + Redis serializes updates with a bounded lease, a monotonic fence, and a logical-version compare-and-set. The lease is the configured summarizer timeout plus a fixed 5-second margin for the bounded state load, validation, and commit; @@ -208,7 +212,10 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem origins: "ai.example.com": @@ -239,13 +246,30 @@ origins: ``` Selecting Redis without `proxy.l2_cache_settings.driver: redis` is a startup -configuration error. An invalid Redis DSN is also rejected. Once the runtime is -active, a Redis command or connection failure makes the stateful lever fail -open for that request. The current internal bounds are 500 milliseconds for -connection setup, 1 second for a command response, and 2 seconds for a complete -state operation. A failed cached connection is replaced so a restarted Redis -service can recover without restarting SBproxy. There is no worker-local -summary fallback. +configuration error. Invalid DSN semantics, invalid TLS field combinations, +and bad local PEM material are also rejected before serving. Each configuration +compile reads and validates the Redis PEM files once. The general L2 store and +compression state adapter then clone the same immutable validated connection +snapshot; constructing compression or admin adapters later does not reopen +those files. A configuration reload compiles a new snapshot and therefore +reads the files for that reload. Configuration validation does not open a +network connection. TLS verification, authentication, and database selection +happen when the lazy compression connection is first used. + +Once the runtime is active, a Redis connection, TLS, authentication, database, +or command failure makes the stateful lever fail open for that request. The +current internal bounds are 500 milliseconds for connection setup, 1 second +for a command response, and 2 seconds for a complete state operation. A failed +cached connection is replaced, and a later request can recover without +restarting SBproxy. There is no worker-local summary fallback. + +The general synchronous L2 metrics named `sbproxy_redis_kv_*` cover +`RedisKVStore` consumers such as shared response cache and rate limiting. The +compression runtime remains covered by +`sbproxy_ai_compression_state_operations_total`, +`sbproxy_ai_compression_state_operation_duration_seconds`, and +`sbproxy_ai_compression_redis_coordination_total`; it does not double-count its +async operations in the synchronous families. ## Why mesh is not a supported state backend diff --git a/docs/configuration.md b/docs/configuration.md index 5adf9f8b5..31be0d40c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -571,23 +571,52 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem ``` `params` keys for the `redis` driver: | Key | Type | Default | Description | |-----|------|---------|-------------| -| `dsn` | string | | Connection address. The general L2 client currently supports only an unauthenticated, plaintext Redis endpoint supplied as `redis.internal:6379` or `redis://redis.internal:6379`. | - -Pool size and acquire timeout are not exposed via `params` and use built-in defaults (pool size 8, acquire timeout 5 seconds). - -The general L2 client does not yet implement TLS, `AUTH`, or `SELECT`. Its -legacy parser accepts credentials, a database path, and a `rediss://` scheme -but discards them before opening a plaintext connection, so do not rely on -those forms for L2 traffic. AI context compression uses a separate async -client built from the complete DSN and does support credentials, database -selection, `redis://`, and `rediss://`. +| `dsn` | string | required | Redis connection. Accepts a legacy hostname or `host:port`, a `redis://` URL, or a verified `rediss://` URL. URL paths select a non-negative logical database. | +| `ca_file` | string | unset | PEM trust anchor for a private Redis CA. Valid only with `rediss://`. When omitted, verified TLS uses system trust roots. | +| `cert_file` | string | unset | PEM client certificate chain for Redis mTLS. Must appear with `key_file` and requires `rediss://`. | +| `key_file` | string | unset | PEM private key matching `cert_file`. Must appear with `cert_file` and requires `rediss://`. | + +Legacy `redis.internal` and `redis.internal:6379` values remain compatible and +normalize to plaintext `redis://` connections. Bracketed IPv6 addresses are +accepted. Unbracketed ambiguous IPv6 addresses are rejected. + +Use `redis://` only for an intentionally plaintext connection. `rediss://` +performs certificate verification and never retries as plaintext. Redis ACL +username and password authentication, password-only authentication, and +database paths are preserved during connection setup. Percent-encode reserved +characters in credentials, such as `%40` for `@` and `%2F` for `/`; environment +interpolation does not URL-encode a value for you. + +Configuration loading validates the URL, supported scheme, database syntax, +TLS field combinations, PEM material, and client certificate/key match. It +does not contact Redis. The first L2 operation opens the connection and performs +TLS, `AUTH`, and `SELECT`, so an unreachable service or a server-side trust, +authentication, or database rejection appears at runtime. Query parameters, +URL fragments such as `#insecure`, negative databases, and a username without +a password are rejected instead of being weakened. + +Pool size, pool acquisition timeout, connection timeout, and command timeout +are not exposed through `params`. The built-in pool size is 8 and each timeout +defaults to 5 seconds. + +AI context compression with `summary_buffer` reuses this same validated DSN, +private CA, client certificate, and client key. The compression block does not +accept a separate Redis connection. + +Do not roll a secure deployment back to a release that predates these fields. +Older releases are safe only for unauthenticated plaintext database-zero +deployments because they did not preserve TLS, authentication, or database +selection. ### messenger_settings @@ -611,7 +640,7 @@ Supported drivers and their `params` keys: | Key | Type | Default | Description | |-----|------|---------|-------------| -| `dsn` | string | `redis://127.0.0.1:6379` | Redis connection string. Same parsing rules as the L2 cache `dsn`. | +| `dsn` | string | `redis://127.0.0.1:6379` | Redis messenger connection string. The secure L2 fields and connection behavior described above do not apply to `messenger_settings`. | `sqs` (all required): @@ -3965,7 +3994,11 @@ proxy: ## Redis integration -Redis has two roles in SBproxy: distributed caching (L2 cache) and real-time messaging (config sync, cache invalidation). Both blocks are nested under `proxy:`. +Redis has two roles in SBproxy: distributed caching and shared state through the +general L2 store, plus real-time messaging for config sync and cache +invalidation. Both blocks are nested under `proxy`, but they use separate +connection implementations. The verified TLS, authentication, database, and +client-certificate contract in this section applies to `l2_cache_settings`. ### L2 cache (distributed rate limiting and caching) @@ -3974,10 +4007,20 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem ``` -When configured, rate limit counters are shared across all proxy instances. Response cache entries can also be stored in Redis for shared caching. The deserializer also accepts `l2_cache:` as a canonical alias. +When configured, rate limit counters are shared across all proxy instances. +Response cache entries can also be stored in Redis for shared caching. The +deserializer accepts `l2_cache:` as an alias. See +[`l2_cache_settings`](#l2_cache_settings) for legacy address compatibility, +verified TLS, credential encoding, database selection, startup validation, and +lazy connection behavior. The runnable +[`redis-l2-secure`](../examples/redis-l2-secure/) example exercises private-CA +verification, client mTLS, password authentication, and database 7. ### Messenger (real-time config updates) @@ -3993,7 +4036,7 @@ When configured, config changes pushed via the API propagate to all proxy instan The Redis driver expects `params.dsn`. SQS uses `queue_url`, `region`, `api_key`. GCP Pub/Sub uses `project`, `topic`, `subscription`, `access_token`. The `memory` driver takes no params and is single-replica only. -### Full Redis setup +### L2 plus messenger setup ```yaml proxy: @@ -4002,7 +4045,10 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem messenger_settings: driver: redis params: @@ -4021,6 +4067,10 @@ origins: ttl_secs: 300 ``` +The messenger DSN above is intentionally shown separately. Do not add the L2 +TLS file fields under `messenger_settings` or assume that the messenger inherits +the L2 connection. + --- ## Validation diff --git a/docs/degradation.md b/docs/degradation.md index adec71cc3..14b70ba32 100644 --- a/docs/degradation.md +++ b/docs/degradation.md @@ -17,7 +17,7 @@ What happens when each dependency that SBproxy talks to is unavailable, and how |---|---|---|---|---| | Upstream target (`proxy` or `load_balancer`) | Connection error / timeout | Active health checks + outlier detection + circuit breaker eject the target. Retries pick the next healthy peer. With every target ejected, the LB falls back to the unfiltered list rather than 502'ing the client. | Auto on next probe success / breaker recovery window | `sbproxy_requests_total{status}`, `sbproxy_origin_requests_total{origin,method,status}` | | AI provider (OpenAI, Anthropic, OpenRouter, ...) | 5xx, timeout, rate-limit | Routing strategy picks the next provider in the chain (`fallback_chain` / `cost_optimized`). All-providers-failed returns 502. | Auto on next successful request | `sbproxy_ai_failovers_total`, `sbproxy_ai_provider_errors_total` | -| Redis (`proxy.l2_cache_settings`) | Connection / command failure | General response caching and rate limiting fall back to per-process behavior. AI `summary_buffer` state never falls back to worker memory: that lever fails open, preserves the last committed message list, and lets later levers run. | Auto-reconnect; summary updates resume on a later request | `sbproxy_ai_compression_state_operations_total`, `sbproxy_ai_compression_redis_coordination_total` for compression state | +| Redis (`proxy.l2_cache_settings`) | Connection, TLS, authentication, database selection, protocol, or command failure | A response-cache lookup failure bypasses the cache and does not arm write-back for that request. A shared rate-limit operation failure admits the request fail-open instead of switching to a local bucket. AI `summary_buffer` state never falls back to worker memory: that lever fails open, preserves the last committed message list, and lets later levers run. Other L2 consumers keep their feature-specific failure posture. | A later operation opens a fresh connection automatically; summary updates resume on a later request | `sbproxy_redis_kv_connections_total`, `sbproxy_redis_kv_operation_duration_seconds`, `sbproxy_redis_kv_operation_errors_total`, plus the compression state metrics | | Dedicated AI compression summarizer | Timeout, provider failure, invalid output, policy denial, or budget denial | `summary_buffer` skips safe admission denials or fails open on runtime errors. The primary AI request continues with the last committed messages, and a later `window_fit` lever still runs. | Next eligible request retries under the configured policy and timeout | `sbproxy_ai_compression_lever_total`, `sbproxy_ai_compression_requests_total`, `sbproxy_ai_compression_duration_seconds` | | Governed-key budget backend (`key_management.governance.backend`, strict tier only) | Connection / command failure | Only affects keys governed under `consistency: strict`. The default `approximate` tier does not depend on this backend at all; its per-node counters keep disseminating over the cluster mesh. For a strict key, a reserve call that cannot reach the backend denies the request (`503`) by default (`failure_mode: closed`); `failure_mode: allow_unreserved` admits it instead without a reservation. A settle call on an already-admitted request is unaffected by `failure_mode` and stays best-effort. | Auto-reconnect; enforcement resumes on the next successful call | `sbproxy_governance_fail_open_total{key_id}` on `allow_unreserved`; also logged at WARN (fail-open/fail-closed) or DEBUG (other reserve/settle errors) | | ACME CA (Let's Encrypt) | Renewal request fails | Existing cert keeps serving until expiry. With no usable cert, an HTTP-01 self-signed bootstrap is served and an `ERROR` is logged loudly. | Retry with exponential backoff (1m to 24h) | `sbproxy_acme_renewals_total{result}` | @@ -111,20 +111,49 @@ action: --- -### Redis (l2 cache + cross-replica state) +### Redis L2 cache and cross-replica state -**When down:** Redis connect or command fails. +**When down:** a lazy Redis connection can fail during TCP setup, verified TLS, +authentication, or database selection. An established connection can fail on a +pool deadline, command deadline, transport error, server error, or protocol +error. Invalid DSN syntax, unsupported query parameters or fragments, and bad +local PEM material are configuration errors caught before the runtime starts; +they do not enter degradation mode. -**Fallback:** for the general L2 consumers, the proxy keeps using the per-origin in-memory cache. Rate-limit counters become node-local; with multiple replicas, slightly more traffic may sneak through the global limit until Redis recovers. Response cache entries written during the outage are local and not shared. Reconnects use exponential backoff with a circuit breaker so a sustained outage does not pile up retry attempts. +**Fallback:** degradation depends on the L2 consumer. A response-cache lookup +failure bypasses the cache and fetches the response from the upstream. Unlike a +true cache miss, the failed lookup does not retain the cache key for the +response phase, so that request's upstream response is not written to Redis or +to a local outage cache. When a shared rate-limit increment fails, SBproxy +admits the request fail-open; it does not consult a process-local token bucket. +A local token bucket is used only when no shared store is configured. Other L2 +consumers retain their own feature-specific failure posture. + +A broken pooled connection is discarded. A later operation can open a fresh +connection, so recovery does not require an SBproxy restart. AI context summary state is intentionally different. When an AI handler selects `compression.state.backend: redis`, Redis is the only canonical summary store. -On a connection or command failure, `summary_buffer` records +On a connection, TLS, authentication, database, or command failure, +`summary_buffer` records `state_unavailable`, preserves the last committed message list, and continues to later levers and upstream dispatch. It never creates a worker-local summary -fork. - -**Log level:** `ERROR` on initial disconnect, `WARN` per reconnect attempt, `INFO` on recovery. +fork. The compression runtime uses its existing bounded async reconnect policy +and inherits the same validated L2 DSN and TLS material. + +**Log level:** the platform events named `redis store health failed`, +`redis store health remains failed`, and `redis store health recovered` are +transition-based. The first is `WARN`, repeated platform health failures are +`DEBUG`, and the recovery event is `INFO`. This sequence applies only to those +platform events. Response-cache lookup, write, and invalidation call sites and +the shared rate-limit increment call site can emit their own `WARN` for each +failed operation, so an outage can produce more than one warning. + +The platform transition events contain only the closed `operation` and +`reason` values. They do not contain a DSN, endpoint, username, database, key, +value, or certificate path. When troubleshooting consumer warnings, correlate +their fixed message text with the closed-label metrics. Do not print the DSN, +credentials, cache keys, or cache values into tickets or shell history. **Alert:** yes when running clustered. Redis unavailability degrades multi-replica consistency. @@ -134,9 +163,25 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem ``` +The synchronous L2 store exposes three bounded metric families: + +| Metric | Labels | +|---|---| +| `sbproxy_redis_kv_connections_total` | `result`: `success` or `error` | +| `sbproxy_redis_kv_operation_duration_seconds` | `operation`: `get`, `set`, `set_ttl`, `delete`, `increment`, `lock`, `unlock`, or `scan` | +| `sbproxy_redis_kv_operation_errors_total` | `operation` above and `reason`: `pool_timeout`, `connect_timeout`, `command_timeout`, `tls`, `auth`, `transport`, `server`, or `protocol` | + +Every general L2 call records one duration observation. A failed call adds one +error count, and each new connection attempt adds one connection result. None +of these labels includes an endpoint, tenant, application key, username, +database, or free-form error text. + For strict Redis leases, fences, coordination events, and the full fail-open table, see [AI context compression](ai-context-compression.md). diff --git a/docs/llms-full.txt b/docs/llms-full.txt index d956b0d4d..cebb6de89 100644 --- a/docs/llms-full.txt +++ b/docs/llms-full.txt @@ -7,7 +7,7 @@ Pairs with `/llms.txt` (the small AI-discoverable feature catalog at `docs/llms. Regenerated by `scripts/regen-llms-full.sh`. Generated; do not hand-edit. Source: https://github.com/soapbucket/sbproxy -Generated: 2026-07-19T05:36:36Z +Generated: 2026-07-19T08:19:40Z --- @@ -4541,8 +4541,12 @@ reported output count and a conservative local estimate must both fit ## Redis state `backend: redis` reuses the process-wide Redis L2 configuration and Redis -service. The compression runtime opens its own lazy multiplexed connection from -that configured DSN; the compression block does not accept a separate DSN. +service. It inherits all four connection fields: `dsn`, `ca_file`, `cert_file`, +and `key_file`. The compression runtime clones the same validated Redis client +and opens its own lazy multiplexed connection. The compression block does not +accept a separate DSN, CA, or client identity, so it cannot silently lose the +L2 trust or mTLS configuration. + Redis serializes updates with a bounded lease, a monotonic fence, and a logical-version compare-and-set. The lease is the configured summarizer timeout plus a fixed 5-second margin for the bounded state load, validation, and commit; @@ -4553,7 +4557,10 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem origins: "ai.example.com": @@ -4584,13 +4591,30 @@ origins: ``` Selecting Redis without `proxy.l2_cache_settings.driver: redis` is a startup -configuration error. An invalid Redis DSN is also rejected. Once the runtime is -active, a Redis command or connection failure makes the stateful lever fail -open for that request. The current internal bounds are 500 milliseconds for -connection setup, 1 second for a command response, and 2 seconds for a complete -state operation. A failed cached connection is replaced so a restarted Redis -service can recover without restarting SBproxy. There is no worker-local -summary fallback. +configuration error. Invalid DSN semantics, invalid TLS field combinations, +and bad local PEM material are also rejected before serving. Each configuration +compile reads and validates the Redis PEM files once. The general L2 store and +compression state adapter then clone the same immutable validated connection +snapshot; constructing compression or admin adapters later does not reopen +those files. A configuration reload compiles a new snapshot and therefore +reads the files for that reload. Configuration validation does not open a +network connection. TLS verification, authentication, and database selection +happen when the lazy compression connection is first used. + +Once the runtime is active, a Redis connection, TLS, authentication, database, +or command failure makes the stateful lever fail open for that request. The +current internal bounds are 500 milliseconds for connection setup, 1 second +for a command response, and 2 seconds for a complete state operation. A failed +cached connection is replaced, and a later request can recover without +restarting SBproxy. There is no worker-local summary fallback. + +The general synchronous L2 metrics named `sbproxy_redis_kv_*` cover +`RedisKVStore` consumers such as shared response cache and rate limiting. The +compression runtime remains covered by +`sbproxy_ai_compression_state_operations_total`, +`sbproxy_ai_compression_state_operation_duration_seconds`, and +`sbproxy_ai_compression_redis_coordination_total`; it does not double-count its +async operations in the synchronous families. ## Why mesh is not a supported state backend @@ -10393,23 +10417,52 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem ``` `params` keys for the `redis` driver: | Key | Type | Default | Description | |-----|------|---------|-------------| -| `dsn` | string | | Connection address. The general L2 client currently supports only an unauthenticated, plaintext Redis endpoint supplied as `redis.internal:6379` or `redis://redis.internal:6379`. | - -Pool size and acquire timeout are not exposed via `params` and use built-in defaults (pool size 8, acquire timeout 5 seconds). - -The general L2 client does not yet implement TLS, `AUTH`, or `SELECT`. Its -legacy parser accepts credentials, a database path, and a `rediss://` scheme -but discards them before opening a plaintext connection, so do not rely on -those forms for L2 traffic. AI context compression uses a separate async -client built from the complete DSN and does support credentials, database -selection, `redis://`, and `rediss://`. +| `dsn` | string | required | Redis connection. Accepts a legacy hostname or `host:port`, a `redis://` URL, or a verified `rediss://` URL. URL paths select a non-negative logical database. | +| `ca_file` | string | unset | PEM trust anchor for a private Redis CA. Valid only with `rediss://`. When omitted, verified TLS uses system trust roots. | +| `cert_file` | string | unset | PEM client certificate chain for Redis mTLS. Must appear with `key_file` and requires `rediss://`. | +| `key_file` | string | unset | PEM private key matching `cert_file`. Must appear with `cert_file` and requires `rediss://`. | + +Legacy `redis.internal` and `redis.internal:6379` values remain compatible and +normalize to plaintext `redis://` connections. Bracketed IPv6 addresses are +accepted. Unbracketed ambiguous IPv6 addresses are rejected. + +Use `redis://` only for an intentionally plaintext connection. `rediss://` +performs certificate verification and never retries as plaintext. Redis ACL +username and password authentication, password-only authentication, and +database paths are preserved during connection setup. Percent-encode reserved +characters in credentials, such as `%40` for `@` and `%2F` for `/`; environment +interpolation does not URL-encode a value for you. + +Configuration loading validates the URL, supported scheme, database syntax, +TLS field combinations, PEM material, and client certificate/key match. It +does not contact Redis. The first L2 operation opens the connection and performs +TLS, `AUTH`, and `SELECT`, so an unreachable service or a server-side trust, +authentication, or database rejection appears at runtime. Query parameters, +URL fragments such as `#insecure`, negative databases, and a username without +a password are rejected instead of being weakened. + +Pool size, pool acquisition timeout, connection timeout, and command timeout +are not exposed through `params`. The built-in pool size is 8 and each timeout +defaults to 5 seconds. + +AI context compression with `summary_buffer` reuses this same validated DSN, +private CA, client certificate, and client key. The compression block does not +accept a separate Redis connection. + +Do not roll a secure deployment back to a release that predates these fields. +Older releases are safe only for unauthenticated plaintext database-zero +deployments because they did not preserve TLS, authentication, or database +selection. ### messenger_settings @@ -10433,7 +10486,7 @@ Supported drivers and their `params` keys: | Key | Type | Default | Description | |-----|------|---------|-------------| -| `dsn` | string | `redis://127.0.0.1:6379` | Redis connection string. Same parsing rules as the L2 cache `dsn`. | +| `dsn` | string | `redis://127.0.0.1:6379` | Redis messenger connection string. The secure L2 fields and connection behavior described above do not apply to `messenger_settings`. | `sqs` (all required): @@ -13787,7 +13840,11 @@ proxy: ## Redis integration -Redis has two roles in SBproxy: distributed caching (L2 cache) and real-time messaging (config sync, cache invalidation). Both blocks are nested under `proxy:`. +Redis has two roles in SBproxy: distributed caching and shared state through the +general L2 store, plus real-time messaging for config sync and cache +invalidation. Both blocks are nested under `proxy`, but they use separate +connection implementations. The verified TLS, authentication, database, and +client-certificate contract in this section applies to `l2_cache_settings`. ### L2 cache (distributed rate limiting and caching) @@ -13796,10 +13853,20 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem ``` -When configured, rate limit counters are shared across all proxy instances. Response cache entries can also be stored in Redis for shared caching. The deserializer also accepts `l2_cache:` as a canonical alias. +When configured, rate limit counters are shared across all proxy instances. +Response cache entries can also be stored in Redis for shared caching. The +deserializer accepts `l2_cache:` as an alias. See +[`l2_cache_settings`](#l2_cache_settings) for legacy address compatibility, +verified TLS, credential encoding, database selection, startup validation, and +lazy connection behavior. The runnable +[`redis-l2-secure`](../examples/redis-l2-secure/) example exercises private-CA +verification, client mTLS, password authentication, and database 7. ### Messenger (real-time config updates) @@ -13815,7 +13882,7 @@ When configured, config changes pushed via the API propagate to all proxy instan The Redis driver expects `params.dsn`. SQS uses `queue_url`, `region`, `api_key`. GCP Pub/Sub uses `project`, `topic`, `subscription`, `access_token`. The `memory` driver takes no params and is single-replica only. -### Full Redis setup +### L2 plus messenger setup ```yaml proxy: @@ -13824,7 +13891,10 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem messenger_settings: driver: redis params: @@ -13843,6 +13913,10 @@ origins: ttl_secs: 300 ``` +The messenger DSN above is intentionally shown separately. Do not add the L2 +TLS file fields under `messenger_settings` or assume that the messenger inherits +the L2 connection. + --- ## Validation @@ -14709,7 +14783,7 @@ What happens when each dependency that SBproxy talks to is unavailable, and how |---|---|---|---|---| | Upstream target (`proxy` or `load_balancer`) | Connection error / timeout | Active health checks + outlier detection + circuit breaker eject the target. Retries pick the next healthy peer. With every target ejected, the LB falls back to the unfiltered list rather than 502'ing the client. | Auto on next probe success / breaker recovery window | `sbproxy_requests_total{status}`, `sbproxy_origin_requests_total{origin,method,status}` | | AI provider (OpenAI, Anthropic, OpenRouter, ...) | 5xx, timeout, rate-limit | Routing strategy picks the next provider in the chain (`fallback_chain` / `cost_optimized`). All-providers-failed returns 502. | Auto on next successful request | `sbproxy_ai_failovers_total`, `sbproxy_ai_provider_errors_total` | -| Redis (`proxy.l2_cache_settings`) | Connection / command failure | General response caching and rate limiting fall back to per-process behavior. AI `summary_buffer` state never falls back to worker memory: that lever fails open, preserves the last committed message list, and lets later levers run. | Auto-reconnect; summary updates resume on a later request | `sbproxy_ai_compression_state_operations_total`, `sbproxy_ai_compression_redis_coordination_total` for compression state | +| Redis (`proxy.l2_cache_settings`) | Connection, TLS, authentication, database selection, protocol, or command failure | A response-cache lookup failure bypasses the cache and does not arm write-back for that request. A shared rate-limit operation failure admits the request fail-open instead of switching to a local bucket. AI `summary_buffer` state never falls back to worker memory: that lever fails open, preserves the last committed message list, and lets later levers run. Other L2 consumers keep their feature-specific failure posture. | A later operation opens a fresh connection automatically; summary updates resume on a later request | `sbproxy_redis_kv_connections_total`, `sbproxy_redis_kv_operation_duration_seconds`, `sbproxy_redis_kv_operation_errors_total`, plus the compression state metrics | | Dedicated AI compression summarizer | Timeout, provider failure, invalid output, policy denial, or budget denial | `summary_buffer` skips safe admission denials or fails open on runtime errors. The primary AI request continues with the last committed messages, and a later `window_fit` lever still runs. | Next eligible request retries under the configured policy and timeout | `sbproxy_ai_compression_lever_total`, `sbproxy_ai_compression_requests_total`, `sbproxy_ai_compression_duration_seconds` | | Governed-key budget backend (`key_management.governance.backend`, strict tier only) | Connection / command failure | Only affects keys governed under `consistency: strict`. The default `approximate` tier does not depend on this backend at all; its per-node counters keep disseminating over the cluster mesh. For a strict key, a reserve call that cannot reach the backend denies the request (`503`) by default (`failure_mode: closed`); `failure_mode: allow_unreserved` admits it instead without a reservation. A settle call on an already-admitted request is unaffected by `failure_mode` and stays best-effort. | Auto-reconnect; enforcement resumes on the next successful call | `sbproxy_governance_fail_open_total{key_id}` on `allow_unreserved`; also logged at WARN (fail-open/fail-closed) or DEBUG (other reserve/settle errors) | | ACME CA (Let's Encrypt) | Renewal request fails | Existing cert keeps serving until expiry. With no usable cert, an HTTP-01 self-signed bootstrap is served and an `ERROR` is logged loudly. | Retry with exponential backoff (1m to 24h) | `sbproxy_acme_renewals_total{result}` | @@ -14803,20 +14877,49 @@ action: --- -### Redis (l2 cache + cross-replica state) +### Redis L2 cache and cross-replica state + +**When down:** a lazy Redis connection can fail during TCP setup, verified TLS, +authentication, or database selection. An established connection can fail on a +pool deadline, command deadline, transport error, server error, or protocol +error. Invalid DSN syntax, unsupported query parameters or fragments, and bad +local PEM material are configuration errors caught before the runtime starts; +they do not enter degradation mode. -**When down:** Redis connect or command fails. +**Fallback:** degradation depends on the L2 consumer. A response-cache lookup +failure bypasses the cache and fetches the response from the upstream. Unlike a +true cache miss, the failed lookup does not retain the cache key for the +response phase, so that request's upstream response is not written to Redis or +to a local outage cache. When a shared rate-limit increment fails, SBproxy +admits the request fail-open; it does not consult a process-local token bucket. +A local token bucket is used only when no shared store is configured. Other L2 +consumers retain their own feature-specific failure posture. -**Fallback:** for the general L2 consumers, the proxy keeps using the per-origin in-memory cache. Rate-limit counters become node-local; with multiple replicas, slightly more traffic may sneak through the global limit until Redis recovers. Response cache entries written during the outage are local and not shared. Reconnects use exponential backoff with a circuit breaker so a sustained outage does not pile up retry attempts. +A broken pooled connection is discarded. A later operation can open a fresh +connection, so recovery does not require an SBproxy restart. AI context summary state is intentionally different. When an AI handler selects `compression.state.backend: redis`, Redis is the only canonical summary store. -On a connection or command failure, `summary_buffer` records +On a connection, TLS, authentication, database, or command failure, +`summary_buffer` records `state_unavailable`, preserves the last committed message list, and continues to later levers and upstream dispatch. It never creates a worker-local summary -fork. - -**Log level:** `ERROR` on initial disconnect, `WARN` per reconnect attempt, `INFO` on recovery. +fork. The compression runtime uses its existing bounded async reconnect policy +and inherits the same validated L2 DSN and TLS material. + +**Log level:** the platform events named `redis store health failed`, +`redis store health remains failed`, and `redis store health recovered` are +transition-based. The first is `WARN`, repeated platform health failures are +`DEBUG`, and the recovery event is `INFO`. This sequence applies only to those +platform events. Response-cache lookup, write, and invalidation call sites and +the shared rate-limit increment call site can emit their own `WARN` for each +failed operation, so an outage can produce more than one warning. + +The platform transition events contain only the closed `operation` and +`reason` values. They do not contain a DSN, endpoint, username, database, key, +value, or certificate path. When troubleshooting consumer warnings, correlate +their fixed message text with the closed-label metrics. Do not print the DSN, +credentials, cache keys, or cache values into tickets or shell history. **Alert:** yes when running clustered. Redis unavailability degrades multi-replica consistency. @@ -14826,9 +14929,25 @@ proxy: l2_cache_settings: driver: redis params: - dsn: redis://redis.internal:6379/0 + dsn: rediss://cache-user:${REDIS_PASSWORD_URLENCODED}@redis.internal:6380/7 + ca_file: /etc/sbproxy/redis/ca.pem + cert_file: /etc/sbproxy/redis/client.pem + key_file: /etc/sbproxy/redis/client-key.pem ``` +The synchronous L2 store exposes three bounded metric families: + +| Metric | Labels | +|---|---| +| `sbproxy_redis_kv_connections_total` | `result`: `success` or `error` | +| `sbproxy_redis_kv_operation_duration_seconds` | `operation`: `get`, `set`, `set_ttl`, `delete`, `increment`, `lock`, `unlock`, or `scan` | +| `sbproxy_redis_kv_operation_errors_total` | `operation` above and `reason`: `pool_timeout`, `connect_timeout`, `command_timeout`, `tls`, `auth`, `transport`, `server`, or `protocol` | + +Every general L2 call records one duration observation. A failed call adds one +error count, and each new connection attempt adds one connection result. None +of these labels includes an endpoint, tenant, application key, username, +database, or free-form error text. + For strict Redis leases, fences, coordination events, and the full fail-open table, see [AI context compression](ai-context-compression.md). @@ -23585,6 +23704,9 @@ Every metric SBproxy emits, what writes it, and what we promise about its name. | `sbproxy_rate_limit_decisions_total` | Counter | `config_only` (nothing emits this yet) | `alpha` | `policy`, `result` | Rate-limit middleware decisions, by policy and outcome. | | `sbproxy_rate_limit_suspend_total` | Counter | `stable` | `beta` | `workspace` | Workspace auto-suspend transitions. | | `sbproxy_rate_limit_total` | Counter | `stable` | `beta` | `workspace`, `result` | Workspace rate-limit budget outcomes by workspace and result (soft/throttle). | +| `sbproxy_redis_kv_connections_total` | Counter | `stable` | `beta` | `result` | Redis KV connection attempts by result. | +| `sbproxy_redis_kv_operation_duration_seconds` | Histogram | `stable` | `beta` | `operation` | Redis KV operation duration in seconds. | +| `sbproxy_redis_kv_operation_errors_total` | Counter | `stable` | `beta` | `operation`, `reason` | Redis KV operation failures by operation and reason. | | `sbproxy_request_duration_seconds` | Histogram | `stable` | `stable` | `hostname` | Request latency. | | `sbproxy_requests_total` | Counter | `stable` | `stable` | `hostname`, `method`, `status`, `agent_id`, `agent_class`, `agent_vendor`, `payment_rail`, `content_shape` | Total HTTP requests. | | `sbproxy_response_body_bytes` | Histogram | `stable` | `beta` | `direction` | Response body size, by compression direction. | @@ -32552,7 +32674,7 @@ The oracle engine, the `sb.yml` gate, and the runtime enforcement ship today. ================================================================ ## Troubleshooting -*Last modified: 2026-07-13* +*Last modified: 2026-07-18* When something breaks, this is the first place to look. Each section is one failure: the symptom, the likely cause, and the fix. For *why* these things happen, see [architecture.md](architecture.md); for what the proxy does on its own while a dependency is down, see [degradation.md](degradation.md); for the dashboard-to-action triage flow, see [operator-runbook.md](operator-runbook.md). @@ -32574,7 +32696,7 @@ Jump by symptom: | No traces in the backend | [Traces never arrive at the collector](#traces-never-arrive-at-the-collector) | | Admin port dead or 401/403 | [The admin server is unreachable or rejects you](#the-admin-server-is-unreachable-or-rejects-you) | | Dashboards empty | [Grafana dashboards show no data](#grafana-dashboards-show-no-data) | -| Cluster limits or shared cache misbehaving | [Redis went down and cluster behavior changed](#redis-went-down-and-cluster-behavior-changed) | +| Cluster limits or shared cache misbehaving | [Redis shared state is degraded](#redis-shared-state-is-degraded) | | TLS errors | [TLS handshake fails](#tls-handshake-fails) | | Cert expiring, renewal not happening | [ACME renewal is failing](#acme-renewal-is-failing) | | No HTTP/3 | [HTTP/3 requests fall back to HTTP/2](#http3-requests-fall-back-to-http2) | @@ -32729,15 +32851,96 @@ Check: - Send some traffic. Counters that have never incremented emit no series, and a fresh proxy with zero requests renders empty panels that look broken but are not. - Alert panels need the recording rules: `dashboards/prometheus/alerts.yml` references series computed by `dashboards/prometheus/recording-rules.yml`, so load both files. -## Redis went down and cluster behavior changed +## Redis shared state is degraded -With `proxy.l2_cache_settings` on Redis, an outage does not stop traffic, but shared state degrades to per-node until it reconnects. +With `proxy.l2_cache_settings` on Redis, a runtime connection failure does not +stop general traffic. A response-cache lookup failure bypasses the cache and +does not write that request's upstream response to an outage cache. A shared +rate-limit operation failure admits the request fail-open instead of switching +to a local bucket. `summary_buffer` also fails open for the primary AI request, +but it does not create worker-local summary state. Other L2 consumers retain +their feature-specific failure posture. Check: -- Expected during the outage: rate-limit counters go node-local (a multi-replica fleet lets slightly more traffic through a global limit), and response-cache entries written meanwhile stay local. This is the designed fallback behavior; see [degradation.md](degradation.md). -- There is no dedicated Redis metric family; confirm the outage in the logs, where failed Redis operations surface as errors on the rate-limit and cache paths. -- Reconnection is automatic: the client connects lazily and re-establishes the connection on the next operation once Redis is back. There is nothing to restart; fix Redis and the proxy re-attaches. -- Alert on this when running clustered, since the visible symptom (limits slightly leaky, cache hit rate down) is easy to miss. + +- Run `sbproxy validate --config sb.yml` with the required secret environment + already set. Do not print the expanded DSN. Validation catches malformed or + unsupported schemes, query strings, fragments, bad database syntax, missing + certificate/key partners, unreadable PEM files, and a mismatched client key. +- Remember that validation does not contact Redis. The first L2 operation + performs TLS, `AUTH`, and `SELECT`, so trust, credential, server-side database, + and reachability failures appear only when traffic uses shared state. +- Check that the SBproxy process can read `ca_file`, `cert_file`, and `key_file`. + `openssl x509 -in -noout -subject -issuer` safely checks certificate + parsing. Let `sbproxy validate` check that the client certificate and key + match, rather than dumping either file. +- Test Redis without putting the password or DSN on the command line. Set + `REDISCLI_AUTH` in the command environment and pass the host, port, CA, client + certificate, client key, username, and database as separate `redis-cli` + options. Run `PING` or `DBSIZE`; do not run `KEYS`, `SCAN`, or `GET` during a + privacy-sensitive incident. +- Expected during an outage: failed response-cache lookups fetch from the + upstream and do not arm cache write-back for that request. Failed shared + rate-limit increments admit the request without consulting a local bucket. + See [degradation.md](degradation.md). +- Reconnection is automatic. Broken connections leave the pool, and a later + operation opens a new connection. Fix Redis or the trust/authentication + configuration, then send a new cache miss or shared-state operation. SBproxy + does not need a restart when the configured connection material is unchanged. + +The runtime error reason points at the next check without exposing the Redis +response: + +| Reason | Check | +|---|---| +| `pool_timeout` | Pool saturation or an operation holding a slot too long | +| `connect_timeout` | Reachability and time spent in TCP, TLS, `AUTH`, or `SELECT` setup | +| `command_timeout` | Redis command latency and server load | +| `tls` | CA trust, server name, client certificate, and client key | +| `auth` | ACL username and password; percent-encoding of reserved URL characters | +| `transport` | Listener, network, reset, or dropped connection | +| `server` | Redis application errors, including a server-side `SELECT` rejection | +| `protocol` | Invalid or unexpected Redis protocol data | + +The platform health events named `redis store health failed`, +`redis store health remains failed`, and `redis store health recovered` are +transition-based. The first failure from an unknown or healthy state is +`WARN`, repeated platform health failures are `DEBUG`, and the first successful +operation after failure is `INFO`. Safe platform events look like this: + +```text +WARN operation="get" reason="tls" redis store health failed +INFO operation="get" redis store health recovered +``` + +That `WARN` to `DEBUG` to `INFO` sequence applies only to the platform health +events. Response-cache lookup, write, and invalidation call sites and the shared +rate-limit increment call site can emit a separate `WARN` for every failed +operation. Repeated consumer warnings do not mean that the platform transition +suppression failed. + +The three Redis L2 metric families use only closed labels: + +```bash +curl -fsS http://127.0.0.1:8080/metrics | + grep -E '^(# (HELP|TYPE) sbproxy_redis_kv_|sbproxy_redis_kv_)' +``` + +- `sbproxy_redis_kv_connections_total{result}` uses `success` or `error`. +- `sbproxy_redis_kv_operation_duration_seconds{operation}` uses `get`, `set`, + `set_ttl`, `delete`, `increment`, `lock`, `unlock`, or `scan`. +- `sbproxy_redis_kv_operation_errors_total{operation,reason}` uses the operation + values above and `pool_timeout`, `connect_timeout`, `command_timeout`, `tls`, + `auth`, `transport`, `server`, or `protocol`. + +No Redis metric or platform transition log includes the endpoint, tenant, application +key, username, database, credential, certificate path, or free-form server +error. Consumer warnings have their own fixed message text and are not governed +by the platform transition cadence. Correlate them with the closed-label +metrics; do not paste a DSN, expanded configuration, credential, cache key, or +cache value into a diagnostic command or incident ticket. Alert on connection +errors or operation errors when running more than one replica because the +application can continue while shared-state guarantees are degraded. ## TLS handshake fails diff --git a/docs/metrics-stability.md b/docs/metrics-stability.md index f551b6500..a899a5fa2 100644 --- a/docs/metrics-stability.md +++ b/docs/metrics-stability.md @@ -182,6 +182,9 @@ Every metric SBproxy emits, what writes it, and what we promise about its name. | `sbproxy_rate_limit_decisions_total` | Counter | `config_only` (nothing emits this yet) | `alpha` | `policy`, `result` | Rate-limit middleware decisions, by policy and outcome. | | `sbproxy_rate_limit_suspend_total` | Counter | `stable` | `beta` | `workspace` | Workspace auto-suspend transitions. | | `sbproxy_rate_limit_total` | Counter | `stable` | `beta` | `workspace`, `result` | Workspace rate-limit budget outcomes by workspace and result (soft/throttle). | +| `sbproxy_redis_kv_connections_total` | Counter | `stable` | `beta` | `result` | Redis KV connection attempts by result. | +| `sbproxy_redis_kv_operation_duration_seconds` | Histogram | `stable` | `beta` | `operation` | Redis KV operation duration in seconds. | +| `sbproxy_redis_kv_operation_errors_total` | Counter | `stable` | `beta` | `operation`, `reason` | Redis KV operation failures by operation and reason. | | `sbproxy_request_duration_seconds` | Histogram | `stable` | `stable` | `hostname` | Request latency. | | `sbproxy_requests_total` | Counter | `stable` | `stable` | `hostname`, `method`, `status`, `agent_id`, `agent_class`, `agent_vendor`, `payment_rail`, `content_shape` | Total HTTP requests. | | `sbproxy_response_body_bytes` | Histogram | `stable` | `beta` | `direction` | Response body size, by compression direction. | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index c191ce357..9391e6a1b 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -1,5 +1,5 @@ # Troubleshooting -*Last modified: 2026-07-13* +*Last modified: 2026-07-18* When something breaks, this is the first place to look. Each section is one failure: the symptom, the likely cause, and the fix. For *why* these things happen, see [architecture.md](architecture.md); for what the proxy does on its own while a dependency is down, see [degradation.md](degradation.md); for the dashboard-to-action triage flow, see [operator-runbook.md](operator-runbook.md). @@ -21,7 +21,7 @@ Jump by symptom: | No traces in the backend | [Traces never arrive at the collector](#traces-never-arrive-at-the-collector) | | Admin port dead or 401/403 | [The admin server is unreachable or rejects you](#the-admin-server-is-unreachable-or-rejects-you) | | Dashboards empty | [Grafana dashboards show no data](#grafana-dashboards-show-no-data) | -| Cluster limits or shared cache misbehaving | [Redis went down and cluster behavior changed](#redis-went-down-and-cluster-behavior-changed) | +| Cluster limits or shared cache misbehaving | [Redis shared state is degraded](#redis-shared-state-is-degraded) | | TLS errors | [TLS handshake fails](#tls-handshake-fails) | | Cert expiring, renewal not happening | [ACME renewal is failing](#acme-renewal-is-failing) | | No HTTP/3 | [HTTP/3 requests fall back to HTTP/2](#http3-requests-fall-back-to-http2) | @@ -176,15 +176,96 @@ Check: - Send some traffic. Counters that have never incremented emit no series, and a fresh proxy with zero requests renders empty panels that look broken but are not. - Alert panels need the recording rules: `dashboards/prometheus/alerts.yml` references series computed by `dashboards/prometheus/recording-rules.yml`, so load both files. -## Redis went down and cluster behavior changed +## Redis shared state is degraded -With `proxy.l2_cache_settings` on Redis, an outage does not stop traffic, but shared state degrades to per-node until it reconnects. +With `proxy.l2_cache_settings` on Redis, a runtime connection failure does not +stop general traffic. A response-cache lookup failure bypasses the cache and +does not write that request's upstream response to an outage cache. A shared +rate-limit operation failure admits the request fail-open instead of switching +to a local bucket. `summary_buffer` also fails open for the primary AI request, +but it does not create worker-local summary state. Other L2 consumers retain +their feature-specific failure posture. Check: -- Expected during the outage: rate-limit counters go node-local (a multi-replica fleet lets slightly more traffic through a global limit), and response-cache entries written meanwhile stay local. This is the designed fallback behavior; see [degradation.md](degradation.md). -- There is no dedicated Redis metric family; confirm the outage in the logs, where failed Redis operations surface as errors on the rate-limit and cache paths. -- Reconnection is automatic: the client connects lazily and re-establishes the connection on the next operation once Redis is back. There is nothing to restart; fix Redis and the proxy re-attaches. -- Alert on this when running clustered, since the visible symptom (limits slightly leaky, cache hit rate down) is easy to miss. + +- Run `sbproxy validate --config sb.yml` with the required secret environment + already set. Do not print the expanded DSN. Validation catches malformed or + unsupported schemes, query strings, fragments, bad database syntax, missing + certificate/key partners, unreadable PEM files, and a mismatched client key. +- Remember that validation does not contact Redis. The first L2 operation + performs TLS, `AUTH`, and `SELECT`, so trust, credential, server-side database, + and reachability failures appear only when traffic uses shared state. +- Check that the SBproxy process can read `ca_file`, `cert_file`, and `key_file`. + `openssl x509 -in -noout -subject -issuer` safely checks certificate + parsing. Let `sbproxy validate` check that the client certificate and key + match, rather than dumping either file. +- Test Redis without putting the password or DSN on the command line. Set + `REDISCLI_AUTH` in the command environment and pass the host, port, CA, client + certificate, client key, username, and database as separate `redis-cli` + options. Run `PING` or `DBSIZE`; do not run `KEYS`, `SCAN`, or `GET` during a + privacy-sensitive incident. +- Expected during an outage: failed response-cache lookups fetch from the + upstream and do not arm cache write-back for that request. Failed shared + rate-limit increments admit the request without consulting a local bucket. + See [degradation.md](degradation.md). +- Reconnection is automatic. Broken connections leave the pool, and a later + operation opens a new connection. Fix Redis or the trust/authentication + configuration, then send a new cache miss or shared-state operation. SBproxy + does not need a restart when the configured connection material is unchanged. + +The runtime error reason points at the next check without exposing the Redis +response: + +| Reason | Check | +|---|---| +| `pool_timeout` | Pool saturation or an operation holding a slot too long | +| `connect_timeout` | Reachability and time spent in TCP, TLS, `AUTH`, or `SELECT` setup | +| `command_timeout` | Redis command latency and server load | +| `tls` | CA trust, server name, client certificate, and client key | +| `auth` | ACL username and password; percent-encoding of reserved URL characters | +| `transport` | Listener, network, reset, or dropped connection | +| `server` | Redis application errors, including a server-side `SELECT` rejection | +| `protocol` | Invalid or unexpected Redis protocol data | + +The platform health events named `redis store health failed`, +`redis store health remains failed`, and `redis store health recovered` are +transition-based. The first failure from an unknown or healthy state is +`WARN`, repeated platform health failures are `DEBUG`, and the first successful +operation after failure is `INFO`. Safe platform events look like this: + +```text +WARN operation="get" reason="tls" redis store health failed +INFO operation="get" redis store health recovered +``` + +That `WARN` to `DEBUG` to `INFO` sequence applies only to the platform health +events. Response-cache lookup, write, and invalidation call sites and the shared +rate-limit increment call site can emit a separate `WARN` for every failed +operation. Repeated consumer warnings do not mean that the platform transition +suppression failed. + +The three Redis L2 metric families use only closed labels: + +```bash +curl -fsS http://127.0.0.1:8080/metrics | + grep -E '^(# (HELP|TYPE) sbproxy_redis_kv_|sbproxy_redis_kv_)' +``` + +- `sbproxy_redis_kv_connections_total{result}` uses `success` or `error`. +- `sbproxy_redis_kv_operation_duration_seconds{operation}` uses `get`, `set`, + `set_ttl`, `delete`, `increment`, `lock`, `unlock`, or `scan`. +- `sbproxy_redis_kv_operation_errors_total{operation,reason}` uses the operation + values above and `pool_timeout`, `connect_timeout`, `command_timeout`, `tls`, + `auth`, `transport`, `server`, or `protocol`. + +No Redis metric or platform transition log includes the endpoint, tenant, application +key, username, database, credential, certificate path, or free-form server +error. Consumer warnings have their own fixed message text and are not governed +by the platform transition cadence. Correlate them with the closed-label +metrics; do not paste a DSN, expanded configuration, credential, cache key, or +cache value into a diagnostic command or incident ticket. Alert on connection +errors or operation errors when running more than one replica because the +application can continue while shared-state guarantees are degraded. ## TLS handshake fails diff --git a/examples/redis-l2-secure/.gitignore b/examples/redis-l2-secure/.gitignore new file mode 100644 index 000000000..df9128702 --- /dev/null +++ b/examples/redis-l2-secure/.gitignore @@ -0,0 +1 @@ +certs/ diff --git a/examples/redis-l2-secure/README.md b/examples/redis-l2-secure/README.md new file mode 100644 index 000000000..9cef5935e --- /dev/null +++ b/examples/redis-l2-secure/README.md @@ -0,0 +1,208 @@ +# Secure Redis L2 development setup + +*Last modified: 2026-07-18* + +This example runs Redis with a TLS-only listener, required client certificates, +password authentication, and logical database 7. SBproxy verifies the generated +CA, presents its client identity, authenticates as the Redis `default` user, and +uses the service for shared L2 cache state. + +> Development only. The generated CA, leaf certificates, private keys, and +> `development-only-password` are short-lived local fixtures. Do not copy them +> into a production deployment or commit anything under `certs/`. + +Run every command below from the repository root. You need OpenSSL, Docker with +Compose, curl, and the Rust toolchain. + +## Generate the local PKI + +Export the fixture password first. The certificate script hashes it into an +ignored Redis ACL file and defaults to the same value when the variable is +unset. + +```bash +export REDIS_PASSWORD='development-only-password' +./examples/redis-l2-secure/generate-certs.sh +``` + +The script creates a development CA, a localhost Redis server identity, an +SBproxy client identity, a second untrusted CA for the negative test, and the +ACL under `examples/redis-l2-secure/certs/`. The directory is ignored by Git. + +## Start TLS-only Redis + +```bash +REDIS_PASSWORD='development-only-password' \ + docker compose -f examples/redis-l2-secure/docker-compose.yml up -d --wait redis +``` + +The container publishes its TLS port on `127.0.0.1:6380`. Plaintext Redis is +disabled with `port 0`, and `tls-auth-clients yes` rejects clients that do not +present a certificate signed by the development CA. + +Confirm that the authenticated mTLS connection works without placing the +password on the `redis-cli` command line: + +```bash +REDIS_PASSWORD='development-only-password' \ + docker compose -f examples/redis-l2-secure/docker-compose.yml exec -T redis \ + sh -ec 'REDISCLI_AUTH="$REDIS_PASSWORD" redis-cli --tls \ + --cacert /source-certs/ca.pem \ + --cert /source-certs/client.pem \ + --key /source-certs/client.key \ + -h localhost -p 6379 ping' +``` + +The command prints `PONG`. + +## Validate and start SBproxy + +Validation reads and checks the DSN and PEM files but does not open a network +connection: + +```bash +REDIS_PASSWORD='development-only-password' \ + cargo run -q -p sbproxy -- validate examples/redis-l2-secure/sb.yml +``` + +Start SBproxy in the first terminal: + +```bash +REDIS_PASSWORD='development-only-password' \ + cargo run -q -p sbproxy -- serve -f examples/redis-l2-secure/sb.yml +``` + +## Prove cache storage in database 7 + +In a second terminal, send the same cacheable request twice. The first request +loads the response and writes shared state. The second response carries the +cache hit header. + +```bash +curl -fsS -D /tmp/sbproxy-redis-l2-first.headers -o /dev/null \ + -H 'Host: redis-l2.local' \ + 'http://127.0.0.1:8080/get?redis_l2_secure=cache-proof' + +curl -fsS -D /tmp/sbproxy-redis-l2-second.headers -o /dev/null \ + -H 'Host: redis-l2.local' \ + 'http://127.0.0.1:8080/get?redis_l2_secure=cache-proof' + +grep -i '^x-sbproxy-cache: HIT' /tmp/sbproxy-redis-l2-second.headers +``` + +Check key counts without printing cache keys or values: + +```bash +REDIS_PASSWORD='development-only-password' \ + docker compose -f examples/redis-l2-secure/docker-compose.yml exec -T redis \ + sh -ec ' + export REDISCLI_AUTH="$REDIS_PASSWORD" + printf "database 7 keys: " + redis-cli --tls --cacert /source-certs/ca.pem \ + --cert /source-certs/client.pem --key /source-certs/client.key \ + -h localhost -p 6379 -n 7 DBSIZE + printf "database 0 keys: " + redis-cli --tls --cacert /source-certs/ca.pem \ + --cert /source-certs/client.pem --key /source-certs/client.key \ + -h localhost -p 6379 -n 0 DBSIZE + ' +``` + +Database 7 has at least one key after the cache write. Database 0 remains at +zero in a fresh example. + +## Prove trust and authentication failures + +Stop the running SBproxy process with `Ctrl-C` before each probe. Configuration +still validates because network establishment is intentionally lazy. The first +cache operation triggers the connection failure, bypasses the cache, and +continues to the upstream. A failed lookup does not arm response write-back, so +that request does not create either a Redis entry or a process-local outage +entry. This example does not enable rate limiting; when a configured shared +rate-limit operation fails, SBproxy admits the request fail-open instead of +switching to a local bucket. + +Start with the untrusted CA: + +```bash +REDIS_PASSWORD='development-only-password' \ +REDIS_CA_FILE='examples/redis-l2-secure/certs/wrong-ca.pem' \ + cargo run -q -p sbproxy -- serve -f examples/redis-l2-secure/sb.yml +``` + +From the second terminal, trigger a new cache operation: + +```bash +curl -fsS -o /dev/null -H 'Host: redis-l2.local' \ + 'http://127.0.0.1:8080/get?redis_l2_secure=wrong-ca' +``` + +The first Redis failure produces a platform `WARN` named +`redis store health failed` with `reason="tls"`. That platform event contains no +DSN, endpoint, database, key, value, certificate path, username, or password. +The response-cache call site also emits `cache lookup error, bypassing cache` at +`WARN` for the failed lookup. + +Stop that process, then start with the wrong password: + +```bash +REDIS_PASSWORD='wrong-development-password' \ + cargo run -q -p sbproxy -- serve -f examples/redis-l2-secure/sb.yml +``` + +Trigger another new cache operation: + +```bash +curl -fsS -o /dev/null -H 'Host: redis-l2.local' \ + 'http://127.0.0.1:8080/get?redis_l2_secure=wrong-password' +``` + +The platform transition warning uses `reason="auth"` and does not include the +server's response text. Repeated platform events named +`redis store health remains failed` stay at `DEBUG` until a successful operation +moves the store back to healthy and emits the `INFO` event +`redis store health recovered`. Response-cache call sites can still emit their +own `WARN` for every failed operation. Correlate those fixed messages with the +closed-label metrics below; do not paste the expanded DSN, credentials, cache +keys, or cache values into shell history or an incident ticket. + +## Query the Redis L2 metrics + +After either negative probe, query all three metric families from the local +metrics endpoint: + +```bash +curl -fsS http://127.0.0.1:8080/metrics | + grep -E '^(# (HELP|TYPE) sbproxy_redis_kv_|sbproxy_redis_kv_)' +``` + +The output is limited to these families and labels: + +| Metric | Labels and allowed values | +|---|---| +| `sbproxy_redis_kv_connections_total` | `result`: `success`, `error` | +| `sbproxy_redis_kv_operation_duration_seconds` | `operation`: `get`, `set`, `set_ttl`, `delete`, `increment`, `lock`, `unlock`, `scan` | +| `sbproxy_redis_kv_operation_errors_total` | `operation` above; `reason`: `pool_timeout`, `connect_timeout`, `command_timeout`, `tls`, `auth`, `transport`, `server`, `protocol` | + +The labels never contain an endpoint, tenant, application key, username, +password, or database number. A family appears after the process first records +the corresponding event. + +## Stop the example + +Stop SBproxy with `Ctrl-C`, then remove the Redis container: + +```bash +REDIS_PASSWORD='development-only-password' \ + docker compose -f examples/redis-l2-secure/docker-compose.yml down +``` + +The generated files remain under the ignored `certs/` directory so you can run +the example again. Re-run `generate-certs.sh` whenever you want fresh fixtures. + +## See also + +- [Configuration reference](../../docs/configuration.md#redis-integration) +- [Dependency degradation](../../docs/degradation.md#redis-l2-cache-and-cross-replica-state) +- [Troubleshooting](../../docs/troubleshooting.md#redis-shared-state-is-degraded) +- [AI context compression](../../docs/ai-context-compression.md#redis-state) diff --git a/examples/redis-l2-secure/docker-compose.yml b/examples/redis-l2-secure/docker-compose.yml new file mode 100644 index 000000000..8cfba2f16 --- /dev/null +++ b/examples/redis-l2-secure/docker-compose.yml @@ -0,0 +1,44 @@ +# Development-only TLS Redis for the secure L2 example. +# Generate examples/redis-l2-secure/certs before starting this service. + +services: + redis: + image: redis:7-alpine + ports: + - "127.0.0.1:6380:6379" + environment: + REDIS_PASSWORD: "${REDIS_PASSWORD:-development-only-password}" + command: + - /bin/sh + - -ec + - | + mkdir -p /run/redis-tls + cp /source-certs/ca.pem /source-certs/server.pem /source-certs/server.key /source-certs/users.acl /run/redis-tls/ + chown -R redis:redis /run/redis-tls + chmod 0444 /run/redis-tls/ca.pem /run/redis-tls/server.pem + chmod 0400 /run/redis-tls/server.key /run/redis-tls/users.acl + exec su-exec redis redis-server /usr/local/etc/redis/redis.conf + volumes: + - ./redis/redis.conf:/usr/local/etc/redis/redis.conf:ro + - ./certs/ca.pem:/source-certs/ca.pem:ro + - ./certs/server.pem:/source-certs/server.pem:ro + - ./certs/server.key:/source-certs/server.key:ro + - ./certs/client.pem:/source-certs/client.pem:ro + - ./certs/client.key:/source-certs/client.key:ro + - ./certs/users.acl:/source-certs/users.acl:ro + read_only: true + tmpfs: + - /run/redis-tls + - /tmp + healthcheck: + test: + - CMD-SHELL + - >- + REDISCLI_AUTH="$$REDIS_PASSWORD" + redis-cli --tls --cacert /source-certs/ca.pem + --cert /source-certs/client.pem --key /source-certs/client.key + -h localhost -p 6379 ping | grep -qx PONG + interval: 2s + timeout: 2s + retries: 20 + start_period: 2s diff --git a/examples/redis-l2-secure/generate-certs.sh b/examples/redis-l2-secure/generate-certs.sh new file mode 100755 index 000000000..5fea767ec --- /dev/null +++ b/examples/redis-l2-secure/generate-certs.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash + +set -euo pipefail + +example_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cert_dir="${example_dir}/certs" +redis_password="${REDIS_PASSWORD:-development-only-password}" + +for required_command in openssl awk; do + if ! command -v "${required_command}" >/dev/null 2>&1; then + echo "missing required command: ${required_command}" >&2 + exit 1 + fi +done + +umask 077 +mkdir -p "${cert_dir}" + +server_csr="${cert_dir}/server.csr" +server_ext="${cert_dir}/server-ext.cnf" +client_csr="${cert_dir}/client.csr" +client_ext="${cert_dir}/client-ext.cnf" +ca_serial="${cert_dir}/ca.srl" + +cleanup() { + rm -f "${server_csr}" "${server_ext}" "${client_csr}" "${client_ext}" "${ca_serial}" +} +trap cleanup EXIT +cleanup + +openssl req -x509 -newkey rsa:2048 -sha256 -nodes -days 30 \ + -keyout "${cert_dir}/ca.key" \ + -out "${cert_dir}/ca.pem" \ + -subj "/CN=sbproxy-redis-development-ca" \ + -addext "basicConstraints=critical,CA:TRUE" \ + -addext "keyUsage=critical,keyCertSign,cRLSign" \ + >/dev/null 2>&1 + +openssl req -newkey rsa:2048 -sha256 -nodes \ + -keyout "${cert_dir}/server.key" \ + -out "${server_csr}" \ + -subj "/CN=localhost" \ + >/dev/null 2>&1 + +printf '%s\n' \ + 'basicConstraints=critical,CA:FALSE' \ + 'keyUsage=critical,digitalSignature,keyEncipherment' \ + 'extendedKeyUsage=serverAuth' \ + 'subjectAltName=DNS:localhost,DNS:redis,IP:127.0.0.1' \ + >"${server_ext}" + +openssl x509 -req -sha256 -days 30 \ + -in "${server_csr}" \ + -CA "${cert_dir}/ca.pem" \ + -CAkey "${cert_dir}/ca.key" \ + -CAcreateserial \ + -out "${cert_dir}/server.pem" \ + -extfile "${server_ext}" \ + >/dev/null 2>&1 + +openssl req -newkey rsa:2048 -sha256 -nodes \ + -keyout "${cert_dir}/client.key" \ + -out "${client_csr}" \ + -subj "/CN=sbproxy-redis-development-client" \ + >/dev/null 2>&1 + +printf '%s\n' \ + 'basicConstraints=critical,CA:FALSE' \ + 'keyUsage=critical,digitalSignature,keyEncipherment' \ + 'extendedKeyUsage=clientAuth' \ + >"${client_ext}" + +openssl x509 -req -sha256 -days 30 \ + -in "${client_csr}" \ + -CA "${cert_dir}/ca.pem" \ + -CAkey "${cert_dir}/ca.key" \ + -CAserial "${ca_serial}" \ + -out "${cert_dir}/client.pem" \ + -extfile "${client_ext}" \ + >/dev/null 2>&1 + +# A second CA is useful for the documented trust-failure check. It never signs +# the Redis server identity. +openssl req -x509 -newkey rsa:2048 -sha256 -nodes -days 30 \ + -keyout "${cert_dir}/wrong-ca.key" \ + -out "${cert_dir}/wrong-ca.pem" \ + -subj "/CN=sbproxy-redis-wrong-development-ca" \ + -addext "basicConstraints=critical,CA:TRUE" \ + -addext "keyUsage=critical,keyCertSign,cRLSign" \ + >/dev/null 2>&1 + +# Redis ACL files accept SHA-256 password hashes. Keeping even the development +# password out of the generated file makes accidental inspection less harmful. +password_hash="$(printf '%s' "${redis_password}" | openssl dgst -sha256 -r | awk '{print $1}')" +printf 'user default on #%s ~* &* +@all\n' "${password_hash}" >"${cert_dir}/users.acl" + +chmod 0600 \ + "${cert_dir}/ca.key" \ + "${cert_dir}/server.key" \ + "${cert_dir}/client.key" \ + "${cert_dir}/wrong-ca.key" \ + "${cert_dir}/users.acl" +chmod 0644 \ + "${cert_dir}/ca.pem" \ + "${cert_dir}/server.pem" \ + "${cert_dir}/client.pem" \ + "${cert_dir}/wrong-ca.pem" + +openssl verify -purpose sslserver -CAfile "${cert_dir}/ca.pem" "${cert_dir}/server.pem" +openssl verify -purpose sslclient -CAfile "${cert_dir}/ca.pem" "${cert_dir}/client.pem" + +echo +echo "Generated development-only Redis TLS fixtures in ${cert_dir}" +echo "Do not use these certificates, keys, or the example password in production." diff --git a/examples/redis-l2-secure/redis/redis.conf b/examples/redis-l2-secure/redis/redis.conf new file mode 100644 index 000000000..1285ece97 --- /dev/null +++ b/examples/redis-l2-secure/redis/redis.conf @@ -0,0 +1,20 @@ +# Development-only Redis configuration for the secure L2 example. +# Redis accepts TLS connections only and requires a client certificate. + +bind 0.0.0.0 +protected-mode yes + +port 0 +tls-port 6379 +tls-cert-file /run/redis-tls/server.pem +tls-key-file /run/redis-tls/server.key +tls-ca-cert-file /run/redis-tls/ca.pem +tls-auth-clients yes + +aclfile /run/redis-tls/users.acl +databases 16 + +save "" +appendonly no +dir /tmp +loglevel notice diff --git a/examples/redis-l2-secure/sb.yml b/examples/redis-l2-secure/sb.yml new file mode 100644 index 000000000..dc4352a26 --- /dev/null +++ b/examples/redis-l2-secure/sb.yml @@ -0,0 +1,22 @@ +# yaml-language-server: $schema=../../schemas/sb-config.schema.json +# Development-only Redis L2 example with verified TLS, mTLS, password auth, +# and logical database 7. Generate the ignored certs/ files before validation. + +proxy: + http_bind_port: 8080 + l2_cache_settings: + driver: redis + params: + dsn: "rediss://default:${REDIS_PASSWORD}@localhost:6380/7" + ca_file: "${REDIS_CA_FILE:-examples/redis-l2-secure/certs/ca.pem}" + cert_file: "${REDIS_CLIENT_CERT_FILE:-examples/redis-l2-secure/certs/client.pem}" + key_file: "${REDIS_CLIENT_KEY_FILE:-examples/redis-l2-secure/certs/client.key}" + +origins: + "redis-l2.local": + action: + type: proxy + url: https://test.sbproxy.dev + response_cache: + enabled: true + ttl_secs: 60 diff --git a/schemas/sb-config.schema.json b/schemas/sb-config.schema.json index 6e14f71cc..2aa62b085 100644 --- a/schemas/sb-config.schema.json +++ b/schemas/sb-config.schema.json @@ -4067,7 +4067,10 @@ "params": { "description": "Driver-specific parameters.", "default": { - "dsn": "" + "ca_file": null, + "cert_file": null, + "dsn": "", + "key_file": null }, "allOf": [ { @@ -4082,9 +4085,33 @@ "type": "object", "properties": { "dsn": { - "description": "Connection DSN. For `redis` drivers this is a `redis://host:port[/db]` URL. Only the host:port portion is parsed today; the DB index is ignored.", + "description": "Redis connection DSN. Supports `redis://`, `rediss://`, credentials, bracketed IPv6 addresses, and a non-negative logical database.", "default": "", "type": "string" + }, + "ca_file": { + "description": "Optional path to PEM-encoded Redis trust anchors for a private CA.", + "default": null, + "type": [ + "string", + "null" + ] + }, + "cert_file": { + "description": "Optional path to a PEM-encoded Redis client certificate chain. Must be configured together with `key_file` and requires `rediss://`.", + "default": null, + "type": [ + "string", + "null" + ] + }, + "key_file": { + "description": "Optional path to the PEM-encoded Redis client private key. Must be configured together with `cert_file` and requires `rediss://`.", + "default": null, + "type": [ + "string", + "null" + ] } } },