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:
2026-07-12 03:49:08 +02:00
parent db2257c5e7
commit ff8e49b8ce
2 changed files with 23 additions and 2 deletions
+18 -2
View File
@@ -1,3 +1,4 @@
import ipaddress
import logging
import secrets
from datetime import datetime, timedelta, timezone
@@ -172,15 +173,30 @@ def get_or_create_demo_user(db: Session) -> 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:
"""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
— 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
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"
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")
parts = [p.strip() for p in (xff or "").split(",") if p.strip()]
if parts:
+5
View File
@@ -10,4 +10,9 @@ echo "Starting Siftlode API..."
# 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,
# 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