fix(redis-resolve): refuse ssl_include_verify_flags and ssl_exclude_verify_flags

Both ask for X.509 checks such as CRL revocation that rustls cannot perform,
and redis-py 5.3.1's async client already refuses them with a TypeError, so a
set value now fails with Error::Unenforceable like the OCSP settings instead of
connecting without the check. An empty list asks for nothing.

A Reject row now says which values ask for its check: true for the OCSP flags,
any present value for the verify flag lists. tests/catalog.rs fails if the
locked redis-py's async, cluster or connection signature accepts a Reject row,
so a redis-py bump forces the decision again
This commit is contained in:
Yujong Lee 2026-11-05 12:47:57 +09:00
parent f9e19055d2
commit d664e21c57
7 changed files with 107 additions and 32 deletions

View file

@ -12,9 +12,11 @@
- Precedence matches Python. The target is cluster, then URL, then Sentinel (`sentinel_nodes` plus `service_name`), then host. Auth is a `credential_provider` handle, then GCP, then Azure, then AWS, and the first one wins with a warning
- 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 `Error::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 `Error::Invalid`, so a TLS setting is never guessed
- 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 on its row, because a Python deployment that worked must keep working when its stores move to Rust. Only a value that is malformed (`Error::Invalid`) or a target that is missing fails. Promoting a `Warn` row to `Read` is then a pure improvement with no behavior cliff
- 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 on its row, because a Python deployment that worked must keep working when its stores move to Rust. Only a value that is malformed (`Error::Invalid`), a target that is missing or a `Reject` row that asks for its check fails. Promoting a `Warn` row to `Read` is then a pure improvement with no behavior cliff, while moving one to `Reject` can fail a config that connects today
- A `Warn` row with a `met` check is dropped silently when its value asks for nothing more than Rust does, so `ssl_min_version` of TLS 1.2 or lower never warns
- 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 `Error::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
- A `Reject` row is a security check rustls cannot perform that Python's locked async client already refuses with a `TypeError`, so failing closed breaks no proxy that runs today. `tests/catalog.rs` fails if the locked redis-py's async, cluster or connection signature accepts a `Reject` row, so a redis-py bump forces the decision again. A row's `Asked` says which values ask for the check, from the kwargs, `REDIS_*` or a url query, and those are `Error::Unenforceable`
- `ssl_validate_ocsp` and `ssl_validate_ocsp_stapled` ask when true, because rustls has no client-side OCSP. False asks for nothing and is dropped without a warning
- `ssl_include_verify_flags` and `ssl_exclude_verify_flags` ask when set, because rustls takes no X.509 verify flags such as CRL checking. An empty list asks for nothing
- `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, including one with no row, is ignored naming `url ?<name>`, because redis-py turns each one into a client kwarg. Which ones a url carries is the `FromUrl` column

View file

@ -12,8 +12,18 @@ pub enum Handling {
consequence: &'static str,
met: Option<fn(&Value) -> bool>,
},
/// A security check the Rust client cannot perform, so setting it to true fails instead of connecting without it.
Reject { check: &'static str },
/// A security check the Rust client cannot perform and Python's async client already refuses, so asking for it
/// fails instead of connecting without it.
Reject { check: &'static str, asked: Asked },
}
/// Which values of a `Reject` row ask for its check.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum Asked {
/// A flag read as true. False asks for nothing.
WhenTrue,
/// Any present value, like a list of flags. An empty one asks for nothing.
WhenSet,
}
/// What the value is, which decides where an unread setting lands in a `Resolution`.
@ -127,8 +137,8 @@ const fn warn_unless(
)
}
const fn reject(kwarg: &'static str, check: &'static str) -> Setting {
Setting::new(kwarg, Handling::Reject { check })
const fn reject(kwarg: &'static str, check: &'static str, asked: Asked) -> Setting {
Setting::new(kwarg, Handling::Reject { check, asked })
}
const NO_EQUIVALENT: &str = "the Rust client has no equivalent";
@ -139,7 +149,7 @@ const MAINTENANCE: &str = "the Rust client does not follow server maintenance no
const RUSTLS: &str = "rustls negotiates TLS 1.2+ with its own ECDHE AEAD suites";
const REVOCATION: &str = "OCSP revocation checks";
const OCSP: &str = "it only configures ssl_validate_ocsp, which the Rust client refuses when set";
const VERIFY_FLAGS: &str = "rustls verifies certificates with its own fixed rules, so no extra X.509 flag such as CRL checking applies";
const VERIFY_FLAGS: &str = "X.509 verify flags such as CRL checking";
/// Every Redis kwarg redis-py or `litellm/_redis.py` accepts, one row each. `redis-py-kwargs.json` records which
/// redis-py versions and `REDIS_*` names accept them, and `tests/catalog.rs` keeps the two in step.
@ -243,15 +253,15 @@ pub const SETTINGS: &[Setting] = &[
read("ssl_certfile").env().in_url(FromUrl::Rediss),
read("ssl_check_hostname").env().in_url(FromUrl::Rediss),
warn("ssl_ciphers", RUSTLS).env(),
warn("ssl_exclude_verify_flags", VERIFY_FLAGS),
warn("ssl_include_verify_flags", VERIFY_FLAGS),
reject("ssl_exclude_verify_flags", VERIFY_FLAGS, Asked::WhenSet),
reject("ssl_include_verify_flags", VERIFY_FLAGS, Asked::WhenSet),
read("ssl_keyfile").env().in_url(FromUrl::Rediss),
warn_unless("ssl_min_version", RUSTLS, is_met_by_rustls).env(),
warn("ssl_ocsp_context", OCSP).env().object(),
warn("ssl_ocsp_expected_cert", OCSP).env(),
read("ssl_password").env().in_url(FromUrl::Rediss),
reject("ssl_validate_ocsp", REVOCATION).env(),
reject("ssl_validate_ocsp_stapled", REVOCATION).env(),
reject("ssl_validate_ocsp", REVOCATION, Asked::WhenTrue).env(),
reject("ssl_validate_ocsp_stapled", REVOCATION, Asked::WhenTrue).env(),
read("startup_nodes").or_secrets(&["REDIS_CLUSTER_NODES"]),
read("unix_socket_path").env(),
read("url").env(),
@ -278,10 +288,10 @@ pub fn setting(kwarg: &str) -> Option<&'static Setting> {
SETTINGS.iter().find(|setting| setting.kwarg == kwarg)
}
/// The check a `Reject` row asks for when it is set to true.
pub(crate) fn refused_check(kwarg: &str) -> Option<&'static str> {
/// The check a `Reject` row stands for and which values ask for it.
pub(crate) fn refusal(kwarg: &str) -> Option<(&'static str, Asked)> {
match setting(kwarg)?.handling {
Handling::Reject { check } => Some(check),
Handling::Reject { check, asked } => Some((check, asked)),
Handling::Read | Handling::Warn { .. } => None,
}
}

View file

@ -11,7 +11,7 @@ mod options;
mod tls;
pub use error::{Error, Expected};
pub use keys::{FromUrl, Handling, SETTINGS, Setting, Shape, setting};
pub use keys::{Asked, FromUrl, Handling, SETTINGS, Setting, Shape, setting};
pub use options::is_present;
use options::Options;

View file

@ -7,7 +7,7 @@ use serde_json::Value;
use crate::{
Entry, Error, Expected, Kwargs,
keys::{self, Shape},
keys::{self, Asked, Shape},
};
/// Every setting the resolver reads. A setting is `None` when it is unset, null or empty.
@ -168,19 +168,25 @@ impl Options {
}
}
/// A `Reject` row that is set to true asks for a check Rust cannot perform. One set to false asks for nothing.
/// A `Reject` row whose value asks for its check fails, because Rust cannot perform it.
pub(super) fn refuse_unenforceable<'a>(
entries: impl IntoIterator<Item = (&'a String, &'a Entry)>,
) -> Result<(), Error> {
entries
.into_iter()
.filter_map(|(key, entry)| Some((keys::refused_check(key)?, entry)))
.try_for_each(|(check, entry)| match flag(Some(entry))? {
true => Err(Error::Unenforceable {
setting: entry.source.clone(),
check,
}),
false => Ok(()),
.filter_map(|(key, entry)| Some((keys::refusal(key)?, entry)))
.try_for_each(|((check, asked), entry)| {
let asks = match asked {
Asked::WhenTrue => flag(Some(entry))?,
Asked::WhenSet => is_present(&entry.value),
};
match asks {
true => Err(Error::Unenforceable {
setting: entry.source.clone(),
check,
}),
false => Ok(()),
}
})
}

View file

@ -1,18 +1,37 @@
use std::collections::{BTreeMap, BTreeSet};
use litellm_redis_resolve::{Error, Handling, SETTINGS, Shape, interpret, kwargs};
use litellm_redis_resolve::{Asked, Error, Handling, SETTINGS, Setting, Shape, interpret, kwargs};
use rstest::rstest;
use serde::Deserialize;
use serde_json::json;
#[derive(Deserialize)]
struct Catalog {
meta: Meta,
args: BTreeMap<String, Arg>,
}
#[derive(Deserialize)]
struct Meta {
redis_py_locked: String,
}
#[derive(Deserialize)]
struct Arg {
environment: bool,
redis_py: BTreeMap<String, Vec<String>>,
}
// the signatures of the clients the proxy builds, where an unknown kwarg raises TypeError
const PROXY_SIGNATURES: &[&str] = &["async", "cluster", "connection"];
fn rejected() -> impl Iterator<Item = (&'static Setting, Asked)> {
SETTINGS
.iter()
.filter_map(|setting| match setting.handling {
Handling::Reject { asked, .. } => Some((setting, asked)),
Handling::Read | Handling::Warn { .. } => None,
})
}
fn catalog() -> Catalog {
@ -78,17 +97,38 @@ fn every_warned_setting_is_reported_and_still_reaches_the_host() {
}
#[rstest]
fn every_rejected_setting_fails_when_asked_for() {
let rejected = SETTINGS
.iter()
.filter(|setting| matches!(setting.handling, Handling::Reject { .. }));
for setting in rejected {
let error = interpret(&host_with(setting.kwarg, json!(true)), None).unwrap_err();
fn every_rejected_setting_fails_when_asked_for_and_not_otherwise() {
for (setting, asked) in rejected() {
let (asking, not_asking) = match asked {
Asked::WhenTrue => (json!(true), json!(false)),
Asked::WhenSet => (json!(["VERIFY_CRL_CHECK_LEAF"]), json!([])),
};
let error = interpret(&host_with(setting.kwarg, asking), None).unwrap_err();
assert!(
matches!(&error, Error::Unenforceable { setting: source, .. } if source == setting.kwarg),
"{error:?}"
);
assert!(interpret(&host_with(setting.kwarg, not_asking), None).is_ok());
}
}
#[rstest]
fn python_already_refuses_every_rejected_setting() {
let catalog = catalog();
let accepted: Vec<&str> = rejected()
.map(|(setting, _)| setting.kwarg)
.filter(|kwarg| {
catalog.args[*kwarg]
.redis_py
.get(&catalog.meta.redis_py_locked)
.is_some_and(|signatures| {
signatures
.iter()
.any(|signature| PROXY_SIGNATURES.contains(&signature.as_str()))
})
})
.collect();
assert!(accepted.is_empty(), "{accepted:?}");
}

View file

@ -277,6 +277,11 @@ fn expected_view(fields: serde_json::Value) -> serde_json::Value {
&[],
Err("ssl_validate_ocsp asks for OCSP revocation checks, which rustls cannot perform, so Redis is not connected without them"),
)]
#[case::ssl_include_verify_flags_is_unsupported(
json!({"host": "redis", "ssl": true, "ssl_include_verify_flags": ["VERIFY_CRL_CHECK_LEAF"]}),
&[],
Err("ssl_include_verify_flags asks for X.509 verify flags such as CRL checking, 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"}),
&[],

View file

@ -2170,6 +2170,18 @@ def _expected_view(fields: dict[str, object]) -> dict[str, object]:
},
id="ssl_validate_ocsp_is_unsupported",
),
pytest.param(
{"host": "redis", "ssl": True, "ssl_include_verify_flags": ["VERIFY_CRL_CHECK_LEAF"]},
{},
{"host": "redis", "ssl": True, "ssl_include_verify_flags": ["VERIFY_CRL_CHECK_LEAF"]},
{
"rejected": {
"message": "Redis.__init__() got an unexpected keyword argument 'ssl_include_verify_flags'",
"type": "TypeError",
}
},
id="ssl_include_verify_flags_is_unsupported",
),
pytest.param(
{
"db": 2,