fix(security): authenticate the orchestrator control plane (issue #400 Critical)
The control plane had no caller authentication — dispatch() routed every request straight through, and the only guards anywhere were the (source_ip, identity_token) checks inside /resolve and /attribute, which authenticate the *bottle* a request is about, not the *caller*. On the consolidated backends the agent shares a network with the control-plane port (macOS: the infra container runs 8099 on the agent's --internal network; docker: the orchestrator container sits on the shared gateway network), so a prompt-injected agent could GET /bottles (enumerate siblings + policies), PUT /bottles/<id>/policy (rewrite its own egress allowlist to allow-all), POST /resolve (read the injected upstream credentials it's never meant to see), and POST /supervise/respond (self-approve its own proposals). Every README guarantee collapsed to the agent's discretion. Fix: a per-host control-plane secret required on every route but GET /health, compared with hmac.compare_digest. It is held only by the trusted callers and never handed to an agent: - minted + persisted 0600 at <root>/control-plane-token (paths.host_control_plane_token); - injected as $BOT_BOTTLE_CONTROL_PLANE_TOKEN into the orchestrator + gateway containers via bare `--env NAME` (value inherited from the launch process, so it never lands on argv or in `container/docker inspect`); - presented by the gateway's PolicyResolver (reads the env) on /resolve, and by the host CLI's OrchestratorClient (reads the host file) on every call. The agent container is never given the env var or the host file, so from a bottle every /bottles*, /resolve, /attribute, and /supervise/* call now returns 401 — closing the enumeration, allowlist-rewrite, credential-lift, and self-approval. The existing (source_ip, identity_token) checks stay as defense-in-depth. Enforced when configured: macOS + docker inject the secret (→ enforced). With no secret set the server runs open and warns loudly at startup — a fail-visible fallback for the unit suite and for Firecracker, whose port-scoped nft already blocks agents from 8099 (wiring the secret into its infra-VM init is a clean fast-follow, left out here to avoid churning the prebuilt-artifact hash). Verified end-to-end on real Apple Container: infra comes up healthy, the host CLI (with the secret) lists bottles while an unauthenticated GET /bottles gets 401, all five issue-#400 attacks from inside the agent get 401, and egress policy still works (200 allowed / 403 denied) — proving the gateway authenticates to /resolve with the secret. 1829 unit tests pass, pyright clean, pylint 9.91. Refs #400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -11,6 +11,7 @@ import secrets
|
||||
import tempfile
|
||||
import threading
|
||||
import unittest
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
@@ -243,6 +244,82 @@ class TestServerRoundTrip(unittest.TestCase):
|
||||
self.assertEqual(reg["bottle_id"], attr["bottle_id"])
|
||||
|
||||
|
||||
class TestControlPlaneAuth(unittest.TestCase):
|
||||
"""The per-host control-plane secret (issue #400): every route but /health
|
||||
is a trusted-caller op an agent must not be able to drive just because it
|
||||
can reach the port."""
|
||||
|
||||
def setUp(self) -> None:
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self._tmp.cleanup)
|
||||
self.orch = _orchestrator(Path(self._tmp.name) / "r.db")
|
||||
|
||||
def test_health_is_public_even_unauthorized(self) -> None:
|
||||
status, _ = dispatch(self.orch, "GET", "/health", b"", authorized=False)
|
||||
self.assertEqual(200, status)
|
||||
|
||||
def test_unauthorized_denies_every_other_route(self) -> None:
|
||||
for method, path, body in [
|
||||
("GET", "/bottles", b""),
|
||||
("POST", "/bottles", _body({"source_ip": "10.0.0.1"})),
|
||||
("PUT", "/bottles/x/policy", _body({"policy": "routes: []"})),
|
||||
("DELETE", "/bottles/x", b""),
|
||||
("POST", "/resolve", _body({"source_ip": "10.0.0.1", "identity_token": "t"})),
|
||||
("POST", "/attribute", _body({"source_ip": "10.0.0.1", "identity_token": "t"})),
|
||||
("GET", "/supervise/proposals", b""),
|
||||
("POST", "/supervise/respond", _body({"proposal_id": "p", "bottle_slug": "s", "decision": "approve"})),
|
||||
]:
|
||||
status, _ = dispatch(self.orch, method, path, body, authorized=False)
|
||||
self.assertEqual(401, status, f"{method} {path} should be 401 unauthorized")
|
||||
|
||||
def test_deny_happens_before_the_registry_is_touched(self) -> None:
|
||||
"""An unauthorized DELETE must not tear a bottle down. 401, and the
|
||||
bottle is still there."""
|
||||
rec = self.orch.registry.register("10.0.0.9", policy="", metadata="")
|
||||
status, _ = dispatch(
|
||||
self.orch, "DELETE", f"/bottles/{rec.bottle_id}", b"", authorized=False)
|
||||
self.assertEqual(401, status)
|
||||
self.assertIsNotNone(self.orch.registry.get(rec.bottle_id))
|
||||
|
||||
def _server_with_secret(self, secret: str):
|
||||
with patch.dict("os.environ", {"BOT_BOTTLE_CONTROL_PLANE_TOKEN": secret}):
|
||||
server = make_server(self.orch, "127.0.0.1", 0)
|
||||
self.addCleanup(server.server_close)
|
||||
threading.Thread(target=server.serve_forever, daemon=True).start()
|
||||
self.addCleanup(server.shutdown)
|
||||
host, port = server.server_address[0], server.server_address[1]
|
||||
return f"http://{host}:{port}"
|
||||
|
||||
def _status(self, url: str, *, header: str | None = None) -> int:
|
||||
req = urllib.request.Request(url)
|
||||
if header is not None:
|
||||
req.add_header("x-bot-bottle-control-auth", header)
|
||||
try:
|
||||
return urllib.request.urlopen(req, timeout=5).status
|
||||
except urllib.error.HTTPError as e:
|
||||
return e.code
|
||||
|
||||
def test_configured_server_enforces_the_header_over_http(self) -> None:
|
||||
base = self._server_with_secret("s3cret-admin")
|
||||
# /health is public — no header needed.
|
||||
self.assertEqual(200, self._status(f"{base}/health"))
|
||||
# /bottles requires the secret.
|
||||
self.assertEqual(401, self._status(f"{base}/bottles"))
|
||||
self.assertEqual(401, self._status(f"{base}/bottles", header="wrong"))
|
||||
self.assertEqual(200, self._status(f"{base}/bottles", header="s3cret-admin"))
|
||||
|
||||
def test_unconfigured_server_runs_open(self) -> None:
|
||||
"""No secret set (tests / nft-protected Firecracker): open mode, so the
|
||||
existing round-trip and unit behavior are unchanged."""
|
||||
with patch.dict("os.environ", {}, clear=False):
|
||||
import os
|
||||
os.environ.pop("BOT_BOTTLE_CONTROL_PLANE_TOKEN", None)
|
||||
server = make_server(self.orch, "127.0.0.1", 0)
|
||||
self.addCleanup(server.server_close)
|
||||
self.assertTrue(server.is_authorized(""))
|
||||
self.assertTrue(server.is_authorized("anything"))
|
||||
|
||||
|
||||
class TestDispatchSupervise(unittest.TestCase):
|
||||
"""The /supervise/* routes over the pure dispatch()."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user