diff --git a/docs/trust-boundary/security.md b/docs/trust-boundary/security.md index ef6c7a75..c6fa8982 100644 --- a/docs/trust-boundary/security.md +++ b/docs/trust-boundary/security.md @@ -45,28 +45,6 @@ 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 00454f5b..b7ca8b75 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -47,9 +47,7 @@ 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; @@ -1622,15 +1620,8 @@ 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(hives), + hives: Arc::new(load_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 46f8d38c..31843bda 100644 --- a/swarm-controller/src/matrix_account.rs +++ b/swarm-controller/src/matrix_account.rs @@ -26,11 +26,20 @@ use axum::extract::State; use axum::http::StatusCode; use serde::{Deserialize, Serialize}; use swarm_queue_client::{CredentialNotice, credential_subject}; -use swarm_secret_client::matrix; +use swarm_secret_client::{SecretStore, 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() } @@ -147,7 +156,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 = crate::store::connect().await.map_err(|e| { + let store = SecretStore::from_env(CERT_ROLE).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 deleted file mode 100644 index f00297e4..00000000 --- a/swarm-controller/src/read_policy.rs +++ /dev/null @@ -1,48 +0,0 @@ -//! 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 deleted file mode 100644 index d7875876..00000000 --- a/swarm-controller/src/store.rs +++ /dev/null @@ -1,25 +0,0 @@ -//! 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 080032a7..98140b1e 100644 --- a/swarm-queue-client/src/wanted.rs +++ b/swarm-queue-client/src/wanted.rs @@ -92,12 +92,9 @@ pub enum AgentState { /// Exists on the hive and is not running. Offline, /// Torn down entirely — the hive runs the destroy template once, then - /// 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. + /// 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. Destroyed, } diff --git a/swarm-secret-client/src/client.rs b/swarm-secret-client/src/client.rs index fcd9bda1..74e61af8 100644 --- a/swarm-secret-client/src/client.rs +++ b/swarm-secret-client/src/client.rs @@ -151,22 +151,6 @@ 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 d54d5d68..a146e0cd 100644 --- a/swarm-secret-client/src/lib.rs +++ b/swarm-secret-client/src/lib.rs @@ -8,11 +8,6 @@ //! ([`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. @@ -20,7 +15,6 @@ 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 deleted file mode 100644 index de45771e..00000000 --- a/swarm-secret-client/src/policy.rs +++ /dev/null @@ -1,115 +0,0 @@ -//! 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)); - } -}