diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md new file mode 100644 index 00000000..055926d2 --- /dev/null +++ b/THREAT_MODEL.md @@ -0,0 +1,515 @@ +# tines — threat model and security assessment + +- **Assessed:** `master` at `3b1a644` (tines-core / tines-store / tines-bt / + tines-ros2 / tines 0.3.0, fleet-deployment milestone mostly landed) +- **Date:** 2026-10-19 +- **Status:** first version. This is a living document, not a spec: update + it when the code or the trust assumptions change. +- **Related:** the 1.0 spec (`docs/superpowers/specs/2026-10-18-tines-1-0-design.md`), + especially §2a (signature slot) and §2b (age and the 1.1 threat model). + +Every finding below was checked against the source; `file:line` references +are to `3b1a644`. + +## 1. Summary + +tines v1 has **no trust layer by design**. The core spec lists security as a +non-goal for v1, and the fleet spec repeats "v1 has no trust layer". So most +of what an attacker can do today is expected: + +- Anyone in radio range can read everything. +- Anyone in radio range can inject observations under any identity. +- Anyone in radio range can take custody of addressed messages. + +That's acceptable while the fleet operates in a trusted RF environment, and +not otherwise. Signing arrives in 1.1 (spec §2a), and link authentication is +the likely 1.1 mechanism for everything that changes per hop (spec §2b). + +Separate from the by-design gaps, the assessment found defects that matter +**now**, even with no attacker present. Several are correctness bugs that +the fleet milestone needs fixed regardless of security: + +| # | Severity | Finding | +|---|---|---| +| F1 | **High** | A receiver whose store is full still acks, so the sender releases custody of a message nobody kept. | +| F2 | **High** | The standalone `tines` service never polls its `Node`, so it never exchanges anything over BLE. | +| F3 | **High** | `tines-bt` doesn't bind a connection to the advertised NodeId, has no inbound listener, and loses frame boundaries on a stream socket. | +| F4 | **High** | No host calls `LogStore::flush`, and both seed `Node` from the config `first_seq`. Released flags and `next_seq` are never persisted, so seqs repeat after every restart. | +| F5 | Medium | `LogStore` recovery truncates every record after a single corrupt one, and the writer can produce records the reader rejects. | +| F6 | Medium | A peer's NodeId is taken from each frame's `from`, unchecked and changeable per frame, so a peer can claim any identity and be handed that identity's addressed mail. | +| F7–F17 | Medium/Low | Resource exhaustion, amplification, local-IPC and ROS2 exposure, and smaller defects (§6). | + +Recommended order: fix F1–F5 in the fleet milestone (they're bugs), harden +the hosts (F10–F12) before any robot leaves the lab, and settle the 1.1 +threat questions (§8) before designing signing. + +## 2. System and trust boundaries + +```mermaid +flowchart LR + subgraph Robot["Robot (one tines instance per device)"] + direction TB + App["Robot software
(ROS2 nodes or IPC clients)"] + Host["Host: tines-ros2 or tines service"] + Core["tines-core
Node + Exchange"] + Store[("tines-store
append-only log")] + BT["tines-bt
BlueZ L2CAP"] + App -- "B2: DDS domain / Unix socket" --> Host + Host --> Core + Core --> Store + Core --> BT + end + Air(("B1: radio
BLE advertising + L2CAP")) + BT <--> Air + Air <--> Other["Other robots
(and anyone in range)"] + Disk["B3: filesystem"] -.- Store +``` + +| Boundary | What crosses it | Who can be on the other side today | +|---|---|---| +| **B1 radio** | Advertisements (NodeId in clear), L2CAP frames: Summary, Obs, Frag, Ack | Any BLE device in range. There's no pairing, encryption or allowlist. | +| **B2 local IPC / ROS2** | Observations in, beliefs/payloads and events out | Any local user who can reach the socket (permissions come from umask); any participant on the ROS2 DDS domain (no SROS2). | +| **B3 storage** | The observation log, released set, `next_seq` | Anyone with write access to the log file (mode from umask). | +| Core ↔ transport | `Addr`, raw frames | Trusted code. Core relies on the transport to bind `Addr` to a NodeId (AGENTS.md: "core never maps Addr to NodeId"). | + +## 3. Assets + +1. **World state integrity:** the facts robots plan and navigate on. Forged + or stale facts can make a robot act wrongly. +2. **Directive integrity and delivery:** addressed messages (tasks, + telemetry home), including that they arrive, arrive once, and don't + outlive their lifetime. +3. **Custody:** "every unexpired addressed message has at least one holder". +4. **Availability of exchange:** contacts are rare and short; wasting one is + costly. +5. **Robot resources:** RAM (MCU-class targets), disk, battery, airtime. +6. **Identity:** which robot said what (`observer`), which robot is on the + other end of a link. +7. **Confidentiality:** world state and directives (not a v1 goal) and + robot location/movement privacy (stable advertised NodeId). +8. **Durability:** the log, the released set and `next_seq` across reboots. + +## 4. Adversaries + +| Adversary | Capabilities | In scope for | +|---|---|---| +| **A1 radio outsider** | No fleet key. In BLE range: listens, advertises any NodeId, connects, injects arbitrary frames, replays, floods. Can't easily alter frames in flight on an established link, but can impersonate a peer. | v1 assessment; 1.1 design | +| **A2 compromised robot (insider)** | Valid fleet member (and, from 1.1, a valid key). Can already drop, delay or withhold anything it carries; custody accepts carrier failure. | 1.1 design (what signatures do and don't buy) | +| **A3 local user / process** | Unprivileged account or process on the robot's computer. | Host hardening (now) | +| **A4 ROS2 domain participant** | Any process on the same DDS domain, local or on the network the ROS graph is exposed to. | Host hardening (now) | +| **A5 physical / storage access** | Can read or edit the log, swap the SD card, or power-cycle the robot at will. | Partially (integrity and crash safety); full physical attack is out of scope | +| **A0 no adversary** | Faults only: power loss, full disk, full store, radio loss, clock jumps. | Always. Several findings below are A0 bugs. | + +Out of scope: side channels, RF jamming (physical-layer DoS can't be fixed +in software), compromise of BlueZ/the kernel, and supply chain. + +## 5. Security properties: claimed vs actual + +| Property | Claimed (specs/AGENTS.md) | Actual at `3b1a644` | +|---|---|---| +| No panic on untrusted input (core) | Hard constraint | **Holds** as far as reviewed: saturating age arithmetic, guarded shifts and indexing, `expect`s unreachable from wire input (exchange.rs:437, 1092; frag.rs:147). The sim's `garbage_frames_never_panic` covers random bytes only (§9). | +| Malformed frames dropped | Hard constraint | **Holds** in core (wire.rs:64-69). The 1.0 spec adds reporting. | +| Bounded memory in core | Fixed-capacity buffers | **Holds** for the exchange (§6 table). **Doesn't hold for the system**: MemStore and LogStore are unbounded without `trim`, and no host calls `trim` (F10). | +| Custody: ≥ 1 holder (no carrier failure) | Fleet spec, sim property | **Breaks without an attacker** when a store is full (F1). Breaks trivially under A1 (F7). | +| Frames bound to the advertised NodeId | Fleet spec ("frames whose `from` differs … are dropped at the transport") | **Doesn't hold** (F3, F6). | +| Released messages not re-forwarded after reboot | Fleet spec | **Doesn't hold**: release flags are never flushed (F4). Safe direction (duplicate custodian), but wasteful. | +| `next_seq` persisted across reboots | Core contract | **Doesn't hold** in either host (F4). | +| Age continues across reboot | Fleet spec | Partly. Hosts open the store at tick 0, and the service's tick is wall-clock *seconds* (F17). | + +## 6. Findings + +Severity reflects impact on the fleet as it's meant to be deployed (trusted +RF, single-user robot computers) unless stated. "By design (v1)" marks +attacker capabilities that follow from having no trust layer; they're +listed so the 1.1 work can check them off. + +### F1 — Full store acks what it didn't keep (High, A0/A1) + +`Exchange::accept` enqueues an ack for every receive except a released +duplicate, including when `insert` fails (exchange.rs ~805-813, comment: +"even an un-stored id"). `Err(Full)` counts as a duplicate and +`released` is false, so the ack goes out. The sender then releases custody +of a message the receiver didn't keep. + +- **Effect:** an addressed message is lost with no carrier failure. This + violates the custody invariant the fleet spec and sim property rely on. + An attacker doesn't need to do anything clever: filling a robot's store + (F10) turns it into a custody sink. +- **Why it's not caught:** `MemStore` never returns `Full`, and the sim uses + `MemStore`. +- **Fix:** never ack an addressed observation unless `insert` returned + `Ok(_)` (and, for duplicates, the copy isn't released). Add a sim/unit + test with a store that returns `Full`. This was the fleet spec's own rule + ("B never acks a … store-full receive"). + +### F2 — The standalone service never runs the exchange (High, A0) + +`tines/src` never calls `Node::poll` or `Node::trim` (grep; the plan calls +for a poll loop, `docs/superpowers/plans/2026-10-18-tines-fleet-deployment.md:209`). +`serve()` only answers IPC requests. + +- **Effect:** a non-ROS2 robot never sends or receives anything. Inbound BLE + data accumulates in an unbounded channel (F10). +- **Security relevance:** availability, plus unbounded memory growth while + connected. Mostly a functional bug. +- **Fix:** a poll loop (with `trim` on a schedule and `flush` after + releases), off the IPC thread or interleaved with it. + +### F3 — tines-bt: no identity binding, no listener, no framing (High, A0/A1) + +- `read_loop` stamps every inbound chunk with `from: peer`, where `peer` + comes from config (tines-bt/src/transport.rs:340-363). The drop check + `if from != self.peer` (transport.rs:113-115) therefore never fires, + whichever device connected. +- The scanner connects to the first device advertising the tines service + UUID and never decodes or compares the advertised NodeId + (transport.rs:290-308). There's no allowlist. +- There's no `L2capListener`, so nothing accepts inbound connections, and + `is_central` is never consulted. Every instance dials out. Two robots + can only connect if something else accepts. +- The L2CAP socket is in **stream** mode and each `read()` result is + treated as one frame (transport.rs:345-356). Frame boundaries aren't + preserved, so frames can be split or merged. Without an attacker, that + produces sporadic malformed drops. With the 1.0 drop reporting, it would + also produce false "malformed" alarms. +- `Contact`/`Lost` events are never emitted, so core's per-contact state + (bounce records, ack validation "this contact", peer removal) never + resets (see F8). +- No BLE security level (pairing, encryption, LE Secure Connections) is + requested. + +**Fix (fleet milestone):** +- Use SeqPacket L2CAP, or add explicit length-prefix framing. +- Add a listener plus the smaller-NodeId-initiates rule. +- Decode the advertised NodeId and bind the connection to it. +- Drop frames whose `from` differs from the bound NodeId. +- Emit `Contact`/`Lost`. + +**Later (1.1):** authenticate the link (spec §2b). Advertised NodeIds are +unauthenticated, so the binding only becomes meaningful once there's a +handshake. + +### F4 — Durability promises aren't kept by the hosts (High, A0) + +- No host calls `LogStore::flush` (grep of `tines-ros2/src`, `tines/src`). + `Meta { next_seq, released }` is only written by `flush`. So the + released set never reaches disk, and after a restart every released + message is forwarded again. That's the safe direction (a duplicate + custodian) but contradicts the fleet spec. +- Both hosts build `Node` with the config `first_seq` + (`tines/src/main.rs:44-48`, `tines-ros2/src/node.rs:147-149`) instead of + a persisted value. Core requires `next_seq` to be persisted + (`boot_count << 32 | counter`). After a restart, seqs repeat. A new + observation with the same topic, payload and recipient as an earlier one + then has the same id and **dedupes silently**: the robot reports + something and it vanishes. +- `LogStore::next_seq` counts *all* inserts, including received ones + (log.rs:352), so it isn't the node's seq either. + +**Fix:** +- Persist the node's `next_seq` (boot counter plus counter), and seed + `Node` from it. +- Flush on release and on a timer. +- Add a host-level restart test. + +### F5 — Log recovery can destroy good data (Medium, A0/A5) + +- `LogStore::new` stops replay at the first record that fails to parse and + truncates the file there (log.rs:180-206). A single corrupt record in the + *middle* (a bad sector, a bit flip, a hand edit) deletes every record + after it. The code comment says "only the partial tail of a crashed + append is lost, never a complete record", which is true only for tail + damage. +- `write_record` doesn't enforce `MAX_RECORD_BYTES` (log.rs:274-281), but + the reader rejects longer records. A `Meta` record grows by 16 bytes per + released id, so above roughly 500 released ids the store writes a record + that makes it truncate itself on the next open. +- There's no checksum, so silent corruption that still decodes is accepted. + The stored `id` is trusted without recomputing `obs.id()` (log.rs:154). +- There's no file locking. Two processes on one log interleave appends, and + one's recovery `set_len` can cut the other's records. +- The log is read whole into memory (log.rs:127). The file is created with + umask permissions, and the truncation isn't fsynced. + +**Fix:** +- Add a per-record CRC (or a keyed MAC later). +- On a bad record in the middle, skip to the next valid one (a sync marker) + instead of truncating; truncate only a torn *tail*. +- Enforce the size cap on write, and keep `Meta` small (store release + flags per record, or compact). +- Take an exclusive `flock` on open. +- Create the file with mode `0600`. +- Recompute ids on load. + +### F6 — Peer identity is whatever the last frame said (Medium; High combined with F3) + +`on_frame` sets `peer.id = Some(frame.sender())` on every frame, unchecked +(exchange.rs:723). The `from` field inside Obs/Frag is ignored, and +`observer` is never compared to anything. + +- **Effect (A1):** send one frame with `from = X`, and `forward_addressed` + hands you every message addressed to X (exchange.rs:584-593), as + "delivery". Combined with F7 you can then ack them, and the honest + carrier releases them. +- **Design note:** core deliberately leaves the `Addr` ↔ NodeId binding to + the transport, so this is F3's consequence. Still, core could cheaply + refuse a NodeId *change* for an existing peer within a contact, and + should, once the transport binds identities. + +### F7 — Custody can be taken by anyone the carrier sends to (Medium; by design v1) + +An ack is accepted if it comes from the address the message was sent to +(bounce `Sent` record, exchange.rs:756-784). Under the custody chain, a +carrier hands an addressed message to *any* peer with a known id +(exchange.rs:594-595), then releases on that peer's ack. A1 can collect +custody of every addressed message in range and drop it. + +- That matches "carrier failure" (accepted for v1) but needs no key and no + compromise. 1.1's link authentication limits it to fleet members (A2), + where it really is carrier failure. +- Also: repeated Acks produce duplicate confidence events (`bcast_sent` + isn't consumed). + +### F8 — Peer-table lockout (Medium, A1) + +`MAX_PEERS = 8`. Peers are added on `Contact` or on the first decodable +frame, and removed only on `Lost` (exchange.rs:674-708). A full table drops +all frames from new addresses. There's no idle eviction. With BLE never +emitting `Lost` (F3), entries effectively never leave on real hardware. +Eight spoofed addresses lock a robot out of exchange until restart. + +**Fix:** idle-timeout eviction in core (a local constant), and real +`Lost` events from tines-bt. + +### F9 — Summary amplification (Medium, A1) + +A Summary of about 20 bytes with an all-zero filter makes the receiver +rebuild that peer's send queue with up to 256 of its youngest broadcasts +(exchange.rs:1065-1125). Each one can be up to 64 fragments, so the worst +case is about 16,000 frames of airtime per crafted summary. That's +throttled to one frame per `poll`, but sustained. Re-sending the summary +re-queues what was already sent. + +- Each rebuild also scans every broadcast and clones payloads, which costs + CPU and allocation per summary, with no rate limit. +- **Fix:** rate-limit summary processing per peer (honour at most one per + `SUMMARY_INTERVAL_MS`), and remember what was already sent this contact + (`bcast_sent` already exists) so a repeated summary doesn't re-queue it. + +### F10 — Unbounded growth (Medium, A0/A1/A4) + +- `MemStore` and `LogStore` are bounded only by `Node::trim`. Neither host + calls it. The log also never compacts (log.rs: append-only, and every + flush appends the full released list). +- Unbounded channels: tines-bt's inbound and outbound `mpsc` + (transport.rs:207-208), and tines-ros2's inbound `mpsc` (node.rs:152). +- The IPC server allocates up to 1 MiB per request frame. +- **Effect:** A1 can fill memory and disk by gossiping junk. A4 can do it by + publishing on a bridged topic. Either drives F1 (store full) once a + bounded store exists. +- **Fix:** + - Hosts trim on a schedule with a documented default policy. + - Use bounded channels (drop or backpressure). + - Add log compaction. + - Add per-peer and per-observer insert quotas (for example, a per-peer + token bucket on new inserts). + +### F11 — Local IPC is open to any local user (Medium, A3) + +- The socket gets umask permissions, and there's no `SO_PEERCRED` check + (tines/src/ipc.rs:194-200; grep finds no `set_permissions` or peer-cred + code). +- Any local process that can reach it can: + - publish observations **as this robot** (`Observe`), which are then + gossiped fleet-wide under its identity; + - read any belief; + - drain the shared event queue, destructively. +- The configured socket path is unlinked unconditionally before binding, + whatever file is there (ipc.rs:197). +- **Availability:** clients are served one at a time with no timeouts, so + an idle client blocks everyone. An `accept` error ends the daemon. An + oversized frame (> 1 MiB) gets an error reply without consuming its body, + which desynchronizes the stream (contrary to the module docs). +- **Fix:** + - Create the socket in a `0700` runtime directory with mode `0600` (or a + group-restricted mode). + - Check `SO_PEERCRED` against an allowed uid/gid. + - Only unlink a path that is a socket. + - Set per-connection read timeouts, or serve clients concurrently. + - Close the connection on an oversized frame. + - Survive `accept` errors. + +### F12 — ROS2 bridge trusts the whole DDS domain (Medium, A4) + +- **Read:** any participant on the domain can read the latest payload of + every bridged topic and every receipt. +- **Write:** any participant can inject observations on a managed topic. + They're stored and gossiped as this robot's own (`observe(..., None, now)`, + tines-ros2/src/node.rs:197), with no rate limit. +- SROS2/DDS security isn't mentioned or configured anywhere. +- **Likely self-echo loop:** the node subscribes to the topics it + publishes, with no filter for its own messages (node.rs:207-238). If DDS + delivers its own publications back, which the default settings allow + (not verified on hardware), each republish is re-observed with a new seq. + That's a feedback loop that grows the log and floods the fleet. +- **Fix:** + - Ignore local publications (`ignore_local_publications` or a + publisher-GID filter). + - Document running with SROS2 enclaves and a dedicated domain id. + - Rate-limit inbound. + - Bound the channel. + +### F13 — Smaller core defects (Low) + +- **Event queue drops newest, not oldest** (`let _ = self.events.push`, + exchange.rs:777), contrary to its docs and the fleet spec. +- **Pending acks for a lost peer aren't purged** on `remove_peer`. On + send errors they're re-queued, and since acks go ahead of data, retries + to a dead peer can starve data frames. +- **Reassembler displacement:** one fragment with a different id or count + resets a peer's in-progress stream (frag.rs:121-133). Same-address only, + so it's limited by F3/F6 binding. +- **Trailing bytes:** `postcard::from_bytes` accepts trailing bytes after a + valid frame. That's harmless now, but the 1.0 vectors should pin + rejection so the wire format is canonical. + +### F14 — Belief policies trust attacker-controlled fields (Medium; by design v1) + +- `LatestWins` picks the smallest `age_ms` (belief.rs:36-39). An attacker + sets age 0 to win any topic (see spec §2b). +- `SelfFirst` trusts `obs.observer == me` (belief.rs:75-77). A forged + observation claiming to be from *this* robot beats everything. Signing in + 1.1 fixes the observer claim. Until then, a host could refuse inbound + observations whose `observer` is its own NodeId (a robot never needs to + receive its own observations from a peer if it still holds them). +- First-copy-wins dedupe fixes `age_ms`, `observed_at` and `lifetime_ms` + forever at a receiver (mem_store.rs:72-75). A huge `lifetime_ms` makes an + addressed message effectively immortal. + +### F15 — No confidentiality, trackable robots (Low for v1; by design) + +The BLE link is unencrypted, so world state and directives are readable by +anyone in range. Each robot advertises a stable NodeId in cleartext and is +discoverable (transport.rs:276-282), so it can be tracked across BLE address +rotation. That's the design (the NodeId *is* the rendezvous). If tracking +matters, 1.1+ needs rotating, keyed rendezvous identifiers. + +### F16 — The 16-byte id is unkeyed and truncated (Low, A1) + +Ids are the first 128 bits of Blake2s-256 (64-bit collision resistance). +Producing a collision with a *specific* existing id needs a second +preimage, which is infeasible, so an attacker can't squat a particular +id. Generic 64-bit collisions are within reach of a determined attacker, +but only between two observations they produced themselves, which gains +nothing. This is acceptable for dedupe. It's the reason spec §2a signs the +full preimage, not the id. + +### F17 — Service clock is wall-clock seconds (Medium, A0) + +`tines/src/ipc.rs` `now_ms()` returns `as_secs()` from `SystemTime` +(ipc.rs:276-281). Core's `Tick` is monotonic milliseconds. + +- Ages advance 1000× too slowly, so addressed lifetimes last 1000× too + long. +- An NTP step backwards freezes ages, because they saturate. +- The store is opened at tick 0 while requests use epoch values, so the + first read can add the whole epoch to every age. + +**Fix:** use a monotonic millisecond clock (`Instant` since process start) +everywhere in the host, and open the store with the same clock. + +## 7. Threats by STRIDE, mapped to findings + +| | Threat | Status today | Findings | Planned mitigation | +|---|---|---|---|---| +| **S**poofing | Impersonate a robot on the link | Trivial (A1) | F3, F6 | Transport binding now; link auth (1.1) | +| | Forge an observation's `observer` | Trivial (A1, A3, A4) | F11, F12, F14 | Host hardening now; signatures (1.1) | +| **T**ampering | Alter content in transit | No integrity (A1) | — | Signatures cover content (1.1) | +| | Alter age / lifetime | Unprotected | F14, §2b | 1.1 threat-model decision | +| | Alter the log on disk | No integrity (A5) | F5 | CRC now; MAC optional | +| **R**epudiation | Deny having said something | Unattributable | — | Signatures (1.1) | +| **I**nformation disclosure | Read world state / directives | Cleartext (A1, A4) | F12, F15 | Out of scope v1; link encryption candidate | +| | Track robots | Stable NodeId adverts | F15 | Rotating identifiers (post-1.1) | +| **D**enial of service | Lose custody traffic | Easy (A0!, A1) | F1, F7 | Fix F1 now; link auth (1.1) | +| | Lock out peers / waste airtime | Easy (A1) | F8, F9 | Eviction and rate limits now | +| | Exhaust memory / disk | Easy (A1, A4) | F10 | Trim, bounded channels, quotas now | +| | Block local clients | Easy (A3) | F11 | Timeouts, concurrency now | +| **E**levation of privilege | Act as the robot from a local account | Any local user (A3) | F11 | Socket permissions + peer creds now | +| | Act as the robot from the ROS graph | Any domain participant (A4) | F12 | SROS2, domain isolation | + +## 8. The age question (for 1.1) + +Spec §2b asks whether `age_ms` must be authenticated. This assessment +supports its framing, and adds evidence: + +- Today the question is moot. Everything is forgeable, and fixing F14 + needs signatures first. +- After 1.1 signs content, age is the main remaining *freshness* lever: + stale facts winning `LatestWins`, expired directives coming back to life, + and fresh facts suppressed. For A1 it's fully exploitable without link + authentication. For A2 it adds freshness attacks to what a bad carrier + can already do by dropping. +- **Recommendation for 1.1:** link authentication (handshake plus per-frame + tag), which removes A1 from age, acks, summaries and identity in one + mechanism. Accept A2 age forgery as equivalent to carrier misbehaviour, + and document it. Cross-check against signed `observed_at` where robots + have clocks. + +## 9. Test gaps + +The sim covers honest-node custody properties and random garbage frames +(`tests/props.rs:281`, `tests/relay.rs`). It doesn't cover: + +- well-formed malicious frames: forged acks, spoofed `from`/`observer`, + NodeId changes mid-contact; +- a store returning `Full` (F1); +- summary floods and peer-table exhaustion (F8, F9); +- reassembly abuse; +- `LogStore` corruption in the middle, an oversized `Meta`, concurrent + openers, a full disk, and restart and seq continuity (F4, F5); +- anything in the hosts: IPC abuse, ROS2 injection and self-echo + (F11, F12). + +The 1.0 fuzz harness (spec §7) should take an *adversarial peer* mode, not +just random bytes: structured frames with chosen ids and identities. + +## 10. Where each finding is being fixed + +| Findings | Where | Spec | +|---|---|---| +| F1–F5, F17 | Fleet milestone (M3), now | `docs/superpowers/specs/2026-10-19-tines-fleet-fixes.md` | +| F10–F12 | Host hardening, before a robot leaves the lab | `docs/superpowers/specs/2026-10-19-tines-host-hardening.md` | +| F6, F8, F9, F13 | tines-core 1.0 | `docs/superpowers/specs/2026-10-18-tines-1-0-design.md` §10 | +| F7, F14, §8 | 1.1 (threat model, link authentication, signing) | 1.0 spec §2a, §2b | +| F15, F16 | Accepted for now | — | + +Mark each finding fixed here, with its commit, when it lands. + +## 11. Recommendations, in order + +1. **Fleet milestone bugs:** + - F1 (ack only what was stored). + - F2 (poll loop). + - F3 (framing, listener, NodeId binding, Contact/Lost). + - F4 (persist seq, flush). + - F5 (CRC, recovery that skips rather than truncates, write cap, + locking). + - F17 (monotonic ms clock). + - Each with a regression test. +2. **Host hardening before any robot leaves the lab:** + - F10 (trim, bounded channels, compaction). + - F11 (socket permissions, peer creds, timeouts). + - F12 (ignore own publications, SROS2 guidance, rate limits). +3. **Core hardening (can ride with 1.0):** + - F8 (idle eviction). + - F9 (summary rate limit, no re-queue). + - F13 (event drop-oldest, purge acks on peer loss, reject trailing + bytes). + - F6 (refuse NodeId change within a contact). +4. **1.1:** + - Write the full threat model from §4/§8. + - Choose link authentication. + - Add signing per spec §2a (F14, F7). + - Add "replace if better signed" dedupe. +5. **Later:** confidentiality and rotating rendezvous identifiers (F15), + only if the deployment needs them. diff --git a/docs/superpowers/specs/2026-10-18-tines-1-0-design.md b/docs/superpowers/specs/2026-10-18-tines-1-0-design.md index b61131a1..e61018af 100644 --- a/docs/superpowers/specs/2026-10-18-tines-1-0-design.md +++ b/docs/superpowers/specs/2026-10-18-tines-1-0-design.md @@ -1,9 +1,10 @@ # tines-core 1.0 — design Date: 2026-10-18 -Status: draft v4. Kyle decided D1–D5 and the follow-ups Q1–Q3 -(2026-10-18). §2b records the 1.1 direction on age and authenticity. Not yet -reviewed. +Status: draft v6. Kyle decided D1–D5 and Q1–Q3 (2026-10-18) and the +review decisions R1–R3 and the `observed_at` definition (2026-10-19); +review fixes applied. v6 adds §10 (core hardening from `THREAT_MODEL.md`) +and the adversarial fuzz mode; these additions aren't reviewed yet. Builds on: `2026-10-16-tines-design.md`, `2026-10-18-tines-addressed-delivery.md`, `2026-10-18-tines-fleet-deployment-design.md` (assumed fully implemented) @@ -114,26 +115,52 @@ installed base to migrate (D5). milestone). Any value ≥ 4 is an unknown variant and decodes as `Malformed`. Conversely, a 0.x frame starts with 0–3, which a 1.0 node rejects as a version mismatch. No robot runs 0.x, but test rigs and - simulators might, so this costs nothing to get right. Pick a value well - clear of small tags (e.g. `0xA1`). + simulators might, so this costs nothing to get right. Avoid 0–3 (valid + 0.x tags). Prefer a value in 4–0x7F: postcard reads it as a single-byte + unknown tag. A value ≥ 0x80 also ends up `Malformed`, but only because + postcard treats it as a varint continuation and reads a second byte, + which is a needless subtlety. - **Every drop is reported (D3).** A frame dropped at the envelope or in - decoding raises a `NodeEvent::Dropped { reason }`, where `reason` is a + decoding is reported against the transport `Addr` it arrived from, with a `#[non_exhaustive]` `DropReason`: - `VersionMismatch { version: u8 }`: an un-updated or foreign peer. + - `IdentityChanged`: a frame's `from` differs from the NodeId already + pinned for this contact (§10). - `Malformed`: the version was right but the body didn't decode, or decoded to something core must reject (e.g. a `Signature` whose length doesn't fit its algorithm, §2a). An operator then sees "met an un-updated robot" or "a peer is sending - garbage" instead of a silent non-exchange. With the version byte, - "Malformed" from a v1 peer means a bug or an attack, not drift. The - report carries no `NodeId`, since the sender can't be trusted before - decode. Reports are rate-limited per transport address so one peer can't - flood the event queue (builder's call how; the limit is a local - constant). + garbage" instead of a silent non-exchange. `Malformed` from a v1 peer + means one of three things: + - a bug, + - an attack, + - a *newer* 1.x peer sending a `Frame` variant this node predates. That + case is expected and safe (see "What a version bump means" below), so + tooling shouldn't treat every `Malformed` as an alarm. +- **Reports carry the `Addr`, not a `NodeId` (R1).** The sender's NodeId is + inside the frame and can't be trusted before decode. The transport `Addr` + is local truth that doesn't depend on the frame, so it's what makes a + drop attributable. The report is only as good as the transport's + binding: on BLE, `Addr` must be the NodeId bound to the connection (see + `THREAT_MODEL.md` F3). +- **Drops are counted, not queued (R2).** Core keeps a small fixed table of + counters keyed by `(Addr, DropReason)`, so that `VersionMismatch` with + different versions counts separately. Each new drop increments its + counter instead of pushing an event. `drain_events` reports each nonzero + counter once as `NodeEvent::Dropped { addr, reason, count }` and resets + it. + - A flood of bad frames therefore costs one table slot, not the event + queue. `Delivered` receipts can never be evicted by drops. + - The counting is itself the rate limit; no separate limiter is needed. + - When the table is full, a new `(Addr, reason)` pair goes into one + overflow counter, reported with no address. Losing attribution under a + flood is acceptable; losing the count isn't. + - The table size is a local constant (§5). - Expired addressed observations are *not* drops. They're well-formed and are discarded by policy; reporting them would be noise. -- `SUMMARY_OVERHEAD` and `FRAG_OVERHEAD` absorb the extra byte. Every frame +- `SUMMARY_OVERHEAD` (private, `exchange.rs`) and `FRAG_OVERHEAD` absorb + the extra byte. Every frame still fits `mtu()` (Review Focus #6). - **What a version bump means:** any change to the encoding of an existing variant, the meaning of an existing field, or a wire constant (§5) is a new @@ -174,8 +201,24 @@ pub struct Dest { pub to: NodeId, pub lifetime_ms: u64 } plus `with_observed_at`. Applications normally never build one — `Node::observe`/`observe_to` do. 1.0 has no public way to set `sig`; 1.1 adds it. -- The id hash reads `dest.map(|d| d.to)` and is byte-for-byte the same as - 0.x; the pinned vector proves it. `sig` is excluded from the id. +- **The observation's wire encoding changes in v1; only its identity is + preserved.** `dest` replaces `to` + `lifetime_ms`, and `sig` is appended, + so postcard encodes an `Observation` differently from 0.x. That's fine + under the version byte (§1). The id hash reads `dest.map(|d| d.to)` and + is byte-for-byte the same as 0.x; the pinned vector proves it. `sig` is + excluded from the id. +- **`observed_at` gets a defined meaning (Kyle confirmed, 2026-10-19).** + Today the field has no docs and no pinned units, but §2a signs it and + §2b cross-checks against it. Proposed definition: + - **Units:** the observer's wall-clock time when it made the + observation, in milliseconds since the Unix epoch (UTC). + - **`None`:** the observer had no clock it trusts. A robot without an + RTC or time sync must leave it `None` rather than guess. + - **Never changed:** set once by the observer; relays never change it. + - **Not a local tick:** it's unrelated to `Tick`, which stays local + monotonic milliseconds and never crosses the wire. + + The rustdoc on the field says all of this. - `reject()` keeps only the expiry check; the malformed-pair branch and its tests go away (the pairing is now enforced by the type, including on decode). @@ -319,17 +362,18 @@ Public in 1.0 (everything else becomes `pub(crate)`): | Item | Why it's public | |---|---| | `Node`, `ObserveError` | the facade applications use | -| `NodeEvent` (non-exhaustive enum; replaces `ConfidenceEvent`), `DropReason` | `Delivered { peer, id }`, `Dropped { reason }`; room for more | +| `NodeEvent` (non-exhaustive enum, generic over the transport `Addr`; replaces `ConfidenceEvent`), `DropReason` | `Delivered { peer, id }`, `Dropped { addr: Option, reason, count }` (`addr` is `None` only for the overflow counter, §1); room for more | | `Observation`, `Dest`, `Signature`, `SigAlgo`, `NodeId`, `ObservationId`, `Tick`, `Topic`, `Payload` | the data model | | `Store`, `StoreError`, `TrimPolicy`, `MemStore` (`alloc`) | backends and their reference impl | | `Transport`, `Event`, `SendError`, `FrameBuf` | transport adapters | | `BeliefPolicy`, `LatestWins`, `SelfFirst` | the memory layer | | `consts` (wire constants + local caps, §5), `WIRE_VERSION` | sizing | +| `frame_sender(bytes) -> Option` | lets a transport check a frame's claimed sender against the identity it bound to the connection without core's private `Frame` layout (fleet fixes spec §3; `THREAT_MODEL.md` F3) | Made private: `Exchange`, `Peer`, `InFlight`, `Bloom` (including the public `salt` field), `Reassembler`, `fragment`, `Frame`, `WireError`, -`FRAG_OVERHEAD`. Nothing outside `tines-core` uses them except one -`tines-sim` test that touches `Exchange` directly; it moves to `Node`. +`FRAG_OVERHEAD`. Nothing outside `tines-core` uses them; the only mention +is a doc-comment link in `tines-sim/src/world.rs`, which is updated. - **All public enums are `#[non_exhaustive]`**: `ObserveError`, `StoreError`, `SendError`, `Event`, `NodeEvent`, `DropReason`, @@ -341,7 +385,8 @@ public `salt` field), `Reassembler`, `fragment`, `Frame`, `WireError`, (`TrimPolicy::default()` + field assignment still works). - `Node` keeps its current methods: `new`, `id`, `next_seq`, `observe`, `observe_to`, `poll`, `drain_events`, `belief`, `trim`, `store`, - `transport_mut`. `drain_events` returns `NodeEvent`s. + `transport_mut`. `drain_events` returns `NodeEvent`s: queued + `Delivered` events, then one `Dropped` per nonzero drop counter (§1). - Every remaining public item is documented; `#![deny(missing_docs)]` goes in `tines-core/src/lib.rs` (26 items are missing docs as of the fleet milestone; most of them are the fields of the now-private types). @@ -387,18 +432,30 @@ changes: ### 5. Constants: wire vs local -`consts.rs` splits into two documented groups: +`consts.rs` splits into two documented groups. The rule: **a constant is +a wire constant if changing it changes how a v1 peer decodes or interprets +a frame.** That covers capacity bounds a decoder enforces, and constants +both sides must agree on to read a field. Everything else is local. - **Wire constants**, frozen for wire v1: changing any is a `WIRE_VERSION` - bump. These are `MAX_TOPIC`, `MAX_PAYLOAD`, `MAX_FRAME`, `SUMMARY_BYTES`, - `BLOOM_K`, `ACK_IDS_CAP` and `MAX_SIG` (= 64, Ed25519; Q1), plus the - private `MAX_FRAGMENTS`. Post-quantum signatures don't fit in 64 bytes, so - adding one is a wire v2 / tines-core 2.0 change. + bump. The capacity bounds are `MAX_TOPIC`, `MAX_PAYLOAD`, `MAX_FRAME`, + `SUMMARY_BYTES`, `ACK_IDS_CAP` and `MAX_SIG` (= 64, Ed25519; Q1), plus the private + `MAX_FRAGMENTS`. `BLOOM_K` is a wire constant because both sides must + agree on it to read a summary. + - Post-quantum signatures don't fit in 64 bytes, so adding one is a wire + v2 / tines-core 2.0 change. + - **`MAX_FRAME = 1500` is a known 2.0 trigger (R3).** `Frag.data` is an + inline `Vec`, so a peer can't decode a bigger frame. A + future radio that needs frames over 1500 bytes means wire v2. BLE (L2CAP + CoC) and LoRa are comfortably under it, and a bigger radio can still run + v1 with 1500-byte frames. - **Local capacities and timings**, which can change in a minor release because they never cross the wire: `MAX_PEERS`, `SEND_QUEUE_CAP`, `ACK_BUFFER_CAP`, `BCAST_SENT_CAP`, `EVENT_QUEUE_CAP`, - `SUMMARY_INTERVAL_MS`, `REASSEMBLY_TIMEOUT_MS`, and the drop-report rate - limit. + `SUMMARY_INTERVAL_MS`, `REASSEMBLY_TIMEOUT_MS`, and the size of the + drop-counter table (§1). `EVENT_QUEUE_CAP` is local, even though + `ACK_IDS_CAP` is wire: the event queue never leaves the node, while an + `Ack` frame's id list is decoded by the peer. AGENTS.md's constant list is updated to show the split. @@ -435,9 +492,27 @@ can overflow during a large sync. The 1.0 spec pins what overflow costs: - no panic, - every sent frame is no larger than `mtu()` (randomised per run), - no addressed observation is stored past its lifetime, - - every rejected frame produced a `Dropped` report (up to the rate limit), + - every rejected frame is counted: the drained `Dropped` counts for an + address add up to the frames rejected from it (the overflow counter + included), - stored signatures round-trip unchanged. + **Adversarial-peer mode.** Random bytes rarely get past the decoder, so + a second input mode builds *well-formed* frames from the fuzz input: + - Summaries with chosen bits, including all-zero, all-one and repeated. + - Obs and Frag frames with chosen ids, observers (including the node's + own NodeId), seqs, ages, lifetimes and addressing. + - Acks for ids that were or weren't sent to that address. + - Fragments that conflict, repeat or arrive out of order. + - `from` values that change within a contact. + + These are driven from many addresses, beyond `MAX_PEERS`. On top of the + checks above, this mode checks the §10 properties: peer-table recovery, + bounded output per received summary, no NodeId change within a contact, + and drop-oldest event behaviour. It also checks the custody invariant + under the fleet fixes' F1 rule: an ack is sent only for a stored + observation. + Fuzzing through `Node` keeps every internal private. It runs as a scheduled CI job (not a PR gate, since the toolchain is pinned stable), with the corpus checked in. @@ -464,7 +539,8 @@ can overflow during a large sync. The 1.0 spec pins what overflow costs: - **Wire:** every 1.x release speaks wire v1. Only new optional `Frame` variants may be added, plus new `SigAlgo` ids and signature versions within the 64-byte `MAX_SIG` budget. A wire-v2 change, including any - post-quantum signature, ships as tines-core 2.0. + post-quantum signature or a `MAX_FRAME` above 1500, ships as + tines-core 2.0. - **Identity:** `Observation::id` never changes within a major version, and should not change across majors unless the hash itself is broken. - **API:** standard semver, enforced by `cargo-semver-checks`; the @@ -483,14 +559,73 @@ can overflow during a large sync. The 1.0 spec pins what overflow costs: `tines-store` (0.x) moves to `tines-core = "1"` and its log format follows the new `Observation` encoding. There is no migration: -- The log gains a format header. Opening a log with a missing or older - header is an error that names the fix (delete the log). Silently - misparsing an old log is not allowed. +- The log already has a versioned header and per-record CRC by then: the + fleet fixes (`2026-10-19-tines-fleet-fixes.md` §5) introduce both for + integrity reasons. 1.0 bumps that header's `format_version` for the new + `Observation` encoding. Opening a log with an older version is an error + that names the fix (delete the log). Silently misparsing an old log is + not allowed. - Hosts wipe pre-1.0 logs on upgrade. That's acceptable because no robot runs tines yet, and none will be deployed with a pre-1.0 `tines-core`. - The header is what makes a later migration possible if one is ever needed. Adding it is cheap now and expensive later. +### 10. Core hardening (from `THREAT_MODEL.md`) + +The security assessment at `3b1a644` found core behaviours that a peer can +abuse without any key. All of them are local behaviour (§8), so 1.x could +change them. They're fixed before 1.0 anyway: they're cheap, and 1.0 is +when core's behaviour is written down as a promise. None of them adds +authentication. They bound what an unauthenticated peer can do; 1.1 +decides who is allowed to be a peer. + +1. **F6 — a peer's NodeId is fixed for the contact.** + - Today `on_frame` overwrites `peer.id` with each frame's `from` + (exchange.rs:723), so a peer can claim another identity mid-contact + and be handed that identity's addressed mail. + - New rule: the first decoded frame on a contact sets `peer.id`. A + later frame with a different `from` is dropped and counted as + `DropReason::IdentityChanged` (§1). + - The pin resets on `Contact`/`Lost`. On transports that bind identities + (tines-bt after the fleet fixes), the two always agree. +2. **F8 — idle peers are evicted.** A peer with no inbound frame for + `PEER_IDLE_MS` (local constant, default 30 s) is removed as if `Lost`. + Today peers leave only on `Lost`, so eight spoofed addresses can fill + the table until restart. With the fleet fixes' `Lost` events this + rarely triggers for honest peers, but it bounds the lockout. +3. **F9 — a summary can't be replayed for amplification.** + - At most one received Summary per peer is acted on per + `SUMMARY_INTERVAL_MS`. Extra ones are ignored; that's a policy, not a + drop. + - The send-queue rebuild skips ids already sent to that peer this + contact, using the existing per-peer sent record. That record is + best-effort under its cap, and a miss only costs a resend. + - The worst-case output per received summary stays bounded by + `SEND_QUEUE_CAP`, but repeating the summary no longer re-sends. +4. **F13 — smaller defects:** + - **Event queue drops the oldest** when full, as its docs and the fleet + spec say (today `push` silently drops the newest, exchange.rs:777). + This applies to `Delivered` events; `Dropped` uses counters (§1). + - **Pending acks for a removed peer are purged** on `Lost` or eviction, + so retries to a dead peer can't starve data frames. + - **Frames must be canonical.** Decode rejects trailing bytes after a + complete `Frame` (`postcard::take_from_bytes` and an empty remainder), + counted as `Malformed`. This is wire format, so it can't wait for 1.x, + and §7's vectors pin it with a valid frame plus one trailing byte. +5. **Considered and left out:** + - **A per-peer quota on new inserts** (threat model F10). A big honest + sync is indistinguishable from a flood without authentication, and a + quota would slow convergence for everyone. Storage bounds belong to + the host's trim policy and store cap (host hardening spec §1). + Revisit after 1.1. + - **Refusing inbound observations that claim this node as observer** + (threat model F14). It would stop forged "self" observations beating + `SelfFirst`, but also stop a robot recovering its own history from + peers after losing its log. Signatures in 1.1 fix the real problem. + +New local constants, `PEER_IDLE_MS` and the drop-counter table size, join +§5's local group. `DropReason` gains `IdentityChanged`. + ## Decisions (Kyle, 2026-10-18) - **D1. Reserve the signature slot now: yes.** `Signature { version: u8, @@ -519,10 +654,31 @@ the new `Observation` encoding. There is no migration: - **Q3. Malformed frames are reported the same way as version mismatches**, through `NodeEvent::Dropped { reason }` (§1). +## Review decisions (Kyle, 2026-10-19) + +- **R1. Drop reports carry the transport `Addr`**, not a NodeId (§1). +- **R2. No second queue: drops are counted by `(Addr, reason)`** and + reported as `Dropped { addr, reason, count }` when drained. `Delivered` + receipts can't be evicted by drops, and the counting doubles as the rate + limit (§1). +- **R3. `MAX_FRAME` stays a wire constant at 1500**, and the spec names it + as a known 2.0 trigger (§5, §8). + +The review's other fixes are applied too: +- `Malformed` can also mean a newer peer's optional variant (§1). +- The rule for wire vs local constants is written down (§5). +- The `Observation` encoding change is stated outright (§2). +- The `tines-sim` claim is corrected (§3). +- The version-byte guidance is corrected (§1). + +- **R4. `observed_at` is defined** as the observer's wall-clock time in + milliseconds since the Unix epoch (UTC), or `None` if the observer has no + trusted clock. Relays never change it, and it's unrelated to `Tick` (§2). + ## Left to the builder - The exact `WIRE_VERSION` value, within the rules in §1. -- How drop reports are rate-limited per peer, and the limit's value. +- The size of the drop-counter table (§1). - The `SigAlgo` catch-all representation and the `version` sanity rule (§2a). - Fair-share vs shared FIFO for the bounce table (§6). @@ -532,15 +688,17 @@ the new `Observation` encoding. There is no migration: 2. the envelope and drop reports, 3. `Dest`, then `Signature`, 4. the API narrowing, - 5. docs, fuzzing and MSRV, - 6. the 1.0.0 release, with `cargo-semver-checks` switched on. + 5. core hardening (§10), + 6. docs, fuzzing (including adversarial mode) and MSRV, + 7. the 1.0.0 release, with `cargo-semver-checks` switched on. ## Done means - `sh scripts/ci.sh` is green with `#![deny(missing_docs)]`, an MSRV job and the vector tests. -- The fuzz target has run for an agreed budget (e.g. 24 CPU-hours) with no - findings, and the corpus is committed. +- The fuzz target has run for an agreed budget (e.g. 24 CPU-hours) in both + input modes with no findings, and the corpus is committed. +- `THREAT_MODEL.md` marks F6, F8, F9 and F13 fixed. - README has a "Stability" section with the §8 policy, including the 1.1 signature note; CHANGELOG has a 1.0.0 entry listing every break from 0.3. - AGENTS.md lists this spec, the wire/local constant split and the trait diff --git a/docs/superpowers/specs/2026-10-19-tines-fleet-fixes.md b/docs/superpowers/specs/2026-10-19-tines-fleet-fixes.md new file mode 100644 index 00000000..84d104cd --- /dev/null +++ b/docs/superpowers/specs/2026-10-19-tines-fleet-fixes.md @@ -0,0 +1,320 @@ +# tines fleet deployment v1 — fixes from the security assessment + +Date: 2026-10-19 +Status: draft v1. Not yet reviewed. +Fixes: findings F1–F5 and F17 in `THREAT_MODEL.md` (assessed at `3b1a644`) +Amends: `2026-10-18-tines-fleet-deployment-design.md` (milestone M3) + +## Intent + +The fleet milestone's code has landed, but the security assessment found +that several of its promises don't hold even with no attacker present: + +- custody is lost when a store is full; +- the standalone service never exchanges anything; +- BLE can't accept connections and doesn't keep frames intact; +- restarts repeat sequence numbers and forget released messages; +- one corrupt log record deletes the rest of the log; +- the service's clock runs in the wrong units. + +This spec fixes those six findings so M3 does what its spec says. It adds +no features. + +## Why now, and why not in 1.0 + +These are M3 bugs. M3 can't ship with them, so they can't wait for 1.0. +All but F1 live in host crates (`tines-store`, `tines-bt`, `tines`, +`tines-ros2`), which stay 0.x and aren't covered by the 1.0 promise (1.0 +spec, D4). The core-side hardening that does belong with 1.0 (F6, F8, F9, +F13) is in the 1.0 spec, §10. Host hardening (F10–F12) has its own spec, +`2026-10-19-tines-host-hardening.md`. + +## Goals + +1. Custody is never released for a message the receiver didn't keep (F1). +2. The standalone `tines` service exchanges observations over its + transport (F2). +3. `tines-bt` has the following properties (F3): + - it accepts as well as initiates connections; + - it preserves frame boundaries; + - it binds each connection to the advertised NodeId; + - it reports `Contact`/`Lost`. +4. A robot's `next_seq` never repeats across restarts, and released flags + survive restarts (F4). +5. Log recovery loses only a torn tail. A corrupt record elsewhere costs + that record, not the rest of the log (F5). +6. Every host uses one monotonic millisecond clock for `Tick` (F17). + +## Non-goals + +- **Authentication or encryption of any kind.** The NodeId binding in + goal 3 is only as trustworthy as an unauthenticated advertisement. It + stops accidents and makes `from` checks meaningful; it doesn't stop an + attacker (1.1, 1.0 spec §2b). +- Growth bounds, trimming defaults, bounded channels, IPC permissions, ROS2 + isolation: host hardening spec. +- Peer eviction, summary rate limits, identity pinning in core: 1.0 spec + §10. +- Multi-peer configuration of the service beyond what F3 needs (see §3). + +## Constraints + +- tines-core invariants are unchanged (no_std, no I/O, no panics on + untrusted input, `Addr` ↔ NodeId mapping stays in the transport). +- **The F1 fix is a core behaviour change** and must keep the fleet sim + properties green, plus add the one they missed. +- No robots run tines yet, so log formats may change without migration: + existing logs are wiped (1.0 spec, D5). +- TDD per AGENTS.md: each finding gets a failing regression test first. + +## The how + +### 1. F1 — ack only what was kept (tines-core) + +`Exchange::accept` (exchange.rs ~795-820) currently enqueues an ack for +every receive except a released addressed duplicate, including when +`insert` fails. + +New rule, matching the fleet spec's own eligibility rule ("B never acks a +… store-full receive"): + +| `insert` result | Addressed | Broadcast | +|---|---|---| +| `Ok(true)` (new) | ack | ack | +| `Ok(false)`, local copy not released | ack | ack | +| `Ok(false)`, local copy released | **no ack** (existing rule) | n/a | +| `Err(_)` (full, backend error) | **no ack** | **no ack** | + +- Broadcasts follow the same rule. Acking an unstored broadcast would fire + a false confidence event at the sender. +- No bounce record on `Err` (already true). +- **Tests:** + - A unit test with a store that returns `Full`: no Ack frame is + emitted, and the sender's copy stays unreleased. + - A sim property: the "≥ 1 holder" invariant holds when some nodes use a + store with a small capacity that returns `Full`. Add a capped wrapper + store to tines-sim for this; `MemStore` never fills. +- Fix the `accept` comment ("even an un-stored id"). + +### 2. F2 — the service runs the exchange (tines) + +The service gets a poll loop. Today `serve()` only answers IPC. + +- **Structure:** + - `Node` lives behind a `Mutex`, shared by the IPC thread and a new + exchange thread. + - The exchange thread loops: + 1. `poll(now)` until it returns 0 or a per-iteration cap is reached; + 2. sleep for `poll_interval_ms` (config, default 20 ms); + 3. on schedule, `trim(policy, now)` and `store.flush()`. The trim + policy and its defaults are in the host hardening spec; this spec + only requires that the hook exists and is called. +- **Lock poisoning:** a panic in either thread exits the process with a + non-zero code. Systemd restarts it (host hardening spec). Continuing with + a node in an unknown state isn't allowed. +- **Generic over `Transport`:** the service's core loop is generic over + `Transport`, so tests can run it on an in-memory link. +- **Test:** two service instances over an in-memory transport. An + observation made through one's IPC becomes a belief readable through the + other's IPC. + +### 3. F3 — tines-bt: framing, roles, binding, contact events + +1. **Preserve frame boundaries.** Switch from `l2cap::Stream` to + `l2cap::SeqPacket` (`SeqPacket::connect` and `SeqPacketListener`, both + in bluer 0.17). One L2CAP SDU is one frame. + - The negotiated MTU is still reported through `mtu()`, clamped to + `MAX_FRAME`. + - If SeqPacket proves unusable on the Jetson or Pi BlueZ versions, the + fallback is an explicit `u16` length prefix on a stream. The builder + records which was chosen and why. +2. **Roles.** Every node both advertises and scans. + - When a node discovers an eligible peer (see binding), the one with the + numerically smaller NodeId dials, per the fleet spec rule + (`is_central`). + - The larger one only accepts, on a `SeqPacketListener` bound to the + tines PSM (`0x008D`). + - So every node runs a listener for the whole session, not only after it + loses a race. +3. **Binding.** The scanner decodes the advertised NodeId from service data + (UUID `0xFE7E`, checking the UUID, which `decode_node_id` doesn't do + today) and records `bt address → NodeId` for each eligible advertiser. + - **Eligible** means either a NodeId in the configured allowlist + (`peers`, a list; empty means any tines advertiser), or any tines + advertiser if no allowlist is set. + - **Outbound connection:** bound to the NodeId that was advertised by the + address it dials. + - **Inbound connection:** bound via `peer_addr()` looked up in the scan + map. An inbound connection from an address with no recent eligible + advertisement is closed immediately. + - **Use the discovered address type.** Today it's hard-coded to + `LePublic` (transport.rs:307). +4. **Addr and the `from` check.** + - `Addr` is the bound NodeId of the connection a frame arrived on. + `read_loop` stamps frames with that bound NodeId, not the configured + `peer`. + - `BtTransport::poll` drops any frame whose decoded `from` differs from + that bound `Addr`. The check now means something; it remains + unauthenticated. + - The transport doesn't decode frames; it peeks at the `from` field. + Because core's `Frame` layout is private after 1.0, core exposes a + `pub fn frame_sender(bytes) -> Option` for this. That's new + public API, called out per AGENTS.md, and the only one this spec adds. + It's added to the 1.0 public surface (1.0 spec §3). +5. **Contact and Lost.** + - A connection that is up emits `Event::Contact(addr)`. + - Its end (EOF, error, adapter loss) emits `Event::Lost(addr)`. Today + neither is emitted, so core's per-contact state never resets. + - After a loss, scanning and listening continue, so the node reconnects. +6. **Several peers.** The radio supports up to `MAX_PEERS` concurrent + connections, one per bound NodeId; a second connection to the same + NodeId replaces the first. The single `peer` config field becomes the + optional `peers` allowlist (see 3). This is the minimum needed for a + fleet of more than two robots to work at all. +7. **Errors aren't swallowed.** Failures of the radio thread, adapter + power-on and advertising are surfaced, either through `RadioEvent` or as + a returned error the host logs. The radio thread's exit is reported. + Today all of these are discarded or only logged. + +**Tests** (the `Radio` trait already makes the transport testable without +hardware): +- frame boundaries are preserved for back-to-back frames; +- a frame with a mismatched `from` is dropped; +- `Contact`/`Lost` are emitted; +- an inbound connection from an unknown address is rejected; +- the role is decided by NodeId order. + +The SeqPacket and BlueZ path gets a manual two-device check, written up in +`tines-bt/README.md`. + +### 4. F4 — seq continuity and durable releases (tines-store, both hosts) + +1. **Boot counter.** `LogStore` keeps a persisted boot counter. + - `LogStore::open` increments it and durably writes it (`fsync`) before + returning. + - It then exposes `first_seq() -> u64 = boot_count << 32`, which is + core's documented contract. + - Both hosts seed `Node::new` from `store.first_seq()`, never from + config. The `first_seq` config field is removed. + - A boot can make up to 2³² observations, which is enough for any + robot. +2. **Remove the insert-counting `next_seq`.** `LogStore::next_seq` + (log.rs:352) counts received observations too, so it isn't the node's + seq. It's removed along with the `Meta` record's `next_seq` field. +3. **Releases are written when they happen.** + - `Store::release` appends a `Release { id }` record immediately, rather + than relying on a later `flush` to write the whole released set in + `Meta`. `Meta`'s `released` list goes away, which also removes the + oversized-`Meta` self-truncation in F5. + - `release` stays infallible (1.0 spec §4): an append error is logged, + and the worst case is a duplicate custodian after a reboot. +4. **Flush on a schedule.** Both hosts call `store.flush()` at least every + `flush_interval_ms` (config, default 1000 ms) and on clean shutdown. + `flush` is `fsync`. +5. **Clock marker for ages across reboot.** The fleet spec promised that + pre-reboot elapsed time is folded into ages, but there's no clock-marker + record today. + - `flush` appends a `Clock { tick }` record. On load, + `age += last_clock_tick − stored_at` for each live observation, + saturating, before rebasing. + - Downtime stays invisible (no RTC), as the fleet spec accepted. + - The marker is only meaningful within a boot, so the hosts must use + the F17 clock. +6. **Tests:** + - Reopen twice: `first_seq` strictly increases, and observations made + after each reopen don't collide. + - Release, kill without flush, reopen: the release survives. + - Ages after reopen include the time since `stored_at` up to the last + clock marker. + +### 5. F5 — log integrity and recovery (tines-store) + +New log format, version 1. Old logs are refused, and logs are wiped +(D5). + +``` +file = header record* +header = magic "TINESLOG" ‖ format_version: u16 ‖ reserved: u16 +record = sync: u32 (constant) ‖ len: u32 ‖ crc32c: u32 ‖ body[len] +``` + +- **CRC32C** covers `len ‖ body`. A per-record checksum protects against + accidental corruption only; a keyed MAC is out of scope (threat model + F5, A5). +- **Write cap.** `write_record` refuses a record over `MAX_RECORD_BYTES` + (8 KiB) with `StoreError::Backend`, so the writer can no longer produce + a record the reader rejects. +- **Recovery rule:** + - A record whose CRC, length or decode is bad is **skipped**: scan + forward for the next `sync` value, and check that candidate's CRC + before accepting it. + - Only bad bytes at the end, with no valid record after them, count as a + torn tail. + - A torn tail is truncated, then the file and its directory are fsynced. + - Skipped corrupt records are counted and reported once on open (stderr, + and a counter the host can read). +- **Ids are recomputed** from the decoded observation on load. The stored + `id` field is dropped from the format. +- **Exclusive lock.** `open` takes an exclusive lock (`File::try_lock`, + stable since Rust 1.89; the toolchain is 1.99). A second opener gets a + clear error instead of interleaving. +- **Permissions.** Create the file with mode `0600`, and create its parent + directory (mode `0700`) if it's missing. +- **Header errors.** A missing or unknown header is an error that names + the fix (delete the log), per 1.0 spec §9. The 1.0 work then bumps + `format_version` for the new `Observation` encoding, and doesn't + introduce the header itself. +- **Tests:** + - corrupt middle record: the records after it survive; + - torn tail: truncated; + - an oversized record is refused at write; + - a second `open` fails while the first holds the lock; + - permissions are `0600`. + +### 6. F17 — one monotonic millisecond clock (tines, tines-ros2) + +- Each host has one `HostClock`: an `Instant` captured at process start. + `now() -> Tick` is the milliseconds elapsed since then. + - It replaces `now_ms()` in `tines/src/ipc.rs`, which returns + wall-clock *seconds*. + - It's the clock the ROS2 host documents for callers of + `TinesNode::poll(now)`. +- `LogStore::open` takes `now` from the same clock. That's `0` at + startup, which is now correct by construction: the clock starts at 0. +- **Wall-clock time is used only for `observed_at`** (1.0 spec §2, + milliseconds since the Unix epoch), and only when the host is configured + to trust its clock. +- **Test:** the service's tick advances in milliseconds, and an addressed + observation with `lifetime_ms = 1000` expires after about 1 s, not + about 1000 s. + +## Acceptance + +- `sh scripts/ci.sh` is green, with every regression test above added + first and seen failing. +- The fleet sim properties still pass, plus the new full-store custody + property. +- A manual two-robot BLE check is done: both directions connect, frames + round-trip, the connection is lost and re-established, and an + observation propagates. +- `THREAT_MODEL.md` marks F1–F5 and F17 fixed, with the fixing commit. +- The fleet spec's claims that were false (F3 binding, released flags + across reboot, ages across reboot) are now true, so no errata are + needed. + +## Left to the builder + +- SeqPacket vs length-prefixed stream, if SeqPacket misbehaves on the + target BlueZ (record the outcome). +- The `sync` constant value, and the CRC crate (`crc32c` or equivalent, + `std` only). +- The poll loop's per-iteration cap and lock granularity. +- The scan-map retention time for "recent eligible advertisement". + +## Sequencing + +1. F1 (core, small). +2. F17 (both hosts; the other fixes depend on the clock). +3. F4 and F5 together (one log format change). +4. F2. +5. F3 (largest; needs `frame_sender` from core). diff --git a/docs/superpowers/specs/2026-10-19-tines-host-hardening.md b/docs/superpowers/specs/2026-10-19-tines-host-hardening.md new file mode 100644 index 00000000..fbd4de89 --- /dev/null +++ b/docs/superpowers/specs/2026-10-19-tines-host-hardening.md @@ -0,0 +1,231 @@ +# tines host hardening — before a robot leaves the lab + +Date: 2026-10-19 +Status: draft v1. Not yet reviewed. +Fixes: findings F10–F12 in `THREAT_MODEL.md` (assessed at `3b1a644`) +Depends on: `2026-10-19-tines-fleet-fixes.md`, which adds the poll loop, +the trim/flush hooks and the log format this spec builds on + +## Intent + +The hosts that run tines on a robot (`tines`, `tines-ros2`, and the +`tines-bt` and `tines-store` crates under them) currently trust three +things they shouldn't: +- that storage never fills, +- that every local process is friendly, +- that every participant on the ROS2 domain is friendly. + +This spec bounds every resource a peer, a local process or a ROS2 +publisher can grow, and limits who can act as the robot. It's the bar a +robot must meet before it operates anywhere other than a trusted lab. + +## Why separately from 1.0 + +Everything here is in host crates, which stay 0.x with their own versions +(1.0 spec, D4). None of it changes the wire or core's API. Tying it to the +core 1.0 release would make one wait on the other for no benefit. The bar +is deployment readiness, not a version number. + +## Goals + +1. **Nothing grows without bound** (F10): store size, log size, channels, + and per-request allocations. +2. **Only authorised local users can act as the robot** through the + service's IPC (F11), and no client can block the others. +3. **The ROS2 bridge doesn't feed on itself** (F12): it can be restricted + to a trusted DDS scope, can run read-only, and rate-limits what it + accepts. +4. **Hosts run with least privilege**, under a provided systemd unit. + +## Non-goals + +- Authenticating peers or signing observations (1.1). These limits make + floods *bounded*; they don't make them *impossible*. +- Confidentiality of the radio link (threat model F15). +- Core changes. Per-peer behaviour in the exchange (eviction, summary rate + limits) is in the 1.0 spec §10. + +## Constraints + +- No changes to tines-core's API or wire format. +- Defaults must be safe without configuration: a robot that gets an + untuned config must still be bounded. +- Every limit is configurable, has a documented default, and is reported + (counter or log line) when it bites. A silent limit looks like a bug. + +## The how + +### 1. F10 — bounded growth + +1. **Default trim policy.** Both hosts call `trim` on the poll loop's + schedule (fleet fixes §2), every `trim_interval_ms` (default 10 s), with + a config `TrimPolicy` whose defaults are always on: + - `max_count`: default 50,000 observations (about 50 MB worst case at + the 1 KiB payload cap; size it per robot). + - `max_age_ms`: default 7 days. + - `keep_per_topic`: unset by default. + + The fleet spec's sizing guidance applies: custody traffic must outlive + its lifetime. The host docs say how to size `max_count` for that. +2. **Store full stops being silent.** `LogStore` gains an optional hard cap + (`max_records`, defaulting to 2 × `max_count`). Above it, `insert` + returns `StoreError::Full`. With the fleet fix for F1, a full store no + longer acks, so custody stays with the sender. That's the safe + direction, and it's the point of having a cap. +3. **Log compaction.** When dead records (trimmed or superseded) exceed + `compact_ratio` of the file (default 0.5), and at most once per + `compact_interval_ms`, `LogStore` writes the live records and current + release flags to a new file. It fsyncs the file, renames it over the + old one, and fsyncs the directory. A crash during compaction leaves the + old log intact. Releases written as records (fleet fixes §4) are + carried over. +4. **Bounded channels.** + - tines-bt inbound: a bounded `mpsc` of `BT_INBOUND_CAP` frames (default + 256). When full, the oldest is dropped and counted. A frame is + droppable; the protocol tolerates loss. + - tines-bt outbound: a bounded channel. When full, `send` returns + `SendError::Busy`, which core already retries via `on_send_error`. + - tines-ros2 inbound: bounded (default 256), dropping the oldest with a + counter. +5. **Per-request allocation.** The IPC frame cap drops from 1 MiB to + 16 KiB. The largest valid request is a hex-encoded 64-byte topic plus a + 1 KiB payload in JSON, about 2.3 KiB. +6. **Load.** `LogStore::open` reads the log in a stream instead of + `fs::read` of the whole file, so memory use at open is bounded by the + index, not the file size. + +### 2. F11 — local IPC + +1. **Socket location and mode.** + - Default path `$RUNTIME_DIRECTORY/tines.sock`, provided by systemd's + `RuntimeDirectory=tines` with mode `0750`. + - The socket is created `0660`, owned by the service user and a + configurable `ipc_group` (default `tines`). + - Only members of that group can connect. +2. **Peer credentials.** On accept, read `SO_PEERCRED`. Allow if the uid is + the service uid, or the gid or any supplementary group is `ipc_group`, + or the uid is in the config `ipc_allow_uids`. Anything else is closed + with a logged refusal. This is defence in depth on top of the file + mode. +3. **Read-only clients.** Config `ipc_readonly_uids` and + `ipc_readonly_gid` get `Belief`, `Status` and `Events` only. `Observe` + requires the full grant. Most consumers only read, and shouldn't be + able to speak as the robot. +4. **Don't unlink arbitrary paths.** Remove the configured path only if + it's a socket (`symlink_metadata` → file type socket). Otherwise fail + with an error. +5. **Concurrency and timeouts.** + - Serve each client on its own thread, up to `ipc_max_clients` + (default 8); beyond that, accept and close. + - Each connection has read and write timeouts (`ipc_timeout_ms`, + default 5 s). + - An idle client is closed. No client can block another. +6. **Framing errors close the connection.** An oversized length is + rejected and the connection is closed, so the stream can't be left + desynchronised. Today it returns an error and keeps reading. +7. **`accept` errors** (for example `EMFILE`) are logged and retried with + backoff; they no longer end the daemon. +8. **Events.** `Events` stays a destructive drain (one consumer), and the + docs say so. Fan-out to several consumers is a later feature if it's + needed. + +### 3. F12 — the ROS2 bridge + +1. **No self-echo.** The bridge must not re-observe its own publications. + - Prefer the middleware option that ignores local publications, if + rclrs exposes it. + - Otherwise, filter in the callback by publisher GID if available, or + else drop an inbound message whose payload equals the last payload + the bridge published on that topic within `echo_window_ms` (default + 1 s). + - **Test:** publish once, poll several times; the store gains exactly + one observation. +2. **Read-only mode.** Config `bridge_inbound` (default `false`). When + false, the bridge creates publishers only. ROS2 consumers can read + tines state, but nothing on the domain can make the robot say anything. + Enabling inbound is an explicit decision per deployment. +3. **Rate limits.** When inbound is enabled, a per-topic token bucket + applies (`inbound_rate_per_s`, default 2; burst 4). Excess messages are + dropped and counted. A bridged topic should carry facts, not telemetry + streams. +4. **DDS scope.** + - The docs and the sample unit set a dedicated `ROS_DOMAIN_ID` and + `ROS_AUTOMATIC_DISCOVERY_RANGE=LOCALHOST` unless the deployment needs + the ROS graph off-box. + - For off-box graphs, the docs describe enabling SROS2: an enclave for + the `tines` node, with governance that allows only listed + participants to publish on `/tines/**`. + - This spec documents SROS2 setup; it doesn't automate it. +5. **Receipts topic.** `/tines/receipts` reveals peer NodeIds and + observation ids. It stays on, but can be disabled + (`publish_receipts`, default `true`) for deployments that bridge + off-box without SROS2. + +### 4. Least privilege and the systemd unit + +Provide `packaging/systemd/tines.service` (standalone) and a ROS2 launch +note. The unit: + +```ini +[Service] +User=tines +Group=tines +SupplementaryGroups=bluetooth +StateDirectory=tines # log lives here, 0700 +RuntimeDirectory=tines # socket lives here +RuntimeDirectoryMode=0750 +Restart=on-failure +NoNewPrivileges=yes +ProtectSystem=strict +ProtectHome=yes +PrivateTmp=yes +RestrictAddressFamilies=AF_UNIX AF_BLUETOOTH +CapabilityBoundingSet= +``` + +- The service never runs as root. +- BlueZ access goes through D-Bus policy for the `bluetooth` group. The + docs list the bluetoothd configuration the target images need. +- A panic exits non-zero, and `Restart=on-failure` brings the service back + (fleet fixes §2). + +## Acceptance + +- Each limit has a test that drives it past its cap: the counter or log + fires, and memory or disk stays bounded: + - trim, + - the store cap, + - compaction (including a crash mid-compaction), + - both BT channels, + - the ROS2 inbound channel and rate limit, + - the IPC frame cap. +- **IPC tests:** + - wrong uid refused; + - read-only uid can't `Observe`; + - an idle client doesn't block a second client; + - an oversized frame closes the connection; + - a non-socket path isn't unlinked. +- **ROS2 tests:** the self-echo test, and that read-only mode creates no + subscriptions. These run in the existing ROS2 Docker job (`Dockerfile.ros2`). +- The systemd unit passes `systemd-analyze verify` and runs the service on + the Pi image. +- `THREAT_MODEL.md` marks F10–F12 mitigated and updates §5's + "Bounded memory" row. + +## Left to the builder + +- Exact defaults where marked; they get tuned once real robots are + measured. +- Whether rclrs exposes local-publication filtering (and so which echo + filter is used). +- Compaction triggers beyond the ratio, for example on open. + +## Sequencing + +Follows the fleet fixes. Within this spec, the order is: +1. channels and the IPC cap (small, independent); +2. trim defaults and the store cap; +3. compaction; +4. IPC permissions, credentials and concurrency; +5. ROS2; +6. the systemd unit.