mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-16 23:41:43 +00:00
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.
This commit is contained in:
parent
ca896ac073
commit
6f3fc5eba4
6 changed files with 75 additions and 16 deletions
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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]:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue