diff --git a/swarm-nats-auth/src/introspect.rs b/swarm-nats-auth/src/introspect.rs index 5e5ec02c..30aae01c 100644 --- a/swarm-nats-auth/src/introspect.rs +++ b/swarm-nats-auth/src/introspect.rs @@ -1,17 +1,25 @@ -//! Validating a presented bearer token against the `IdP`. +//! Validating a presented bearer token against the `IdP`, and learning who +//! presented it. //! //! RFC 7662 token introspection: POST the token to authelia, which answers -//! `{"active": true|false, ...}`. `active` is the whole verdict — this -//! responder does not second-guess it, and does not inspect scopes, because a -//! second place that decides who may connect is a second place to get it -//! wrong. +//! `{"active": true|false, "client_id": "...", ...}`. `active` is the whole +//! **admission** verdict — this responder does not second-guess it, and does +//! not inspect scopes, because a second place that decides who may connect is +//! a second place to get it wrong. //! //! # Everything that is not an explicit `active: true` is a denial //! //! Network error, timeout, non-2xx, unparseable body, `active: false` — all -//! deny. That is not defensive coding, it is the only shape that is safe: the -//! failure modes of an HTTP call are exactly the conditions under which an -//! attacker would most like this to fall open. +//! deny. Not defensive coding but the only safe shape: the failure modes of +//! an HTTP call are exactly when an attacker would like this to fall open. +//! +//! # `client_id` is modelled, and is not a second admission check +//! +//! `sub`/`scope`/`exp` stay unmodelled: modelling a field implies checking +//! it. `client_id` differs in kind — `active` decides **whether** to admit, +//! `client_id` decides **as whom**, which is what lets a grant be scoped to +//! one hive. Admission stays one decision in one place; the identity is +//! returned rather than a bool so "admitted but unscoped" cannot exist. //! //! # The timeout is not a tuning knob //! @@ -31,32 +39,64 @@ use serde::Deserialize; /// already happened. pub const INTROSPECTION_TIMEOUT: Duration = Duration::from_millis(1500); -/// The one field this responder acts on. Authelia returns more (`sub`, -/// `scope`, `exp`); modelling them would imply we check them. +/// 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)] struct IntrospectionResponse { + /// Whether to admit at all. active: bool, + /// Which client presented the token, i.e. which hive to scope its grant + /// to. Optional on the wire (RFC 7662 makes every field but `active` + /// optional), and its absence is a denial rather than a default — see + /// [`IntrospectionResponse::caller`]. + #[serde(default)] + client_id: Option, } -/// Ask the `IdP` whether `token` is currently valid. +impl IntrospectionResponse { + /// The identity to admit this token as, or `None` for every shape that is + /// not an admission. + /// + /// An inactive token has no identity here even when the body names one: a + /// `client_id` on an expired token says who it *was* minted for, which is + /// not a statement that the bearer may connect. + /// + /// An empty `client_id` is treated as absent. It would otherwise become an + /// empty component in a subject the grant is scoped to, and a scope + /// assembled from a blank is not a narrower permission, it is a different + /// one nobody reviewed. + fn caller(&self) -> Option<&str> { + if !self.active { + return None; + } + self.client_id.as_deref().filter(|id| !id.is_empty()) + } +} + +/// Ask the `IdP` whether `token` is currently valid, and as whom. /// -/// Returns `Ok(true)` **only** on a 2xx whose body says `active: true`. -/// Every other outcome is `Ok(false)` with the reason logged, or `Err` when -/// the call could not be made at all — callers must treat both as a denial. +/// 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. +/// +/// `own_client_id` / `own_client_secret` are *this responder's* credential for +/// the introspection endpoint. They are not the caller's identity and must +/// never be used as one; the caller's is the return value. /// /// The token is never logged, not even truncated: a prefix is enough to /// correlate across logs, and a credential that is 80% redacted is still a /// credential in a journal. -pub async fn is_active( +pub async fn identify_caller( http: &reqwest::Client, url: &str, - client_id: &str, - client_secret: &str, + own_client_id: &str, + own_client_secret: &str, token: &str, -) -> Result { +) -> Result> { let resp = http .post(url) - .basic_auth(client_id, Some(client_secret)) + .basic_auth(own_client_id, Some(own_client_secret)) .form(&[("token", token)]) .timeout(INTROSPECTION_TIMEOUT) .send() @@ -66,16 +106,22 @@ pub async fn is_active( let status = resp.status(); if !status.is_success() { tracing::warn!(%status, "introspection returned non-2xx; denying"); - return Ok(false); + return Ok(None); } let body: IntrospectionResponse = match resp.json().await { Ok(b) => b, Err(e) => { tracing::warn!(error = ?e, "introspection body did not parse; denying"); - return Ok(false); + return Ok(None); } }; - Ok(body.active) + if body.active && body.caller().is_none() { + // Loud on purpose: this is a token the `IdP` says is good, refused by + // us. Without a line here the operator sees a working credential + // rejected for no visible reason. + tracing::warn!("introspection said active but named no client; denying"); + } + Ok(body.caller().map(str::to_owned)) } #[cfg(test)] @@ -95,15 +141,46 @@ mod tests { ); } + fn parse(body: &str) -> IntrospectionResponse { + serde_json::from_str(body).expect("introspection body") + } + #[test] fn only_active_true_deserializes_to_a_grant() { - let yes: IntrospectionResponse = serde_json::from_str(r#"{"active":true}"#).unwrap(); - assert!(yes.active); - let no: IntrospectionResponse = serde_json::from_str(r#"{"active":false}"#).unwrap(); - assert!(!no.active); + assert!(parse(r#"{"active":true,"client_id":"hive-alpha"}"#).active); + assert!(!parse(r#"{"active":false}"#).active); // A body that omits `active` is not a grant with a default - it is a // response we do not understand, and it must fail to parse rather // than deserialize to `false` quietly under some future `#[serde(default)]`. assert!(serde_json::from_str::(r#"{"sub":"someone"}"#).is_err()); } + + #[test] + fn an_active_token_is_admitted_as_the_client_that_holds_it() { + assert_eq!( + parse(r#"{"active":true,"client_id":"hive-alpha","scope":"x"}"#).caller(), + Some("hive-alpha") + ); + } + + #[test] + fn active_without_a_client_id_is_a_denial_not_an_unscoped_grant() { + // The invariant the whole scoping design rests on. If this ever + // returns `Some`, a token the `IdP` could not attribute would be + // admitted with whatever scope the caller assembles from nothing - + // which is how a per-hive permission becomes a swarm-wide one. + assert_eq!(parse(r#"{"active":true}"#).caller(), None); + assert_eq!(parse(r#"{"active":true,"client_id":""}"#).caller(), None); + } + + #[test] + fn an_inactive_token_has_no_identity_even_when_the_body_names_one() { + // Admission is decided by `active` alone. A `client_id` alongside + // `active: false` says who the token *was* for, and reading it as an + // identity would turn an expired credential into a working one. + assert_eq!( + parse(r#"{"active":false,"client_id":"hive-alpha"}"#).caller(), + None + ); + } } diff --git a/swarm-nats-auth/src/main.rs b/swarm-nats-auth/src/main.rs index fa948747..64bd1487 100644 --- a/swarm-nats-auth/src/main.rs +++ b/swarm-nats-auth/src/main.rs @@ -135,8 +135,12 @@ async fn main() -> anyhow::Result<()> { // No token is a denial, not an error: an anonymous connect is a // normal thing for a client to attempt and an abnormal thing to // grant. Introspection is only reached once something was presented. - let granted = match &req.connect_opts.auth_token { - Some(token) => introspect::is_active( + // + // The caller is an identity or nothing — see `introspect`'s module + // docs. There is no "admitted, identity unknown" branch to write here + // because there is no such value to receive. + let caller = match &req.connect_opts.auth_token { + Some(token) => introspect::identify_caller( &http, &args.introspection_url, &args.client_id, @@ -149,14 +153,17 @@ async fn main() -> anyhow::Result<()> { // which an attacker would most like this to fall open. .unwrap_or_else(|e| { tracing::warn!(error = ?e, "introspection failed; denying"); - false + None }), - None => false, + None => None, }; + // The client id is an identifier, not a credential, and it is the + // only thing tying a connection in this log to a hive. tracing::info!( user_nkey = %req.user_nkey, server_id = %req.server_id.id, - granted, + granted = caller.is_some(), + caller = caller.as_deref().unwrap_or("-"), "auth request" ); @@ -168,7 +175,11 @@ async fn main() -> anyhow::Result<()> { tracing::warn!("auth request had no reply subject; dropping"); continue; }; - let token = if granted { + // The grant is still unscoped: knowing *who* connected is what makes + // scoping possible, not what performs it. Narrowing the permissions + // to the caller's own subjects is the next slice, and lands in + // `respond::grant` where the JWT is minted. + let token = if caller.is_some() { respond::grant(&issuer, &args.account, &req.server_id.id, &req.user_nkey) } else { respond::deny(&issuer, &req.server_id.id, &req.user_nkey)