From e2222bd96bc7fad8dc00992d44d678424ff779b7 Mon Sep 17 00:00:00 2001 From: codex Date: Sun, 26 Jul 2026 22:27:51 +0000 Subject: [PATCH] fix(security): prohibit unauthenticated orchestrator --- bot_bottle/orchestrator/__main__.py | 15 +++-- bot_bottle/orchestrator/server.py | 61 ++++++++++--------- bot_bottle/trust_domain.py | 7 +-- .../0079-control-plane-auth-provisioning.md | 3 +- tests/unit/test_orchestrator_server.py | 49 +++++++++------ tests/unit/test_trust_domain.py | 3 +- 6 files changed, 79 insertions(+), 59 deletions(-) diff --git a/bot_bottle/orchestrator/__main__.py b/bot_bottle/orchestrator/__main__.py index 20fd4ec6..529598ab 100644 --- a/bot_bottle/orchestrator/__main__.py +++ b/bot_bottle/orchestrator/__main__.py @@ -1,6 +1,7 @@ """Run the orchestrator control plane as a plain process (PRD 0070 dev-harness). - python -m bot_bottle.orchestrator [--host H] [--port P] [--db PATH] + BOT_BOTTLE_ORCHESTRATOR_TOKEN= \ + python -m bot_bottle.orchestrator [--host H] [--port P] [--db PATH] The PRD sequences the orchestrator as a plain-process dev-harness first, so the consolidation core (registry + attribution + HTTP control plane + live @@ -16,12 +17,13 @@ import secrets from pathlib import Path from .. import log -from .store.store_manager import StoreManager +from ..trust_domain import CONTROL_PLANE from .broker import LaunchBroker, StubBroker -from .server import make_server from .docker_broker import DockerBroker -from .store.registry_store import RegistryStore, default_db_path +from .server import make_server from .service import OrchestratorCore +from .store.store_manager import StoreManager +from .store.registry_store import RegistryStore, default_db_path def main(argv: list[str] | None = None) -> int: @@ -38,6 +40,11 @@ def main(argv: list[str] | None = None) -> int: help="launch broker: 'stub' records requests; 'docker' runs containers", ) args = parser.parse_args(argv) + if not CONTROL_PLANE.key_from_env(): + log.die( + f"{CONTROL_PLANE.key_env} is required; refusing to start the " + "orchestrator without caller authentication" + ) registry = RegistryStore(args.db) registry.migrate() diff --git a/bot_bottle/orchestrator/server.py b/bot_bottle/orchestrator/server.py index bead932b..68347cc7 100644 --- a/bot_bottle/orchestrator/server.py +++ b/bot_bottle/orchestrator/server.py @@ -116,9 +116,8 @@ def dispatch( # pylint: disable=too-many-return-statements,too-many-branches no I/O beyond the orchestrator — so it is fully testable without a socket. `role` is the caller's verified control-plane role (`gateway` or `cli`), or - None for an unauthenticated request; an open-mode server (no signing key - configured — see `OrchestratorServer`) passes `cli`. Every route except - `GET /health` requires a role: a missing role is 401, and a role that + None for an unauthenticated request. Every route except `GET /health` + requires a role: a missing role is 401, and a role that doesn't cover the route is 403 — so a `gateway` data-plane token can reach `/resolve` + `/supervise/{propose,poll}` but not the operator routes (rewrite policy, read injected tokens, approve its own supervise proposals). @@ -414,48 +413,50 @@ class OrchestratorServer(socketserver.ThreadingMixIn, http.server.HTTPServer): Holds the per-host control-plane *signing key* (from `$BOT_BOTTLE_ORCHESTRATOR_TOKEN`, injected by the launcher into the orchestrator process only) and verifies each request's role-scoped token - against it. When a key is set, every route but `/health` requires a valid - token whose role covers the route; when it is unset the server runs **open** - (full `cli` access) and says so loudly at startup — a fail-visible fallback - for tests and any backend that hasn't wired the key yet (e.g. Firecracker, - whose nft boundary already blocks agents from the control-plane port).""" + against it. Every route but `/health` requires a valid token whose role + covers the route. Construction fails when the key is absent so a new or + misconfigured launcher cannot accidentally expose an open control plane.""" daemon_threads = True allow_reuse_address = True - def __init__(self, address: tuple[str, int], orchestrator: OrchestratorCore) -> None: + def __init__( + self, + address: tuple[str, int], + orchestrator: OrchestratorCore, + *, + signing_key: str, + ) -> None: self.orchestrator = orchestrator - # The control-plane trust domain's signing key, as injected into THIS - # (the owning) process by the launcher (#476). Unset → open mode below. - self._signing_key = CONTROL_PLANE.key_from_env() + self._signing_key = signing_key.strip() if not self._signing_key: - sys.stderr.write( - "orchestrator: WARNING — no control-plane signing key " - f"(${CONTROL_PLANE.key_env}); running WITHOUT caller " - "authentication. Any client that can reach this port can drive " - "it. Backends that put the control plane on an agent-reachable " - "network MUST set this.\n" + raise ValueError( + "orchestrator control-plane signing key is required; " + "refusing to start without caller authentication" ) - sys.stderr.flush() super().__init__(address, Handler) def role_for(self, presented: str) -> str | None: - """The role the request is authorized as, or None if unauthenticated. - Open mode (no signing key) grants full `cli` access — the fail-visible - fallback. Otherwise verify the presented signed token; a missing/invalid - token yields None (→ 401), a valid one yields its `gateway`/`cli` - role (→ per-route 401/403 in `dispatch`).""" - if not self._signing_key: - return ROLE_CLI + """The verified caller role, or None for a missing/invalid token.""" return CONTROL_PLANE.verify(presented, self._signing_key) def make_server( - orchestrator: OrchestratorCore, host: str = "127.0.0.1", port: int = 0 + orchestrator: OrchestratorCore, + host: str = "127.0.0.1", + port: int = 0, + *, + signing_key: str | None = None, ) -> OrchestratorServer: - """Build (but do not start) a control-plane server. `port=0` binds an - ephemeral port — read `server.server_address` for the actual one.""" - return OrchestratorServer((host, port), orchestrator) + """Build an authenticated control-plane server. + + ``signing_key=None`` reads the owning process's injected environment. + Empty or missing keys are rejected by :class:`OrchestratorServer`. + """ + key = CONTROL_PLANE.key_from_env() if signing_key is None else signing_key + return OrchestratorServer( + (host, port), orchestrator, signing_key=key, + ) __all__ = [ diff --git a/bot_bottle/trust_domain.py b/bot_bottle/trust_domain.py index 83478b4f..3dc1cf7d 100644 --- a/bot_bottle/trust_domain.py +++ b/bot_bottle/trust_domain.py @@ -40,7 +40,7 @@ from .paths import ( class ProvisioningError(RuntimeError): """A control-plane auth invariant would be violated (e.g. starting the - orchestrator without its signing key — which would run OPEN).""" + orchestrator without its signing key).""" @dataclass(frozen=True) @@ -67,9 +67,8 @@ class TrustDomain: def key_from_env(self, environ: Mapping[str, str] | None = None) -> str: """The signing key as the owning process sees it — read from `key_env` - (default `os.environ`). "" when unset; the caller decides whether that is - fatal (`ControlPlaneProvisioning`) or the open-mode fallback - (`OrchestratorServer`).""" + (default `os.environ`). ``""`` when unset; owning services reject that + value rather than start without authentication.""" env = os.environ if environ is None else environ return env.get(self.key_env, "").strip() diff --git a/docs/prds/0079-control-plane-auth-provisioning.md b/docs/prds/0079-control-plane-auth-provisioning.md index 3d38c549..268b95bd 100644 --- a/docs/prds/0079-control-plane-auth-provisioning.md +++ b/docs/prds/0079-control-plane-auth-provisioning.md @@ -56,8 +56,7 @@ key. - Rewriting the HMAC primitive: `orchestrator_auth.mint/verify` gain an optional `roles=` arg (default unchanged) so a key can carry a different role set; nothing else changes. -- Network topology, the plane split (#469), or the server's open-mode fallback - for tests. +- Network topology or the plane split (#469). ## Design diff --git a/tests/unit/test_orchestrator_server.py b/tests/unit/test_orchestrator_server.py index 50657485..745d71ae 100644 --- a/tests/unit/test_orchestrator_server.py +++ b/tests/unit/test_orchestrator_server.py @@ -255,7 +255,11 @@ class TestServerRoundTrip(unittest.TestCase): tmp = tempfile.TemporaryDirectory() self.addCleanup(tmp.cleanup) orch = _orchestrator(Path(tmp.name) / "r.db") - server = make_server(orch, "127.0.0.1", 0) + signing_key = "round-trip-key" + auth = mint(ROLE_CLI, signing_key) + server = make_server( + orch, "127.0.0.1", 0, signing_key=signing_key, + ) self.addCleanup(server.server_close) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() @@ -267,7 +271,10 @@ class TestServerRoundTrip(unittest.TestCase): reg = json.load(urllib.request.urlopen( urllib.request.Request( f"{base}/bottles", data=_body({"source_ip": "10.243.0.7"}), - method="POST", headers={"Content-Type": "application/json"}, + method="POST", headers={ + "Content-Type": "application/json", + "x-bot-bottle-orchestrator-auth": auth, + }, ), timeout=5, )) self.assertTrue(reg["bottle_id"]) @@ -279,7 +286,10 @@ class TestServerRoundTrip(unittest.TestCase): urllib.request.Request( f"{base}/attribute", data=_body({"source_ip": "10.243.0.7", "identity_token": reg["identity_token"]}), - method="POST", headers={"Content-Type": "application/json"}, + method="POST", headers={ + "Content-Type": "application/json", + "x-bot-bottle-orchestrator-auth": auth, + }, ), timeout=5, )) self.assertEqual(reg["bottle_id"], attr["bottle_id"]) @@ -287,15 +297,25 @@ class TestServerRoundTrip(unittest.TestCase): def test_internal_failure_is_contextual_but_redacted(self) -> None: orch = MagicMock() orch.registry.all.side_effect = RuntimeError("SENSITIVE request value") + signing_key = "failure-path-key" + auth = mint(ROLE_CLI, signing_key) with patch("sys.stderr", io.StringIO()) as stderr: - server = make_server(orch, "127.0.0.1", 0) + server = make_server( + orch, "127.0.0.1", 0, signing_key=signing_key, + ) self.addCleanup(server.server_close) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() self.addCleanup(server.shutdown) host, port = server.server_address[0], server.server_address[1] with self.assertRaises(urllib.error.HTTPError) as raised: - urllib.request.urlopen(f"http://{host}:{port}/bottles", timeout=5) + urllib.request.urlopen( + urllib.request.Request( + f"http://{host}:{port}/bottles", + headers={"x-bot-bottle-orchestrator-auth": auth}, + ), + timeout=5, + ) payload = json.loads(raised.exception.read()) output = stderr.getvalue() self.assertEqual({"error": "internal error"}, payload) @@ -369,8 +389,9 @@ class TestOrchestratorAuth(unittest.TestCase): self.assertIsNotNone(self.orch.registry.get(rec.bottle_id)) def _server_with_key(self, signing_key: str): - with patch.dict("os.environ", {"BOT_BOTTLE_ORCHESTRATOR_TOKEN": signing_key}): - server = make_server(self.orch, "127.0.0.1", 0) + server = make_server( + self.orch, "127.0.0.1", 0, signing_key=signing_key, + ) self.addCleanup(server.server_close) threading.Thread(target=server.serve_forever, daemon=True).start() self.addCleanup(server.shutdown) @@ -399,16 +420,10 @@ class TestOrchestratorAuth(unittest.TestCase): self.assertEqual(403, self._status(f"{base}/bottles", header=gateway_tok)) self.assertEqual(200, self._status(f"{base}/bottles", header=cli_tok)) - def test_unconfigured_server_runs_open(self) -> None: - """No signing key set (tests / nft-protected Firecracker): open mode - grants full cli access, so existing round-trip behavior is unchanged.""" - with patch.dict("os.environ", {}, clear=False): - import os - os.environ.pop("BOT_BOTTLE_ORCHESTRATOR_TOKEN", None) - server = make_server(self.orch, "127.0.0.1", 0) - self.addCleanup(server.server_close) - self.assertEqual(ROLE_CLI, server.role_for("")) - self.assertEqual(ROLE_CLI, server.role_for("anything")) + def test_unconfigured_server_refuses_to_start(self) -> None: + with patch.dict("os.environ", {}, clear=True): + with self.assertRaisesRegex(ValueError, "signing key is required"): + make_server(self.orch, "127.0.0.1", 0) class TestDispatchSupervise(unittest.TestCase): diff --git a/tests/unit/test_trust_domain.py b/tests/unit/test_trust_domain.py index 608388fb..e78192fb 100644 --- a/tests/unit/test_trust_domain.py +++ b/tests/unit/test_trust_domain.py @@ -78,8 +78,7 @@ class TestControlPlaneProvisioning(unittest.TestCase): self.assertEqual("key", prov.orchestrator_key()) def test_orchestrator_key_fail_closes_when_empty(self) -> None: - # Invariant 4: the orchestrator must never start without a key — it would - # run OPEN and grant every caller that reaches it full `cli`. There is no + # Invariant 4: the orchestrator must never start without a key. There is no # topology opt-out: a separate host does not stop the gateway (or any # other caller) from reaching the control-plane listener. prov = ControlPlaneProvisioning()