diff --git a/nix/host-modules/swarm-nats.nix b/nix/host-modules/swarm-nats.nix index 242dd1a4..519b1bbb 100644 --- a/nix/host-modules/swarm-nats.nix +++ b/nix/host-modules/swarm-nats.nix @@ -363,6 +363,11 @@ in "--issuer-seed-file \${CREDENTIALS_DIRECTORY}/issuer.seed" "--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 + # own `accounts` block, so this and the block above have to + # be the same string — which is why both come from one let. + "--account ${lib.escapeShellArg clientAccount}" "--introspection-url ${lib.escapeShellArg introspectionUrl}" ]; # Every credential arrives by `LoadCredential` and is named diff --git a/swarm-nats-auth/Cargo.toml b/swarm-nats-auth/Cargo.toml index e9610f82..6677312a 100644 --- a/swarm-nats-auth/Cargo.toml +++ b/swarm-nats-auth/Cargo.toml @@ -39,15 +39,19 @@ futures = "0.3" # is not, and hand-rolling a key format on an auth path is how you get a # CRC bug nobody reviews. nkeys = "0.4" -# NATS JWT claim types + signing. The reply this responder sends is a *signed -# user JWT*, whose `jti` is base32(sha256(claims)) and whose header must say -# `ed25519-nkey` - format details with no feedback loop until the server -# rejects the token. Its own deps (data-encoding, nkeys, serde, serde_json, -# sha2) are already in the tree, so this costs no new transitive weight. -nats-jwt = "0.3" -# The jti digest. Already in the tree via nats-jwt; named directly because -# this crate computes one itself for the response wrapper. +# The jti digest: base32hex(sha256(claims)) over every JWT this crate signs. sha2 = "0.10" +[dev-dependencies] +# A TEST ORACLE, not part of the production path. Neither JWT this crate emits +# is expressible through it - `Claims` has no `aud`, which the response wrapper +# needs (the server id) and the user token needs (the account name), and +# `Token::new_user` always sets `issuer_account`, which a non-operator server +# rejects outright. So both are hand-built, and this crate is what the encoder +# is checked *against*: `respond::tests::hand_built_matches_the_reference` +# builds a user token both ways and requires byte equality, on the one shape +# nats-jwt does model. +nats-jwt = "0.3" + [lints] workspace = true diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index 3f68131d..94217902 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -63,6 +63,13 @@ struct Args { #[arg(long, default_value = "swarm-nats")] client_id: String, + /// Account an admitted client is placed in. Must name an entry in the + /// server's own `accounts` block — in server-config mode the account is + /// resolved by *name*, so a value the server does not know is a grant it + /// refuses. The module passes its `clientAccount`; the default mirrors it. + #[arg(long, default_value = "APP")] + account: String, + /// Path to this responder's own OIDC client secret. #[arg(long)] client_secret_file: PathBuf, @@ -162,12 +169,7 @@ async fn main() -> anyhow::Result<()> { continue; }; let token = if granted { - respond::grant( - &issuer, - &issuer.public_key(), - &req.server_id.id, - &req.user_nkey, - ) + respond::grant(&issuer, &args.account, &req.server_id.id, &req.user_nkey) } else { respond::deny(&issuer, &req.server_id.id, &req.user_nkey) }; diff --git a/swarm-nats-auth/src/respond.rs b/swarm-nats-auth/src/respond.rs index bd01ee2e..8234991f 100644 --- a/swarm-nats-auth/src/respond.rs +++ b/swarm-nats-auth/src/respond.rs @@ -1,16 +1,30 @@ //! Minting the reply the server expects on `msg.reply`. //! -//! Two JWTs are involved and only one of them can be produced by a crate: +//! Two JWTs are involved and **neither** can be produced by `nats-jwt`, for +//! the same underlying reason: its `Claims` struct has no `aud`, and both of +//! these need one. //! -//! * the **user JWT** — `nats-jwt` builds and signs it; -//! * the **`authorization_response`** wrapper that carries it — `nats-jwt` -//! cannot. Its `IntoNatsClaims` trait returns the closed enum -//! `NatsClaims { User, Account }`, so a third claim type is inexpressible -//! through it, and its `Claims` struct has no `aud`, which this response -//! requires (`aud` is the server id, so a reply cannot be replayed at a -//! different server in the cluster). +//! * the **`authorization_response`** wrapper — `aud` is the asking server's +//! id, so a captured reply cannot be replayed at a different server in the +//! cluster. Its `IntoNatsClaims` trait also returns the closed enum +//! `NatsClaims { User, Account }`, so the claim type is inexpressible +//! through it twice over. +//! * the **user JWT** it carries — in **server-config mode** (which is what +//! `nix/host-modules/swarm-nats.nix` renders: `accounts { AUTH, APP }`, +//! no operator), `aud` is the *account name* the admitted client lands in, +//! and `issuer_account` must be **absent**. `nats_jwt::Token::new_user` +//! always sets `issuer_account` and can express no `aud` at all. //! -//! So the wrapper is hand-built here, following the algorithm read out of +//! 🩸 That second one was found by running the thing, not by reading it. The +//! responder answered `granted=true` and the server still refused the client, +//! logging `Error non operator mode account "AUTH": attempted to use +//! issuer_account`. Every unit test passed throughout: they assert the shape +//! this module *intends*, and the field that broke it was one nobody had +//! reason to assert was missing. `issuer_account` is an operator-mode field — +//! it names the account when a *signing* key rather than the account identity +//! key signed the token — and in config mode its mere presence is fatal. +//! +//! So both are hand-built here, following the algorithm read out of //! `nats-jwt`'s own `sign()`: serialise the claims with `jti` **empty**, //! sha256 that, `BASE32HEX_NOPAD` the digest into `jti`, re-serialise, and //! sign `"."` with the account key. @@ -21,8 +35,10 @@ //! is also why [`tests::hand_built_matches_the_reference`] exists: it builds //! a *user* token both ways and asserts byte equality, so this encoder is //! checked against `nats-jwt` on the shape that crate does model. Trusting it -//! on the shape `nats-jwt` does **not** model has to rest on something better -//! than my reading of its source. +//! on the shapes `nats-jwt` does **not** model has to rest on something +//! better than my reading of its source. `nats-jwt` is therefore a +//! **dev-dependency**: it is this module's test oracle, not part of the path +//! that runs in production. use data_encoding::{BASE32HEX_NOPAD, BASE64URL_NOPAD}; use nkeys::KeyPair; @@ -133,10 +149,59 @@ fn now_secs() -> i64 { .expect("seconds since the epoch fit in an i64") } -/// Grant: mint a user JWT in `account` for `user_nkey` and wrap it. +/// The `nats` object of a **user** JWT, server-config mode. +/// +/// Field-for-field what `nats-jwt` emits **minus `issuer_account`**, whose +/// presence a non-operator server rejects outright (see the module docs). The +/// account is named by the enclosing claims' `aud` instead. +/// +/// The `-1`s are NATS' "unlimited": no cap on subscriptions, payload size or +/// total data. Limits belong on the account, which the module owns; putting +/// them here would mean every issued token silently disagreed with the +/// deployed policy the moment that policy changed. +#[derive(Serialize)] +struct UserNats { + #[serde(rename = "type")] + kind: &'static str, + subs: i64, + data: i64, + payload: i64, + bearer_token: bool, + version: i64, +} + +/// Mint the user JWT an admitted client presents. +/// +/// `account` is the account **name** from the server's `accounts` block (the +/// module's `clientAccount`), not a public key — in config mode the server +/// resolves `aud` against its own config rather than against a key. +fn user_jwt(now: i64, issuer: &KeyPair, account: &str, user_nkey: &str) -> String { + sign( + Claims { + iat: now, + iss: issuer.public_key(), + jti: String::new(), + sub: user_nkey.to_owned(), + name: user_nkey.to_owned(), + aud: Some(account.to_owned()), + nats: UserNats { + kind: "user", + subs: -1, + data: -1, + payload: -1, + bearer_token: false, + version: 2, + }, + }, + issuer, + ) +} + +/// Grant: mint a user JWT placing the client in `account` and wrap it. pub fn grant(issuer: &KeyPair, account: &str, server_id: &str, user_nkey: &str) -> String { - let user_jwt = nats_jwt::Token::new_user(account, user_nkey).sign(issuer); - response(now_secs(), issuer, server_id, user_nkey, Ok(user_jwt)) + let now = now_secs(); + let jwt = user_jwt(now, issuer, account, user_nkey); + response(now, issuer, server_id, user_nkey, Ok(jwt)) } /// Deny, with a reason the server can log. @@ -160,6 +225,12 @@ mod tests { /// Mirror of what `nats-jwt` puts in a user token, **in its field order**. /// + /// Deliberately *not* [`super::UserNats`]: this one carries + /// `issuer_account`, because the reference does. Production must not, and + /// keeping the two structs separate is what lets one test pin the encoder + /// against `nats-jwt` while another asserts production diverges from it in + /// exactly one field. + /// /// 🩸 The first version of the equality test round-tripped `nats` through /// `serde_json::Value` and failed — not because the encoder was wrong, but /// because `Value` is a `BTreeMap`, so re-serialising sorted the keys and @@ -168,7 +239,7 @@ mod tests { /// fields between hashing and signing is a live hazard here, not merely a /// test artefact. #[derive(serde::Serialize)] - struct UserNats { + struct ReferenceUserNats { #[serde(rename = "type")] kind: &'static str, issuer_account: String, @@ -216,7 +287,7 @@ mod tests { sub: user.public_key(), name: user.public_key(), aud: None, - nats: UserNats { + nats: ReferenceUserNats { kind: "user", issuer_account: account.public_key(), subs: -1, @@ -250,12 +321,7 @@ mod tests { #[test] fn a_grant_carries_a_user_jwt_and_no_error() { let account = KeyPair::new_account(); - let claims = decode_claims(&grant( - &account, - &account.public_key(), - "NSERVER", - "UCLIENT", - )); + let claims = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT")); assert!(claims["nats"]["error"].is_null()); assert!( claims["nats"]["jwt"] @@ -264,6 +330,37 @@ mod tests { ); } + /// 🩸 THE REGRESSION TEST FOR THE BUG THE UNIT TESTS COULD NOT SEE. + /// + /// The first version of this module minted the user JWT with + /// `nats_jwt::Token::new_user`, which always sets `issuer_account`. Every + /// test passed, the responder logged `granted=true`, and the server still + /// refused every client: `Error non operator mode account "AUTH": + /// attempted to use issuer_account`. + /// + /// The lesson is in what this asserts — an **absence**. The suite was full + /// of assertions about fields that had to be *present*, and a field that + /// must not exist is invisible to all of them. It took running a real + /// server to find, so the cheap check goes here to keep it found. + #[test] + fn the_issued_user_jwt_names_its_account_by_aud_and_sets_no_issuer_account() { + let account = KeyPair::new_account(); + let wrapper = decode_claims(&grant(&account, "APP", "NSERVER", "UCLIENT")); + let user = decode_claims(wrapper["nats"]["jwt"].as_str().expect("a user jwt")); + + // In server-config mode the account is named by `aud`... + assert_eq!(user["aud"], "APP", "the user jwt must name its account"); + // ...and `issuer_account` is an operator-mode field whose mere + // presence makes the server reject the client. + assert!( + user["nats"]["issuer_account"].is_null(), + "issuer_account is fatal in non-operator mode" + ); + assert_eq!(user["nats"]["type"], "user"); + assert_eq!(user["sub"], "UCLIENT"); + assert_eq!(user["iss"], account.public_key()); + } + fn decode_claims(jwt: &str) -> serde_json::Value { let body = jwt.split('.').nth(1).expect("claims segment"); serde_json::from_slice(&BASE64URL_NOPAD.decode(body.as_bytes()).expect("base64url"))