harden(auth): SA3 review follow-ups — IPv6-mapped match + no-proxy-headers guardrail

Post-review hardening (both fail-safe, not bugs):
- _client_ip normalizes IPs via ipaddress and unwraps IPv4-mapped IPv6, so a plain
  TRUSTED_PROXY_IPS=10.10.0.1 also matches a peer surfaced as ::ffff:10.10.0.1 (a
  Docker/IPv6 self-host footgun that would otherwise silently collapse everyone into
  one rate-limit bucket).
- entrypoint.sh + _client_ip docstring: explicit warning never to add uvicorn
  --proxy-headers, which would rewrite request.client from the forgeable XFF and defeat
  the trust check.
This commit is contained in:
npeter83 2026-07-12 03:49:08 +02:00
parent 7297dbd2a8
commit 776fb38b9b
2 changed files with 23 additions and 2 deletions

View file

@ -1,3 +1,4 @@
import ipaddress
import logging import logging
import secrets import secrets
from datetime import datetime, timedelta, timezone from datetime import datetime, timedelta, timezone
@ -172,15 +173,30 @@ def get_or_create_demo_user(db: Session) -> User:
return user return user
def _norm_ip(s: str) -> ipaddress.IPv4Address | ipaddress.IPv6Address | None:
"""Parse an IP for comparison, unwrapping IPv4-mapped IPv6 (`::ffff:10.0.0.1` → `10.0.0.1`) so a
plain-IPv4 allowlist still matches a peer that surfaces in mapped form. None for a non-IP string."""
try:
ip = ipaddress.ip_address(s.strip())
except ValueError:
return None
return ip.ipv4_mapped if getattr(ip, "ipv4_mapped", None) else ip
def _client_ip(request: Request) -> str: def _client_ip(request: Request) -> str:
"""Best-effort client IP for rate limiting. X-Forwarded-For is client-controlled, so we trust it """Best-effort client IP for rate limiting. X-Forwarded-For is client-controlled, so we trust it
ONLY when the request actually arrived from a configured reverse proxy (settings.trusted_proxy_ips ONLY when the request actually arrived from a configured reverse proxy (settings.trusted_proxy_ips
the address the proxy connects FROM, e.g. the VPS Caddy's WireGuard peer IP). Our proxy APPENDS the address the proxy connects FROM, e.g. the VPS Caddy's WireGuard peer IP). Our proxy APPENDS
the client it saw to XFF, so the RIGHTMOST entry is the real client and can't be forged by a the client it saw to XFF, so the RIGHTMOST entry is the real client and can't be forged by a
client pre-seeding the header. Any other peer (someone hitting the app port directly, bypassing the client pre-seeding the header. Any other peer (someone hitting the app port directly, bypassing the
proxy) is untrusted we use its real socket IP, so it can't spoof its rate-limit identity.""" proxy) is untrusted we use its real socket IP, so it can't spoof its rate-limit identity.
SAFETY: this relies on `request.client` being the TRUE socket peer. uvicorn is started WITHOUT
`--proxy-headers` (see entrypoint.sh) precisely so it never rewrites `request.client` from XFF
do NOT add that flag, or the trust check below would compare against a forgeable value."""
peer = request.client.host if request.client else "unknown" peer = request.client.host if request.client else "unknown"
if peer in settings.trusted_proxy_ip_set: trusted = {_norm_ip(p) for p in settings.trusted_proxy_ip_set} - {None}
if _norm_ip(peer) in trusted:
xff = request.headers.get("x-forwarded-for") xff = request.headers.get("x-forwarded-for")
parts = [p.strip() for p in (xff or "").split(",") if p.strip()] parts = [p.strip() for p in (xff or "").split(",") if p.strip()]
if parts: if parts:

View file

@ -10,4 +10,9 @@ echo "Starting Siftlode API..."
# timeout, so Caddy would reuse a connection uvicorn had just closed -> broken pipe / reset # timeout, so Caddy would reuse a connection uvicorn had just closed -> broken pipe / reset
# on the next request -> 502. Non-idempotent POSTs (the player's progress/state writes, # on the next request -> 502. Non-idempotent POSTs (the player's progress/state writes,
# fired every 5s) can't be auto-retried by the proxy, so those 502s reached the user. # fired every 5s) can't be auto-retried by the proxy, so those 502s reached the user.
#
# SECURITY: do NOT add --proxy-headers / --forwarded-allow-ips here. auth._client_ip trusts
# X-Forwarded-For only when the TRUE socket peer is a configured proxy (TRUSTED_PROXY_IPS); those
# flags would make uvicorn overwrite request.client from the forgeable XFF, defeating that check and
# reopening the rate-limit bypass (SA3).
exec uvicorn app.main:app --host 0.0.0.0 --port 8000 --log-config log_config.json --timeout-keep-alive 75 exec uvicorn app.main:app --host 0.0.0.0 --port 8000 --log-config log_config.json --timeout-keep-alive 75