diff --git a/docs/trust-boundary/security.md b/docs/trust-boundary/security.md index c6fa8982..ef6c7a75 100644 --- a/docs/trust-boundary/security.md +++ b/docs/trust-boundary/security.md @@ -45,6 +45,28 @@ matrix **identities** — the public handles (`name`, `user_id` `@user:server`, peers can find and address one another on a shared matrix instance. Only the public handle crosses that boundary; the token never does. +### The swarm secret store isn't a boundary between hives + +A hive reads its agents' credentials out of the swarm's secret store with its +own certificate. **Every hive's policy grants read on every agent's +credentials**, not only on the agents it hosts — so a compromised hive can read +the matrix token of an agent running on a different hive. + +That's deliberate and it's the interim state, not the intent. An agent's +credential path doesn't name the hive hosting it (agents move between hives), +so a per-hive grant has to be an enumeration the controller re-emits whenever +the roster changes — and an enumeration that can drift or land out of order +advertises a boundary it doesn't actually hold. A wide grant that says what it +is beats a narrow one that only looks narrow. + +What still holds: the grant is **read-only** (a hive can't write an agent's +credential, so it can't hand itself an agent's identity), and it's scoped to +the agent-credential prefix — nothing else in the store is reachable with it. + +**#4137** tracks narrowing it, with the two candidate directions: scope +the grant per hive (and pay for the re-emission), or give each agent container +its own store identity so credentials never pass through a hive at all. + ### Threat model: prompt injection → confused deputy The realistic adversary **never needs to breach the container**. They supply diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index b7ca8b75..00454f5b 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -47,7 +47,9 @@ mod forge; mod issue_report; mod matrix_account; mod otel_http_client; +mod read_policy; mod status; +mod store; mod vcs_metrics; mod wanted; mod webhook; @@ -1620,8 +1622,15 @@ async fn main() -> Result<()> { let config_prs = forge_client.clone().map(config_pr::spawn); let state_forge = keep_forge_for_state(forge_client, webhook_secret.clone()); + let hives = load_hives(); + // Before serving, because a hive whose policy does not exist cannot read + // anything this daemon writes for it. Same "log and carry on" shape as + // every connect above. + read_policy::ensure_hive_policies(&hives.iter().map(|h| h.name.clone()).collect::>()) + .await; + let state = AppState { - hives: Arc::new(load_hives()), + hives: Arc::new(hives), links: Arc::new(load_links()), wanted: wanted_writer(status.as_ref()), agent_status: agent_status_reader(status.as_ref()), diff --git a/swarm-controller/src/matrix_account.rs b/swarm-controller/src/matrix_account.rs index 31843bda..46f8d38c 100644 --- a/swarm-controller/src/matrix_account.rs +++ b/swarm-controller/src/matrix_account.rs @@ -26,20 +26,11 @@ use axum::extract::State; use axum::http::StatusCode; use serde::{Deserialize, Serialize}; use swarm_queue_client::{CredentialNotice, credential_subject}; -use swarm_secret_client::{SecretStore, matrix}; +use swarm_secret_client::matrix; use utoipa::ToSchema; use super::{AppState, error_problem, swarm_hive}; -/// The cert-auth role this daemon logs into the secret store as. -/// -/// `nix/host-modules/swarm-bao.nix`'s `controllerPolicyName` creates the role, -/// names the policy after it, and `nix/module-eval.nix` pins the literal. -/// -/// ⚠️ Not the certificate's CN. The role *matches on* the CN -/// (`allowed_common_names`), so the two are deliberately different strings. -const CERT_ROLE: &str = "swarm-controller"; - fn default_mode() -> String { "token".to_owned() } @@ -156,7 +147,7 @@ pub async fn put_matrix_account( // touched, so a failed login leaves no partial state behind. let (token, homeserver, user_id) = resolve_credential(&req).await.map_err(|b| *b)?; - let store = SecretStore::from_env(CERT_ROLE).await.map_err(|e| { + let store = crate::store::connect().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; diff --git a/swarm-controller/src/read_policy.rs b/swarm-controller/src/read_policy.rs new file mode 100644 index 00000000..f00297e4 --- /dev/null +++ b/swarm-controller/src/read_policy.rs @@ -0,0 +1,48 @@ +//! Writing the policy a hive's own certificate logs in with. +//! +//! The controller writes a credential; the hive fetches it with its own +//! certificate. Nothing said which paths that certificate may read, so the +//! read half of every delivery answers 403. +//! +//! A startup pass, not a hook: the document is the same for every hive and +//! does not depend on which agents exist ([`swarm_secret_client::policy`] +//! explains why it is that wide), so there is nothing to keep in step with +//! anything. Per-hive failures are reported and skipped — one unwritable +//! policy should not take down a daemon that serves everything else, and the +//! next start retries it. + +use anyhow::{Context, Result}; +use swarm_secret_client::{SecretStore, policy}; + +/// Give every hive in `hives` the read policy its certificate will carry. +/// +/// Does nothing when this deployment has no store identity, which is the +/// shape a controller without a secret store runs in. +pub async fn ensure_hive_policies(hives: &[String]) { + if hives.is_empty() { + return; + } + let store = match crate::store::connect().await { + Ok(store) => store, + Err(e) => { + tracing::info!(reason = %e, "no secret store reachable; hive read policies are not managed here"); + return; + } + }; + for hive in hives { + if let Err(e) = write_one(&store, hive).await { + tracing::warn!(hive, error = %format!("{e:#}"), "writing this hive's read policy failed"); + } + } +} + +/// One hive's policy: its own name, the shared document. +async fn write_one(store: &SecretStore, hive: &str) -> Result<()> { + let name = policy::hive_object_name(hive)?; + store + .write_policy(&name, &policy::render()) + .await + .with_context(|| format!("writing the read policy {name}"))?; + tracing::info!(hive, policy = %name, "hive read policy in place"); + Ok(()) +} diff --git a/swarm-controller/src/store.rs b/swarm-controller/src/store.rs new file mode 100644 index 00000000..d7875876 --- /dev/null +++ b/swarm-controller/src/store.rs @@ -0,0 +1,25 @@ +//! The controller's own identity at the swarm's secret store. +//! +//! Two paths log in: writing an agent's credential, and writing the read +//! grant that lets a hive fetch one back. Both present the same certificate +//! under the same role, so the role is named here rather than at each caller. + +use swarm_secret_client::{Error, SecretStore}; + +/// The cert-auth role the controller logs in under. +/// +/// `nix/host-modules/swarm-bao.nix`'s `controllerPolicyName` creates the role, +/// names the policy after it, and `nix/module-eval.nix` pins the literal. +/// +/// ⚠️ Not the certificate's CN. The role *matches on* the CN +/// (`allowed_common_names`), so the two are deliberately different strings. +pub const CERT_ROLE: &str = "swarm-controller"; + +/// Log in to the store with this deployment's certificate. +/// +/// # Errors +/// Whatever [`SecretStore::from_env`] raises — an unset `BAO_*` variable, an +/// unreadable identity file, or a store that refuses the login. +pub async fn connect() -> Result { + SecretStore::from_env(CERT_ROLE).await +} diff --git a/swarm-queue-client/src/wanted.rs b/swarm-queue-client/src/wanted.rs index 98140b1e..080032a7 100644 --- a/swarm-queue-client/src/wanted.rs +++ b/swarm-queue-client/src/wanted.rs @@ -92,9 +92,12 @@ pub enum AgentState { /// Exists on the hive and is not running. Offline, /// Torn down entirely — the hive runs the destroy template once, then - /// this agent drops out of the declared set rather than staying as a - /// third steady state. Irreversible: there is no state that brings a - /// destroyed agent back short of a fresh deploy. + /// treats absence as agreement rather than re-running it. The key + /// stays in the declared set with this state forever, on purpose: it's + /// what lets a redeclare of an already-destroyed agent be rejected as + /// a terminal-state transition instead of silently reviving it. + /// Irreversible: there is no state that brings a destroyed agent back + /// short of a fresh deploy. Destroyed, } diff --git a/swarm-secret-client/src/client.rs b/swarm-secret-client/src/client.rs index 74e61af8..fcd9bda1 100644 --- a/swarm-secret-client/src/client.rs +++ b/swarm-secret-client/src/client.rs @@ -151,6 +151,22 @@ impl SecretStore { vaultrs::kv2::set(&self.inner, MOUNT, path, value).await?; Ok(()) } + + /// Replace the ACL policy named `name` with `policy`. + /// + /// A whole-document write, not a merge: the store has no other verb, and + /// the caller renders the document from the current agent set anyway, so + /// a stanza that is gone from the render is meant to be gone from the + /// grant. Render the text with [`crate::policy`] rather than by hand. + /// + /// # Errors + /// [`Error::Vault`] when the token's own policy does not cover + /// `sys/policies/acl/` — which is what a controller scoped to the + /// `hive-*` namespace gets for any other name. + pub async fn write_policy(&self, name: &str, policy: &str) -> Result<(), Error> { + vaultrs::sys::policy::set(&self.inner, name, policy).await?; + Ok(()) + } } #[cfg(test)] diff --git a/swarm-secret-client/src/lib.rs b/swarm-secret-client/src/lib.rs index a146e0cd..d54d5d68 100644 --- a/swarm-secret-client/src/lib.rs +++ b/swarm-secret-client/src/lib.rs @@ -8,6 +8,11 @@ //! ([`matrix`]). Each of those is a thing the controller and a hive must say //! identically, so it is said once here. //! +//! [`policy`] is the same kind of agreement seen from the other side: which of +//! those paths a hive's own token may read. It belongs here rather than in the +//! controller because the grant and the path are one statement — spelled +//! differently they produce a 403 that names neither. +//! //! [`client`] is deliberately ignorant of all of it: it moves whatever type a //! caller names, so a second kind of secret is a new module beside [`matrix`] //! and not another field on a struct shared with it. @@ -15,6 +20,7 @@ pub mod client; pub mod matrix; pub mod path; +pub mod policy; pub use client::SecretStore; diff --git a/swarm-secret-client/src/policy.rs b/swarm-secret-client/src/policy.rs new file mode 100644 index 00000000..de45771e --- /dev/null +++ b/swarm-secret-client/src/policy.rs @@ -0,0 +1,115 @@ +//! The read agreement: which credentials a hive's own token may fetch. +//! +//! The mirror of [`crate::matrix`]. That module says where a credential lives; +//! this one says who is allowed to read it, and the two have to agree on the +//! same path or a delivery fails with a 403 that names nothing. +//! +//! ⚠️ Every hive gets the same document, and it grants read on **every** +//! agent's credentials rather than on the ones that hive hosts. That is a +//! decision, not an oversight: an agent's path does not name its hive, so a +//! per-hive grant has to be enumerated and re-emitted, and an enumeration that +//! can silently drift advertises a boundary it does not hold. A wide grant that +//! says so beats a narrow one that only looks narrow. The narrower shapes, and +//! what they would cost, are in `docs/trust-boundary/security.md`. +//! +//! Rendering stays separate from writing so the text can be asserted with no +//! store to talk to. + +use crate::{ + Error, + path::{AGENT_PREFIX, MOUNT, checked_segment}, +}; + +/// Namespace for a hive's own policy and cert-auth role. +/// +/// The controller's own grant is scoped to `hive-*` for both, so this prefix is +/// the difference between a hive the controller may provision and a policy it +/// must not be able to rewrite — including its own. +pub const HIVE_PREFIX: &str = "hive-"; + +/// The policy and cert-auth role name for `hive`. One name, both objects: the +/// role attaches the policy by spelling it identically. +/// +/// # Errors +/// [`Error::PathSegment`] when `hive` holds anything but `[A-Za-z0-9_-]`. +pub fn hive_object_name(hive: &str) -> Result { + checked_segment("hive", hive)?; + Ok(format!("{HIVE_PREFIX}{hive}")) +} + +/// Render the document every hive's policy holds: read on every agent's +/// credentials. +/// +/// Takes no arguments because it depends on nothing — same text for every +/// hive, unchanged by which agents exist. That is what makes it a deploy-time +/// object rather than derived state with a re-emission to get wrong. +/// +/// Read-only: the controller mints these and never reads one back. +#[must_use] +pub fn render() -> String { + format!("path \"{MOUNT}/data/{AGENT_PREFIX}/*\" {{\n capabilities = [\"read\"]\n}}\n") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_document_grants_read_over_the_whole_agent_prefix() { + assert_eq!( + render(), + "path \"secret/data/swarm/agents/*\" {\n capabilities = [\"read\"]\n}\n" + ); + } + + #[test] + fn the_grant_is_read_only() { + // The half of the old policy that survives the widening: a hive reads + // credentials, and a hive that could write one could hand itself an + // agent's identity. + let p = render(); + assert!(!p.contains("create")); + assert!(!p.contains("update")); + assert!(!p.contains("delete")); + assert!(!p.contains("list")); + } + + #[test] + fn no_name_reaches_the_document_at_all() { + // Why the injection cases that used to live here are gone rather than + // relaxed: nothing interpolates into the text any more, so there is no + // stanza for a name to close. `hive_object_name` still validates, + // because a name does reach the policy's *identifier*. + let p = render(); + assert!(!p.contains("pr1ma")); + assert_eq!(p.matches("path \"").count(), 1, "one stanza, no per-agent"); + assert!( + hive_object_name("atlas/*\" { capabilities = [\"root\"] }").is_err(), + "the object NAME is still a place a name can do damage" + ); + } + + #[test] + fn the_legal_charset_is_actually_reachable() { + // The control for the case above: if every name were refused, that + // assertion would pass while proving nothing. + assert!(hive_object_name("a-b_C9").is_ok()); + } + + #[test] + fn every_hive_gets_a_byte_identical_document() { + // The property the deploy-time write depends on: nothing about a hive + // or its agents changes the text, so there is nothing to re-emit. + assert_eq!(render(), render()); + } + + #[test] + fn the_object_name_sits_inside_the_namespace_the_controller_may_write() { + // `hive-` is what the controller's own policy scopes both + // `sys/policies/acl/` and `auth/cert/certs/` to, so a name outside it + // is one the controller cannot create at all. + let n = hive_object_name("pr1ma").expect("legal"); + assert_eq!(n, "hive-pr1ma"); + assert!(n.starts_with(HIVE_PREFIX)); + } +}