diff --git a/CLAUDE.md b/CLAUDE.md index d699049..a0e9ca0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,6 +38,7 @@ pixi run agent # Robot -> fleet bridge (mote-agent.service) pixi run fleet-server # Off-board: fleet API + operator dashboard pixi run fleetctl # Operator CLI: token / operator / robots / dispatch / audit / watch pixi run fleet-broker-ws # Off-board: MQTT broker with WebSockets (needs docker) +# fleetctl broker sync # regenerate the broker's credentials from the registry # Dev environment only (installs ros-jazzy-desktop) pixi run rviz # RViz2 with mote config @@ -97,7 +98,11 @@ Milestone M1 of `docs/design/fleet.md`, built in the **`mote_fleet`** package (b ## Fleet: the operator view + dispatch API (M3) -Milestone M3 of `docs/design/fleet.md`, and the end of v0. The **HTTP** wire is specified as its own versioned contract in **`docs/fleet/fleet-api.md`** (M1's MQTT one is `control-plane.md`); the operator flow is `docs/fleet/README.md` §6–9 and the measurements are `m3-verification.md`. **The two directions of the loop take different paths on purpose.** *Reads* ride MQTT: the browser subscribes to `mote/v1/+/{presence,health,pose,task/status}` over WebSockets, and because all of those are retained it has the whole fleet's state within a second of loading — no polling, no service in the middle. *Writes* ride HTTP: `POST /v1/robots//dispatch` authorizes an operator token (`fleetctl operator new --name `; the name is what the audit row records), writes the audit row, then publishes to the same `task/command` topic. **The topic tree did not change — only who publishes to it**, and `fleetctl dispatch` moved to the API too, so there is one write path rather than one per client. The command grammar is still parsed only by the robot's task layer: a parser in the server would be a second grammar to keep in step. **The browser cannot publish**: `server/ui/mqtt.mjs` is a hand-rolled subscribe-only MQTT 3.1.1 client that implements no PUBLISH packet, so the split is enforced by omission (M7 makes it structural with a subscribe-only broker credential). The UI is static ES modules — no bundler, no npm, no vendored library — served by the same stdlib `http.server`; `map.mjs` holds the Q5 world→pixel transform (`px = (wx-origin_x)/res`, `py = height - (wy-origin_y)/res`) and a pan/zoom/follow canvas, and only draws robots on the *same* site+floor as the selected one because a pose from another floor is a different map frame. **Basemaps come from site bundles on the fleet box** (`--maps-dir`, default `$MOTE_FLEET_HOME/sites`, the layout `sites.py` writes, seeded by rsync until **M4** makes the registry canonical behind the same two routes). **M1's websockets blocker is settled**: `pixi run fleet-broker-ws` runs `eclipse-mosquitto` under docker with the repo's own `mosquitto.conf`, because conda-forge's build has none; `broker.sh` strips the WS stanza when the local binary cannot honour it and says so. Two things that run in the same file (`test_ui.py` → `ui_test.mjs`) are the MQTT codec and the transform, tested under node against the very files the browser loads; `browser_check.mjs` drives a real headless Chrome over CDP against a running stack and is an operator's tool, not a CI test. +Milestone M3 of `docs/design/fleet.md`, and the end of v0. The **HTTP** wire is specified as its own versioned contract in **`docs/fleet/fleet-api.md`** (M1's MQTT one is `control-plane.md`); the operator flow is `docs/fleet/README.md` §6–9 and the measurements are `m3-verification.md`. **The two directions of the loop take different paths on purpose.** *Reads* ride MQTT: the browser subscribes to `mote/v1/+/{presence,health,pose,task/status}` over WebSockets, and because all of those are retained it has the whole fleet's state within a second of loading — no polling, no service in the middle. *Writes* ride HTTP: `POST /v1/robots//dispatch` authorizes an operator token (`fleetctl operator new --name `; the name is what the audit row records), writes the audit row, then publishes to the same `task/command` topic. **The topic tree did not change — only who publishes to it**, and `fleetctl dispatch` moved to the API too, so there is one write path rather than one per client. The command grammar is still parsed only by the robot's task layer: a parser in the server would be a second grammar to keep in step. **The browser cannot publish**: `server/ui/mqtt.mjs` is a hand-rolled subscribe-only MQTT 3.1.1 client that implements no PUBLISH packet, so the split is enforced by omission — and since **M7** also by the broker's ACL, which gives that credential no write rule at all. The UI is static ES modules — no bundler, no npm, no vendored library — served by the same stdlib `http.server`; `map.mjs` holds the Q5 world→pixel transform (`px = (wx-origin_x)/res`, `py = height - (wy-origin_y)/res`) and a pan/zoom/follow canvas, and only draws robots on the *same* site+floor as the selected one because a pose from another floor is a different map frame. **Basemaps come from site bundles on the fleet box** (`--maps-dir`, default `$MOTE_FLEET_HOME/sites`, the layout `sites.py` writes, seeded by rsync until **M4** makes the registry canonical behind the same two routes). **M1's websockets blocker is settled**: `pixi run fleet-broker-ws` runs `eclipse-mosquitto` under docker with the repo's own `mosquitto.conf`, because conda-forge's build has none; `broker.sh` strips the WS stanza when the local binary cannot honour it and says so. Two things that run in the same file (`test_ui.py` → `ui_test.mjs`) are the MQTT codec and the transform, tested under node against the very files the browser loads; `browser_check.mjs` drives a real headless Chrome over CDP against a running stack and is an operator's tool, not a CI test. + +## Fleet: security hardening (M7) + +Milestone M7 of `docs/design/fleet.md`, cross-cutting. The authorization rules are specified as the fleet's **third** versioned contract in **`docs/fleet/security.md`** (M1's MQTT wire is `control-plane.md`, M3's HTTP wire is `fleet-api.md`); the measurements are `m7-verification.md`. **Through M3 the fleet had one credential on one path** — an operator token on dispatch — and everything else leaned on the tailnet. The argument for changing that is not that WireGuard is weak but that **a single boundary makes every robot as trusted as the fleet server**: a robot is physically reachable and carries the biggest dependency tree in the system, so one compromised robot would own the fleet. **The broker is no longer anonymous.** `mote_fleet/server/credentials.py` generates mosquitto's `password_file` and `acl_file` *from the registry* into `$MOTE_FLEET_HOME/broker/`, and the broker re-reads them on SIGHUP (`broker.sh reload`, which knows whether it started a local process or a container) — so who may connect is derived, never hand-maintained, and a revoked operator disappears because the query no longer returns them. **Three principals, disjoint by construction**: a robot's username is its `robot_id` and it may publish only under its own prefix and read only its own `task/command`; an operator's is `op__` and may subscribe fleet-wide and **publish nothing**; the server's is `fleet_server` and is the only principal that may write `task/command`. A `robot_id` is a lowercase DNS label so it can never contain the underscore the other two carry — no runtime collision check, and `test_credentials.py` asserts the property rather than the paragraph. Robot credentials are issued **in the enrollment transaction** and returned once (only the hash is stored), which makes **rotation just `pixi run enroll` again**; `$MOTE_HOME/fleet.yaml` and the registry are both `0600`. **Every `/v1` route now needs an operator token**, checked by one gate in front of routing (so a new route is authenticated by default, and an unauthenticated caller gets 401 rather than a 404 that would disclose which routes exist); `/healthz` and the static UI stay open, the latter because the page must load before it can ask for a token. `/v1/config` hands the browser *that operator's* subscribe-only broker credential, which is why it needs a token. The mosquitto `$7$` hash (PBKDF2-HMAC-SHA512, 101 iterations) is generated in stdlib rather than by shelling out to `mosquitto_passwd`, because the fleet server's whole dependency list is "python" — and a test verifies a real `mosquitto_passwd` hash so the two cannot drift. **Gotcha, and every test in `test_broker_acl.py` depends on it: mosquitto denies silently.** A forbidden publish is accepted and dropped; a forbidden subscribe is *granted* at SUBACK and never delivers — so denial must be asserted on delivery, and a suite checking return codes would pass with no ACL loaded. The network half is `mote_bringup/tailscale/policy.hujson` (committed source of truth, pasted into the admin console), whose own `tests` block asserts robots cannot reach each other; Tailscale refuses to save a policy that fails it. **Breaking change for a deployed fleet**: a robot enrolled before M7 has no credential and will be refused — re-run `enroll`. Deliberately *not* done, each for a reason in the doc: mTLS, expiring operator sessions, package signing (nothing to sign until M5 ships a package; pinning is what M7 could deliver — `pixi install --locked` at provisioning), and Foxglove tokens (M2 does not exist yet). ## Sites (maps & zones) diff --git a/docs/design/fleet.md b/docs/design/fleet.md index 8171c8c..69a1918 100644 --- a/docs/design/fleet.md +++ b/docs/design/fleet.md @@ -732,7 +732,11 @@ per-channel auth on top; do **not** build a PKI or a custom auth server for v1. tailnets or broker vhosts + topic-prefix ACLs per customer — so one customer's operator can never see another's robots. Designed-for, not built in v1. - **Proportionality.** Small trusted fleet, not a public product: mTLS-everywhere - and a custom PKI wait until the fleet runs *outside* the trusted overlay. Note + and a custom PKI wait until the fleet runs *outside* the trusted overlay. The + argument that made M7 worth building anyway is that a single boundary makes + **every robot as trusted as the fleet server** — a robot is physically + reachable and runs the largest dependency tree in the system, so one boundary + means one compromised robot owns the fleet. Note Zenoh's TLS is *hop-by-hop, not end-to-end* ([Zenoh security analysis](https://census-labs.com/news/2025/03/17/zenoh-protocol-security-analysis/)) — a reason the encryption baseline lives at the WireGuard layer (end-to-end @@ -973,6 +977,24 @@ M7 (security hardening) : cross-cutting, folds into each; can start after M0 signed if available) packages. *Accept:* a device off the tailnet reaches nothing; a robot can't read another robot's command topic. *Depends on:* M0; folds into each milestone as it lands. + **Built** as [`docs/fleet/security.md`](../fleet/security.md) — the fleet's + third contract, alongside M1's MQTT one and M3's HTTP one — with the policy in + `mote_fleet/server/credentials.py` and + [`mote_bringup/tailscale/policy.hujson`](../../mote_bringup/tailscale/policy.hujson), + and the measurements in [`m7-verification.md`](../fleet/m7-verification.md). + The broker is no longer anonymous: credentials are **generated from the + registry** and the broker reloads them on SIGHUP, so a robot may publish only + under its own `robot_id`, an operator may only subscribe, and only the fleet + API may write `task/command`. Every `/v1` route now needs an operator token, + behind one gate rather than per-handler. Three things the milestone did *not* + do, each for a stated reason: **mTLS** (username/password inside WireGuard is + proportionate, and the seam is unchanged), **expiring operator sessions** + (Q7's OIDC/GitHub answer replaces the minting, not the checking), and + **package signing** — there is nothing to sign until the channel ships a robot + package, so that stays M5's, and pinning is what M7 could actually deliver + (`pixi install --locked` at provisioning, every dependency sha256-pinned). + **Foxglove tokens wait for M2**, which does not exist yet; what M7 contributes + is the tailnet rule already scoping port 8765 to operators. --- diff --git a/docs/fleet/README.md b/docs/fleet/README.md index ae30dad..2e14776 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -7,10 +7,12 @@ every robot (**M1**), and a browser you watch and drive the fleet from (**M3**). The architecture and the milestones after these are in [`docs/design/fleet.md`](../design/fleet.md); the measurements are in [`m0-verification.md`](m0-verification.md), -[`m1-verification.md`](m1-verification.md) and -[`m3-verification.md`](m3-verification.md); the two wires are specified in -[`control-plane.md`](control-plane.md) (MQTT) and [`fleet-api.md`](fleet-api.md) -(HTTP). +[`m1-verification.md`](m1-verification.md), +[`m3-verification.md`](m3-verification.md) and +[`m7-verification.md`](m7-verification.md); the three contracts are +[`control-plane.md`](control-plane.md) (the MQTT wire), +[`fleet-api.md`](fleet-api.md) (the HTTP wire) and +[`security.md`](security.md) (who may say what on either). | | | |---|---| @@ -65,24 +67,22 @@ on new tailnets, and it is the thing that makes `ssh michael@mote-01` work — t device's tailnet hostname becomes its name. Without it you are typing `100.x.y.z` addresses everywhere. -**3. Declare the tags.** Admin console → **Access controls**, in the policy file. -Tags do not exist until an owner is declared for them, and `--advertise-tags` on -a machine fails with "requested tags are invalid or not permitted" if you skip -this: - -```jsonc -{ - "tagOwners": { - "tag:robot": ["autogroup:admin"], - "tag:fleet": ["autogroup:admin"], - "tag:inference": ["autogroup:admin"], - }, - // The default policy already allows every device to reach every other, which - // is what M0 wants. M7 replaces this with per-tag rules (operators reach - // robots; robots reach the broker and their own inference box; robots cannot - // reach each other). -} -``` +**3. Apply the access policy.** Admin console → **Access controls** → *Edit +file*, then paste +[`mote_bringup/tailscale/policy.hujson`](../../mote_bringup/tailscale/policy.hujson) +and Save. That file is the source of truth; the console holds a copy of it, so +edit the repo first. + +It does two things. It **declares the tags** — they do not exist until an owner +is declared, and `--advertise-tags` fails with "requested tags are invalid or not +permitted" if you skip this, which is the first thing to check when joining a +robot fails. And it **replaces the default allow-everything policy** with rules +that grant only what something in this repo actually dials: operators reach the +fleet server, the robots' Foxglove/SSH ports and the GPU box; robots reach the +broker, the registry API and their inference server; and **robots cannot reach +each other on any port**, which is one half of M7's acceptance criterion. The +policy's own `tests` block asserts that, and Tailscale refuses to save a policy +that fails it. **4. Mint an auth key per robot.** Admin console → **Settings → Keys → Generate auth key**. For a robot: @@ -407,14 +407,24 @@ memory of who is in it. fleet box's MagicDNS name (`fleet-box`), not `localhost` — it is handed out verbatim in every enrollment answer. It defaults to the box's hostname. -**Security, plainly:** the broker is anonymous and the API's *read* routes are -unauthenticated. Dispatch is not — it needs an operator token (§8) — but that is -one credential on one path, not an auth story. It is proportionate only because -the tailnet is the boundary: WireGuard authenticates, and nothing here is -exposed to the internet. Do not put either on a network the robots are not -already trusted on. Per-robot broker credentials and operator auth everywhere -are M7; the shape of what changes is in -[`control-plane.md`](control-plane.md#security-posture-and-what-m7-changes). +**Security, plainly (M7):** the broker authenticates every client and confines +each to its own part of the topic tree, and every API route that discloses +anything about the fleet needs an operator token. A robot gets its broker +credential from enrollment (§7); an operator gets theirs with their token (§8). +The two generated files the broker reads — `$MOTE_FLEET_HOME/broker/{passwd,acl}` +— are written by this server from the registry and reloaded automatically, so +there is nothing to hand-maintain. The full contract is +[`security.md`](security.md). + +Start the broker **before** the server the first time. It comes up with empty +credential files, refusing everybody, and the server fills them in and reloads +it. If you ever restart the broker against a stale or empty directory, `pixi run +-e fleet fleetctl -- broker sync` regenerates and reloads without touching the +server. + +The tailnet is still the outer boundary and this port should still not be +published to the internet — but it is no longer the *only* boundary, which is +the difference M7 makes. To run it unattended, wrap the two commands in systemd units on that box. They are deliberately *not* part of `pixi run setup`, which provisions robots. @@ -437,14 +447,23 @@ pixi run enroll -- --server http://fleet-box:8080 --token tskey… --name Scout That writes both files the agent needs, and nothing else: ```yaml -# $MOTE_HOME/robot.yaml # $MOTE_HOME/fleet.yaml +# $MOTE_HOME/robot.yaml # $MOTE_HOME/fleet.yaml (0600) schema: 1 schema: 1 id: mote-01 server: http://fleet-box:8080 name: Scout broker: site: home host: fleet-box port: 1883 + username: mote-01 + password: "…" ``` +**The same exchange issues the robot's broker credential** (M7), and this is the +only time that password is sent — the registry keeps a hash. So **rotation is +just re-running `enroll`**: idempotent on the hardware fingerprint, same +identity, new password. A robot enrolled before M7 has no credential and the +broker will refuse it; the agent's log says exactly that, and the fix is one +`pixi run enroll`. + Three properties make this safe to run unattended, or twice, or after a mistake: - **Idempotent.** The registry keys on a stable hardware id (the Pi's SoC @@ -478,7 +497,7 @@ it logs and retries — enrolling later brings it up without a restart. ```bash # [fleet box] mint yourself an operator credential, once pixi run -e fleet fleetctl -- operator new --name michael -export MOTE_FLEET_TOKEN= +export MOTE_FLEET_TOKEN= # every verb below needs it now pixi run fleetctl -- robots # the registry roster pixi run fleetctl -- watch # live: presence, health, pose, status @@ -497,6 +516,13 @@ network; the **name on it is what the audit log records**, which is why an unnamed one is refused. `fleetctl operator list|revoke` are the other two verbs, and the route contract is [`fleet-api.md`](fleet-api.md). +**One credential, both paths.** Since M7 the read routes need the token too, and +so does the broker — `watch` and the status half of `dispatch` fetch the +operator's own *subscribe-only* broker login from `/v1/config` using that same +token, so there is still only one secret to hold. `fleetctl operator revoke` +closes both at once, because they are minted together and the broker's password +file is regenerated from the registry rows. + `dispatch` exits 0 only if the task **succeeded**, so it composes into scripts. The command grammar is the task layer's own, unchanged (`fetch `, `goto ` — see `mote_tasks`); the fleet adds no second @@ -563,11 +589,16 @@ no request/response loop, and no service between the broker and the browser. That is the read path in [`fleet.md`](../design/fleet.md) Q5, and it is why the broker needs the WebSocket listener from §6. -**Paste an operator token to dispatch.** Without one the page is read-only, -which is a perfectly good wall display. The token is kept in the browser's local -storage and sent to the fleet API as a bearer credential; the page holds **no -broker credential that can publish**, and its MQTT client implements no PUBLISH -packet at all. +**Paste an operator token to sign in.** Since M7 there is no anonymous +read-only mode: `/v1/config` is itself operator-only, because it is what hands +the page its broker credential. So the page has two states — signed in, or +asking to be — and pasting a token needs no reload. The token is kept in the +browser's local storage and sent to the fleet API as a bearer credential. + +The page holds **no broker credential that can publish**, and that is now true +twice over: its MQTT client implements no PUBLISH packet at all, *and* the +credential it is given has no write rule in the broker's ACL. The second is what +makes the split a property of the broker rather than of our own code. **The map.** A floor's PNG basemap with live robot markers on it: pan by dragging, zoom with the wheel, click a robot to select it, `follow` to keep the diff --git a/docs/fleet/control-plane.md b/docs/fleet/control-plane.md index acca2df..1eaa7bd 100644 --- a/docs/fleet/control-plane.md +++ b/docs/fleet/control-plane.md @@ -236,8 +236,8 @@ supersedes M0's operator-set id: the server owns the id space. | Route | Purpose | |---|---| | `GET /healthz` | liveness, contract version, robot count | -| `GET /v1/robots` | the roster | -| `GET /v1/robots/` | one row | +| `GET /v1/robots` | the roster (operator token, since M7) | +| `GET /v1/robots/` | one row (operator token, since M7) | | `POST /v1/enroll` | allocate (or return) a robot id | ### `POST /v1/enroll` @@ -254,10 +254,16 @@ the server records it rather than renumbering the fleet. ```json {"schema":1,"robot_id":"mote-01","name":"Scout","site":"home","created":true, - "enrolled_at":"2026-07-26T16:12:46Z","broker":{"host":"fleet-box","port":1883}, + "enrolled_at":"2026-07-26T16:12:46Z", + "broker":{"host":"fleet-box","port":1883,"username":"mote-01","password":"…"}, "contract":"mote/v1"} ``` +`broker.username`/`broker.password` are M7's, and additive — a field a consumer +did not know about does not bump `schema`. **This is the only time the password +is sent**; the registry keeps a hash, so re-enrolling rotates it rather than +recovering it. + `201` for a new robot, `200` for one that was already enrolled. The robot writes `$MOTE_HOME/robot.yaml` (identity) and `$MOTE_HOME/fleet.yaml` (server + broker) from that answer — it learns where its broker is from the same exchange @@ -280,25 +286,32 @@ robots enrolling at once get eight distinct ids. --- -## Security posture (and what M7 changes) - -M1 is proportionate to the M0 substrate and no further. Stated plainly so it is -not mistaken for a finished story: - -- **The broker is anonymous.** Any client that can reach it may publish or - subscribe anywhere in the tree. WireGuard is the authentication boundary; - nothing here is reachable from the public internet. -- **The fleet API has no auth** on its read routes. Enrollment tokens and, since - M3, operator tokens are the only credentials in the system. -- **Dispatch is mediated, as of M3.** M1's `fleetctl` published straight to the - broker; now it and the dashboard both POST to `/v1/robots//dispatch`, - which authorizes an operator token and writes an audit row before publishing - ([`fleet-api.md`](fleet-api.md)). As this section promised, **the topic tree - did not change** — only who publishes to it. The browser holds no broker - credential that can publish; making that structural on the broker side, with a - subscribe-only credential, is still M7's. - -M7 adds per-robot broker credentials (username = `robot_id`, publish confined to -its own prefix), operator auth on the API, and the Tailscale ACLs that stop -robots reaching each other. Until then: do not put the broker or the API on a -network the robots are not already trusted on. +## Security posture + +**The topic tree above is enforced, not merely agreed.** Since M7 the broker +authenticates every client and confines each to its own part of the tree; the +full contract — the principals, the rules, and how a credential is issued, +rotated and revoked — is [`security.md`](security.md). In one table: + +| | publish | subscribe | +|---|---|---| +| robot `mote-01` | `mote/v1/mote-01/{presence,health,pose,task/status}` | `mote/v1/mote-01/task/command` | +| operator | *nothing* | `mote/v1/+/{presence,health,pose,task/status}` | +| fleet server | `mote/v1/+/task/command` | `mote/v1/#` | + +So a robot cannot read another robot's commands or forge another robot's health, +and an operator cannot publish a command at all — dispatch stays the fleet API's, +because that is the only place it can be *attributed* +([`fleet-api.md`](fleet-api.md)). **The topic tree itself did not change** across +any of this; only who may speak on which part of it. + +A robot receives its broker credential from the same `POST /v1/enroll` exchange +that gives it its id — `broker.username` and `broker.password` in the answer — +and re-enrolling rotates it. A robot enrolled before M7 has none and will be +refused; `pixi run enroll` is the fix, and the agent's log says so. + +The boundary outside all of this is still the tailnet, now with a policy that +denies robot-to-robot traffic on every port +([`policy.hujson`](../../mote_bringup/tailscale/policy.hujson)). What is *not* +in place: mTLS, expiring operator sessions, and package signing — +[`security.md`](security.md#what-m7-does-not-do) says why for each. diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index 6bbffb7..9e5e731 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -1,16 +1,17 @@ # Fleet API — interface contract v1 The HTTP wire: enrollment, the registry, **mediated dispatch**, the audit log, -and what the dashboard needs to bootstrap. This is the second of the fleet's two -contracts — [`control-plane.md`](control-plane.md) specifies the MQTT one — and -the versioned spec [`fleet.md`](../design/fleet.md) requires M3 to publish. +and what the dashboard needs to bootstrap. One of the fleet's three contracts — +[`control-plane.md`](control-plane.md) specifies the MQTT wire and +[`security.md`](security.md) the authorization rules applied to both — and the +versioned spec [`fleet.md`](../design/fleet.md) requires M3 to publish. | | | |---|---| | **Contract version** | `v1` (routes under `/v1/…`, payload `schema: 1`) | | **Authority** | [`mote_fleet/server/fleet_server.py`](../../mote_fleet/server/fleet_server.py) | | **Kept honest by** | `mote_fleet/test/test_fleet_server.py`, and `test_e2e_fleet.py` for the dispatch path end to end | -| **Milestone** | M3. Operator runbook: [`README.md`](README.md) §6–9. Measurements: [`m3-verification.md`](m3-verification.md) | +| **Milestone** | M3, with authorization from M7. Operator runbook: [`README.md`](README.md) §6–9. Measurements: [`m3-verification.md`](m3-verification.md), [`m7-verification.md`](m7-verification.md) | ## Why there are two contracts @@ -46,9 +47,17 @@ Status codes are part of the contract: a client may switch on them. | Route | Credential | |---|---| +| `GET /healthz` | none — a liveness probe that needs a secret is one nobody wires up | +| `GET /` and the static UI | none — the page must load before it can ask for a token | | `POST /v1/enroll` | an **enrollment token** in the body (single-use by default) | -| `POST /v1/robots//dispatch`, `GET /v1/audit` | an **operator token** as `Authorization: Bearer ` | -| everything else | none — see the security note below | +| **everything else under `/v1`** | an **operator token** as `Authorization: Bearer ` | + +**Since M7 the read routes are authorized too**, not just dispatch — M3 left the +roster, the basemaps and the broker's address readable by anything that could +reach the port. The check sits in one gate in front of every `/v1` path rather +than in each handler, so a route added later is authenticated by default; a +`404` is never returned to an unauthenticated caller, because that would +disclose which routes are real. Operator tokens are minted on the fleet box, against the registry file, never over the network: @@ -60,25 +69,27 @@ pixi run -e fleet fleetctl -- operator revoke --token ``` The token's **name is what the audit log records**, which is why an unnamed one -is refused. Revocation keeps the row: who *had* access is part of the record. +is refused. Revocation keeps the row: who *had* access is part of the record — +and it also withdraws that operator's **broker** credential, because the two are +minted together and the broker's password file is regenerated from these rows +([`security.md`](security.md)). Bearer header only — never a query parameter, which would put the credential in every access log between here and the browser. -**Security posture, plainly.** The read routes are unauthenticated and the -broker is anonymous, exactly as M1 left them. M3 adds a credential on the -*write* path and a record of who used it, which is the milestone's brief; it is -proportionate only while the tailnet is the boundary. M7 adds operator auth on -the read routes, per-robot broker credentials, and the Tailscale ACLs. Until -then, do not expose this port to a network the robots are not already trusted -on. +**Security posture, plainly.** Every route that discloses anything about the +fleet requires an operator; the broker requires a per-principal credential; and +the tailnet policy denies robot-to-robot traffic. What is still absent — token +expiry, mTLS, package signing — is listed with reasons in +[`security.md`](security.md#what-m7-does-not-do). The tailnet remains the outer +boundary: this port should still not be published to the internet. --- ## Routes ``` -GET /healthz liveness, contract, robot count +GET /healthz liveness, contract, robot count [open] GET /v1/config what the browser needs to bootstrap GET /v1/robots the roster GET /v1/robots/ one row @@ -176,6 +187,20 @@ Everything the dashboard cannot work out from its own URL. falls back to the host it was loaded from — so reaching the fleet box by MagicDNS, by tailnet address or over localhost all work with no per-deployment build. +Since M7 the `broker` object also carries **the calling operator's own** +subscribe-only MQTT credential: + +```json +"broker": {"ws_host": null, "ws_port": 9001, "host": "fleet-box", "port": 1883, + "username": "op_michael_3f9a", "password": "…"} +``` + +That is the reason this route needs a token: it hands out a credential, and the +one it hands out is the caller's. Two operators get two different logins, and a +revoked operator's next page load has nothing to connect with. The credential +grants `mote/v1/+/{presence,health,pose,task/status}` and **no write rule at +all** ([`security.md`](security.md)). + ### `GET /v1/maps`, `/v1/maps///map.json|map.png` The basemap a pose is meaningful on. @@ -212,9 +237,15 @@ than a bespoke map format. ## What the browser is allowed to do -The dashboard holds an operator token for this API and **no broker credential at -all that can publish**. The read path connects to the broker's WebSocket -listener with a client that implements no PUBLISH packet -([`ui/mqtt.mjs`](../../mote_fleet/server/ui/mqtt.mjs)) — the split is enforced by -omission, not by intention. M7 makes that structural on the broker side too, -with a subscribe-only credential. +The dashboard holds an operator token for this API and **no broker credential +that can publish**. That is now true twice over: + +- its MQTT client implements no PUBLISH packet at all + ([`ui/mqtt.mjs`](../../mote_fleet/server/ui/mqtt.mjs)) — enforced by omission; +- and since M7 the broker credential it is given has no write rule in the + broker's ACL — enforced by the broker, so it holds for `curl`, a hand-rolled + client, or anywhere else that credential is pasted. + +The second is what makes the split structural rather than a property of our own +code, which is what M3 said it owed. Both credentials arrive together, from this +route and its token, and die together when the operator is revoked. diff --git a/docs/fleet/m7-verification.md b/docs/fleet/m7-verification.md new file mode 100644 index 0000000..b1321b7 --- /dev/null +++ b/docs/fleet/m7-verification.md @@ -0,0 +1,282 @@ +# M7 verification ledger + +What was measured for the security hardening, how, and what is still unverified. +The interface it verifies is [`security.md`](security.md); the two wires it +constrains are [`control-plane.md`](control-plane.md) and +[`fleet-api.md`](fleet-api.md), neither of which changed shape. + +The milestone's acceptance criteria are **"a device off the tailnet reaches +nothing"** and **"a robot cannot read another robot's command topic."** §1 +answers the second against a real broker; §6 explains why the first is asserted +by the Tailscale policy rather than measured here. + +## 1. A robot cannot read another robot's command topic — **confirmed** + +Against a real mosquitto 2.0.20, reading the real generated `password_file` and +`acl_file`, with two robots enrolled through the real HTTP endpoint. The whole +run is `mote_fleet/test/test_broker_acl.py` (13 tests); this is the same thing +driven by hand through the shipped `broker.sh` and `fleet_server.py`: + +```console +=== 8. THE CRITERION: mote-01 cannot read mote-02's command topic + connacks: mote-01=Success mote-02=Success server=Success operator=Success + mote-02 (addressed) received: ['goto kitchen'] + mote-01 (eavesdropper) : [] + operator : [] + operator saw on health : ['genuine'] (FORGED absent) + anonymous connack : Not authorized +``` + +Five separate properties in that block, each of which was open before M7: + +| | before | now | +|---|---|---| +| anonymous client | full read/write of the tree | `Not authorized` at CONNACK | +| `mote-01` reading `mote-02`'s commands | delivered | nothing delivered | +| `mote-01` publishing as `mote-02` | accepted | dropped; the operator sees only `genuine` | +| operator publishing a command | accepted | dropped | +| operator reading a command | delivered | nothing delivered | + +### The finding that shapes every test above: **denial is silent** + +mosquitto does not report an ACL denial to the client. A publish to a forbidden +topic is accepted at the socket and discarded; a subscribe to a forbidden filter +is **granted at SUBACK** and simply never delivers: + +``` +mote-01 SUBACK for mote/v1/mote-02/task/command: [1] # granted QoS 1 +mote-01 messages received: [] # ...and nothing arrives +``` + +So a test that asserted on a return code would pass against a broker with **no +ACL file at all**. Everything in `test_broker_acl.py` therefore asserts on +*delivery*, and one test (`test_the_denial_is_silent_…`) exists purely to pin +that measurement in place. It is also the first thing to check when a robot +"publishes and nothing happens": compare the topic against the ACL table in +[`security.md`](security.md), because nothing will have logged an error. + +## 2. The credential files reach a running broker — **confirmed, via SIGHUP** + +The ordering problem: a robot is issued a credential at enrollment, and the +broker has never heard of it. Measured with `broker.sh` started *first*, empty +files and all — which is exactly how a fleet box comes up: + +```console +=== 1. broker starts before the server has ever run + created an empty …/broker/passwd — nothing can connect until the fleet + server writes it (start fleet-server, or run 'fleetctl broker sync') + Opening ipv4 listen socket on port 43963. + passwd bytes: 68 mode: 600 + +=== 2. nothing can connect yet (fail closed) + Connection error: Connection Refused: not authorised. + +=== 3. fleet server starts, generates credentials, reloads the broker + mote-fleet on http://127.0.0.1:36211 … credentials=…/broker + passwd now: 1 line(s) +``` + +and from the broker's own log, mid-run: + +``` +1785141364: Client auto-59A75FD4… disconnected, not authorised. +1785141364: Reloading config. +1785141365: New client connected … (p2, c1, k60, u'mote-01'). +``` + +**An empty password file admits nobody**, which is the right state for a broker +whose fleet server has never run — as opposed to a broker that refuses to +*start*, which reads like a broken deployment. The bootstrap is therefore: start +the broker, start the server, and the server fills in and reloads it. + +`fleetctl broker sync` covers the two cases nothing else would — a broker +restarted against a stale directory, and a registry restored from backup: + +```console +=== 7. fleetctl broker show + password_file …/broker/passwd + acl_file …/broker/acl + principals 2 robots, 1 operators, 1 server + fleet_server mote-01 mote-02 op_michael_6e6c +``` + +## 3. Every API route is authorized — **confirmed** + +```console +=== 5. the API refuses an anonymous read + /v1/robots 401 an operator token is required (Authorization: Bearer) + /v1/config 401 an operator token is required (Authorization: Bearer) + /v1/maps 401 an operator token is required (Authorization: Bearer) + /v1/audit 401 an operator token is required (Authorization: Bearer) + /healthz 200 +``` + +`/healthz` open is deliberate and is the only `/v1`-adjacent route that is; the +static UI is the other exception, because the page must load before it can ask +for a token. Parametrised over all seven read routes in +`test_fleet_server.py`, three ways each — no token, an unknown token, a revoked +token. + +One choice worth recording: **an unauthenticated caller never gets a 404.** The +gate runs before routing, so `/v1/nothing` answers `401` rather than disclosing +which routes exist. + +## 4. The whole suite — **212 tests, 0 failures** + +```console +$ pixi run -e dev test-fleet +212 passed in 86.77s +``` + +(One of those 212 is `test_ui.py`, which runs the whole of `ui_test.mjs` under +node as a single pytest case — 16 node subtests, four of them M7's.) + +That includes the four tiers from M3 plus M7's two new files, and — the part +that matters — **`test_e2e_fleet.py` now runs against an authenticated, +ACL-enforcing broker**. Its fixture starts mosquitto with `allow_anonymous +false` and *empty* credential files, so the enrollment → issue → reload → agent +connects → dispatch → status chain only completes if every link of M7 works. If +the credential plumbing broke, those four tests would fail rather than quietly +falling back to an anonymous connection. + +The 13 broker-ACL tests and the real-broker e2e skip without mosquitto, so +`pixi run test` (the robot environment) still runs everything else. + +## 5. The dashboard, in a real browser — **confirmed, including the revocation** + +Driven with Playwright against the shipped stack: the container broker +(`eclipse-mosquitto:2`, 2.1.2) reading the generated credential files, the real +fleet server, two enrolled robots. + +**Signed out**, which is a state that did not exist before M7: + +``` +operator: "read-only — paste an operator token" +broker-state: "not connected — no operator token" +roster: "Paste an operator token to see the fleet. Mint one on the fleet + box: fleetctl operator new --name " +``` + +**After pasting the token**, with no page reload: + +``` +operator: "operator token accepted" +broker-state: "broker connected" +contract: "mote/v1" +roster: mote-01 · Scout mote-02 · Rover +``` + +and from the broker's own log, which is what proves the WebSocket connection was +*authenticated* rather than merely accepted: + +``` +New client connected from 127.0.0.1:46886 as mote-ui-14f78c6b + (p4, c1, k30, u'op_michael_4d54') +``` + +**Then the operator was revoked**, and one act closed both paths — the HTTP one +on the next request and the MQTT one *mid-session*, without the page doing +anything: + +``` +1785143413: Client mote-ui-14f78c6b disconnected. +1785143414: Client mote-ui-14f78c6b disconnected: not authorised. + +GET /v1/robots -> 401 {"error": "unknown or revoked operator token"} +broker-state: "broker offline: connection closed" +``` + +That is the property the whole credential design is for, observed end to end. + +### Two things this run found + +- **`hidden` was not hiding anything.** `.dispatch { display: flex }` outranks + the UA's `[hidden] { display: none }`, so the dispatch form and the Foxglove + link were on screen whenever the code believed they were hidden — a defect + since M3, invisible until M7 gave the page a signed-out state that shows a + dispatch box you cannot use. Fixed with an explicit `[hidden]` rule in + `style.css`. +- **A false positive worth recording.** The first browser run reported "broker + connected" against a broker that was *not* the one under test: the workstation + already had an M3-era mosquitto on 9001, and the page connects to + `location.hostname:`. The measurement above uses a container broker on + a non-default port, confirmed by its own connection log. If you are testing + this on a box that already runs a fleet, check *which* broker answered before + believing a green light. + +## 6. The tailnet half — **asserted by the policy, not measured here** + +"A device off the tailnet reaches nothing" is a property of +[`policy.hujson`](../../mote_bringup/tailscale/policy.hujson), which is applied +in a browser and enforced by Tailscale's coordination plane. Two things make +that better than an untested claim: + +- **Tailscale evaluates the policy's own `tests` block on save** and refuses to + store a policy that fails one. The block asserts `tag:robot` is denied + `tag:robot:22`, `:8765` and `:1883` — the acceptance criterion, checked by the + thing that enforces it. +- WireGuard denies by default and nothing in this repo publishes a port to the + internet, which is the M0 property this milestone inherits rather than adds. + +**Not yet applied to the live tailnet.** The policy file is committed and +reviewed; pasting it into the admin console is a one-time operator action, and +the M0 default (allow-all between devices) stands until then. Worth doing +alongside the next robot provisioning so the tag rules and a real enrollment are +exercised together. + +## 7. Cost + +| | measured | +|---|---| +| password hash (PBKDF2-SHA512, 101 iterations) | **0.034 ms** | +| ACL file, 10 robots + 2 operators | **3640 bytes, 108 lines** | +| regenerate + reload, per enrollment | one file write pair + one `kill -HUP` | + +Nothing here is a scaling concern at any fleet size this design targets. The ACL +grows ~9 lines per robot, so a hundred robots is a ~30 KB file mosquitto reads +once per SIGHUP. + +**On the iteration count.** 101 is mosquitto's own default and is low by +password-hashing standards — deliberately not raised, because it is not what the +security rests on: these are 24-byte `secrets.token_urlsafe` passwords (~192 +bits of entropy), never human-chosen, so an offline attack on the hash is +infeasible regardless of the KDF cost. Raising it would slow every broker +connection to defend against a threat that does not apply. The count is encoded +*in* each hash, so it can be raised later without invalidating existing entries. + +## 8. Interoperability checks + +- **Our `$7$` hash is mosquitto's.** `test_credentials.py` generates a hash with + the real `mosquitto_passwd` and verifies it with our code, so the + reimplementation cannot drift from the broker that has to read it. (Format + confirmed against mosquitto 2.0.20: `$7$101$<12-byte salt b64>$<64-byte key + b64>`, PBKDF2-HMAC-SHA512.) +- **The browser's CONNECT is well-formed.** `ui_test.mjs` asserts the username + and password flags and payload ordering under node, against the same file the + browser loads. It also asserts the module still exports no `encodePublish` — + if that ever appears, the "enforced by omission" half of the split is gone and + the test is what should stop it. +- **An M3 registry upgrades in place.** `test_registry.py` builds an M1/M3-era + SQLite file by hand, opens it with the M7 `Registry`, and checks the rows + survive and the new columns appear — `CREATE TABLE IF NOT EXISTS` cannot alter + an existing table, so without the migration every enrollment would fail on a + missing column. + +## 9. Not verified here + +- **Nothing has run on real hardware.** No robot has been re-enrolled against an + M7 server, and `mote-01`'s agent is still using an M1/M3 anonymous connection. + The upgrade is one `pixi run enroll` per robot (§7 of the runbook) and it is + the first thing to do on the next bench session. Until then, **an M7 broker + will refuse the existing robot** — this is a breaking change for a deployed + fleet, by design, and the agent's log says exactly what to run. +- **The tailnet policy is not applied** (§6). +- **`pixi run fleet-broker-ws` itself was not run.** §5 used a container + mosquitto with the same image and the same generated credential files, but + started by hand on non-default ports rather than through `broker.sh --docker`, + because the workstation already had a broker on 1883/9001. What that leaves + untested is the script's own `--network host` invocation and its + `docker kill -s HUP` reload path — the *credentials* reaching a container + broker is confirmed, the wrapper around it is not. +- **No adversarial testing.** Nothing here attempts to defeat the ACL, only to + confirm it denies the specific things the design says it should. diff --git a/docs/fleet/security.md b/docs/fleet/security.md new file mode 100644 index 0000000..986f085 --- /dev/null +++ b/docs/fleet/security.md @@ -0,0 +1,234 @@ +# Fleet security — the authorization contract + +Who may connect to what, what each of them may say, and how a credential is +issued, rotated and revoked. This is the third of the fleet's contracts — +[`control-plane.md`](control-plane.md) specifies the MQTT wire and +[`fleet-api.md`](fleet-api.md) the HTTP one; this specifies the **rules applied +to both**. + +| | | +|---|---| +| **Contract version** | `v1` | +| **Authority** | [`mote_fleet/server/credentials.py`](../../mote_fleet/server/credentials.py) (policy), [`mote_bringup/tailscale/policy.hujson`](../../mote_bringup/tailscale/policy.hujson) (network) | +| **Kept honest by** | `mote_fleet/test/test_broker_acl.py` (a real broker), `test_credentials.py`, `test_fleet_server.py`, and the policy file's own `tests` block | +| **Milestone** | M7. Measurements: [`m7-verification.md`](m7-verification.md) | + +## What changed, and why it is worth the words + +Through M3 the fleet had **one credential on one path**: an operator token on +`POST /v1/robots//dispatch`. Everything else — the roster, the basemaps, the +broker's address, and the whole MQTT topic tree — was open to anything that +could reach the port. The justification was the tailnet: WireGuard authenticates +every peer, and nothing is exposed to the internet. + +That justification is true and it is not enough, for one reason: **it makes +every robot as trusted as the fleet server.** A robot is a small computer that +drives around a building, is physically reachable, and runs the largest +dependency tree in the system. If one is compromised, a single boundary means +the attacker inherits the whole fleet — reads every robot's commands, forges +every robot's health, and dispatches to all of them. + +M7 does not replace the tailnet. It stops the tailnet being the *only* thing +between a compromised robot and the rest of the fleet. + +--- + +## The principals + +Four kinds of thing hold a credential. Their namespaces are **disjoint by +construction**, not by a runtime check: a `robot_id` is a lowercase DNS label +(`protocol.ID_RE`), so it can never contain an underscore, and the other two +usernames always do. + +| Principal | Broker username | HTTP credential | +|---|---|---| +| robot | its `robot_id` — `mote-01` | an enrollment token, once | +| operator | `op__<4 hex>` | an operator token (bearer) | +| fleet server | `fleet_server` | — (it *is* the server) | +| anyone else | — | — | + +### What each may do on the broker + +Generated into `$MOTE_FLEET_HOME/broker/acl` from the registry. `allow_anonymous +false` is the other half: a client with no username never reaches the ACL. + +| | publish | subscribe | +|---|---|---| +| **robot `mote-01`** | `mote/v1/mote-01/{presence,health,pose,task/status}` | `mote/v1/mote-01/task/command` | +| **operator** | *nothing, anywhere* | `mote/v1/+/{presence,health,pose,task/status}` | +| **fleet server** | `mote/v1/+/task/command` | `mote/v1/#` | + +Three consequences worth stating outright: + +- **A robot cannot read another robot's commands, or forge another robot's + health.** That is the milestone's acceptance criterion, and + `test_broker_acl.py` asserts it against a real mosquitto. +- **An operator cannot publish a command.** Through M3 this was a property of + *our* browser client (`ui/mqtt.mjs` implements no PUBLISH packet). It is now + also a rule of the broker's, so it holds for a hand-rolled client, `curl`, or + anything else an operator's credential is pasted into. Dispatch goes through + the API, where it is authorized and audited, because that is the only place it + can be *attributed*. +- **An operator cannot read `task/command` either.** "Who dispatched what" is a + question the audit log answers with a name attached; the broker cannot + attribute a message to a person, so reading commands off it would be evidence + that looks authoritative and is not. + +### What each may do over HTTP + +| Route | Credential | +|---|---| +| `GET /healthz` | none | +| `GET /` and the static UI | none | +| `POST /v1/enroll` | an enrollment token, in the body | +| **everything else under `/v1`** | an operator token, `Authorization: Bearer` | + +Two routes stay open on purpose. `/healthz` because a liveness probe that needs +a secret is a liveness probe nobody wires up, and it discloses only that a fleet +server is running. The **static UI** because the page has to load before it can +ask for a token — it is public code and contains no fleet data until it has one. + +The gate lives in one place (`do_GET`, in front of every `/v1` path) rather than +in each handler, so a route added later is authenticated by default and has to +opt *out* somewhere a reviewer looks. + +--- + +## Credential lifecycle + +### Robots — issued at enrollment, rotated by re-enrolling + +`POST /v1/enroll` answers with the broker credential alongside the identity: + +```json +{"schema":1,"robot_id":"mote-01","broker":{"host":"fleet-box","port":1883, + "username":"mote-01","password":"…"}} +``` + +The robot writes it to `$MOTE_HOME/fleet.yaml`, mode `0600`. That is the **only +time the plaintext is ever sent**; the registry keeps a hash. So: + +- **Rotation is `pixi run enroll`.** Enrollment is idempotent on the hardware + fingerprint, so re-running it returns the same identity with a *new* password. + There is no second mechanism to build, document, or forget. +- **A lost password cannot be looked up**, only replaced. If a robot's + `~/.mote` is wiped, enrol it again. +- **An update cannot clobber it**, because `MOTE_HOME` is outside the package — + the same property that protects identity, maps and calibration (M0). + +### Operators — one act, two credentials, one revocation + +`fleetctl operator new --name ` mints the HTTP token *and* the +subscribe-only broker login together. The browser fetches the broker half from +`/v1/config` using the HTTP half, so an operator only ever handles one secret. + +`fleetctl operator revoke --token ` closes both, because the broker's +password file is *regenerated from the rows* — a revoked operator is absent from +the query and therefore absent from the file. The row itself is kept: who *had* +access is part of the record. + +### The fleet server — generated once, kept + +Stored in the registry's `settings` table. Regenerating it per restart would +lock the server out of its own broker for the window between starting and +reloading. + +--- + +## How the broker learns about a credential + +The `password_file` and `acl_file` are **generated, never hand-edited** — a +projection of the registry, rewritten whole: + +``` +registry.db ──(credentials.render_*)──> $MOTE_FLEET_HOME/broker/{passwd,acl} + │ + broker.sh reload ──SIGHUP──> mosquitto +``` + +Regenerated and reloaded on every enrollment (by the fleet server) and on every +operator change (by `fleetctl`). `fleetctl broker sync` does it on demand, for +the two cases where nothing else would: a broker restarted with empty files, and +a registry restored from a backup. + +**A failed reload is reported, never fatal.** The files are correct on disk +either way, and a broker started afterwards reads them; what must not happen is +a silent success while a robot is being refused. + +Both files are written `0600` via a temporary file that is chmod-ed *before* the +rename, so a secret is never briefly world-readable. So is the registry, which +holds enrollment tokens, operator tokens and operator broker passwords in the +clear. **`$MOTE_FLEET_HOME` is a secret store** — back it up accordingly. + +--- + +## The network boundary + +[`mote_bringup/tailscale/policy.hujson`](../../mote_bringup/tailscale/policy.hujson) +is the source of truth for the tailnet policy; the admin console is a copy of +it. Paste it at `login.tailscale.com/admin/acls/file`. + +It grants exactly what something in this repo dials, and it **denies robot to +robot on every port** — there is no rule granting `tag:robot` access to +`tag:robot`, because in v1 there is no robot-to-robot anything. That absence is +checked by the policy's own `tests` block, which Tailscale evaluates on save and +refuses to store a policy that fails. + +The rule that matters most for something *other* than the fleet: robots reach +`tag:inference:5601,5602`, and nothing else does. The inference wire is +unauthenticated by design (`depth_wire.py`), so the network is the only thing +deciding who may speak it. + +--- + +## Packages + +Every dependency is pinned by sha256 in the committed `pixi.lock` (2274 hashes +at the time of writing), and provisioning runs `pixi install --locked`, which +**aborts** if the lockfile is not up to date with the manifest rather than +solving something new. A robot therefore installs the exact dependency set that +was tested; a silent re-solve on a machine nobody is watching is the thing this +forbids. + +**Signing is not in place**, and honestly cannot be until there is something to +sign: the prefix.dev `mote` channel does not ship a robot package yet (that is +M5). The trust root today is: the lockfile's hashes, HTTPS to the channel, and +the tailnet. Signature verification is M5's to add, and the design doc's +verification ledger still carries it as **(verify)**. + +--- + +## What M7 does *not* do + +Stated so the posture is not read as more than it is. + +- **No mTLS on the broker.** Username/password over the WireGuard tunnel, which + is already end-to-end encrypted between peers. Client certificates would add a + CA to operate and rotate for a fleet whose transport is already authenticated. + The seam is unchanged: mosquitto reads `cafile`/`certfile` instead of + `password_file`, and the ACL is keyed on the certificate CN rather than the + username. +- **No expiry on operator tokens.** They are revocable but not + self-expiring, and the browser keeps one in `localStorage`. Real sessions — + OIDC or GitHub, per fleet.md Q7 — are what replaces this, and they replace the + *minting*, not the checking: the API's gate does not change shape. +- **No Foxglove tokens**, because `foxglove_bridge` is M2 and does not exist + yet. What M7 contributes is the tailnet rule that already scopes port 8765 to + operators; the bridge's own token lands with the bridge. +- **No per-customer isolation.** One tailnet, one broker, one registry — Regime + C in the design doc, designed for and not built. +- **No secret at rest encryption.** `$MOTE_FLEET_HOME` and `$MOTE_HOME` are + `0600` files on a disk you trust. + +--- + +## If something is refused + +| Symptom | Cause | Fix | +|---|---|---| +| agent logs `broker refused this agent (Not authorized)` | the broker has no credential for this robot | `pixi run enroll` on the robot; check the server reloaded the broker | +| every robot refused at once, right after a restart | broker started before the fleet server ever wrote the files | `pixi run -e fleet fleetctl -- broker sync` | +| dashboard shows `broker refused: bad username or password` | the operator was revoked, or the broker has not reloaded | mint a new operator, or `fleetctl broker sync` | +| `fleetctl` says `401` on `robots`/`audit` | read routes need a token since M7 | `export MOTE_FLEET_TOKEN=` | +| a robot publishes and nothing arrives | it published outside its own prefix — mosquitto drops it **silently** | check the topic against the ACL table above | +| `--advertise-tags` fails, "tags are invalid or not permitted" | the tag has no owner in the tailnet policy | apply `policy.hujson` first | diff --git a/mote_bringup/provisioning/user-data.template b/mote_bringup/provisioning/user-data.template index 52e8eea..3b8f55f 100644 --- a/mote_bringup/provisioning/user-data.template +++ b/mote_bringup/provisioning/user-data.template @@ -84,8 +84,15 @@ runcmd: # 3. The software. Until the prefix.dev channel ships a robot package (M5), # this is the same source checkout + pixi build the robot runs today. + # + # `install --locked` before the build (M7): it aborts if pixi.lock is not + # up to date with pixi.toml, rather than solving something new. Every + # package in that lockfile is pinned by sha256, so a robot provisions the + # exact dependency set that was tested — a silent re-solve on a robot + # nobody is watching is the thing this forbids. - [bash, -c, "sudo -u @USER@ -H bash -lc 'curl -fsSL https://pixi.sh/install.sh | bash'"] - [bash, -c, "sudo -u @USER@ -H bash -lc 'git clone @REPO@ ~/Mote'"] + - [bash, -c, "sudo -u @USER@ -H bash -lc 'cd ~/Mote && ~/.pixi/bin/pixi install --locked'"] - [bash, -c, "sudo -u @USER@ -H bash -lc 'cd ~/Mote && ~/.pixi/bin/pixi run build'"] # 4. Host-level setup the package cannot carry: udev rules, wifi power save, diff --git a/mote_bringup/tailscale/policy.hujson b/mote_bringup/tailscale/policy.hujson new file mode 100644 index 0000000..fdb390c --- /dev/null +++ b/mote_bringup/tailscale/policy.hujson @@ -0,0 +1,153 @@ +// The Mote tailnet access policy (fleet.md Q7, milestone M7). +// +// This is the *outer* boundary. Inside it, the broker has per-robot credentials +// and an ACL (docs/fleet/security.md) and the fleet API has operator tokens; +// this file decides which machines may open a socket to which port at all, and +// it is what answers the milestone's first acceptance criterion — a device that +// is not on the tailnet reaches nothing, because nothing here is published to +// the internet and Tailscale denies by default. +// +// The second criterion it contributes to: **robots cannot reach each other.** +// There is deliberately no rule granting tag:robot any access to tag:robot. In +// v1 there is no robot-to-robot anything — no coordination, no shared map +// service, no peer discovery (fleet.md "Non-goals") — so the absence of a rule +// here is the design, not an omission. +// +// APPLYING IT. Tailscale keeps the policy in its admin console, not in a repo, +// so this file is the source of truth and the console is a copy: +// +// 1. https://login.tailscale.com/admin/acls/file +// 2. paste this file, "Preview" to see what changes, then Save. +// +// Keep the two in step by editing here first. `tailscale-policy` in a CI job or +// the `gitops-pusher` tool can automate the push; at one-operator scale, don't. +// The tags must exist (below) before any machine runs `pixi run tailnet` with +// them — `--advertise-tags` on an undeclared tag fails with "requested tags are +// invalid or not permitted", which is the first thing to check if joining a +// robot fails. +{ + // ---- who owns the tags ------------------------------------------------ + // + // A tagged device belongs to the tailnet rather than to a person, which is + // what lets it outlive the operator's account and — the practical reason — + // stops its key expiring after 180 days and silently dropping a robot off the + // network months later. + "tagOwners": { + "tag:robot": ["autogroup:admin"], + "tag:fleet": ["autogroup:admin"], + "tag:inference": ["autogroup:admin"], + }, + + // ---- the ports, named once ------------------------------------------- + "hosts": {}, + + // ---- who may reach what ---------------------------------------------- + // + // Tailscale denies anything not listed. Every rule below exists because + // something in this repo dials that port; if a rule looks unused, the thing + // that used it has gone, and the rule should go too. + "acls": [ + // Operators (you, on a laptop or a phone) reach the fleet server's API and + // dashboard, and the broker's WebSocket listener the dashboard subscribes + // through. 1883 as well, so `fleetctl watch` works from a workstation. + { + "action": "accept", + "src": ["autogroup:member"], + "dst": ["tag:fleet:8080", "tag:fleet:1883", "tag:fleet:9001"], + }, + + // Operators reach a robot for the single-robot deep view (foxglove_bridge, + // M2) and for hands-on work. Note this is the *only* rule that opens a + // robot's ports to anything. + { + "action": "accept", + "src": ["autogroup:member"], + "dst": ["tag:robot:22", "tag:robot:8765"], + }, + + // Operators reach the GPU box directly, for `pixi run inference-health` and + // the bench tools. + { + "action": "accept", + "src": ["autogroup:member"], + "dst": ["tag:inference:5601", "tag:inference:5602", "tag:inference:22"], + }, + + // A robot reaches the fleet server: enrollment + the registry API on 8080, + // the control-plane broker on 1883. Nothing else, and in particular not the + // dashboard's WebSocket listener — a robot has no browser. + { + "action": "accept", + "src": ["tag:robot"], + "dst": ["tag:fleet:8080", "tag:fleet:1883"], + }, + + // A robot reaches the inference server's depth and detect wires. This is + // the rule that makes "run it on a trusted network" true for a protocol + // that is unauthenticated by design (depth_wire.py): the wire has no + // credentials, so the network is what decides who may speak it. + { + "action": "accept", + "src": ["tag:robot"], + "dst": ["tag:inference:5601", "tag:inference:5602"], + }, + + // Deliberately absent, and each absence is load-bearing: + // + // tag:robot -> tag:robot no robot-to-robot anything in v1, and + // this is the acceptance criterion + // tag:fleet -> tag:robot the fleet server never dials a robot; the + // agent is the sole egress and it dials out + // tag:inference -> anything the GPU box answers, it never initiates + // * -> autogroup:internet no exit nodes; robots use their own link + ], + + // ---- SSH -------------------------------------------------------------- + // + // Tailscale SSH, so getting onto a robot needs a tailnet identity rather than + // a key that has been copied around. "check" re-authenticates the operator in + // a browser periodically; robots are not reachable by each other here either. + "ssh": [ + { + "action": "check", + "src": ["autogroup:member"], + "dst": ["tag:robot", "tag:fleet", "tag:inference"], + "users": ["autogroup:nonroot", "root"], + }, + ], + + // ---- tests ------------------------------------------------------------ + // + // Tailscale evaluates these on every save and refuses a policy that breaks + // one, so the acceptance criteria are checked by the thing enforcing them + // rather than by a person reading the rules above. + "tests": [ + { + "src": "tag:robot", + "accept": [ + "tag:fleet:1883", + "tag:fleet:8080", + "tag:inference:5601", + ], + "deny": [ + // The M7 criterion: one robot cannot reach another, on any port. + "tag:robot:22", + "tag:robot:8765", + "tag:robot:1883", + // A robot has no business in the dashboard's listener or on the GPU + // box's shell. + "tag:fleet:9001", + "tag:inference:22", + ], + }, + { + "src": "tag:inference", + "deny": ["tag:robot:8765", "tag:fleet:8080", "tag:fleet:1883"], + }, + { + "src": "tag:fleet", + // The agent dials the server, never the other way round. + "deny": ["tag:robot:8765", "tag:robot:22"], + }, + ], +} diff --git a/mote_fleet/README.md b/mote_fleet/README.md index ba6d5f0..06e24b5 100644 --- a/mote_fleet/README.md +++ b/mote_fleet/README.md @@ -56,10 +56,11 @@ pixi run -e fleet fleetctl -- dispatch mote-01 goto kitchen | Script | | |---|---| | [`server/fleet_server.py`](server/fleet_server.py) | the fleet API: enrollment, roster, dispatch, audit, basemaps, and the UI — stdlib `http.server` | -| [`server/registry.py`](server/registry.py) | the SQLite row store: robots, enrollment tokens, operators, the audit log, transactional id allocation | -| [`server/fleetctl.py`](server/fleetctl.py) | operator CLI: tokens, roster, dispatch, audit, watch | +| [`server/registry.py`](server/registry.py) | the SQLite row store: robots, enrollment tokens, operators, broker credentials, the audit log, transactional id allocation | +| [`server/credentials.py`](server/credentials.py) | who may connect to the broker and say what — mosquitto's `$7$` hash, and the `password_file`/`acl_file` generated from the registry (M7) | +| [`server/fleetctl.py`](server/fleetctl.py) | operator CLI: tokens, operators, broker sync, roster, dispatch, audit, watch | | [`server/ui/`](server/ui/) | the dashboard: `index.html`, `app.mjs`, `map.mjs` (basemap + the Q5 transform), `mqtt.mjs` (a subscribe-only MQTT client) | -| [`server/mosquitto.conf`](server/mosquitto.conf), [`broker.sh`](server/broker.sh) | the broker, its WebSocket listener, and where its state goes | +| [`server/mosquitto.conf`](server/mosquitto.conf), [`broker.sh`](server/broker.sh) | the broker, its WebSocket listener, its generated credentials, and where its state goes | The server imports `mote_fleet.protocol` from the source tree by path (the `depth_server.py` pattern) and nothing else — no ROS, no framework, no ament. @@ -86,9 +87,10 @@ pixi run test # colcon: everything except the broker tests pixi run -e dev test-fleet # + the real-broker end-to-end run ``` -Four tiers, so the same files give full coverage wherever they run: +Five tiers, so the same files give full coverage wherever they run: -- **contract** (`test_protocol.py`, `test_fleet_server.py`, `test_registry.py`) +- **contract** (`test_protocol.py`, `test_fleet_server.py`, `test_registry.py`, + `test_credentials.py`) — the code, the JSON Schema files and the doc's field tables checked against each other, and every HTTP route over a real socket with an injected publisher, so a payload or status-code change that nobody described fails here @@ -101,7 +103,16 @@ Four tiers, so the same files give full coverage wherever they run: loads. Skips where there is no node. `browser_check.mjs` is the other half — a real headless browser against a running stack, which needs more than CI has, so it is an operator's tool rather than a test. +- **authorization** (`test_broker_acl.py`) — a real mosquitto reading the real + generated `password_file` and `acl_file`, asked to do the things M7 says it + must refuse. Asserts on *delivery*, never on a return code: mosquitto denies + silently, granting a forbidden subscription at SUBACK and simply never + delivering, so a suite checking return codes would pass with no ACL at all. + Skips where there is no broker. - **end to end** (`test_e2e_fleet.py`) — a real mosquitto, the real fleet server, the `enroll` CLI, a real paho client, and the actual `mote_tasks` behaviour tree driving a mock Nav2; including a dispatch that goes out through - the API. Skips where there is no broker. + the API. Since M7 the broker it runs against is **authenticated and ACL'd, + starting from empty credential files**, so the chain only completes if + enrollment issues a credential and the reload puts it in force. Skips where + there is no broker. diff --git a/mote_fleet/mote_fleet/agent.py b/mote_fleet/mote_fleet/agent.py index c70c4cf..e72b79e 100644 --- a/mote_fleet/mote_fleet/agent.py +++ b/mote_fleet/mote_fleet/agent.py @@ -104,6 +104,20 @@ def yaw_from_quaternion(q) -> float: ) +def _connack_ok(args) -> bool: + """Did this ``on_connect`` describe a *successful* CONNACK? + + paho hands the code as the second positional argument under both callback + APIs — an int in 1.x, a ``ReasonCode`` in 2.x — and both compare equal to 0 + on success. A callback with no code at all (the test fake, older shims) is + taken as success, because refusal is the thing that has to be explicit. + """ + if len(args) < 2: + return True + code = args[1] + return getattr(code, "value", code) == 0 + + def paho_client(client_id: str): """A paho client, imported here so the module stays importable without it.""" import paho.mqtt.client as mqtt @@ -125,6 +139,11 @@ def __init__(self, client_factory=paho_client, **node_kwargs): self.broker_host = self.declare_parameter("broker_host", "").value self.broker_port = self.declare_parameter("broker_port", 0).value + # Normally the credential comes from fleet.yaml, written by `enroll`. + # These exist for the same reason broker_host does: a bench robot + # pointed at a throwaway broker, without an enrollment behind it. + self.broker_username = self.declare_parameter("broker_username", "").value + self.broker_password = self.declare_parameter("broker_password", "").value health_period = self.declare_parameter("health_period", 5.0).value pose_period = self.declare_parameter("pose_period", 1.0).value self.map_frame = self.declare_parameter("map_frame", "map").value @@ -168,8 +187,8 @@ def __init__(self, client_factory=paho_client, **node_kwargs): # ---- connection ----------------------------------------------------- - def _resolve(self) -> tuple[str, str, int] | None: - """``(robot_id, broker_host, broker_port)`` once this robot can join.""" + def _resolve(self) -> tuple[str, str, int, tuple[str, str] | None] | None: + """``(robot_id, host, port, credential)`` once this robot can join.""" robot_id = identity.robot_id() if not robot_id: self.get_logger().warning( @@ -180,8 +199,12 @@ def _resolve(self) -> tuple[str, str, int] | None: return None host = self.broker_host port = self.broker_port + credential = None + if self.broker_username: + credential = (self.broker_username, self.broker_password) if not host: - configured = fleet_config.broker() + config = fleet_config.load() + configured = fleet_config.broker(config) if not configured: self.get_logger().warning( f"no broker configured at {fleet_config.config_path()} — " @@ -190,7 +213,18 @@ def _resolve(self) -> tuple[str, str, int] | None: ) return None host, port = configured - return robot_id, host, int(port or fleet_config.DEFAULT_BROKER_PORT) + if credential is None: + credential = fleet_config.broker_credentials(config) + if credential is None: + # An M7 broker refuses this outright; an older one does not. Say + # what to do rather than leaving "not authorised" to be decoded. + self.get_logger().warning( + f"no broker credential in {fleet_config.config_path()} — this " + "robot enrolled before credentials existed. Re-run " + "'pixi run enroll' to be issued one; connecting anonymously", + throttle_duration_sec=300.0, + ) + return robot_id, host, int(port or fleet_config.DEFAULT_BROKER_PORT), credential def _try_connect(self): if self.client is not None: @@ -198,9 +232,14 @@ def _try_connect(self): resolved = self._resolve() if resolved is None: return - self.robot_id, host, port = resolved + self.robot_id, host, port, credential = resolved client = self._client_factory(f"mote-agent-{self.robot_id}") + if credential is not None: + # The broker's ACL is keyed on this username, so it is what confines + # the agent to its own branch of the topic tree (M7). Set before the + # connect, because it travels in the CONNECT packet. + client.username_pw_set(*credential) client.on_connect = self._on_connect client.on_disconnect = self._on_disconnect client.on_message = self._on_mqtt_message @@ -231,6 +270,20 @@ def _try_connect(self): def _on_connect(self, client, _userdata, *args): """paho's thread. Subscribe here; leave anything ROS-shaped to the executor by asking _drain_inbound for a snapshot.""" + # paho calls this for a *refused* CONNACK too, and since M7 that is a + # thing that happens — a robot whose credential the broker has not + # reloaded yet. Reporting it as connected would leave the agent + # publishing into a socket the broker is about to close, silently. + if not _connack_ok(args): + self.connected = False + self.get_logger().error( + f"broker refused this agent ({args[1] if len(args) > 1 else '?'}). " + "If this says 'not authorised', the broker has no credential for " + f"{self.robot_id} — re-run 'pixi run enroll' on this robot, and " + "make sure the fleet server reloaded the broker.", + throttle_duration_sec=60.0, + ) + return self.connected = True self.get_logger().info(f"connected to broker as {self.robot_id}") client.subscribe( diff --git a/mote_fleet/mote_fleet/enroll.py b/mote_fleet/mote_fleet/enroll.py index a692214..aa21083 100644 --- a/mote_fleet/mote_fleet/enroll.py +++ b/mote_fleet/mote_fleet/enroll.py @@ -98,6 +98,10 @@ def persist(server: str, answer: dict) -> dict: server=server, broker_host=broker.get("host") or "", broker_port=broker.get("port") or fleet_config.DEFAULT_BROKER_PORT, + # M7: the answer also carries this robot's broker credential, and this + # is the only time it is ever sent. Enrolling again issues a new one. + broker_username=broker.get("username") or "", + broker_password=broker.get("password") or "", ) @@ -164,6 +168,13 @@ def main(argv=None): print(f" identity: {identity.identity_path()}") print(f" fleet: {fleet_config.config_path()}") print(f" broker: {broker.get('host')}:{broker.get('port')}") + if broker.get("username"): + print(f" login: {broker['username']} (password written, not shown)") + else: + print( + " login: none — this server issued no broker credential, so the " + "agent will connect anonymously" + ) print("next: start the agent with 'pixi run agent'") diff --git a/mote_fleet/mote_fleet/fleet_config.py b/mote_fleet/mote_fleet/fleet_config.py index 6edd121..8f0cd58 100644 --- a/mote_fleet/mote_fleet/fleet_config.py +++ b/mote_fleet/mote_fleet/fleet_config.py @@ -5,6 +5,8 @@ broker: host: fleet-box # MQTT control plane port: 1883 + username: mote-01 # M7: this robot's broker credential + password: "…" Written by ``enroll`` from the server's own answer, so a robot learns its broker from the same exchange that gave it its id — there is no second thing to @@ -15,6 +17,12 @@ cannot take the robot off the fleet. The host names are normally MagicDNS names on the tailnet (``fleet-box``, not an IP), which is what keeps the file valid when the fleet server changes networks. + +**Since M7 this file holds a secret**, so it is written ``0600``. The broker +password is issued at enrollment and exists nowhere else in plaintext — the +registry keeps only its hash — which means the way to replace a lost or leaked +one is to enrol again. That is deliberate: rotation is an idempotent command the +robot already runs, not a separate mechanism to build and remember. """ import os @@ -52,15 +60,42 @@ def broker(config: dict | None = None) -> tuple[str, int] | None: return host, int(entry.get("port", DEFAULT_BROKER_PORT)) -def save(*, server: str, broker_host: str, broker_port: int = DEFAULT_BROKER_PORT): - record = { - "schema": SCHEMA, - "server": server, - "broker": {"host": broker_host, "port": int(broker_port)}, - } +def broker_credentials(config: dict | None = None) -> tuple[str, str] | None: + """``(username, password)`` for the broker, or None if this robot has none. + + None is the honest answer for a robot enrolled before M7: it means "connect + anonymously and find out", and against an M7 broker it will be refused with + a message that says to enrol again. + """ + config = load() if config is None else config + if not config: + return None + entry = config.get("broker") or {} + username = entry.get("username") + if not username: + return None + return username, entry.get("password") or "" + + +def save( + *, + server: str, + broker_host: str, + broker_port: int = DEFAULT_BROKER_PORT, + broker_username: str = "", + broker_password: str = "", +): + broker_entry = {"host": broker_host, "port": int(broker_port)} + if broker_username: + broker_entry["username"] = broker_username + broker_entry["password"] = broker_password + record = {"schema": SCHEMA, "server": server, "broker": broker_entry} path = config_path() path.parent.mkdir(parents=True, exist_ok=True) tmp = path.with_name(f".fleet.yaml.{os.getpid()}") tmp.write_text(yaml.safe_dump(record, sort_keys=False)) + # 0600 on the temporary file, before the rename: the broker password must + # never exist at the final path in a world-readable state, not even briefly. + os.chmod(tmp, 0o600) os.replace(tmp, path) return record diff --git a/mote_fleet/server/broker.sh b/mote_fleet/server/broker.sh index dd9409d..221a9c2 100755 --- a/mote_fleet/server/broker.sh +++ b/mote_fleet/server/broker.sh @@ -20,20 +20,74 @@ # The config is one file either way. This script strips the websockets stanza # out of a copy when the local binary cannot honour it, rather than shipping two # configs that drift. +# +# CREDENTIALS (M7). The broker is no longer anonymous: it reads broker/passwd +# and broker/acl under $MOTE_FLEET_HOME, both GENERATED from the registry by the +# fleet server. This script creates them empty if they are missing, so a broker +# started before the server has ever run comes up refusing everyone rather than +# refusing to start -- and `broker.sh reload` is how the server tells a running +# broker to re-read them when a robot enrols or an operator is revoked. set -euo pipefail FLEET_HOME="${MOTE_FLEET_HOME:-$HOME/.mote-fleet}" HERE="$(cd "$(dirname "$0")" && pwd)" -CONF="$HERE/mosquitto.conf" +# The shipped config, unless something points elsewhere. A second fleet on one +# box, and the tests, need a listener on another port, and there is no way to +# say that on mosquitto's command line once the file names an explicit listener. +CONF="${MOTE_BROKER_CONF:-$HERE/mosquitto.conf}" IMAGE="${MOTE_BROKER_IMAGE:-eclipse-mosquitto:2}" +CONTAINER="${MOTE_BROKER_CONTAINER:-mote-broker}" MODE=local +# How a running broker is told to re-read the generated credential files. The +# mode is recorded at startup because "which broker is running" is not something +# a reload arriving minutes later can work out for itself. +PID_FILE="$FLEET_HOME/broker.pid" +MODE_FILE="$FLEET_HOME/broker.mode" + +if [ "${1:-}" = "reload" ]; then + RUNNING_MODE="$(cat "$MODE_FILE" 2>/dev/null || echo)" + case "$RUNNING_MODE" in + docker) + exec docker kill -s HUP "$CONTAINER" + ;; + local) + PID="$(cat "$PID_FILE" 2>/dev/null || echo)" + if [ -z "$PID" ] || ! kill -0 "$PID" 2>/dev/null; then + echo "no running broker at pid '${PID:-}' ($PID_FILE)" >&2 + exit 1 + fi + exec kill -HUP "$PID" + ;; + *) + echo "no broker started by this script (looked in $MODE_FILE)" >&2 + echo " the credential files are correct on disk regardless; a" >&2 + echo " broker started by hand or by systemd needs its own SIGHUP" >&2 + exit 1 + ;; + esac +fi + if [ "${1:-}" = "--docker" ]; then MODE=docker shift fi -mkdir -p "$FLEET_HOME" +mkdir -p "$FLEET_HOME" "$FLEET_HOME/broker" + +# Fail closed, and start anyway. An empty password file admits nobody, which is +# the right state for a broker whose fleet server has never run -- as opposed to +# a broker that will not start, which looks like a broken deployment. +for generated in passwd acl; do + if [ ! -f "$FLEET_HOME/broker/$generated" ]; then + printf '# Generated by the mote fleet server. Empty: no client may connect.\n' \ + > "$FLEET_HOME/broker/$generated" + chmod 600 "$FLEET_HOME/broker/$generated" + echo " created an empty $FLEET_HOME/broker/$generated — nothing can" >&2 + echo " connect until the fleet server writes it (start fleet-server, or" >&2 + echo " run 'fleetctl broker sync')" >&2 + fi +done if [ "$MODE" = docker ]; then if ! command -v docker >/dev/null; then @@ -47,7 +101,8 @@ if [ "$MODE" = docker ]; then # --user: the image's mosquitto user cannot write a state directory owned by # whoever runs this, and the fleet's retained state should stay theirs. echo "broker: $IMAGE (docker) state: $FLEET_HOME config: $CONF" - exec docker run --rm --name mote-broker --network host \ + echo docker > "$MODE_FILE" + exec docker run --rm --name "$CONTAINER" --network host \ --user "$(id -u):$(id -g)" \ -v "$CONF:/mosquitto/config/mosquitto.conf:ro" \ -v "$FLEET_HOME:/mosquitto/data" \ @@ -84,4 +139,8 @@ else fi cd "$FLEET_HOME" +# `exec` keeps this shell's pid, so $$ is the broker's pid once it replaces us — +# which is what `broker.sh reload` sends its SIGHUP to. +echo local > "$MODE_FILE" +echo $$ > "$PID_FILE" exec "$BROKER" -c "$RUNTIME_CONF" "$@" diff --git a/mote_fleet/server/credentials.py b/mote_fleet/server/credentials.py new file mode 100644 index 0000000..9a6ff74 --- /dev/null +++ b/mote_fleet/server/credentials.py @@ -0,0 +1,276 @@ +"""Broker credentials: who may connect, and what they may say. + +M1 shipped an anonymous broker and said so in three places. This module is what +replaces it (fleet.md Q7, milestone M7): every client authenticates, and every +client is confined by an ACL derived from the registry — so *the topic tree +stops being a convention and becomes an enforced boundary*. A robot that is +compromised, misconfigured, or simply running the wrong ``robot_id`` cannot read +another robot's commands or forge another robot's health. + +Three principals, and their namespaces are **disjoint by construction** rather +than by a runtime check: + +=============== ========================== ==================================== +principal broker username may +=============== ========================== ==================================== +robot its ``robot_id`` publish its own presence/health/pose/ + task-status; subscribe its own + task/command — nothing else +operator ``op__<4 hex>`` subscribe the fleet-wide read topics; + **publish nothing, anywhere** +fleet server ``fleet_server`` publish task/command for any robot; + subscribe the whole tree +=============== ========================== ==================================== + +A ``robot_id`` is a lowercase DNS label (``protocol.ID_RE``), so it can never +contain an underscore — which is exactly why the other two usernames carry one. +No enrolled robot can ever collide with an operator or with the server, and +there is no ordering or precedence question to get wrong. ``test_credentials`` +asserts that property directly rather than trusting this paragraph. + +**Why generate mosquitto's hash here rather than shell out to +``mosquitto_passwd``.** The fleet server's whole dependency list is "python" +(``fleet_server.py``), and it may well run in a container that has no broker +binary in it at all. The format is published and stable — ``$7$`` is PBKDF2- +HMAC-SHA512, 101 iterations, 12-byte salt, 64-byte key, both base64 — so +``hashlib`` produces it in six lines. ``test_credentials.py`` checks the output +against the real ``mosquitto_passwd`` wherever one exists, so this cannot drift +from the broker that has to read it. + +**Why the ACL enumerates robots instead of using mosquitto's ``pattern`` / +``%u`` substitution.** One ``pattern read mote/v1/%u/task/command`` line would +cover every robot forever and never need regenerating. It is rejected because +the password file has to be regenerated on every enrollment regardless, so the +pattern buys no reload the fleet was not already doing — and it would silently +extend robot-shaped rights to *any* authenticated username, including +operators. The enumerated form is what an operator can read top to bottom and +confirm that ``mote-01`` appears in exactly its own five lines. +""" + +import base64 +import hashlib +import os +import re +import secrets +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from mote_fleet import protocol # noqa: E402 + +#: How a running broker is told to re-read the files this module writes. +#: ``broker.sh`` is the only thing that knows whether the broker it started is a +#: local process or a container, so the signal goes through it rather than +#: through a pid this module would have to guess at. +DEFAULT_RELOAD_CMD = [str(Path(__file__).resolve().parent / "broker.sh"), "reload"] + +#: mosquitto's ``$7$`` password hash: PBKDF2-HMAC-SHA512. The iteration count is +#: part of the encoded hash, so raising it here is backwards compatible — old +#: entries keep verifying at their own count until they are next rotated. +HASH_TAG = "7" +ITERATIONS = 101 +SALT_BYTES = 12 +KEY_BYTES = 64 + +#: The server's own broker identity. Underscore, so it cannot be a ``robot_id``. +SERVER_USER = "fleet_server" + +#: Operator usernames. Same reasoning; the slug is only there to make a broker +#: log line legible, and the suffix is what makes it unique. +OPERATOR_PREFIX = "op_" + +#: What an operator's browser subscribes to: the read half of the control plane, +#: every robot. Deliberately *not* ``mote/v1/#`` — that would include +#: ``task/command``, and an operator has no business reading commands issued to +#: a robot by somebody else out of the broker rather than out of the audit log. +OPERATOR_READ = (protocol.PRESENCE, protocol.HEALTH, protocol.POSE, protocol.STATUS) + +#: What a robot publishes. The mirror of OPERATOR_READ, scoped to itself. +ROBOT_WRITE = (protocol.PRESENCE, protocol.HEALTH, protocol.POSE, protocol.STATUS) + +#: ...and the one topic it reads. +ROBOT_READ = (protocol.COMMAND,) + +_SLUG_RE = re.compile(r"[^a-z0-9]+") + + +# ---- passwords ----------------------------------------------------------- + + +def new_password() -> str: + """A broker password. URL-safe so it survives every config file and CLI + quoting rule between here and the robot.""" + return secrets.token_urlsafe(24) + + +def hash_password(password: str, *, salt: bytes | None = None) -> str: + """A password in mosquitto's ``password_file`` format. + + ``$7$$$`` — verified byte for byte + against ``mosquitto_passwd`` in the tests. + """ + salt = secrets.token_bytes(SALT_BYTES) if salt is None else salt + key = hashlib.pbkdf2_hmac("sha512", password.encode(), salt, ITERATIONS, KEY_BYTES) + return ( + f"${HASH_TAG}${ITERATIONS}$" + f"{base64.b64encode(salt).decode()}${base64.b64encode(key).decode()}" + ) + + +def verify_password(password: str, encoded: str) -> bool: + """Does ``password`` produce ``encoded``? Used by the tests, and by anyone + debugging a broker that says "bad username or password".""" + try: + _, tag, iterations, salt_b64, key_b64 = encoded.split("$") + if tag != HASH_TAG: + return False + salt = base64.b64decode(salt_b64) + expected = base64.b64decode(key_b64) + except (ValueError, TypeError): + return False + key = hashlib.pbkdf2_hmac( + "sha512", password.encode(), salt, int(iterations), len(expected) + ) + return secrets.compare_digest(key, expected) + + +def operator_username(name: str) -> str: + """``op_michael_3f9a`` — legible in a broker log, unique, and impossible to + confuse with a ``robot_id``.""" + slug = _SLUG_RE.sub("_", name.strip().lower()).strip("_")[:24] or "operator" + return f"{OPERATOR_PREFIX}{slug}_{secrets.token_hex(2)}" + + +def is_robot_username(username: str) -> bool: + return protocol.valid_id(username) + + +# ---- the two files mosquitto reads --------------------------------------- + + +def render_passwd(users: dict) -> str: + """``username:hash`` per line, sorted so the file only changes when the + credentials do (a diff of noise is a diff nobody reads).""" + lines = [f"{user}:{hashed}" for user, hashed in sorted(users.items())] + return "\n".join(lines) + ("\n" if lines else "") + + +def render_acl(*, robots=(), operators=(), server_user: str = SERVER_USER) -> str: + """The ACL file, generated from the registry. + + Read it as the fleet's authorization policy in full: there is no default + rule, no ``topic`` line outside a ``user`` block, and therefore nothing any + principal may do that is not written here. ``allow_anonymous false`` in + ``mosquitto.conf`` is the other half — a client with no username never gets + as far as this file. + """ + root = f"{protocol.ROOT}/{protocol.VERSION}" + out = [ + "# Generated by the mote fleet server from the registry — do not edit.", + "# Regenerated on every enrollment and every operator change; the broker", + "# re-reads it on SIGHUP (mote_fleet/server/broker.sh reload).", + "#", + "# There is no rule outside a `user` block, so nothing is permitted that", + "# is not written below, and mosquitto.conf refuses anonymous clients.", + "", + f"user {server_user}", + "# The fleet API: the only principal that may issue a command, and it may", + "# only do so after authorizing an operator and writing the audit row.", + f"topic write {root}/+/{protocol.COMMAND}", + f"topic read {root}/#", + "", + ] + for user in sorted(operators): + out.append(f"user {user}") + out.append("# An operator: the read half of the control plane, fleet-wide.") + out.append("# No write rule at all — dispatch is the fleet API's, not theirs.") + out.extend(f"topic read {root}/+/{leaf}" for leaf in OPERATOR_READ) + out.append("") + for robot_id in sorted(robots): + out.append(f"user {robot_id}") + out.append(f"# {robot_id}: its own branch of the tree and nothing else.") + out.extend(f"topic write {root}/{robot_id}/{leaf}" for leaf in ROBOT_WRITE) + out.extend(f"topic read {root}/{robot_id}/{leaf}" for leaf in ROBOT_READ) + out.append("") + return "\n".join(out) + + +def write_private(path: Path, text: str) -> Path: + """Write ``text`` to ``path`` atomically, readable only by this user. + + Atomic because mosquitto may re-read either file at any moment — a SIGHUP it + was already handling, or a restart — and half a password file is a fleet + that cannot connect. ``0600`` set on the temporary file *before* it is + renamed, so the secret is never briefly world-readable. + """ + path = Path(path) + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_name(f".{path.name}.{os.getpid()}") + tmp.write_text(text) + os.chmod(tmp, 0o600) + os.replace(tmp, path) + return path + + +class BrokerAuth: + """The generated ``password_file`` + ``acl_file`` pair, and the reload. + + Owned by the fleet server, which regenerates both from the registry whenever + a credential changes: a robot enrolls, an operator is minted, an operator is + revoked. Nothing else writes them, and hand-edits are lost on the next + enrollment — which is why both files say so at the top. + """ + + def __init__(self, directory, *, reload_cmd=None): + self.directory = Path(directory).expanduser() + self.passwd_path = self.directory / "passwd" + self.acl_path = self.directory / "acl" + self.reload_cmd = reload_cmd + self.last_reload_error = "" + + def write(self, *, users: dict, robots=(), operators=()) -> None: + write_private(self.passwd_path, render_passwd(users)) + write_private(self.acl_path, render_acl(robots=robots, operators=operators)) + + def reload(self) -> tuple[bool, str]: + """Ask the broker to re-read both files. ``(ok, detail)``. + + A credential the broker has not read yet is a robot that cannot connect, + so the failure is reported to the caller rather than swallowed — but it + is never fatal: the files are correct on disk and the next broker start + picks them up. + """ + if not self.reload_cmd: + return True, "no reload command configured" + try: + done = subprocess.run( + self.reload_cmd, + shell=isinstance(self.reload_cmd, str), + capture_output=True, + text=True, + timeout=15, + ) + except (OSError, subprocess.SubprocessError) as exc: + self.last_reload_error = str(exc) + return False, str(exc) + if done.returncode != 0: + detail = (done.stderr or done.stdout or "").strip() + self.last_reload_error = detail + return False, detail + self.last_reload_error = "" + return True, (done.stdout or "").strip() + + +def sync(registry, directory, *, reload_cmd=DEFAULT_RELOAD_CMD) -> tuple[bool, str]: + """Regenerate both files from ``registry`` and reload the broker. + + The one entry point for "make the broker agree with the registry", shared by + the fleet server (which calls it on every enrollment) and ``fleetctl`` + (which calls it when an operator is minted or revoked). ``registry`` is + duck-typed on ``broker_principals()`` rather than imported, because + ``registry.py`` imports *this* module. + """ + auth = BrokerAuth(directory, reload_cmd=reload_cmd) + auth.write(**registry.broker_principals()) + return auth.reload() diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index 02297d2..b138d09 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -38,13 +38,24 @@ credential that can publish, and the UI's MQTT client cannot publish at all (``ui/mqtt.mjs`` implements no PUBLISH packet). -**Security posture for M3:** the read routes are still unauthenticated, exactly -as M1 left them, and the broker is still anonymous. What M3 adds is a credential -on the *write* path and a record of who used it. That stays proportionate only -while the tailnet is the boundary; M7 is where operator auth reaches the read -routes, per-robot broker credentials land, and Tailscale ACLs stop robots -reaching each other. Until then, do not expose this port to a network the robots -are not already trusted on. +**Security posture (M7).** Every ``/v1`` route now needs a credential: +``POST /v1/enroll`` an enrollment token, everything else an operator token as a +bearer header. Two things stay deliberately open — ``/healthz``, because a +liveness probe that needs a secret is a liveness probe nobody wires up, and the +static UI, because the page has to load in order to ask for a token, and it +contains no fleet data until it has one. + +This process is also the **issuer** of broker credentials. It generates the +``password_file`` and ``acl_file`` mosquitto reads, from the registry, whenever +a credential changes — a robot enrols, an operator is minted or revoked — and +asks the broker to re-read them (``--broker-reload-cmd``). So the answer to "who +may connect to the control plane, and say what" lives in one place and is +derived, never hand-maintained. :mod:`credentials` holds the format and the +policy; ``docs/fleet/security.md`` is the contract. + +The tailnet is still the outer boundary and still the thing that keeps this port +off the public internet. What changed is that it is no longer the *only* +boundary. """ import argparse @@ -59,6 +70,7 @@ from pathlib import Path sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +import credentials # noqa: E402 from mote_fleet import protocol # noqa: E402 from registry import ( # noqa: E402 ID_PREFIX, @@ -68,6 +80,8 @@ fleet_home, ) +DEFAULT_RELOAD_CMD = credentials.DEFAULT_RELOAD_CMD + MAX_BODY = 64 * 1024 #: Longest task string the API will forward. The grammar itself belongs to the @@ -110,10 +124,21 @@ class BrokerLink: the tests inject a fake in its place. """ - def __init__(self, host: str, port: int = 1883, keepalive: int = 30): + def __init__( + self, + host: str, + port: int = 1883, + keepalive: int = 30, + *, + credential=None, + ): self.host = host self.port = int(port) self.keepalive = keepalive + # ``(username, password)``. Since M7 the broker's ACL grants exactly one + # principal the right to publish task/command, and it is this one — so + # dispatch cannot be bypassed by talking to the broker directly. + self.credential = credential self._client = None self._lock = threading.Lock() @@ -126,6 +151,8 @@ def _connect(self): ) except AttributeError: # paho 1.x client = mqtt.Client(client_id="mote-fleet-api") + if self.credential: + client.username_pw_set(*self.credential) client.reconnect_delay_set(min_delay=1, max_delay=30) client.connect(self.host, self.port, keepalive=self.keepalive) client.loop_start() @@ -224,6 +251,26 @@ def do_GET(self): path = path.rstrip("/") or "/" params = urllib.parse.parse_qs(query) + # One gate, in front of everything under /v1 (M7). Before this, only + # /v1/audit checked — which meant the roster, the basemaps and the + # broker's address were readable by anything that could reach the port. + # Putting it here rather than in each handler is deliberate: a route + # added later is authenticated by default, and has to opt out in a place + # a reviewer looks at. + # + # The two things outside /v1 are open for reasons, not by omission: + # /healthz, because a liveness probe that needs a secret is one nobody + # wires up, and the static UI, because the page has to load before it + # can ask for a token. (`POST /v1/enroll` is open too, in do_POST — it + # carries its own credential in the body.) + operator = None + if path.startswith("/v1/"): + try: + operator = self._operator() + except Unauthorized as exc: + self._error(401, str(exc)) + return + if path == "/healthz": self._send( 200, @@ -236,7 +283,7 @@ def do_GET(self): }, ) elif path == "/v1/config": - self._send(200, self.server.ui_config()) + self._send(200, self.server.ui_config(operator)) elif path == "/v1/robots": self._send( 200, @@ -308,9 +355,15 @@ def _enroll(self, body: dict): self._error(code, str(exc)) return + # The robot now has a broker credential the broker has never heard of. + # Publishing the generated files and reloading is what closes that gap, + # and it happens before the answer goes out so a robot that connects the + # instant it is enrolled is not refused. + reloaded, detail = self.server.publish_credentials() print( f"enrolled {robot['robot_id']} " - f"({'new' if created else 'existing'}, {robot['fingerprint']})", + f"({'new' if created else 'existing'}, {robot['fingerprint']})" + f"{'' if reloaded else f'; BROKER NOT RELOADED: {detail}'}", file=sys.stderr, ) self._send( @@ -325,6 +378,10 @@ def _enroll(self, body: dict): "broker": { "host": self.server.broker_host, "port": self.server.broker_port, + # The one time this password is ever sent. The registry keeps + # only its hash, so a robot that loses it enrols again. + "username": robot["robot_id"], + "password": robot["broker_password"], }, "contract": f"{protocol.ROOT}/{protocol.VERSION}", }, @@ -425,11 +482,8 @@ def _dispatch(self, robot_id: str, body: dict): ) def _audit(self, params: dict): - try: - self._operator() - except Unauthorized as exc: - self._error(401, str(exc)) - return + # Authorization happens in do_GET's gate, which covers every /v1 route + # rather than only this one. try: limit = int((params.get("limit") or ["100"])[0]) except ValueError: @@ -516,6 +570,7 @@ def __init__( broker_ws_host, broker_ws_port, foxglove_url, + broker_auth=None, ): super().__init__(address, FleetHandler) self.registry = registry @@ -528,16 +583,50 @@ def __init__( self.broker_ws_host = broker_ws_host self.broker_ws_port = broker_ws_port self.foxglove_url = foxglove_url + self.broker_auth = broker_auth + + # -- broker credentials ----------------------------------------------- + + def publish_credentials(self) -> tuple[bool, str]: + """Regenerate the broker's ``password_file`` + ``acl_file``, and reload. + + Called after anything that changes who may connect. Regenerating the + whole pair from the registry rather than appending is what keeps the + files a *projection* of the registry: a revoked operator disappears + because the query no longer returns them, not because something + remembered to delete a line. + """ + if self.broker_auth is None: + return True, "no broker credential directory configured" + try: + self.broker_auth.write(**self.registry.broker_principals()) + except OSError as exc: + return False, f"could not write broker credentials: {exc}" + return self.broker_auth.reload() # -- what the browser needs to bootstrap ------------------------------ - def ui_config(self) -> dict: + def ui_config(self, operator: dict | None = None) -> dict: """Everything the UI cannot work out from its own URL. ``ws_host`` is null by default and the page falls back to the host it was loaded from, so reaching the fleet box by MagicDNS, by tailnet address or over localhost all work with no per-deployment build step. + + Since M7 it also carries **this operator's** subscribe-only broker + credential. That is why the route needs a token: it hands out a + credential, and the one it hands out is the caller's own — revoke the + operator and the next page load has nothing to connect with. """ + broker = { + "ws_host": self.broker_ws_host, + "ws_port": self.broker_ws_port, + "host": self.broker_host, + "port": self.broker_port, + } + if operator and operator.get("broker_user"): + broker["username"] = operator["broker_user"] + broker["password"] = operator["broker_password"] return { "schema": protocol.SCHEMA, "contract": f"{protocol.ROOT}/{protocol.VERSION}", @@ -548,12 +637,7 @@ def ui_config(self) -> dict: "pose": protocol.POSE, "status": protocol.STATUS, }, - "broker": { - "ws_host": self.broker_ws_host, - "ws_port": self.broker_ws_port, - "host": self.broker_host, - "port": self.broker_port, - }, + "broker": broker, "foxglove_url": self.foxglove_url, "maps": bool(self.maps_dir and self.maps_dir.is_dir()), } @@ -663,22 +747,44 @@ def serve( broker_ws_host=None, broker_ws_port=9001, foxglove_url=FOXGLOVE_URL, + broker_auth_dir=None, + broker_reload_cmd=None, ) -> FleetServer: """Build a listening server. The caller runs it (or its ``serve_forever``).""" broker_host = broker_host or socket.gethostname() - return FleetServer( + registry = Registry(db) + auth = ( + credentials.BrokerAuth(broker_auth_dir, reload_cmd=broker_reload_cmd) + if broker_auth_dir + else None + ) + server = FleetServer( (host, port), - Registry(db), + registry, broker_host=broker_host, broker_port=broker_port, id_prefix=id_prefix, - publisher=publisher or BrokerLink(broker_host, broker_port), + publisher=publisher + or BrokerLink( + broker_host, + broker_port, + credential=( + credentials.SERVER_USER, + registry.server_broker_password(), + ), + ), maps_dir=maps_dir, ui_dir=ui_dir, broker_ws_host=broker_ws_host, broker_ws_port=broker_ws_port, foxglove_url=foxglove_url, + broker_auth=auth, ) + # Write the credential files at startup, before serving. A fleet box that + # was restored from a registry backup, or upgraded from M3, has rows but no + # generated files; this is what makes "start the server" enough. + server.publish_credentials() + return server def main(argv=None): @@ -727,6 +833,20 @@ def main(argv=None): parser.add_argument( "--id-prefix", default=ID_PREFIX, help="allocated ids look like -01" ) + parser.add_argument( + "--broker-auth-dir", + default=str(fleet_home() / "broker"), + help="where to generate the broker's password_file and acl_file " + "(default: $MOTE_FLEET_HOME/broker). Empty disables generation, which " + "leaves whatever credentials are already there untouched.", + ) + parser.add_argument( + "--broker-reload-cmd", + default=" ".join(DEFAULT_RELOAD_CMD), + help="how to make a running broker re-read those files " + "(default: broker.sh reload). Empty means never reload, for a broker " + "somebody else restarts.", + ) args = parser.parse_args(argv) server = serve( @@ -741,18 +861,30 @@ def main(argv=None): ui_dir=None if args.no_ui else UI_DIR, foxglove_url=args.foxglove_url, id_prefix=args.id_prefix, + broker_auth_dir=args.broker_auth_dir or None, + broker_reload_cmd=args.broker_reload_cmd or None, ) print( f"mote-fleet on http://{args.host}:{args.port} db={args.db} " f"broker={server.broker_host}:{server.broker_port} " f"ws={server.broker_ws_host or ''}:{server.broker_ws_port} " - f"maps={len(server.list_maps())}", + f"maps={len(server.list_maps())} " + f"credentials={args.broker_auth_dir or ''}", file=sys.stderr, ) + if server.broker_auth and server.broker_auth.last_reload_error: + print( + "the broker did not reload the credential files " + f"({server.broker_auth.last_reload_error}). They are correct on " + "disk; a broker started later will read them, and a running one " + "needs a SIGHUP.", + file=sys.stderr, + ) if not server.registry.operators(): print( - "no operator tokens exist yet — dispatch will refuse every request. " - "Mint one with: fleetctl operator new --name ", + "no operator tokens exist yet — every route except /healthz and " + "enrollment will refuse. Mint one with: " + "fleetctl operator new --name ", file=sys.stderr, ) thread = threading.Thread(target=server.serve_forever, daemon=True) diff --git a/mote_fleet/server/fleetctl.py b/mote_fleet/server/fleetctl.py index 58e5444..c20d00e 100644 --- a/mote_fleet/server/fleetctl.py +++ b/mote_fleet/server/fleetctl.py @@ -6,11 +6,18 @@ fleetctl token new mint an enrollment token fleetctl operator new --name michael mint an operator token + fleetctl broker sync push credentials to the broker fleetctl robots the registry roster fleetctl dispatch mote-01 "goto kitchen" send a task, follow it to terminal fleetctl audit who dispatched what fleetctl watch tail the whole fleet's topics +**Everything except `token`/`operator`/`broker` needs an operator token** since +M7 — the API's read routes are authorized too, not just dispatch — and the +broker is no longer anonymous, so `watch` and `dispatch` fetch the operator's +own subscribe-only broker login from `/v1/config` rather than connecting +unauthenticated. One credential in your environment, both paths. + **Dispatch goes through the fleet API, not to the broker.** M1's version published straight to `task/command`, which is right when there is one operator and no record; from M3 the API is the single write path, so every dispatch is @@ -39,8 +46,9 @@ import urllib.error # noqa: E402 import urllib.request # noqa: E402 +import credentials # noqa: E402 from mote_fleet import protocol # noqa: E402 -from registry import Registry, RegistryError, default_db # noqa: E402 +from registry import Registry, RegistryError, default_db, fleet_home # noqa: E402 DEFAULT_SERVER = "http://localhost:8080" DEFAULT_BROKER = "localhost" @@ -88,7 +96,23 @@ def on_connect(client, _userdata, *_args): return on_connect -def _client(broker: str, port: int, client_id: str, subscriptions=()): +def _broker_credential(args) -> tuple[str, str] | None: + """This operator's subscribe-only broker login, from ``/v1/config``. + + Since M7 the broker is not anonymous, so ``watch`` and the status half of + ``dispatch`` need a credential — and the right one to use is the operator's + own, fetched with the operator token they already hold. The alternative, + reading the registry file, would only work while sitting on the fleet box, + which is exactly the constraint ``fleetctl`` exists to avoid. + """ + config = _get(args.server, "/v1/config", token=_token(args)) + broker = config.get("broker") or {} + if not broker.get("username"): + return None + return broker["username"], broker.get("password") or "" + + +def _client(broker: str, port: int, client_id: str, subscriptions=(), credential=None): """A connected client that **re-subscribes every time it connects**. Subscriptions belong to an MQTT session, and paho's default session is a @@ -105,6 +129,8 @@ def _client(broker: str, port: int, client_id: str, subscriptions=()): except AttributeError: # paho 1.x client = mqtt.Client(client_id=client_id) + if credential: + client.username_pw_set(*credential) client.on_connect = subscriber(subscriptions) # Reconnect forever with backoff: an operator's terminal should outlive a # fleet-server redeploy without them noticing. @@ -134,7 +160,7 @@ def cmd_token(args): def cmd_robots(args): - robots = _get(args.server, "/v1/robots").get("robots", []) + robots = _get(args.server, "/v1/robots", token=_token(args)).get("robots", []) if not robots: print("no robots enrolled") return @@ -147,6 +173,28 @@ def cmd_robots(args): ) +def _sync_broker(registry, args, why: str): + """Push the registry's credentials to the broker, and say what happened. + + Minting or revoking an operator changes who may connect, and a change the + broker has not read is not in force yet — a revoked operator would keep + receiving the fleet's telemetry until something reloaded it. Reported rather + than silent, because the files are right on disk either way and the operator + needs to know which half succeeded. + """ + ok, detail = credentials.sync( + registry, args.broker_auth_dir, reload_cmd=args.broker_reload_cmd or None + ) + if ok: + print(f" (broker credentials {why} and reloaded)", file=sys.stderr) + else: + print( + f" (broker credentials {why}, but the broker did not reload: " + f"{detail or 'no running broker'} — it will read them on next start)", + file=sys.stderr, + ) + + def cmd_operator(args): registry = Registry(args.db) if args.action == "new": @@ -160,15 +208,46 @@ def cmd_operator(args): "into the dashboard)", file=sys.stderr, ) + _sync_broker(registry, args, "written") return if args.action == "revoke": if not args.token: sys.exit("which token? fleetctl operator revoke --token ") - sys.exit(0 if registry.revoke_operator(args.token) else "no such live token") + if not registry.revoke_operator(args.token): + sys.exit("no such live token") + # Revocation is only real once the broker has forgotten the credential. + _sync_broker(registry, args, "rewritten") + return for row in registry.operators(): state = f"revoked {row['revoked_at']}" if row["revoked_at"] else "live" used = row["last_used_at"] or "never used" - print(f"{row['name']:16} {state:24} {used:22} {row['note']}") + print( + f"{row['name']:16} {state:24} {used:22} " + f"{row['broker_user'] or '-':24} {row['note']}" + ) + + +def cmd_broker(args): + """Regenerate the broker's credential files from the registry, and reload. + + The fleet server does this by itself on every enrollment. This verb is for + the two cases where nothing else would: a broker restarted with an empty + file, and a registry restored from a backup. + """ + registry = Registry(args.db) + auth = credentials.BrokerAuth(args.broker_auth_dir) + principals = registry.broker_principals() + if args.action == "show": + print(f"password_file {auth.passwd_path}") + print(f"acl_file {auth.acl_path}") + print( + f"principals {len(principals['robots'])} robots, " + f"{len(principals['operators'])} operators, 1 server" + ) + for user in sorted(principals["users"]): + print(f" {user}") + return + _sync_broker(registry, args, "regenerated") def cmd_audit(args): @@ -240,6 +319,7 @@ def report(command_id): args.port, f"fleetctl-{os.getpid()}", subscriptions=[protocol.topic(args.robot_id, protocol.STATUS)], + credential=_broker_credential(args), ) client.on_message = on_message client.loop_start() @@ -297,6 +377,7 @@ def on_message(_client, _userdata, message): protocol.STATUS, ) ], + credential=_broker_credential(args), ) client.on_message = on_message print(f"watching {robot} on {args.broker}:{args.port} (ctrl-c to stop)") @@ -341,7 +422,20 @@ def main(argv=None): parser.add_argument( "--token", default="", - help=f"operator token for the write routes (default: ${TOKEN_ENV})", + help=f"operator token for the API (default: ${TOKEN_ENV})", + ) + parser.add_argument( + "--broker-auth-dir", + default=str(fleet_home() / "broker"), + help="the broker's generated password_file/acl_file directory " + "(default: $MOTE_FLEET_HOME/broker)", + ) + parser.add_argument( + "--broker-reload-cmd", + default=" ".join(credentials.DEFAULT_RELOAD_CMD), + help="how to make a running broker re-read those files (default: " + "broker.sh reload). Needed when the broker was not started by " + "broker.sh — e.g. a container someone else runs.", ) sub = parser.add_subparsers(dest="cmd", required=True) @@ -363,6 +457,12 @@ def main(argv=None): p_operator.add_argument("--note", default="", help="what it is for") p_operator.set_defaults(func=cmd_operator) + p_broker = sub.add_parser( + "broker", help="regenerate the broker's credential files from the registry" + ) + p_broker.add_argument("action", choices=["sync", "show"], nargs="?", default="sync") + p_broker.set_defaults(func=cmd_broker) + p_robots = sub.add_parser("robots", help="list enrolled robots") p_robots.set_defaults(func=cmd_robots) diff --git a/mote_fleet/server/mosquitto.conf b/mote_fleet/server/mosquitto.conf index ec02f51..d607132 100644 --- a/mote_fleet/server/mosquitto.conf +++ b/mote_fleet/server/mosquitto.conf @@ -5,20 +5,29 @@ # HiveMQ at Regime B/C -- same protocol, same topic tree, same agent code, so # that swap is a broker change and not a redesign (fleet.md Q2). # -# SECURITY: anonymous. Every client on the tailnet may publish and subscribe -# anywhere in the tree. That is the M0 substrate's bargain -- WireGuard is the -# authentication boundary and nothing here is reachable from the public internet -# -- and it is what M7 replaces with per-robot credentials (username = robot_id, -# publish confined to its own prefix) and a subscribe-only operator credential. -# Until then: do not put this broker on an interface the robots are not already -# trusted on. The bind line below is the lever for that. +# SECURITY (M7). Every client authenticates and every client is confined by an +# ACL. The two files below are GENERATED from the registry by the fleet server +# (mote_fleet/server/credentials.py) -- a robot may publish only under its own +# robot_id and read only its own task/command; an operator may read the fleet's +# presence/health/pose/task-status and publish nothing at all; only the fleet +# API may write task/command, and only after authorizing an operator and +# writing the audit row. WireGuard is still the outer boundary; this is what +# makes the topic tree an enforced one rather than a convention. +# +# Both paths are relative, and mosquitto resolves them against its working +# directory -- which broker.sh sets to $MOTE_FLEET_HOME (and the container +# mounts there), so one config file serves both ways of running it without any +# templating. broker.sh creates them empty if they are missing, which fails +# closed: an empty password file admits nobody. +allow_anonymous false +password_file broker/passwd +acl_file broker/acl # The control-plane listener the agents connect to. Left unbound (all # interfaces) because a fresh fleet box has no stable tailnet address to name # yet; on a real one, pin it: # listener 1883 100.x.y.z # the box's Tailscale address listener 1883 -allow_anonymous true # >>> websockets # MQTT-over-WebSockets, for the M3 fleet dashboard: a browser cannot speak raw diff --git a/mote_fleet/server/registry.py b/mote_fleet/server/registry.py index 978a43b..120ffaa 100644 --- a/mote_fleet/server/registry.py +++ b/mote_fleet/server/registry.py @@ -12,6 +12,20 @@ it writes before publishing. Every table is created ``IF NOT EXISTS`` on open, so an M1 registry file gains them by being opened by an M3 server. +M7 makes this file the source of truth for **broker credentials** as well +(:mod:`credentials`): a robot's is issued at enrollment, an operator's is minted +with their token, and the server's own lives in ``settings``. Nothing else knows +who may connect to the broker — the ``password_file`` and ``acl_file`` mosquitto +reads are *generated* from these rows, which is what makes revoking an operator +close their HTTP and their MQTT access in the same step. Columns added after the +table already exists are applied by :func:`_migrate`, since ``CREATE TABLE IF +NOT EXISTS`` cannot alter one. + +**This file is a secret store**, and since M7 it is created ``0600``. Enrollment +tokens, operator tokens and the operators' broker passwords are all recoverable +from it. Robot broker passwords are not: only their hash is kept, so a lost one +is re-issued by re-enrolling rather than looked up. + Two invariants are worth naming because everything else leans on them: **Allocation is transactional.** ``mote-03`` is derived from the ids already in @@ -35,9 +49,13 @@ import os import secrets import sqlite3 +import sys from datetime import datetime, timezone from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import credentials # noqa: E402 + #: The fleet box's state root — the server-side analogue of the robot's #: ``MOTE_HOME``. The registry and the broker's persistence both live here, so a #: redeploy of the server software (a container image, a git pull) replaces code @@ -84,8 +102,27 @@ remote TEXT NOT NULL DEFAULT '' ); CREATE INDEX IF NOT EXISTS audit_stamp ON audit (stamp DESC); +CREATE TABLE IF NOT EXISTS settings ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL +); """ +#: Columns added to already-existing tables after M1. ``CREATE TABLE IF NOT +#: EXISTS`` is a no-op on a table that is already there, so an M1/M3 registry +#: file would silently keep its old shape without these. +MIGRATIONS = ( + ("robots", "broker_hash", "TEXT NOT NULL DEFAULT ''"), + ("robots", "credential_issued_at", "TEXT"), + ("operators", "broker_user", "TEXT NOT NULL DEFAULT ''"), + ("operators", "broker_password", "TEXT NOT NULL DEFAULT ''"), +) + +#: The server's own broker password, in ``settings``. Generated on first use and +#: kept, because regenerating it on every restart would lock the server out of +#: its own broker until the next reload. +SERVER_PASSWORD_KEY = "broker_server_password" + ID_PREFIX = "mote" @@ -113,6 +150,11 @@ def __init__(self, path): self.path.parent.mkdir(parents=True, exist_ok=True) with self._connect() as conn: conn.executescript(SCHEMA) + _migrate(conn) + # Tokens and operator broker passwords live in here in the clear, so the + # file is the operator's to read and nobody else's. Applied on every + # open rather than only at creation, so an M1 file tightens on upgrade. + os.chmod(self.path, 0o600) def _connect(self): # isolation_level=None: transactions are opened explicitly with @@ -152,11 +194,84 @@ def _redeem(self, conn, token: str, robot_id: str): (now(), robot_id, token), ) + # ---- broker credentials --------------------------------------------- + + def server_broker_password(self) -> str: + """The fleet API's own broker password, generated once and kept. + + Kept rather than regenerated per start because the broker only learns a + new password when the generated files are reloaded — a server that + re-minted its own on every restart would spend the window between + starting and reloading unable to publish a dispatch. + """ + with self._connect() as conn: + row = conn.execute( + "SELECT value FROM settings WHERE key = ?", (SERVER_PASSWORD_KEY,) + ).fetchone() + if row is not None: + return row["value"] + password = credentials.new_password() + conn.execute( + "INSERT OR REPLACE INTO settings (key, value) VALUES (?, ?)", + (SERVER_PASSWORD_KEY, password), + ) + return password + + def issue_robot_credential(self, robot_id: str) -> str: + """Mint this robot's broker password, returning the *only* plaintext + copy there will ever be. Re-enrolling rotates it, which is the fleet's + whole robot-credential-rotation story: one command, on the robot.""" + password = credentials.new_password() + with self._connect() as conn: + changed = conn.execute( + "UPDATE robots SET broker_hash = ?, credential_issued_at = ? " + "WHERE robot_id = ?", + (credentials.hash_password(password), now(), robot_id), + ).rowcount + if not changed: + raise RegistryError(f"no such robot {robot_id}") + return password + + def broker_principals(self) -> dict: + """Everything the generated broker files are built from. + + ``{"users": {username: hash}, "robots": [...], "operators": [...]}`` — + one query so the password file and the ACL can never describe different + fleets. + """ + server_user = credentials.SERVER_USER + users = { + server_user: credentials.hash_password(self.server_broker_password()), + } + robots, operators = [], [] + with self._connect() as conn: + for row in conn.execute( + "SELECT robot_id, broker_hash FROM robots WHERE broker_hash != ''" + ): + users[row["robot_id"]] = row["broker_hash"] + robots.append(row["robot_id"]) + for row in conn.execute( + "SELECT broker_user, broker_password FROM operators " + "WHERE revoked_at IS NULL AND broker_user != ''" + ): + users[row["broker_user"]] = credentials.hash_password( + row["broker_password"] + ) + operators.append(row["broker_user"]) + return {"users": users, "robots": robots, "operators": operators} + # ---- operators ------------------------------------------------------ def new_operator(self, *, name: str, note: str = "") -> str: - """Mint an operator token. This is the credential the dispatch API - authorizes against, and the name it writes into the audit line.""" + """Mint an operator token, and the subscribe-only broker credential that + goes with it. + + One act, two credentials, deliberately: an operator needs the HTTP + bearer token to dispatch *and* a broker login to see the fleet, and + minting them together is what lets ``revoke`` close both at once. The + broker password is stored in the clear because the dashboard fetches it + on every load; the registry file is ``0600`` and that is the boundary. + """ if not name.strip(): raise RegistryError( "an operator needs a name — it is what the audit log records" @@ -164,9 +279,16 @@ def new_operator(self, *, name: str, note: str = "") -> str: token = secrets.token_urlsafe(24) with self._connect() as conn: conn.execute( - "INSERT INTO operators (token, name, note, created_at) " - "VALUES (?, ?, ?, ?)", - (token, name.strip(), note, now()), + "INSERT INTO operators (token, name, note, created_at, " + "broker_user, broker_password) VALUES (?, ?, ?, ?, ?, ?)", + ( + token, + name.strip(), + note, + now(), + credentials.operator_username(name), + credentials.new_password(), + ), ) return token @@ -344,6 +466,16 @@ def enroll( ), ) created = True + # The broker credential is issued *inside* the enrollment + # transaction, so a robot is never a registry row without one — + # and re-enrolling rotates it, which is the whole rotation + # story: one idempotent command, run on the robot. + password = credentials.new_password() + conn.execute( + "UPDATE robots SET broker_hash = ?, credential_issued_at = ? " + "WHERE robot_id = ?", + (credentials.hash_password(password), now(), robot_id), + ) row = conn.execute( "SELECT * FROM robots WHERE robot_id = ?", (robot_id,) ).fetchone() @@ -351,7 +483,12 @@ def enroll( except Exception: conn.execute("ROLLBACK") raise - return _row_to_robot(row), created + robot = _row_to_robot(row) + # The only moment this plaintext exists. It is not a column and never + # appears on `robots()` or `/v1/robots`; the caller hands it to the + # robot that just enrolled, and after that only its hash is anywhere. + robot["broker_password"] = password + return robot, created def _next_id(conn, prefix: str) -> str: @@ -367,7 +504,26 @@ def _next_id(conn, prefix: str) -> str: raise RegistryError(f"no free id left under the prefix {prefix!r}") +def _migrate(conn) -> None: + """Add any column MIGRATIONS names that this file does not have yet. + + SQLite has no ``ADD COLUMN IF NOT EXISTS``, so the existing columns are read + back and compared. Every migration is a nullable/defaulted add, which is the + only shape that can be applied to a live registry without a rewrite. + """ + for table, column, decl in MIGRATIONS: + present = { + row["name"] + for row in conn.execute(f"PRAGMA table_info({table})").fetchall() + } + if column not in present: + conn.execute(f"ALTER TABLE {table} ADD COLUMN {column} {decl}") + + def _row_to_robot(row) -> dict: robot = dict(row) robot["facts"] = json.loads(robot.get("facts") or "{}") + # A password hash is not roster data. Dropped here rather than in the API so + # that no future route can serve it by forgetting to. + robot.pop("broker_hash", None) return robot diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index 90b5a0d..14d4bc7 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -5,8 +5,13 @@ // state of the fleet the instant it loads and never polls (fleet.md Q5). The // write path is the fleet API: `POST /v1/robots//dispatch` authorizes the // operator and writes an audit line before anything reaches `task/command`. +// // The two paths are deliberately different, and the browser holds no credential -// that can publish to the broker. +// that can publish to the broker. Since M7 that is enforced twice over: this +// page's MQTT client has no PUBLISH encoder, *and* the broker credential it is +// given — the operator's own, from /v1/config — has no write rule in the +// broker's ACL. Both credentials arrive together and die together, because +// revoking the operator removes the row both are read from. // // What this view is *not* is a Foxglove replacement. It answers "where is every // robot, what is each doing, send one somewhere" and hands the single-robot @@ -38,6 +43,7 @@ const state = { const dom = {}; let mapView = null; let pending = false; +let reader = null; // -- small helpers ------------------------------------------------------- @@ -203,6 +209,9 @@ function scheduleRender() { } function render() { + // At the gate there is nothing to render, and the periodic re-render must not + // replace "paste a token" with "no robots" — which is a different claim. + if (!state.config) return; const records = [...state.robots.values()].sort((a, b) => a.id.localeCompare(b.id)); if (!state.selected && records.length) state.selected = records[0].id; renderRoster(records); @@ -392,30 +401,31 @@ async function onToken(event) { event.preventDefault(); const token = dom.token.value.trim(); localStorage.setItem(TOKEN_KEY, token); - await checkOperator(); dom.token.value = ''; + await start(); } -// The one call that tells us whether this token is any good — the audit route -// is operator-only, so a 200 is proof and a 401 is the reason to say so. -async function checkOperator() { - const token = localStorage.getItem(TOKEN_KEY); - if (!token) { - state.operator = null; - dom.operator.textContent = 'read-only — paste an operator token to dispatch'; - dom.operator.className = 'operator anonymous'; - return; - } - try { - await api('/v1/audit?limit=1'); - state.operator = token; - dom.operator.textContent = 'operator token accepted'; - dom.operator.className = 'operator ok'; - } catch (error) { - state.operator = null; - dom.operator.textContent = `token refused: ${error.message}`; - dom.operator.className = 'operator error'; - } +// Since M7 there is no read-only mode to fall back to: /v1/config is itself +// operator-only, because it hands out this operator's broker credential. So the +// page has exactly two states — signed in, or asking to be — and this is the +// asking one. It is not an error; a wall display that has been logged out looks +// like this until somebody pastes a token back in. +function showGate(reason) { + state.operator = null; + state.config = null; + dom.operator.textContent = reason; + dom.operator.className = 'operator anonymous'; + dom.brokerState.textContent = 'not connected — no operator token'; + dom.brokerState.className = 'broker offline'; + dom.roster.replaceChildren( + el('p', { + class: 'empty', + text: + 'Paste an operator token to see the fleet. Mint one on the fleet box: ' + + 'fleetctl operator new --name ', + }), + ); + renderDetail(null); } // -- boot ---------------------------------------------------------------- @@ -450,6 +460,48 @@ function brokerUrl(config) { return `${scheme}://${host}:${config.broker.ws_port}/`; } +// Everything that needs a credential. Called at load, and again whenever a +// token is pasted — so signing in never needs a page reload, and a token that +// stops working puts the page back at the gate rather than into a silent +// half-state. +async function start() { + if (!localStorage.getItem(TOKEN_KEY)) { + showGate('read-only — paste an operator token'); + return; + } + try { + state.config = await api('/v1/config'); + } catch (error) { + showGate(`token refused: ${error.message}`); + return; + } + state.operator = localStorage.getItem(TOKEN_KEY); + dom.operator.textContent = 'operator token accepted'; + dom.operator.className = 'operator ok'; + document.getElementById('contract').textContent = state.config.contract; + await loadRoster().catch((error) => console.warn('roster unavailable', error)); + + if (reader) reader.close(); + const { root, presence, health, pose, status } = state.config.topics; + reader = new BrokerReader({ + url: brokerUrl(state.config), + topics: [presence, health, pose, status].map((leaf) => `${root}/+/${leaf}`), + // The broker is authenticated since M7, and this credential is + // subscribe-only by the broker's own ACL — see docs/fleet/security.md. + credential: state.config.broker.username + ? { username: state.config.broker.username, password: state.config.broker.password } + : null, + onMessage: onBrokerMessage, + onState: (status_, detail) => { + dom.brokerState.textContent = + status_ === 'connected' ? `broker connected` : `broker ${status_}: ${detail}`; + dom.brokerState.className = `broker ${status_}`; + }, + }); + reader.connect(); + scheduleRender(); +} + export async function boot() { bind(); mapView = new MapView(dom.canvas, { @@ -471,23 +523,7 @@ export async function boot() { dom.dispatch.addEventListener('submit', onDispatch); document.getElementById('token-form').addEventListener('submit', onToken); - state.config = await api('/v1/config'); - document.getElementById('contract').textContent = state.config.contract; - await checkOperator(); - await loadRoster().catch((error) => console.warn('roster unavailable', error)); - - const { root, presence, health, pose, status } = state.config.topics; - const reader = new BrokerReader({ - url: brokerUrl(state.config), - topics: [presence, health, pose, status].map((leaf) => `${root}/+/${leaf}`), - onMessage: onBrokerMessage, - onState: (status_, detail) => { - dom.brokerState.textContent = - status_ === 'connected' ? `broker connected` : `broker ${status_}: ${detail}`; - dom.brokerState.className = `broker ${status_}`; - }, - }); - reader.connect(); + await start(); // Ages are the only thing on the page that changes without a message. setInterval(scheduleRender, 5000); diff --git a/mote_fleet/server/ui/mqtt.mjs b/mote_fleet/server/ui/mqtt.mjs index 3ab9d71..50c86a0 100644 --- a/mote_fleet/server/ui/mqtt.mjs +++ b/mote_fleet/server/ui/mqtt.mjs @@ -51,15 +51,33 @@ function packet(type, flags, body) { return new Uint8Array([(type << 4) | flags, ...encodeLength(body.length), ...body]); } -export function encodeConnect(clientId, keepalive = 30) { +// Connect flags (MQTT 3.1.1 §3.1.2.3). Only these three are ever set: there is +// no will to register, and this client has nothing to publish. +const CLEAN_SESSION = 0x02; +const HAS_PASSWORD = 0x40; +const HAS_USERNAME = 0x80; + +export function encodeConnect(clientId, keepalive = 30, credential = null) { + // Since M7 the broker is not anonymous, and what this carries is the + // operator's own subscribe-only login, fetched from /v1/config with their + // token. It grants the four read topics and no write anywhere — so "the + // browser cannot publish" is now the broker's rule, not only this file's + // omission of a PUBLISH encoder. + let flags = CLEAN_SESSION; + if (credential && credential.username) { + flags |= HAS_USERNAME; + if (credential.password) flags |= HAS_PASSWORD; + } const body = [ ...encodeString('MQTT'), 4, // protocol level 3.1.1 - 0x02, // clean session; no will, no credentials (the broker is anonymous) + flags, keepalive >> 8, keepalive & 0xff, ...encodeString(clientId), ]; + if (flags & HAS_USERNAME) body.push(...encodeString(credential.username)); + if (flags & HAS_PASSWORD) body.push(...encodeString(credential.password)); return packet(CONNECT, 0, body); } @@ -150,9 +168,10 @@ const CONNACK_REASONS = { // reconnecting. A fleet UI left open on a wall display has to survive the fleet // box restarting without somebody pressing reload. export class BrokerReader { - constructor({ url, topics, onMessage, onState, clientId, keepalive = 30 }) { + constructor({ url, topics, onMessage, onState, clientId, keepalive = 30, credential = null }) { this.url = url; this.topics = topics; + this.credential = credential; this.onMessage = onMessage || (() => {}); this.onState = onState || (() => {}); this.clientId = clientId || `mote-ui-${Math.random().toString(16).slice(2, 10)}`; @@ -178,7 +197,8 @@ export class BrokerReader { } socket.binaryType = 'arraybuffer'; this.socket = socket; - socket.onopen = () => socket.send(encodeConnect(this.clientId, this.keepalive)); + socket.onopen = () => + socket.send(encodeConnect(this.clientId, this.keepalive, this.credential)); socket.onmessage = (event) => this._onBytes(new Uint8Array(event.data)); socket.onerror = () => {}; socket.onclose = () => { diff --git a/mote_fleet/server/ui/style.css b/mote_fleet/server/ui/style.css index ef0f179..1aef966 100644 --- a/mote_fleet/server/ui/style.css +++ b/mote_fleet/server/ui/style.css @@ -350,6 +350,15 @@ main { flex: none; } +/* `hidden` is an attribute, and the UA rule that honours it is `display: none` + at the lowest specificity — so any `display` a class sets silently wins, and + the element stays on screen with `.hidden === true`. Both the dispatch form + and the Foxglove link are toggled that way, and both looked hidden in the + code and were not. Restated here so the attribute means what it says. */ +[hidden] { + display: none !important; +} + .dispatch { display: flex; gap: 6px; diff --git a/mote_fleet/test/test_agent.py b/mote_fleet/test/test_agent.py index d922298..5a04915 100644 --- a/mote_fleet/test/test_agent.py +++ b/mote_fleet/test/test_agent.py @@ -34,12 +34,16 @@ def __init__(self, client_id): self.target = None self.loop_running = False self.disconnected = False + self.credential = None self.on_connect = self.on_disconnect = self.on_message = None # -- the paho surface the agent uses -- def reconnect_delay_set(self, **_kwargs): pass + def username_pw_set(self, username, password=None): + self.credential = (username, password) + def will_set(self, topic, payload, qos=0, retain=False): self.will = SimpleNamespace(topic=topic, payload=payload, retain=retain) @@ -122,7 +126,13 @@ def home(tmp_path, monkeypatch): from mote_fleet import fleet_config identity.set_identity(id="mote-01", name="Scout", site="home") - fleet_config.save(server="http://fleet:8080", broker_host="fleet", broker_port=1883) + fleet_config.save( + server="http://fleet:8080", + broker_host="fleet", + broker_port=1883, + broker_username="mote-01", + broker_password="issued-at-enrollment", + ) # The site/floor on the wire is the *active map bundle*, not identity's # entitlement field: a coordinate is only meaningful against the floor it # was measured in. @@ -397,3 +407,44 @@ def test_pose_follows_the_map_frame_transform(fleet): assert pose["yaw"] == pytest.approx(math.pi / 2, abs=1e-3) # The coordinate travels with the floor whose map frame it is measured in. assert (pose["site"], pose["floor"]) == ("home", "ground") + + +# ---- M7: the agent authenticates ------------------------------------------- + + +def test_the_agent_presents_the_credential_from_fleet_yaml(fleet): + """Which is what confines it, at the broker, to its own branch of the tree: + the ACL is keyed on this username.""" + assert fleet.mqtt.credential == ("mote-01", "issued-at-enrollment") + + +def test_a_robot_with_no_credential_still_tries(home, monkeypatch): + """An M0/M1 robot that has not re-enrolled. It will be refused by an M7 + broker — and the log says to enrol — but the agent must not refuse to start, + because a robot whose fleet link is broken still has to boot and navigate. + """ + from mote_fleet import fleet_config + + fleet_config.save(server="http://fleet:8080", broker_host="fleet", broker_port=1883) + assert fleet_config.broker_credentials() is None + + +def test_the_credential_file_is_not_world_readable(home): + import stat + + from mote_fleet import fleet_config + + mode = stat.S_IMODE(fleet_config.config_path().stat().st_mode) + assert mode == 0o600 + + +def test_a_refused_connack_is_not_reported_as_connected(fleet): + """paho calls on_connect for a refusal too. Treating that as connected would + leave the agent publishing into a socket the broker is about to close.""" + fleet.mqtt.on_connect(fleet.mqtt, None, {}, 5, None) # 5 = not authorized + assert fleet.agent.connected is False + + +def test_a_successful_connack_is_reported_as_connected(fleet): + fleet.mqtt.on_connect(fleet.mqtt, None, {}, 0, None) + assert fleet.agent.connected is True diff --git a/mote_fleet/test/test_broker_acl.py b/mote_fleet/test/test_broker_acl.py new file mode 100644 index 0000000..9a7d5ac --- /dev/null +++ b/mote_fleet/test/test_broker_acl.py @@ -0,0 +1,329 @@ +"""M7's acceptance test: the topic tree is enforced, not merely documented. + +The milestone's criterion is *"a robot cannot read another robot's command +topic"*, and the only thing that can answer it is a real broker reading the real +generated files. So this starts mosquitto with the ``password_file`` and +``acl_file`` the fleet server would have written, from a real registry with two +enrolled robots and one operator, and then tries the things that must not work. + +Denial is asserted on **delivery**, not on a return code, because that is how +mosquitto denies: a publish to a forbidden topic is accepted at the socket and +silently dropped, and a subscribe to a forbidden filter is granted at SUBACK and +never delivers. A test that asserted an error code would pass against a broker +with no ACL at all. (Measured, and recorded in ``docs/fleet/m7-verification.md`` +§2, because it is the sort of thing that reads like a bug until you know.) + +The half this cannot prove is "off the tailnet reaches nothing", which is a +property of the Tailscale policy rather than of this code — +``mote_bringup/tailscale/policy.hujson`` and the m7 ledger cover that. +""" + +import os +import shutil +import socket +import subprocess +import time +from pathlib import Path + +import credentials +import pytest +from registry import Registry + +from mote_fleet import protocol + +mqtt = pytest.importorskip("paho.mqtt.client") + + +def mosquitto_bin() -> str | None: + """conda-forge installs the broker into ``$PREFIX/sbin``, and pixi only puts + ``bin`` on PATH — so ``which mosquitto`` says no in an environment that + has it (the same lookup ``test_e2e_fleet.py`` and ``broker.sh`` do).""" + prefix = os.environ.get("CONDA_PREFIX") + if prefix: + candidate = Path(prefix) / "sbin" / "mosquitto" + if candidate.is_file() and os.access(candidate, os.X_OK): + return str(candidate) + return shutil.which("mosquitto") + + +BROKER_BIN = mosquitto_bin() + +pytestmark = pytest.mark.skipif( + BROKER_BIN is None, + reason="needs a mosquitto broker (pixi run -e dev / -e fleet)", +) + +#: How long to wait before concluding a message is not coming. Everything here +#: is loopback and retained, so real deliveries land in single-digit +#: milliseconds; this is generous by two orders of magnitude. +SETTLE_S = 0.6 + + +def free_port() -> int: + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + return probe.getsockname()[1] + + +class Fleet: + """A registry, its generated credential files, and a broker reading them.""" + + def __init__(self, tmp_path): + self.registry = Registry(tmp_path / "registry.db") + self.passwords = {} + for fingerprint in ("serial:aaa", "serial:bbb"): + token = self.registry.new_token() + robot, _ = self.registry.enroll(token=token, fingerprint=fingerprint) + self.passwords[robot["robot_id"]] = robot["broker_password"] + self.operator_token = self.registry.new_operator(name="michael") + operator = self.registry.operator(self.operator_token) + self.operator_user = operator["broker_user"] + self.passwords[self.operator_user] = operator["broker_password"] + self.passwords[credentials.SERVER_USER] = self.registry.server_broker_password() + + auth = credentials.BrokerAuth(tmp_path / "broker") + auth.write(**self.registry.broker_principals()) + self.acl_path = auth.acl_path + + self.port = free_port() + conf = tmp_path / "mosquitto.conf" + conf.write_text( + f"listener {self.port} 127.0.0.1\n" + "allow_anonymous false\n" + f"password_file {auth.passwd_path}\n" + f"acl_file {auth.acl_path}\n" + "persistence false\n" + ) + self.process = subprocess.Popen( + [BROKER_BIN, "-c", str(conf)], + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + ) + deadline = time.monotonic() + 10 + while time.monotonic() < deadline: + try: + with socket.create_connection(("127.0.0.1", self.port), timeout=0.5): + break + except OSError: + time.sleep(0.05) + else: + self.process.kill() + pytest.fail(f"mosquitto did not start: {self.process.stderr.read()}") + self.clients = [] + + def client(self, username=None, *, password=None): + """A connected (or refused) client. ``.connack`` is the verdict.""" + handle = mqtt.Client( + mqtt.CallbackAPIVersion.VERSION2, + client_id=f"{username or 'anon'}-{len(self.clients)}", + ) + handle.received = [] + handle.connack = None + if username: + handle.username_pw_set( + username, + self.passwords[username] if password is None else password, + ) + handle.on_connect = lambda c, u, f, rc, *a: setattr(handle, "connack", rc) + handle.on_message = lambda c, u, m: handle.received.append( + (m.topic, m.payload.decode()) + ) + handle.connect("127.0.0.1", self.port, keepalive=20) + handle.loop_start() + deadline = time.monotonic() + 5 + while handle.connack is None and time.monotonic() < deadline: + time.sleep(0.02) + self.clients.append(handle) + return handle + + def close(self): + for handle in self.clients: + handle.loop_stop() + self.process.terminate() + self.process.wait(timeout=10) + + +@pytest.fixture +def fleet(tmp_path): + made = Fleet(tmp_path) + yield made + made.close() + + +def connected(handle) -> bool: + return str(handle.connack) in ("Success", "0") + + +def payloads(handle) -> list[str]: + return [payload for _, payload in handle.received] + + +# ---- authentication ----------------------------------------------------- + + +def test_an_anonymous_client_cannot_connect(fleet): + """The M1 posture, gone: reaching the port is no longer enough.""" + assert not connected(fleet.client()) + + +def test_a_wrong_password_cannot_connect(fleet): + assert not connected(fleet.client("mote-01", password="not-the-password")) + + +def test_an_enrolled_robot_connects_with_its_issued_credential(fleet): + assert connected(fleet.client("mote-01")) + + +def test_a_revoked_operator_is_gone_from_the_regenerated_files(fleet, tmp_path): + """Revocation closes the MQTT half too, because the file is a projection of + the rows — the operator is absent from the query, so absent from the file.""" + fleet.registry.revoke_operator(fleet.operator_token) + principals = fleet.registry.broker_principals() + assert fleet.operator_user not in principals["users"] + assert fleet.operator_user not in principals["operators"] + + +# ---- the acceptance criterion ------------------------------------------- + + +def test_a_robot_cannot_read_another_robots_command_topic(fleet): + """*The* M7 criterion, end to end.""" + eavesdropper = fleet.client("mote-01") + target = fleet.client("mote-02") + server = fleet.client(credentials.SERVER_USER) + eavesdropper.subscribe(protocol.topic("mote-02", protocol.COMMAND), qos=1) + target.subscribe(protocol.topic("mote-02", protocol.COMMAND), qos=1) + time.sleep(SETTLE_S) + + server.publish(protocol.topic("mote-02", protocol.COMMAND), "goto kitchen", qos=1) + time.sleep(SETTLE_S) + + assert payloads(target) == ["goto kitchen"] + assert payloads(eavesdropper) == [] + + +def test_a_robot_cannot_publish_as_another_robot(fleet): + """The other direction: no robot can forge another's health or pose, so a + roster reading `ok` is a claim that robot actually made.""" + liar = fleet.client("mote-01") + watcher = fleet.client(fleet.operator_user) + watcher.subscribe(protocol.any_robot(protocol.HEALTH), qos=1) + time.sleep(SETTLE_S) + + liar.publish(protocol.topic("mote-02", protocol.HEALTH), "forged", qos=1) + liar.publish(protocol.topic("mote-01", protocol.HEALTH), "genuine", qos=1) + time.sleep(SETTLE_S) + + assert payloads(watcher) == ["genuine"] + + +def test_a_robot_cannot_read_another_robots_health(fleet): + """Robots have no business reading each other at all — there is no + robot-to-robot coordination in v1, and this is what keeps it that way.""" + nosy = fleet.client("mote-01") + other = fleet.client("mote-02") + nosy.subscribe(protocol.any_robot(protocol.HEALTH), qos=1) + time.sleep(SETTLE_S) + + other.publish(protocol.topic("mote-02", protocol.HEALTH), "mine", qos=1) + time.sleep(SETTLE_S) + + assert payloads(nosy) == [] + + +# ---- the operator is read-only ------------------------------------------ + + +def test_an_operator_sees_the_whole_fleets_telemetry(fleet): + watcher = fleet.client(fleet.operator_user) + for leaf in (protocol.PRESENCE, protocol.HEALTH, protocol.POSE, protocol.STATUS): + watcher.subscribe(protocol.any_robot(leaf), qos=1) + time.sleep(SETTLE_S) + + fleet.client("mote-01").publish( + protocol.topic("mote-01", protocol.PRESENCE), "one", qos=1 + ) + fleet.client("mote-02").publish( + protocol.topic("mote-02", protocol.POSE), "two", qos=1 + ) + time.sleep(SETTLE_S) + + assert sorted(payloads(watcher)) == ["one", "two"] + + +def test_an_operator_cannot_publish_a_command(fleet): + """The browser's "no PUBLISH packet" is a property of our client. This is + the same property as a rule of the broker's, which is the point of M7.""" + operator = fleet.client(fleet.operator_user) + robot = fleet.client("mote-01") + robot.subscribe(protocol.topic("mote-01", protocol.COMMAND), qos=1) + time.sleep(SETTLE_S) + + operator.publish(protocol.topic("mote-01", protocol.COMMAND), "goto nowhere", qos=1) + time.sleep(SETTLE_S) + + assert payloads(robot) == [] + + +def test_an_operator_cannot_read_a_command_topic(fleet): + """Who dispatched what is answered by the audit log, which attributes it — + not by the broker, which cannot.""" + operator = fleet.client(fleet.operator_user) + server = fleet.client(credentials.SERVER_USER) + operator.subscribe(protocol.any_robot(protocol.COMMAND), qos=1) + time.sleep(SETTLE_S) + + server.publish(protocol.topic("mote-01", protocol.COMMAND), "secret", qos=1) + time.sleep(SETTLE_S) + + assert payloads(operator) == [] + + +def test_an_operator_cannot_forge_a_robots_health(fleet): + operator = fleet.client(fleet.operator_user) + watcher = fleet.client(credentials.SERVER_USER) + watcher.subscribe(protocol.any_robot(protocol.HEALTH), qos=1) + time.sleep(SETTLE_S) + + operator.publish(protocol.topic("mote-01", protocol.HEALTH), "forged", qos=1) + time.sleep(SETTLE_S) + + assert payloads(watcher) == [] + + +# ---- the server is the only writer -------------------------------------- + + +def test_the_fleet_server_can_dispatch_to_any_robot(fleet): + server = fleet.client(credentials.SERVER_USER) + one = fleet.client("mote-01") + two = fleet.client("mote-02") + one.subscribe(protocol.topic("mote-01", protocol.COMMAND), qos=1) + two.subscribe(protocol.topic("mote-02", protocol.COMMAND), qos=1) + time.sleep(SETTLE_S) + + server.publish(protocol.topic("mote-01", protocol.COMMAND), "for-one", qos=1) + server.publish(protocol.topic("mote-02", protocol.COMMAND), "for-two", qos=1) + time.sleep(SETTLE_S) + + assert payloads(one) == ["for-one"] + assert payloads(two) == ["for-two"] + + +def test_the_denial_is_silent_which_is_why_delivery_is_what_is_asserted(fleet): + """Documents the measurement the rest of this file depends on. + + mosquitto grants the subscription at SUBACK and simply never delivers, so + "did the client get an error" is not a usable signal — and a suite that + checked for one would pass against a broker with no ACL loaded. + """ + granted = [] + eavesdropper = fleet.client("mote-01") + eavesdropper.on_subscribe = lambda c, u, mid, codes, *a: granted.extend( + getattr(code, "value", code) for code in codes + ) + eavesdropper.subscribe(protocol.topic("mote-02", protocol.COMMAND), qos=1) + time.sleep(SETTLE_S) + + assert granted == [1], "mosquitto grants a forbidden subscription" + assert payloads(eavesdropper) == [] diff --git a/mote_fleet/test/test_credentials.py b/mote_fleet/test/test_credentials.py new file mode 100644 index 0000000..2b03c3d --- /dev/null +++ b/mote_fleet/test/test_credentials.py @@ -0,0 +1,220 @@ +"""Broker credentials: the hash format, the ACL, and the namespace split. + +These are the pure-function half of M7. The half that matters more — that a real +mosquitto *enforces* what ``render_acl`` writes — is ``test_broker_acl.py``, +which needs a broker; this file needs nothing and therefore runs everywhere. + +The one test here that reaches outside is the hash comparison against the real +``mosquitto_passwd``, which is what stops our reimplementation of the ``$7$`` +format from drifting away from the broker that has to read it. +""" + +import os +import shutil +import stat +import subprocess + +import credentials +import pytest + +from mote_fleet import protocol + + +def mosquitto_passwd() -> str | None: + """conda-forge puts the clients in ``bin`` (unlike the broker, which is in + ``sbin``), so PATH is enough for this one.""" + prefix = os.environ.get("CONDA_PREFIX") + if prefix: + candidate = os.path.join(prefix, "bin", "mosquitto_passwd") + if os.access(candidate, os.X_OK): + return candidate + return shutil.which("mosquitto_passwd") + + +# ---- passwords ---------------------------------------------------------- + + +def test_a_hash_round_trips(): + password = credentials.new_password() + assert credentials.verify_password(password, credentials.hash_password(password)) + + +def test_a_wrong_password_does_not_verify(): + encoded = credentials.hash_password("hunter2") + assert not credentials.verify_password("hunter3", encoded) + + +def test_the_same_password_hashes_differently_each_time(): + """A fresh salt per call, so two robots with the same password — or one + robot's two rotations — are not visibly the same in the password file.""" + first = credentials.hash_password("same") + second = credentials.hash_password("same") + assert first != second + assert credentials.verify_password("same", first) + assert credentials.verify_password("same", second) + + +def test_the_encoded_shape_is_mosquittos(): + encoded = credentials.hash_password("pw", salt=b"0123456789ab") + marker, tag, iterations, salt, key = encoded.split("$") + assert (marker, tag, iterations) == ("", "7", "101") + assert salt == "MDEyMzQ1Njc4OWFi" + assert len(key) == 88 # 64 bytes, base64 + + +def test_a_garbled_hash_is_refused_rather_than_raising(): + for bad in ("", "$7$", "notahash", "$6$101$aaaa$bbbb", "$7$x$y$z"): + assert credentials.verify_password("pw", bad) is False + + +@pytest.mark.skipif(mosquitto_passwd() is None, reason="needs mosquitto_passwd") +def test_our_hasher_agrees_with_mosquitto_passwd(tmp_path): + """The contract that matters: a hash mosquitto wrote, verified by us. + + Compared this way round rather than by generating one and asking mosquitto, + because ``mosquitto_passwd`` has no verify mode — and this direction is the + one that proves we can read what the broker reads. + """ + path = tmp_path / "passwd" + subprocess.run( + [mosquitto_passwd(), "-c", "-b", str(path), "someone", "s3cret"], check=True + ) + _, encoded = path.read_text().strip().split(":", 1) + assert credentials.verify_password("s3cret", encoded) + assert not credentials.verify_password("s3crer", encoded) + + +# ---- the namespace split ------------------------------------------------ + + +def test_an_operator_username_can_never_be_a_robot_id(): + """The whole reason operator names carry an underscore. + + Robot ids are lowercase DNS labels, so the two namespaces cannot overlap and + there is no precedence rule to get wrong — which is what lets the ACL grant + robot rights by username without checking anything at runtime. + """ + for name in ("michael", "Night Shift", "a", "!!!", "mote-01", "x" * 60): + username = credentials.operator_username(name) + assert username.startswith(credentials.OPERATOR_PREFIX) + assert not protocol.valid_id(username), username + + +def test_the_server_username_can_never_be_a_robot_id(): + assert not protocol.valid_id(credentials.SERVER_USER) + + +def test_operator_usernames_are_unique_per_mint(): + names = {credentials.operator_username("michael") for _ in range(50)} + assert len(names) > 1 + + +def test_a_robot_id_is_its_own_broker_username(): + assert credentials.is_robot_username("mote-01") + assert not credentials.is_robot_username(credentials.SERVER_USER) + + +# ---- the generated files ------------------------------------------------ + + +def test_the_password_file_is_sorted_and_one_line_per_user(): + text = credentials.render_passwd({"b": "$7$b", "a": "$7$a"}) + assert text == "a:$7$a\nb:$7$b\n" + + +def test_an_empty_password_file_is_empty_not_blank_lines(): + assert credentials.render_passwd({}) == "" + + +def test_a_robot_gets_its_own_branch_and_nothing_else(): + acl = credentials.render_acl(robots=["mote-01", "mote-02"]) + block = _block(acl, "mote-01") + assert "topic write mote/v1/mote-01/health" in block + assert "topic read mote/v1/mote-01/task/command" in block + # The acceptance criterion, as a property of the generated text. + assert "mote-02" not in block + assert "topic read mote/v1/mote-01/health" not in block + + +def test_an_operator_may_read_the_fleet_and_write_nothing(): + acl = credentials.render_acl(operators=["op_michael_1a2b"]) + block = _block(acl, "op_michael_1a2b") + assert "topic read mote/v1/+/health" in block + assert "topic read mote/v1/+/pose" in block + assert "topic write" not in block + # Not the command topic: an operator reads who dispatched what from the + # audit log, which is attributable, rather than off the broker, which is not. + assert "task/command" not in block + + +def test_only_the_server_may_write_a_command(): + acl = credentials.render_acl( + robots=["mote-01"], operators=["op_a_1111", "op_b_2222"] + ) + writers = [ + line for line in acl.splitlines() if line.startswith("topic write mote/v1/+/") + ] + assert writers == [f"topic write mote/v1/+/{protocol.COMMAND}"] + assert _block(acl, credentials.SERVER_USER).count("topic write") == 1 + + +def test_the_acl_has_no_rule_outside_a_user_block(): + """A ``topic`` line before any ``user`` line is a rule for *everyone*, which + would quietly undo the whole file. Assert the file never opens with one.""" + acl = credentials.render_acl(robots=["mote-01"], operators=["op_a_1111"]) + lines = [ + line for line in acl.splitlines() if line.strip() and not line.startswith("#") + ] + assert lines[0].startswith("user ") + assert not any(line.startswith("pattern") for line in lines) + + +def test_the_generated_files_are_private(tmp_path): + auth = credentials.BrokerAuth(tmp_path / "broker") + auth.write(users={"mote-01": "$7$x"}, robots=["mote-01"], operators=[]) + for path in (auth.passwd_path, auth.acl_path): + assert stat.S_IMODE(path.stat().st_mode) == 0o600, path + + +def test_writing_twice_replaces_rather_than_appends(tmp_path): + auth = credentials.BrokerAuth(tmp_path / "broker") + auth.write(users={"mote-01": "$7$x"}, robots=["mote-01"], operators=[]) + auth.write(users={"mote-02": "$7$y"}, robots=["mote-02"], operators=[]) + # The point of regenerating from the registry: a robot that is gone from the + # rows is gone from the file, without anything remembering to delete it. + assert "mote-01" not in auth.passwd_path.read_text() + assert "mote-01" not in auth.acl_path.read_text() + + +def test_no_reload_command_is_not_a_failure(tmp_path): + auth = credentials.BrokerAuth(tmp_path / "broker") + ok, _ = auth.reload() + assert ok + + +def test_a_failing_reload_is_reported_not_raised(tmp_path): + auth = credentials.BrokerAuth(tmp_path / "broker", reload_cmd=["false"]) + ok, _ = auth.reload() + assert not ok + assert auth.last_reload_error == "" # `false` says nothing; the code is the news + + +def test_a_missing_reload_command_is_reported_not_raised(tmp_path): + auth = credentials.BrokerAuth( + tmp_path / "broker", reload_cmd=["/nonexistent/broker.sh", "reload"] + ) + ok, detail = auth.reload() + assert not ok + assert detail + + +def _block(acl: str, user: str) -> str: + """The lines belonging to one ``user`` stanza.""" + out, collecting = [], False + for line in acl.splitlines(): + if line.startswith("user "): + collecting = line == f"user {user}" + continue + if collecting: + out.append(line) + return "\n".join(out) diff --git a/mote_fleet/test/test_e2e_fleet.py b/mote_fleet/test/test_e2e_fleet.py index ee764f6..2ea58e2 100644 --- a/mote_fleet/test/test_e2e_fleet.py +++ b/mote_fleet/test/test_e2e_fleet.py @@ -28,6 +28,7 @@ import threading import time from pathlib import Path +from types import SimpleNamespace import pytest @@ -94,15 +95,23 @@ def execute(self, goal_handle): class Operator: - """The off-robot side: publishes commands, collects everything retained.""" + """The off-robot side: publishes commands, collects everything retained. - def __init__(self, port): + It connects with the **fleet server's** broker credential, because that is + whose role it is playing — the ACL grants `task/command` to exactly one + principal (M7), and publishing a command is the thing this class exists to + do. A human operator's credential cannot do it, which is the point, and is + asserted in ``test_broker_acl.py`` rather than here. + """ + + def __init__(self, broker, credential): self.messages = [] self.client = mqtt.Client( mqtt.CallbackAPIVersion.VERSION2, client_id="operator" ) + self.client.username_pw_set(*credential) self.client.on_message = self._on_message - self.client.connect("127.0.0.1", port, keepalive=30) + self.client.connect("127.0.0.1", broker.port, keepalive=30) self.client.subscribe("mote/v1/#", qos=1) self.client.loop_start() @@ -138,10 +147,25 @@ def close(self): @pytest.fixture def broker(tmp_path): + """A broker with M7's posture: authenticated, ACL'd, nothing anonymous. + + It starts with the credential files *empty*, exactly as a fleet box does + before its server has ever run — so what this fixture sets up is the real + bootstrap, and the robot below can only connect because enrolling wrote a + credential and the reload made the broker read it. + """ port = free_port() + auth_dir = tmp_path / "broker" + auth_dir.mkdir() + (auth_dir / "passwd").write_text("") + (auth_dir / "acl").write_text("") conf = tmp_path / "mosquitto.conf" conf.write_text( - f"listener {port} 127.0.0.1\nallow_anonymous true\npersistence false\n" + f"listener {port} 127.0.0.1\n" + "allow_anonymous false\n" + f"password_file {auth_dir / 'passwd'}\n" + f"acl_file {auth_dir / 'acl'}\n" + "persistence false\n" ) process = subprocess.Popen( [BROKER_BIN, "-c", str(conf)], @@ -158,7 +182,13 @@ def broker(tmp_path): else: process.kill() pytest.fail("mosquitto did not start") - yield port + yield SimpleNamespace( + port=port, + auth_dir=auth_dir, + # The real reload path: the fleet server runs this, mosquitto re-reads + # both files, and a robot enrolled a moment ago can connect. + reload_cmd=["kill", "-HUP", str(process.pid)], + ) process.terminate() process.wait(timeout=10) @@ -172,16 +202,31 @@ def fleet_api(tmp_path, broker): host="127.0.0.1", port=0, broker_host="127.0.0.1", - broker_port=broker, + broker_port=broker.port, + # M7: the server is the issuer. It writes the broker's credential files + # and SIGHUPs it, so this exercises the real path rather than a fixture + # that pre-loads credentials the code never generated. + broker_auth_dir=broker.auth_dir, + broker_reload_cmd=broker.reload_cmd, ) thread = threading.Thread(target=server.serve_forever, daemon=True) thread.start() server.url = f"http://127.0.0.1:{server.server_address[1]}" + # Every /v1 read route needs an operator now, and so does the broker. + server.operator_token = server.registry.new_operator(name="e2e") + server.publish_credentials() yield server server.shutdown() server.server_close() +def server_credential(fleet_api): + """The fleet server's own broker login, from its registry.""" + import credentials + + return (credentials.SERVER_USER, fleet_api.registry.server_broker_password()) + + def spin_until(executor, condition, timeout=30.0): deadline = time.monotonic() + timeout while time.monotonic() < deadline: @@ -208,7 +253,7 @@ def test_enroll_then_dispatch_over_mqtt(tmp_path, monkeypatch, broker, fleet_api robot_id = identity.robot_id() assert robot_id == "mote-01" - assert fleet_config.broker() == ("127.0.0.1", broker) + assert fleet_config.broker() == ("127.0.0.1", broker.port) # Idempotent: the same machine enrolling again is the same robot. enroll.main(["--token", token]) assert identity.robot_id() == "mote-01" @@ -244,7 +289,7 @@ def test_enroll_then_dispatch_over_mqtt(tmp_path, monkeypatch, broker, fleet_api for node in (agent, tasks, nav): executor.add_node(node) - operator = Operator(broker) + operator = Operator(broker, server_credential(fleet_api)) try: assert spin_until(executor, lambda: agent.connected), "agent never connected" @@ -358,7 +403,7 @@ def test_a_dead_agent_is_reported_offline_by_the_broker( agent = MoteAgent(parameter_overrides=[Parameter("keepalive", value=2)]) executor = SingleThreadedExecutor() executor.add_node(agent) - operator = Operator(broker) + operator = Operator(broker, server_credential(fleet_api)) try: assert spin_until(executor, lambda: agent.connected) assert spin_until(executor, lambda: operator.of("presence")) @@ -378,7 +423,7 @@ def test_a_dead_agent_is_reported_offline_by_the_broker( # And a subscriber arriving afterwards is told immediately, because the # will was retained. - latecomer = Operator(broker) + latecomer = Operator(broker, server_credential(fleet_api)) try: deadline = time.monotonic() + 10 while time.monotonic() < deadline and not latecomer.of("presence"): @@ -472,7 +517,7 @@ def test_dispatch_through_the_fleet_api(tmp_path, monkeypatch, broker, fleet_api for node in (agent, tasks, nav): executor.add_node(node) - operator = Operator(broker) + operator = Operator(broker, server_credential(fleet_api)) try: assert spin_until(executor, lambda: agent.connected), "agent never connected" diff --git a/mote_fleet/test/test_fleet_server.py b/mote_fleet/test/test_fleet_server.py index d52138f..e9cbef4 100644 --- a/mote_fleet/test/test_fleet_server.py +++ b/mote_fleet/test/test_fleet_server.py @@ -95,25 +95,43 @@ def server(tmp_path): thread = threading.Thread(target=httpd.serve_forever, daemon=True) thread.start() httpd.url = f"http://127.0.0.1:{httpd.server_address[1]}" + # Since M7 every /v1 route needs an operator, so one comes with the fixture + # and `get`/`post` send it unless a test says otherwise. The tests that care + # about *absence* pass token="" and assert the 401. + httpd.token = httpd.registry.new_operator(name="fixture") yield httpd httpd.shutdown() httpd.server_close() -def get(server, path, token=None): - request = urllib.request.Request(server.url + path) +#: Sent when a call does not name a token. ``token=""`` means "send none". +DEFAULT = object() + + +def _authorize(request, server, token): + token = server.token if token is DEFAULT else token if token: request.add_header("Authorization", f"Bearer {token}") + + +def get(server, path, token=DEFAULT): + request = urllib.request.Request(server.url + path) + _authorize(request, server, token) with urllib.request.urlopen(request, timeout=10) as response: return response.status, json.loads(response.read()) -def get_bytes(server, path): - with urllib.request.urlopen(server.url + path, timeout=10) as response: +def get_bytes(server, path, token=DEFAULT): + request = urllib.request.Request(server.url + path) + _authorize(request, server, token) + with urllib.request.urlopen(request, timeout=10) as response: return response.status, response.headers["Content-Type"], response.read() def post(server, path, payload, token=None): + # Unlike `get`, this defaults to *no* token: /v1/enroll carries its own + # credential in the body, and every dispatch test is explicit about which + # operator it is acting as. request = urllib.request.Request( server.url + path, data=json.dumps(payload).encode(), @@ -155,7 +173,12 @@ def test_enrollment_returns_an_id_and_the_broker(server): assert body["name"] == "Scout" assert body["site"] == "home" assert body["created"] is True - assert body["broker"] == {"host": "fleet-box", "port": 1883} + assert body["broker"]["host"] == "fleet-box" + assert body["broker"]["port"] == 1883 + # M7: the same exchange issues this robot's broker credential. The username + # is the robot id because that is what the broker's ACL is keyed on. + assert body["broker"]["username"] == "mote-01" + assert body["broker"]["password"] def test_re_enrolling_answers_200_and_the_same_id(server): @@ -373,7 +396,7 @@ def test_a_broker_that_is_down_is_reported_not_swallowed(server, operator, robot def test_the_audit_route_needs_an_operator_token(server, operator, robot): - expect_error(lambda: get(server, "/v1/audit"), 401) + expect_error(lambda: get(server, "/v1/audit", token=""), 401) dispatch(server, robot, "goto kitchen", token=operator) status, body = get(server, "/v1/audit", token=operator) assert status == 200 @@ -452,3 +475,137 @@ def subscribe(self, topic, qos=0): on_connect(client, None, {}, 0) assert [t for t, _ in client.subscribed] == topics + topics assert {qos for _, qos in client.subscribed} == {protocol.QOS} + + +# ---- M7: every read route is authorized ------------------------------------- + + +READ_ROUTES = [ + "/v1/config", + "/v1/robots", + "/v1/robots/mote-01", + "/v1/audit", + "/v1/maps", + "/v1/maps/home/ground/map.json", + "/v1/maps/home/ground/map.png", +] + + +@pytest.mark.parametrize("route", READ_ROUTES) +def test_every_read_route_refuses_an_anonymous_request(server, route): + """M3 put a token on dispatch only, so the roster, the basemaps and the + broker's address were readable by anything that could reach the port. This + is the parametrised statement that they are not.""" + enroll(server, "serial:aaa") + body = expect_error(lambda: get(server, route, token=""), 401) + assert "operator token" in body["error"] + + +@pytest.mark.parametrize("route", READ_ROUTES) +def test_every_read_route_refuses_an_unknown_token(server, route): + enroll(server, "serial:aaa") + expect_error(lambda: get(server, route, token="not-a-real-token"), 401) + + +@pytest.mark.parametrize("route", READ_ROUTES) +def test_every_read_route_refuses_a_revoked_token(server, route): + enroll(server, "serial:aaa") + token = server.registry.new_operator(name="leaver") + server.registry.revoke_operator(token) + expect_error(lambda: get(server, route, token=token), 401) + + +def test_authorization_is_checked_before_the_route_exists(server): + """A 404 for an unauthenticated caller would leak which routes are real.""" + expect_error(lambda: get(server, "/v1/nothing", token=""), 401) + + +def test_healthz_stays_open(server): + """Deliberately: a liveness probe that needs a secret is a liveness probe + nobody wires up.""" + status, body = get(server, "/healthz", token="") + assert (status, body["ok"]) == (200, True) + + +def test_the_ui_stays_open(server): + """The page has to load before it can ask for a token, and it carries no + fleet data until it has one.""" + status, content_type, body = get_bytes(server, "/index.html", token="") + assert status == 200 + assert content_type.startswith("text/html") + assert b"operator token" in body + + +def test_enrollment_needs_no_operator_token(server): + """It carries its own credential. A robot enrolling has no operator behind + it — that is the whole point of an unattended first boot.""" + status, _ = enroll(server, "serial:aaa") + assert status == 201 + + +# ---- M7: what an authorized caller is given --------------------------------- + + +def test_config_carries_this_operators_broker_credential(server): + """The reason /v1/config needs a token: it hands out a credential, and the + one it hands out is the caller's own.""" + _, body = get(server, "/v1/config") + operator = server.registry.operator(server.token) + assert body["broker"]["username"] == operator["broker_user"] + assert body["broker"]["password"] == operator["broker_password"] + + +def test_two_operators_get_different_broker_credentials(server): + other = server.registry.new_operator(name="someone else") + _, mine = get(server, "/v1/config") + _, theirs = get(server, "/v1/config", token=other) + assert mine["broker"]["username"] != theirs["broker"]["username"] + assert mine["broker"]["password"] != theirs["broker"]["password"] + + +def test_the_roster_never_serves_a_password_hash(server): + """Dropped in `_row_to_robot`, so a route added later cannot serve it by + forgetting to.""" + enroll(server, "serial:aaa") + _, roster = get(server, "/v1/robots") + _, one = get(server, "/v1/robots/mote-01") + assert "broker_hash" not in one + assert all("broker_hash" not in robot for robot in roster["robots"]) + assert "broker_hash" not in json.dumps(roster) + + +def test_enrolling_writes_the_broker_credential_files(tmp_path): + """The server is the issuer: a robot is never given a credential the broker + has not been told about.""" + import credentials + + broker_dir = tmp_path / "broker" + httpd = serve( + db=tmp_path / "registry.db", + host="127.0.0.1", + port=0, + broker_host="fleet-box", + publisher=FakeBroker(), + broker_auth_dir=broker_dir, + broker_reload_cmd=None, + ) + thread = threading.Thread(target=httpd.serve_forever, daemon=True) + thread.start() + httpd.url = f"http://127.0.0.1:{httpd.server_address[1]}" + httpd.token = "" + try: + _, body = enroll(httpd, "serial:aaa") + passwd = (broker_dir / "passwd").read_text() + acl = (broker_dir / "acl").read_text() + assert "mote-01:$7$" in passwd + assert credentials.SERVER_USER in passwd + assert "user mote-01" in acl + assert "topic read mote/v1/mote-01/task/command" in acl + # ...and the plaintext it just handed out verifies against the file. + line = next(row for row in passwd.splitlines() if row.startswith("mote-01:")) + assert credentials.verify_password( + body["broker"]["password"], line.split(":", 1)[1] + ) + finally: + httpd.shutdown() + httpd.server_close() diff --git a/mote_fleet/test/test_registry.py b/mote_fleet/test/test_registry.py index 37df84f..d9d93b1 100644 --- a/mote_fleet/test/test_registry.py +++ b/mote_fleet/test/test_registry.py @@ -1,8 +1,10 @@ """Allocation and idempotency — the two properties the id space depends on.""" import sqlite3 +import stat import threading +import credentials import pytest from registry import Registry, RegistryError @@ -212,3 +214,130 @@ def test_an_m1_registry_gains_the_new_tables(tmp_path): registry = Registry(path) assert [r["robot_id"] for r in registry.robots()] == ["mote-01"] assert registry.operator(registry.new_operator(name="michael"))["name"] == "michael" + + +# ---- M7: broker credentials ------------------------------------------------- + + +def test_enrolling_issues_a_broker_credential(registry): + robot, _ = enroll(registry, "serial:aaa") + assert robot["broker_password"] + assert credentials.verify_password( + robot["broker_password"], registry.broker_principals()["users"]["mote-01"] + ) + + +def test_the_plaintext_password_is_never_stored(registry): + robot, _ = enroll(registry, "serial:aaa") + password = robot["broker_password"] + raw = (registry.path).read_bytes() + assert password.encode() not in raw + # ...and no query can produce it either. + assert "broker_password" not in registry.robot("mote-01") + + +def test_re_enrolling_rotates_the_credential(registry): + """Rotation is an idempotent command the robot already runs, rather than a + separate mechanism nobody would remember exists.""" + first, _ = enroll(registry, "serial:aaa") + second, _ = enroll(registry, "serial:aaa") + assert first["broker_password"] != second["broker_password"] + hashed = registry.broker_principals()["users"]["mote-01"] + assert credentials.verify_password(second["broker_password"], hashed) + assert not credentials.verify_password(first["broker_password"], hashed) + + +def test_a_password_hash_is_not_part_of_a_robot_row(registry): + enroll(registry, "serial:aaa") + assert "broker_hash" not in registry.robot("mote-01") + assert all("broker_hash" not in row for row in registry.robots()) + + +def test_an_operator_gets_a_broker_credential_with_their_token(registry): + token = registry.new_operator(name="michael") + operator = registry.operator(token) + assert operator["broker_user"].startswith(credentials.OPERATOR_PREFIX) + assert operator["broker_password"] + assert operator["broker_user"] in registry.broker_principals()["operators"] + + +def test_revoking_an_operator_removes_their_broker_credential(registry): + """One act closes both paths, which is the reason they are minted together.""" + token = registry.new_operator(name="michael") + user = registry.operator(token)["broker_user"] + assert user in registry.broker_principals()["users"] + registry.revoke_operator(token) + assert registry.operator(token) is None + assert user not in registry.broker_principals()["users"] + + +def test_the_servers_password_is_stable_across_calls(registry): + """Regenerating it per restart would lock the server out of its own broker + until something reloaded the credential files.""" + assert registry.server_broker_password() == registry.server_broker_password() + assert credentials.SERVER_USER in registry.broker_principals()["users"] + + +def test_principals_describe_one_fleet(registry): + enroll(registry, "serial:aaa") + enroll(registry, "serial:bbb") + registry.new_operator(name="michael") + principals = registry.broker_principals() + named = set(principals["robots"]) | set(principals["operators"]) + named.add(credentials.SERVER_USER) + # Every ACL entry has a password, and every password has an ACL entry — the + # reason both come out of one query. + assert named == set(principals["users"]) + + +def test_the_registry_file_is_private(registry): + assert stat.S_IMODE(registry.path.stat().st_mode) == 0o600 + + +def test_an_m3_registry_gains_the_new_columns(tmp_path): + """An existing fleet box upgrades by being opened, as it did at M3. + + `CREATE TABLE IF NOT EXISTS` cannot alter a table that is already there, so + without the migration an M3 file would silently keep its old shape and every + enrollment would fail on a missing column. + """ + path = tmp_path / "registry.db" + conn = sqlite3.connect(path) + conn.executescript( + """ + CREATE TABLE robots ( + robot_id TEXT PRIMARY KEY, name TEXT NOT NULL DEFAULT '', + site TEXT NOT NULL DEFAULT '', fingerprint TEXT NOT NULL UNIQUE, + facts TEXT NOT NULL DEFAULT '{}', enrolled_at TEXT NOT NULL, + last_enrolled_at TEXT NOT NULL); + CREATE TABLE tokens ( + token TEXT PRIMARY KEY, single_use INTEGER NOT NULL DEFAULT 1, + note TEXT NOT NULL DEFAULT '', created_at TEXT NOT NULL, + used_at TEXT, used_by TEXT); + CREATE TABLE operators ( + token TEXT PRIMARY KEY, name TEXT NOT NULL, + note TEXT NOT NULL DEFAULT '', created_at TEXT NOT NULL, + last_used_at TEXT, revoked_at TEXT); + INSERT INTO robots VALUES + ('mote-01','Scout','home','serial:aaa','{}','2026-07-01T00:00:00Z', + '2026-07-01T00:00:00Z'); + """ + ) + conn.commit() + conn.close() + + registry = Registry(path) + # The M3 rows survive, and are simply without a credential until they enrol. + assert registry.robot("mote-01")["name"] == "Scout" + assert registry.broker_principals()["robots"] == [] + robot, created = enroll(registry, "serial:aaa") + assert (robot["robot_id"], created) == ("mote-01", False) + assert registry.broker_principals()["robots"] == ["mote-01"] + + +def test_opening_twice_is_harmless(tmp_path): + path = tmp_path / "registry.db" + Registry(path) + registry = Registry(path) + enroll(registry, "serial:aaa") + assert registry.robot("mote-01") diff --git a/mote_fleet/test/ui_test.mjs b/mote_fleet/test/ui_test.mjs index b672790..260713b 100644 --- a/mote_fleet/test/ui_test.mjs +++ b/mote_fleet/test/ui_test.mjs @@ -41,6 +41,38 @@ test('CONNECT names protocol 3.1.1 and a clean session', () => { assert.equal(packet[9], 0x02); // clean session, no will, no credentials }); +// -- M7: the page authenticates to the broker --------------------------- + +test('CONNECT carries the operator credential when there is one', () => { + const packet = encodeConnect('mote-ui-test', 30, { + username: 'op_michael_1a2b', + password: 's3cret', + }); + assert.equal(packet[9], 0x02 | 0x80 | 0x40, 'clean session + username + password'); + const text = new TextDecoder().decode(packet); + assert.ok(text.includes('op_michael_1a2b')); + assert.ok(text.includes('s3cret')); + // Order matters: client id, then username, then password (MQTT 3.1.1 3.1.3). + assert.ok(text.indexOf('mote-ui-test') < text.indexOf('op_michael_1a2b')); + assert.ok(text.indexOf('op_michael_1a2b') < text.indexOf('s3cret')); +}); + +test('CONNECT sets no credential flags when there is no credential', () => { + for (const credential of [null, undefined, {}, { username: '' }]) { + assert.equal(encodeConnect('mote-ui-test', 30, credential)[9], 0x02); + } +}); + +test('the client still has no way to publish', async () => { + // The subscribe-only split is enforced twice over since M7 — by the broker's + // ACL, and by this file simply not being able to build the packet. If an + // `encodePublish` ever appears here, that second half is gone and this test + // is the thing that should stop it. + const module = await import('../server/ui/mqtt.mjs'); + const exported = Object.keys(module); + assert.ok(!exported.some((name) => /^encodePublish/i.test(name)), exported.join()); +}); + test('SUBSCRIBE carries the mandatory 0x02 flags', () => { const packet = encodeSubscribe(1, ['mote/v1/+/health']); assert.equal(packet[0], (8 << 4) | 0x02);