fix(guard): operator-scoped XFF hop peers + tailnet allowlist (live-incident fix) (#650)

Two coupled hardenings from the chain-peers adversarial rounds plus the root-cause fix for the live post-deploy incident where the CEO was blocked from the panel ('IP not allowed: 100.x.x.x'). Hop peel-set: the whole docker bridge pool leaves the XFF hop set — hops are now loopback plus operator-named single addresses only (ROBOCO_GUARD_TRUSTED_CHAIN_PEERS, plain IPs; CIDR entries rejected with a warning because a range readmits sibling containers). Default-empty closes the CGNAT-forge residual outright; a gateway-fronted Tailscale Serve deploy sets its real gateway IP, and a rate-limited detection log names exactly that IP when an unconfigured host-proxied tailnet chain is seen, so the silent-regression shape is observable. The connecting-peer gate (may nginx present XFF at all) deliberately keeps the broad bridge pool — different check, unchanged. Incident root cause: guard-core's whitelist is an EXCLUSIVE allowlist (any non-member is refused), so honestly resolving the tailnet client IP made ip_security reject the CEO. The tailnet CGNAT range joins _guard_whitelist() deliberately: Tailscale authenticates device membership before a packet arrives, real-IP stamping still buys correct attribution, and any future non-tailnet exposure keeps full scrutiny. Both compose files now pass ROBOCO_GUARD_EMERGENCY_WHITELIST through to the orchestrator (the operator escape hatch previously did nothing in a compose deploy). NAS is running ROBOCO_GUARD_PASSIVE_MODE=true as interim mitigation — flip back to false when this deploys. 66 tests. Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-08-03 07:23:24 +02:00 · 2026-07-23 00:05:29 +02:00
parent da4d9b333d
commit 3806317aa7
8 changed files with 372 additions and 43 deletions
@@ -209,10 +209,20 @@ def test_internal_agent_mesh_is_whitelisted() -> None:
    """Agents reach the orchestrator directly on the docker bridge, HMAC-
    authenticated; the guard's threat-ban is for the external surface. Without
    this the guard IP-banned agent containers the moment it went active
-    (2026-07-20 incident) and wedged every subsequent gateway verb."""
+    (2026-07-20 incident) and wedged every subsequent gateway verb.
+
+    The tailnet CGNAT range (100.64.0.0/10) rides the same whitelist:
+    guard-core's whitelist is EXCLUSIVE once non-empty (any non-member IP is
+    refused, not merely unexempted), and the resolver now honestly resolves
+    a host-proxied tailnet client to its real 100.64.0.0/10 address instead
+    of a loopback/bridge hop — omitting it here blocked the CEO's own
+    tailnet IP live (2026-07-22 incident). Tailscale is an authenticated
+    overlay gating device membership before a packet arrives, so coupling
+    allowlisting with scrutiny-exemption is the deliberate posture for this
+    one range."""
    cfg = security.build_security_config()
    assert cfg.whitelist is not None
-    for net in ("127.0.0.1", "::1", "172.16.0.0/12"):
+    for net in ("127.0.0.1", "::1", "172.16.0.0/12", "100.64.0.0/10"):
        assert net in cfg.whitelist


@@ -3,12 +3,16 @@
 fastapi-guard peels a fixed trusted_proxy_depth=1 from X-Forwarded-For (the
 rightmost entry, which nginx itself recorded). That is correct for every
 chain except host-proxied tailnet traffic (Tailscale Serve → nginx), which
-arrives as ``[tailnet-client, <loopback-or-bridge-gateway>]`` — depth-1
+arrives as ``[tailnet-client, <loopback-or-configured-gateway>]`` — depth-1
 resolves it to a whitelisted hop IP and the WAF goes inert for /tg.
 ``ClientIpResolutionMiddleware`` stamps guard_core's ``state.client_ip``
 cache (the supported pre-resolution seam) for EXACTLY that shape and
 abstains on every other, so no path resolves differently from the depth-1
 baseline unless the candidate is a tailnet CGNAT address behind real hops.
+
+The XFF hop-peel set is loopback ALWAYS plus operator-named
+``guard_trusted_chain_peers`` (default empty) — NOT the whole docker bridge
+pool, so an unnamed 172.x address can never be treated as a hop.
 """

 from __future__ import annotations
@@ -27,6 +31,19 @@ from roboco.security import (
 )
 from starlette.requests import Request

+
+def _configure_chain_peers(monkeypatch: pytest.MonkeyPatch, csv: str) -> None:
+    """Set guard_trusted_chain_peers and rebuild the effective hop set.
+
+    Mirrors what config load does once at import time; tests need to redo
+    it per-case since ``_TRUSTED_HOP_NETWORKS`` is otherwise built once.
+    """
+    monkeypatch.setattr(settings, "guard_trusted_chain_peers", csv)
+    monkeypatch.setattr(
+        security, "_TRUSTED_HOP_NETWORKS", security._build_trusted_hop_networks()
+    )
+
+
 # ---------------------------------------------------------------------------
 # resolve_forwarded_client_ip — stamps ONLY the tailnet-behind-hops shape
 # ---------------------------------------------------------------------------
@@ -38,14 +55,25 @@ def test_tailscale_serve_behind_loopback_resolves_tailnet_peer() -> None:
    )


-def test_tailscale_serve_behind_bridge_gateway_resolves_tailnet_peer() -> None:
+def test_tailscale_serve_behind_bridge_gateway_resolves_tailnet_peer(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
    # Docker DNAT presents host-originated connections as the bridge gateway,
-    # so nginx may record 172.x instead of loopback for Tailscale Serve.
+    # so nginx may record 172.x instead of loopback for Tailscale Serve. That
+    # 172.x address only peels once the operator names it explicitly.
+    _configure_chain_peers(monkeypatch, "172.18.0.1")
    assert (
        resolve_forwarded_client_ip("100.101.102.103, 172.18.0.1") == "100.101.102.103"
    )


+def test_bridge_gateway_unconfigured_by_default_never_peels() -> None:
+    # Same chain as above, but with NO configured chain peers (the default):
+    # the 172.x gateway is no longer a recognized hop, so the resolver
+    # abstains and depth-1 keeps resolving to the gateway IP itself.
+    assert resolve_forwarded_client_ip("100.101.102.103, 172.18.0.1") is None
+
+
 def test_forged_prefix_behind_tailscale_chain_ignored() -> None:
    assert (
        resolve_forwarded_client_ip("6.6.6.6, 100.101.102.103, 127.0.0.1")
@@ -66,11 +94,25 @@ def test_bridge_peer_with_forged_public_prefix_abstains() -> None:
    assert resolve_forwarded_client_ip("9.9.9.9, 172.20.0.7") is None


-def test_bridge_peer_forging_tailnet_prefix_only_deprivileges() -> None:
-    # Documented residual: forging a CGNAT prefix IS stamped — the forger
-    # loses its whitelist exemption (fake tailnet IPs eat the WAF); it can
-    # never gain privilege this way.
-    assert resolve_forwarded_client_ip("100.99.1.1, 172.20.0.7") == "100.99.1.1"
+def test_default_empty_chain_peers_closes_the_forge_residual() -> None:
+    # THE fixed residual: a same-bridge container relaying a forged
+    # tailnet-CGNAT XFF prefix used to have its unnamed 172.x rightmost
+    # entry peeled as a "trusted hop" (the whole /12 was the hop set),
+    # stamping the forged CGNAT address. With no chain peers configured
+    # (the default), 172.20.0.7 is never a recognized hop, so no hop is
+    # peeled and the resolver abstains — the forge no longer lands.
+    assert resolve_forwarded_client_ip("100.99.1.1, 172.20.0.7") is None
+
+
+def test_configured_peer_does_not_extend_to_other_bridge_addresses(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    # Even with a real chain peer configured (the docker gateway), a forger
+    # relaying through its OWN bridge IP — never the reserved gateway
+    # address — still abstains: only the exact configured peer(s) peel, not
+    # the whole bridge pool.
+    _configure_chain_peers(monkeypatch, "172.18.0.1")
+    assert resolve_forwarded_client_ip("100.99.1.1, 172.20.0.7") is None


 def test_non_tailnet_client_behind_hop_abstains() -> None:
@@ -93,6 +135,123 @@ def test_malformed_entries_abstain() -> None:
    assert resolve_forwarded_client_ip("100.99.1.1:443, 127.0.0.1") is None


+def test_loopback_hop_chains_unaffected_by_peer_config(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    # Loopback is unconditionally part of the hop set regardless of what
+    # (if anything) is configured — a chain peer config only ADDS to it.
+    _configure_chain_peers(monkeypatch, "172.18.0.1")
+    assert (
+        resolve_forwarded_client_ip("100.101.102.103, 127.0.0.1") == "100.101.102.103"
+    )
+    assert resolve_forwarded_client_ip("127.0.0.1") is None
+
+
+# ---------------------------------------------------------------------------
+# _build_trusted_hop_networks — csv parsing at config-load time
+# ---------------------------------------------------------------------------
+
+
+def test_invalid_chain_peer_entry_skipped_without_crash(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(
+        settings, "guard_trusted_chain_peers", "not-a-cidr, 172.18.0.1, "
+    )
+    networks = security._build_trusted_hop_networks()
+    assert networks == ("127.0.0.1/32", "::1/128", "172.18.0.1/32")
+
+
+def test_empty_chain_peers_yields_loopback_only() -> None:
+    assert security._build_trusted_hop_networks() == ("127.0.0.1/32", "::1/128")
+
+
+def test_plain_ip_chain_peer_stored_as_its_own_slash_32(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(settings, "guard_trusted_chain_peers", "172.18.0.1")
+    assert security._build_trusted_hop_networks() == (
+        "127.0.0.1/32",
+        "::1/128",
+        "172.18.0.1/32",
+    )
+
+
+def test_subnet_chain_peer_entry_rejected_and_not_in_hop_set(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    # Docker's typical bridge allocation — an entirely plausible operator
+    # copy-paste — must be rejected, not admitted: a range would readmit
+    # every sibling container's real address into the hop set.
+    monkeypatch.setattr(settings, "guard_trusted_chain_peers", "172.16.0.0/12")
+    assert security._build_trusted_hop_networks() == ("127.0.0.1/32", "::1/128")
+    # And the forge chain a wrongly-admitted /12 would have reopened still
+    # abstains end-to-end.
+    _configure_chain_peers(monkeypatch, "172.16.0.0/12")
+    assert resolve_forwarded_client_ip("100.99.1.1, 172.20.0.7") is None
+
+
+def test_host_bits_typo_cidr_rejected_same_as_a_subnet(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    # A typo'd CIDR the operator meant as a plain address — ip_network's
+    # default strict=True would silently accept this; ip_address rejects it.
+    monkeypatch.setattr(settings, "guard_trusted_chain_peers", "172.18.0.5/24")
+    assert security._build_trusted_hop_networks() == ("127.0.0.1/32", "::1/128")
+
+
+# ---------------------------------------------------------------------------
+# _warn_unconfigured_tailnet_gateway_once — the silent-default-gap signal
+# ---------------------------------------------------------------------------
+
+
+def test_unconfigured_gateway_chain_warns_once_per_gateway(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(security, "_WARNED_UNCONFIGURED_GATEWAYS", set())
+    calls: list[tuple[Any, ...]] = []
+    monkeypatch.setattr(
+        security.logger, "warning", lambda *a, **kw: calls.append((a, kw))
+    )
+    for _ in range(3):
+        assert resolve_forwarded_client_ip("100.101.102.103, 172.19.0.9") is None
+    assert len(calls) == 1
+    assert "172.19.0.9" in calls[0][0][0]
+
+
+def test_configured_peer_suppresses_the_gap_warning(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    # A DIFFERENT gateway than the one configured would otherwise trip the
+    # detection, but ANY configured peer means the operator has already
+    # addressed the awareness gap — the warning never fires at all.
+    monkeypatch.setattr(security, "_WARNED_UNCONFIGURED_GATEWAYS", set())
+    _configure_chain_peers(monkeypatch, "172.18.0.1")
+    calls: list[tuple[Any, ...]] = []
+    monkeypatch.setattr(
+        security.logger, "warning", lambda *a, **kw: calls.append((a, kw))
+    )
+    assert resolve_forwarded_client_ip("100.99.1.1, 172.20.0.7") is None
+    assert calls == []
+
+
+def test_non_host_proxied_shapes_never_trigger_the_gap_warning(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setattr(security, "_WARNED_UNCONFIGURED_GATEWAYS", set())
+    calls: list[tuple[Any, ...]] = []
+    monkeypatch.setattr(
+        security.logger, "warning", lambda *a, **kw: calls.append((a, kw))
+    )
+    # A forged public-IP prefix behind a bridge address isn't the tailnet
+    # (CGNAT) shape.
+    resolve_forwarded_client_ip("9.9.9.9, 172.20.0.7")
+    # A LAN client behind a genuine loopback hop resolves — a hop WAS
+    # peeled, so this never reaches the unconfigured-gateway branch.
+    resolve_forwarded_client_ip("203.0.113.9, 127.0.0.1")
+    assert calls == []
+
+
 # ---------------------------------------------------------------------------
 # ClientIpResolutionMiddleware stamping
 # ---------------------------------------------------------------------------