fix(swarm): name the account by aud, never issuer_account
The responder answered granted=true and the server still refused every client: Error non operator mode account "AUTH": attempted to use issuer_account nats_jwt::Token::new_user always sets issuer_account, which is an operator-mode field naming the account when a signing key rather than the account identity key signed the token. In server-config mode - what this module renders - its mere presence is fatal, and the account is named by the claims' aud instead. nats-jwt can express neither aud nor the omission, so the user JWT is now hand-built by the same signer as the response wrapper, and nats-jwt moves to dev-dependencies as the encoder's test oracle. Every unit test passed throughout: they assert fields that must be present, and the defect was a field that must be absent.
This commit is contained in:
atlas
mara
parent
207fc4d2a6
commit
188e27478a
4 changed files with 144 additions and 36 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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 `"<b64url header>.<b64url claims>"` 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"))
|
||||
|
|
|
|||
Loading…
Reference in a new issue