diff --git a/backend/tests/app_guest_jwt_entry.rs b/backend/tests/app_guest_jwt_entry.rs index 84664fb772..9f39b3a863 100644 --- a/backend/tests/app_guest_jwt_entry.rs +++ b/backend/tests/app_guest_jwt_entry.rs @@ -305,8 +305,24 @@ async fn guest_jwt_refusals(db: Pool) -> anyhow::Result<()> { c.app_path = "u/test-user/*".to_string(); let wildcard_app_path = bearer(&c, PRIV1, Algorithm::ES256); - // a token past the length cap is refused before any signature work or caching. - let oversized_token = format!("jwt_guest_{}", "a".repeat(9000)); + // a valid, signed token past the length cap: without the cap it would deserialize into + // GuestJwtClaims (the extra claim ignored) and verify, so this pins the length check. + let mut payload = serde_json::to_value(Claims::valid()).unwrap(); + payload["padding"] = serde_json::json!("a".repeat(9000)); + let big_jwt = encode( + &Header::new(Algorithm::ES256), + &payload, + &EncodingKey::from_ec_pem(PRIV1.as_bytes()).unwrap(), + ) + .unwrap(); + let oversized_token = format!("jwt_guest_{big_jwt}"); + + // a repeated prefix must not strip down to a valid short token that verifies and is then + // cached under the full bearer key (trim_start_matches would; strip_prefix must not). + let repeated_prefix = format!( + "jwt_guest_{}", + bearer(&Claims::valid(), PRIV1, Algorithm::ES256) + ); for (label, token) in [ ("wrong workspace", wrong_ws), @@ -321,6 +337,7 @@ async fn guest_jwt_refusals(db: Pool) -> anyhow::Result<()> { ("oversized email", oversized_email), ("wildcard app_path", wildcard_app_path), ("oversized token", oversized_token), + ("repeated prefix", repeated_prefix), ] { let resp = whoami(port, ws, &token).send().await?; assert_eq!(resp.status(), 401, "{label} must be refused"); diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index 6f88ed9a4e..6d96112ec5 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -222,7 +222,15 @@ impl AuthCache { let Some(w_id) = w_id.as_deref() else { return None; }; - let jwt = token.trim_start_matches(windmill_common::guest_jwt::BEARER_PREFIX); + // The cache keys on the whole bearer, so bound it here before verify or caching; + // strip exactly one prefix, or repeated prefixes would shrink an oversized + // bearer past the length check while it is still cached at full size. + if token.len() > windmill_common::guest_jwt::MAX_GUEST_JWT_LEN { + return None; + } + let jwt = token + .strip_prefix(windmill_common::guest_jwt::BEARER_PREFIX) + .unwrap_or(token); let claims = match windmill_common::guest_jwt::verify_for_workspace(&self.db, w_id, jwt) .await diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index 49496ff7db..f84e8394ba 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -274,7 +274,7 @@ const MAX_GUEST_PEM_LEN: usize = 8 * 1024; /// A guest JWT is refused past this before any signature work or caching: the auth cache keys /// on the bearer, so an oversized token (unauthenticated at this point) would otherwise be /// decoded and, if it verified, cached at its full size. A real JWT is well under this. -const MAX_GUEST_JWT_LEN: usize = 8 * 1024; +pub const MAX_GUEST_JWT_LEN: usize = 8 * 1024; /// Fetch a JWKS, keeping only the keys usable here. The URL was set by a workspace /// admin, so it is validated against private ranges and the connect is pinned to the