diff --git a/docs/swarm/credentials.md b/docs/swarm/credentials.md index 1421b0f5..b2c209e9 100644 --- a/docs/swarm/credentials.md +++ b/docs/swarm/credentials.md @@ -468,7 +468,15 @@ The longest hold is the controller's forge work, which stops after two minutes. A client that keeps a `SecretStore` handle longer than an hour must log in again, because nothing renews the token. A role the controller writes takes the TTL at its next write: a hive's at the controller's start, an agent's at its -next certificate renewal. At that rate an hour keeps about 700 tokens live. +next certificate mint or renewal. At its start the controller also reads every +existing `hive-agent-*` role and, for one whose `token_ttl` or `token_max_ttl` +differs from the TTL, writes those two fields and no others. The store keeps the +fields a write leaves out, so the role's certificate, allowed names and +policies stay. A role already at the TTL isn't written. The start logs one line +with `rewritten`, `already_current` and `failed` counts, and a role that fails +gets a warning that names it, then the pass moves on. Tokens issued before +expire on their own; the controller revokes nothing. At that rate an hour keeps +about 700 tokens live. The granter's role keeps 15 minutes. Only the one-time bootstrap token writes it, and a deployed store never rewrites it, so the longer TTL would not reach diff --git a/swarm-controller/src/agent_role_ttl.rs b/swarm-controller/src/agent_role_ttl.rs new file mode 100644 index 00000000..c1d8b310 --- /dev/null +++ b/swarm-controller/src/agent_role_ttl.rs @@ -0,0 +1,233 @@ +//! The token lifetime of the agents' cert-auth roles that already exist. +//! +//! A role is written with its lifetimes only when an agent's certificate is +//! minted or renewed, and a role written before the lifetimes were set leaves +//! every login at the mount's default of 32 days, one store lease each. The +//! start pass in [`crate::read_policy`] calls [`bring_current`] so those roles +//! take [`swarm_secret_client::client::CERT_ROLE_TOKEN_TTL`] without waiting for +//! a renewal. +//! +//! Only the two lifetimes are written: the role's certificate, allowed names +//! and policies stay as they are, and no token or lease is revoked. Tokens +//! issued before expire on their own. + +use swarm_secret_client::{client::DEFAULT_CERT_MOUNT, policy}; + +use crate::store::AccessStore; + +/// What one pass did, one count per role. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct Counts { + pub rewritten: usize, + pub already_current: usize, + pub failed: usize, +} + +/// Give every agent role among `roles`, a listing of the cert mount's roles, +/// the current token lifetimes, and log the counts once. +/// +/// A role that cannot be read or written is named in a warning and skipped, so +/// it never stops the rest or the start. A role gone since the listing is +/// counted nowhere: there is nothing left to bring current. +pub async fn bring_current(store: &impl AccessStore, roles: &[String]) -> Counts { + let mut counts = Counts::default(); + for agent in policy::agents_from_role_names(roles) { + let Ok(role) = policy::agent_object_name(&agent) else { + continue; + }; + match bring_one(store, &role).await { + Ok(Outcome::Rewritten) => counts.rewritten += 1, + Ok(Outcome::AlreadyCurrent) => counts.already_current += 1, + Ok(Outcome::Gone) => {} + Err(e) => { + counts.failed += 1; + tracing::warn!(role, error = %e, "this agent role's token lifetime could not be brought current"); + } + } + } + tracing::info!( + rewritten = counts.rewritten, + already_current = counts.already_current, + failed = counts.failed, + "agent role token lifetimes checked" + ); + counts +} + +enum Outcome { + Rewritten, + AlreadyCurrent, + Gone, +} + +async fn bring_one( + store: &impl AccessStore, + role: &str, +) -> Result { + let Some(ttls) = store.cert_role_token_ttls(DEFAULT_CERT_MOUNT, role).await? else { + return Ok(Outcome::Gone); + }; + if ttls.is_current() { + return Ok(Outcome::AlreadyCurrent); + } + store + .set_cert_role_token_ttls(DEFAULT_CERT_MOUNT, role) + .await?; + Ok(Outcome::Rewritten) +} + +#[cfg(test)] +mod tests { + use swarm_secret_client::client::{CERT_ROLE_TOKEN_TTL_SECS, CertRoleTokenTtls}; + + use crate::store::fake::{FakeAccess, FakeRole}; + + use super::{Counts, bring_current}; + + const CURRENT: CertRoleTokenTtls = CertRoleTokenTtls { + token_ttl: CERT_ROLE_TOKEN_TTL_SECS, + token_max_ttl: CERT_ROLE_TOKEN_TTL_SECS, + }; + const STALE: CertRoleTokenTtls = CertRoleTokenTtls { + token_ttl: 0, + token_max_ttl: 0, + }; + + fn role(agent: &str, ttls: CertRoleTokenTtls) -> FakeRole { + FakeRole { + certificate: format!("CA-{agent}"), + common_name: format!("hive-agent-{agent}"), + policies: vec!["hive-agent".to_owned()], + ttls, + } + } + + fn listing(agents: &[&str]) -> Vec { + agents.iter().map(|a| format!("hive-agent-{a}")).collect() + } + + #[tokio::test] + async fn a_stale_role_takes_the_ttl_and_keeps_its_certificate_names_and_policies() { + let before = role("a", STALE); + let store = FakeAccess::default().with_role("hive-agent-a", before.clone()); + let counts = bring_current(&store, &listing(&["a"])).await; + assert_eq!( + counts, + Counts { + rewritten: 1, + ..Counts::default() + } + ); + assert_eq!( + store.role("hive-agent-a"), + Some(FakeRole { + ttls: CURRENT, + ..before + }) + ); + } + + #[tokio::test] + async fn a_role_with_a_lifetime_unset_or_different_is_rewritten() { + let store = FakeAccess::default() + .with_role( + "hive-agent-a", + role( + "a", + CertRoleTokenTtls { + token_ttl: CERT_ROLE_TOKEN_TTL_SECS, + token_max_ttl: 0, + }, + ), + ) + .with_role( + "hive-agent-b", + role( + "b", + CertRoleTokenTtls { + token_ttl: 900, + token_max_ttl: 900, + }, + ), + ); + let counts = bring_current(&store, &listing(&["a", "b"])).await; + assert_eq!(counts.rewritten, 2); + assert_eq!(store.role("hive-agent-a").expect("kept").ttls, CURRENT); + assert_eq!(store.role("hive-agent-b").expect("kept").ttls, CURRENT); + } + + #[tokio::test] + async fn a_current_role_is_read_and_not_written() { + let store = FakeAccess::default().with_role("hive-agent-a", role("a", CURRENT)); + let counts = bring_current(&store, &listing(&["a"])).await; + assert_eq!( + counts, + Counts { + already_current: 1, + ..Counts::default() + } + ); + assert_eq!(store.calls(), ["cert_role_token_ttls hive-agent-a"]); + } + + #[tokio::test] + async fn a_role_that_fails_does_not_stop_the_others() { + let store = FakeAccess::default() + .with_role("hive-agent-a", role("a", STALE)) + .with_role("hive-agent-b", role("b", STALE)) + .with_role("hive-agent-c", role("c", STALE)) + .with_role("hive-agent-d", role("d", CURRENT)) + .with_unreadable_role("hive-agent-a") + .with_unwritable_role("hive-agent-b"); + let counts = bring_current(&store, &listing(&["a", "b", "c", "d"])).await; + assert_eq!( + counts, + Counts { + rewritten: 1, + already_current: 1, + failed: 2, + } + ); + // The ones after the failures were still handled. + assert_eq!(store.role("hive-agent-c").expect("kept").ttls, CURRENT); + // The failed ones were left as they were. + assert_eq!(store.role("hive-agent-a").expect("kept").ttls, STALE); + assert_eq!(store.role("hive-agent-b").expect("kept").ttls, STALE); + } + + #[tokio::test] + async fn only_agent_roles_are_touched() { + let store = FakeAccess::default() + .with_role("hive-agent-a", role("a", STALE)) + .with_role("hive-pr1ma", role("pr1ma", STALE)) + .with_role("swarm-controller", role("c", STALE)); + let roles = vec![ + "hive-agent-a".to_owned(), + "hive-pr1ma".to_owned(), + "swarm-controller".to_owned(), + ]; + let counts = bring_current(&store, &roles).await; + assert_eq!(counts.rewritten, 1); + assert_eq!(store.role("hive-agent-a").expect("kept").ttls, CURRENT); + assert_eq!(store.role("hive-pr1ma").expect("kept").ttls, STALE); + assert_eq!(store.role("swarm-controller").expect("kept").ttls, STALE); + } + + #[tokio::test] + async fn a_role_gone_since_the_listing_is_counted_nowhere() { + let store = FakeAccess::default().with_role("hive-agent-b", role("b", STALE)); + let counts = bring_current(&store, &listing(&["a", "b"])).await; + assert_eq!( + counts, + Counts { + rewritten: 1, + ..Counts::default() + } + ); + assert!( + !store + .calls() + .contains(&"set_cert_role_token_ttls hive-agent-a".to_owned()) + ); + } +} diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 2a70d58d..0b0be9b3 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -31,6 +31,7 @@ mod agent_limits; mod agent_login; mod agent_permissions; mod agent_renewal; +mod agent_role_ttl; mod agent_state_stream; mod agent_status; mod agents_md; @@ -224,7 +225,8 @@ async fn main() -> Result<()> { let hive_names: Vec = hives.iter().map(|h| h.name.clone()).collect(); read_policy::ensure_hive_access(hive_names).await; // The one policy every agent's role attaches, and the entity each agent's - // login is read under, are checked here at every start. + // login is read under, are checked here at every start, and so is the + // token lifetime of the roles that exist. read_policy::ensure_agent_policies().await; let state = AppState { diff --git a/swarm-controller/src/read_policy.rs b/swarm-controller/src/read_policy.rs index c651d189..cc236ba4 100644 --- a/swarm-controller/src/read_policy.rs +++ b/swarm-controller/src/read_policy.rs @@ -22,7 +22,8 @@ //! a config change means a redeploy. //! //! The shared agent policy is written by the same kind of pass -//! ([`ensure_agent_policies`]), together with each agent's identity entity. +//! ([`ensure_agent_policies`]), together with each agent's identity entity and +//! the token lifetimes of its existing role ([`crate::agent_role_ttl`]). //! Its roster is the store's `hive-agent-*` cert-auth roles. use std::{future::Future, time::Duration}; @@ -299,12 +300,13 @@ pub async fn ensure_agent_policies() { } } -/// One pass: log in once, list the agents, write the shared agent policy and -/// give each agent its identity entity. +/// One pass: log in once, list the agents, write the shared agent policy, give +/// each agent its identity entity and bring each existing role's token +/// lifetimes current. /// -/// The policy and entities only. An agent's cert-auth role and certificate are -/// `agent_identity::mint_and_verify`'s, and that reissues the certificate, so -/// it is not what a pass on every start may call. +/// The roles' certificate, names and policies are not touched. They and the +/// certificate are `agent_identity::mint_and_verify`'s, and that reissues the +/// certificate, so it is not what a pass on every start may call. /// /// # Errors /// [`Unreachable`] when the store cannot be reached or will not list its @@ -342,6 +344,7 @@ async fn rewrite_agent_policies() -> Result<(), Unreachable> { ), } crate::agent_entity::ensure_agents(&store, &policy::agents_from_role_names(&roles)).await; + crate::agent_role_ttl::bring_current(&store, &roles).await; Ok(()) } diff --git a/swarm-controller/src/store.rs b/swarm-controller/src/store.rs index eff037d2..210c7709 100644 --- a/swarm-controller/src/store.rs +++ b/swarm-controller/src/store.rs @@ -8,7 +8,10 @@ use std::future::Future; -use swarm_secret_client::{Error, SecretStore, client::EntityRef}; +use swarm_secret_client::{ + Error, SecretStore, + client::{CertRoleTokenTtls, EntityRef}, +}; /// The cert-auth role the controller logs in under. /// @@ -48,6 +51,20 @@ pub(crate) trait AccessStore { policies: &[&str], ) -> impl Future> + Send; + /// As [`SecretStore::cert_role_token_ttls`]. + fn cert_role_token_ttls( + &self, + mount: &str, + name: &str, + ) -> impl Future, Error>> + Send; + + /// As [`SecretStore::set_cert_role_token_ttls`]. + fn set_cert_role_token_ttls( + &self, + mount: &str, + name: &str, + ) -> impl Future> + Send; + /// As [`SecretStore::auth_mount_accessor`]. fn auth_mount_accessor( &self, @@ -99,6 +116,18 @@ impl AccessStore for SecretStore { SecretStore::write_cert_role(self, mount, name, certificate, common_name, policies).await } + async fn cert_role_token_ttls( + &self, + mount: &str, + name: &str, + ) -> Result, Error> { + SecretStore::cert_role_token_ttls(self, mount, name).await + } + + async fn set_cert_role_token_ttls(&self, mount: &str, name: &str) -> Result<(), Error> { + SecretStore::set_cert_role_token_ttls(self, mount, name).await + } + async fn auth_mount_accessor(&self, mount: &str) -> Result { SecretStore::auth_mount_accessor(self, mount).await } @@ -137,16 +166,35 @@ impl AccessStore for SecretStore { /// outcome of the access calls. #[cfg(test)] pub(crate) mod fake { - use std::sync::Mutex; + use std::{collections::BTreeMap, sync::Mutex}; - use swarm_secret_client::{Error, client::EntityRef}; + use swarm_secret_client::{ + Error, + client::{CERT_ROLE_TOKEN_TTL_SECS, CertRoleTokenTtls, EntityRef}, + }; use super::AccessStore; + /// A cert-auth role as the fake keeps it: what a write of the role sets, + /// and the lifetimes a write of only those can change. + #[derive(Clone, Debug, PartialEq, Eq)] + pub(crate) struct FakeRole { + pub(crate) certificate: String, + pub(crate) common_name: String, + pub(crate) policies: Vec, + pub(crate) ttls: CertRoleTokenTtls, + } + /// What the fake answers; the default is a store with a readable cert mount /// and no entities, which accepts every write. #[derive(Default)] pub(crate) struct FakeAccess { + /// The cert-auth roles, by name. + roles: Mutex>, + /// Roles whose lifetimes cannot be read. + unreadable_roles: Vec, + /// Roles whose lifetimes cannot be written. + unwritable_roles: Vec, /// The accessor read fails. no_accessor: bool, /// Every policy write is refused. @@ -182,6 +230,32 @@ pub(crate) mod fake { } } + /// A store that holds `role` under `name`. + pub(crate) fn with_role(self, name: &str, role: FakeRole) -> Self { + self.roles + .lock() + .expect("not poisoned") + .insert(name.to_owned(), role); + self + } + + /// A store that cannot read the lifetimes of the role `name`. + pub(crate) fn with_unreadable_role(mut self, name: &str) -> Self { + self.unreadable_roles.push(name.to_owned()); + self + } + + /// A store that refuses to write the lifetimes of the role `name`. + pub(crate) fn with_unwritable_role(mut self, name: &str) -> Self { + self.unwritable_roles.push(name.to_owned()); + self + } + + /// The role `name` as the store holds it now. + pub(crate) fn role(&self, name: &str) -> Option { + self.roles.lock().expect("not poisoned").get(name).cloned() + } + /// The calls so far, as `verb name` strings, in order. pub(crate) fn calls(&self) -> Vec { self.calls.lock().expect("not poisoned").clone() @@ -217,11 +291,50 @@ pub(crate) mod fake { &self, _mount: &str, name: &str, - _certificate: &str, - _common_name: &str, + certificate: &str, + common_name: &str, policies: &[&str], ) -> Result<(), Error> { self.log(format!("write_cert_role {name} {}", policies.join(","))); + self.roles.lock().expect("not poisoned").insert( + name.to_owned(), + FakeRole { + certificate: certificate.to_owned(), + common_name: common_name.to_owned(), + policies: policies.iter().map(|p| (*p).to_owned()).collect(), + ttls: CertRoleTokenTtls { + token_ttl: CERT_ROLE_TOKEN_TTL_SECS, + token_max_ttl: CERT_ROLE_TOKEN_TTL_SECS, + }, + }, + ); + Ok(()) + } + + async fn cert_role_token_ttls( + &self, + _mount: &str, + name: &str, + ) -> Result, Error> { + self.log(format!("cert_role_token_ttls {name}")); + if self.unreadable_roles.iter().any(|r| r == name) { + return Err(Self::denied()); + } + Ok(self.role(name).map(|r| r.ttls)) + } + + async fn set_cert_role_token_ttls(&self, _mount: &str, name: &str) -> Result<(), Error> { + self.log(format!("set_cert_role_token_ttls {name}")); + if self.unwritable_roles.iter().any(|r| r == name) { + return Err(Self::denied()); + } + // Only the lifetimes: the store keeps the fields a write omits. + if let Some(role) = self.roles.lock().expect("not poisoned").get_mut(name) { + role.ttls = CertRoleTokenTtls { + token_ttl: CERT_ROLE_TOKEN_TTL_SECS, + token_max_ttl: CERT_ROLE_TOKEN_TTL_SECS, + }; + } Ok(()) } diff --git a/swarm-secret-client/src/client.rs b/swarm-secret-client/src/client.rs index 17a236e3..f1be2e8f 100644 --- a/swarm-secret-client/src/client.rs +++ b/swarm-secret-client/src/client.rs @@ -37,6 +37,10 @@ pub const DEFAULT_CERT_MOUNT: &str = "cert"; /// again rather than keep a handle past this. pub const CERT_ROLE_TOKEN_TTL: &str = "1h"; +/// [`CERT_ROLE_TOKEN_TTL`] in seconds, the unit the store reports a role's +/// lifetimes in. +pub const CERT_ROLE_TOKEN_TTL_SECS: u64 = 3600; + /// Upper bound on one request to the store, login included. /// /// vaultrs sets no timeout by default, so a store that accepts the connection @@ -388,6 +392,46 @@ impl SecretStore { Ok(()) } + /// The token lifetimes of the cert-auth role `name`, or `None` when the + /// role does not exist. + /// + /// # Errors + /// [`Error::Vault`] for anything that is not a 404: a denial, or an + /// unreachable store. + pub async fn cert_role_token_ttls( + &self, + mount: &str, + name: &str, + ) -> Result, Error> { + let request = ReadCertRoleTtls { + mount: mount.to_owned(), + name: name.to_owned(), + }; + match vaultrs::api::exec_with_result(&self.inner, request).await { + Ok(ttls) => Ok(Some(ttls)), + Err(e) if is_absent(&e) => Ok(None), + Err(e) => Err(e.into()), + } + } + + /// Set the token lifetimes of the existing cert-auth role `name` to + /// [`CERT_ROLE_TOKEN_TTL`], leaving the rest of the role as it is. + /// + /// The write names no other field, and the store keeps the fields a write + /// leaves out (`builtin/credential/cert/path_certs.go`, `pathCertWrite`, + /// `OpenBao` 2.6.5), so the role's certificate, allowed names and policies + /// stay. A role that does not exist would be created without a certificate + /// and refused; callers read the role first. + /// + /// # Errors + /// [`Error::Vault`] when the token's policy does not cover + /// `auth//certs/`, or the store refuses the write. + pub async fn set_cert_role_token_ttls(&self, mount: &str, name: &str) -> Result<(), Error> { + vaultrs::api::exec_with_empty(&self.inner, set_cert_role_token_ttls_request(mount, name)) + .await?; + Ok(()) + } + /// Have the PKI role `role` on `mount` issue a client certificate for /// `common_name`, returning it with its key and the issuing CA. /// @@ -647,6 +691,61 @@ struct WriteAclPolicy { policy: String, } +/// A cert-auth role's token lifetimes in seconds, `0` when unset (the mount's +/// default then applies). The role's certificate and names are not read. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)] +pub struct CertRoleTokenTtls { + pub token_ttl: u64, + pub token_max_ttl: u64, +} + +impl CertRoleTokenTtls { + /// Whether both lifetimes are [`CERT_ROLE_TOKEN_TTL`]. + #[must_use] + pub fn is_current(self) -> bool { + self.token_ttl == CERT_ROLE_TOKEN_TTL_SECS && self.token_max_ttl == CERT_ROLE_TOKEN_TTL_SECS + } +} + +/// Read a cert-auth role. Spelled out because vaultrs's response type requires +/// every field the store answers with, and only the two lifetimes are wanted. +#[derive(Endpoint)] +#[endpoint( + path = "auth/{self.mount}/certs/{self.name}", + method = "GET", + response = "CertRoleTokenTtls" +)] +struct ReadCertRoleTtls { + #[endpoint(skip)] + mount: String, + #[endpoint(skip)] + name: String, +} + +/// Write the two token lifetimes of a cert-auth role and nothing else. +/// vaultrs's `CreateCaCertificateRoleRequest` always sends `certificate`, which +/// would replace the role's. +#[derive(Endpoint)] +#[endpoint(path = "auth/{self.mount}/certs/{self.name}", method = "POST")] +struct SetCertRoleTtls { + #[endpoint(skip)] + mount: String, + #[endpoint(skip)] + name: String, + token_ttl: String, + token_max_ttl: String, +} + +/// The request [`SecretStore::set_cert_role_token_ttls`] sends. +fn set_cert_role_token_ttls_request(mount: &str, name: &str) -> SetCertRoleTtls { + SetCertRoleTtls { + mount: mount.to_owned(), + name: name.to_owned(), + token_ttl: CERT_ROLE_TOKEN_TTL.to_owned(), + token_max_ttl: CERT_ROLE_TOKEN_TTL.to_owned(), + } +} + /// The options [`SecretStore::create`] writes with: version 0, which only a /// path holding nothing is at. fn create_only() -> vaultrs::api::kv2::requests::SetSecretRequestOptions { @@ -749,6 +848,48 @@ mod tests { assert_eq!(r.certificate, "PEM"); } + #[test] + fn the_ttl_in_seconds_is_the_ttl_the_roles_are_written_with() { + assert_eq!(CERT_ROLE_TOKEN_TTL, "1h"); + assert_eq!(CERT_ROLE_TOKEN_TTL_SECS, 3600); + } + + #[test] + fn a_role_is_current_only_when_both_lifetimes_are_the_ttl() { + let ttls = |token_ttl, token_max_ttl| CertRoleTokenTtls { + token_ttl, + token_max_ttl, + }; + assert!(ttls(3600, 3600).is_current()); + assert!(!ttls(0, 0).is_current(), "unset takes the mount default"); + assert!(!ttls(3600, 0).is_current()); + assert!(!ttls(0, 3600).is_current()); + assert!(!ttls(900, 900).is_current()); + } + + /// The store keeps the fields a write leaves out, so the write names these + /// two and no others: a `certificate` here would replace the role's. + #[test] + fn the_ttl_write_names_the_two_lifetimes_and_nothing_else() { + use rustify::Endpoint as _; + let r = set_cert_role_token_ttls_request("cert", "hive-agent-a") + .request("http://store.invalid") + .expect("builds"); + assert_eq!(r.uri().path(), "/auth/cert/certs/hive-agent-a"); + let body: serde_json::Value = serde_json::from_slice(r.body()).expect("json body"); + assert_eq!( + body, + serde_json::json!({"token_ttl": "1h", "token_max_ttl": "1h"}) + ); + // The control: the full role write does carry the certificate, so a + // body without one is the narrow write and not a failure to serialise. + let full = cert_role_request("cert", "hive-agent-a", "PEM", "hive-agent-a", &["p"]) + .request("http://store.invalid") + .expect("builds"); + let full: serde_json::Value = serde_json::from_slice(full.body()).expect("json body"); + assert_eq!(full["certificate"], "PEM"); + } + #[test] fn a_complete_environment_is_accepted() { // The control: without this, every assertion below could be passing