diff --git a/docs/swarm/secrets.md b/docs/swarm/secrets.md index e7a57374..9faf7131 100644 --- a/docs/swarm/secrets.md +++ b/docs/swarm/secrets.md @@ -52,7 +52,7 @@ neither is a renaming of the other. | the swarm collector's copy of its OIDC secret | `swarm-bao-otel-oidc.service` reads it out of the swarm secret store, **on every host that runs the collector and holds a store identity** | `/var/lib/swarm-otel-oidc/.secret` inside the `swarm-otel` container | same unit, same path — one route, co-located or not. A collector with no store identity has `clientSecretFile == null`, its already-supported unauthenticated-push degrade — see below | | Grafana's copy of its OIDC secret | `swarm-bao-grafana-oidc.service` reads it out of the swarm secret store, **on every host that runs Grafana** | `/var/lib/grafana-oidc/.secret` inside the `swarm-grafana` container | same unit, same path — one route, co-located or not. Nothing for an operator to place beyond this host's store leaf, see below | | the swarm controller's copy of its OIDC secret | `swarm-controller` reads it out of the swarm secret store at start, under its own leaf | the daemon's memory only; never on disk | same read, co-located or not. Nothing for an operator to place beyond the controller's store leaf (`deploy.swarm-controller.baoClientCertFile`) | -| the queue responder's copy of its OIDC secret | `swarm-bao-nats-auth-oidc.service` reads it out of the swarm secret store, on the host that runs the queue, under the responder's own leaf | `/var/lib/swarm-nats-auth/oidc-client.secret` inside the `swarm-nats` container, handed to the responder through `LoadCredential` | same unit, same path, wherever authelia runs. Nothing for an operator to place beyond the responder's store leaf (`deploy.nats.authBaoClientCertFile`) | +| the queue responder's copy of its OIDC secret | the responder reads it out of the swarm secret store itself as it starts, under its own leaf, and again when authelia refuses it | the daemon's memory only; never on disk | same read, wherever authelia runs. Nothing for an operator to place beyond the responder's store leaf (`deploy.nats.authBaoClientCertFile`) | | authelia subject store | `swarmctl` and `swarm-authelia-bridge` | `users.yml` — one file, read and written by both | `swarmctl`, on the host that runs authelia | | wireguard private key | **the operator** — `wg genkey` | whatever `deploy.wireguard.privateKeyFile` names | always operator-provided; nothing generates this for you | | queue auth-callout nkeys (user seed + account seed) | `swarm-nats-auth ensure-callout-keys`, run by `swarm-bao-nats-callout-xkey` on the queue's host when the store holds no seed; the verb stores a seed it finds at `/var/lib/swarm-nats-callout/{callout-user,issuer}.seed` as is, then removes the file | the swarm secret store: the seeds at `swarm/services/swarm-nats/callout/{user,issuer}-seed`, read by the `swarm-nats-auth` role alone; the public halves at `swarm/public/swarm-nats/callout-{user,issuer}` | nothing to place beyond the responder's store leaf (`deploy.nats.authBaoClientCertFile`) | @@ -303,10 +303,10 @@ so it's declared on the host running authelia, which isn't necessarily the host running Grafana. The swarm controller, the queue and the homeserver register the same way (`glue-controller-oidc-client.nix`, `glue-nats-oidc-client.nix`, `glue-matrix-oidc-client.nix`), and the queue's responder and the homeserver read -their secrets from the store too: `swarm-bao-nats-auth-oidc.service` under the -responder's own leaf, `swarm-bao-matrix-oidc.service` under matrix-ctl's. Each -renders wherever its consumer runs and fails, naming the option pair, when that -leaf is unset — neither consumer starts without its secret. +their secrets from the store too: the responder itself under its own leaf, +`swarm-bao-matrix-oidc.service` under matrix-ctl's. Each fails, naming the +option pair, when that leaf is unset — neither consumer starts without its +secret. The **third reader** is the swarm collector's own OIDC client secret, the same shape one level down: `swarm-bao-otel-oidc.service` reads diff --git a/nix/host-modules/glue-bao-readers-policy-order.nix b/nix/host-modules/glue-bao-readers-policy-order.nix index 6293a726..de9413df 100644 --- a/nix/host-modules/glue-bao-readers-policy-order.nix +++ b/nix/host-modules/glue-bao-readers-policy-order.nix @@ -53,15 +53,10 @@ let # Readers that log in under another unit's principal: reader unit → the # gate its own module defines it under, and the policy units it waits for. readersOfOtherPolicy = { - # ./swarm-nats.nix, under the queue responder's leaf. - swarm-bao-nats-auth-oidc = { - here = deployCfg.nats.enable; - policies = [ "swarm-bao-nats-auth-policy" ]; - }; - # ./swarm-nats.nix, under the same leaf. Also after the publisher's - # policy, which denies the publisher the callout seeds' paths: until it - # is written the publisher may create them, and this unit keeps a seed it - # finds there. + # ./swarm-nats.nix, under the queue responder's leaf. Also after the + # publisher's policy, which denies the publisher the callout seeds' paths: + # until it is written the publisher may create them, and this unit keeps + # a seed it finds there. swarm-bao-nats-callout-xkey = { here = deployCfg.nats.enable; policies = [ diff --git a/nix/host-modules/swarm-bao.nix b/nix/host-modules/swarm-bao.nix index d73a2c3f..3d6543bc 100644 --- a/nix/host-modules/swarm-bao.nix +++ b/nix/host-modules/swarm-bao.nix @@ -866,7 +866,7 @@ let # presents against the one stored for it. One leaf under every agent: `+` is # one path segment, where `*` globs only at the end and would reach every # other credential an agent holds. Plus the responder's own OIDC client - # secret, which `swarm-bao-nats-auth-oidc` reads under this same leaf. + # secret, which it reads as it starts and again when the IdP refuses it. # # And the two leaves of each callout key, which `ensure-callout-keys` # creates when the store holds no seed. `create` without `update`, so a diff --git a/nix/host-modules/swarm-nats.nix b/nix/host-modules/swarm-nats.nix index e8a7ada5..8fa80cd7 100644 --- a/nix/host-modules/swarm-nats.nix +++ b/nix/host-modules/swarm-nats.nix @@ -48,10 +48,6 @@ let secretDir = "/var/lib/nixos-containers/${machine}${secretDirInContainer}"; hostPath = name: "${secretDir}/${name}"; - # Where swarm-secret-publish on authelia's host leaves this client's secret, - # composed from the same swarm-wide client id it publishes under. The - # responder's store role grants read on this one path. - storeSecretPath = "secret/swarm/services/${cfg.clientId}/oidc/client"; introspectionUrl = "${toString autheliaUrl}/api/oidc/introspection"; # Host seed files `ensure-callout-keys` stores when the secret store holds @@ -402,10 +398,9 @@ in example = "/var/lib/swarm-bao-pki/nats-auth.pem"; description = '' Client certificate the auth-callout responder presents to the swarm's - secret store to read the callout xkey and agent queue credentials, and - that `swarm-bao-nats-auth-oidc` and `swarm-bao-nats-callout-xkey` - present to read the responder's own OIDC client secret and the xkey's - public half. Its subject must be + secret store to read the callout keys, its own OIDC client secret and + agent queue credentials, and that `swarm-bao-nats-callout-xkey` + presents to read the callout keys' public halves. Its subject must be {option}`services.hyperhive.deploy.bao.natsAuthCommonName`. Required when `deploy.nats.enable` is set; unset fails at eval. @@ -415,9 +410,8 @@ in host it is a file the operator places. Never derived from {option}`services.hyperhive.deploy.bao.clientCertFile`, which is the hive's own reader. Set to a file that does - not exist, `swarm-nats-auth-secrets`, `swarm-bao-nats-auth-oidc` and - `swarm-bao-nats-callout-xkey` fail, and the queue's server does not - start without the xkey. + not exist, `swarm-nats-auth-secrets` and `swarm-bao-nats-callout-xkey` + fail, and the queue's server does not start without the xkey. A path, never a value. ''; @@ -519,7 +513,13 @@ in # The bind source has to exist before the container starts, leaf or not: # nixos-container refuses to start over a missing one, which would take # the whole queue down rather than only its TLS. - systemd.tmpfiles.rules = [ "d ${tlsDir} 0755 root root -" ]; + # + # The `r` removes a copy of the responder's OIDC client secret a container + # may still hold: the responder reads it from the store and names no file. + systemd.tmpfiles.rules = [ + "d ${tlsDir} 0755 root root -" + "r ${hostPath "oidc-client.secret"}" + ]; containers.swarm-nats = { autoStart = true; @@ -697,7 +697,6 @@ in # container's resolver goes through the bridge, where dnsmasq # answers it. "--nats-url tls://${cfg.domain}:${toString cfg.port}" - "--client-secret-file \${CREDENTIALS_DIRECTORY}/oidc-client.secret" "--client-id ${lib.escapeShellArg cfg.clientId}" # The account admitted clients land in, by NAME: in # server-config mode the server resolves `aud` against its @@ -748,14 +747,12 @@ in "--agent-token-publish-subject ${lib.escapeShellArg "\$\$JS.API.DIRECT.GET.KV_agent-schedules.\$\$KV.agent-schedules.{agent}"}" "--store-cert-role ${lib.escapeShellArg authCertRole}" ]; - # Every credential arrives by `LoadCredential` and is named - # on the command line only as a **path** — `argv` is - # world-readable via /proc//cmdline, so a value there - # would be readable by every process on the host netns. - LoadCredential = [ - "oidc-client.secret:${inContainer "oidc-client.secret"}" - ] - ++ lib.mapAttrsToList (id: _: "${id}:${inContainer id}") authStoreFiles; + # The store identity arrives by `LoadCredential` and is named + # only as a **path**: `argv` is world-readable via + # /proc//cmdline. Every secret the responder holds beyond + # it, its own OIDC client secret included, it reads from the + # store with that identity. + LoadCredential = lib.mapAttrsToList (id: _: "${id}:${inContainer id}") authStoreFiles; DynamicUser = true; Restart = "on-failure"; RestartSec = "5s"; @@ -815,108 +812,6 @@ in ); }; - # The responder's OIDC client secret, out of the swarm secret store under - # the responder's own leaf — the one route, wherever authelia runs. Same - # shape as ./swarm-bao.nix's `swarm-bao-forwarder-oidc`: the login and the - # read both fail loudly, and `storeRetry` retries until the publisher on - # authelia's host has written the secret, because the responder has no - # mode that runs without it. - # - # ⚠️ Without the secret the responder does not start (its `LoadCredential` - # source is missing), and `auth_callout` with no responder refuses every - # client. - systemd.services.swarm-bao-nats-auth-oidc = { - description = "fetch the swarm queue responder's OIDC client secret from the swarm secret store"; - # The store's units exist only where the store runs; `Requires=` on an - # absent unit fails the job, so off-host the timeout below bounds the - # attempt instead. - after = [ - "swarm-nats-auth-secrets.service" - ] - ++ lib.optionals baoDeploy.enable [ - "swarm-bao-pki.service" - "container@${baoCfg.machine}.service" - ]; - wants = lib.optionals baoDeploy.enable [ "container@${baoCfg.machine}.service" ]; - requires = lib.optionals baoDeploy.enable [ "swarm-bao-pki.service" ]; - before = [ "container@${machine}.service" ]; - wantedBy = [ - "multi-user.target" - "container@${machine}.service" - ]; - path = [ - baoDeploy.package - pkgs.coreutils - pkgs.systemd - ]; - # ./lib/store-retry.nix. The container waits for one attempt; a secret a - # later attempt lands restarts the responder (below). - inherit (storeRetry) startLimitBurst startLimitIntervalSec; - serviceConfig = storeRetry.serviceConfig // { - Type = "oneshot"; - RemainAfterExit = true; - SyslogIdentifier = "swarm-bao-nats-auth-oidc"; - TimeoutStartSec = 30; - }; - environment = { - BAO_ADDR = "https://${baoCfg.domain}:${toString baoCfg.port}"; - BAO_CLIENT_CERT = deployCfg.nats.authBaoClientCertFile; - BAO_CLIENT_KEY = deployCfg.nats.authBaoClientKeyFile; - } - // lib.optionalAttrs (baoDeploy.serverCaFile != null) { - BAO_CACERT = baoDeploy.serverCaFile; - }; - script = '' - set -euo pipefail - - ${atomicWriteSecret} - ${refreshConsumer} - - err="$(mktemp)" - trap 'rm -f "$err"' EXIT - - # `-token-only` answers on stdout and skips the token helper, a - # `sh` this unit's `path` does not carry. - if ! BAO_TOKEN="$(bao login -method=cert -token-only 2>"$err")"; then - echo "could not log in to swarm-bao with the queue responder's certificate; leaving its OIDC client secret as it is." >&2 - if [ -s "$err" ]; then - cat "$err" >&2 - else - echo "bao failed without writing a diagnostic." >&2 - fi - exit 1 - fi - export BAO_TOKEN - - if ! secret="$(bao kv get -field=value ${lib.escapeShellArg storeSecretPath} 2>"$err")"; then - echo "swarm-bao did not return ${storeSecretPath}; the queue responder has no OIDC client secret yet." >&2 - if [ -s "$err" ]; then - cat "$err" >&2 - else - echo "bao failed without writing a diagnostic." >&2 - fi - exit 1 - fi - - if [ -z "$secret" ]; then - echo "swarm-bao returned an empty ${storeSecretPath}; the queue responder has no OIDC client secret yet." >&2 - exit 1 - fi - - # Root-owned 0400: `LoadCredential` reads it as root before the - # responder's `DynamicUser` exists. `printf` inside - # `atomic_write_secret` is a builtin, so the plaintext never - # becomes an argument in /proc. - install -d -m 0700 ${lib.escapeShellArg secretDir} - if secret_differs ${lib.escapeShellArg (hostPath "oidc-client.secret")} "$secret"; then - atomic_write_secret 0400 "" ${lib.escapeShellArg (hostPath "oidc-client.secret")} "$secret" - fi - - # `LoadCredential` copies the file once, at the responder's start. - refresh_consumer ${lib.escapeShellArg machine} swarm-nats-auth.service ${lib.escapeShellArg (hostPath "oidc-client.secret")} - ''; - }; - # The callout's three public keys, from the store into the files # `renderConfig` reads at every server start. `ensure-callout-keys` first # creates each pair the store holds none of; it needs no queue, so on a @@ -936,8 +831,9 @@ in # responder's own leaf, the one principal that may create the keys' paths. systemd.services.swarm-bao-nats-callout-xkey = { description = "create the swarm queue's callout keys if absent and fetch their public halves for the server"; - # As `swarm-bao-nats-auth-oidc`: the store's units exist only where it - # runs. + # The store's units exist only where the store runs; `Requires=` on an + # absent unit fails the job, so off-host the timeout below bounds the + # attempt instead. after = lib.optionals baoDeploy.enable [ "swarm-bao-pki.service" "container@${baoCfg.machine}.service" diff --git a/nix/host-modules/swarm-secret-publisher.nix b/nix/host-modules/swarm-secret-publisher.nix index 71029540..ef67d5e0 100644 --- a/nix/host-modules/swarm-secret-publisher.nix +++ b/nix/host-modules/swarm-secret-publisher.nix @@ -79,8 +79,8 @@ let # The store's browser UI, whose `oidc` auth method the store's host # configures with this secret. hyperhiveCfg.swarm.bao.ui.oidc.clientId - # The queue's auth-callout responder, read by `swarm-bao-nats-auth-oidc` - # on the queue's host. + # The queue's auth-callout responder, which reads it itself on the + # queue's host. hyperhiveCfg.swarm.nats.clientId # The homeserver, read by `swarm-bao-matrix-oidc` on its host. hyperhiveCfg.swarm.matrix.sso.clientId diff --git a/nix/module-eval/bao-grants.nix b/nix/module-eval/bao-grants.nix index 8ca4dc2b..90e5dbe7 100644 --- a/nix/module-eval/bao-grants.nix +++ b/nix/module-eval/bao-grants.nix @@ -823,7 +823,7 @@ let && lib.elem "swarm-bao-matrix-ctl-policy.service" u.wants && !(lib.elem "swarm-bao-matrix-ctl-policy.service" u.requires) && !(noReaders ? swarm-bao-matrix-oidc) - && !(noReaders ? swarm-bao-nats-auth-oidc); + && !(noReaders ? swarm-bao-nats-callout-xkey); } { # The ordering is set apart from each reader's own definition, so it can diff --git a/nix/module-eval/nats-authelia.nix b/nix/module-eval/nats-authelia.nix index d12e19bf..69cb2a37 100644 --- a/nix/module-eval/nats-authelia.nix +++ b/nix/module-eval/nats-authelia.nix @@ -347,8 +347,8 @@ let } { # The responder names no seed file, by flag or by credential, and the - # copies a container may still hold are removed. The OIDC secret is the - # control that the credential list is read at all. + # copies a container may still hold are removed. The store identity is + # the control that the credential list is read at all. name = "the responder takes no seed from a file, and the container's seed copies are removed"; ok = let @@ -357,7 +357,7 @@ let dir = "/var/lib/nixos-containers/swarm-nats/var/lib/swarm-nats-auth"; rules = natsWithStoreIdentity.systemd.tmpfiles.rules; in - lib.elem "oidc-client.secret:/var/lib/swarm-nats-auth/oidc-client.secret" creds + lib.elem "bao-client.pem:/var/lib/swarm-nats-auth/bao-client.pem" creds && !(lib.any (lib.hasInfix ".seed") creds) && !(lib.hasInfix "seed-file" u.serviceConfig.ExecStart) && lib.elem "r ${dir}/issuer.seed" rules @@ -404,41 +404,31 @@ let && !(lib.any (path: refused natsWithStoreIdentity path) (lib.attrNames setting)); } { - # The responder's OIDC secret has one route, the store, under the - # responder's own leaf. The copy unit's arms are the control: on a queue - # host without authelia nothing reads authelia's tree, and nothing - # copies the secret from any host file. - name = "a queue host without authelia reads the responder's OIDC secret from the store, and copies none from a host file"; + # The responder reads its OIDC secret from the store itself, so no host + # unit fetches it, no credential or flag carries it, and a copy a + # container still holds is removed. On or off the store's host alike; + # the callout key fetch beside it is the control that the unit set is + # read at all, and the store cert role the control on the flags. + name = "the responder's OIDC secret is fetched by no unit and carried by no credential or flag, and the container's copy is removed"; ok = let - services = natsWithStoreIdentity.systemd.services; - fetch = services.swarm-bao-nats-auth-oidc; - copy = services.swarm-nats-auth-secrets.script; + fetchedNowhere = + h: + !(h.systemd.services ? swarm-bao-nats-auth-oidc) + && h.systemd.services ? swarm-bao-nats-callout-xkey; + u = responderOf natsWithStoreIdentity; + copy = natsWithStoreIdentity.systemd.services.swarm-nats-auth-secrets.script; + rules = natsWithStoreIdentity.systemd.tmpfiles.rules; in - !natsWithStoreIdentity.services.hyperhive.deploy.authelia.enable - && fetch.environment.BAO_CLIENT_CERT == "/etc/pki/nats-auth.pem" - && fetch.environment.BAO_CLIENT_KEY == "/etc/pki/nats-auth-key.pem" - && lib.hasInfix "bao kv get -field=value secret/swarm/services/swarm-nats/oidc/client" fetch.script - && lib.hasInfix "/var/lib/nixos-containers/swarm-nats/var/lib/swarm-nats-auth/oidc-client.secret" fetch.script - && lib.hasInfix "refresh_consumer swarm-nats swarm-nats-auth.service" fetch.script - # An unpublished or empty secret fails the unit, so `storeRetry` retries it. - && !(lib.hasInfix "exit 0" fetch.script) - && lib.elem "container@swarm-nats.service" fetch.before + fetchedNowhere natsWithStoreIdentity + && fetchedNowhere natsOnStoreHost + && !(lib.any (lib.hasInfix "oidc-client") u.serviceConfig.LoadCredential) + && !(lib.hasInfix "client-secret" u.serviceConfig.ExecStart) + && lib.hasInfix "--store-cert-role swarm-nats-auth" u.serviceConfig.ExecStart + && lib.elem "r /var/lib/nixos-containers/swarm-nats/var/lib/swarm-nats-auth/oidc-client.secret" rules && !(lib.hasInfix "oidc-client" copy) && !(lib.hasInfix "authelia" copy); } - { - # On the store's host the fetch logs in under the responder's leaf, so it - # waits for the unit writing that role. - name = "on the store's host the responder's OIDC secret fetch uses its leaf and waits for its role"; - ok = - let - u = natsOnStoreHost.systemd.services.swarm-bao-nats-auth-oidc; - in - u.environment.BAO_CLIENT_CERT == "/var/lib/swarm-bao-pki/nats-auth.pem" - && lib.elem "swarm-bao-nats-auth-policy.service" u.after - && lib.elem "swarm-bao-nats-auth-policy.service" u.wants; - } { # On the store's host the glue pairs the responder with a leaf of its # own, never the queue's TLS-issuing one or the hive's. diff --git a/swarm-nats-auth/Cargo.toml b/swarm-nats-auth/Cargo.toml index 5b9eca7f..e77b546a 100644 --- a/swarm-nats-auth/Cargo.toml +++ b/swarm-nats-auth/Cargo.toml @@ -48,6 +48,9 @@ tracing-subscriber.workspace = true # builds a user token both ways and requires byte equality, on the one shape # nats-jwt does model. nats-jwt.workspace = true +# `test-util` for `#[tokio::test(start_paused = true)]`: `oidc_secret`'s tests +# run a re-read slower than the introspection budget on paused time. +tokio = { workspace = true, features = ["test-util"] } [lints] workspace = true diff --git a/swarm-nats-auth/README.md b/swarm-nats-auth/README.md index 5c40ee83..99e217e2 100644 --- a/swarm-nats-auth/README.md +++ b/swarm-nats-auth/README.md @@ -64,10 +64,15 @@ paths and the boot order. - **Every credential is a path, never a value.** A value in nix config lands in the world-readable store; a value in `argv` is readable via `/proc//cmdline`, which is `0444`. Paths are not secrets, so passing - them as flags is fine. + them as flags is fine. The secrets themselves, the callout seeds and the + responder's own OIDC client secret, are read from the secret store and held + in memory. - **The introspection timeout is pinned below the server's `authorization. timeout`**, with a test asserting the relation — a responder that answers after the server gave up is indistinguishable from one that never answered. + It is one request's whole budget: when the `IdP` refuses the responder's own + secret, the re-read from the store and the one retry fit inside it, or the + request is denied and the re-read lands for the next one. ## What the tests do and do not cover diff --git a/swarm-nats-auth/src/introspect.rs b/swarm-nats-auth/src/introspect.rs index 30aae01c..dfce9c0f 100644 --- a/swarm-nats-auth/src/introspect.rs +++ b/swarm-nats-auth/src/introspect.rs @@ -33,12 +33,26 @@ use std::time::Duration; use anyhow::{Context, Result}; use serde::Deserialize; +use tokio::time::Instant; /// Kept under `swarm-nats.nix`'s `authorization.timeout = "2s"` — see the /// module docs. A slower answer is not a late success, it is a denial that -/// already happened. +/// already happened. It bounds a whole request, the re-read of this +/// responder's own secret and the retry after it included, which is why +/// [`identify_caller`] takes a deadline rather than a duration. pub const INTROSPECTION_TIMEOUT: Duration = Duration::from_millis(1500); +/// What one introspection call said. +#[derive(Debug, PartialEq, Eq)] +pub enum Answer { + /// A 2xx: the caller, or `None` for every shape that is not an admission. + Caller(Option), + /// A non-2xx: the `IdP` did not answer about the token at all, which is + /// also how it refuses this responder's own credential. `oidc_secret` + /// re-reads that credential and retries once before this denies. + Refused, +} + /// The two fields this responder acts on — one per decision. Authelia returns /// more (`sub`, `scope`, `exp`); modelling them would imply we check them. #[derive(Debug, Deserialize)] @@ -75,10 +89,12 @@ impl IntrospectionResponse { /// Ask the `IdP` whether `token` is currently valid, and as whom. /// -/// Returns `Ok(Some(client_id))` **only** on a 2xx whose body says `active: -/// true` and names the client. Every other outcome is `Ok(None)` with the -/// reason logged, or `Err` when the call could not be made at all — callers -/// must treat both as a denial. +/// Returns `Ok(Answer::Caller(Some(client_id)))` **only** on a 2xx whose +/// body says `active: true` and names the client. Every other 2xx is +/// `Ok(Answer::Caller(None))` with the reason logged, a non-2xx is +/// `Ok(Answer::Refused)`, and `Err` is a call that could not be made at all +/// or did not finish by `deadline` — callers must treat all but the first as +/// a denial. /// /// `own_client_id` / `own_client_secret` are *this responder's* credential for /// the introspection endpoint. They are not the caller's identity and must @@ -93,26 +109,27 @@ pub async fn identify_caller( own_client_id: &str, own_client_secret: &str, token: &str, -) -> Result> { + deadline: Instant, +) -> Result { let resp = http .post(url) .basic_auth(own_client_id, Some(own_client_secret)) .form(&[("token", token)]) - .timeout(INTROSPECTION_TIMEOUT) + .timeout(deadline.saturating_duration_since(Instant::now())) .send() .await .context("introspection request")?; let status = resp.status(); if !status.is_success() { - tracing::warn!(%status, "introspection returned non-2xx; denying"); - return Ok(None); + tracing::warn!(%status, "introspection returned non-2xx"); + return Ok(Answer::Refused); } let body: IntrospectionResponse = match resp.json().await { Ok(b) => b, Err(e) => { tracing::warn!(error = ?e, "introspection body did not parse; denying"); - return Ok(None); + return Ok(Answer::Caller(None)); } }; if body.active && body.caller().is_none() { @@ -121,7 +138,7 @@ pub async fn identify_caller( // rejected for no visible reason. tracing::warn!("introspection said active but named no client; denying"); } - Ok(body.caller().map(str::to_owned)) + Ok(Answer::Caller(body.caller().map(str::to_owned))) } #[cfg(test)] diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index cf6ab0b4..ebde0b60 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -22,12 +22,12 @@ //! //! # Secrets //! -//! Every credential is taken as a **path**, never a value. Two reasons, both -//! previously learned the hard way here: a value in nix config is rendered into -//! the world-readable store, and a value in `argv` is readable by anyone via -//! `/proc//cmdline`, which is `0444`. Paths are not secrets, so passing -//! them as flags is fine. The callout's three seeds are not files: they are -//! read from the secret store, at the paths `xkey` and `nkey` name. +//! Every credential is taken as a **path**, never a value: a value in nix +//! config is rendered into the world-readable store, and a value in `argv` is +//! readable by anyone via `/proc//cmdline`, which is `0444`. Paths are +//! not secrets, so passing them as flags is fine. The callout's three seeds +//! and the responder's own OIDC client secret are not files: they are read +//! from the secret store, at the paths `xkey`, `nkey` and `oidc_secret` name. use std::path::PathBuf; @@ -39,6 +39,7 @@ mod agent_token; mod introspect; mod keystore; mod nkey; +mod oidc_secret; mod policy; mod request; mod respond; @@ -95,7 +96,8 @@ struct Args { #[arg(long)] introspection_url: String, - /// `OAuth2` client id this responder introspects as. Must match + /// `OAuth2` client id this responder introspects as, and whose secret it + /// reads from the store. Must match /// `services.hyperhive.swarm.nats.clientId`, whose default this mirrors. #[arg(long, default_value = "swarm-nats")] client_id: String, @@ -107,10 +109,6 @@ struct Args { #[arg(long, default_value = "APP")] account: String, - /// Path to this responder's own OIDC client secret. - #[arg(long)] - client_secret_file: PathBuf, - /// Client-id prefix that marks a hive. `swarm-authelia.nix` mints one /// machine client per roster entry as `hive-`, while the KV key is /// the bare `` — this is the contract between the two, declared @@ -160,27 +158,14 @@ struct Args { agent_token_publish_subjects: Vec, /// Role on the secret store's `cert` auth mount this responder logs in - /// as to read the callout keys and agent credentials. The store is found + /// as to read the callout keys, its own OIDC client secret and agent + /// credentials. The store is found /// through the `BAO_*` environment, which the responder does not start /// without. #[arg(long, default_value = "swarm-nats-auth")] store_cert_role: String, } -/// Read a secret file and strip surrounding whitespace. -/// -/// The trim matters: an `echo`-created file ends in a newline, which is not -/// part of the secret. The value is never logged, and the error deliberately -/// names only the path. -fn read_secret(path: &std::path::Path) -> anyhow::Result { - let raw = std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?; - let trimmed = raw.trim(); - if trimmed.is_empty() { - anyhow::bail!("{} is empty", path.display()); - } - Ok(trimmed.to_owned()) -} - #[tokio::main] async fn main() -> anyhow::Result<()> { swarm_queue_client::install_crypto_provider(); @@ -219,7 +204,6 @@ async fn main() -> anyhow::Result<()> { /// The responder: answers every authorization request until its /// subscription ends. async fn serve(args: &Args) -> anyhow::Result<()> { - let client_secret = read_secret(&args.client_secret_file)?; // The bucket is NOT a flag. `swarm_queue_client::status`'s own docs say // why: reader and writer must name the same bucket, and an option is a // way for two deployments to disagree about which one that is. This @@ -241,9 +225,17 @@ async fn serve(args: &Args) -> anyhow::Result<()> { let settings = swarm_secret_client::client::Settings::from_env() .context("reading the secret store's address and this responder's identity")?; // Before the connect: a responder that cannot open a sealed request, sign - // a grant or authenticate to the queue must not start answering. - let (callout_xkey, issuer, user_seed) = - load_callout_keys(&settings, &args.store_cert_role).await?; + // a grant, authenticate to the queue or introspect must not start + // answering. + let keys = swarm_secret_client::SecretStore::connect( + &settings, + &args.store_cert_role, + swarm_secret_client::client::DEFAULT_CERT_MOUNT, + ) + .await + .context("logging in to the secret store for the callout keys and the OIDC client secret")?; + let (callout_xkey, issuer, user_seed) = load_callout_keys(&keys).await?; + let client_secret = load_client_secret(&keys, args).await?; let store = agent_token::Store::new(settings, args.store_cert_role.clone()); let client = async_nats::ConnectOptions::with_nkey(user_seed) .name("swarm-nats-auth") @@ -368,21 +360,13 @@ fn open_request( /// The callout xkey, the issuer key, and the user key's seed, read from the /// store and never created here; see `xkey` and `nkey`. async fn load_callout_keys( - settings: &swarm_secret_client::client::Settings, - store_cert_role: &str, + keys: &impl keystore::KeyStore, ) -> anyhow::Result<(nkeys::XKey, nkeys::KeyPair, String)> { - let keys = swarm_secret_client::SecretStore::connect( - settings, - store_cert_role, - swarm_secret_client::client::DEFAULT_CERT_MOUNT, - ) - .await - .context("logging in to the secret store for the callout keys")?; - let callout_xkey = xkey::load(&keys).await?; + let callout_xkey = xkey::load(keys).await?; tracing::info!(public_key = %callout_xkey.public_key(), "loaded the callout xkey"); - let issuer = nkey::load(&keys, &nkey::ISSUER).await?; + let issuer = nkey::load(keys, &nkey::ISSUER).await?; tracing::info!(public_key = %issuer.public_key(), "loaded the callout issuer key"); - let user = nkey::load(&keys, &nkey::USER).await?; + let user = nkey::load(keys, &nkey::USER).await?; tracing::info!(public_key = %user.public_key(), "loaded the callout user key"); let user_seed = user .seed() @@ -390,6 +374,23 @@ async fn load_callout_keys( Ok((callout_xkey, issuer, user_seed)) } +/// This responder's own OIDC client secret, read from `keys`, and re-read +/// later as `args.store_cert_role`; see `oidc_secret`. +async fn load_client_secret( + keys: &impl keystore::KeyStore, + args: &Args, +) -> anyhow::Result> { + let path = oidc_secret::path(&args.client_id)?; + let secret = oidc_secret::read(keys, &path).await?; + tracing::info!(%path, "loaded the OIDC client secret"); + let source = oidc_secret::Store::new( + swarm_secret_client::client::Settings::from_env()?, + args.store_cert_role.clone(), + path, + ); + Ok(oidc_secret::OidcSecret::new(source, secret)) +} + /// The `ensure-callout-keys` verb: see [`xkey::ensure`] and [`nkey::ensure`]. async fn ensure_callout_keys( store_cert_role: &str, @@ -429,24 +430,30 @@ async fn introspected( policy: &policy::Policy, http: &reqwest::Client, args: &Args, - client_secret: &str, + client_secret: &oidc_secret::OidcSecret, token: &str, ) -> (Option, Option) { - let caller = introspect::identify_caller( - http, - &args.introspection_url, - &args.client_id, - client_secret, - token, - ) - .await - // An introspection that could not be *made* is a denial too. The - // failure modes of an HTTP call are exactly the conditions under - // which an attacker would most like this to fall open. - .unwrap_or_else(|e| { - tracing::warn!(error = ?e, "introspection failed; denying"); - None - }); + let deadline = tokio::time::Instant::now() + introspect::INTROSPECTION_TIMEOUT; + let caller = client_secret + .introspect(deadline, |secret| async move { + introspect::identify_caller( + http, + &args.introspection_url, + &args.client_id, + &secret, + token, + deadline, + ) + .await + }) + .await + // An introspection that could not be *made* is a denial too. The + // failure modes of an HTTP call are exactly the conditions under + // which an attacker would most like this to fall open. + .unwrap_or_else(|e| { + tracing::warn!(error = ?e, "introspection failed; denying"); + None + }); // Admission said who; the policy says what. A caller the `IdP` // vouches for but no rule matches is denied — see `policy`'s module // docs for why that is deny and not "connect with nothing". @@ -484,8 +491,6 @@ mod tests { fn the_units_own_argument_vector_parses() { let args = responder([ "swarm-nats-auth", - "--client-secret-file", - "/run/credentials/oidc-client.secret", "--introspection-url", "https://auth.example/api/oidc/introspection", "--hive-client-prefix", @@ -543,8 +548,6 @@ mod tests { fn an_agent_suffix_without_a_leading_dash_parses_too() { let args = responder([ "swarm-nats-auth", - "--client-secret-file", - "/run/credentials/oidc-client.secret", "--introspection-url", "https://auth.example/api/oidc/introspection", "--agent-client-suffix", @@ -560,8 +563,6 @@ mod tests { fn the_default_suffix_is_what_the_module_passes() { let args = responder([ "swarm-nats-auth", - "--client-secret-file", - "/run/credentials/oidc-client.secret", "--introspection-url", "https://auth.example/api/oidc/introspection", ]) @@ -619,8 +620,8 @@ mod tests { fn the_verb_does_not_take_the_responders_flags() { Cli::try_parse_from([ "swarm-nats-auth", - "--client-secret-file", - "/run/credentials/oidc-client.secret", + "--introspection-url", + "https://auth.example/api/oidc/introspection", "ensure-callout-keys", ]) .expect_err("a verb and the responder's flags conflict"); diff --git a/swarm-nats-auth/src/oidc_secret.rs b/swarm-nats-auth/src/oidc_secret.rs new file mode 100644 index 00000000..9adc29a1 --- /dev/null +++ b/swarm-nats-auth/src/oidc_secret.rs @@ -0,0 +1,397 @@ +//! This responder's own OIDC client secret, the credential it introspects +//! with. Read from the secret store as the responder starts and held in +//! memory, never in a file, `argv` or the environment. +//! +//! The secret can change under a running responder: authelia's host +//! re-publishes it when it is re-minted. So when the `IdP` refuses an +//! introspection (a non-2xx), the secret is read from the store again and, +//! when it changed, the introspection is tried once more. Both fit in the one +//! request's [`INTROSPECTION_TIMEOUT`](crate::introspect::INTROSPECTION_TIMEOUT). +//! A re-read that does not is left running: that request is denied, and the +//! next one introspects with what the re-read landed. One re-read runs at a +//! time; a refusal while one is running waits for it rather than starting +//! another. + +use std::future::Future; +use std::sync::{Arc, Mutex, RwLock}; + +use anyhow::{Context, Result}; +use futures_util::FutureExt; +use futures_util::future::{BoxFuture, Shared}; +use swarm_secret_client::client::{DEFAULT_CERT_MOUNT, Settings}; +use swarm_secret_client::path::{Kind, principal_prefix}; +use tokio::task::AbortHandle; +use tokio::time::Instant; + +use crate::introspect::Answer; +use crate::keystore::KeyStore; + +/// The path under the store's KV mount holding `client_id`'s secret. +/// `swarm-secret-publisher.nix` writes it, and `natsAuthReaders` in +/// `swarm-bao.nix` grants this responder's role the read. +/// +/// # Errors +/// When `client_id` is not a single path segment. +pub fn path(client_id: &str) -> Result { + let prefix = principal_prefix(Kind::Service, client_id)?; + Ok(format!("{prefix}/oidc/client")) +} + +/// The secret at `path`, with surrounding whitespace trimmed: the publisher +/// stores the minted file as is, and a trailing newline is not part of the +/// secret. Errors name the path, never the value. +/// +/// # Errors +/// When the store cannot be read, or holds nothing or only whitespace there. +pub async fn read(store: &impl KeyStore, path: &str) -> Result { + let stored = store.read(path).await?.with_context(|| { + format!("nothing is stored at {path}; swarm-secret-publish on authelia's host writes it") + })?; + let trimmed = stored.trim(); + if trimmed.is_empty() { + anyhow::bail!("{path} holds an empty secret"); + } + Ok(trimmed.to_owned()) +} + +/// Where a re-read of the secret comes from. +pub trait Source: Send + Sync + 'static { + /// The secret as the store holds it now. + fn read(&self) -> impl Future> + Send; +} + +/// The secret store, logged in to per read: a re-read can come long after +/// the start's token has expired. +pub struct Store { + settings: Settings, + cert_role: String, + path: String, +} + +impl Store { + /// The secret at `path`, read as `cert_role` from the store at `settings`. + pub fn new(settings: Settings, cert_role: String, path: String) -> Self { + Self { + settings, + cert_role, + path, + } + } +} + +impl Source for Store { + async fn read(&self) -> Result { + let store = swarm_secret_client::SecretStore::connect( + &self.settings, + &self.cert_role, + DEFAULT_CERT_MOUNT, + ) + .await + .context("logging in to the secret store")?; + read(&store, &self.path).await + } +} + +/// A re-read in progress, shared by every request waiting on it. +type Reread = Shared>; + +/// The secret in use, and the re-read that replaces it. No `Debug`: it holds +/// the secret. +pub struct OidcSecret { + source: Arc, + current: Arc>, + reread: Mutex>, +} + +impl OidcSecret { + /// `initial`, re-read from `source` when the `IdP` refuses it. + pub fn new(source: S, initial: String) -> Self { + Self { + source: Arc::new(source), + current: Arc::new(RwLock::new(initial)), + reread: Mutex::new(None), + } + } + + fn current(&self) -> String { + self.current.read().expect("unpoisoned").clone() + } + + /// The caller `attempt` names, introspecting with the current secret. + /// When the `IdP` refuses that, the secret is re-read and, if it changed + /// and `deadline` has not passed, `attempt` runs once more with the new + /// one. `attempt` bounds itself by the same `deadline`. + /// + /// # Errors + /// When an attempt could not be made at all; the caller denies. + pub async fn introspect(&self, deadline: Instant, attempt: F) -> Result> + where + F: Fn(String) -> Fut, + Fut: Future>, + { + let used = self.current(); + if let Answer::Caller(caller) = attempt(used.clone()).await? { + return Ok(caller); + } + if tokio::time::timeout_at(deadline, self.reread()) + .await + .is_err() + { + tracing::warn!( + "re-reading the OIDC client secret did not finish inside the introspection budget; \ + denying, and the next request introspects with what it reads" + ); + return Ok(None); + } + let fresh = self.current(); + if fresh == used { + tracing::warn!("the stored OIDC client secret is the one the IdP refused; denying"); + return Ok(None); + } + tracing::info!("re-read a changed OIDC client secret; introspecting again"); + match attempt(fresh).await? { + Answer::Caller(caller) => Ok(caller), + Answer::Refused => Ok(None), + } + } + + /// The running re-read, or a new one when none is running. Spawned, so it + /// finishes even when every request waiting on it has given up. + fn reread(&self) -> Reread { + let mut slot = self.reread.lock().expect("unpoisoned"); + if let Some((handle, running)) = slot.as_ref() + && !handle.is_finished() + { + return running.clone(); + } + let source = Arc::clone(&self.source); + let current = Arc::clone(&self.current); + let task = tokio::spawn(async move { + match source.read().await { + Ok(secret) => *current.write().expect("unpoisoned") = secret, + Err(e) => tracing::warn!( + error = format!("{e:#}"), + "could not re-read the OIDC client secret; keeping the one in use" + ), + } + }); + let handle = task.abort_handle(); + let shared = task.map(|_| ()).boxed().shared(); + *slot = Some((handle, shared.clone())); + shared + } +} + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::time::Duration; + + use super::*; + use crate::introspect::INTROSPECTION_TIMEOUT; + use crate::keystore::tests::Fake as FakeKeys; + + /// A store holding `secret`, or failing, that takes `delay` to answer. + struct Fake { + secret: Option<&'static str>, + delay: Duration, + reads: Arc, + } + + impl Fake { + fn holding(secret: &'static str) -> Self { + Self { + secret: Some(secret), + delay: Duration::ZERO, + reads: Arc::default(), + } + } + } + + impl Source for Fake { + async fn read(&self) -> Result { + self.reads.fetch_add(1, Ordering::SeqCst); + tokio::time::sleep(self.delay).await; + self.secret.map(str::to_owned).context("store unreachable") + } + } + + /// An `IdP` that admits as `hive-alpha` only when introspected with + /// `registered`, and refuses every other secret, counting its calls. + fn idp<'a>( + registered: &'static str, + calls: &'a AtomicUsize, + ) -> impl Fn(String) -> std::future::Ready> + 'a { + move |secret| { + calls.fetch_add(1, Ordering::SeqCst); + std::future::ready(Ok(if secret == registered { + Answer::Caller(Some("hive-alpha".to_owned())) + } else { + Answer::Refused + })) + } + } + + fn budget() -> Instant { + Instant::now() + INTROSPECTION_TIMEOUT + } + + #[test] + fn the_path_is_the_one_the_publisher_writes() { + assert_eq!( + path("swarm-nats").expect("path"), + "swarm/services/swarm-nats/oidc/client" + ); + assert!(path("../swarm-grafana").is_err()); + } + + #[tokio::test] + async fn the_stored_secret_is_trimmed_and_an_empty_or_absent_one_refused() { + let at = "swarm/services/swarm-nats/oidc/client"; + let keys = FakeKeys::holding(&[(at, "s3cret\n")]); + assert_eq!(read(&keys, at).await.expect("stored"), "s3cret"); + keys.put(at, " \n"); + assert!(read(&keys, at).await.is_err()); + assert!(read(&FakeKeys::default(), at).await.is_err()); + } + + #[tokio::test] + async fn an_answered_introspection_reads_nothing() { + let store = Fake::holding("new"); + let reads = Arc::clone(&store.reads); + let secret = OidcSecret::new(store, "old".to_owned()); + let calls = AtomicUsize::new(0); + let caller = secret + .introspect(budget(), idp("old", &calls)) + .await + .expect("answered"); + assert_eq!(caller.as_deref(), Some("hive-alpha")); + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert_eq!(reads.load(Ordering::SeqCst), 0); + } + + /// The rotation the re-read exists for: the `IdP` refuses the secret held + /// since start, the store holds the new one, and the same request is + /// admitted with it. + #[tokio::test] + async fn a_refused_secret_is_re_read_and_the_request_retried_once() { + let store = Fake::holding("new"); + let reads = Arc::clone(&store.reads); + let secret = OidcSecret::new(store, "old".to_owned()); + let calls = AtomicUsize::new(0); + let caller = secret + .introspect(budget(), idp("new", &calls)) + .await + .expect("answered"); + assert_eq!(caller.as_deref(), Some("hive-alpha")); + assert_eq!(calls.load(Ordering::SeqCst), 2); + assert_eq!(reads.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn a_refusal_after_the_re_read_is_a_denial_with_no_third_attempt() { + let store = Fake::holding("newer"); + let reads = Arc::clone(&store.reads); + let secret = OidcSecret::new(store, "old".to_owned()); + let calls = AtomicUsize::new(0); + let caller = secret + .introspect(budget(), idp("unknown", &calls)) + .await + .expect("answered"); + assert_eq!(caller, None); + assert_eq!(calls.load(Ordering::SeqCst), 2); + assert_eq!(reads.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn an_unchanged_secret_is_not_retried() { + let secret = OidcSecret::new(Fake::holding("old"), "old".to_owned()); + let calls = AtomicUsize::new(0); + let caller = secret + .introspect(budget(), idp("unknown", &calls)) + .await + .expect("answered"); + assert_eq!(caller, None); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn a_failed_re_read_denies_and_keeps_the_secret_in_use() { + let store = Fake { + secret: None, + delay: Duration::ZERO, + reads: Arc::default(), + }; + let secret = OidcSecret::new(store, "old".to_owned()); + let calls = AtomicUsize::new(0); + let caller = secret + .introspect(budget(), idp("new", &calls)) + .await + .expect("answered"); + assert_eq!(caller, None); + assert_eq!(calls.load(Ordering::SeqCst), 1); + assert_eq!(secret.current(), "old"); + } + + /// A store slower than the budget: the request is denied at its + /// deadline, the re-read finishes behind it, and the next request is + /// admitted on its first attempt without reading again. + #[tokio::test(start_paused = true)] + async fn a_re_read_past_the_budget_serves_the_next_request() { + let store = Fake { + secret: Some("new"), + delay: Duration::from_secs(3), + reads: Arc::default(), + }; + let reads = Arc::clone(&store.reads); + let secret = OidcSecret::new(store, "old".to_owned()); + let calls = AtomicUsize::new(0); + + let started = Instant::now(); + let deadline = started + INTROSPECTION_TIMEOUT; + let first = secret + .introspect(deadline, idp("new", &calls)) + .await + .expect("answered"); + assert_eq!(first, None); + assert_eq!(Instant::now(), deadline); + assert_eq!(calls.load(Ordering::SeqCst), 1); + + tokio::time::sleep(Duration::from_secs(2)).await; + let next = secret + .introspect(budget(), idp("new", &calls)) + .await + .expect("answered"); + assert_eq!(next.as_deref(), Some("hive-alpha")); + assert_eq!(calls.load(Ordering::SeqCst), 2); + assert_eq!(reads.load(Ordering::SeqCst), 1); + } + + /// A refusal while a re-read is still running waits on that one, and is + /// admitted when it lands inside this request's budget. + #[tokio::test(start_paused = true)] + async fn a_refusal_during_a_running_re_read_joins_it() { + let store = Fake { + secret: Some("new"), + delay: Duration::from_secs(2), + reads: Arc::default(), + }; + let reads = Arc::clone(&store.reads); + let secret = OidcSecret::new(store, "old".to_owned()); + let calls = AtomicUsize::new(0); + + let first = secret + .introspect(budget(), idp("new", &calls)) + .await + .expect("answered"); + assert_eq!(first, None); + + let second = secret + .introspect(budget(), idp("new", &calls)) + .await + .expect("answered"); + assert_eq!(second.as_deref(), Some("hive-alpha")); + assert_eq!(calls.load(Ordering::SeqCst), 3); + assert_eq!(reads.load(Ordering::SeqCst), 1); + } +}