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.