Commit 6db55e0
authored
feat(mcp): add mcp_xff_num_trusted_hops to harden X-Forwarded-For client IP resolution (BerriAI#31257)
* feat(mcp): add mcp_xff_num_trusted_hops to harden XFF client IP resolution
MCP per-server IP access control reads the client IP from X-Forwarded-For
and trusts the leftmost entry. Behind an append-style proxy or load
balancer (AWS ALB, nginx with $proxy_add_x_forwarded_for, HAProxy, Envoy,
Cloudflare), a client can prepend an arbitrary value to the header, so the
leftmost entry is attacker-controllable even when the direct peer is a
trusted proxy. An attacker can therefore spoof an internal IP and reach
servers marked available_on_public_internet=false.
This adds an optional mcp_xff_num_trusted_hops general setting modelled on
Envoy's xff_num_trusted_hops. When set to N, the client IP is read N entries
from the right of the chain (where N is the number of trusted appending
proxies in front of the gateway) instead of the leftmost value, so any
entries a client prepends are ignored. It composes with mcp_trusted_proxy_ranges,
which still validates the direct peer, and only takes effect once that check
passes; without a validated direct peer the gateway keeps failing closed, so
hop counting cannot be abused by a direct-to-pod attacker. The chain must
contain at least N valid entries or resolution fails closed.
Default is unset, preserving existing behaviour.
* chore(ui): regenerate dashboard schema for mcp_xff_num_trusted_hops
* fix(mcp): warn when mcp_xff_num_trusted_hops is below the minimum
A 0 or negative value is silently treated as disabled, which could leave
an operator believing they enabled append-style X-Forwarded-For hardening
while client IP resolution stays on the spoofable leftmost value. Emit a
warning, consistent with how the module already surfaces invalid CIDR
config, so the misconfiguration is visible in logs.
* fix(mcp): reject mcp_xff_num_trusted_hops < 1 at config-parse time
Add a ge=1 bound to the ConfigGeneralSettings field so the
update_config_general_settings path rejects 0 and negative values with a
clear validation error instead of accepting them, and self-documents the
valid range. The runtime warning stays as defense-in-depth for raw-dict
config that bypasses model validation.
* style(mcp): black-format ip_address_utils.py
* fix(mcp): fail closed when mcp_xff_num_trusted_hops is set but invalid
A present-but-invalid mcp_xff_num_trusted_hops (non-integer, or below 1)
previously made _resolve_num_trusted_hops return None, which the caller
treated identically to "unset" and silently fell back to the legacy
leftmost X-Forwarded-For value. An operator who set the value to harden
client IP resolution but typo'd it would get weaker security than before,
with no fail-closed signal.
Model the setting as a tagged union (_HopCountUnset, _HopCountInvalid,
_HopCount) so the three states are distinct: unset keeps the legacy path,
a valid count drives hop-counting, and an invalid value fails closed
(returns "") instead of reverting to the spoofable leftmost address. The
caller matches on the union exhaustively.
Add a parametrized regression test asserting get_mcp_client_ip returns ""
for 0, -1, "abc", and 1.5 even with a spoofed internal leftmost entry,
and update the resolver unit tests for the new return type.1 parent 0a8a87a commit 6db55e0
5 files changed
Lines changed: 338 additions & 1 deletion
File tree
- litellm/proxy
- auth
- tests/test_litellm/proxy/auth
- ui/litellm-dashboard/src/lib/http
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2361 | 2361 | | |
2362 | 2362 | | |
2363 | 2363 | | |
| 2364 | + | |
| 2365 | + | |
| 2366 | + | |
| 2367 | + | |
| 2368 | + | |
2364 | 2369 | | |
2365 | 2370 | | |
2366 | 2371 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| 13 | + | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
| |||
25 | 27 | | |
26 | 28 | | |
27 | 29 | | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
28 | 50 | | |
29 | 51 | | |
30 | 52 | | |
| |||
174 | 196 | | |
175 | 197 | | |
176 | 198 | | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
177 | 253 | | |
178 | 254 | | |
179 | 255 | | |
| |||
186 | 262 | | |
187 | 263 | | |
188 | 264 | | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
189 | 271 | | |
190 | 272 | | |
191 | 273 | | |
| |||
245 | 327 | | |
246 | 328 | | |
247 | 329 | | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
248 | 350 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15385 | 15385 | | |
15386 | 15386 | | |
15387 | 15387 | | |
| 15388 | + | |
15388 | 15389 | | |
15389 | 15390 | | |
15390 | 15391 | | |
| |||
0 commit comments