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"); + } +}