From 395ecbdf41961503091d63b94804bb54dc5376a3 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 11 Sep 2026 22:03:59 +0200 Subject: [PATCH] swarm-secret-client: name the hive queue credential, and grant a hive its own kind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agreement half of delivering the agent queue principal's client secret through the store. No producer yet, so nothing writes this path — the unit that does lands in the same PR, with the write grant it needs. queue.rs is the sibling matrix.rs prescribes for a second kind of secret rather than another field on a shared struct. Keyed per HIVE, not per agent: the queue identity is minted once per hive at deploy time and says which hive an agent belongs to, never which agent. The client id rides with the secret for matrix.rs's stated reason — a credential has to be reconstructable from the store alone, and deriving `hive--agent` on the reading side is the split spelling the authelia module warns denies every agent as a timeout. policy.rs's render() takes the hive name now and emits a second, narrow stanza for that hive's own path. The agent stanza is untouched: an agent's path does not name its hive, so narrowing it still needs the enumeration docs/trust-boundary/security.md rejects. A hive path does name its principal, so scoping it costs nothing and drifts nowhere. every_hive_gets_a_byte_identical_document is replaced rather than deleted. Its surviving half is that the text is a function of the deploy-time name alone, so a re-emission cannot drift; the new arms are that one hive's document cannot reach another's path, and that a name which could close the stanza is refused — live again now that a name reaches the document text. Refs #3853 --- swarm-controller/src/read_policy.rs | 4 +- swarm-secret-client/src/client.rs | 9 +- swarm-secret-client/src/lib.rs | 5 +- swarm-secret-client/src/policy.rs | 139 ++++++++++++++++++---------- swarm-secret-client/src/queue.rs | 94 +++++++++++++++++++ 5 files changed, 196 insertions(+), 55 deletions(-) create mode 100644 swarm-secret-client/src/queue.rs diff --git a/swarm-controller/src/read_policy.rs b/swarm-controller/src/read_policy.rs index af437575..b13103ee 100644 --- a/swarm-controller/src/read_policy.rs +++ b/swarm-controller/src/read_policy.rs @@ -165,11 +165,11 @@ async fn provision(hives: &[String]) -> Result<(), Unreachable> { Ok(()) } -/// One hive's policy: its own name, the shared document. +/// One hive's policy: its own name, its own document. async fn write_policy_for(store: &SecretStore, hive: &str) -> Result<()> { let name = policy::hive_object_name(hive)?; store - .write_policy(&name, &policy::render()) + .write_policy(&name, &policy::render(hive)?) .await .with_context(|| format!("writing the read policy {name}"))?; tracing::info!(hive, policy = %name, "hive read policy in place"); diff --git a/swarm-secret-client/src/client.rs b/swarm-secret-client/src/client.rs index 93891cf4..9f39fa9a 100644 --- a/swarm-secret-client/src/client.rs +++ b/swarm-secret-client/src/client.rs @@ -305,7 +305,7 @@ mod tests { let request = WriteAclPolicy { name: "hive-pr1ma".to_owned(), - policy: crate::policy::render(), + policy: crate::policy::render("pr1ma").expect("a plain name is legal"), }; assert_eq!(request.path(), "sys/policies/acl/hive-pr1ma"); } @@ -316,7 +316,7 @@ mod tests { let request = WriteAclPolicy { name: "hive-pr1ma".to_owned(), - policy: crate::policy::render(), + policy: crate::policy::render("pr1ma").expect("a plain name is legal"), }; let body = request .body() @@ -324,7 +324,10 @@ mod tests { .expect("a policy write sends one"); let sent: serde_json::Value = serde_json::from_slice(&body).expect("the body is the JSON the store parses"); - assert_eq!(sent["policy"], crate::policy::render()); + assert_eq!( + sent["policy"], + crate::policy::render("pr1ma").expect("legal") + ); assert!( sent.get("name").is_none(), "`name` addresses the policy in the path; sending it too would make \ diff --git a/swarm-secret-client/src/lib.rs b/swarm-secret-client/src/lib.rs index baf77e10..43815132 100644 --- a/swarm-secret-client/src/lib.rs +++ b/swarm-secret-client/src/lib.rs @@ -5,8 +5,8 @@ //! the rules every path obeys ([`path`]), the translation from this //! deployment's environment into a logged-in client ([`client`]), and, per kind //! of secret, the path it lives at together with the fields it holds -//! ([`matrix`]). Each of those is a thing the controller and a hive must say -//! identically, so it is said once here. +//! ([`matrix`], [`queue`]). 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 @@ -21,6 +21,7 @@ pub mod client; pub mod matrix; pub mod path; pub mod policy; +pub mod queue; pub use client::SecretStore; diff --git a/swarm-secret-client/src/policy.rs b/swarm-secret-client/src/policy.rs index 897b45b5..62201f2d 100644 --- a/swarm-secret-client/src/policy.rs +++ b/swarm-secret-client/src/policy.rs @@ -1,16 +1,25 @@ //! 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. +//! The mirror of [`crate::matrix`] and [`crate::queue`]. Those modules say +//! 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`. +//! The document has one stanza per kind a hive reads, and the two are scoped +//! differently on purpose: +//! +//! ⚠️ The **agent** stanza 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`. +//! +//! The **hive** stanza has no such problem and is therefore narrow: that path +//! names its principal, so scoping it to the reader's own name costs nothing +//! and drifts nowhere. Do not widen it to match its neighbour — the asymmetry +//! is the point. //! //! Rendering stays separate from writing so the text can be asserted with no //! store to talk to. @@ -37,25 +46,46 @@ pub fn hive_object_name(hive: &str) -> Result { Ok(format!("{HIVE_PREFIX}{hive}")) } -/// Render the document every hive's policy holds: read on every agent's -/// credentials. +/// One read stanza. The only shape this module emits, so "read-only" is a +/// property of the renderer rather than of each call site. +fn read_stanza(path: &str) -> String { + format!("path \"{path}\" {{\n capabilities = [\"read\"]\n}}\n") +} + +/// Render `hive`'s policy document: read on every agent's credentials, and on +/// this hive's own. /// -/// 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. +/// The name is the only input, and it is deploy-time — so the document is +/// still a deploy-time object rather than derived state with a re-emission to +/// get wrong. Nothing about which agents exist changes the text. /// /// Read-only: the controller mints these and never reads one back. -/// ⚠️ Still the agent kind alone. The other kinds are deliberately absent: a -/// hive has no business reading a service's or the controller's credentials, -/// and what a hive may read of its *own* kind is a boundary question this -/// module's header answers only for agents. Widening it is a decision, not a -/// consequence of the namespace growing. -#[must_use] -pub fn render() -> String { - format!( - "path \"{MOUNT}/data/{ROOT}/{}/*\" {{\n capabilities = [\"read\"]\n}}\n", +/// +/// ⚠️ The service and controller kinds are deliberately absent: a hive has no +/// business reading a service's or the controller's credentials. Adding either +/// is a boundary decision, not a consequence of the namespace growing. +/// +/// The hive's *own* kind is granted, and that is the decision the agent-only +/// version of this grant said had to be made rather than assumed: a hive holds +/// the queue credential its own agents authenticate with, so it has to read the +/// one principal named after itself — and only that one, which is why the path +/// interpolates the name instead of widening to the whole kind. +/// +/// # Errors +/// [`Error::PathSegment`] when `hive` holds anything but `[A-Za-z0-9_-]` — the +/// name is interpolated into a policy path, so a name that could close the +/// stanza could grant itself anything. +pub fn render(hive: &str) -> Result { + checked_segment("hive", hive)?; + let agents = read_stanza(&format!( + "{MOUNT}/data/{ROOT}/{}/*", <&str>::from(Kind::Agent) - ) + )); + let own = read_stanza(&format!( + "{MOUNT}/data/{ROOT}/{}/{hive}/*", + <&str>::from(Kind::Hive) + )); + Ok(format!("{agents}{own}")) } #[cfg(test)] @@ -63,19 +93,19 @@ mod tests { use super::*; #[test] - fn the_document_grants_read_over_the_whole_agent_prefix() { + fn the_document_grants_the_whole_agent_prefix_and_this_hive_alone() { assert_eq!( - render(), - "path \"secret/data/swarm/agents/*\" {\n capabilities = [\"read\"]\n}\n" + render("pr1ma").expect("a plain name is legal"), + "path \"secret/data/swarm/agents/*\" {\n capabilities = [\"read\"]\n}\n\ + path \"secret/data/swarm/hives/pr1ma/*\" {\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(); + // A hive reads credentials; a hive that could write one could hand + // itself an agent's identity. + let p = render("pr1ma").expect("legal"); assert!(!p.contains("create")); assert!(!p.contains("update")); assert!(!p.contains("delete")); @@ -83,17 +113,36 @@ mod tests { } #[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"); + fn one_hives_document_does_not_reach_another_hives_path() { + // Replaces `every_hive_gets_a_byte_identical_document`: the hive stanza + // is per-reader now, so identical text is no longer the property. This + // is what that test was protecting — that a document says only what the + // deploy-time name puts in it. + let a = render("alpha").expect("legal"); + assert!(a.contains("swarm/hives/alpha/*")); + assert!(!a.contains("beta")); + assert!(!a.contains("swarm/hives/*"), "the hive stanza stays narrow"); + } + + #[test] + fn the_same_name_still_renders_byte_identically() { + // The half of the old property that survives: the text is a function of + // the deploy-time name alone, so a re-emission cannot drift. + assert_eq!( + render("pr1ma").expect("legal"), + render("pr1ma").expect("legal") + ); + } + + #[test] + fn a_name_that_could_close_the_stanza_is_refused() { + // The name reaches the document now, which it did not before — so the + // injection case is live again in the policy TEXT, not just in the + // policy's identifier. + assert!(render("alpha/*\" { capabilities = [\"root\"] }").is_err()); assert!( hive_object_name("atlas/*\" { capabilities = [\"root\"] }").is_err(), - "the object NAME is still a place a name can do damage" + "the object NAME is a place a name can do damage too" ); } @@ -102,13 +151,7 @@ mod tests { // 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()); + assert!(render("a-b_C9").is_ok()); } #[test] diff --git a/swarm-secret-client/src/queue.rs b/swarm-secret-client/src/queue.rs new file mode 100644 index 00000000..da1fdb35 --- /dev/null +++ b/swarm-secret-client/src/queue.rs @@ -0,0 +1,94 @@ +//! The queue agreement: where a hive's agent-container credential lives in the +//! store, and what the object at that path holds. +//! +//! The sibling of [`crate::matrix`], and it differs from it in one way worth +//! reading before using either: a matrix credential is keyed per **agent**, +//! this one per **hive**. Agents are created at runtime, so the queue +//! identity they present is minted once per hive at deploy time and says which +//! hive an agent belongs to, never which agent. + +use serde::{Deserialize, Serialize}; + +use crate::{ + Error, + path::{Kind, principal_prefix}, +}; + +/// The path holding the client secret that agent containers on `hive` present +/// to the swarm queue. +/// +/// # Errors +/// [`Error::PathSegment`] when `hive` contains anything but `[A-Za-z0-9_-]`, +/// which is what keeps one hive's name from addressing another hive's secret. +pub fn agent_client_path(hive: &str) -> Result { + let prefix = principal_prefix(Kind::Hive, hive)?; + Ok(format!("{prefix}/queue/agent")) +} + +/// What the path holds: the client secret, plus the client id it belongs to. +/// +/// The id rides with the secret for the same reason the homeserver rides with +/// a matrix token — a credential has to be reconstructable from the store +/// alone. Deriving it on the reading side instead would mean spelling +/// `hive--agent` in a second place, and the authelia module's own option +/// says what a split spelling costs: every agent is denied as a timeout. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct Credential { + /// The secret itself. Named to match [`crate::matrix::Credential::value`] + /// so a nix-side reader spells `bao kv get -field=value` for either kind. + pub value: String, + + /// The OIDC client id the secret authenticates. + pub client_id: String, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_hive_name_lands_under_its_own_principal_prefix() { + assert_eq!( + agent_client_path("alpha").expect("a plain name is legal"), + "swarm/hives/alpha/queue/agent" + ); + } + + #[test] + fn a_traversal_in_the_hive_name_is_refused() { + let e = agent_client_path("../beta").expect_err("a traversal is not"); + assert!(matches!(e, Error::PathSegment { kind: "hive", .. }), "{e}"); + } + + #[test] + fn two_hives_never_share_a_path() { + assert_ne!( + agent_client_path("alpha").expect("legal"), + agent_client_path("beta").expect("legal") + ); + } + + #[test] + fn the_object_round_trips_through_the_store_representation() { + let c = Credential { + value: "s3cr3t".to_owned(), + client_id: "hive-alpha-agent".to_owned(), + }; + let json = serde_json::to_string(&c).expect("serialises"); + assert_eq!( + serde_json::from_str::(&json).expect("deserialises"), + c + ); + } + + #[test] + fn the_field_names_the_nix_reader_asks_for_are_the_ones_written() { + let json = serde_json::to_value(Credential { + value: "s3cr3t".to_owned(), + client_id: "hive-alpha-agent".to_owned(), + }) + .expect("serialises"); + assert_eq!(json["value"], "s3cr3t"); + assert_eq!(json["client_id"], "hive-alpha-agent"); + } +}