From 3de1f0caec79d298c9bddff15930ec31e12e7112 Mon Sep 17 00:00:00 2001 From: Yujong Lee Date: Thu, 5 Nov 2026 06:19:06 +0900 Subject: [PATCH] fix(config): refuse OCSP validation instead of connecting without it ssl_validate_ocsp and ssl_validate_ocsp_stapled were dropped with a warning, so a config that asked for revocation checks connected without them. rustls has no client-side OCSP, so a true value from the kwargs, REDIS_* or a url query now fails resolution with RedisError::Unenforceable. redis-py 5.3.1's async client refuses both with a TypeError, so this matches the proxy's path. A false value asks for nothing and is dropped silently Co-Authored-By: Claude Opus 5.5 --- litellm-rust/crates/config/AGENTS.md | 5 ++- litellm-rust/crates/config/src/redis/error.rs | 2 + litellm-rust/crates/config/src/redis/keys.rs | 17 ++++--- .../crates/config/src/redis/options.rs | 16 +++++++ litellm-rust/crates/config/src/redis/tls.rs | 4 +- litellm-rust/crates/config/tests/redis.rs | 45 +++++++++++++++++++ tests/unit/test_redis.py | 11 +++++ 7 files changed, 90 insertions(+), 10 deletions(-) diff --git a/litellm-rust/crates/config/AGENTS.md b/litellm-rust/crates/config/AGENTS.md index 2c43a88a5c9..7d89ee8d4ff 100644 --- a/litellm-rust/crates/config/AGENTS.md +++ b/litellm-rust/crates/config/AGENTS.md @@ -14,8 +14,9 @@ - Numbers are read with `litellm_python_compat::number`, so whitespace and digit underscores parse like Python's `int()` and `float()`. A value they cannot parse (`port: abc`, `socket_timeout: slow`) is dropped with a warning and the default is used, like `_coerce_redis_kwargs_types`. A number that parses but is out of range (negative seconds, a port above 65535) fails with `RedisError::Invalid` - Flags (`ssl`, `ssl_check_hostname`, `aws_iam_auth`, `aws_iam_serverless`) are read one way on every surface. `true`, `1` and `yes` are true and `false`, `0`, `no`, `off`, `f` and `n` are false, the text redis-py's `_str_to_bool` and the proxy's pydantic models agree on. Any other text is `RedisError::Invalid`, so a TLS setting is never guessed - `RedisSettings::coordination` mirrors the proxy's `CoordinationRedisParams` pydantic model, which rejects a malformed `port` before redis-py coerces anything -- The resolver never fails on a setting it cannot honor. A `Warn` row is dropped and returned as an `Ignored` naming its source (`ssl_ciphers` or `REDIS_SSL_CIPHERS`) and the consequence from `keys::consequence`, because a Python deployment that worked must keep working when its stores move to Rust. Only a value that is malformed (`RedisError::Invalid`) or a target that is missing fails. Promoting a `Warn` row to `Read` is then a pure improvement with no behavior cliff - - `ssl_ciphers`, `ssl_min_version` above TLS 1.2, the OCSP options, `connection_pool`, `event_dispatcher`, `redis_connect_func` and `retry` are ignored. Ignoring OCSP connects without revocation checks, and the consequence says so +- The resolver fails on a setting it cannot honor only when that setting is a security check. A `Warn` row is dropped and returned as an `Ignored` naming its source (`ssl_ciphers` or `REDIS_SSL_CIPHERS`) and the consequence from `keys::consequence`, because a Python deployment that worked must keep working when its stores move to Rust. Only a value that is malformed (`RedisError::Invalid`) or a target that is missing fails. Promoting a `Warn` row to `Read` is then a pure improvement with no behavior cliff + - `ssl_ciphers`, `ssl_min_version` above TLS 1.2, `ssl_ocsp_context`, `ssl_ocsp_expected_cert`, `connection_pool`, `event_dispatcher`, `redis_connect_func` and `retry` are ignored + - A `Reject` row fails closed. `ssl_validate_ocsp` and `ssl_validate_ocsp_stapled` set to true, from the kwargs, `REDIS_*` or a url query, are `RedisError::Unenforceable`, because rustls has no client-side OCSP and connecting without the check would silently drop it. redis-py 5.3.1's async client refuses both with a `TypeError`, so this matches the proxy's path. Set to false they ask for nothing and are dropped without a warning - `ssl_ca_data` and `ssl_ca_path` are read into `TlsOptions` and trusted next to `ssl_ca_certs`. `ssl_password` is read into `TlsOptions::key_password`, which `redis-client` uses to decrypt a PKCS#8 key like the sync client's `load_cert_chain(password=...)`. redis-py 5.3.1's async client refuses it, so this is a superset of the path the proxy uses, like `ssl_ca_path` `unix_socket_path` resolves to `Target::Unix`, which like redis-py's `UnixDomainSocketConnection` replaces host, port and TLS. Python still requires `host` next to it, so Rust does too - A url's `?db=`, `?username=` and `?password=` are read like `parse_url` reads them, so `?db=` wins over the path and the userinfo wins over `?username=` and `?password=`. Any other query parameter except the TLS ones on a `rediss://` url is ignored naming `url ?`. redis-py turns each one into a client kwarg - `retry_on_timeout`, `retry_on_error`, `protocol`, `cache`, `cache_config`, `socket_keepalive` and `max_connections` are ignored. Cluster clients still turn TCP keepalive on, like Python's async cluster default diff --git a/litellm-rust/crates/config/src/redis/error.rs b/litellm-rust/crates/config/src/redis/error.rs index c8e9c6079df..dd93cfd53c7 100644 --- a/litellm-rust/crates/config/src/redis/error.rs +++ b/litellm-rust/crates/config/src/redis/error.rs @@ -12,6 +12,8 @@ pub enum RedisError { DbInCluster { setting: String }, #[error("credential_provider is a host object, and the host passed no handle for it")] ProviderWithoutHandle, + #[error("{setting} asks for OCSP revocation checks, which rustls cannot perform, so Redis is not connected without them")] + Unenforceable { setting: String }, #[error("Redis setting {setting} is not plain data")] NotPlainData { setting: String }, #[error("AWS ElastiCache IAM Redis authentication requires TLS")] diff --git a/litellm-rust/crates/config/src/redis/keys.rs b/litellm-rust/crates/config/src/redis/keys.rs index 7d0bac71cc5..66ed0a450fd 100644 --- a/litellm-rust/crates/config/src/redis/keys.rs +++ b/litellm-rust/crates/config/src/redis/keys.rs @@ -6,12 +6,14 @@ use crate::Value; // Every Redis setting litellm/_redis.py reads, with how Rust handles it and where else Python looks for it. // Warn rows are read by Python and dropped here with a warning naming the consequence, never an error. +// Reject rows are security checks Rust cannot perform, so asking for one fails instead of connecting without it. // environment marks the kwargs _get_redis_env_kwarg_mapping reads as REDIS_, and fallback lists the names // _get_redis_client_logic consults with get_secret when the merged kwargs lack the setting #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(super) enum Handling { Read, Warn, + Reject, } pub(super) struct Setting { @@ -150,8 +152,8 @@ pub(super) const SETTINGS: &[Setting] = &[ Setting::new("ssl_ocsp_context", Handling::Warn, true, &[]), Setting::new("ssl_ocsp_expected_cert", Handling::Warn, true, &[]), Setting::new("ssl_password", Handling::Read, true, &[]), - Setting::new("ssl_validate_ocsp", Handling::Warn, true, &[]), - Setting::new("ssl_validate_ocsp_stapled", Handling::Warn, true, &[]), + Setting::new("ssl_validate_ocsp", Handling::Reject, true, &[]), + Setting::new("ssl_validate_ocsp_stapled", Handling::Reject, true, &[]), Setting::new( "startup_nodes", Handling::Read, @@ -163,6 +165,10 @@ pub(super) const SETTINGS: &[Setting] = &[ Setting::new("username", Handling::Read, true, &[]), ]; +pub(super) fn is_refused_when_set(kwarg: &str) -> bool { + setting(kwarg).is_some_and(|setting| setting.handling == Handling::Reject) +} + pub(super) fn ignored(setting: String, kwarg: &str) -> Ignored { Ignored { setting, @@ -215,11 +221,8 @@ pub(super) fn is_host_object(kwarg: &str) -> bool { /// rather than only the key name. fn consequence(kwarg: &str) -> &'static str { match kwarg { - "ssl_validate_ocsp" - | "ssl_validate_ocsp_stapled" - | "ssl_ocsp_context" - | "ssl_ocsp_expected_cert" => { - "rustls has no client-side OCSP, so the connection is made without revocation checks" + "ssl_ocsp_context" | "ssl_ocsp_expected_cert" => { + "it only configures ssl_validate_ocsp, which the Rust client refuses when set" } "ssl_ciphers" | "ssl_min_version" => { "rustls negotiates TLS 1.2+ with its own ECDHE AEAD suites" diff --git a/litellm-rust/crates/config/src/redis/options.rs b/litellm-rust/crates/config/src/redis/options.rs index 70af41ad074..eba4f655b64 100644 --- a/litellm-rust/crates/config/src/redis/options.rs +++ b/litellm-rust/crates/config/src/redis/options.rs @@ -88,6 +88,7 @@ pub(super) struct Unmodeled { impl Unmodeled { fn read(kwargs: &Kwargs) -> Result { + refuse_unenforceable(kwargs.iter())?; let unmodeled = || { kwargs .iter() @@ -185,6 +186,21 @@ impl Options { } } +/// A `Reject` row that is set to true asks for a check Rust cannot perform. One set to false asks for nothing. +pub(super) fn refuse_unenforceable<'a>( + entries: impl IntoIterator, +) -> Result<(), RedisError> { + entries + .into_iter() + .filter(|(key, _)| keys::is_refused_when_set(key)) + .try_for_each(|(_, entry)| match flag(Some(entry))? { + true => Err(RedisError::Unenforceable { + setting: entry.source.clone(), + }), + false => Ok(()), + }) +} + fn handling(key: &str) -> Option { keys::setting(key).map(|setting| setting.handling) } diff --git a/litellm-rust/crates/config/src/redis/tls.rs b/litellm-rust/crates/config/src/redis/tls.rs index d9daaef6b67..91f0f03ac5d 100644 --- a/litellm-rust/crates/config/src/redis/tls.rs +++ b/litellm-rust/crates/config/src/redis/tls.rs @@ -5,7 +5,7 @@ use litellm_secret_value::SecretValue; use super::{ Expected, RedisError, keys, layers::Entry, - options::{TlsInput, flag, parsed, secret, text}, + options::{self, TlsInput, flag, parsed, secret, text}, }; use crate::Value; @@ -83,6 +83,7 @@ pub(super) fn url_query(url: &str) -> Result { ) }) .collect(); + options::refuse_unenforceable(query.iter().map(|(name, entry)| (name, entry)))?; let (query, ignored): (Vec<_>, Vec<_>) = query.into_iter().partition(|(name, _)| { URL_CONNECTION_SETTINGS.contains(&name.as_str()) || (tls_scheme && URL_TLS_SETTINGS.contains(&name.as_str())) @@ -118,6 +119,7 @@ pub(super) fn url_query(url: &str) -> Result { password: secret(get("password").as_ref()), ignored: ignored .into_iter() + .filter(|(name, _)| !keys::is_refused_when_set(name)) .map(|(name, entry)| keys::ignored(entry.source, &name)) .collect(), }) diff --git a/litellm-rust/crates/config/tests/redis.rs b/litellm-rust/crates/config/tests/redis.rs index c3dbce45ce1..6ce4f52fe41 100644 --- a/litellm-rust/crates/config/tests/redis.rs +++ b/litellm-rust/crates/config/tests/redis.rs @@ -187,6 +187,46 @@ fn unsupported_settings_resolve_and_are_named_by_their_source( assert!(!ignored.consequence.is_empty()); } +#[rstest] +#[case::kwarg( + "general_settings:\n coordination_redis:\n host: redis\n ssl: true\n ssl_validate_ocsp: true", + &[], + "ssl_validate_ocsp" +)] +#[case::stapled_from_environment( + "model_list: []", + &[("REDIS_HOST", "redis"), ("REDIS_SSL", "true"), ("REDIS_SSL_VALIDATE_OCSP_STAPLED", "yes")], + "REDIS_SSL_VALIDATE_OCSP_STAPLED" +)] +#[case::rediss_url_query( + "general_settings:\n coordination_redis:\n url: rediss://cache:6380/0?ssl_validate_ocsp=true", + &[], + "url ?ssl_validate_ocsp" +)] +fn an_ocsp_check_fails_instead_of_connecting_without_it( + #[case] yaml: &str, + #[case] environment: &[(&str, &str)], + #[case] setting: &str, +) { + let error = coordination_resolution(yaml, environment).unwrap_err(); + + assert!( + matches!(&error, RedisError::Unenforceable { setting: named } if named == setting), + "{error:?}" + ); +} + +#[rstest] +#[case::kwarg(" host: redis\n ssl: true\n ssl_validate_ocsp: false")] +#[case::rediss_url_query(" url: rediss://cache:6380/0?ssl_validate_ocsp=false")] +fn an_ocsp_flag_set_to_false_asks_for_nothing(#[case] block: &str) { + let yaml = format!("general_settings:\n coordination_redis:\n{block}"); + + let resolution = coordination_resolution(&yaml, &[]).unwrap(); + + assert!(resolution.ignored.is_empty(), "{:?}", resolution.ignored); +} + fn static_auth(username: Option<&str>, password: Option<&str>) -> Auth { Auth::Static(StaticAuth { username: username.map(litellm_secret_value::SecretValue::new), @@ -975,6 +1015,11 @@ fn expected_view(fields: serde_json::Value) -> serde_json::Value { &[], Ok(expected_view(json!({"mode": "host", "host": "redis", "port": 6379, "db": 0, "tls": {"ca_certs": null, "ca_data": null, "ca_path": "/ca", "certfile": null, "keyfile": null, "verify_certificate": true, "check_hostname": false}}))), )] +#[case::ssl_validate_ocsp_is_unsupported( + json!({"host": "redis", "ssl": true, "ssl_validate_ocsp": true}), + &[], + Err("ssl_validate_ocsp asks for OCSP revocation checks, which rustls cannot perform, so Redis is not connected without them"), +)] #[case::ssl_password_is_unsupported( json!({"host": "redis", "ssl": true, "ssl_certfile": "/client.pem", "ssl_password": "key-secret"}), &[], diff --git a/tests/unit/test_redis.py b/tests/unit/test_redis.py index 9cba2a02e7d..c7fd750e205 100644 --- a/tests/unit/test_redis.py +++ b/tests/unit/test_redis.py @@ -2147,6 +2147,17 @@ def _expected_view(fields: dict[str, object]) -> dict[str, object]: }, id="ssl_password_is_unsupported", ), + pytest.param( + {"host": "redis", "ssl": True, "ssl_validate_ocsp": True}, + {}, + { + "rejected": { + "message": "Redis.__init__() got an unexpected keyword argument 'ssl_validate_ocsp'", + "type": "TypeError", + } + }, + id="ssl_validate_ocsp_is_unsupported", + ), pytest.param( { "db": 2,