From 744c1ba77e05d264e158d7100fd8d19afc199ca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D7=A0=CF=85=CE=B1=CE=B7=20=D7=A0=CF=85=CE=B1=CE=B7=D1=95?= =?UTF-8?q?=CF=83=CE=B7?= Date: Thu, 17 Sep 2026 20:59:44 -0700 Subject: [PATCH] docs(osapi): plan the per-agent public key store Store the accepted key in the existing enrollment bucket under a second prefix, keyed by machine ID, written only at acceptance. Verify job responses and registrations against it, and keep unverified registrations out of target resolution so a hostname cannot be claimed by a machine that never enrolled under it. Rotation mirrors the controller key: a superseded key with an expiry instant, not a duration. Enforcement uses the PKI flags that already exist and already claim to cover signing, so no new knob appears and an upgrade changes nothing until an operator turns it on. No constitution violations, so Complexity Tracking is omitted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FuKUsHFG1EqZXamffh9M2c --- .../contracts/key-store.md | 72 ++++++++++ .../specs/002-agent-key-store/data-model.md | 89 +++++++++++++ .../osapi/specs/002-agent-key-store/plan.md | 119 +++++++++++++++++ .../specs/002-agent-key-store/quickstart.md | 96 ++++++++++++++ .../specs/002-agent-key-store/research.md | 125 ++++++++++++++++++ 5 files changed, 501 insertions(+) create mode 100644 components/osapi/specs/002-agent-key-store/contracts/key-store.md create mode 100644 components/osapi/specs/002-agent-key-store/data-model.md create mode 100644 components/osapi/specs/002-agent-key-store/plan.md create mode 100644 components/osapi/specs/002-agent-key-store/quickstart.md create mode 100644 components/osapi/specs/002-agent-key-store/research.md diff --git a/components/osapi/specs/002-agent-key-store/contracts/key-store.md b/components/osapi/specs/002-agent-key-store/contracts/key-store.md new file mode 100644 index 0000000..bed218e --- /dev/null +++ b/components/osapi/specs/002-agent-key-store/contracts/key-store.md @@ -0,0 +1,72 @@ +# Contract: the key store and its callers + +Phase 1. The store is internal, so this states the behavioural contract its +callers depend on rather than a wire format. + +## The store + +Three operations, all keyed by machine ID. + +| Operation | Called by | Contract | +| --------- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Record | Enrollment acceptance only | Creates or replaces the record. On replacement, the outgoing key becomes the superseded key with an expiry set from the configured grace period. Never called by any message path. | +| Look up | Response and registration verification, fleet view | Returns the record, or a distinct "no record" answer. A failure to read is its own answer and never resembles "no record". | +| Remove | Enrollment rejection, agent removal | Deletes the record. After it returns, nothing signed by the removed key verifies. | + +**Concurrency**: acceptance and removal are rare and serialised through +enrollment; lookups are frequent and read-only. A cached lookup must be +invalidated by record and remove, not by elapsed time. + +## Response verification + +| Condition | Result | +| -------------------------------------------------------- | -------------------------------- | +| Controller not enforcing | Unchanged from today | +| Signature verifies against current key | Response is a result | +| Signature verifies against superseded key, inside grace | Response is a result | +| Signature verifies against superseded key, grace expired | Rejected, signature mismatch | +| Signature absent or malformed | Rejected, distinct from mismatch | +| No stored record | Rejected, "no stored key" | +| Store unreadable | Rejected, "store unavailable" | + +Rejection is never a silent drop on a single-target call: the job reports +failure. For a broadcast, the response does not count as that agent's reply and +the agent is reported as not having answered. + +## Registration verification + +| Condition | Result | +| ------------------------------------------------------------- | ------------------------------------------ | +| Controller not enforcing | Unchanged from today | +| Signature verifies, hostname and fingerprint match the record | Resolvable | +| Signature verifies, hostname differs from the record | Not resolvable; the record's hostname wins | +| Signature absent, malformed, or mismatched | Not resolvable | +| No stored record | Not resolvable | +| Store unreadable | Not resolvable | + +"Not resolvable" means invisible to target resolution, label matching, facts and +fleet status. It is not an error returned to the agent — the agent keeps +heartbeating, and the fleet view shows why it is not authoritative. + +## Target resolution + +| Situation | Behaviour | +| ----------------------------------------------- | ---------------------------------------------------------------------------- | +| One resolvable registration claims the hostname | Resolves to it | +| Several resolvable registrations claim it | Deterministic choice, preferring the enrolled machine; never iteration order | +| Only unresolvable registrations claim it | Resolves to nothing; the caller is told the target is unknown | +| Controller not enforcing | Unchanged from today | + +## Fleet view + +Each agent in the list reports whether a key is stored and, when it is, the +fingerprint. An operator can therefore see, before enabling enforcement, exactly +which agents would be refused. + +## Rollout + +Enforcement is per side and opt-in (FR-009). Enabling the controller first makes +responses and registrations verifiable while agents that have not re-enrolled +are visible in the fleet view. Enabling agents makes them refuse unsigned jobs, +which the GHSA-3jh4 fix already implements. Neither switch flips as a +consequence of upgrading. diff --git a/components/osapi/specs/002-agent-key-store/data-model.md b/components/osapi/specs/002-agent-key-store/data-model.md new file mode 100644 index 0000000..1d2d694 --- /dev/null +++ b/components/osapi/specs/002-agent-key-store/data-model.md @@ -0,0 +1,89 @@ +# Data Model: Per-agent public key store + +Phase 1. Entities, their rules, and the transitions between them. + +## AcceptedAgent + +The stored record. One per accepted agent, in the enrollment KV bucket under +`accepted.`. + +| Field | Meaning | Rules | +| ---------------- | ----------------------------------------------- | --------------------------------------------------------------------- | +| Machine ID | The agent's permanent identifier | Required; the key of the record; never changes for a record | +| Hostname | The hostname claimed at acceptance | Required; what a registration's hostname is checked against | +| Public key | The key every later message is verified against | Required; recorded only at acceptance (FR-002) | +| Fingerprint | Digest of the public key | Required; what the fleet view shows | +| Accepted at | When acceptance happened | Required | +| Superseded key | The key replaced by the most recent rotation | Optional; absent unless a rotation is inside its grace period | +| Superseded until | When the superseded key stops being accepted | Required when a superseded key is present; an instant, not a duration | + +**Identity**: machine ID. Two records cannot share one, and a record is replaced +in place rather than duplicated. + +**Lifecycle**: + +```text +(none) ──accept──> current key +current key ──accept again (rotation)──> new current key + superseded key + expiry +current key + superseded ──expiry passes──> current key only +any state ──reject or remove──> (none) +``` + +Only enrollment acceptance moves a record rightwards. Nothing an agent sends +creates, changes or refreshes one (FR-002). + +## Registration claim + +What the agent publishes about itself, already present as `AgentRegistration` in +the registry bucket. This feature adds a signature and reclassifies two fields. + +| Field | Before | After | +| ----------- | ------------------------------------ | -------------------------------------------------------------------------------------- | +| Machine ID | Self-reported, trusted | Self-reported, used only to find the stored record | +| Hostname | Self-reported, trusted for targeting | A claim, valid only when the signature verifies against the record found by machine ID | +| Fingerprint | Self-reported, unchecked | A claim, must match the stored fingerprint | +| Signature | Absent | Required when the controller is enforcing; covers the identity-bearing fields | + +**Rule**: a registration is *resolvable* — visible to target resolution, labels, +facts and fleet status — only when its signature verifies against the stored +record for its machine ID (FR-004, FR-005). An unresolvable registration is not +an error to the agent; it is simply not authoritative. + +**Contested hostname**: when more than one registration claims a hostname, only +resolvable ones are candidates, and among those the enrolled machine wins +deterministically (FR-006). + +## Job response claim + +What an agent returns for a unit of work. Already signed by the agent; this +feature makes the signature checkable. + +| Property | Rule | +| ------------------- | ---------------------------------------------------------------------------------------------------------------- | +| Verified | Signature checks out against the stored record for the responding agent | +| Rejected | Signature absent, malformed, or signed by a key that is neither current nor within-grace superseded | +| Effect of rejection | The response is not a result: single-target reports failure, broadcast drops it from that agent's tally (FR-003) | + +## Verification outcome + +Every verification resolves to exactly one of these, and they are never +collapsed (FR-010): + +| Outcome | Meaning | What an operator does | +| ------------------ | ------------------------------------------------------- | ------------------------------------------------------- | +| Verified | Signature matched current or within-grace key | Nothing | +| No stored key | The agent has not been accepted since the store existed | Re-enrol that agent; expected during rollout | +| Signature mismatch | A key that is not this agent's signed the message | Investigate; this is the attack the advisories describe | +| Store unavailable | The record could not be read | Investigate the store; never treated as verified | +| Not enforcing | PKI is off for this side | Nothing; pre-existing behaviour (FR-009) | + +## Relationships + +```text +AcceptedAgent 1 ──── * Registration claim (by machine ID; verifies it) +AcceptedAgent 1 ──── * Job response claim (by machine ID; verifies it) +AcceptedAgent 0..1 ── 1 Superseded key (only during a rotation grace period) +``` + +The store is the authority. Both claim types carry self-reported identity, and +neither may write to the store. diff --git a/components/osapi/specs/002-agent-key-store/plan.md b/components/osapi/specs/002-agent-key-store/plan.md new file mode 100644 index 0000000..7b3eb3d --- /dev/null +++ b/components/osapi/specs/002-agent-key-store/plan.md @@ -0,0 +1,119 @@ +# Implementation Plan: Per-agent public key store + +**Branch**: `002-agent-key-store` | **Date**: 2026-09-18 | **Spec**: +[spec.md](spec.md) + +**Input**: Feature specification from `specs/002-agent-key-store/spec.md` + +## Summary + +Keep an accepted agent's public key after enrollment, and make the two things an +agent sends — its job responses and its registration — verifiable against it. +Today the key is written onto the pending-enrollment record and deleted with +that record on acceptance, so every controller-side verification path finds no +key and skips. One store closes both open advisories: GHSA-3jh4's deferred +response half, and GHSA-j73r, where an unauthenticated registration lets a +machine claim a hostname it never enrolled under and receive that host's work. + +Approach: store the key in the enrollment KV under a distinct prefix at the +moment of acceptance, sign registrations with the agent's existing key, and +verify both paths against the store. Enforcement is opt-in per side, so an +upgrade changes nothing until an operator turns it on. + +## Technical Context + +**Language/Version**: Go, `go 1.26.0` directive, CI builds the floor and stable. + +**Primary Dependencies**: NATS JetStream KV (`nats-io/nats.go/jetstream`), +`crypto/ed25519`, the sibling `osapi-io/nats-client`. No new dependency. + +**Storage**: NATS JetStream KV. The enrollment bucket already exists and already +holds `PendingAgent` records under the `enrollment.` prefix. Accepted keys go in +the same bucket under a second prefix, so no new bucket, config field, or +provisioning step appears. + +**Testing**: `testify/suite` table tests with `validateFunc`, generated mocks, +`just test` as the gate at 99.9% coverage; integration under `test/integration` +behind the `integration` build tag. + +**Target Platform**: Linux controller and agents; Darwin for development. + +**Project Type**: Single Go module — controller, agent and shared packages. + +**Performance Goals**: Verification adds one KV read per verified message on the +controller. Agents heartbeat every 10s and jobs are human-triggered, so the +added load is proportional to fleet size, not throughput. A per-machine-ID cache +with invalidation on acceptance and removal keeps steady-state reads near zero. + +**Constraints**: No new configuration knob — `ControllerPKI.Enabled`, +`AgentPKI.Enabled` and `ControllerPKI.RotationGracePeriod` already exist and are +documented as covering exactly this. Behaviour with PKI disabled must be +byte-for-byte unchanged. Signature verification must never fail open. + +**Scale/Scope**: Fleets in the hundreds. One key per machine ID, plus at most +one superseded key during a rotation grace period. + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Assessment | +| --------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **Documentation** — a repository states in full the conventions binding it | `agent-identity.md` gains what is signed, what is verified, what each failure means, and the rollout order. Nothing is left to this plan alone. **Pass.** | +| **Verification** — a claim is measured, not inspected | Every requirement maps to a test: forged response rejected, unsigned registration invisible to targeting, contested hostname resolved deterministically, grace period honoured then expired, removal effective. `just test` is the evidence. **Pass.** | +| **Tooling** — a tool a repository invokes is declared where it declares its tools | No new tool or dependency. **Pass.** | +| **Correction** — when applying a rule shows the rule is wrong, fix the rule first | The spec already records one such correction: FR-009 began as an open question and was settled by clarify before planning, not during implementation. **Pass.** | +| **Workflow** — design output goes where the workflow reads it | This plan, its research and design artifacts live in the feature directory and consolidate into memory on archive. **Pass.** | + +No violations. Complexity Tracking is therefore empty and omitted. + +## Project Structure + +### Documentation (this feature) + +```text +specs/002-agent-key-store/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── quickstart.md # Phase 1 output +├── contracts/ +│ └── key-store.md # Phase 1 output: the store's contract and failure modes +├── checklists/ +│ └── requirements.md # From /speckit-specify, updated by /speckit-clarify +└── tasks.md # Phase 2 output (/speckit-tasks, not created here) +``` + +### Source Code (repository root) + +```text +internal/controller/enrollment/ +├── types.go # AcceptedAgent record, store interface +├── accept.go # write the key on accept; remove it on reject +├── keystore.go # new: lookup, put, remove, rotation grace +└── keystore_public_test.go + +internal/job/client/ +├── signing.go # verify responses against the store, not a nil check +├── agent.go # ListAgents surfaces whether a key is held +└── client.go # response paths fail closed when enforcing + +internal/agent/ +├── heartbeat.go # sign the registration +└── pki/ # existing Sign/Fingerprint/VerifyWithGrace, unchanged + +internal/validation/ +└── target.go # only verified registrations are resolvable + +internal/controller/api/agent/ +└── agent_list.go # expose key-held state in the fleet view + +docs/docs/sidebar/features/ +└── agent-identity.md # what is signed, what is verified, rollout order +``` + +**Structure Decision**: The store belongs to the enrollment package, because +acceptance is the only event allowed to write it (FR-002) and enrollment already +owns that moment and the KV handle. Verification callers depend on a narrow +lookup interface rather than on the enrollment package's internals, so the job +client and target resolution do not import enrollment wholesale. diff --git a/components/osapi/specs/002-agent-key-store/quickstart.md b/components/osapi/specs/002-agent-key-store/quickstart.md new file mode 100644 index 0000000..b7739c5 --- /dev/null +++ b/components/osapi/specs/002-agent-key-store/quickstart.md @@ -0,0 +1,96 @@ +# Quickstart: validating the key store + +Phase 1. Scenarios that demonstrate the feature end to end. Each states what to +run and what proves it worked. + +## Prerequisites + +```bash +mise exec -- just react-build # ui/dist must exist before anything lints +mise exec -- just deps +``` + +## 1. Nothing changes with PKI off + +```bash +mise exec -- just test +``` + +**Proves**: with `controller.pki.enabled` and `agent.pki.enabled` unset, the +suite passes exactly as before. Responses and registrations behave as they do +today (FR-009, SC-005). + +## 2. An accepted agent keeps its key + +Start a controller and agent with PKI enabled on the controller, accept the +agent, then inspect the fleet view: + +```bash +osapi client agent list +``` + +**Proves**: the agent shows a stored key and its fingerprint (SC-003). Before +acceptance it shows none. + +## 3. A forged response is rejected + +Exercised in unit tests rather than by hand, since forging requires a second +key: + +```bash +mise exec -- go test ./internal/job/client/... -run Signature +``` + +**Proves**: a response signed by a key that is not the agent's is rejected, and +the job reports failure rather than a result (FR-003, SC-001). + +## 4. A hostname cannot be stolen + +```bash +mise exec -- go test ./internal/validation/... ./internal/job/client/... -run Resolve +``` + +**Proves**: an unsigned or mismatched registration is not resolvable, a second +machine claiming an enrolled hostname does not displace it, and resolution among +several claimants is deterministic (FR-004, FR-005, FR-006, SC-002). + +## 5. Rotation does not cause an outage + +```bash +mise exec -- go test ./internal/controller/enrollment/... -run Rotation +``` + +**Proves**: after a rotation the new key verifies, the superseded key verifies +until its expiry instant and not after, and a removed agent's key verifies never +(FR-007, FR-008, SC-004). + +## 6. Failures are distinguishable + +```bash +mise exec -- go test ./internal/job/client/... ./internal/controller/enrollment/... -run Verify +``` + +**Proves**: no stored key, signature mismatch and store unavailable produce +different, identifiable outcomes, and none of them is "verified" (FR-010, +SC-006). + +## 7. A staged rollout works + +Enable the controller side on a fleet where no agent has re-enrolled, then +observe: the fleet view lists every agent as having no stored key, and their +registrations are not authoritative. Re-enrol one agent and it becomes +authoritative without restarting the others. + +**Proves**: enforcement begins when the operator chooses, and progress is +visible throughout (FR-009, SC-007). + +## Gate before review + +```bash +mise exec -- just ready +mise exec -- just test +mise exec -- just docusaurus-fmt-check +``` + +The last is not covered by the first two, and a change touching +`agent-identity.md` needs it. diff --git a/components/osapi/specs/002-agent-key-store/research.md b/components/osapi/specs/002-agent-key-store/research.md new file mode 100644 index 0000000..6251a1c --- /dev/null +++ b/components/osapi/specs/002-agent-key-store/research.md @@ -0,0 +1,125 @@ +# Research: Per-agent public key store + +Phase 0. Each decision below was open when planning began; the Technical Context +now carries no NEEDS CLARIFICATION. + +## 1. Where the store lives + +**Decision**: The existing enrollment KV bucket, under an `accepted.` key +prefix, keyed by machine ID. Pending records keep the `enrollment.` prefix. + +**Rationale**: Acceptance is the only writer (FR-002), and the enrollment +watcher already holds that bucket handle and runs at exactly that moment. The +bucket is already declared, provisioned and configured, so no new bucket name, +config field or startup path appears — and nothing new can be forgotten in a +deployment. Key material here is public, so bucket-level sensitivity does not +change. + +**Alternatives considered**: + +- *A new dedicated KV bucket.* Cleaner separation, but adds a bucket name to + config, a creation path at startup, and an upgrade step, for one small record + per agent. +- *A field on `AgentRegistration` in the registry bucket.* Rejected outright: + the registry is what the agent writes, so storing the authority there would + let the attacker supply the key that validates their own message. That is + GHSA-j73r. +- *A file on the controller's disk beside its own keypair.* Breaks with more + than one controller and has no replication story; the KV already replicates. + +## 2. What identifies an agent + +**Decision**: Machine ID, matching enrollment. + +**Rationale**: Enrollment already records identity by machine ID, and both +advisories stem from hostname being self-asserted and mutable. Hostname becomes +a claim checked against the stored record rather than an identifier. + +**Alternatives considered**: *Hostname* — the thing being attacked. +*Fingerprint* — derived from the key, so it cannot be the lookup for the key +without circularity. + +## 3. How rotation and removal are represented + +**Decision**: The record holds the current key, optionally a superseded key, and +the instant the superseded key stops being accepted. Verification tries current, +then superseded while inside the grace period. Removal deletes the record. + +**Rationale**: Mirrors what the controller key already does — `VerifyWithGrace`, +`PreviousControllerPublicKey` and `ControllerPKI.RotationGracePeriod` — so +operators meet one rotation concept, not two. Storing the expiry instant rather +than a duration means a restart cannot silently extend the window. + +**Alternatives considered**: *Key history list* — unbounded, and nothing needs +the third-oldest key. *No grace at all* — rejects messages signed moments before +a rotation, which is the reason operators turn verification off. + +## 4. How enforcement is switched on + +**Decision**: No new configuration. `ControllerPKI.Enabled` governs the +controller side, `AgentPKI.Enabled` the agent side, per FR-009 and the clarify +answer. Enforcement begins only when the operator sets the flag for that side. + +**Rationale**: Both flags are already documented as covering PKI enrollment +*and* signing, so this makes the documented behaviour true rather than adding a +knob. A new flag would also create a second way to be "half on". + +**Alternatives considered**: *A separate `enforce_signatures` flag* — more +precise, but three states to reason about and a migration story for a field that +duplicates an existing one. + +## 5. What a verification failure reports + +**Decision**: Three distinguishable causes, on both sides: no stored key, +signature mismatch, and store unavailable. Never collapsed into one error. + +**Rationale**: FR-010, and the agent side already does this — GHSA-3jh4 shipped +`ErrControllerKeyUnknown`, `ErrJobEnvelopeMissing` and `ErrJobSignatureInvalid`. +An operator staging a rollout needs "this agent has not re-enrolled yet" to look +nothing like "something is forging messages". + +**Alternatives considered**: *A single verification error* — indistinguishable +in logs at exactly the moment it matters. + +## 6. How the fleet view shows readiness + +**Decision**: `ListAgents` reports, per agent, whether a key is stored and its +fingerprint, and the agent list endpoint surfaces it. + +**Rationale**: SC-003 and SC-007 require an operator to see who would be refused +*before* enabling enforcement. `ListAgents` already walks the registry, so this +is one lookup per agent on a path that is already a list operation. + +**Alternatives considered**: *A separate command* — another surface to learn for +something the fleet view is already for. + +## 7. Cost of verification + +**Decision**: Cache the stored key per machine ID in the controller, invalidated +on acceptance and removal. + +**Rationale**: Verification touches responses and every heartbeat, so an +uncached KV read per message would scale with fleet chatter. Acceptance and +removal are rare and already flow through one package, which makes invalidation +exact rather than time-based. + +**Alternatives considered**: *No cache* — simplest, and acceptable at current +scale, but the read sits on the heartbeat path. *TTL cache* — reintroduces a +window where a removed agent still verifies, which FR-008 forbids. + +## 8. What the agent signs in a registration + +**Decision**: A canonical serialisation of the registration's identity-bearing +fields, with the signature carried beside the record rather than inside the +signed bytes. + +**Rationale**: `AgentRegistration` already carries a self-reported +`Fingerprint`, and today nothing checks it. The stored key is the authority; the +fingerprint becomes a claim that must match. Signing requires a byte sequence +both sides derive identically, which rules out signing the marshalled struct +as-is if field order or optional fields can vary. + +**Alternatives considered**: *Sign the whole marshalled record* — fragile +against serialisation differences and any added field. *Sign only the machine +ID* — a valid signature would then authenticate a registration whose hostname +and labels had been altered, leaving GHSA-j73r open.