diff --git a/doc/bin/relay/auth.md b/doc/bin/relay/auth.md index a8125ce6eb..7b4fac74d2 100644 --- a/doc/bin/relay/auth.md +++ b/doc/bin/relay/auth.md @@ -58,8 +58,8 @@ ingest here). A 2xx with a grant admits. A 401 or 403 refuses. Anything else at connect, a timeout, a 5xx, or an unparseable body, refuses and logs an error; nothing is admitted because the server was down. A grant that names nothing refuses, and one with `revalidate` but no `expires` is refused -as invalid. A grant already less than five seconds past `expires` is accepted -for the remainder of that clock-skew window; future expiries are unchanged. +as invalid, as is one whose `expires` is already at or before now: expiry is +exact, with no grace for clock skew, so keep the auth server's clock in sync. The client and relay snapshot each accepted grant's deadline on a monotonic clock, so later polls, outages, and wall-clock adjustments do not restart it. @@ -165,7 +165,7 @@ after which it can never sign a broader token. | `root` | Base path. Optional. | | `publish` | Patterns the bearer may publish under `root`. `**` means everything; omitted means no publishing. | | `subscribe` | Patterns the bearer may subscribe to under `root`. Same rules. | -| `exp`, `iat`, `nbf` | Expiry, issue time, and not-before. `exp` is enforced for the whole session, not just at connect, and a token is refused before its `nbf`. | +| `exp`, `iat`, `nbf` | Expiry, issue time, and not-before. `exp` is enforced for the whole session, not just at connect, and a token is refused from its `exp` on and before its `nbf`. | | `iss`, `sub`, `jti` | Read and ignored. | Any other claim refuses the token with its name, `aud` included: an unknown diff --git a/doc/lib/rs/moq-auth.md b/doc/lib/rs/moq-auth.md index 5f0de2a969..862868356c 100644 --- a/doc/lib/rs/moq-auth.md +++ b/doc/lib/rs/moq-auth.md @@ -13,8 +13,8 @@ Everything a party needs to ask for or answer an authorization on answers the relay, in a service that mints tokens for clients, or in your own accept loop that decides in process. -- **Request and grant**: `Request` is the JSON a relay POSTs per session event (`connect`, `revalidate`, `end`) with everything it knows: id, node, transport, addresses, SNI and ALPN, the raw path and query, the moq-transport SETUP `token` (its Token Type and bytes, base64url on the wire), the declared role, and the verified certificate facts. `Grant` is the answer: `publish` and `subscribe` pattern unions, an optional `root` alias, `mounts` that read a subtree from elsewhere on the origin, `expires`, `revalidate`, `tier`, and `peer`, which marks the session as a cluster peer so the routes it announces report `Source::Peer`. `Grant::validate` refuses a grant that names nothing, asks to be revalidated without a bound or at no interval, or has already expired, with a few seconds of clock skew on `expires`. -- **Lease**: `lease::Producer` and `lease::Consumer` are the handle a session holds for its grant. The consumer reads the current grant, waits for a change, and learns why the lease ended; the producer updates and revokes. Either side's terminal call returns the reason the lease actually ended with, so whichever got there first is what both report. `Consumer::fixed` is a grant nobody drives. `Consumer::revalidate` nudges a re-check now and the producer observes it via `poll_revalidate` or `revalidate_requested`, which is how the relay's session push lands. Whoever runs the accept loop builds the producer, so an embedder decides in process with no trait and no HTTP. Enforcing `expires` is the holder's job; the `Client` driver also revokes at expiry so its `end` event goes out. `Grant::deadline()` (feature `tokio`, also enabled by `client` and `serve`) snapshots expiry on Tokio's clock. Call it once per accepted grant and retain the deadline: future expiries are unchanged, and one already less than five seconds late gets the remainder of that skew window. +- **Request and grant**: `Request` is the JSON a relay POSTs per session event (`connect`, `revalidate`, `end`) with everything it knows: id, node, transport, addresses, SNI and ALPN, the raw path and query, the moq-transport SETUP `token` (its Token Type and bytes, base64url on the wire), the declared role, and the verified certificate facts. `Grant` is the answer: `publish` and `subscribe` pattern unions, an optional `root` alias, `mounts` that read a subtree from elsewhere on the origin, `expires`, `revalidate`, `tier`, and `peer`, which marks the session as a cluster peer so the routes it announces report `Source::Peer`. `Grant::validate` refuses a grant that names nothing, asks to be revalidated without a bound or at no interval, or has already expired; expiry is exact, with no grace for clock skew. +- **Lease**: `lease::Producer` and `lease::Consumer` are the handle a session holds for its grant. The consumer reads the current grant, waits for a change, and learns why the lease ended; the producer updates and revokes. Either side's terminal call returns the reason the lease actually ended with, so whichever got there first is what both report. `Consumer::fixed` is a grant nobody drives. `Consumer::revalidate` nudges a re-check now and the producer observes it via `poll_revalidate` or `revalidate_requested`, which is how the relay's session push lands. Whoever runs the accept loop builds the producer, so an embedder decides in process with no trait and no HTTP. Enforcing `expires` is the holder's job; the `Client` driver also revokes at expiry so its `end` event goes out. `Grant::deadline()` (feature `tokio`, also enabled by `client` and `serve`) snapshots expiry on Tokio's clock. Call it once per accepted grant and retain the deadline; an expiry already past is now. - **Client**: `Client::new(url, tls)` and `Client::connect(request)` drive a lease against an auth server over `https://`, `unix://`, or loopback `http://`: revalidate on cadence with jittered backoff through an outage until `expires`, revoke on a 401/403 or an invalid grant, and POST `end` with the reason, duration, and byte totals the session reported through `lease::Consumer::close` when it ended. Dropping the consumer reports zero bytes. `end.reason` is `dropped`, `expired`, `refused`, `invalid`, or the session's own classification. - **Server**: `serve::Policy` and `serve::Server` (feature `serve`) are the reference auth server behind `moq auth serve`: a JWT from the `jwt` query or a type-0 SETUP token, verified against a key file or a `{kid}.jwk` directory (a token of another type, or both at once, is refused), an explicit grant for verified certificates, the anonymous permissions, a tier, the revalidation cadence, a default `expires`, and live session caps per token and per remote address. A token is authorized at the dialed path with `Claims::authorize`; residuals become the grant. `Server::router` is an axum `POST /` you can mount in your own service. - **Keys**: generate HS256/384/512, RS256/384/512, PS256/384/512, ES256/384, or EdDSA keys as JWKs, with a `kid` for rotation and an optional immutable scope that caps every token the key signs. diff --git a/rs/moq-auth/src/client.rs b/rs/moq-auth/src/client.rs index 30dc8e991e..935e22c784 100644 --- a/rs/moq-auth/src/client.rs +++ b/rs/moq-auth/src/client.rs @@ -493,24 +493,43 @@ mod tests { } #[tokio::test] - async fn a_grant_within_clock_skew_stays_live() { + async fn an_expired_grant_is_refused() { tokio::time::pause(); let mut grant = Grant::new(patterns(&["**"]), Patterns::new()); grant.expires = Some(SystemTime::now() - Duration::from_secs(1)); let client = clock_server(Log::default(), grant, false).await; + assert!(matches!(client.connect(request()).await, Err(Error::GrantExpired))); + } + + #[tokio::test] + async fn a_grant_closes_at_its_expiry() { + tokio::time::pause(); + // Whole seconds, as the grant crosses the wire, so the client sees this exact instant. + // An hour out, so a slow runner cannot expire it before `connect` answers. + let now = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap(); + let expires = SystemTime::UNIX_EPOCH + Duration::from_secs(now.as_secs() + 3600); + let mut grant = Grant::new(patterns(&["**"]), Patterns::new()); + grant.expires = Some(expires); + let client = clock_server(Log::default(), grant, false).await; + + // The client reads both clocks somewhere inside `connect`, so bracket it: the + // bounds hold however long it takes. + let (wall, tick) = (SystemTime::now(), tokio::time::Instant::now()); let consumer = client.connect(request()).await.unwrap(); + let earliest = tick + expires.duration_since(SystemTime::now()).unwrap(); + let latest = tokio::time::Instant::now() + expires.duration_since(wall).unwrap(); - tokio::time::sleep(Duration::from_millis(500)).await; + // A millisecond either side for Tokio's timer resolution. + let tolerance = Duration::from_millis(1); assert!( - tokio::time::timeout(Duration::from_millis(100), consumer.closed()) + tokio::time::timeout_at(earliest - tolerance, consumer.closed()) .await .is_err(), - "still live inside the skew window" + "live until its expiry" ); - - let reason = tokio::time::timeout(crate::grant::CLOCK_SKEW + Duration::from_secs(1), consumer.closed()) + let reason = tokio::time::timeout_at(latest + tolerance, consumer.closed()) .await - .expect("expired once the skew window ended"); + .expect("closed at its expiry, not later"); assert_eq!(reason, Reason::Expired); } diff --git a/rs/moq-auth/src/grant.rs b/rs/moq-auth/src/grant.rs index 0558b4e6dd..dd774fd52e 100644 --- a/rs/moq-auth/src/grant.rs +++ b/rs/moq-auth/src/grant.rs @@ -4,19 +4,6 @@ use serde_with::{DurationSeconds, TimestampSeconds, serde_as}; use std::collections::BTreeMap; use std::time::{Duration, SystemTime}; -/// A grant that expired this recently still stands: the auth server's clock may run behind. -pub(crate) const CLOCK_SKEW: Duration = Duration::from_secs(5); - -/// How long until `at`. A deadline up to [`CLOCK_SKEW`] in the past still has the -/// remaining window; anything older is zero. Future deadlines are unchanged, so a -/// grant that expires in ten seconds still expires in ten seconds. -fn until(at: SystemTime) -> Duration { - match at.duration_since(SystemTime::now()) { - Ok(remaining) => remaining, - Err(late) => CLOCK_SKEW.saturating_sub(late.duration()), - } -} - /// What a session may do, as the auth server answered. /// /// A 2xx carrying one of these admits; anything else refuses. A grant that names @@ -74,15 +61,15 @@ impl Grant { } } - /// Snapshot the expiry on Tokio's clock, allowing five seconds of past clock skew. + /// Snapshot the expiry on Tokio's clock; one already past is now. #[cfg(feature = "tokio")] pub fn deadline(&self) -> Option { - self.expires.map(|at| tokio::time::Instant::now() + until(at)) + let remaining = self.expires?.duration_since(SystemTime::now()).unwrap_or_default(); + Some(tokio::time::Instant::now() + remaining) } /// Refuse a grant that admits nothing, asks to be revalidated without a bound or - /// at no interval, or has already expired. A few seconds of clock skew are - /// tolerated so an auth server whose clock runs behind still admits. + /// at no interval, or has already expired. pub fn validate(&self) -> crate::Result<()> { if self.publish.is_empty() && self.subscribe.is_empty() { return Err(crate::Error::UselessGrant); @@ -94,7 +81,7 @@ impl Grant { if self.revalidate.is_some_and(|cadence| cadence.is_zero()) { return Err(crate::Error::ZeroRevalidate); } - if self.expires.is_some_and(|expires| until(expires).is_zero()) { + if self.expires.is_some_and(|expires| expires <= SystemTime::now()) { return Err(crate::Error::GrantExpired); } Ok(()) @@ -175,10 +162,11 @@ mod tests { grant.revalidate = Some(Duration::from_secs(1)); assert!(matches!(grant.validate(), Err(crate::Error::UnboundedRevalidate))); - grant.expires = Some(SystemTime::now() - Duration::from_secs(1)); - grant.validate().unwrap(); + // Exact: no grace for an auth server whose clock runs behind. + grant.expires = Some(SystemTime::now()); + assert!(matches!(grant.validate(), Err(crate::Error::GrantExpired))); - grant.expires = Some(SystemTime::now() - CLOCK_SKEW - Duration::from_secs(1)); + grant.expires = Some(SystemTime::now() - Duration::from_secs(1)); assert!(matches!(grant.validate(), Err(crate::Error::GrantExpired))); grant.expires = Some(SystemTime::now() + Duration::from_secs(60)); @@ -187,4 +175,24 @@ mod tests { grant.revalidate = Some(Duration::ZERO); assert!(matches!(grant.validate(), Err(crate::Error::ZeroRevalidate))); } + + #[cfg(feature = "tokio")] + #[tokio::test(start_paused = true)] + async fn deadline_is_the_exact_expiry() { + let start = tokio::time::Instant::now(); + let mut grant = Grant::new(patterns(&["**"]), Patterns::new()); + assert_eq!(grant.deadline(), None); + + grant.expires = Some(SystemTime::now() + Duration::from_secs(10)); + let deadline = grant.deadline().unwrap(); + assert!(deadline <= start + Duration::from_secs(10), "not later than the expiry"); + assert!(deadline > start + Duration::from_secs(9)); + + grant.expires = Some(SystemTime::now() - Duration::from_secs(1)); + assert_eq!( + grant.deadline(), + Some(start), + "a past expiry is now, not a grace window" + ); + } } diff --git a/rs/moq-auth/src/key.rs b/rs/moq-auth/src/key.rs index 903e32e427..1d9f766cb9 100644 --- a/rs/moq-auth/src/key.rs +++ b/rs/moq-auth/src/key.rs @@ -544,18 +544,7 @@ impl Key { let token = jsonwebtoken::decode::(token, decode, &validation)?; - let now = std::time::SystemTime::now(); - if let Some(exp) = token.claims.expires - && exp < now - { - return Err(crate::Error::TokenExpired); - } - if let Some(nbf) = token.claims.not_before - && nbf > now - { - return Err(crate::Error::TokenNotYetValid); - } - + validate_times(&token.claims, std::time::SystemTime::now())?; token.claims.validate()?; self.validate_scope(&token.claims)?; @@ -611,6 +600,17 @@ impl Key { } } +/// Refuse claims expired at `now` (`exp <= now`) or not yet valid (`nbf > now`), as `jose` does. +fn validate_times(claims: &Claims, now: std::time::SystemTime) -> crate::Result<()> { + if claims.expires.is_some_and(|exp| exp <= now) { + return Err(crate::Error::TokenExpired); + } + if claims.not_before.is_some_and(|nbf| nbf > now) { + return Err(crate::Error::TokenNotYetValid); + } + Ok(()) +} + /// Serialize bytes as base64url without padding fn serialize_base64url(bytes: &[u8], serializer: S) -> Result where @@ -1034,6 +1034,31 @@ mod tests { assert!(matches!(key.verify(&at(3600)), Err(crate::Error::TokenNotYetValid))); } + /// `exp` is refused at the instant itself and `nbf` accepted at it, matching `jose`. + #[test] + fn validate_times_at_the_boundary() { + let now = SystemTime::UNIX_EPOCH + Duration::from_secs(1_000); + let second = Duration::from_secs(1); + + let at = |expires: Option, not_before: Option| { + let mut claims = create_test_claims(); + claims.expires = expires; + claims.not_before = not_before; + validate_times(&claims, now) + }; + + assert!(at(Some(now + second), None).is_ok()); + assert!(matches!(at(Some(now), None), Err(crate::Error::TokenExpired))); + assert!(matches!(at(Some(now - second), None), Err(crate::Error::TokenExpired))); + + assert!(at(None, Some(now)).is_ok()); + assert!(at(None, Some(now - second)).is_ok()); + assert!(matches!( + at(None, Some(now + second)), + Err(crate::Error::TokenNotYetValid) + )); + } + #[test] fn test_key_verify_expired_token() { let key = create_test_key(); diff --git a/rs/moq-relay/src/auth.rs b/rs/moq-relay/src/auth.rs index bd610e90b6..6616f88e79 100644 --- a/rs/moq-relay/src/auth.rs +++ b/rs/moq-relay/src/auth.rs @@ -958,27 +958,19 @@ mod tests { assert_eq!(reason, lease::Reason::Expired); } - #[tokio::test] - async fn a_grant_within_clock_skew_stays_live() { - tokio::time::pause(); + #[tokio::test(start_paused = true)] + async fn an_expired_grant_ends_at_once() { use std::time::Duration; let mut grant = Grant::new(patterns(&["**"]), patterns(&["**"])); grant.expires = Some(SystemTime::now() - Duration::from_secs(1)); - grant.validate().expect("accepted inside the skew window"); + assert!(matches!(grant.validate(), Err(moq_auth::Error::GrantExpired))); + + let start = tokio::time::Instant::now(); let mut lease = Lease::new("/room", lease::Consumer::fixed(grant)); - assert!( - tokio::time::timeout(Duration::from_secs(1), lease.ended()) - .await - .is_err(), - "still live inside the skew window" - ); - assert_eq!( - tokio::time::timeout(Duration::from_secs(4), lease.ended()) - .await - .expect("expired once the skew window ended"), - lease::Reason::Expired - ); + assert_eq!(lease.ended().await, lease::Reason::Expired); + // A millisecond for Tokio's timer resolution. + assert!(start.elapsed() <= Duration::from_millis(1), "no grace after expiry"); } #[test]