mirror of
https://github.com/BigBodyCobain/Shadowbroker.git
synced 2026-08-03 02:33:37 +02:00
External security audit by @tg12 (May 17, 2026) filed 11 issues against the backend. PR #227 (May 18, AI-generated) closed seven of them by adding require_local_operator to control-plane endpoints. Four remained live; this PR closes the rest. #192 — CCTV proxy followed redirects without re-validating host Issue: /api/cctv/media validated only the caller-supplied URL host before passing it to requests.get(..., allow_redirects=True). A 302 to http://127.0.0.1 or any internal/disallowed host was silently followed, turning the proxy into an open-redirect-to-SSRF chain. Fix in routers/cctv.py: replace the single allow_redirects=True call with a manual follow loop. Each hop's Location is parsed, the host is rerun through _cctv_host_allowed(), and non-HTTP schemes (file://, ftp://, etc.) are rejected. Cap chain length at 5 hops. Test: backend/tests/test_cctv_redirect_ssrf.py covers - redirect to disallowed host -> 502 - redirect to localhost -> 502 - redirect to another allowed host -> 200 - redirect chain length cap - non-HTTP scheme rejected #198 — Gate introspection GETs were unauthenticated Issue: /api/wormhole/gate/{gate_id}/{identity,personas,key} were callable with no auth dependency. Any caller that could reach the backend could dump the operator's active persona, persona inventory, and key status for any gate_id they knew. The wiki's privacy threat model explicitly markets gate personas as rotating, unlinkable pseudonyms — this leak defeated that property. Fix in routers/wormhole.py: add dependencies=[Depends(require_local_operator)] to all three routes. Test: backend/tests/test_control_surface_auth.py extended with three new parameterized cases (lines 75-77). #199 — GDELT military incident ingestion used plaintext HTTP Issue: backend/services/geopolitics.py fetched http://data.gdeltproject.org/gdeltv2/lastupdate.txt and ~48 export archive URLs over plaintext HTTP. Passive observers could identify Shadowbroker nodes from the fetch pattern. Active MITM could inject doctored military incident records into the global map. Fix in services/geopolitics.py: rewrite the lastupdate.txt fetch and the export download URL constructor to use https://. GDELT's data.gdeltproject.org serves the same content over HTTPS. Test: backend/tests/test_gdelt_https.py asserts no plaintext HTTP URLs to data.gdeltproject.org remain in code (comments excluded) and that the HTTPS URLs we expect are present. #200 — Sentinel token cache lookup used client_id only Issue: routers/tools.py kept a process-global cache of Copernicus bearer tokens. The lookup compared _sh_token_cache["client_id"] == client_id. A caller who knew a valid client_id but supplied any wrong client_secret hit the cache and reused the legitimate caller's bearer token — burning their quota and accessing imagery on their account. Fix in routers/tools.py: replace the client_id field with credential_fp, an HMAC-SHA256 over (client_id, client_secret) under a per-process random key (_SH_TOKEN_CACHE_HMAC_KEY = os.urandom(32), regenerated at startup). A caller who doesn't know the secret cannot compute a matching fingerprint, so they miss the cache and hit the real Copernicus token endpoint — which will reject their wrong secret with a 401. Test: backend/tests/test_sentinel_token_cache.py covers - same client_id + different secrets => different fingerprints - same credentials => same fingerprint (cache still works) - different client_ids + same secret => different fingerprints - cache no longer stores raw client_id (catches regression) - attacker with wrong secret cannot reuse victim's token Validation pytest backend/tests/test_control_surface_auth.py backend/tests/test_cctv_redirect_ssrf.py backend/tests/test_gdelt_https.py backend/tests/test_sentinel_token_cache.py -> 37 passed Credit: @tg12 reported all four of these in their May 17 audit with correct line-number citations and accurate remediation recommendations. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
e3297e9bc0
commit
d00c63abed
+58
-1
@@ -191,11 +191,68 @@ def _cctv_response_headers(resp, cache_seconds: int, include_length: bool = True
|
||||
return headers
|
||||
|
||||
|
||||
# Maximum number of redirects we'll follow on the CCTV upstream. Each hop is
|
||||
# re-validated against _cctv_host_allowed() before continuing, so this caps
|
||||
# the redirect-chain SSRF blast radius.
|
||||
_CCTV_MAX_REDIRECTS = 5
|
||||
|
||||
|
||||
def _fetch_cctv_upstream_response(request: Request, target_url: str, profile: _CCTVProxyProfile):
|
||||
"""Fetch an upstream CCTV URL, following redirects manually with host re-validation.
|
||||
|
||||
Why manual redirect following:
|
||||
The original code used ``allow_redirects=True``, which only validated
|
||||
the initial caller-supplied URL host against the allowlist. An attacker
|
||||
could submit an allowed host that 302-redirected to an internal address
|
||||
(e.g. ``http://localhost:8000/api/...`` or a private RFC1918 range),
|
||||
and the backend would dutifully follow and proxy the response — a
|
||||
classic open-redirect-to-SSRF chain.
|
||||
|
||||
With this loop, we re-run ``_cctv_host_allowed()`` on every hop's
|
||||
``Location`` header. A redirect to a host that isn't on the allowlist
|
||||
is rejected with 502 rather than silently followed.
|
||||
"""
|
||||
import requests as _req
|
||||
from urllib.parse import urlparse, urljoin
|
||||
|
||||
headers = _cctv_upstream_headers(request, profile)
|
||||
current_url = target_url
|
||||
hops = 0
|
||||
try:
|
||||
resp = _req.get(target_url, timeout=profile.timeout, stream=True, allow_redirects=True, headers=headers)
|
||||
while True:
|
||||
resp = _req.get(
|
||||
current_url,
|
||||
timeout=profile.timeout,
|
||||
stream=True,
|
||||
allow_redirects=False,
|
||||
headers=headers,
|
||||
)
|
||||
# Redirect handling — re-validate the next-hop host before following.
|
||||
if resp.is_redirect or resp.status_code in (301, 302, 303, 307, 308):
|
||||
location = resp.headers.get("Location", "")
|
||||
resp.close()
|
||||
if hops >= _CCTV_MAX_REDIRECTS:
|
||||
logger.warning(
|
||||
"CCTV upstream redirect chain exceeded limit [%s] %s",
|
||||
profile.name, target_url,
|
||||
)
|
||||
raise HTTPException(status_code=502, detail="Upstream redirect chain too long")
|
||||
if not location:
|
||||
raise HTTPException(status_code=502, detail="Upstream redirect missing Location")
|
||||
next_url = urljoin(current_url, location)
|
||||
next_parsed = urlparse(next_url)
|
||||
if next_parsed.scheme not in ("http", "https"):
|
||||
raise HTTPException(status_code=502, detail="Upstream redirect to non-HTTP scheme")
|
||||
if not _cctv_host_allowed(next_parsed.hostname):
|
||||
logger.warning(
|
||||
"CCTV upstream redirect to disallowed host [%s] %s -> %s",
|
||||
profile.name, current_url, next_url,
|
||||
)
|
||||
raise HTTPException(status_code=502, detail="Upstream redirect to disallowed host")
|
||||
current_url = next_url
|
||||
hops += 1
|
||||
continue
|
||||
break
|
||||
except _req.exceptions.Timeout as exc:
|
||||
logger.warning("CCTV upstream timeout [%s] %s", profile.name, target_url)
|
||||
raise HTTPException(status_code=504, detail="Upstream timeout") from exc
|
||||
|
||||
@@ -120,7 +120,36 @@ async def api_sentinel_token(request: Request):
|
||||
raise HTTPException(502, "Token request failed")
|
||||
|
||||
|
||||
_sh_token_cache: dict = {"token": None, "expiry": 0, "client_id": ""}
|
||||
# Cache key is an HMAC of (client_id, client_secret) — a caller cannot hit
|
||||
# this cache without knowing the same secret that originally populated it.
|
||||
# Without this binding, the lookup only checked client_id, so anyone who
|
||||
# knew a valid client_id could reuse another caller's cached token (and
|
||||
# burn their Copernicus quota / access tiles on their account).
|
||||
_sh_token_cache: dict = {"token": None, "expiry": 0, "credential_fp": ""}
|
||||
|
||||
|
||||
def _credential_fingerprint(client_id: str, client_secret: str) -> str:
|
||||
"""Return a stable, secret-binding fingerprint for the Sentinel cache key.
|
||||
|
||||
Uses HMAC-SHA256 so the raw secret is never stored in process memory as
|
||||
a cache key. The HMAC key is a per-process random value, which means the
|
||||
fingerprint cannot be precomputed across restarts (additional defense
|
||||
against an attacker who learned a valid client_id but not the secret).
|
||||
"""
|
||||
import hashlib
|
||||
import hmac
|
||||
|
||||
return hmac.new(
|
||||
_SH_TOKEN_CACHE_HMAC_KEY,
|
||||
f"{client_id}\x00{client_secret}".encode("utf-8"),
|
||||
hashlib.sha256,
|
||||
).hexdigest()
|
||||
|
||||
|
||||
# Per-process random HMAC key. Regenerated on each backend startup so cached
|
||||
# fingerprints don't survive restarts.
|
||||
import os as _os
|
||||
_SH_TOKEN_CACHE_HMAC_KEY = _os.urandom(32)
|
||||
|
||||
|
||||
@router.post("/api/sentinel/tile")
|
||||
@@ -146,7 +175,9 @@ async def api_sentinel_tile(request: Request):
|
||||
raise HTTPException(400, "client_id, client_secret, and date required")
|
||||
|
||||
now = _time.time()
|
||||
if (_sh_token_cache["token"] and _sh_token_cache["client_id"] == client_id
|
||||
credential_fp = _credential_fingerprint(client_id, client_secret)
|
||||
if (_sh_token_cache["token"]
|
||||
and _sh_token_cache["credential_fp"] == credential_fp
|
||||
and now < _sh_token_cache["expiry"] - 30):
|
||||
token = _sh_token_cache["token"]
|
||||
else:
|
||||
@@ -161,7 +192,7 @@ async def api_sentinel_tile(request: Request):
|
||||
token = tdata["access_token"]
|
||||
_sh_token_cache["token"] = token
|
||||
_sh_token_cache["expiry"] = now + tdata.get("expires_in", 300)
|
||||
_sh_token_cache["client_id"] = client_id
|
||||
_sh_token_cache["credential_fp"] = credential_fp
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception:
|
||||
|
||||
@@ -793,19 +793,19 @@ async def api_wormhole_gate_leave(request: Request, body: WormholeGateRequest):
|
||||
return leave_gate(str(body.gate_id or ""))
|
||||
|
||||
|
||||
@router.get("/api/wormhole/gate/{gate_id}/identity")
|
||||
@router.get("/api/wormhole/gate/{gate_id}/identity", dependencies=[Depends(require_local_operator)])
|
||||
@limiter.limit("30/minute")
|
||||
async def api_wormhole_gate_identity(request: Request, gate_id: str):
|
||||
return get_active_gate_identity(gate_id)
|
||||
|
||||
|
||||
@router.get("/api/wormhole/gate/{gate_id}/personas")
|
||||
@router.get("/api/wormhole/gate/{gate_id}/personas", dependencies=[Depends(require_local_operator)])
|
||||
@limiter.limit("30/minute")
|
||||
async def api_wormhole_gate_personas(request: Request, gate_id: str):
|
||||
return list_gate_personas(gate_id)
|
||||
|
||||
|
||||
@router.get("/api/wormhole/gate/{gate_id}/key")
|
||||
@router.get("/api/wormhole/gate/{gate_id}/key", dependencies=[Depends(require_local_operator)])
|
||||
@limiter.limit("30/minute")
|
||||
async def api_wormhole_gate_key_status(request: Request, gate_id: str):
|
||||
import main as _m
|
||||
|
||||
Reference in New Issue
Block a user