From eb68d559ac9146a854fa932fa5a3ddaaca15bae0 Mon Sep 17 00:00:00 2001 From: Jason Hernandez <7144515+jasonhernandez@users.noreply.github.com> Date: Thu, 26 Mar 2026 13:25:44 -0700 Subject: [PATCH 1/2] authenticator: Zeroize ValidatedClaims and OidcClaims on drop Implement Zeroize for OidcClaims (issuer, audience, unknown claims) and Zeroize + ZeroizeOnDrop for ValidatedClaims (user identity string). Document that OidcDecodingKey wraps an opaque foreign type and cannot be zeroized, and that borrowed JWT token strings are the caller's responsibility. Part of SEC-115. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/authenticator/src/oidc.rs | 124 ++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) 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" + ); + } } From a1dec0b398ae1aee1884dc82925040b100417f9a Mon Sep 17 00:00:00 2001 From: Jason Hernandez <7144515+jasonhernandez@users.noreply.github.com> Date: Tue, 31 Mar 2026 15:01:04 -0700 Subject: [PATCH 2/2] fixup: Fix downstream callers that move out of ValidatedClaims Adding `Drop` to `ValidatedClaims` prevents moving fields out directly. Use `std::mem::take` to extract values while leaving empty defaults for the Drop impl to zeroize. Fixes: mz-pgwire and mz-environmentd build failures. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/environmentd/src/http.rs | 4 ++-- src/pgwire/src/protocol.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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,