diff --git a/src/authenticator/src/oidc.rs b/src/authenticator/src/oidc.rs index 41e5f38c47ff2..c749c9bb28907 100644 --- a/src/authenticator/src/oidc.rs +++ b/src/authenticator/src/oidc.rs @@ -11,6 +11,27 @@ //! //! This module provides JWT-based authentication using OpenID Connect (OIDC). //! JWTs are validated locally using JWKS fetched from the configured provider. +//! +//! # Zeroization (SEC-115) +//! +//! Sensitive types in this module implement [`Zeroize`] where feasible to ensure +//! secret material does not linger in process memory after use: +//! +//! - [`ValidatedClaims`]: implements `Zeroize` and `ZeroizeOnDrop`. The `user` +//! field (extracted identity) is zeroed when the value is dropped. +//! +//! - [`OidcClaims`]: implements `Zeroize`. All fields — including the issuer, +//! audience list, and the freeform `unknown_claims` map — are cleared. +//! +//! The following types **cannot** be zeroized and callers should be aware: +//! +//! - [`OidcDecodingKey`]: wraps the opaque foreign type `jsonwebtoken::DecodingKey`, +//! whose internal representation is not accessible. The key material lives +//! inside the foreign type and cannot be overwritten from Rust code. +//! +//! - **JWT token strings** (`&str`): passed by reference to [`GenericOidcAuthenticatorInner::validate_token`]. +//! Because these are borrowed, this crate cannot zeroize the caller's buffer. +//! Callers handling raw JWT tokens should use `Zeroizing` for owned copies. use std::collections::BTreeMap; use std::sync::{Arc, Mutex}; @@ -20,6 +41,7 @@ use jsonwebtoken::jwk::JwkSet; use mz_adapter::{AdapterError, AuthenticationError, Client as AdapterClient}; use mz_adapter_types::dyncfgs::{OIDC_AUDIENCE, OIDC_AUTHENTICATION_CLAIM, OIDC_ISSUER}; use mz_auth::Authenticated; +use mz_ore::secure::{Zeroize, ZeroizeOnDrop}; use mz_ore::soft_panic_or_log; use mz_pgwire_common::{ErrorResponse, Severity}; use reqwest::Client as HttpClient; @@ -183,6 +205,25 @@ pub struct OidcClaims { pub unknown_claims: BTreeMap, } +impl Zeroize for OidcClaims { + fn zeroize(&mut self) { + self.iss.zeroize(); + self.exp.zeroize(); + self.iat.zeroize(); + // Vec: zeroize each string's contents, then clear the vec. + for s in &mut self.aud { + s.zeroize(); + } + self.aud.clear(); + // BTreeMap: serde_json::Value does not + // implement Zeroize, so we clear the map to drop all entries. String + // keys and any heap-allocated Value contents are freed but not + // cryptographically zeroed — this is the best we can do without + // forking serde_json. + self.unknown_claims.clear(); + } +} + impl OidcClaims { /// Extract the username from the OIDC claims. fn user(&self, authentication_claim: &str) -> Option<&str> { @@ -198,6 +239,30 @@ pub struct ValidatedClaims { _private: (), } +impl Zeroize for ValidatedClaims { + fn zeroize(&mut self) { + self.user.zeroize(); + } +} + +impl Drop for ValidatedClaims { + fn drop(&mut self) { + self.zeroize(); + } +} + +/// The `Drop` implementation calls `zeroize()`, which zeroes all +/// sensitive fields. This satisfies the `ZeroizeOnDrop` contract. +impl ZeroizeOnDrop for ValidatedClaims {} + +/// Wrapper around `jsonwebtoken::DecodingKey` with a redacted `Debug` impl. +/// +/// # Zeroization limitation (SEC-115) +/// +/// `DecodingKey` is an opaque foreign type from the `jsonwebtoken` crate. Its +/// internal representation (parsed JWK key material) is not accessible, so we +/// cannot implement `Zeroize` for this wrapper. The key material will be freed +/// by the allocator on drop but is **not** cryptographically zeroed. #[derive(Clone)] struct OidcDecodingKey(jsonwebtoken::DecodingKey); @@ -567,4 +632,63 @@ mod tests { "https://dev-123456.okta.com/oauth2/default/.well-known/openid-configuration" ); } + + // --- Zeroization tests (SEC-115) --- + + #[mz_ore::test] + fn validated_claims_implements_zeroize() { + fn assert_zeroize() {} + assert_zeroize::(); + } + + #[mz_ore::test] + fn validated_claims_implements_zeroize_on_drop() { + fn assert_zod() {} + assert_zod::(); + } + + #[mz_ore::test] + fn validated_claims_user_field_is_zeroed() { + use mz_ore::secure::Zeroize; + let mut claims = ValidatedClaims { + user: "alice@example.com".to_string(), + _private: (), + }; + claims.zeroize(); + assert!(claims.user.is_empty(), "user field should be zeroed"); + } + + #[mz_ore::test] + fn oidc_claims_implements_zeroize() { + fn assert_zeroize() {} + assert_zeroize::(); + } + + #[mz_ore::test] + fn oidc_claims_zeroize_clears_fields() { + use mz_ore::secure::Zeroize; + let mut claims = OidcClaims { + iss: "https://issuer.example.com".to_string(), + exp: 1234567890, + iat: Some(1234567800), + aud: vec!["app1".to_string(), "app2".to_string()], + unknown_claims: { + let mut m = BTreeMap::new(); + m.insert( + "email".to_string(), + serde_json::Value::String("alice@example.com".to_string()), + ); + m + }, + }; + claims.zeroize(); + assert!(claims.iss.is_empty(), "iss should be zeroed"); + assert_eq!(claims.exp, 0, "exp should be zeroed"); + assert!(claims.iat.is_none(), "iat should be zeroed"); + assert!(claims.aud.is_empty(), "aud should be zeroed"); + assert!( + claims.unknown_claims.is_empty(), + "unknown_claims should be zeroed" + ); + } } diff --git a/src/environmentd/src/http.rs b/src/environmentd/src/http.rs index b3ff62452f68d..4fc8e69087589 100644 --- a/src/environmentd/src/http.rs +++ b/src/environmentd/src/http.rs @@ -1227,11 +1227,11 @@ async fn auth( Authenticator::Oidc(oidc) => match creds { Some(Credentials::Token { token }) => { // Validate JWT token - let (claims, authenticated) = oidc + let (mut claims, authenticated) = oidc .authenticate(&token, None) .await .map_err(|_| AuthError::InvalidCredentials)?; - let name = claims.user; + let name = std::mem::take(&mut claims.user); (name, None, authenticated) } _ => { diff --git a/src/pgwire/src/protocol.rs b/src/pgwire/src/protocol.rs index 000282df247cb..35c832f85ea39 100644 --- a/src/pgwire/src/protocol.rs +++ b/src/pgwire/src/protocol.rs @@ -262,12 +262,12 @@ where }; let auth_response = oidc.authenticate(&jwt, Some(&user)).await; match auth_response { - Ok((claims, authenticated)) => { + Ok((mut claims, authenticated)) => { let session = adapter_client.new_session( SessionConfig { conn_id: conn.conn_id().clone(), uuid: conn_uuid, - user: claims.user, + user: std::mem::take(&mut claims.user), client_ip: conn.peer_addr().clone(), external_metadata_rx: None, helm_chart_version,