diff --git a/nix/host-modules/deploy.nix b/nix/host-modules/deploy.nix index f88126e4..1ad52b7b 100644 --- a/nix/host-modules/deploy.nix +++ b/nix/host-modules/deploy.nix @@ -95,6 +95,10 @@ in swarm-authelia-bridge listens on a unix socket the gateway proxies to, so it binds no port. Remove this definition. '') + (lib.mkRemovedOptionModule [ "services" "hyperhive" "swarm" "authelia" "agentClientSuffix" ] '' + No client is minted per agent and the queue refuses no client id by + suffix, so there is no suffix to name. Remove this definition. + '') (lib.mkRemovedOptionModule [ "services" "hyperhive" diff --git a/nix/host-modules/swarm-authelia-service.nix b/nix/host-modules/swarm-authelia-service.nix index 53dbeb7d..a6046de5 100644 --- a/nix/host-modules/swarm-authelia-service.nix +++ b/nix/host-modules/swarm-authelia-service.nix @@ -341,22 +341,6 @@ in ''; }; - agentClientSuffix = lib.mkOption { - type = lib.types.str; - readOnly = true; - default = "-agent"; - description = '' - Suffix the queue's auth-callout responder refuses on a hive's client - id: `${config.services.hyperhive.swarm.authelia.hiveClientPrefix}alpha${config.services.hyperhive.swarm.authelia.agentClientSuffix}` - is denied, never read as hive `alpha-agent`. No client is minted with - it; an agent authenticates to the queue with its own secret. - - Read-only for the same reason as `hiveClientPrefix`. A hive named with - this suffix is refused at evaluation, since the queue would refuse its - own client id. - ''; - }; - machine = lib.mkOption { type = lib.types.str; readOnly = true; diff --git a/nix/host-modules/swarm-authelia.nix b/nix/host-modules/swarm-authelia.nix index bffb367e..92ad3963 100644 --- a/nix/host-modules/swarm-authelia.nix +++ b/nix/host-modules/swarm-authelia.nix @@ -682,30 +682,6 @@ 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 a hive's agent client, which it refuses: - # `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. 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 the " - + "queue refuses as a hive's agent client. Such a hive's own client " - + "id reads as one, and the queue refuses it. 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 cf247778..c2d61fdd 100644 --- a/nix/host-modules/swarm-nats.nix +++ b/nix/host-modules/swarm-nats.nix @@ -858,7 +858,6 @@ 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-suffix ${lib.escapeShellArg autheliaCfg.agentClientSuffix}" "--reader-client ${lib.escapeShellArg controllerCfg.queueClientId}" # Hears try-outs and answers each by reply, and nothing else. "--review-classifier-client ${lib.escapeShellArg config.services.hyperhive.swarm.review-classifier.clientId}" diff --git a/nix/host-modules/swarm.nix b/nix/host-modules/swarm.nix index e01cc179..92987475 100644 --- a/nix/host-modules/swarm.nix +++ b/nix/host-modules/swarm.nix @@ -360,12 +360,11 @@ in fragments = reservedFragments; why = '' Hive-scoped identifiers are composed from a hive name — - `hive-`, and that id with a suffix the queue's responder - refuses — so a name containing one of these produces an identifier - that is also somebody else's, or one the responder refuses. The - failure is a wrong grant rather than an error: the client - authenticates and receives another principal's permissions, and a - NATS denial arrives as a timeout. Rename the hive. + `hive-` — so a name containing one of these produces an + identifier that is also somebody else's. The failure is a wrong + grant rather than an error: the client authenticates and receives + another principal's permissions, and a NATS denial arrives as a + timeout. Rename the hive. ''; }) ]; diff --git a/nix/module-eval/name-guards.nix b/nix/module-eval/name-guards.nix index e4aa1a87..38fe1e53 100644 --- a/nix/module-eval/name-guards.nix +++ b/nix/module-eval/name-guards.nix @@ -226,7 +226,7 @@ let } { # The substring guard has to work with the collector off too. - # `h1-agent` would give the hive a client id the responder refuses. + # `h1-agent` contains the reserved word `agent`. name = "a hive name containing a composed-identifier word is refused, with the collector off"; ok = fragmentGuardFired hiveNameWithComposedWord; } diff --git a/nix/module-eval/nats-authelia.nix b/nix/module-eval/nats-authelia.nix index 2fa9af92..d21bc646 100644 --- a/nix/module-eval/nats-authelia.nix +++ b/nix/module-eval/nats-authelia.nix @@ -459,16 +459,16 @@ let !(lib.any (lib.hasSuffix "-policy.service") (u.after ++ u.wants)); } { - # The responder refuses the hive's shared agent client and takes no flag - # granting it subjects: an `--agent-publish-subject` fails the unit at - # start. - name = "the responder grants the hive's shared agent client nothing"; + # The responder takes no agent-client suffix: no such client is minted. + # `--hive-client-prefix` is the control that the lookup finds a flag the + # unit does pass. + name = "the responder is passed no agent-client suffix"; ok = let exec = natsQueueOnly.containers.swarm-nats.config.systemd.services.swarm-nats-auth.serviceConfig.ExecStart; in - !(lib.hasInfix "--agent-publish-subject " exec); + lib.hasInfix "--hive-client-prefix " exec && !(lib.hasInfix "--agent-client-suffix" exec); } { # The per-agent grant: the terminal and agent-state streams keyed on the diff --git a/nix/reserved-hive-fragments.nix b/nix/reserved-hive-fragments.nix index c2dd8b02..c87f3b15 100644 --- a/nix/reserved-hive-fragments.nix +++ b/nix/reserved-hive-fragments.nix @@ -9,8 +9,8 @@ # `hive-` — every hive's OIDC client id, and the string the queue's # auth-callout responder strips to decide which hive a connection is. "hive" - # `hive--agent` — an id the queue's responder refuses. A hive named - # `foo-agent` would get a client id the responder refuses. + # Keeps a hive's identifiers clear of the agent-scoped families, e.g. a hive + # named `agent-` colliding with `hive-` object names. "agent" # Also an exact-match entry next door, for the collector's `/` # components. Here because it is the word this swarm composes identifiers diff --git a/swarm-nats-auth/src/agent_token.rs b/swarm-nats-auth/src/agent_token.rs index c48869ce..3a27f567 100644 --- a/swarm-nats-auth/src/agent_token.rs +++ b/swarm-nats-auth/src/agent_token.rs @@ -86,7 +86,6 @@ mod tests { fn policy() -> Policy { Policy::new( "hive-".to_owned(), - "-agent".to_owned(), "hive-status".to_owned(), vec!["swarm-controller".to_owned()], vec![], @@ -154,14 +153,8 @@ mod tests { #[tokio::test] async fn a_verified_agent_with_no_subjects_configured_is_denied() { - let bare = Policy::new( - "hive-".to_owned(), - "-agent".to_owned(), - "hive-status".to_owned(), - vec![], - vec![], - ) - .expect("valid"); + let bare = Policy::new("hive-".to_owned(), "hive-status".to_owned(), vec![], vec![]) + .expect("valid"); let Presented::Agent(token) = classify("swarm-agent.atlas.Ab9_-zSECRET") else { panic!("an agent token"); }; diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index 39cd2571..59ec2bf5 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -125,21 +125,6 @@ struct Args { #[arg(long, default_value = "hive-")] hive_client_prefix: String, - /// Suffix appended to a hive's own client id: `hive--agent`. No - /// client is minted with it — agents authenticate with their own secret — - /// so the responder refuses it; the suffix is what tells such an id apart - /// from the hive `-agent`. - /// - /// 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. - /// - /// A suffix starts with `-`, so it has to be accepted as a value rather - /// than read as a short-flag cluster — without this the default below - /// cannot be passed back in on the command line that produced it. - #[arg(long, default_value = "-agent", allow_hyphen_values = true)] - 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. #[arg(long = "reader-client", default_values_t = [String::from("swarm-controller")])] @@ -239,7 +224,6 @@ async fn serve(args: Args) -> anyhow::Result<()> { // silently over-broad grant is not. let policy = policy::Policy::new( args.hive_client_prefix.clone(), - args.agent_client_suffix.clone(), swarm_queue_client::status::BUCKET.to_owned(), args.reader_clients.clone(), args.hive_publish_subjects.clone(), @@ -607,11 +591,10 @@ mod tests { }) } - /// The argument vector `swarm-nats.nix` actually builds, in the form it - /// builds it: every flag space-separated, and the suffix's value opening - /// with `-`. Read as a short-flag cluster it aborts before `main` runs, - /// and a responder that never starts denies every client — which reaches - /// the operator as `authorization violation` on an unrelated process. + /// The argument vector `swarm-nats.nix` actually builds, every flag + /// space-separated. A parse failure aborts before `main` runs, and a + /// responder that never starts denies every client — which reaches the + /// operator as `authorization violation` on an unrelated process. #[test] fn the_units_own_argument_vector_parses() { let args = responder([ @@ -620,8 +603,6 @@ mod tests { "https://auth.example/api/oidc/introspection", "--hive-client-prefix", "hive-", - "--agent-client-suffix", - "-agent", "--agent-token-publish-subject", "$SWARM.term.{agent}", "--agent-token-publish-subject", @@ -638,7 +619,6 @@ mod tests { "swarm-nats-auth", ]) .expect("the unit's own argument vector must parse"); - assert_eq!(args.agent_client_suffix, "-agent"); assert_eq!(args.agent_token_publish_subjects.len(), 6); } @@ -704,16 +684,11 @@ mod tests { "https://auth.example/api/oidc/introspection", ]) .expect("the required flags alone must parse"); - let policy = policy::Policy::new( - "hive-".to_owned(), - "-agent".to_owned(), - "hive-status".to_owned(), - vec![], - vec![], - ) - .expect("valid") - .with_agent_token_subjects(vec!["$SWARM.term.{agent}".to_owned()]) - .expect("valid"); + let policy = + policy::Policy::new("hive-".to_owned(), "hive-status".to_owned(), vec![], vec![]) + .expect("valid") + .with_agent_token_subjects(vec!["$SWARM.term.{agent}".to_owned()]) + .expect("valid"); let client_secret = oidc_secret::OidcSecret::new(swarm_secret_client::stored_secret::StoredSecret::new( "swarm-nats", @@ -847,35 +822,6 @@ mod tests { assert!(!expired(None, at(1_760_000_001_000))); } - /// The control for the case above: an ordinary value parses through the - /// same flag, so a failure there is about the leading `-` and not about - /// the argument being unknown. - #[test] - fn an_agent_suffix_without_a_leading_dash_parses_too() { - let args = responder([ - "swarm-nats-auth", - "--introspection-url", - "https://auth.example/api/oidc/introspection", - "--agent-client-suffix", - "agent", - ]) - .expect("an ordinary suffix must parse"); - assert_eq!(args.agent_client_suffix, "agent"); - } - - /// The default is the value the deployment passes, so the two cannot - /// drift into disagreeing about what an agent client is called. - #[test] - fn the_default_suffix_is_what_the_module_passes() { - let args = responder([ - "swarm-nats-auth", - "--introspection-url", - "https://auth.example/api/oidc/introspection", - ]) - .expect("the required flags alone must parse"); - assert_eq!(args.agent_client_suffix, "-agent"); - } - /// The verb as `swarm-bao-nats-callout-xkey` runs it, with none of the /// responder's required flags. #[test] diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index c193f2c9..e9ae3232 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -44,7 +44,6 @@ pub struct Permissions { #[derive(Debug, Clone)] pub struct Policy { hive_prefix: String, - agent_suffix: String, bucket: String, readers: Vec, extra_hive_subjects: Vec, @@ -62,16 +61,10 @@ const HIVE_PLACEHOLDER: &str = "{hive}"; const AGENT_PLACEHOLDER: &str = "{agent}"; impl Policy { - /// `hive_prefix` is the client-id prefix that marks a hive and - /// `agent_suffix` the suffix the responder refuses **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` additional subjects a hive 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. + /// `hive_prefix` is the client-id prefix that marks a hive. `bucket` is 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 the hive's own name). /// /// # Errors /// @@ -81,14 +74,8 @@ 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. - /// - /// 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_suffix: String, bucket: String, readers: Vec, extra_hive_subjects: Vec, @@ -103,21 +90,8 @@ impl Policy { a per-hive namespace" ); } - // 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. The - // agent arm runs first and refuses, so every hive would be refused as - // its own agents' client. - if agent_suffix.is_empty() { - anyhow::bail!( - "--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' client and be refused" - ); - } Ok(Self { hive_prefix, - agent_suffix, bucket, readers, extra_hive_subjects, @@ -221,22 +195,6 @@ 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 { - // ⚠️ 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. - // - // A hive's shared agent client is refused outright. Agents connect with - // their own credential (`Self::agent_token_permissions`); this identity - // names the hive and never the agent (see `Self::agent_hive`), so any - // grant to it would let one agent publish as its siblings. - if self.agent_hive(client_id).is_some() { - return None; - } if let Some(hive) = self.hive_name(client_id) { return Some(Permissions { publish: self.hive_subjects(hive), @@ -438,26 +396,6 @@ impl Policy { .filter(|name| !name.is_empty()) } - /// The hive whose agents a client id names, when it names one. - /// - /// [`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 id carries the hive and never the agent, so this answers *whose - /// agents*, never *which agent*. - /// - A hive genuinely named `foo-agent` has the client id `hive-foo-agent`, - /// which this reads as hive `foo`'s agent client, an id the responder - /// refuses. 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. - fn agent_hive<'a>(&self, client_id: &'a str) -> Option<&'a str> { - client_id - .strip_prefix(&self.hive_prefix)? - .strip_suffix(&self.agent_suffix) - .filter(|name| !name.is_empty()) - } - fn stream(&self) -> String { format!("KV_{}", self.bucket) } @@ -826,7 +764,6 @@ mod tests { fn policy() -> Policy { Policy::new( "hive-".to_owned(), - "-agent".to_owned(), "hive-status".to_owned(), vec!["swarm-controller".to_owned()], Vec::new(), @@ -1087,7 +1024,6 @@ 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()], @@ -1106,7 +1042,6 @@ 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()], @@ -2201,7 +2136,6 @@ 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()], @@ -2212,38 +2146,6 @@ mod tests { assert!(!g.publish.iter().any(|s| s.contains("{hive}"))); } - #[test] - fn the_hives_shared_agent_client_is_refused() { - // Agents connect with their own credential. The shared client names - // the hive and never the agent, so any grant to it would let one agent - // publish as its siblings. - // - // 🩸 The agent id satisfies the HIVE parse too (`hive-alpha-agent` - // strips to a hive named `alpha-agent`), so a hive-first arm order - // would hand this client its hive's grant, status key included, and - // nothing would report it. `is_none` here is what holds the order. - assert!(policy().permissions("hive-alpha-agent").is_none()); - // Control: the hive itself is admitted, so the refusal above is the - // agent arm answering rather than the prefix failing to match. - assert!(policy().permissions("hive-alpha").is_some()); - } - - #[test] - 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 the agent arm, which runs first and refuses, would - // refuse every hive. That fails at construction rather than at connect. - Policy::new( - "hive-".to_owned(), - String::new(), - "hive-status".to_owned(), - Vec::new(), - Vec::new(), - ) - .expect_err("an empty agent suffix must not construct"); - } - #[test] fn an_agent_token_grant_is_the_agents_own_subjects_and_nothing_else() { let p = policy() diff --git a/swarm-secret-client/src/policy.rs b/swarm-secret-client/src/policy.rs index 9f8ce2ac..12ebbfb6 100644 --- a/swarm-secret-client/src/policy.rs +++ b/swarm-secret-client/src/policy.rs @@ -65,10 +65,7 @@ pub fn hive_object_name(hive: &str) -> Result { /// `hive-`, so a collision needs a hive named `agent-` — and /// `nix/reserved-hive-fragments.nix` forbids the substring `agent` in any hive /// name (and `hive` too), applied to every entry of the swarm directory in -/// `swarm.nix`. That guard also keeps a hive's client id clear of the -/// `hive--agent` form the queue refuses; this is a second identifier -/// family leaning on it, which is why -/// the fragment list is what to read before renaming either. +/// `swarm.nix`. The fragment list is what to read before renaming either. /// /// Host principals' **common names** are operator-set options /// (`deploy.bao.controllerCommonName`, `secretPublisherCommonName`, …) that