mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 16:02:23 +00:00
fix: strip one guest bearer prefix and bound the raw bearer
trim_start_matches stripped every jwt_guest_ prefix, so a repeated-prefix bearer shrank to a valid short token that verified and was then cached under the full oversized bearer key. Strip exactly one prefix, and bound the raw bearer length (the auth cache keys on it) before verifying or caching. The refusal test now mints a valid signed token over the cap (which would otherwise verify, the extra claim ignored) and a repeated-prefix bearer, so it fails if either guard regresses. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
0937fb66ca
commit
58eccf0a80
@@ -305,8 +305,24 @@ async fn guest_jwt_refusals(db: Pool<Postgres>) -> 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<Postgres>) -> 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");
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user