Skip to content

Commit 0e58db0

Browse files
dsarnoclaude
andcommitted
harden(transport): gate --allow-host on the real socket peer, not the Host header
When --allow-host widens access to a LAN range, the HTTP transport binds off loopback and authorization previously rested entirely on is_allowed_host, which range-checks the client-controlled Host header. A peer outside the allowed range could pass by spoofing `Host: <an-allowed-ip>`; the real peer address was never consulted (no scope["client"] / remote_address use anywhere). Add peer_ip_allowed(): the authoritative gate keyed on the unforgeable socket peer (scope["client"] for ASGI, remote_address for the WebSocket server). When an allowlist is set, the peer must be loopback or inside an allowed network; the Host header is kept as a secondary range filter but is no longer the authority. Fails closed when opted in and the peer can't be determined. Loopback-only mode (no --allow-host) is a pass-through — the kernel bind already guarantees a loopback peer — so behavior there is byte-for-byte unchanged. The WebSocket port stays loopback-only regardless, so its guard extracts the peer defensively but never gates on it today. Adds peer_ip_allowed unit coverage plus middleware/evaluate_loopback tests for the spoofed-Host-from-foreign-peer case and the fail-closed-on-unknown-peer case. Addresses advisory GHSA-pp3p-fhg5-f429. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent be40e1c commit 0e58db0

3 files changed

Lines changed: 218 additions & 25 deletions

File tree

src/godot_ai/__init__.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,12 @@ def main(argv: Sequence[str] | None = None) -> None:
9494
"Expose the server to a named LAN range for remote agents (issue "
9595
"#421). Takes a CIDR or bare IP (repeatable, or comma-separated), "
9696
"e.g. --allow-host 192.168.1.0/24. When set, both transports bind "
97-
"off loopback and the rebinding guard widens its Host allowlist to "
98-
"these networks ONLY — browser Origin / Sec-Fetch-Site checks stay "
99-
"on. Omit for the default loopback-only behavior. Prefer an SSH "
100-
"tunnel / Tailscale on untrusted networks; only name ranges you trust."
97+
"off loopback and access is gated on the real socket peer address "
98+
"(unforgeable) falling inside these networks ONLY — the Host header "
99+
"is range-checked too but is not the authority, and browser Origin "
100+
"/ Sec-Fetch-Site checks stay on. Omit for the default loopback-only "
101+
"behavior. Prefer an SSH tunnel / Tailscale on untrusted networks; "
102+
"only name ranges you trust."
101103
),
102104
)
103105
parser.add_argument(

src/godot_ai/transport/origin_guard.py

Lines changed: 79 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@
3535
``no-cors`` subresources that wouldn't carry an Origin — is refused with
3636
HTTP 403 long before reaching FastMCP or our session registry.
3737
38+
When the ``--allow-host`` opt-in (#421) binds the transport off loopback,
39+
the **real socket peer** (``scope["client"]`` / ``remote_address``) is the
40+
authoritative LAN gate — see :func:`peer_ip_allowed`. The ``Host`` header
41+
is client-controlled, so a peer outside the allowed range could otherwise
42+
pass the range check by spoofing ``Host: <an-allowed-ip>``; gating on the
43+
unforgeable peer address closes that. In the default loopback-only mode the
44+
peer gate is a pass-through and behavior is byte-for-byte unchanged.
45+
3846
See umbrella #343, finding #1 (audit-v2).
3947
"""
4048

@@ -158,6 +166,39 @@ def _host_ip_in_networks(host_header: str, networks: Sequence[IPNetwork] | None)
158166
return any(ip in net for net in networks)
159167

160168

169+
def peer_ip_allowed(peer_ip: str | None, allowed_networks: Sequence[IPNetwork] | None) -> bool:
170+
"""Whether the real TCP peer address is permitted to reach the transport.
171+
172+
This is the authoritative LAN gate. The ``Host`` header is
173+
client-controlled — a peer outside an allowed network can spoof
174+
``Host: <an-allowed-ip>`` and pass :func:`is_allowed_host` — so when
175+
the transport is bound off loopback (the ``--allow-host`` opt-in, #421)
176+
the *socket peer* (``scope["client"]`` for ASGI, ``remote_address`` for
177+
the WebSocket server), which the client cannot forge, must itself be
178+
loopback or fall inside an allowed network.
179+
180+
With no allowlist (the default), the transport stays bound to loopback,
181+
so the kernel already guarantees a loopback peer and this is a
182+
pass-through that keeps the original behavior byte-for-byte — including
183+
contexts (e.g. unit scopes) where the peer address isn't populated.
184+
When an allowlist *is* set but the peer can't be determined, it fails
185+
closed: better to refuse than to fall back to the spoofable header.
186+
"""
187+
if not allowed_networks:
188+
return True
189+
if not peer_ip:
190+
return False
191+
## Strip an IPv6 zone id (``fe80::1%eth0``) before parsing.
192+
candidate = peer_ip.split("%", 1)[0].strip()
193+
try:
194+
ip = ipaddress.ip_address(candidate)
195+
except ValueError:
196+
return False
197+
if ip.is_loopback:
198+
return True
199+
return any(ip in net for net in allowed_networks)
200+
201+
161202
def is_allowed_host(
162203
host_header: str | None,
163204
allowed_networks: Sequence[IPNetwork] | None = None,
@@ -230,22 +271,30 @@ def evaluate_loopback(
230271
origins: list[str],
231272
sec_fetch_sites: list[str] | None = None,
232273
allowed_networks: Sequence[IPNetwork] | None = None,
274+
peer_ip: str | None = None,
233275
) -> bool:
234-
"""Return True iff the request's headers pass the allowlist.
276+
"""Return True iff the request passes the allowlist.
235277
236278
Both transports (ASGI middleware + WebSocket ``process_request``)
237-
funnel their per-request header extraction through this helper so
238-
the duplicate-header smuggling rule, the value-allowlist rule, and
239-
the Sec-Fetch-Site cross-origin reject rule are evaluated identically.
240-
A divergence between the two transports would be a security
241-
regression — this helper exists to prevent it.
242-
243-
``allowed_networks`` (the ``--allow-host`` opt-in, #421) only widens
244-
the *Host* allowlist to named LAN CIDRs. The Origin and Sec-Fetch-Site
245-
rules are deliberately left untouched: a browser on the LAN sends a
246-
non-loopback Origin (rejected) and a foreign Sec-Fetch-Site (rejected),
247-
so DNS-rebinding defense survives the opt-in. A native remote agent
248-
sends neither header, so it passes once its Host IP is allowed.
279+
funnel their per-request extraction through this helper so the
280+
duplicate-header smuggling rule, the value-allowlist rule, the
281+
Sec-Fetch-Site cross-origin reject rule, and the peer-address gate are
282+
evaluated identically. A divergence between the two transports would be
283+
a security regression — this helper exists to prevent it.
284+
285+
``allowed_networks`` (the ``--allow-host`` opt-in, #421) widens access
286+
to named LAN CIDRs. Authorization is anchored on ``peer_ip`` — the real
287+
socket peer, which the client cannot forge — not on the ``Host`` header
288+
(which it can). The Host header is still range-checked, but only as a
289+
secondary filter; the peer gate is what actually keeps out-of-range
290+
hosts off the server. The Origin and Sec-Fetch-Site rules are left
291+
untouched: a browser on the LAN sends a non-loopback Origin (rejected)
292+
and a foreign Sec-Fetch-Site (rejected), so DNS-rebinding defense
293+
survives the opt-in. A native remote agent sends neither header, so it
294+
passes once both its peer address and Host IP are allowed.
295+
296+
When ``allowed_networks`` is None (the loopback-only default), the peer
297+
gate is a pass-through and behavior is byte-for-byte unchanged.
249298
"""
250299
if len(hosts) > 1 or len(origins) > 1:
251300
return False
@@ -255,7 +304,8 @@ def evaluate_loopback(
255304
origin = origins[0] if origins else None
256305
sec_fetch_site = sec_fetch_sites[0] if sec_fetch_sites else None
257306
return (
258-
is_allowed_host(host, allowed_networks)
307+
peer_ip_allowed(peer_ip, allowed_networks)
308+
and is_allowed_host(host, allowed_networks)
259309
and is_allowed_origin(origin)
260310
and is_allowed_sec_fetch_site(sec_fetch_site)
261311
)
@@ -300,7 +350,13 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
300350
elif key == b"sec-fetch-site":
301351
sec_fetch_sites.append(raw_value.decode("latin-1"))
302352

303-
if evaluate_loopback(hosts, origins, sec_fetch_sites, self.allowed_networks):
353+
## ASGI populates ``scope["client"]`` with the real ``(host, port)``
354+
## peer — unforgeable, unlike the Host header. ``None`` in unusual
355+
## servers; the peer gate fails closed for it only when opted in.
356+
client = scope.get("client")
357+
peer_ip = client[0] if client else None
358+
359+
if evaluate_loopback(hosts, origins, sec_fetch_sites, self.allowed_networks, peer_ip):
304360
await self.app(scope, receive, send)
305361
return
306362
await _send_forbidden(send)
@@ -342,7 +398,14 @@ async def guard(connection, request):
342398
hosts = list(request.headers.get_all("Host"))
343399
origins = list(request.headers.get_all("Origin"))
344400
sec_fetch_sites = list(request.headers.get_all("Sec-Fetch-Site"))
345-
if evaluate_loopback(hosts, origins, sec_fetch_sites, networks):
401+
## ``remote_address`` is the real TCP peer (set before the HTTP
402+
## upgrade) — unforgeable, unlike the Host header. It's a
403+
## ``(host, port[, flowinfo, scopeid])`` tuple, or None if the
404+
## socket is already gone; the peer gate fails closed for None
405+
## only when opted in.
406+
remote = getattr(connection, "remote_address", None)
407+
peer_ip = remote[0] if remote else None
408+
if evaluate_loopback(hosts, origins, sec_fetch_sites, networks, peer_ip):
346409
return None
347410
return connection.respond(HTTPStatus.FORBIDDEN, FORBIDDEN_BODY_TEXT)
348411

tests/unit/test_origin_guard.py

Lines changed: 133 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
is_allowed_origin,
2020
is_allowed_sec_fetch_site,
2121
parse_allow_hosts,
22+
peer_ip_allowed,
2223
)
2324

2425
# ---------------------------------------------------------------------------
@@ -243,8 +244,14 @@ async def _call_middleware(
243244
*,
244245
headers: list[tuple[bytes, bytes]],
245246
scope_type: str = "http",
247+
client: tuple[str, int] | None = None,
246248
) -> tuple[list[dict], bool]:
247-
"""Run the middleware against a synthetic scope; return (sent, inner_called)."""
249+
"""Run the middleware against a synthetic scope; return (sent, inner_called).
250+
251+
``client`` populates ASGI's ``scope["client"]`` (the real ``(host, port)``
252+
peer). Omitted by default so loopback-only tests don't need it — the peer
253+
gate is a pass-through unless ``allowed_networks`` is set.
254+
"""
248255
inner_called = False
249256

250257
async def inner(scope, receive, send):
@@ -263,6 +270,8 @@ async def receive():
263270
return {"type": "http.request", "body": b"", "more_body": False}
264271

265272
scope = {"type": scope_type, "method": "GET", "path": "/x", "headers": headers}
273+
if client is not None:
274+
scope["client"] = client
266275
await middleware(scope, receive, send)
267276
return sent, inner_called
268277

@@ -518,8 +527,8 @@ def test_is_allowed_host_without_networks_unchanged() -> None:
518527

519528
def test_evaluate_loopback_native_lan_agent_passes() -> None:
520529
nets = parse_allow_hosts(["192.168.1.0/24"])
521-
# Native remote agent: LAN Host, no Origin, no Sec-Fetch-Site.
522-
assert evaluate_loopback(["192.168.1.50:8000"], [], [], nets) is True
530+
# Native remote agent: in-range peer, LAN Host, no Origin/Sec-Fetch-Site.
531+
assert evaluate_loopback(["192.168.1.50:8000"], [], [], nets, peer_ip="192.168.1.50") is True
523532

524533

525534
def test_evaluate_loopback_lan_browser_origin_rejected() -> None:
@@ -532,14 +541,34 @@ def test_evaluate_loopback_lan_browser_origin_rejected() -> None:
532541
["http://192.168.1.50:8000"],
533542
[],
534543
nets,
544+
peer_ip="192.168.1.50",
535545
)
536546
is False
537547
)
538548

539549

540550
def test_evaluate_loopback_lan_cross_site_subresource_rejected() -> None:
541551
nets = parse_allow_hosts(["192.168.1.0/24"])
542-
assert evaluate_loopback(["192.168.1.50:8000"], [], ["cross-site"], nets) is False
552+
assert (
553+
evaluate_loopback(["192.168.1.50:8000"], [], ["cross-site"], nets, peer_ip="192.168.1.50")
554+
is False
555+
)
556+
557+
558+
def test_evaluate_loopback_spoofed_host_foreign_peer_rejected() -> None:
559+
"""The core --allow-host fix: an out-of-range peer that spoofs an
560+
in-range Host header is rejected on the peer gate. Host alone is not
561+
the authority."""
562+
nets = parse_allow_hosts(["192.168.1.0/24"])
563+
# Attacker at 10.0.0.5 (outside the CIDR) sends Host: 192.168.1.50.
564+
assert evaluate_loopback(["192.168.1.50:8000"], [], [], nets, peer_ip="10.0.0.5") is False
565+
566+
567+
def test_evaluate_loopback_in_range_peer_loopback_host_passes() -> None:
568+
"""A local client on the LAN-bound server (peer 127.0.0.1, Host
569+
loopback) still works after the opt-in."""
570+
nets = parse_allow_hosts(["192.168.1.0/24"])
571+
assert evaluate_loopback(["127.0.0.1:8000"], [], [], nets, peer_ip="127.0.0.1") is True
543572

544573

545574
async def test_middleware_allows_lan_host_when_opted_in() -> None:
@@ -548,6 +577,7 @@ async def test_middleware_allows_lan_host_when_opted_in() -> None:
548577
sent, inner_called = await _call_middleware(
549578
middleware,
550579
headers=[(b"host", b"192.168.1.50:8000")],
580+
client=("192.168.1.50", 51234),
551581
)
552582
assert inner_called is True
553583
assert sent[0]["status"] == 200
@@ -562,17 +592,62 @@ async def test_middleware_rejects_lan_host_browser_origin_when_opted_in() -> Non
562592
(b"host", b"192.168.1.50:8000"),
563593
(b"origin", b"http://192.168.1.50:8000"),
564594
],
595+
client=("192.168.1.50", 51234),
596+
)
597+
assert inner_called is False
598+
assert sent[0]["status"] == 403
599+
600+
601+
async def test_middleware_rejects_spoofed_host_from_foreign_peer() -> None:
602+
"""The --allow-host fix at the transport boundary: a peer OUTSIDE the
603+
allowed range spoofs an in-range Host header. The peer gate refuses it
604+
even though the Host passes the range check."""
605+
nets = parse_allow_hosts(["192.168.1.0/24"])
606+
middleware = LocalhostOnlyHTTPMiddleware(app=None, allowed_networks=nets) # type: ignore[arg-type]
607+
sent, inner_called = await _call_middleware(
608+
middleware,
609+
headers=[(b"host", b"192.168.1.50:8000")],
610+
client=("10.0.0.5", 51234),
611+
)
612+
assert inner_called is False
613+
assert sent[0]["status"] == 403
614+
615+
616+
async def test_middleware_rejects_when_opted_in_and_peer_unknown() -> None:
617+
"""Bound off loopback but the ASGI server didn't populate scope["client"]
618+
— fail closed rather than fall back to the spoofable Host header."""
619+
nets = parse_allow_hosts(["192.168.1.0/24"])
620+
middleware = LocalhostOnlyHTTPMiddleware(app=None, allowed_networks=nets) # type: ignore[arg-type]
621+
sent, inner_called = await _call_middleware(
622+
middleware,
623+
headers=[(b"host", b"192.168.1.50:8000")],
624+
client=None,
565625
)
566626
assert inner_called is False
567627
assert sent[0]["status"] == 403
568628

569629

630+
async def test_middleware_allows_loopback_peer_when_opted_in() -> None:
631+
"""A local client (peer 127.0.0.1) on a LAN-bound server still works."""
632+
nets = parse_allow_hosts(["192.168.1.0/24"])
633+
middleware = LocalhostOnlyHTTPMiddleware(app=None, allowed_networks=nets) # type: ignore[arg-type]
634+
sent, inner_called = await _call_middleware(
635+
middleware,
636+
headers=[(b"host", b"127.0.0.1:8000")],
637+
client=("127.0.0.1", 51234),
638+
)
639+
assert inner_called is True
640+
assert sent[0]["status"] == 200
641+
642+
570643
async def test_middleware_rejects_lan_host_without_opt_in() -> None:
571-
# Default middleware (no networks) keeps rejecting LAN hosts.
644+
# Default middleware (no networks) keeps rejecting LAN hosts. A loopback
645+
# peer is irrelevant — the loopback bind already guarantees it.
572646
middleware = LocalhostOnlyHTTPMiddleware(app=None) # type: ignore[arg-type]
573647
sent, inner_called = await _call_middleware(
574648
middleware,
575649
headers=[(b"host", b"192.168.1.50:8000")],
650+
client=("127.0.0.1", 51234),
576651
)
577652
assert inner_called is False
578653
assert sent[0]["status"] == 403
@@ -599,3 +674,56 @@ def test_bind_host_for_networks_prioritizes_ipv4_reachability() -> None:
599674
# and silently drop IPv4 reachability. See bind_host_for_networks docstring.
600675
assert bind_host_for_networks(parse_allow_hosts(["192.168.1.0/24", "fd00::/8"])) == "0.0.0.0"
601676
assert bind_host_for_networks(parse_allow_hosts(["fd00::/8", "10.0.0.0/8"])) == "0.0.0.0"
677+
678+
679+
# ---------------------------------------------------------------------------
680+
# peer_ip_allowed — the authoritative, unforgeable LAN gate (--allow-host fix)
681+
# ---------------------------------------------------------------------------
682+
683+
684+
def test_peer_ip_allowed_no_networks_is_passthrough() -> None:
685+
# Loopback-only default: the kernel bind guarantees a loopback peer, so the
686+
# gate is a pass-through — even when the peer is unknown — keeping behavior
687+
# byte-for-byte unchanged.
688+
assert peer_ip_allowed(None, None) is True
689+
assert peer_ip_allowed("127.0.0.1", None) is True
690+
assert peer_ip_allowed("192.168.1.50", None) is True
691+
assert peer_ip_allowed("10.0.0.5", []) is True
692+
693+
694+
def test_peer_ip_allowed_in_network() -> None:
695+
nets = parse_allow_hosts(["192.168.1.0/24"])
696+
assert peer_ip_allowed("192.168.1.50", nets) is True
697+
assert peer_ip_allowed("192.168.1.1", nets) is True
698+
699+
700+
def test_peer_ip_allowed_out_of_network_rejected() -> None:
701+
nets = parse_allow_hosts(["192.168.1.0/24"])
702+
assert peer_ip_allowed("192.168.2.50", nets) is False
703+
assert peer_ip_allowed("10.0.0.5", nets) is False
704+
705+
706+
def test_peer_ip_allowed_loopback_always_ok_when_opted_in() -> None:
707+
nets = parse_allow_hosts(["192.168.1.0/24"])
708+
assert peer_ip_allowed("127.0.0.1", nets) is True
709+
assert peer_ip_allowed("::1", nets) is True
710+
711+
712+
def test_peer_ip_allowed_missing_fails_closed_when_opted_in() -> None:
713+
nets = parse_allow_hosts(["192.168.1.0/24"])
714+
assert peer_ip_allowed(None, nets) is False
715+
assert peer_ip_allowed("", nets) is False
716+
717+
718+
def test_peer_ip_allowed_unparseable_fails_closed() -> None:
719+
nets = parse_allow_hosts(["192.168.1.0/24"])
720+
assert peer_ip_allowed("not-an-ip", nets) is False
721+
# A DNS name never reaches here (peer is always an IP literal), but if it
722+
# somehow did, it must not slip through.
723+
assert peer_ip_allowed("attacker.example.com", nets) is False
724+
725+
726+
def test_peer_ip_allowed_strips_ipv6_zone_id() -> None:
727+
nets = parse_allow_hosts(["fd00::/8"])
728+
assert peer_ip_allowed("fd00::1%eth0", nets) is True
729+
assert peer_ip_allowed("2001:db8::1%eth0", nets) is False

0 commit comments

Comments
 (0)