From 6f3fc5eba4b20a37f0051f6c150fd999138ea794 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Wed, 10 Jun 2026 18:52:26 -0700 Subject: [PATCH] fix(auth_v2): close mTLS spoofing, introspection audience, SAML replay, and deactivated-user gaps Address the SSO and credential-flow security review findings: - mTLS (HIGH): the forwarded subject-DN header was trusted unconditionally, so any caller could send it and mint a service-account principal. Trust it only when the immediate peer is inside trusted_proxy_cidrs (same model as XFF) and fail closed otherwise; the ASGI-TLS-extension path already fails closed when no verified cert is present. - OAuth2 introspection (HIGH): RFC 7662 responses were accepted regardless of audience. Enforce the response aud against OAuth2IntrospectionConfig.audience and reject active tokens whose audience does not match. - SAML (HIGH): default allow_unsolicited to False so IdP-initiated/login-CSRF responses are rejected, add a single-use assertion-id replay cache, and bind the post-login redirect to the RelayState stored against the matched InResponseTo request rather than trusting the echoed form field. - Deactivated users (M1): the resolver now rejects a credential that resolves to a SCIM user with active=False, so deactivation actually blocks authentication. - Stop carrying underscore-prefixed carrier keys (raw api key, basic password) into Principal.claims, which is documented for audit logging. --- litellm/proxy/auth_v2/authenticators.py | 15 ++++++++--- litellm/proxy/auth_v2/config.py | 3 ++- litellm/proxy/auth_v2/errors.py | 4 +++ litellm/proxy/auth_v2/network.py | 4 +++ litellm/proxy/auth_v2/resolver.py | 33 ++++++++++++++++++++++--- litellm/proxy/auth_v2/saml.py | 32 ++++++++++++++++++------ 6 files changed, 75 insertions(+), 16 deletions(-) diff --git a/litellm/proxy/auth_v2/authenticators.py b/litellm/proxy/auth_v2/authenticators.py index db8273bea83..35425cbd067 100644 --- a/litellm/proxy/auth_v2/authenticators.py +++ b/litellm/proxy/auth_v2/authenticators.py @@ -23,6 +23,7 @@ from .config import ( MutualTlsConfig, OAuth2IntrospectionConfig, OidcProviderConfig, + TrustedProxyConfig, ) from .models import ( AuthMethod, @@ -31,6 +32,7 @@ from .models import ( CredentialRef, SecuritySchemeType, ) +from .network import ip_in_trusted_proxies AT_JWT_TYPES = {"at+jwt", "application/at+jwt"} @@ -323,12 +325,15 @@ class OAuth2Authenticator: body = response.json() if not body.get("active"): raise errors.invalid_token("token inactive") + token_audience = _normalize_audience(body.get("aud")) + if config.audience and not set(token_audience) & set(config.audience): + raise errors.invalid_token("audience mismatch") return Credential( scheme=self.scheme, method=AuthMethod.OAUTH2_INTROSPECTION, subject=str(body.get(config.subject_field, "")), issuer=body.get("iss"), - audience=_normalize_audience(body.get("aud")), + audience=token_audience, scopes=_split_scope(body.get("scope")), claims=body, ) @@ -360,8 +365,9 @@ class OidcAuthenticator: class MutualTlsAuthenticator: scheme = SecuritySchemeType.MUTUAL_TLS - def __init__(self, config: MutualTlsConfig) -> None: + def __init__(self, config: MutualTlsConfig, network: TrustedProxyConfig) -> None: self._config = config + self._network = network async def authenticate(self, request: Request) -> Optional[Credential]: cert = self._read_client_cert(request) @@ -376,6 +382,9 @@ class MutualTlsAuthenticator: def _read_client_cert(self, request: Request) -> Optional[ClientCertificate]: if self._config.forwarded_subject_header: + peer = request.client.host if request.client else None + if not ip_in_trusted_proxies(peer, self._network): + return None dn = request.headers.get(self._config.forwarded_subject_header) return ClientCertificate(subject_dn=dn) if dn else None tls = request.scope.get("extensions", {}).get("tls", {}) @@ -402,6 +411,6 @@ def build_authenticators( ) if config.mutual_tls.enabled: by_scheme[SecuritySchemeType.MUTUAL_TLS] = MutualTlsAuthenticator( - config.mutual_tls + config.mutual_tls, config.network ) return [by_scheme[scheme] for scheme in config.scheme_order if scheme in by_scheme] diff --git a/litellm/proxy/auth_v2/config.py b/litellm/proxy/auth_v2/config.py index 738f1a03fcb..9026a23af2d 100644 --- a/litellm/proxy/auth_v2/config.py +++ b/litellm/proxy/auth_v2/config.py @@ -47,6 +47,7 @@ class OAuth2IntrospectionConfig(BaseModel): client_id: str client_secret: SecretStr subject_field: str = "sub" + audience: List[str] = Field(default_factory=list) class MutualTlsConfig(BaseModel): @@ -66,7 +67,7 @@ class SamlConfig(BaseModel): idp_metadata: str = "" sp_key_file: Optional[str] = None sp_cert_file: Optional[str] = None - allow_unsolicited: bool = True + allow_unsolicited: bool = False session_cookie: str = "saml_session" cookie_secure: bool = True session_ttl_seconds: int = 3600 diff --git a/litellm/proxy/auth_v2/errors.py b/litellm/proxy/auth_v2/errors.py index 82953124496..a7c5950790f 100644 --- a/litellm/proxy/auth_v2/errors.py +++ b/litellm/proxy/auth_v2/errors.py @@ -48,3 +48,7 @@ def forbidden_role() -> AuthError: def forbidden_permission() -> AuthError: return AuthError(403, "Forbidden") + + +def account_disabled() -> AuthError: + return AuthError(403, "Account disabled") diff --git a/litellm/proxy/auth_v2/network.py b/litellm/proxy/auth_v2/network.py index c8aee43fffb..4852cf3ddbe 100644 --- a/litellm/proxy/auth_v2/network.py +++ b/litellm/proxy/auth_v2/network.py @@ -30,6 +30,10 @@ def _ip_in_cidrs(ip: Optional[str], cidrs: List[str]) -> bool: return False +def ip_in_trusted_proxies(ip: Optional[str], config: TrustedProxyConfig) -> bool: + return _ip_in_cidrs(ip, config.trusted_proxy_cidrs) + + def resolve_client_ip( request: Request, config: TrustedProxyConfig ) -> Tuple[Optional[str], bool]: diff --git a/litellm/proxy/auth_v2/resolver.py b/litellm/proxy/auth_v2/resolver.py index 55a5883689f..0c33763843f 100644 --- a/litellm/proxy/auth_v2/resolver.py +++ b/litellm/proxy/auth_v2/resolver.py @@ -48,6 +48,10 @@ def _roles_from_claims(claims: Dict[str, Any]) -> List[Role]: return [Role(value) for value in raw if value in valid] +def _public_claims(claims: Dict[str, Any]) -> Dict[str, Any]: + return {key: value for key, value in claims.items() if not key.startswith("_")} + + def _teams_from_claims(claims: Dict[str, Any]) -> List[TeamIdentity]: groups = claims.get("groups", []) if not isinstance(groups, list): @@ -70,8 +74,29 @@ class InMemoryIdentityStore(IdentityResolver, ProvisioningStore): async def resolve(self, credential: Credential) -> Principal: if credential.method == AuthMethod.API_KEY: - return self._resolve_api_key(credential) - return self._resolve_subject(credential) + principal = self._resolve_api_key(credential) + else: + principal = self._resolve_subject(credential) + self._reject_if_deactivated(principal) + return principal + + def _reject_if_deactivated(self, principal: Principal) -> None: + user = self._lookup_scim_user(principal) + if user is not None and user.active is False: + raise errors.account_disabled() + + def _lookup_scim_user(self, principal: Principal) -> Optional[ScimUser]: + if principal.user is None: + return None + by_id = self._users.get(principal.user.id) + if by_id is not None: + return by_id + external = principal.user.external_id + if external: + for user in self._users.values(): + if user.external_id == external: + return user + return None def _resolve_api_key(self, credential: Credential) -> Principal: raw = credential.claims.get("_raw_api_key") @@ -99,7 +124,7 @@ class InMemoryIdentityStore(IdentityResolver, ProvisioningStore): scopes=list(credential.scopes), auth_method=credential.method, credential_ref=credential.credential_ref, - claims=dict(claims), + claims=_public_claims(claims), ) return Principal( principal_type=PrincipalType.HUMAN, @@ -118,7 +143,7 @@ class InMemoryIdentityStore(IdentityResolver, ProvisioningStore): scopes=list(credential.scopes), auth_method=credential.method, credential_ref=credential.credential_ref, - claims=dict(claims), + claims=_public_claims(claims), ) async def upsert_user(self, user: ScimUser) -> ScimUser: diff --git a/litellm/proxy/auth_v2/saml.py b/litellm/proxy/auth_v2/saml.py index b46ff2f2bde..d57811c8f57 100644 --- a/litellm/proxy/auth_v2/saml.py +++ b/litellm/proxy/auth_v2/saml.py @@ -141,6 +141,7 @@ def build_sp_client(config: SamlConfig) -> Saml2Client: class SamlSessionStore: def __init__(self, ttl_seconds: int = 3600, max_size: int = 10000) -> None: self._sessions: Dict[str, Tuple[float, Dict[str, Any]]] = {} + self._seen_assertions: Dict[str, float] = {} self.outstanding: Dict[str, str] = {} self._ttl = ttl_seconds self._max_size = max_size @@ -148,6 +149,16 @@ class SamlSessionStore: def remember_request(self, request_id: str, relay_state: str = "/") -> None: self.outstanding[request_id] = relay_state + def consume_assertion(self, assertion_id: str) -> bool: + now = time.time() + self._seen_assertions = { + aid: exp for aid, exp in self._seen_assertions.items() if exp >= now + } + if assertion_id in self._seen_assertions: + return False + self._seen_assertions[assertion_id] = now + self._ttl + return True + def create_session(self, identity: Dict[str, Any]) -> str: now = time.time() self._evict(now) @@ -244,6 +255,18 @@ def build_saml_router(config: SamlConfig, session_store: SamlSessionStore) -> AP if authn_response is None: raise HTTPException(status_code=401, detail="invalid SAML response") + in_response_to = getattr(authn_response, "in_response_to", None) + bound_relay = ( + session_store.outstanding.pop(in_response_to, None) + if in_response_to + else None + ) + + assertion = getattr(authn_response, "assertion", None) + assertion_id = getattr(assertion, "id", None) + if assertion_id and not session_store.consume_assertion(assertion_id): + raise HTTPException(status_code=401, detail="SAML assertion replay") + name_id = authn_response.get_subject().text ava = authn_response.get_identity() or {} mapped = _map_attributes(ava, config.attribute_map) @@ -252,9 +275,6 @@ def build_saml_router(config: SamlConfig, session_store: SamlSessionStore) -> AP store: ProvisioningStore = request.app.state.auth_v2.resolver await store.upsert_user(user) - in_response_to = getattr(authn_response, "in_response_to", None) - if in_response_to: - session_store.outstanding.pop(in_response_to, None) session_id = session_store.create_session( { "name_id": name_id, @@ -262,11 +282,7 @@ def build_saml_router(config: SamlConfig, session_store: SamlSessionStore) -> AP "claims": _claims_from_mapped(mapped), } ) - relay_state = form.get("RelayState") - target = _safe_relay_state( - relay_state if isinstance(relay_state, str) else None, - config.default_redirect_path, - ) + target = _safe_relay_state(bound_relay, config.default_redirect_path) response = RedirectResponse(target, status_code=303) response.set_cookie( config.session_cookie,