From fe9417ae524429b5ee82ad15517f168d090378cb Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 31 Aug 2026 17:30:00 +0200 Subject: [PATCH 1/2] swarm: give agent containers their own queue principal Agents have authelia *users*; they had no machine identity at all, so an agent could not authenticate to the swarm queue as anything. This mints one `agent-` OIDC client per hive beside the existing `hive-` one, teaches the auth-callout responder an agent arm, and opens the queue's client port on the bridge so a container can reach it. One client per HIVE, not per agent: agents are created at runtime, and a per-agent client would make creating one a config change plus an authelia reload. The cost is that agents on a hive are indistinguishable to the broker, which is deliberate and tracked separately. The agent grant is deny-by-default twice over. An agent id matches no hive rule, so it gets a hive's status-key grant from neither; and with no agent subject configured the responder returns no grant at all rather than an empty publish list, which would be a denial wearing a grant's shape. What an agent may publish is a deployment's decision, taken through `--agent-publish-subject` the same way `--hive-publish-subject` already works. `Policy::new` now refuses two prefixes where one contains the other. The arms are tried in order, so that overlap does not error at match time - it silently hands one principal the other's grant. Not shipped here, and neither is reachable without it: no subject is configured for agents anywhere in nix, and nothing yet delivers `agent-.secret` into an agent container. Both belong to the stream that will be the first consumer. --- docs/networking/network.md | 7 +- nix/host-modules/swarm-authelia.nix | 66 +++++++++- nix/host-modules/swarm-nats.nix | 36 +++++- swarm-nats-auth/src/main.rs | 23 ++++ swarm-nats-auth/src/policy.rs | 182 +++++++++++++++++++++++++++- 5 files changed, 301 insertions(+), 13 deletions(-) diff --git a/docs/networking/network.md b/docs/networking/network.md index cfb72b2e..0fb39114 100644 --- a/docs/networking/network.md +++ b/docs/networking/network.md @@ -181,9 +181,10 @@ namespace. By default agents can only reach the host on 80/443 (+53 DNS), so a host-side service on another port — for example a dev OTLP collector you want -agents to reach directly — is unreachable. (hyperhive's own telemetry -needs none of this: `otel.enable` opens its collector's port itself, and -`otel.endpoint` is the _upstream_, which no agent ever dials. See +agents to reach directly — is unreachable. (hyperhive's own services +need none of this: a module that means to be reachable from agents opens +its own port here, which `otel.enable` and `nats.enable` both do. Note +that `otel.endpoint` is the _upstream_, which no agent ever dials. See `docs/scheduler/observability.md`.) `services.hyperhive.network.exposeHostPorts = [ 4318 ];` opens each diff --git a/nix/host-modules/swarm-authelia.nix b/nix/host-modules/swarm-authelia.nix index a2309257..dc4dc517 100644 --- a/nix/host-modules/swarm-authelia.nix +++ b/nix/host-modules/swarm-authelia.nix @@ -143,6 +143,33 @@ let accessTokenSignedResponseAlg = "RS256"; }) hyperhiveCfg.swarm.hives; + # The identity an agent container presents to the swarm's queue. One per + # HIVE, not per agent, and that is the load-bearing choice rather than a + # shortcut: agents are created at **runtime**, so anything minted per agent + # would make creating one a config change plus an authelia reload. Keyed on + # the hive, this list's length tracks the roster above it — deploy-time, like + # every other entry here. + # + # ⚠️ The cost, stated rather than discovered later: every agent on a hive + # presents the SAME client id, so the broker can tell *hives* apart and not + # *agents*. Deliberate and deferred — closing it requires the client list to + # stop being static, which is the same problem the users-database writer + # already solves for identities. + # + # Mirrors `hiveClients` field for field so the two stay comparable. The + # signing algorithm is not load-bearing here — this client's only consumer is + # the queue's auth-callout responder, which *introspects* rather than + # verifying offline — but it matches its sibling rather than inventing a + # second answer to a question nobody asked. + agentClients = lib.mapAttrsToList (name: _: { + id = "${cfg.agentClientPrefix}${name}"; + description = "HyperHive agents on hive ${name}"; + kind = "machine"; + redirectUris = [ ]; + audience = [ "${cfg.agentClientPrefix}${name}" ]; + accessTokenSignedResponseAlg = "RS256"; + }) hyperhiveCfg.swarm.hives; + # `swarm-authelia-bridge`'s own identity — distinct from # `swarm-controller`'s (`swarm-controller.nix`'s `queueClientId`). A # resource server introspecting a token proves its OWN identity to the @@ -416,9 +443,21 @@ in default = deployCfg.nats.enable; defaultText = lib.literalExpression "services.hyperhive.deploy.nats.enable"; description = '' - Mint one machine client per hive in + Mint machine clients per hive in {option}`services.hyperhive.swarm.hives`, so each hive can - authenticate to swarm services as itself. + authenticate to swarm services as itself: `hive-` for the + hive's own daemons, and `agent-` for the agent containers + running on it. + + Two clients rather than one because they are not the same + principal — a hive's daemons run on the host and an agent runs + in a container the host hands a credential to, so a swarm + service has to be able to grant them different things. It is + one client per *hive* on the agent side, not per agent: agents + are created at runtime, and a per-agent client would make + creating one a config change plus an authelia reload. The cost + is that agents on a hive are indistinguishable from each other, + tracked as a follow-up rather than papered over. Defaults to whether the swarm message queue is enabled, because that is the first service that needs a hive to prove who it is. @@ -667,6 +706,27 @@ in ''; }; + agentClientPrefix = lib.mkOption { + type = lib.types.str; + readOnly = true; + default = "agent-"; + description = '' + Prefix of the OAuth2 client id minted for the *agents* of each hive + in `services.hyperhive.swarm.hives` — agents on hive `alpha` all + present + `${config.services.hyperhive.swarm.authelia.agentClientPrefix}alpha`. + Read-only for the same reason as `hiveClientPrefix`, and read by the + same consumer with the same failure mode: a split spelling denies + every agent as a timeout. + + One id per hive rather than per agent, because agents are created at + runtime and a per-agent client would make creating one a config + change plus a reload. The consequence is that this identity says + *which hive* an agent belongs to and never *which agent* — the + broker cannot tell two agents on one hive apart. + ''; + }; + machine = lib.mkOption { type = lib.types.str; readOnly = true; @@ -828,7 +888,7 @@ in # wherever this module is, so `oidc.clients` is never actually empty # — see `oidcEnabled`'s comment above. services.hyperhive.swarm.authelia.oidc.clients = - lib.optionals cfg.oidc.hiveIdentities hiveClients + lib.optionals cfg.oidc.hiveIdentities (hiveClients ++ agentClients) ++ [ bridgeClient ]; # A redirect URI on a machine client is not harmless-but-unused: it diff --git a/nix/host-modules/swarm-nats.nix b/nix/host-modules/swarm-nats.nix index 84b9e930..90c7d0d0 100644 --- a/nix/host-modules/swarm-nats.nix +++ b/nix/host-modules/swarm-nats.nix @@ -233,6 +233,17 @@ in TCP port the queue listens on. 4222 is upstream's default and sits outside hyperhive's claimed ranges (dashboard 7000, forge 3000, matrix 8008, every agent in 8100..8999 via FNV-1a hash). + + Contributed to + `services.hyperhive.network.exposeHostPorts`, which opens it on + the bridge interface only — so it is reachable from agent + containers and not from the outside world. An agent connects to + `nats://:`; loopback inside a container is the + agent itself, not this host. + + Unlike {option}`monitorPort` and {option}`metricsPort`, the + address is not what bounds who may use this port: the queue's + `auth_callout` refuses every client it cannot identify. ''; }; @@ -564,6 +575,26 @@ in nats = "127.0.0.1:${toString cfg.metricsPort}"; }; + # Open the client port on the bridge, so agent containers can reach + # the queue. `privateNetwork = false` below means the server binds in + # the host netns, which is the precondition this option names — but + # sharing a netns is not reachability: the bridge interface is + # default-deny, so without this an agent's connect attempt is dropped + # by the firewall and looks exactly like every other NATS failure, + # a timeout. + # + # ⚠️ Bridge interface only — never the world. What makes it safe to + # open at all is that the queue is fail-closed: `auth_callout` admits + # nobody until the responder above answers for them, so an agent that + # reaches this port still has to present a token authelia vouches for. + # + # An agent connects to `nats://:`, NOT to + # `nats://127.0.0.1:` — inside a container loopback is the + # *agent*. Every url in this repo today is the loopback one and each + # is correct for its reader, because those readers share the host + # netns; an agent does not. + services.hyperhive.network.exposeHostPorts = [ cfg.port ]; + containers.swarm-nats = { autoStart = true; ephemeral = false; @@ -730,14 +761,15 @@ in # be the same string — which is why both come from one let. "--account ${lib.escapeShellArg clientAccount}" "--introspection-url ${lib.escapeShellArg introspectionUrl}" - # Both of these name a principal some OTHER module mints, - # so both are read out of that module rather than spelled + # Each of these names a principal some OTHER module mints, + # so each is read out of that module rather than spelled # again here — same argument as `--account` above, one # level wider. The responder denies a client id it does # not recognise, and a NATS denial arrives as a timeout, # so a drift here is silent at the point of change and # misattributed at the point of failure. "--hive-client-prefix ${lib.escapeShellArg autheliaCfg.hiveClientPrefix}" + "--agent-client-prefix ${lib.escapeShellArg autheliaCfg.agentClientPrefix}" "--reader-client ${lib.escapeShellArg controllerCfg.queueClientId}" ]; # Every credential arrives by `LoadCredential` and is named diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index a849419c..52969100 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -82,6 +82,16 @@ struct Args { #[arg(long, default_value = "hive-")] hive_client_prefix: String, + /// Client-id prefix that marks an agent container. `swarm-authelia.nix` + /// mints one machine client per roster entry as `agent-` — per + /// **hive**, not per agent, because agents are created at runtime and a + /// per-agent client would make creating one a config change plus a reload. + /// + /// So this identity says which hive an agent belongs to and never which + /// agent: two agents on one hive are indistinguishable to this responder. + #[arg(long, default_value = "agent-")] + agent_client_prefix: String, + /// Client ids allowed to read every hive's status. Repeatable. The /// default is the swarm controller, which is the only reader that exists. #[arg(long = "reader-client", default_values_t = [String::from("swarm-controller")])] @@ -96,6 +106,17 @@ struct Args { /// subject still lands inside that hive's own namespace. #[arg(long = "hive-publish-subject")] hive_publish_subjects: Vec, + + /// Subjects an agent may publish to, with `{hive}` standing for the hive + /// its identity names. Repeatable, empty by default. + /// + /// Empty means agents get **no grant at all** rather than a grant that can + /// do nothing — the identity exists, and what it may say is a deployment's + /// decision, not this responder's. The first consumer is the agent + /// terminal-event stream; until one is configured, an agent that connects + /// is refused, which is loud rather than silently over-broad. + #[arg(long = "agent-publish-subject")] + agent_publish_subjects: Vec, } /// Read a secret file and strip surrounding whitespace. @@ -140,9 +161,11 @@ async fn main() -> anyhow::Result<()> { // silently over-broad grant is not. let policy = policy::Policy::new( args.hive_client_prefix.clone(), + args.agent_client_prefix.clone(), swarm_queue_client::status::BUCKET.to_owned(), args.reader_clients.clone(), args.hive_publish_subjects.clone(), + args.agent_publish_subjects.clone(), )?; let http = reqwest::Client::new(); let issuer = nkeys::KeyPair::from_seed(&read_secret(&args.issuer_seed_file)?) diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 6f11113f..0433b444 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -40,19 +40,24 @@ pub struct Permissions { #[derive(Debug, Clone)] pub struct Policy { hive_prefix: String, + agent_prefix: String, bucket: String, readers: Vec, extra_hive_subjects: Vec, + extra_agent_subjects: Vec, } -/// Placeholder replaced with the hive's own name in `extra_hive_subjects`. +/// Placeholder replaced with the hive's own name in `extra_hive_subjects` and +/// `extra_agent_subjects`. const HIVE_PLACEHOLDER: &str = "{hive}"; impl Policy { - /// `hive_prefix` is the client-id prefix that marks a hive, `bucket` the - /// KV bucket hives report status in, `readers` the client ids allowed to - /// read every hive's key, and `extra_hive_subjects` additional subjects a - /// hive may publish to (with `{hive}` standing for its own name). + /// `hive_prefix` is the client-id prefix that marks a hive and + /// `agent_prefix` the one that marks a hive's agent containers; `bucket` is + /// the KV bucket hives report status in, `readers` the client ids allowed + /// to read every hive's key, and `extra_hive_subjects` / + /// `extra_agent_subjects` additional subjects each may publish to (with + /// `{hive}` standing for the hive's own name). /// /// # Errors /// @@ -62,11 +67,18 @@ impl Policy { /// whose only purpose is to provide one. Returning a `Result` rather than /// checking at the call site is deliberate: it makes an unscoped policy /// unconstructible instead of merely unlikely. + /// + /// Two prefixes where one of them is a prefix of the other is refused for + /// the same reason: the arms are tried in order, so the overlap does not + /// error at match time — it silently hands one principal the other's + /// grant. pub fn new( hive_prefix: String, + agent_prefix: String, bucket: String, readers: Vec, extra_hive_subjects: Vec, + extra_agent_subjects: Vec, ) -> anyhow::Result { if let Some(bad) = extra_hive_subjects .iter() @@ -78,11 +90,40 @@ impl Policy { a per-hive namespace" ); } + // Same rule, same reason: an agent's identity is its *hive's*, so a + // template with no placeholder is one subject shared by every agent in + // the swarm rather than a per-hive one. + if let Some(bad) = extra_agent_subjects + .iter() + .find(|s| !s.contains(HIVE_PLACEHOLDER)) + { + anyhow::bail!( + "--agent-publish-subject {bad:?} contains no {HIVE_PLACEHOLDER}: every agent \ + in the swarm would be granted that exact subject, so it is a shared channel \ + rather than a per-hive namespace" + ); + } + // Checked here rather than trusted from nix, because the two prefixes + // are two flags and nothing downstream compares them. `permissions` + // tries the hive arm first, so an agent prefix of `""` — or either one + // being a prefix of the other — would route agents into the hive grant + // with no error anywhere: a client that authenticates fine and is + // granted more than it should have, which is the one failure this + // module must not have. + if hive_prefix.starts_with(&agent_prefix) || agent_prefix.starts_with(&hive_prefix) { + anyhow::bail!( + "--hive-client-prefix {hive_prefix:?} and --agent-client-prefix \ + {agent_prefix:?} overlap: one is a prefix of the other, so a client id \ + matching the longer one is granted by whichever rule is tried first" + ); + } Ok(Self { hive_prefix, + agent_prefix, bucket, readers, extra_hive_subjects, + extra_agent_subjects, }) } @@ -97,6 +138,15 @@ impl Policy { publish: self.hive_subjects(hive), }); } + // An agent's identity names its hive, never the agent — see + // `Self::agent_hive`. The grant is whatever the deployment configured + // for agents, expanded for that hive, and **nothing when it configured + // none**: an empty publish list would be a denial wearing a grant's + // shape, so this returns `None` and the client is refused outright. + if let Some(hive) = self.agent_hive(client_id) { + let publish = self.agent_subjects(hive); + return (!publish.is_empty()).then_some(Permissions { publish }); + } if self.readers.iter().any(|r| r == client_id) { return Some(Permissions { publish: self.reader_subjects(), @@ -134,6 +184,19 @@ impl Policy { .filter(|name| !name.is_empty()) } + /// The hive whose agents a client id names, when it names one. + /// + /// Deliberately the same shape as [`Self::hive_name`] one prefix over, and + /// every caveat above applies unchanged. One more on top: the agent client + /// is minted per **hive**, so this answers *whose agents*, never *which + /// agent*. Two agents on one hive are indistinguishable here — not a gap in + /// the parsing but the identity itself, which does not carry the agent. + fn agent_hive<'a>(&self, client_id: &'a str) -> Option<&'a str> { + client_id + .strip_prefix(&self.agent_prefix) + .filter(|name| !name.is_empty()) + } + fn stream(&self) -> String { format!("KV_{}", self.bucket) } @@ -314,6 +377,19 @@ impl Policy { subjects } + /// Subjects an agent of `hive` may publish to. + /// + /// No `JetStream` minimum, unlike [`Self::hive_subjects`]: a hive needs it + /// because it writes the status bucket, and an agent publishes to plain + /// subjects. Granting it anyway would be reasoning by analogy about a + /// permission set the module documents as minimal by removal. + fn agent_subjects(&self, hive: &str) -> Vec { + self.extra_agent_subjects + .iter() + .map(|s| s.replace(HIVE_PLACEHOLDER, hive)) + .collect() + } + fn reader_subjects(&self) -> Vec { let stream = self.stream(); let mut subjects = Self::jetstream_minimum().to_vec(); @@ -419,13 +495,30 @@ mod tests { fn policy() -> Policy { Policy::new( "hive-".to_owned(), + "agent-".to_owned(), "hive-status".to_owned(), vec!["swarm-controller".to_owned()], Vec::new(), + Vec::new(), ) .expect("the default policy is valid") } + /// The same policy with agents given something to publish, since the + /// default deployment configures no agent subject and an agent is then + /// refused outright. + fn policy_with_agent_subject() -> Policy { + Policy::new( + "hive-".to_owned(), + "agent-".to_owned(), + "hive-status".to_owned(), + vec!["swarm-controller".to_owned()], + Vec::new(), + vec!["$SWARM.term.{hive}.>".to_owned()], + ) + .expect("a per-hive agent template is valid") + } + #[test] fn the_reader_can_open_and_list_the_agent_status_bucket() { let subjects = policy().reader_subjects(); @@ -520,9 +613,11 @@ mod tests { // way to remove it - silently, and only in the deployment that set it. let err = Policy::new( "hive-".to_owned(), + "agent-".to_owned(), "hive-status".to_owned(), Vec::new(), vec!["$SWARM.events.all".to_owned()], + Vec::new(), ) .expect_err("a subject shared by every hive must not be accepted"); let msg = format!("{err}"); @@ -538,9 +633,11 @@ mod tests { // from the option being broken. Policy::new( "hive-".to_owned(), + "agent-".to_owned(), "hive-status".to_owned(), Vec::new(), vec!["$SWARM.events.{hive}.>".to_owned()], + Vec::new(), ) .expect("a per-hive template is the shape this option is for"); } @@ -944,13 +1041,88 @@ mod tests { // another's events even though its status key is scoped. let p = Policy::new( "hive-".to_owned(), + "agent-".to_owned(), "hive-status".to_owned(), Vec::new(), vec!["$SWARM.events.{hive}.>".to_owned()], + Vec::new(), ) .expect("a per-hive template is valid"); let g = p.permissions("hive-alpha").expect("a hive is admitted"); assert!(g.publish.contains(&"$SWARM.events.alpha.>".to_owned())); assert!(!g.publish.iter().any(|s| s.contains("{hive}"))); } + + #[test] + fn an_agent_with_no_configured_subject_is_refused() { + // The shipped default: the identity exists, what it may say does not. + // A grant with an empty publish list would let it connect and then + // fail on every publish, which reads as a broken queue rather than as + // a deployment that configured nothing. + assert!(policy().permissions("agent-alpha").is_none()); + } + + #[test] + fn an_agent_publishes_inside_its_own_hives_namespace() { + // The presence control for the test above - without it, "refused" + // would also be the answer if the agent arm never matched at all. + let g = policy_with_agent_subject() + .permissions("agent-alpha") + .expect("an agent with a configured subject is admitted"); + assert_eq!(g.publish, vec!["$SWARM.term.alpha.>".to_owned()]); + } + + #[test] + fn an_agent_does_not_get_its_hives_grant() { + // The whole point of a second prefix. An agent holds a credential that + // sits in a container; a hive's grant includes writing that hive's + // status key, which an agent must not be able to forge. + let g = policy_with_agent_subject() + .permissions("agent-alpha") + .expect("an agent is admitted"); + assert!(!g.publish.iter().any(|s| s.starts_with("$KV."))); + assert!(!g.publish.iter().any(|s| s.starts_with("$JS.API."))); + } + + #[test] + fn an_agent_subject_without_the_placeholder_is_refused() { + let err = Policy::new( + "hive-".to_owned(), + "agent-".to_owned(), + "hive-status".to_owned(), + Vec::new(), + Vec::new(), + vec!["$SWARM.term.all".to_owned()], + ) + .expect_err("a subject shared by every agent in the swarm is not per-hive"); + assert!( + format!("{err}").contains("$SWARM.term.all"), + "the error must name the offending template" + ); + } + + #[test] + fn overlapping_prefixes_are_refused() { + // 🩸 The arms are tried in order, so an overlap does not error at match + // time - it hands one principal the other's grant. Both directions, + // because which one wins depends only on the order above. + for (hive, agent) in [("hive-", "hive-agent-"), ("agent-x-", "agent-"), ("h", "h")] { + Policy::new( + hive.to_owned(), + agent.to_owned(), + "hive-status".to_owned(), + Vec::new(), + Vec::new(), + Vec::new(), + ) + .expect_err("prefixes where one contains the other must not construct"); + } + } + + #[test] + fn the_agent_prefix_alone_names_no_hive() { + // Same trap as `the_prefix_alone_names_no_hive`: `agent-` with nothing + // after it would expand `{hive}` to the empty string. + assert!(policy_with_agent_subject().permissions("agent-").is_none()); + } } From 780df10d9d34247f3e8f72fd52ce039ddc932dee Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 31 Aug 2026 18:10:18 +0200 Subject: [PATCH 2/2] swarm: name the agent client after its hive, not after "agent" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent-` reads as "the agent named " — which is the one thing that identity does not carry, since it is minted per hive. It becomes `hive--agent`: the hive's own id, extended. The rename is not a string swap. `hive-foo-agent` satisfies the hive parse too (it strips to a hive named `foo-agent`), so the responder's agent rule now runs BEFORE its hive rule — most specific wins. Hive-first would have handed every agent its hive's grant, including writing that hive's status key, with nothing to report it: the client authenticates and is merely able to do more than it should. `Policy::new`'s overlap check goes with the prefix it was written for. The invariant the suffix form needs instead is that the suffix is non-empty: an empty one makes `strip_suffix` succeed on every hive id, so the two principals become one string and whichever arm runs first answers for both. The suffix form also introduces a collision the prefix form did not have: a hive genuinely named `foo-agent` mints `hive-foo-agent`, which is hive `foo`'s agent id. The responder cannot see it — it has no roster, deliberately — so `swarm-authelia.nix` asserts at eval that no hive name ends with the suffix. The existing duplicate-id assertion does not cover this: it fires only when both `foo` and `foo-agent` are on the roster, and with `foo-agent` alone there is no duplicate, just a hive quietly receiving its agents' grant. A test written by analogy with `the_prefix_alone_names_no_hive` failed, correctly — `hive--agent` is a hive named `-agent` under the hive parse, which this module cannot rule out. It now asserts only the part this module owns: no empty hive name is ever expanded into a subject. --- nix/host-modules/swarm-authelia.nix | 60 ++++++++-- nix/host-modules/swarm-nats.nix | 2 +- swarm-nats-auth/src/main.rs | 22 ++-- swarm-nats-auth/src/policy.rs | 180 ++++++++++++++++++---------- 4 files changed, 181 insertions(+), 83 deletions(-) diff --git a/nix/host-modules/swarm-authelia.nix b/nix/host-modules/swarm-authelia.nix index dc4dc517..38aac164 100644 --- a/nix/host-modules/swarm-authelia.nix +++ b/nix/host-modules/swarm-authelia.nix @@ -156,17 +156,24 @@ let # stop being static, which is the same problem the users-database writer # already solves for identities. # + # 🏷️ The id EXTENDS the hive's own (`hive--agent`) rather than taking a + # prefix of its own (`agent-`), because that reads as *the agent called + # ``* — which is the one thing this identity does not carry. The cost is + # that the two ids are no longer distinguishable by prefix, so the responder's + # agent rule must be tried BEFORE its hive rule; `policy.rs` says so at the + # match site, and the assertion below covers the case that ordering cannot. + # # Mirrors `hiveClients` field for field so the two stay comparable. The # signing algorithm is not load-bearing here — this client's only consumer is # the queue's auth-callout responder, which *introspects* rather than # verifying offline — but it matches its sibling rather than inventing a # second answer to a question nobody asked. agentClients = lib.mapAttrsToList (name: _: { - id = "${cfg.agentClientPrefix}${name}"; + id = "${cfg.hiveClientPrefix}${name}${cfg.agentClientSuffix}"; description = "HyperHive agents on hive ${name}"; kind = "machine"; redirectUris = [ ]; - audience = [ "${cfg.agentClientPrefix}${name}" ]; + audience = [ "${cfg.hiveClientPrefix}${name}${cfg.agentClientSuffix}" ]; accessTokenSignedResponseAlg = "RS256"; }) hyperhiveCfg.swarm.hives; @@ -446,7 +453,7 @@ in Mint machine clients per hive in {option}`services.hyperhive.swarm.hives`, so each hive can authenticate to swarm services as itself: `hive-` for the - hive's own daemons, and `agent-` for the agent containers + hive's own daemons, and `hive--agent` for the agent containers running on it. Two clients rather than one because they are not the same @@ -706,19 +713,22 @@ in ''; }; - agentClientPrefix = lib.mkOption { + agentClientSuffix = lib.mkOption { type = lib.types.str; readOnly = true; - default = "agent-"; + default = "-agent"; description = '' - Prefix of the OAuth2 client id minted for the *agents* of each hive - in `services.hyperhive.swarm.hives` — agents on hive `alpha` all - present - `${config.services.hyperhive.swarm.authelia.agentClientPrefix}alpha`. + Suffix appended to a hive's own client id to name the client its + *agent containers* present — agents on hive `alpha` all present + `${config.services.hyperhive.swarm.authelia.hiveClientPrefix}alpha${config.services.hyperhive.swarm.authelia.agentClientSuffix}`. Read-only for the same reason as `hiveClientPrefix`, and read by the same consumer with the same failure mode: a split spelling denies every agent as a timeout. + A suffix on the hive's id rather than a prefix of its own, because + `agent-alpha` reads as *the agent named alpha* — which is precisely + what this identity does not say. + One id per hive rather than per agent, because agents are created at runtime and a per-agent client would make creating one a config change plus a reload. The consequence is that this identity says @@ -937,6 +947,38 @@ in + "services.hyperhive.swarm.hives; rename the hive or the " + "colliding client."; } + { + # A hive whose name ENDS with the agent suffix mints an id the + # queue's responder reads as somebody else's agents: + # `hive-foo-agent` parses as *the agents of hive foo* before it + # parses as *the hive foo-agent*, because the agent rule is the + # more specific one and therefore runs first. + # + # ⚠️ NOT covered by the duplicate-id assertion above, and the gap is + # the interesting half. That one fires only when BOTH `foo` and + # `foo-agent` are on the roster, because only then are two clients + # actually named the same string. With `foo-agent` alone there is no + # duplicate and nothing to see — the hive simply receives its + # agents' grant instead of its own, and a NATS denial arrives as a + # timeout, so the symptom names nothing. + # + # Checkable here and nowhere downstream: the responder runs in a + # container with no view of the roster (`policy.rs`'s `hive_name` + # says why that is deliberate), so this is the last place that knows + # both the naming scheme and the set of names. + assertion = + !cfg.oidc.hiveIdentities + || !lib.any (h: lib.hasSuffix cfg.agentClientSuffix h) (lib.attrNames hyperhiveCfg.swarm.hives); + message = + "services.hyperhive.swarm.hives contains " + + lib.concatMapStringsSep ", " (h: "'${h}'") ( + lib.filter (h: lib.hasSuffix cfg.agentClientSuffix h) (lib.attrNames hyperhiveCfg.swarm.hives) + ) + + " — names ending with '${cfg.agentClientSuffix}', the suffix that " + + "marks a hive's agent containers. Such a hive's own client id is " + + "indistinguishable from another hive's agent client, and the " + + "queue resolves it as the agents. Rename the hive."; + } # The two obligations `authelia.bearer.authz` carries. Authelia # enforces both itself — but in its `preStart` validator, so a # violation produces a green `nixos-rebuild switch` and an authelia diff --git a/nix/host-modules/swarm-nats.nix b/nix/host-modules/swarm-nats.nix index 90c7d0d0..4aef2a49 100644 --- a/nix/host-modules/swarm-nats.nix +++ b/nix/host-modules/swarm-nats.nix @@ -769,7 +769,7 @@ in # so a drift here is silent at the point of change and # misattributed at the point of failure. "--hive-client-prefix ${lib.escapeShellArg autheliaCfg.hiveClientPrefix}" - "--agent-client-prefix ${lib.escapeShellArg autheliaCfg.agentClientPrefix}" + "--agent-client-suffix ${lib.escapeShellArg autheliaCfg.agentClientSuffix}" "--reader-client ${lib.escapeShellArg controllerCfg.queueClientId}" ]; # Every credential arrives by `LoadCredential` and is named diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index 52969100..f98c896f 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -82,15 +82,20 @@ struct Args { #[arg(long, default_value = "hive-")] hive_client_prefix: String, - /// Client-id prefix that marks an agent container. `swarm-authelia.nix` - /// mints one machine client per roster entry as `agent-` — per - /// **hive**, not per agent, because agents are created at runtime and a - /// per-agent client would make creating one a config change plus a reload. + /// Suffix that marks an agent container, appended to the hive's own client + /// id: `swarm-authelia.nix` mints one machine client per roster entry as + /// `hive--agent` — per **hive**, not per agent, because agents are + /// created at runtime and a per-agent client would make creating one a + /// config change plus a reload. /// /// So this identity says which hive an agent belongs to and never which /// agent: two agents on one hive are indistinguishable to this responder. - #[arg(long, default_value = "agent-")] - agent_client_prefix: String, + /// + /// A suffix on the hive's id rather than a prefix of its own, because + /// `agent-` reads as *the agent called ``* — the one thing + /// this identity does not carry. + #[arg(long, default_value = "-agent")] + agent_client_suffix: String, /// Client ids allowed to read every hive's status. Repeatable. The /// default is the swarm controller, which is the only reader that exists. @@ -108,7 +113,8 @@ struct Args { hive_publish_subjects: Vec, /// Subjects an agent may publish to, with `{hive}` standing for the hive - /// its identity names. Repeatable, empty by default. + /// its identity names — `hive-alpha-agent` expands it to `alpha`. + /// Repeatable, empty by default. /// /// Empty means agents get **no grant at all** rather than a grant that can /// do nothing — the identity exists, and what it may say is a deployment's @@ -161,7 +167,7 @@ async fn main() -> anyhow::Result<()> { // silently over-broad grant is not. let policy = policy::Policy::new( args.hive_client_prefix.clone(), - args.agent_client_prefix.clone(), + args.agent_client_suffix.clone(), swarm_queue_client::status::BUCKET.to_owned(), args.reader_clients.clone(), args.hive_publish_subjects.clone(), diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 0433b444..0edd6536 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -40,7 +40,7 @@ pub struct Permissions { #[derive(Debug, Clone)] pub struct Policy { hive_prefix: String, - agent_prefix: String, + agent_suffix: String, bucket: String, readers: Vec, extra_hive_subjects: Vec, @@ -53,11 +53,16 @@ const HIVE_PLACEHOLDER: &str = "{hive}"; impl Policy { /// `hive_prefix` is the client-id prefix that marks a hive and - /// `agent_prefix` the one that marks a hive's agent containers; `bucket` is - /// the KV bucket hives report status in, `readers` the client ids allowed - /// to read every hive's key, and `extra_hive_subjects` / - /// `extra_agent_subjects` additional subjects each may publish to (with - /// `{hive}` standing for the hive's own name). + /// `agent_suffix` what a hive's agent containers carry **on top of** it — + /// `hive-` and `hive--agent`. `bucket` is the KV bucket hives + /// report status in, `readers` the client ids allowed to read every hive's + /// key, and `extra_hive_subjects` / `extra_agent_subjects` additional + /// subjects each may publish to (with `{hive}` standing for the hive's own + /// name). + /// + /// The agent id is deliberately the hive's id extended, not a second + /// top-level prefix: `agent-` reads as *the agent called ``*, + /// which is the one thing this identity does not carry. /// /// # Errors /// @@ -68,13 +73,13 @@ impl Policy { /// checking at the call site is deliberate: it makes an unscoped policy /// unconstructible instead of merely unlikely. /// - /// Two prefixes where one of them is a prefix of the other is refused for - /// the same reason: the arms are tried in order, so the overlap does not - /// error at match time — it silently hands one principal the other's - /// grant. + /// An empty `agent_suffix` is refused for the same reason: every hive id + /// would also parse as its own agent id, and the arms are tried in order, + /// so the overlap does not error at match time — it silently hands one + /// principal the other's grant. pub fn new( hive_prefix: String, - agent_prefix: String, + agent_suffix: String, bucket: String, readers: Vec, extra_hive_subjects: Vec, @@ -103,23 +108,24 @@ impl Policy { rather than a per-hive namespace" ); } - // Checked here rather than trusted from nix, because the two prefixes - // are two flags and nothing downstream compares them. `permissions` - // tries the hive arm first, so an agent prefix of `""` — or either one - // being a prefix of the other — would route agents into the hive grant - // with no error anywhere: a client that authenticates fine and is - // granted more than it should have, which is the one failure this - // module must not have. - if hive_prefix.starts_with(&agent_prefix) || agent_prefix.starts_with(&hive_prefix) { + // Checked here rather than trusted from nix, because these are two + // flags and nothing downstream compares them. An agent id is a hive id + // plus this suffix, so an empty one makes `strip_suffix` succeed on + // every hive id and the two principals become the same string. What + // that costs is not a parse error but a **grant**: whichever arm runs + // first answers for both, and a client that authenticates fine is + // handed more than it should have — the one failure this module must + // not have. + if agent_suffix.is_empty() { anyhow::bail!( - "--hive-client-prefix {hive_prefix:?} and --agent-client-prefix \ - {agent_prefix:?} overlap: one is a prefix of the other, so a client id \ - matching the longer one is granted by whichever rule is tried first" + "--agent-client-suffix is empty: an agent id is a hive id plus this suffix, \ + so every hive would also parse as its own agents and one of the two would \ + silently receive the other's grant" ); } Ok(Self { hive_prefix, - agent_prefix, + agent_suffix, bucket, readers, extra_hive_subjects, @@ -133,11 +139,15 @@ impl Policy { /// a connected client with no permissions still holds a slot and still /// looks admitted in the logs, which is a worse answer than a refusal. pub fn permissions(&self, client_id: &str) -> Option { - if let Some(hive) = self.hive_name(client_id) { - return Some(Permissions { - publish: self.hive_subjects(hive), - }); - } + // ⚠️ THE AGENT ARM RUNS FIRST, AND THE ORDER IS LOAD-BEARING. An agent + // id is a hive id with a suffix, so `hive-foo-agent` satisfies the hive + // arm too — it strips to the hive `foo-agent`. Hive-first would + // therefore hand every agent its hive's grant, including writing that + // hive's status key, and nothing would report it: the client + // authenticates, connects, and is simply able to do more than it + // should. Most specific wins, so the more specific test is the one + // that has to be asked first. + // // An agent's identity names its hive, never the agent — see // `Self::agent_hive`. The grant is whatever the deployment configured // for agents, expanded for that hive, and **nothing when it configured @@ -147,6 +157,11 @@ impl Policy { let publish = self.agent_subjects(hive); return (!publish.is_empty()).then_some(Permissions { publish }); } + if let Some(hive) = self.hive_name(client_id) { + return Some(Permissions { + publish: self.hive_subjects(hive), + }); + } if self.readers.iter().any(|r| r == client_id) { return Some(Permissions { publish: self.reader_subjects(), @@ -186,14 +201,24 @@ impl Policy { /// The hive whose agents a client id names, when it names one. /// - /// Deliberately the same shape as [`Self::hive_name`] one prefix over, and - /// every caveat above applies unchanged. One more on top: the agent client - /// is minted per **hive**, so this answers *whose agents*, never *which - /// agent*. Two agents on one hive are indistinguishable here — not a gap in - /// the parsing but the identity itself, which does not carry the agent. + /// [`Self::hive_name`]'s caveats all apply — this is that parse with the + /// suffix stripped as well, so `hive-foo-agent` answers `foo`. Two more on + /// top: + /// + /// - The agent client is minted per **hive**, so this answers *whose + /// agents*, never *which agent*. Two agents on one hive are + /// indistinguishable here — not a gap in the parsing but the identity + /// itself, which does not carry the agent. + /// - A hive genuinely named `foo-agent` mints `hive-foo-agent` and is + /// therefore **indistinguishable from hive `foo`'s agents** right here. + /// Unguardable from inside this responder, which has no roster (see + /// [`Self::hive_name`]) — `swarm-authelia.nix` asserts it at eval, where + /// the roster is known, so the deployment fails to build rather than + /// minting two principals with one id. fn agent_hive<'a>(&self, client_id: &'a str) -> Option<&'a str> { client_id - .strip_prefix(&self.agent_prefix) + .strip_prefix(&self.hive_prefix)? + .strip_suffix(&self.agent_suffix) .filter(|name| !name.is_empty()) } @@ -495,7 +520,7 @@ mod tests { fn policy() -> Policy { Policy::new( "hive-".to_owned(), - "agent-".to_owned(), + "-agent".to_owned(), "hive-status".to_owned(), vec!["swarm-controller".to_owned()], Vec::new(), @@ -510,7 +535,7 @@ mod tests { fn policy_with_agent_subject() -> Policy { Policy::new( "hive-".to_owned(), - "agent-".to_owned(), + "-agent".to_owned(), "hive-status".to_owned(), vec!["swarm-controller".to_owned()], Vec::new(), @@ -613,7 +638,7 @@ mod tests { // way to remove it - silently, and only in the deployment that set it. let err = Policy::new( "hive-".to_owned(), - "agent-".to_owned(), + "-agent".to_owned(), "hive-status".to_owned(), Vec::new(), vec!["$SWARM.events.all".to_owned()], @@ -633,7 +658,7 @@ mod tests { // from the option being broken. Policy::new( "hive-".to_owned(), - "agent-".to_owned(), + "-agent".to_owned(), "hive-status".to_owned(), Vec::new(), vec!["$SWARM.events.{hive}.>".to_owned()], @@ -1041,7 +1066,7 @@ mod tests { // another's events even though its status key is scoped. let p = Policy::new( "hive-".to_owned(), - "agent-".to_owned(), + "-agent".to_owned(), "hive-status".to_owned(), Vec::new(), vec!["$SWARM.events.{hive}.>".to_owned()], @@ -1059,7 +1084,7 @@ mod tests { // A grant with an empty publish list would let it connect and then // fail on every publish, which reads as a broken queue rather than as // a deployment that configured nothing. - assert!(policy().permissions("agent-alpha").is_none()); + assert!(policy().permissions("hive-alpha-agent").is_none()); } #[test] @@ -1067,18 +1092,28 @@ mod tests { // The presence control for the test above - without it, "refused" // would also be the answer if the agent arm never matched at all. let g = policy_with_agent_subject() - .permissions("agent-alpha") + .permissions("hive-alpha-agent") .expect("an agent with a configured subject is admitted"); assert_eq!(g.publish, vec!["$SWARM.term.alpha.>".to_owned()]); } #[test] fn an_agent_does_not_get_its_hives_grant() { - // The whole point of a second prefix. An agent holds a credential that - // sits in a container; a hive's grant includes writing that hive's - // status key, which an agent must not be able to forge. + // An agent holds a credential that sits in a container; a hive's grant + // includes writing that hive's status key, which an agent must not be + // able to forge. + // + // 🩸 This became a REGRESSION test when the id moved from `agent-` + // to `hive--agent`: the agent id now satisfies the HIVE parse too + // (`hive-alpha-agent` strips to a hive named `alpha-agent`), so a + // hive-first arm order hands every agent its hive's grant, and nothing + // reports it — the client authenticates and is merely able to do more + // than it should. Together with the `assert_eq` in + // `an_agent_publishes_inside_its_own_hives_namespace` (which pins the + // subject to `alpha`, not `alpha-agent`) this is what holds the + // ordering in place. let g = policy_with_agent_subject() - .permissions("agent-alpha") + .permissions("hive-alpha-agent") .expect("an agent is admitted"); assert!(!g.publish.iter().any(|s| s.starts_with("$KV."))); assert!(!g.publish.iter().any(|s| s.starts_with("$JS.API."))); @@ -1088,7 +1123,7 @@ mod tests { fn an_agent_subject_without_the_placeholder_is_refused() { let err = Policy::new( "hive-".to_owned(), - "agent-".to_owned(), + "-agent".to_owned(), "hive-status".to_owned(), Vec::new(), Vec::new(), @@ -1102,27 +1137,42 @@ mod tests { } #[test] - fn overlapping_prefixes_are_refused() { - // 🩸 The arms are tried in order, so an overlap does not error at match - // time - it hands one principal the other's grant. Both directions, - // because which one wins depends only on the order above. - for (hive, agent) in [("hive-", "hive-agent-"), ("agent-x-", "agent-"), ("h", "h")] { - Policy::new( - hive.to_owned(), - agent.to_owned(), - "hive-status".to_owned(), - Vec::new(), - Vec::new(), - Vec::new(), - ) - .expect_err("prefixes where one contains the other must not construct"); - } + fn an_empty_agent_suffix_is_refused() { + // 🩸 An agent id is a hive id plus the suffix, so an empty suffix makes + // `strip_suffix` succeed on every hive id: the two principals become + // one string, and whichever arm runs first answers for both. That is a + // GRANT, not a parse error, which is why it has to fail at + // construction. + Policy::new( + "hive-".to_owned(), + String::new(), + "hive-status".to_owned(), + Vec::new(), + Vec::new(), + Vec::new(), + ) + .expect_err("an empty agent suffix must not construct"); } #[test] - fn the_agent_prefix_alone_names_no_hive() { - // Same trap as `the_prefix_alone_names_no_hive`: `agent-` with nothing - // after it would expand `{hive}` to the empty string. - assert!(policy_with_agent_subject().permissions("agent-").is_none()); + fn the_agent_suffix_alone_names_no_hive() { + // `hive--agent` is the prefix and the suffix with nothing between them. + // The agent arm must refuse it rather than expand `{hive}` to the empty + // string and hand out `$SWARM.term..>`. + // + // 🩸 Written first as `is_none()`, by analogy with + // `the_prefix_alone_names_no_hive`, and it FAILED — correctly. The + // analogy does not hold: under the hive parse the same id is a hive + // named `-agent`, which is a non-empty name this responder cannot rule + // out, because it has no roster. `swarm-authelia.nix` refuses such a + // name at eval, where the roster is known. So the assertion here is + // narrowed to the only part this module owns. + let g = policy_with_agent_subject().permissions("hive--agent"); + assert!( + !g.iter() + .flat_map(|p| &p.publish) + .any(|s| s.contains("$SWARM.term..")), + "an empty hive name must never be expanded into a subject: {g:?}" + ); } }