mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-12 08:42:18 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
bd4a12649c
commit
3de1f0caec
7 changed files with 90 additions and 10 deletions
|
|
@ -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 ?<name>`. 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
|
||||
|
|
|
|||
|
|
@ -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")]
|
||||
|
|
|
|||
|
|
@ -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_<KWARG>, 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"
|
||||
|
|
|
|||
|
|
@ -88,6 +88,7 @@ pub(super) struct Unmodeled {
|
|||
|
||||
impl Unmodeled {
|
||||
fn read(kwargs: &Kwargs) -> Result<Self, RedisError> {
|
||||
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<Item = (&'a String, &'a Entry)>,
|
||||
) -> 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<Handling> {
|
||||
keys::setting(key).map(|setting| setting.handling)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<UrlQuery, RedisError> {
|
|||
)
|
||||
})
|
||||
.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<UrlQuery, RedisError> {
|
|||
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(),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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"}),
|
||||
&[],
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue