From 1db2a9eb671a60adb1d6773e5437de7396bfdba0 Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 24 Jul 2026 05:18:39 +0000 Subject: [PATCH] feat(control-plane): role-scoped signed tokens so the gateway can't drive operator routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #469: the data plane held the same control-plane secret that authorizes every route, so a compromised egress/git-gate could queue a supervise proposal AND approve it (or rewrite policy, read injected tokens) — the (source_ip, identity_token) checks attribute the *bottle*, not the caller. Replace the single shared bearer secret with role-scoped, HMAC-signed tokens (compact HS256 JWTs, stdlib-only — no new dependency): * new `control_auth` mints/verifies `{role}` tokens; roles are `gateway` (data plane) and `cli` (host operator/launcher). * the orchestrator holds only the signing *key* and verifies; `dispatch` gates each route by role — `gateway` reaches /resolve + /supervise/ {propose,poll}, everything else is `cli`-only (401 unauthenticated, 403 wrong role). * the gateway is handed a pre-minted `gateway` token it cannot rewrite into `cli`; the host CLI mints its own `cli` token from the host key. * `gateway_init` scopes the signing key to the orchestrator process and the gateway token to the data-plane daemons, so even in the combined infra container a compromised data-plane daemon never sees the key. Launchers (docker gateway + infra, macOS infra) inject the minted token(s); Firecracker stays open behind its nft boundary. Open mode (no key) still grants full `cli` access — the fail-visible fallback for tests. Co-Authored-By: Claude Opus 4.8 --- bot_bottle/backend/macos_container/infra.py | 24 +++-- bot_bottle/control_auth.py | 100 +++++++++++++++++ bot_bottle/gateway_init.py | 37 +++++-- bot_bottle/orchestrator/client.py | 13 ++- bot_bottle/orchestrator/control_plane.py | 101 +++++++++++------- bot_bottle/orchestrator/gateway.py | 16 +-- bot_bottle/orchestrator/lifecycle.py | 16 ++- bot_bottle/paths.py | 10 ++ bot_bottle/policy_resolver.py | 19 ++-- ..._orchestrator_docker_control_plane_auth.py | 24 ++++- tests/unit/test_control_auth.py | 86 +++++++++++++++ tests/unit/test_gateway_init.py | 24 +++++ tests/unit/test_orchestrator_client.py | 15 +++ tests/unit/test_orchestrator_control_plane.py | 91 ++++++++++------ tests/unit/test_policy_resolver.py | 20 +++- 15 files changed, 485 insertions(+), 111 deletions(-) create mode 100644 bot_bottle/control_auth.py create mode 100644 tests/unit/test_control_auth.py diff --git a/bot_bottle/backend/macos_container/infra.py b/bot_bottle/backend/macos_container/infra.py index cb92540..731a62a 100644 --- a/bot_bottle/backend/macos_container/infra.py +++ b/bot_bottle/backend/macos_container/infra.py @@ -48,7 +48,9 @@ from ...orchestrator.lifecycle import ( OrchestratorStartError, source_hash, ) +from ...control_auth import ROLE_GATEWAY, mint from ...paths import ( + CONTROL_AUTH_JWT_ENV, CONTROL_PLANE_TOKEN_ENV, HOST_DB_FILENAME, host_control_plane_token, @@ -237,18 +239,26 @@ class MacosInfraService: # Baked onto the container so `_source_current` can detect a real # control-plane code change and recreate. "--env", f"BOT_BOTTLE_SOURCE_HASH={current_hash}", - # The control-plane secret, for BOTH the control plane (to require - # it) and the gateway's PolicyResolver (to present it) — they share - # this one container. Bare `--env NAME` inherits the value from the - # run process below, so the secret never lands on argv or in - # `container inspect`'s command line. The agent runs in a SEPARATE - # container that is never given this var, which is the whole point. + # The control-plane signing key (control plane: verifies tokens) and + # the pre-minted `gateway` JWT (the gateway's PolicyResolver: presents + # it) — they share this one container, and gateway_init scopes each to + # its process so a compromised data-plane daemon never sees the key + # (issue #469 review). Bare `--env NAME` inherits the value from the + # run process below, so neither lands on argv or in `container + # inspect`'s command line. The agent runs in a SEPARATE container that + # is never given these vars, which is the whole point. "--env", CONTROL_PLANE_TOKEN_ENV, + "--env", CONTROL_AUTH_JWT_ENV, "--entrypoint", "sh", self.image, "-c", _init_script(self.port), ] - run_env = {**os.environ, CONTROL_PLANE_TOKEN_ENV: host_control_plane_token()} + _signing_key = host_control_plane_token() + run_env = { + **os.environ, + CONTROL_PLANE_TOKEN_ENV: _signing_key, + CONTROL_AUTH_JWT_ENV: mint(ROLE_GATEWAY, _signing_key), + } result = container_mod.run_container_argv(argv, env=run_env) if result.returncode != 0: raise OrchestratorStartError( diff --git a/bot_bottle/control_auth.py b/bot_bottle/control_auth.py new file mode 100644 index 0000000..23cf351 --- /dev/null +++ b/bot_bottle/control_auth.py @@ -0,0 +1,100 @@ +"""Role-scoped control-plane credentials (issue #469 review follow-up). + +The control plane no longer trusts a single shared bearer secret for every +route. Instead the orchestrator holds a *signing key* and issues short, +HMAC-signed tokens (compact HS256 JWTs) that embed a **role** naming the kind +of caller: + + * ``gateway`` — the data plane (egress / git-gate / supervise). Restricted to + the agent-facing routes it actually needs (``/resolve``, + ``/supervise/propose``, ``/supervise/poll``). + * ``cli`` — the host operator / launcher. Full access to the mutating and + operator routes (launch/teardown, policy, ``/supervise/respond``, …). + +Only the orchestrator (and the host CLI, which shares the host trust domain) +holds the signing key; the gateway is handed a pre-minted ``gateway`` token it +cannot rewrite into a ``cli`` token. So a compromised data-plane process can no +longer approve its own supervise proposals or drive operator routes — it can +only present the ``gateway`` role it was issued (control_plane rejects it on +operator routes with 403). + +Stdlib-only (HMAC-SHA256 over a JSON payload); no JWT dependency — the project +carries no runtime pip deps. Tokens are **signed, not encrypted** (the role is +not a secret) and **long-lived** (parity with the static token they replace; +the security win is the unforgeable role claim, not rotation). +""" + +from __future__ import annotations + +import base64 +import binascii +import hashlib +import hmac +import json + +ROLE_GATEWAY = "gateway" +ROLE_CLI = "cli" +ROLES: frozenset[str] = frozenset({ROLE_GATEWAY, ROLE_CLI}) + +_ALG = "HS256" + + +def _b64url_encode(raw: bytes) -> str: + return base64.urlsafe_b64encode(raw).rstrip(b"=").decode("ascii") + + +def _b64url_decode(text: str) -> bytes: + padded = text + "=" * (-len(text) % 4) + return base64.urlsafe_b64decode(padded.encode("ascii")) + + +def _sign(secret: str, signing_input: str) -> str: + mac = hmac.new(secret.encode("utf-8"), signing_input.encode("ascii"), hashlib.sha256) + return _b64url_encode(mac.digest()) + + +# The fixed, canonical JOSE header — the same for every token we mint. +_HEADER_SEGMENT = _b64url_encode( + json.dumps({"alg": _ALG, "typ": "JWT"}, separators=(",", ":")).encode("utf-8") +) + + +def mint(role: str, secret: str) -> str: + """A compact HS256 token asserting `role`, signed with `secret`. + + Raises ValueError for an unknown role (mint only what the control plane will + accept) or an empty signing key (an unsigned credential is never valid).""" + if role not in ROLES: + raise ValueError(f"unknown control-plane role {role!r}") + if not secret: + raise ValueError("cannot mint a control-plane token without a signing key") + payload = _b64url_encode(json.dumps({"role": role}, separators=(",", ":")).encode("utf-8")) + signing_input = f"{_HEADER_SEGMENT}.{payload}" + return f"{signing_input}.{_sign(secret, signing_input)}" + + +def verify(token: str, secret: str) -> str | None: + """The role a valid `token` carries, or None if it is malformed, wrongly + signed, or names an unknown role. Constant-time signature check; rejects any + header whose alg isn't HS256 (no alg-confusion / `none`).""" + if not token or not secret: + return None + parts = token.split(".") + if len(parts) != 3: + return None + header_b64, payload_b64, sig = parts + expected = _sign(secret, f"{header_b64}.{payload_b64}") + if not hmac.compare_digest(sig, expected): + return None + try: + header = json.loads(_b64url_decode(header_b64)) + payload = json.loads(_b64url_decode(payload_b64)) + except (ValueError, binascii.Error): + return None + if not isinstance(header, dict) or header.get("alg") != _ALG: + return None + role = payload.get("role") if isinstance(payload, dict) else None + return role if isinstance(role, str) and role in ROLES else None + + +__all__ = ["ROLE_GATEWAY", "ROLE_CLI", "ROLES", "mint", "verify"] diff --git a/bot_bottle/gateway_init.py b/bot_bottle/gateway_init.py index 0f24b26..7e9df7e 100644 --- a/bot_bottle/gateway_init.py +++ b/bot_bottle/gateway_init.py @@ -54,6 +54,15 @@ class _DaemonSpec: _EGRESS_ONLY_ENV_PREFIXES: tuple[str, ...] = ("EGRESS_TOKEN_",) _READY_GATED_DAEMONS: tuple[str, ...] = ("git-gate", "git-http") +# The control-plane signing key is the orchestrator's alone — verifying tokens. +# The data-plane daemons instead hold the pre-minted `gateway` JWT they present. +# Scoping each to its process (even in the combined infra container) keeps a +# compromised data-plane daemon from reading the key and minting a `cli` token +# (issue #469 review). Values match paths.CONTROL_PLANE_TOKEN_ENV / +# CONTROL_AUTH_JWT_ENV; hardcoded here so this supervisor stays import-light. +_SIGNING_KEY_ENV = "BOT_BOTTLE_CONTROL_PLANE_TOKEN" +_GATEWAY_JWT_ENV = "BOT_BOTTLE_CONTROL_AUTH_JWT" + # Daemons that must be requested explicitly via BOT_BOTTLE_GATEWAY_DAEMONS # and are NOT started in the default (env-var-unset) case. The orchestrator # only runs in the combined infra container, never in a standalone gateway. @@ -61,16 +70,24 @@ _OPT_IN_DAEMONS: frozenset[str] = frozenset({"orchestrator"}) def _env_for_daemon(name: str, base_env: dict[str, str]) -> dict[str, str]: - """Egress sees the full bundle env. Everyone else gets a copy - with `EGRESS_TOKEN_*` (and any other future egress-only - credential slots) stripped. Returns a fresh dict — callers - can mutate without affecting `base_env`.""" - if name == "egress": - return dict(base_env) - return { - k: v for k, v in base_env.items() - if not any(k.startswith(p) for p in _EGRESS_ONLY_ENV_PREFIXES) - } + """Per-daemon env, scoped to what each process legitimately needs. + Returns a fresh dict — callers can mutate without affecting `base_env`. + + * `EGRESS_TOKEN_*` upstream-auth slots go to egress only. + * the control-plane signing key goes to the orchestrator only. + * the pre-minted `gateway` JWT goes to the data-plane daemons only (the + orchestrator verifies tokens, it never presents one).""" + env = dict(base_env) + if name != "egress": + env = { + k: v for k, v in env.items() + if not any(k.startswith(p) for p in _EGRESS_ONLY_ENV_PREFIXES) + } + if name == "orchestrator": + env.pop(_GATEWAY_JWT_ENV, None) + else: + env.pop(_SIGNING_KEY_ENV, None) + return env # The orchestrator is listed first so it starts before the gateway daemons, diff --git a/bot_bottle/orchestrator/client.py b/bot_bottle/orchestrator/client.py index b3128bf..a349c64 100644 --- a/bot_bottle/orchestrator/client.py +++ b/bot_bottle/orchestrator/client.py @@ -18,6 +18,7 @@ import urllib.request from collections.abc import Iterable from dataclasses import dataclass +from ..control_auth import ROLE_CLI, mint from ..paths import host_control_plane_token from .control_plane import CONTROL_AUTH_HEADER @@ -25,12 +26,14 @@ DEFAULT_TIMEOUT_SECONDS = 5.0 def _host_auth_token() -> str: - """The per-host control-plane secret, or "" if it can't be read. "" means - 'send no auth header' — correct against an open (unconfigured) control - plane, and harmlessly rejected by a secured one.""" + """A freshly minted `cli`-role token, signed with the host control-plane + key, or "" if the key can't be read. The host CLI shares the orchestrator's + trust domain, so it holds the signing key and mints its own operator token; + "" means 'send no auth header' — correct against an open (unconfigured) + control plane, and harmlessly rejected by a secured one.""" try: - return host_control_plane_token() - except OSError: + return mint(ROLE_CLI, host_control_plane_token()) + except (OSError, ValueError): return "" diff --git a/bot_bottle/orchestrator/control_plane.py b/bot_bottle/orchestrator/control_plane.py index 87d7714..add9420 100644 --- a/bot_bottle/orchestrator/control_plane.py +++ b/bot_bottle/orchestrator/control_plane.py @@ -53,7 +53,6 @@ returned only once, to the caller that launches the bottle. from __future__ import annotations -import hmac import http.server import json import os @@ -62,6 +61,7 @@ import sys import typing from urllib.parse import urlsplit +from ..control_auth import ROLE_CLI, ROLES, verify from ..paths import CONTROL_PLANE_TOKEN_ENV from ..supervise_types import TOOLS from .service import Orchestrator @@ -69,14 +69,32 @@ from .service import Orchestrator # JSON body payload type (parsed request / rendered response). Json = dict[str, object] -# The request header carrying the per-host control-plane secret. Every route -# except `GET /health` requires it (see `dispatch`). The trusted callers hold -# the secret (the gateway's PolicyResolver, the host CLI's OrchestratorClient); -# an agent that can merely *reach* the port cannot present it, so it can't -# enumerate bottles, rewrite policy, read injected upstream tokens, or approve -# its own supervise proposals. +# The request header carrying the caller's role-scoped control-plane token (a +# signed JWT naming the caller's role — see control_auth). The role gates which +# routes the caller may reach: the data plane holds a `gateway` token good only +# for the agent-facing lookups; the host CLI holds a `cli` token for the +# operator/mutating routes. An agent that can merely *reach* the port holds no +# token at all, and a compromised gateway holds only `gateway` — neither can +# drive the operator routes (approve proposals, rewrite policy, read tokens). CONTROL_AUTH_HEADER = "x-bot-bottle-control-auth" +# The routes the data plane (role `gateway`) is allowed to reach — exactly the +# per-request lookups PolicyResolver makes. Every other authenticated route is +# operator-only. `cli` is a superset role: it may reach any route. +_GATEWAY_ROUTES: frozenset[tuple[str, str]] = frozenset({ + ("POST", "/resolve"), + ("POST", "/supervise/propose"), + ("POST", "/supervise/poll"), +}) + + +def _allowed_roles(method: str, route: str) -> frozenset[str]: + """The roles permitted on `(method, route)`: `gateway` or `cli` on the + data-plane routes, `cli`-only everywhere else.""" + if (method, route) in _GATEWAY_ROUTES: + return ROLES + return frozenset({ROLE_CLI}) + def _parse_json_object(body: bytes) -> Json: """Parse a JSON object body. Raises ValueError for non-objects / bad JSON.""" @@ -89,28 +107,33 @@ def _parse_json_object(body: bytes) -> Json: def dispatch( # pylint: disable=too-many-return-statements,too-many-branches - orch: Orchestrator, method: str, path: str, body: bytes, *, authorized: bool = True, + orch: Orchestrator, method: str, path: str, body: bytes, *, role: str | None = ROLE_CLI, ) -> tuple[int, Json]: """Route one control-plane request to a (status, payload) pair. Pure — no I/O beyond the orchestrator — so it is fully testable without a socket. - `authorized` is whether the request presented the control-plane secret (or - no secret is configured — see `ControlPlaneServer`). Every route except - `GET /health` requires it: the source-IP + identity-token checks inside - `/resolve` and `/attribute` authenticate the *bottle* a request is about, - not the *caller*, so without this gate any agent that can reach the port - could rewrite another bottle's policy, read the injected upstream tokens, - or approve its own supervise proposals. Defaults True so unit tests of the - routing logic don't have to thread it through.""" + `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 `ControlPlaneServer`) passes `cli`. 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). + The source-IP + identity-token checks inside `/resolve` and `/attribute` + authenticate the *bottle* a request is about, not the *caller*, so this role + gate is what protects the caller-privileged routes. Defaults `cli` so unit + tests of the routing logic don't have to thread it through.""" route = urlsplit(path).path.rstrip("/") or "/" if method == "GET" and route == "/health": return 200, {"status": "ok"} - if not authorized: - # Everything below is a trusted-caller operation. Deny before touching - # the registry / broker / supervise store. + # Role gate — every route below is a trusted-caller operation. Deny before + # touching the registry / broker / supervise store. + if role is None: return 401, {"error": "control-plane authentication required"} + if role not in _allowed_roles(method, route): + return 403, {"error": "insufficient role for this route"} if method == "GET" and route == "/gateway": return 200, orch.gateway_status() @@ -343,10 +366,10 @@ class Handler(http.server.BaseHTTPRequestHandler): assert isinstance(server, ControlPlaneServer) length = int(self.headers.get("Content-Length") or 0) body = self.rfile.read(length) if length > 0 else b"" - authorized = server.is_authorized(self.headers.get(CONTROL_AUTH_HEADER, "")) + role = server.role_for(self.headers.get(CONTROL_AUTH_HEADER, "")) try: status, payload = dispatch( - server.orchestrator, method, self.path, body, authorized=authorized) + server.orchestrator, method, self.path, body, role=role) except Exception as e: # noqa: BLE001 — the control plane must stay up sys.stderr.write(f"orchestrator: {method} {self.path} failed: {e!r}\n") sys.stderr.flush() @@ -374,22 +397,24 @@ class Handler(http.server.BaseHTTPRequestHandler): class ControlPlaneServer(socketserver.ThreadingMixIn, http.server.HTTPServer): """Threading HTTP server that carries the orchestrator for its handlers. - Holds the per-host control-plane secret (from `$BOT_BOTTLE_CONTROL_PLANE_TOKEN`, - injected by the launcher into this container only). When a secret is set, - every route but `/health` requires it; when it is unset the server runs - **open** and says so loudly at startup — a fail-visible fallback for tests - and any backend that hasn't wired the secret yet (e.g. Firecracker, whose - nft boundary already blocks agents from the control-plane port).""" + Holds the per-host control-plane *signing key* (from + `$BOT_BOTTLE_CONTROL_PLANE_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).""" daemon_threads = True allow_reuse_address = True def __init__(self, address: tuple[str, int], orchestrator: Orchestrator) -> None: self.orchestrator = orchestrator - self._auth_token = os.environ.get(CONTROL_PLANE_TOKEN_ENV, "").strip() - if not self._auth_token: + self._signing_key = os.environ.get(CONTROL_PLANE_TOKEN_ENV, "").strip() + if not self._signing_key: sys.stderr.write( - "orchestrator: WARNING — no control-plane secret " + "orchestrator: WARNING — no control-plane signing key " f"(${CONTROL_PLANE_TOKEN_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 " @@ -398,13 +423,15 @@ class ControlPlaneServer(socketserver.ThreadingMixIn, http.server.HTTPServer): sys.stderr.flush() super().__init__(address, Handler) - def is_authorized(self, presented: str) -> bool: - """True iff the request may proceed past `/health`: either no secret is - configured (open mode) or the presented header matches it. Constant-time - compare so a wrong token leaks nothing timing-wise.""" - if not self._auth_token: - return True - return hmac.compare_digest(presented, self._auth_token) + 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 + return verify(presented, self._signing_key) def make_server( diff --git a/bot_bottle/orchestrator/gateway.py b/bot_bottle/orchestrator/gateway.py index 16bf1e6..cc12330 100644 --- a/bot_bottle/orchestrator/gateway.py +++ b/bot_bottle/orchestrator/gateway.py @@ -22,9 +22,10 @@ import os import time from pathlib import Path +from ..control_auth import ROLE_GATEWAY, mint from ..docker_cmd import run_docker from ..paths import ( - CONTROL_PLANE_TOKEN_ENV, + CONTROL_AUTH_JWT_ENV, host_control_plane_token, host_gateway_ca_dir, ) @@ -252,11 +253,14 @@ class DockerGateway(Gateway): # policy against the control plane per request (guaranteed non-empty by # the check above). argv += ["--env", f"BOT_BOTTLE_ORCHESTRATOR_URL={self._orchestrator_url}"] - # ...and present the control-plane secret on those /resolve calls (the - # control plane requires it). Bare `--env NAME` keeps the value off argv - # / `docker inspect`; only the gateway (not the agent) is given it. - argv += ["--env", CONTROL_PLANE_TOKEN_ENV] - run_env[CONTROL_PLANE_TOKEN_ENV] = host_control_plane_token() + # ...presenting a role-scoped `gateway` token (a signed JWT minted from + # the host signing key) on those calls. The gateway never receives the + # signing key — only this pre-minted token, which it cannot rewrite into + # a `cli` token, so a compromised data-plane process can't drive the + # operator routes (issue #469 review). Bare `--env NAME` keeps the value + # off argv / `docker inspect`; only the gateway (not the agent) is given it. + argv += ["--env", CONTROL_AUTH_JWT_ENV] + run_env[CONTROL_AUTH_JWT_ENV] = mint(ROLE_GATEWAY, host_control_plane_token()) argv.append(self.image_ref) proc = run_docker(argv, env=run_env) if proc.returncode != 0: diff --git a/bot_bottle/orchestrator/lifecycle.py b/bot_bottle/orchestrator/lifecycle.py index 9560c07..b75e87d 100644 --- a/bot_bottle/orchestrator/lifecycle.py +++ b/bot_bottle/orchestrator/lifecycle.py @@ -22,8 +22,10 @@ import urllib.request from pathlib import Path from .. import log +from ..control_auth import ROLE_GATEWAY, mint from ..docker_cmd import run_docker from ..paths import ( + CONTROL_AUTH_JWT_ENV, CONTROL_PLANE_TOKEN_ENV, bot_bottle_root, host_control_plane_token, @@ -186,6 +188,7 @@ class OrchestratorService: so a later `ensure_running` can detect a real code change.""" self._ensure_network() run_docker(["docker", "rm", "--force", self._infra_name]) + _signing_key = host_control_plane_token() proc = run_docker([ "docker", "run", "--detach", "--name", self._infra_name, @@ -209,16 +212,23 @@ class OrchestratorService: # Orchestrator registry DB on the host (sole writer: control plane). "--volume", f"{self._host_root}:{_ROOT_IN_CONTAINER}", "--env", f"BOT_BOTTLE_ROOT={_ROOT_IN_CONTAINER}", - # Control-plane secret: required by the orchestrator (to enforce) - # and by the gateway daemons (to present on /resolve calls). + # Control-plane signing key (orchestrator: verifies tokens) + the + # pre-minted `gateway` JWT (data-plane daemons: present it). gateway_init + # scopes each to its process, so a compromised data-plane daemon never + # sees the key and can't mint a `cli` token (issue #469 review). "--env", CONTROL_PLANE_TOKEN_ENV, + "--env", CONTROL_AUTH_JWT_ENV, # Gateway daemons reach the orchestrator over loopback at its # fixed internal port (DEFAULT_PORT), independent of self.port. "--env", f"BOT_BOTTLE_ORCHESTRATOR_URL=http://127.0.0.1:{DEFAULT_PORT}", # Opt the orchestrator into gateway_init's supervise tree. "--env", f"BOT_BOTTLE_GATEWAY_DAEMONS={_INFRA_DAEMONS}", self.image, - ], env={**os.environ, CONTROL_PLANE_TOKEN_ENV: host_control_plane_token()}) + ], env={ + **os.environ, + CONTROL_PLANE_TOKEN_ENV: _signing_key, + CONTROL_AUTH_JWT_ENV: mint(ROLE_GATEWAY, _signing_key), + }) if proc.returncode != 0: raise OrchestratorStartError( f"infra container failed to start: {proc.stderr.strip()}" diff --git a/bot_bottle/paths.py b/bot_bottle/paths.py index a64179c..629edb5 100644 --- a/bot_bottle/paths.py +++ b/bot_bottle/paths.py @@ -37,7 +37,16 @@ HOST_DB_FILENAME = "bot-bottle.db" # trusted callers (control plane, gateway, host CLI) and never handed to an # agent, so an agent that can reach the control-plane port still can't drive it. CONTROL_PLANE_TOKEN_FILENAME = "control-plane-token" +# The env var carrying the control-plane *signing key* — held only by the +# orchestrator (to verify tokens) and the host CLI (to mint its own), never by +# the data plane. Same value as the host token file; the name is unchanged for +# backward compatibility with existing launchers. CONTROL_PLANE_TOKEN_ENV = "BOT_BOTTLE_CONTROL_PLANE_TOKEN" +# The env var carrying the data plane's pre-minted `gateway`-role token (a +# signed JWT the launcher mints from the signing key). The gateway presents this +# on /resolve + /supervise/{propose,poll}; it never holds the signing key, so it +# cannot forge a higher-privilege `cli` token (issue #469 review). +CONTROL_AUTH_JWT_ENV = "BOT_BOTTLE_CONTROL_AUTH_JWT" # The host directory holding the gateway's persistent mitmproxy CA. Bind-mounted # into the infra/gateway container at mitmproxy's confdir so the self-generated @@ -124,6 +133,7 @@ __all__ = [ "HOST_DB_FILENAME", "CONTROL_PLANE_TOKEN_FILENAME", "CONTROL_PLANE_TOKEN_ENV", + "CONTROL_AUTH_JWT_ENV", "GATEWAY_CA_DIRNAME", "bot_bottle_root", "host_db_path", diff --git a/bot_bottle/policy_resolver.py b/bot_bottle/policy_resolver.py index 5a5dd15..9298a12 100644 --- a/bot_bottle/policy_resolver.py +++ b/bot_bottle/policy_resolver.py @@ -34,21 +34,22 @@ import urllib.request DEFAULT_TIMEOUT_SECONDS = 2.0 -# The control-plane secret this gateway presents on every /resolve call, read -# from the env the launcher injects into the gateway container. The control -# plane requires it (orchestrator/control_plane.py). Constant + env-var name are -# duplicated here rather than imported because this module is COPYed flat into -# the gateway image, free of bot-bottle imports — same rationale as -# IDENTITY_HEADER in egress_addon / git_http_backend. +# The role-scoped `gateway` token this data plane presents on every control-plane +# call, read from the env the launcher injects into the gateway (a pre-minted +# signed JWT — the gateway never holds the signing key, so it can't forge a +# higher-privilege `cli` token). Constant + env-var name are duplicated here +# rather than imported because this module is COPYed flat into the gateway image, +# free of bot-bottle imports — same rationale as IDENTITY_HEADER in egress_addon +# / git_http_backend. CONTROL_AUTH_HEADER = "x-bot-bottle-control-auth" -CONTROL_PLANE_TOKEN_ENV = "BOT_BOTTLE_CONTROL_PLANE_TOKEN" +CONTROL_AUTH_JWT_ENV = "BOT_BOTTLE_CONTROL_AUTH_JWT" def _control_auth_headers() -> dict[str, str]: - """The auth header to send, or {} when no secret is configured (an open + """The auth header to send, or {} when no token is configured (an open control plane, e.g. Firecracker behind its nft boundary — sending nothing is correct there and harmlessly ignored).""" - token = os.environ.get(CONTROL_PLANE_TOKEN_ENV, "").strip() + token = os.environ.get(CONTROL_AUTH_JWT_ENV, "").strip() return {CONTROL_AUTH_HEADER: token} if token else {} diff --git a/tests/integration/test_orchestrator_docker_control_plane_auth.py b/tests/integration/test_orchestrator_docker_control_plane_auth.py index 2c66887..aedb8ca 100644 --- a/tests/integration/test_orchestrator_docker_control_plane_auth.py +++ b/tests/integration/test_orchestrator_docker_control_plane_auth.py @@ -25,6 +25,7 @@ import tempfile import unittest from pathlib import Path +from bot_bottle.control_auth import ROLE_CLI, ROLE_GATEWAY, mint from bot_bottle.orchestrator.client import OrchestratorClient from bot_bottle.orchestrator.lifecycle import OrchestratorService from bot_bottle.paths import host_control_plane_token @@ -85,7 +86,11 @@ class TestDockerControlPlaneAuthIntegration(unittest.TestCase): host_root=host_root, ) cls.svc.ensure_running() - cls.token = host_control_plane_token() + # The control plane now verifies role-scoped signed tokens, not the raw + # key. Mint one of each role from the host signing key (issue #469 review). + signing_key = host_control_plane_token() + cls.cli_token = mint(ROLE_CLI, signing_key) + cls.gateway_token = mint(ROLE_GATEWAY, signing_key) @staticmethod def _teardown_docker( @@ -136,11 +141,17 @@ class TestDockerControlPlaneAuthIntegration(unittest.TestCase): status, _ = self._request("GET", "/bottles", token="not-the-real-secret") self.assertEqual(401, status) - def test_bottles_accepts_the_real_token(self) -> None: - status, payload = self._request("GET", "/bottles", token=self.token) + def test_bottles_accepts_the_cli_token(self) -> None: + status, payload = self._request("GET", "/bottles", token=self.cli_token) self.assertEqual(200, status) self.assertEqual([], payload["bottles"]) + def test_bottles_rejects_the_gateway_token(self) -> None: + """A compromised gateway holds only a `gateway` token — it must not be + able to enumerate bottles (an operator route) with it (issue #469).""" + status, _ = self._request("GET", "/bottles", token=self.gateway_token) + self.assertEqual(403, status) + def test_resolve_rejects_a_caller_with_no_token(self) -> None: """The credential-lift attack from issue #400: an agent could POST /resolve directly and read back the upstream tokens it's never meant @@ -148,6 +159,13 @@ class TestDockerControlPlaneAuthIntegration(unittest.TestCase): status, _ = self._request("POST", "/resolve") self.assertEqual(401, status) + def test_resolve_accepts_the_gateway_token(self) -> None: + """The gateway's own token DOES reach /resolve: with an empty body it + gets 400 (missing source_ip) rather than the 401 a rejected caller sees + — proving the role gate let the gateway token through.""" + status, _ = self._request("POST", "/resolve", token=self.gateway_token) + self.assertEqual(400, status) # past the role gate; body validation fails + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_control_auth.py b/tests/unit/test_control_auth.py new file mode 100644 index 0000000..bfcb7cc --- /dev/null +++ b/tests/unit/test_control_auth.py @@ -0,0 +1,86 @@ +"""Unit: role-scoped control-plane tokens (issue #469 review).""" + +from __future__ import annotations + +import base64 +import json +import unittest + +from bot_bottle.control_auth import ROLE_CLI, ROLE_GATEWAY, ROLES, mint, verify + +_KEY = "test-key" + + +def _b64(obj: object) -> str: + return base64.urlsafe_b64encode( + json.dumps(obj).encode()).rstrip(b"=").decode("ascii") + + +class TestMintVerify(unittest.TestCase): + def test_round_trip_each_role(self) -> None: + for role in ROLES: + self.assertEqual(role, verify(mint(role, _KEY), _KEY)) + + def test_gateway_and_cli_are_distinct(self) -> None: + self.assertEqual(ROLE_GATEWAY, verify(mint(ROLE_GATEWAY, _KEY), _KEY)) + self.assertEqual(ROLE_CLI, verify(mint(ROLE_CLI, _KEY), _KEY)) + + def test_wrong_key_rejected(self) -> None: + self.assertIsNone(verify(mint(ROLE_GATEWAY, _KEY), "other-key")) + + def test_tampered_signature_rejected(self) -> None: + tok = mint(ROLE_CLI, _KEY) + self.assertIsNone(verify(tok[:-1] + ("A" if tok[-1] != "A" else "B"), _KEY)) + + def test_tampered_payload_rejected(self) -> None: + header, _payload, sig = mint(ROLE_GATEWAY, _KEY).split(".") + forged = f"{header}.{_b64({'role': 'cli'})}.{sig}" + self.assertIsNone(verify(forged, _KEY)) + + def test_alg_none_rejected(self) -> None: + # An unsigned "alg: none" token must never verify. + tok = f"{_b64({'alg': 'none', 'typ': 'JWT'})}.{_b64({'role': 'cli'})}." + self.assertIsNone(verify(tok, _KEY)) + + def test_validly_signed_but_wrong_alg_rejected(self) -> None: + # Alg-confusion: even a *correctly signed* token whose header claims a + # non-HS256 alg must be rejected. + from bot_bottle.control_auth import _sign # noqa: PLC0415 + signing_input = f"{_b64({'alg': 'none', 'typ': 'JWT'})}.{_b64({'role': 'cli'})}" + self.assertIsNone(verify(f"{signing_input}.{_sign(_KEY, signing_input)}", _KEY)) + + def test_validly_signed_but_undecodable_rejected(self) -> None: + # A correct signature over a header that isn't valid base64/JSON still + # fails closed rather than raising. + from bot_bottle.control_auth import _sign # noqa: PLC0415 + signing_input = f"!!!not-base64!!!.{_b64({'role': 'cli'})}" + self.assertIsNone(verify(f"{signing_input}.{_sign(_KEY, signing_input)}", _KEY)) + + def test_malformed_tokens_rejected(self) -> None: + for bad in ("", "a", "a.b", "a.b.c.d", "not-base64.$$.$$"): + self.assertIsNone(verify(bad, _KEY)) + + def test_empty_key_never_verifies(self) -> None: + self.assertIsNone(verify(mint(ROLE_CLI, _KEY), "")) + + def test_unknown_role_in_payload_rejected(self) -> None: + header, _p, _s = mint(ROLE_CLI, _KEY).split(".") + # Re-sign a token carrying an unknown role — a valid signature but a + # role the control plane doesn't recognise must still be rejected. + from bot_bottle.control_auth import _sign # noqa: PLC0415 + payload = _b64({"role": "root"}) + signing_input = f"{header}.{payload}" + forged = f"{signing_input}.{_sign(_KEY, signing_input)}" + self.assertIsNone(verify(forged, _KEY)) + + def test_mint_rejects_unknown_role(self) -> None: + with self.assertRaises(ValueError): + mint("root", _KEY) + + def test_mint_rejects_empty_key(self) -> None: + with self.assertRaises(ValueError): + mint(ROLE_GATEWAY, "") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_gateway_init.py b/tests/unit/test_gateway_init.py index 3a3406d..17b5213 100644 --- a/tests/unit/test_gateway_init.py +++ b/tests/unit/test_gateway_init.py @@ -64,6 +64,30 @@ class TestEnvForDaemon(unittest.TestCase): self.assertNotIn("X", self._BASE) +class TestControlPlaneEnvScoping(unittest.TestCase): + """The control-plane signing key stays with the orchestrator; the pre-minted + `gateway` JWT goes to the data-plane daemons (issue #469 review). Scoping + them per-process keeps a compromised data-plane daemon from reading the key + and minting a higher-privilege token, even in the combined infra container.""" + + _BASE = { + "PATH": "/usr/bin", + "BOT_BOTTLE_CONTROL_PLANE_TOKEN": "sk-x", + "BOT_BOTTLE_CONTROL_AUTH_JWT": "gw-jwt", + } + + def test_orchestrator_gets_key_not_jwt(self): + env = _env_for_daemon("orchestrator", self._BASE) + self.assertEqual("sk-x", env["BOT_BOTTLE_CONTROL_PLANE_TOKEN"]) + self.assertNotIn("BOT_BOTTLE_CONTROL_AUTH_JWT", env) + + def test_data_plane_daemons_get_jwt_not_key(self): + for name in ("egress", "git-gate", "git-http", "supervise"): + env = _env_for_daemon(name, self._BASE) + self.assertNotIn("BOT_BOTTLE_CONTROL_PLANE_TOKEN", env, name) + self.assertEqual("gw-jwt", env["BOT_BOTTLE_CONTROL_AUTH_JWT"], name) + + class TestSelectedDaemons(unittest.TestCase): """Env-var subset filtering. The compose renderer is the source of truth for which daemons are wired; the supervisor just diff --git a/tests/unit/test_orchestrator_client.py b/tests/unit/test_orchestrator_client.py index 88a060b..4086ade 100644 --- a/tests/unit/test_orchestrator_client.py +++ b/tests/unit/test_orchestrator_client.py @@ -7,15 +7,30 @@ import unittest import urllib.error from unittest.mock import MagicMock, patch +from bot_bottle.control_auth import ROLE_CLI, verify from bot_bottle.orchestrator.client import ( OrchestratorClient, OrchestratorClientError, RegisteredBottle, + _host_auth_token, ) _URLOPEN = "bot_bottle.orchestrator.client.urllib.request.urlopen" +class TestHostAuthToken(unittest.TestCase): + def test_mints_a_cli_token_from_the_host_key(self) -> None: + with patch("bot_bottle.orchestrator.client.host_control_plane_token", + return_value="signing-key"): + tok = _host_auth_token() + self.assertEqual(ROLE_CLI, verify(tok, "signing-key")) + + def test_returns_empty_when_key_unreadable(self) -> None: + with patch("bot_bottle.orchestrator.client.host_control_plane_token", + side_effect=OSError("no host root")): + self.assertEqual("", _host_auth_token()) + + def _resp(status: int, payload: object) -> MagicMock: m = MagicMock() inner = m.__enter__.return_value diff --git a/tests/unit/test_orchestrator_control_plane.py b/tests/unit/test_orchestrator_control_plane.py index c30222f..a47b376 100644 --- a/tests/unit/test_orchestrator_control_plane.py +++ b/tests/unit/test_orchestrator_control_plane.py @@ -19,6 +19,7 @@ from contextlib import closing from pathlib import Path from unittest.mock import patch +from bot_bottle.control_auth import ROLE_CLI, ROLE_GATEWAY, mint from bot_bottle.orchestrator.broker import StubBroker from bot_bottle.orchestrator.control_plane import dispatch, make_server from bot_bottle.orchestrator.registry import BottleRecord, RegistryStore @@ -285,45 +286,71 @@ class TestServerRoundTrip(unittest.TestCase): 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.""" + """Role-scoped control-plane tokens (issue #400 / #469 review): every route + but /health needs a valid token, and the token's role gates which routes it + reaches — a `gateway` data-plane token can't drive the operator routes.""" + + _OPERATOR_ROUTES = [ + ("GET", "/bottles", b""), + ("POST", "/bottles", _body({"source_ip": "10.0.0.1"})), + ("PUT", "/bottles/x/policy", _body({"policy": "routes: []"})), + ("DELETE", "/bottles/x", b""), + ("POST", "/reconcile", _body({"live_source_ips": []})), + ("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": "a"})), + ] 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) + def test_health_is_public_even_unauthenticated(self) -> None: + status, _ = dispatch(self.orch, "GET", "/health", b"", role=None) 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", "/reconcile", _body({"live_source_ips": []})), + def test_unauthenticated_denies_every_other_route(self) -> None: + routes = self._OPERATOR_ROUTES + [ ("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") + ("POST", "/supervise/propose", + _body({"source_ip": "1", "tool": "egress-allow", "proposed_file": "x", "justification": "j"})), + ("POST", "/supervise/poll", _body({"source_ip": "1", "proposal_id": "p"})), + ] + for method, path, body in routes: + status, _ = dispatch(self.orch, method, path, body, role=None) + self.assertEqual(401, status, f"{method} {path} should be 401 unauthenticated") + + def test_gateway_role_denied_on_operator_routes(self) -> None: + # A compromised data-plane process holds only a `gateway` token — it must + # not reach the operator routes (approve proposals, rewrite policy, …). + for method, path, body in self._OPERATOR_ROUTES: + status, payload = dispatch(self.orch, method, path, body, role=ROLE_GATEWAY) + self.assertEqual(403, status, f"{method} {path} should be 403 for gateway") + self.assertIn("insufficient role", str(payload.get("error"))) + + def test_gateway_role_allowed_on_data_routes(self) -> None: + # The gateway CAN reach its own lookups. Register a bottle so /resolve + # returns 200 rather than a 403 — proving the role gate let it through. + rec = self.orch.registry.register("10.0.0.9", policy="routes: []", metadata="") + status, _ = dispatch( + self.orch, "POST", "/resolve", + _body({"source_ip": "10.0.0.9", "identity_token": rec.identity_token}), + role=ROLE_GATEWAY) + self.assertEqual(200, status) def test_deny_happens_before_the_registry_is_touched(self) -> None: - """An unauthorized DELETE must not tear a bottle down. 401, and the + """An unauthenticated 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.orch, "DELETE", f"/bottles/{rec.bottle_id}", b"", role=None) 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}): + def _server_with_key(self, signing_key: str): + with patch.dict("os.environ", {"BOT_BOTTLE_CONTROL_PLANE_TOKEN": signing_key}): server = make_server(self.orch, "127.0.0.1", 0) self.addCleanup(server.server_close) threading.Thread(target=server.serve_forever, daemon=True).start() @@ -340,25 +367,29 @@ class TestControlPlaneAuth(unittest.TestCase): 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") + def test_configured_server_enforces_roles_over_http(self) -> None: + key = "test-key" + base = self._server_with_key(key) + gateway_tok = mint(ROLE_GATEWAY, key) + cli_tok = mint(ROLE_CLI, key) # /health is public — no header needed. self.assertEqual(200, self._status(f"{base}/health")) - # /bottles requires the secret. + # /bottles (operator) needs a valid cli token. 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")) + 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 secret set (tests / nft-protected Firecracker): open mode, so the - existing round-trip and unit behavior are unchanged.""" + """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_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")) + self.assertEqual(ROLE_CLI, server.role_for("")) + self.assertEqual(ROLE_CLI, server.role_for("anything")) class TestDispatchSupervise(unittest.TestCase): diff --git a/tests/unit/test_policy_resolver.py b/tests/unit/test_policy_resolver.py index cc5bde0..83544ff 100644 --- a/tests/unit/test_policy_resolver.py +++ b/tests/unit/test_policy_resolver.py @@ -7,7 +7,13 @@ import unittest import urllib.error from unittest.mock import MagicMock, patch -from bot_bottle.policy_resolver import PolicyResolveError, PolicyResolver +from bot_bottle.policy_resolver import ( + CONTROL_AUTH_HEADER, + CONTROL_AUTH_JWT_ENV, + PolicyResolveError, + PolicyResolver, + _control_auth_headers, +) _URLOPEN = "bot_bottle.policy_resolver.urllib.request.urlopen" @@ -23,6 +29,18 @@ def _http_error(code: int) -> urllib.error.HTTPError: return urllib.error.HTTPError("http://x/resolve", code, "err", {}, None) # type: ignore[arg-type] +class TestControlAuthHeaders(unittest.TestCase): + def test_sends_the_gateway_jwt_when_configured(self) -> None: + with patch.dict("os.environ", {CONTROL_AUTH_JWT_ENV: "gateway.jwt.tok"}): + self.assertEqual({CONTROL_AUTH_HEADER: "gateway.jwt.tok"}, _control_auth_headers()) + + def test_sends_nothing_when_unset(self) -> None: + import os + with patch.dict("os.environ", {}, clear=False): + os.environ.pop(CONTROL_AUTH_JWT_ENV, None) + self.assertEqual({}, _control_auth_headers()) + + class TestPolicyResolver(unittest.TestCase): def setUp(self) -> None: self.r = PolicyResolver("http://orch:8080")