fix(security): prohibit unauthenticated orchestrator
This commit is contained in:
@@ -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=<signing-key> \
|
||||
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()
|
||||
|
||||
@@ -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__ = [
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user