mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(oauth2-proxy): switch privileged-field denylist to identity-only allowlist
Greptile flagged that the denylist was incomplete: ``user_max_budget``, ``user_tpm_limit``, ``user_rpm_limit``, and ``user_spend`` were not on it. Inspection of the auth model showed dozens more privileged fields across the ``LiteLLM_VerificationTokenView`` hierarchy (team / org / end-user / region budget / spend / limit fields, plus ``allowed_model_region``, ``rpm_limit_per_model``, etc.) — a denylist of "privileged fields" is unmaintainable here. Inverted the model. ``ALLOWED_OAUTH2_PROXY_FIELDS`` is now an identity-only allowlist: ``user_id``, ``user_email``, ``team_id``, ``team_alias``, ``org_id``, ``models``. Any mapping to a non-identity field is rejected at request time. Default-secure: a future field added to ``UserAPIKeyAuth`` is automatically blocked from header-trust. Use case for OAuth2-proxy auth is identity assertion from a trusted upstream. Anything beyond that (privileges, budgets, rate limits) is policy and should be authenticated with a signature, not a header — operators who need this should switch to JWT auth. Tests: - ``test_refuses_to_map_non_identity_fields`` parametrized over 22 fields including all four ``user_*`` Greptile flagged, plus team/org/end-user budget/limit fields, plus a fabricated field name to confirm "anything not on the allowlist" is the rule. - ``test_allowlist_is_identity_only`` locks in the allowlist's intent so future additions of budget / role / permission entries are caught in review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e6867c143a
commit
b35287a062
2 changed files with 96 additions and 50 deletions
|
|
@ -5,36 +5,32 @@ from fastapi import Request
|
|||
from litellm._logging import verbose_proxy_logger
|
||||
from litellm.proxy._types import CommonProxyErrors, UserAPIKeyAuth
|
||||
|
||||
# Fields on ``UserAPIKeyAuth`` that grant privileges directly (``user_role``
|
||||
# is the canonical privesc — coerced from the string ``"proxy_admin"`` into
|
||||
# ``LitellmUserRoles.PROXY_ADMIN`` by Pydantic) or break trust assumptions
|
||||
# (``api_key`` / ``token`` short-circuit the validated-key contract;
|
||||
# ``permissions`` / ``allowed_routes`` directly grant route access; budget
|
||||
# and limit fields can be set to wild values to bypass enforcement;
|
||||
# ``metadata`` is too broad to safely admit from caller-controlled headers).
|
||||
# OAuth2-proxy header trust is for **identity assertion** from a trusted
|
||||
# upstream auth proxy (oauth2-proxy, Authelia, etc.). The allowlist below
|
||||
# is the only safe surface — anything else (``user_role``, ``api_key``,
|
||||
# ``permissions``, ``max_budget``, ``user_max_budget``,
|
||||
# ``team_tpm_limit``, ``end_user_max_budget``, ``allowed_model_region``,
|
||||
# and dozens of similar policy fields scattered across the
|
||||
# ``LiteLLM_VerificationTokenView`` hierarchy) is a privilege grant that
|
||||
# would let a caller forge their own enforcement parameters by sending
|
||||
# the matching header.
|
||||
#
|
||||
# Operators who legitimately need any of these to flow from a trusted
|
||||
# upstream proxy should switch to JWT authentication, which validates a
|
||||
# A denylist of "privileged fields" is unmaintainable in this codebase:
|
||||
# the auth model has ~50 budget/spend/limit/permission fields and gains
|
||||
# more with each release. An allowlist scoped to identity assertion is
|
||||
# default-secure — new fields are blocked automatically.
|
||||
#
|
||||
# Operators who need a trusted upstream to assert anything beyond
|
||||
# identity should switch to JWT authentication, which validates a
|
||||
# signature on the assertion rather than blindly trusting headers.
|
||||
PRIVILEGED_OAUTH2_PROXY_FIELDS: FrozenSet[str] = frozenset(
|
||||
ALLOWED_OAUTH2_PROXY_FIELDS: FrozenSet[str] = frozenset(
|
||||
{
|
||||
"user_role",
|
||||
"api_key",
|
||||
"token",
|
||||
"key_alias",
|
||||
"key_name",
|
||||
"permissions",
|
||||
"allowed_routes",
|
||||
"max_budget",
|
||||
"spend",
|
||||
"model_max_budget",
|
||||
"model_spend",
|
||||
"tpm_limit",
|
||||
"rpm_limit",
|
||||
"team_max_budget",
|
||||
"team_spend",
|
||||
"blocked",
|
||||
"metadata",
|
||||
"user_id",
|
||||
"user_email",
|
||||
"team_id",
|
||||
"team_alias",
|
||||
"org_id",
|
||||
"models",
|
||||
}
|
||||
)
|
||||
|
||||
|
|
@ -54,15 +50,15 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth:
|
|||
previously did not, which let any open-source deployment turn
|
||||
the feature on without realising it requires a hardened
|
||||
deployment topology.
|
||||
2. **Privileged-field denylist.** ``oauth2_config_mappings`` maps
|
||||
header names to ``UserAPIKeyAuth`` fields. Without a denylist,
|
||||
an admin who maps the wrong header to ``user_role`` (or who
|
||||
hasn't fully locked down their reverse proxy) lets any caller
|
||||
set the ``user_role`` header to ``"proxy_admin"`` and gain full
|
||||
admin privileges — Pydantic coerces the string into the enum.
|
||||
Mapping any privileged field is rejected at startup-style auth
|
||||
time so the misconfiguration surfaces loudly rather than as a
|
||||
silent privesc.
|
||||
2. **Identity-only allowlist.** ``oauth2_config_mappings`` maps
|
||||
header names to ``UserAPIKeyAuth`` fields. Without an allowlist,
|
||||
an admin who maps the wrong header to ``user_role`` lets any
|
||||
caller send ``X-User-Role: proxy_admin`` and gain full admin
|
||||
privileges (Pydantic coerces the string into the enum). Only
|
||||
fields in ``ALLOWED_OAUTH2_PROXY_FIELDS`` (identity assertion
|
||||
only — see the constant's comment) may be mapped; any other
|
||||
mapping is rejected at request time so the misconfiguration
|
||||
surfaces loudly rather than as a silent privesc.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import general_settings, premium_user
|
||||
|
||||
|
|
@ -81,17 +77,18 @@ async def handle_oauth2_proxy_request(request: Request) -> UserAPIKeyAuth:
|
|||
if not oauth2_config_mappings:
|
||||
raise ValueError("Oauth2 config mappings not found in general_settings")
|
||||
|
||||
privileged_mapped = sorted(
|
||||
set(oauth2_config_mappings.keys()) & PRIVILEGED_OAUTH2_PROXY_FIELDS
|
||||
disallowed = sorted(
|
||||
set(oauth2_config_mappings.keys()) - ALLOWED_OAUTH2_PROXY_FIELDS
|
||||
)
|
||||
if privileged_mapped:
|
||||
if disallowed:
|
||||
raise ValueError(
|
||||
"Oauth2 proxy auth refuses to map privileged UserAPIKeyAuth "
|
||||
f"fields from request headers: {privileged_mapped}. These "
|
||||
"fields would grant privileges (e.g. proxy_admin), bypass "
|
||||
"budget enforcement, or short-circuit key validation if a "
|
||||
"caller can spoof the corresponding header. If you need a "
|
||||
"trusted upstream to assert one of these, use JWT auth "
|
||||
"Oauth2 proxy auth refuses to map non-identity UserAPIKeyAuth "
|
||||
f"fields from request headers: {disallowed}. Only identity "
|
||||
f"fields are accepted ({sorted(ALLOWED_OAUTH2_PROXY_FIELDS)}); "
|
||||
"anything else (privileges, budgets, rate limits, metadata) "
|
||||
"would let a caller forge enforcement parameters by spoofing "
|
||||
"the matching header. If you need a trusted upstream to "
|
||||
"assert anything beyond identity, use JWT auth "
|
||||
"(signature-validated) instead of header-trust."
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ sys.path.insert(0, os.path.abspath("../../../.."))
|
|||
|
||||
from litellm.proxy._types import LitellmUserRoles
|
||||
from litellm.proxy.auth.oauth2_proxy_hook import (
|
||||
PRIVILEGED_OAUTH2_PROXY_FIELDS,
|
||||
ALLOWED_OAUTH2_PROXY_FIELDS,
|
||||
handle_oauth2_proxy_request,
|
||||
)
|
||||
|
||||
|
|
@ -90,13 +90,46 @@ async def test_rejects_when_not_premium(configure_proxy):
|
|||
|
||||
@pytest.mark.parametrize(
|
||||
"privileged_field",
|
||||
sorted(PRIVILEGED_OAUTH2_PROXY_FIELDS),
|
||||
[
|
||||
# The GHSA-5c3m-qffq-4r9m primary privesc field.
|
||||
"user_role",
|
||||
# Key-level enforcement bypass shapes.
|
||||
"api_key",
|
||||
"token",
|
||||
"permissions",
|
||||
"allowed_routes",
|
||||
"max_budget",
|
||||
"spend",
|
||||
"tpm_limit",
|
||||
"rpm_limit",
|
||||
"model_max_budget",
|
||||
"metadata",
|
||||
# User-level enforcement bypass — flagged by Greptile as a denylist gap.
|
||||
"user_max_budget",
|
||||
"user_tpm_limit",
|
||||
"user_rpm_limit",
|
||||
"user_spend",
|
||||
# Team / org / end-user / region — same class, all denied by the
|
||||
# identity-only allowlist.
|
||||
"team_max_budget",
|
||||
"team_spend",
|
||||
"team_member_tpm_limit",
|
||||
"organization_max_budget",
|
||||
"organization_tpm_limit",
|
||||
"end_user_max_budget",
|
||||
"allowed_model_region",
|
||||
# Anything not on ALLOWED_OAUTH2_PROXY_FIELDS is blocked, even
|
||||
# fabricated field names admins might try.
|
||||
"definitely_not_a_real_field",
|
||||
],
|
||||
)
|
||||
@pytest.mark.asyncio
|
||||
async def test_refuses_to_map_privileged_fields(configure_proxy, privileged_field):
|
||||
async def test_refuses_to_map_non_identity_fields(configure_proxy, privileged_field):
|
||||
# GHSA-5c3m-qffq-4r9m attack shape: admin maps a privileged field
|
||||
# to a header and a caller forges the value. The hook must reject
|
||||
# the misconfiguration outright at request time.
|
||||
# to a header and a caller forges the value. The allowlist rejects
|
||||
# any non-identity mapping at request time, regardless of whether
|
||||
# the field ever appeared on a denylist — which is the whole reason
|
||||
# we use an allowlist instead.
|
||||
configure_proxy(mappings={privileged_field: f"x-{privileged_field}"})
|
||||
request = _request_with_headers({f"x-{privileged_field}": "proxy_admin"})
|
||||
|
||||
|
|
@ -105,6 +138,22 @@ async def test_refuses_to_map_privileged_fields(configure_proxy, privileged_fiel
|
|||
assert privileged_field in str(exc.value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("identity_field", sorted(ALLOWED_OAUTH2_PROXY_FIELDS))
|
||||
def test_allowlist_is_identity_only(identity_field):
|
||||
# Lock in the allowlist's intent: only identity-assertion fields are
|
||||
# safe to populate from a header. If anyone proposes adding budget /
|
||||
# spend / role / permission to ``ALLOWED_OAUTH2_PROXY_FIELDS``, this
|
||||
# assertion forces them to update the test deliberately.
|
||||
assert identity_field in {
|
||||
"user_id",
|
||||
"user_email",
|
||||
"team_id",
|
||||
"team_alias",
|
||||
"org_id",
|
||||
"models",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_role_header_forgery_attack_is_blocked(configure_proxy):
|
||||
# End-to-end form of the privesc: with ``user_role`` mapped, the
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue