diff --git a/Cargo.lock b/Cargo.lock index e7bd31a4..097329b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4893,6 +4893,7 @@ dependencies = [ "bytes", "forgejo-api", "futures-util", + "getrandom 0.3.4", "hive-jobq", "hive-jobq-metrics", "hive-jobq-wire", diff --git a/Cargo.toml b/Cargo.toml index 28c22610..edae4e54 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,6 +81,12 @@ chrono = { version = "0.4", default-features = false, features = [ clap = { version = "4", features = ["derive"] } clap_complete = "4" enumflags2 = { version = "0.7.12", features = ["serde"] } +# The OS CSPRNG, for the one thing in this tree that generates a secret rather +# than receiving one (`swarm-controller::agent_identity`). `getrandom` rather +# than `rand`: the whole need is "fill these bytes from the kernel", and `rand` +# would add `rand_core` + `rand_chacha` to do it through a userspace generator +# this has no use for. +getrandom = "0.3" indicatif = "0.18" hive-sh4re = { path = "hive-sh4re" } hive-agent-sock = { path = "hive-agent-sock" } diff --git a/swarm-controller/Cargo.toml b/swarm-controller/Cargo.toml index c64eea31..412c9fa5 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -26,6 +26,10 @@ forgejo-api.workspace = true # `repo_change_files` calls — forgejo's content API takes base64, never # raw bytes. base64.workspace = true +# `agent_identity.rs` mints the per-agent queue secret, the one value in this +# tree this daemon invents rather than receives. Straight from the kernel's +# CSPRNG — see the workspace entry for why this and not `rand`. +getrandom.workspace = true futures-util.workspace = true # RFC 9457 `application/problem+json` error bodies. Same version + `axum` # feature as hive-c0re: the two daemons answer the same operator UIs, so a diff --git a/swarm-controller/src/agent_identity.rs b/swarm-controller/src/agent_identity.rs index 8475222c..262ec77c 100644 --- a/swarm-controller/src/agent_identity.rs +++ b/swarm-controller/src/agent_identity.rs @@ -5,10 +5,17 @@ //! The swarm mints the agent's certificate so that no hive ever needs the //! capability to mint one; the hive only carries it down. The controller is //! the swarm-level service that does it because it already logs in to the -//! store, and its grant already covers exactly the three objects written here +//! store, and its grant already covers exactly the objects written here //! (`swarm-bao.nix`'s `controllerPolicyText`: `create/update` on //! `secret/data/swarm/agents/*`, on `sys/policies/acl/hive-*`, and on -//! `auth/cert/certs/hive-*`). No new authority is asked for anywhere. +//! `auth/cert/certs/hive-*`). No new authority is asked for anywhere — the +//! agent's queue secret lives under the same `swarm/agents/` prefix the +//! certificate does, which is why adding it costs no grant on either side. +//! +//! **Two credentials, deliberately unrelated.** The certificate is how the +//! agent reaches the store; the queue secret is how it identifies itself to the +//! swarm queue. The second is not derived from the first, so renewing either is +//! a question that can be answered without reference to the other. //! //! Four separate strings have to agree before an agent can authenticate: the //! policy's name, the cert-auth role's name, the certificate's common name, @@ -30,7 +37,7 @@ use anyhow::{Context, Result, bail}; use swarm_secret_client::{ SecretStore, client::{DEFAULT_CERT_MOUNT, Settings}, - mtls, policy, + mtls, policy, queue, }; use time::OffsetDateTime; @@ -191,22 +198,64 @@ fn validity(now: SystemTime, lifetime: Duration) -> Result<(OffsetDateTime, Offs Ok((not_before, not_after)) } +/// How many bytes of kernel randomness a queue secret is before encoding. +/// +/// Thirty-two because the secret is a bearer token compared for equality and +/// nothing else — there is no work factor and no rate limit behind it, so the +/// only defence is that guessing is not worth attempting. Encoded it is 43 +/// characters. +const QUEUE_SECRET_BYTES: usize = 32; + +/// Generate a queue secret: [`QUEUE_SECRET_BYTES`] from the kernel's CSPRNG, +/// base64url without padding. +/// +/// The alphabet matters and is the reason for `URL_SAFE_NO_PAD` rather than +/// the standard engine: the secret is destined to be carried in a token the +/// verifying end splits on `.`, so it must not be able to contain one. This +/// alphabet is `[A-Za-z0-9_-]`, and `=` padding is dropped as well so the +/// value survives anything that treats it as a word. +/// +/// # Errors +/// When the kernel will not supply randomness. Bubbled rather than panicked +/// on: the caller is a job node that reports a named failure, and a secret +/// from a degraded source is worse than no secret. +fn generate_queue_secret() -> Result { + let mut bytes = [0u8; QUEUE_SECRET_BYTES]; + getrandom::fill(&mut bytes).context("drawing a queue secret from the kernel's CSPRNG")?; + Ok(base64::Engine::encode( + &base64::engine::general_purpose::URL_SAFE_NO_PAD, + bytes, + )) +} + /// Give `agent` an identity at the store, and prove it works. /// -/// Four store writes' worth of agreement, then the login that checks it: +/// Five store writes' worth of agreement, then the login that checks it: /// /// 1. mint a leaf whose common name is [`policy::agent_object_name`]; /// 2. publish it at [`mtls::identity_path`], where the agent's hive collects /// it under the hive's own certificate; -/// 3. write the ACL document [`policy::render_agent_with_queue`] renders — +/// 3. publish a queue secret at [`queue::agent_queue_path`] — the agent's own +/// identity at the swarm queue, minted here so that the credential an agent +/// presents names *it* rather than its hive; +/// 4. write the ACL document [`policy::render_agent_with_queue`] renders — /// read on this one agent's paths, plus the hive-shared queue credential /// every agent container on `hive` already receives out of band; -/// 4. write the cert-auth role that ties the three together. +/// 5. write the cert-auth role that ties the three together. /// /// Policy before role, for the reason `read_policy::provision` gives: the role /// names the policy, so the other order leaves a window in which it points at /// nothing. /// +/// ⚠️ **Step 3 is idempotent and step 2 is not.** Re-running this function +/// re-mints the agent's certificate — a fresh leaf the agent picks up on its +/// next boot — but leaves an existing queue secret exactly as it is. The +/// asymmetry is deliberate: an agent holds its queue secret in a live +/// connection, so replacing it would drop that agent off the queue until it +/// reconnected, and this function is re-run deliberately (by the backfill +/// route) against agents that are already running. Nothing here rotates a +/// queue secret; revoking one means deleting the path. +/// /// # Errors /// Anything that stops one of those five steps, with the step named. A /// failure here fails the job node and nothing else — the agent is still @@ -214,6 +263,7 @@ fn validity(now: SystemTime, lifetime: Duration) -> Result<(OffsetDateTime, Offs pub async fn mint_and_verify(authority: &Authority, agent: &str, hive: &str) -> Result<()> { let name = policy::agent_object_name(agent)?; let path = mtls::identity_path(agent)?; + let queue_path = queue::agent_queue_path(agent)?; let (cert, key) = authority.mint_leaf(&name)?; let credential = mtls::Credential { @@ -229,6 +279,50 @@ pub async fn mint_and_verify(authority: &Authority, agent: &str, hive: &str) -> .write(&path, &credential) .await .with_context(|| format!("publishing the agent identity at {path}"))?; + + // Read before write, and `read_optional` rather than `read`, so that only + // a genuine 404 leads to a new secret — see that method for why every + // other failure has to stay a failure here. + let existing: Option = store + .read_optional(&queue_path) + .await + .with_context(|| format!("checking whether {queue_path} already holds a credential"))?; + // The secret survives a re-run; the principal it names does not get to. + // An object whose `hive` disagrees with the hive this node was invoked + // with would grant its holder subjects on the wrong hive, so it is + // corrected — but by rewriting the two name fields around the *same* + // `value`, which is a correction no live connection notices. + let wanted = queue::AgentCredential { + value: match &existing { + Some(existing) => existing.value.clone(), + None => generate_queue_secret()?, + }, + agent: agent.to_owned(), + hive: hive.to_owned(), + }; + if existing.as_ref() == Some(&wanted) { + tracing::info!( + agent, + %queue_path, + "agent queue credential already published; left as it is" + ); + } else { + store + .write(&queue_path, &wanted) + .await + .with_context(|| format!("publishing the agent queue credential at {queue_path}"))?; + tracing::info!( + agent, + hive, + %queue_path, + // Never "rotated": the secret is the same one, only the names + // around it moved. + corrected = existing.is_some(), + "agent queue credential published" + ); + } + let queue_credential = wanted; + store .write_policy(&name, &policy::render_agent_with_queue(agent, hive)?) .await @@ -239,29 +333,42 @@ pub async fn mint_and_verify(authority: &Authority, agent: &str, hive: &str) -> .with_context(|| format!("writing the cert-auth role {name}"))?; tracing::info!(agent, %path, role = %name, "agent store identity published"); - read_back_as_agent(&credential, &name, &path).await?; + read_back_as_agent(&credential, &name, &path, &queue_path, &queue_credential).await?; tracing::info!( agent, role = %name, - "agent store identity verified: the minted leaf logged in and read its own path" + "agent store identity verified: the minted leaf logged in and read both its own paths" ); Ok(()) } /// The consumer of everything [`mint_and_verify`] wrote: log in **as the -/// agent**, with the leaf just minted, and read back the path just published. +/// agent**, with the leaf just minted, and read back both paths just +/// published. /// /// The address and the store's CA come from this process's own `BAO_*` /// environment; the *identity* deliberately does not — see /// [`SecretStore::connect_with_identity`]. The freshly minted private key /// never touches a filesystem. /// +/// Both paths, not just the certificate's, for the reason this function +/// exists at all: the queue credential is readable only because it sits inside +/// the stanza [`policy::render_agent`] already grants, and a policy that +/// drifted from that path would otherwise fail at the agent's first connection +/// — far from here, as a denial naming neither the policy nor the path. +/// /// # Errors /// When the store refuses the login (the role, the authority or the common -/// name disagree), when the read is denied (the policy does not cover the +/// name disagree), when either read is denied (the policy does not cover the /// path, or the role attached the wrong policy), or when what comes back is /// not what went in. -async fn read_back_as_agent(credential: &mtls::Credential, role: &str, path: &str) -> Result<()> { +async fn read_back_as_agent( + credential: &mtls::Credential, + role: &str, + path: &str, + queue_path: &str, + queue_credential: &queue::AgentCredential, +) -> Result<()> { let settings = Settings::from_env() .context("reading this daemon's own store settings for the read-back")?; @@ -287,12 +394,25 @@ async fn read_back_as_agent(credential: &mtls::Credential, role: &str, path: &st if read_back != *credential { bail!("the store returned a different object at {path} than the one just published"); } + + let read_back: queue::AgentCredential = as_agent + .read(queue_path) + .await + .with_context(|| format!("reading {queue_path} back under {role}'s own token"))?; + // The two name fields are compared as well as the secret: they are what + // the verifying end will grant subjects from, so a mismatch here is the + // same class of fault as an unreadable path. + if read_back != *queue_credential { + bail!("the store returned a different object at {queue_path} than the one just published"); + } Ok(()) } #[cfg(test)] mod tests { - use super::{Authority, CLOCK_SKEW, LEAF_LIFETIME, validity}; + use super::{ + Authority, CLOCK_SKEW, LEAF_LIFETIME, QUEUE_SECRET_BYTES, generate_queue_secret, validity, + }; use std::time::{Duration, SystemTime, UNIX_EPOCH}; /// A throwaway CA, minted in-process so no test needs a fixture file. @@ -378,6 +498,28 @@ mod tests { reqwest::Identity::from_pem(&identity).expect("the leaf and its key form a TLS identity"); } + /// The alphabet claim the token format rests on: the secret is carried in + /// a composite the verifying end splits on `.`, so a secret that could + /// contain one would make that split ambiguous. Also the entropy claim — + /// a generator that silently returned a short or constant value would pass + /// every other test in this file. + #[test] + fn a_queue_secret_is_high_entropy_and_carries_no_separator() { + let a = generate_queue_secret().expect("the kernel supplies randomness"); + let b = generate_queue_secret().expect("twice"); + assert_ne!(a, b, "two draws must not agree"); + + // base64 without padding: one character per six bits, rounded up. + assert_eq!(a.len(), (QUEUE_SECRET_BYTES * 8).div_ceil(6)); + assert!( + a.chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_'), + "{a}" + ); + assert!(!a.contains('.'), "{a}"); + assert!(!a.contains('='), "{a}"); + } + /// A misconfiguration that would otherwise look like "not configured": /// half an authority has to be an error, not a silent `None`. /// diff --git a/swarm-secret-client/src/client.rs b/swarm-secret-client/src/client.rs index ad9deef3..0a8232f3 100644 --- a/swarm-secret-client/src/client.rs +++ b/swarm-secret-client/src/client.rs @@ -174,6 +174,31 @@ impl SecretStore { Ok(vaultrs::kv2::read(&self.inner, MOUNT, path).await?) } + /// Read the object stored at `path`, or `None` when nothing is stored + /// there. + /// + /// The verb for a caller whose write must be a no-op if the path is + /// already populated. [`read`][Self::read] cannot serve that: it collapses + /// *absent*, *denied* and *undecodable* into one `Err`, so a caller + /// treating every failure as absence would overwrite a live credential + /// whenever the store was merely unreachable. + /// + /// **Only a 404 is absence.** A denial is a 403 and stays an error — a + /// principal whose grant does not cover the path must not conclude the + /// path is empty, which is how a write ends up clobbering something the + /// caller was never allowed to see. + /// + /// # Errors + /// [`Error::Vault`] for anything that is not a 404: a denial, an + /// unreachable store, or an object that does not decode as `T`. + pub async fn read_optional(&self, path: &str) -> Result, Error> { + match vaultrs::kv2::read(&self.inner, MOUNT, path).await { + Ok(value) => Ok(Some(value)), + Err(vaultrs::error::ClientError::APIError { code: 404, .. }) => Ok(None), + Err(e) => Err(e.into()), + } + } + /// Write `value` at `path`, creating a new version. /// /// # Errors diff --git a/swarm-secret-client/src/queue.rs b/swarm-secret-client/src/queue.rs index ee424e42..3bd5a781 100644 --- a/swarm-secret-client/src/queue.rs +++ b/swarm-secret-client/src/queue.rs @@ -1,11 +1,27 @@ -//! The queue agreement: where a hive's agent-container credential lives in the -//! store, and what the object at that path holds. +//! The queue agreements: where a queue 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. +//! **Two kinds live here, and which is which is the thing to get right.** +//! +//! - [`agent_client_path`] / [`Credential`] are keyed per **hive**: one OIDC +//! client minted per hive at deploy time, shared by every agent container on +//! it. It says which hive a caller belongs to, never which agent. +//! - [`agent_queue_path`] / [`AgentCredential`] are keyed per **agent**: a +//! secret the swarm mints for one agent, at swarm level, with no hive in the +//! chain. It is what makes one agent distinguishable from its co-hived +//! neighbours. +//! +//! They are separate objects rather than one with a nullable field because they +//! are minted by different principals on different events — the hive-scoped one +//! by authelia at deploy time, the per-agent one by `swarm-controller` when an +//! agent is created — and an object whose shape depends on who wrote it is a +//! reader that has to guess. +//! +//! The per-agent secret is deliberately **not** derived from the agent's mTLS +//! identity (the leaf at [`crate::mtls::identity_path`]). That leaf is for +//! reaching the store and nothing else; deriving a queue identity from it would +//! couple the two credentials' lifetimes, so that renewing one would mean +//! renewing the other. use serde::{Deserialize, Serialize}; @@ -25,6 +41,48 @@ pub fn agent_client_path(hive: &str) -> Result { Ok(format!("{prefix}/queue/agent")) } +/// The path holding the secret **one agent** presents to the swarm queue. +/// +/// Under the agent's own principal prefix, and that is the whole reason for +/// this spelling rather than a new top-level one: every agent's ACL document +/// already grants read on `swarm/agents//*` +/// ([`crate::policy::render_agent`]), so a credential here needs no new grant, +/// no policy re-render, and no rewrite of any existing agent's document. +/// +/// # Errors +/// [`Error::PathSegment`] when `agent` contains anything but `[A-Za-z0-9_-]`, +/// which is what keeps one agent's name from addressing another agent's secret. +pub fn agent_queue_path(agent: &str) -> Result { + let prefix = principal_prefix(Kind::Agent, agent)?; + Ok(format!("{prefix}/queue")) +} + +/// What [`agent_queue_path`] holds: the secret, and the principal it proves. +/// +/// Both names ride **in the object** rather than being parsed back out of a +/// composite principal string. Hive and agent names draw from the same +/// alphabet (`hive_types::Ident`, `[a-z0-9-]`), so a principal spelled +/// `hive--agent-` parses two ways for a name containing `-agent-` +/// — and an ambiguous principal parse in an authorisation path is a caller that +/// authenticates fine and is handed somebody else's grant. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AgentCredential { + /// The secret itself. Named to match [`Credential::value`] and + /// [`crate::matrix::Credential::value`] so a nix-side reader spells + /// `bao kv get -field=value` for every kind. + pub value: String, + + /// The agent this secret authenticates. + pub agent: String, + + /// The hive that agent belongs to. + /// + /// Here because the verifying end has no roster to look it up in, and + /// because the subjects an agent is granted are hive-templated — without + /// this field the verifier would know *who* is connecting and not *where*. + pub hive: String, +} + /// 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 @@ -97,4 +155,108 @@ mod tests { assert_eq!(json["value"], "s3cr3t"); assert_eq!(json["client_id"], "hive-alpha-agent"); } + + #[test] + fn an_agent_name_lands_under_its_own_principal_prefix() { + assert_eq!( + agent_queue_path("atlas").expect("a plain name is legal"), + "swarm/agents/atlas/queue" + ); + } + + #[test] + fn a_traversal_in_the_agent_name_is_refused() { + let e = agent_queue_path("../beta").expect_err("a traversal is not"); + assert!(matches!(e, Error::PathSegment { kind: "agent", .. }), "{e}"); + } + + #[test] + fn two_agents_never_share_a_path() { + assert_ne!( + agent_queue_path("atlas").expect("legal"), + agent_queue_path("argus").expect("legal") + ); + } + + /// The reason this path was chosen over a new top-level prefix: it is + /// already inside the stanza every agent's own ACL document grants, so no + /// policy has to change for the credential to be readable by the one agent + /// it belongs to. + #[test] + fn the_agents_existing_policy_already_covers_its_queue_path() { + let document = crate::policy::render_agent("atlas").expect("legal"); + let path = agent_queue_path("atlas").expect("legal"); + let stanza = path + .rsplit_once('/') + .map(|(prefix, _)| format!("secret/data/{prefix}/*")) + .expect("the path has a parent"); + assert!(document.contains(&stanza), "{document}"); + } + + #[test] + fn the_agent_object_round_trips_through_the_store_representation() { + let c = AgentCredential { + value: "s3cr3t".to_owned(), + agent: "atlas".to_owned(), + hive: "alpha".to_owned(), + }; + let json = serde_json::to_string(&c).expect("serialises"); + assert_eq!( + serde_json::from_str::(&json).expect("deserialises"), + c + ); + } + + #[test] + fn the_agent_objects_field_names_are_the_ones_a_nix_reader_asks_for() { + let json = serde_json::to_value(AgentCredential { + value: "s3cr3t".to_owned(), + agent: "atlas".to_owned(), + hive: "alpha".to_owned(), + }) + .expect("serialises"); + assert_eq!(json["value"], "s3cr3t"); + assert_eq!(json["agent"], "atlas"); + assert_eq!(json["hive"], "alpha"); + } + + /// Neither name is optional. An object missing one is not a usable + /// credential — a verifier holding `None` for the hive can only guess at + /// the subjects to grant, and guessing is the failure this shape exists to + /// prevent. + #[test] + fn an_agent_object_missing_a_principal_does_not_decode() { + assert!( + serde_json::from_str::(r#"{"value":"s","agent":"atlas"}"#).is_err() + ); + assert!( + serde_json::from_str::(r#"{"value":"s","hive":"alpha"}"#).is_err() + ); + } + + /// The two kinds are different objects at different paths, and neither + /// decodes as the other — the property that keeps a reader from picking up + /// a hive-shared credential where a per-agent one was meant. + #[test] + fn the_hive_credential_and_the_agent_credential_are_not_interchangeable() { + let hive_json = serde_json::to_string(&Credential { + value: "s".to_owned(), + client_id: "hive-alpha-agent".to_owned(), + }) + .expect("serialises"); + assert!(serde_json::from_str::(&hive_json).is_err()); + + let agent_json = serde_json::to_string(&AgentCredential { + value: "s".to_owned(), + agent: "atlas".to_owned(), + hive: "alpha".to_owned(), + }) + .expect("serialises"); + assert!(serde_json::from_str::(&agent_json).is_err()); + + assert_ne!( + agent_queue_path("atlas").expect("legal"), + agent_client_path("atlas").expect("legal"), + ); + } }