mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 00:01:49 +00:00
fix: refuse a private PEM as a guest verification key; fix stale log
jsonwebtoken 8.3's from_rsa_pem accepts private encodings (PKCS#1, PKCS#8), so a pasted private key would be stored and then served back through the settings response. Refuse any private PEM in decoding_key_from_pem, the single choke point for both the save endpoint and per-request verification. Also drop the last "canonical" references the is_scope_literal_path switch left in the JWT arm: the refusal log no longer misdiagnoses a reserved character as a malformed path, and the relocated guest_session_scopes doc carries the sentinel/label distinction for both its callers. 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
259f979a72
commit
51776ef87c
@@ -219,10 +219,13 @@ impl AuthCache {
|
||||
_ if token.starts_with(windmill_common::guest_jwt::BEARER_PREFIX) => {
|
||||
// A workspace-less route never accepts a guest JWT: the identity is
|
||||
// pinned to the workspace its claim names, like a DB guest session.
|
||||
let Some(w_id) = w_id.as_deref() else { return None };
|
||||
let Some(w_id) = w_id.as_deref() else {
|
||||
return None;
|
||||
};
|
||||
let jwt = token.trim_start_matches(windmill_common::guest_jwt::BEARER_PREFIX);
|
||||
let claims =
|
||||
match windmill_common::guest_jwt::verify_for_workspace(&self.db, w_id, jwt).await
|
||||
match windmill_common::guest_jwt::verify_for_workspace(&self.db, w_id, jwt)
|
||||
.await
|
||||
{
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
@@ -235,8 +238,12 @@ impl AuthCache {
|
||||
// and the no-account rule per request through the sentinel below
|
||||
// (guest_session_stands), so turning any of them off stops a cached JWT
|
||||
// session on its next call.
|
||||
match windmill_common::workspaces::guest_app_admits(&self.db, w_id, &claims.app_path)
|
||||
.await
|
||||
match windmill_common::workspaces::guest_app_admits(
|
||||
&self.db,
|
||||
w_id,
|
||||
&claims.app_path,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(true) => {}
|
||||
Ok(false) => return None,
|
||||
@@ -268,11 +275,11 @@ impl AuthCache {
|
||||
}
|
||||
// guest_session_scopes already carries the sentinel, and it is the whole
|
||||
// grant; a JWT has no label, so the sentinel is what governs it. It also
|
||||
// re-checks the path is canonical (verify already did); refuse if not.
|
||||
// re-checks the path holds no scope metacharacter (verify already did).
|
||||
let scopes = match crate::scopes::guest_session_scopes(&claims.app_path) {
|
||||
Ok(s) => Some(s),
|
||||
Err(e) => {
|
||||
tracing::error!("guest JWT app_path not canonical for {w_id}: {e:#}");
|
||||
tracing::error!("guest JWT app_path cannot be scoped for {w_id}: {e:#}");
|
||||
return None;
|
||||
}
|
||||
};
|
||||
@@ -301,7 +308,11 @@ impl AuthCache {
|
||||
};
|
||||
AUTH_CACHE.insert(
|
||||
key,
|
||||
ExpiringAuthCache { authed: authed.clone(), expiry: cache_expiry, job_id: None },
|
||||
ExpiringAuthCache {
|
||||
authed: authed.clone(),
|
||||
expiry: cache_expiry,
|
||||
job_id: None,
|
||||
},
|
||||
);
|
||||
Some(OptJobAuthed { authed, job_id: None })
|
||||
}
|
||||
|
||||
@@ -797,6 +797,9 @@ pub fn with_guest_sentinel(mut scopes: Vec<String>) -> Vec<String> {
|
||||
/// allowlist by the sentinel (`guest_route_denied`), plus the two path-scoped app
|
||||
/// grants. A guest has no `usr` row, so this list is the whole of what it can do. The
|
||||
/// single source both the mint (a signed-in guest) and the JWT auth arm build from.
|
||||
///
|
||||
/// The sentinel here only narrows. A signed-in guest is made one by the server-minted
|
||||
/// label; a JWT guest has no label, so for it the sentinel is what governs.
|
||||
pub fn guest_session_scopes(app_path: &str) -> windmill_common::error::Result<Vec<String>> {
|
||||
// The path is spliced into a scope, whose grammar reserves `:`, `,`, `*` and a leading
|
||||
// `/`; app paths may otherwise carry spaces and `@`, so guard only those reserved chars.
|
||||
|
||||
@@ -81,6 +81,14 @@ pub async fn key_source(db: &DB, w_id: &str) -> Result<Option<GuestJwtKeySource>
|
||||
/// from here by construction; the JWKS path refuses it explicitly.
|
||||
pub fn decoding_key_from_pem(pem: &str) -> Result<(DecodingKey, &'static [Algorithm])> {
|
||||
let pem = pem.trim();
|
||||
// A verification key must be public. jsonwebtoken 8.3's `from_rsa_pem` also accepts private
|
||||
// encodings (PKCS#1, PKCS#8), so a pasted private key would be stored and then served back
|
||||
// through the settings response. Refuse any private PEM before it is parsed or persisted.
|
||||
if pem.contains("PRIVATE KEY") {
|
||||
return Err(Error::BadRequest(
|
||||
"expected a PEM public key (-----BEGIN PUBLIC KEY-----), not a private key".to_string(),
|
||||
));
|
||||
}
|
||||
if let Ok(key) = DecodingKey::from_rsa_pem(pem.as_bytes()) {
|
||||
return Ok((key, &RSA_ALGORITHMS));
|
||||
}
|
||||
@@ -525,6 +533,18 @@ mod tests {
|
||||
.is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_private_pem_is_refused() {
|
||||
// A verification key must be public; a private key must never be stored, as it is
|
||||
// served back through the settings response. PRIV1 is a real EC private key the PEM
|
||||
// parser would otherwise accept; the RSA label covers jsonwebtoken's PKCS#1/#8 case.
|
||||
assert!(decoding_key_from_pem(PRIV1).is_err());
|
||||
assert!(decoding_key_from_pem(
|
||||
"-----BEGIN RSA PRIVATE KEY-----\nMIIBOgIBAAJBAKj\n-----END RSA PRIVATE KEY-----\n"
|
||||
)
|
||||
.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_concurrent_cold_burst_makes_one_jwks_fetch() {
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
|
||||
Reference in New Issue
Block a user