feat(control-plane): role-scoped signed tokens so the gateway can't drive operator routes
test / integration-docker (pull_request) Successful in 17s
tracker-policy-pr / check-pr (pull_request) Successful in 16s
test / unit (pull_request) Successful in 39s
lint / lint (push) Successful in 56s
test / integration-firecracker (pull_request) Successful in 3m21s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
test / integration-docker (pull_request) Successful in 17s
tracker-policy-pr / check-pr (pull_request) Successful in 16s
test / unit (pull_request) Successful in 39s
lint / lint (push) Successful in 56s
test / integration-firecracker (pull_request) Successful in 3m21s
test / coverage (pull_request) Successful in 19s
test / publish-infra (pull_request) Has been skipped
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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()
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user