From 7553715f62053a3dc54b380613fb072237ef0c3a Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 7 Aug 2026 22:17:56 +0800 Subject: [PATCH] fix(admin): stop logging STS AssumeRole JWT claims (#5802) --- rustfs/src/admin/handlers/sts.rs | 73 +++++++++++++++++++++++++++-- scripts/check_logging_guardrails.sh | 11 +++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/rustfs/src/admin/handlers/sts.rs b/rustfs/src/admin/handlers/sts.rs index a0a0135f9..9d137bb3e 100644 --- a/rustfs/src/admin/handlers/sts.rs +++ b/rustfs/src/admin/handlers/sts.rs @@ -49,7 +49,7 @@ use serde::Deserialize; use serde_json::Value; use serde_urlencoded::from_bytes; use time::{Duration, OffsetDateTime}; -use tracing::{debug, error, info, warn}; +use tracing::{debug, error, warn}; const ASSUME_ROLE_ACTION: &str = "AssumeRole"; const ASSUME_ROLE_WITH_WEB_IDENTITY_ACTION: &str = "AssumeRoleWithWebIdentity"; @@ -76,6 +76,15 @@ fn clamp_assume_role_duration(duration_seconds: usize) -> usize { } } +/// Record that AssumeRole assembled its session claims without echoing any claim values. +/// +/// Claims carry caller-supplied identity material (parent user, session policy, arbitrary +/// JWT fields); interpolating them into logs repeats the GHSA-r54g-49rx-98cr / +/// GHSA-8cm2-h255-v749 credential-leak class. Only derived metadata may be logged here. +fn trace_assume_role_claims(claims: &std::collections::HashMap) { + debug!(claim_count = claims.len(), "AssumeRole assembled session claims"); +} + /// Build the site-replication IAM item that mirrors an AssumeRole temporary credential to peers. fn assume_role_site_replication_item(cred: &rustfs_credentials::Credentials, updated_at: OffsetDateTime) -> SRIAMItem { SRIAMItem { @@ -141,7 +150,7 @@ pub struct AssumeRoleHandle {} #[async_trait::async_trait] impl Operation for AssumeRoleHandle { async fn call(&self, req: S3Request, _params: Params<'_, '_>) -> S3Result> { - warn!("handle AssumeRoleHandle"); + debug!("handle AssumeRoleHandle"); let mut input = req.input; @@ -241,7 +250,7 @@ async fn handle_assume_role( return Err(s3_error!(InvalidArgument, "global active sk not init")); }; - info!("AssumeRole get claims {:?}", &claims); + trace_assume_role_claims(&claims); let mut new_cred = get_new_credentials_with_metadata(&claims, &secret) .map_err(|e| S3Error::with_message(S3ErrorCode::InternalError, format!("get new cred failed {e}")))?; @@ -386,6 +395,64 @@ fn xml_escape(s: &str) -> String { mod tests { use super::*; + #[test] + fn assume_role_claims_log_never_echoes_claim_values() { + #[derive(Clone, Default)] + struct CapturedLog(std::sync::Arc>>); + + struct CapturedLogWriter(std::sync::Arc>>); + + impl std::io::Write for CapturedLogWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0.lock().expect("captured log lock").extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + impl<'writer> tracing_subscriber::fmt::MakeWriter<'writer> for CapturedLog { + type Writer = CapturedLogWriter; + + fn make_writer(&'writer self) -> Self::Writer { + CapturedLogWriter(self.0.clone()) + } + } + + let captured = CapturedLog::default(); + let subscriber = tracing_subscriber::fmt() + .with_ansi(false) + .without_time() + .with_max_level(tracing::Level::TRACE) + .with_writer(captured.clone()) + .finish(); + + let mut claims = std::collections::HashMap::new(); + claims.insert("parent".to_string(), Value::String("sensitive-parent-user".to_string())); + claims.insert("sessionPolicy".to_string(), Value::String("eyJzZWNyZXQtcG9saWN5LWJsb2Ii".to_string())); + claims.insert("sub".to_string(), Value::String("sensitive-subject-id".to_string())); + + tracing::subscriber::with_default(subscriber, || { + trace_assume_role_claims(&claims); + }); + + let logs = String::from_utf8(captured.0.lock().expect("captured log lock").clone()).expect("captured logs must be UTF-8"); + assert!( + logs.contains("claim_count"), + "expected the redacted claims log line to be emitted: {logs}" + ); + for secret in [ + "sensitive-parent-user", + "eyJzZWNyZXQtcG9saWN5LWJsb2Ii", + "sensitive-subject-id", + "sessionPolicy", + ] { + assert!(!logs.contains(secret), "AssumeRole claims log leaked {secret}: {logs}"); + } + } + #[test] fn test_xml_escape() { assert_eq!(xml_escape("hello"), "hello"); diff --git a/scripts/check_logging_guardrails.sh b/scripts/check_logging_guardrails.sh index cf6a85ba1..42fc3e271 100755 --- a/scripts/check_logging_guardrails.sh +++ b/scripts/check_logging_guardrails.sh @@ -690,6 +690,17 @@ if [[ -n "$unmasked_revoke_fields" ]]; then exit 1 fi +# STS claims carry caller-supplied identity material (parent user, session policy, JWT +# fields). Interpolating the claims map into any log or error repeats the +# GHSA-r54g-49rx-98cr / GHSA-8cm2-h255-v749 credential-leak class. Only derived metadata +# such as claims.len() may be logged (see trace_assume_role_claims in sts.rs). +sts_claims_content_logs="$(rg -n '\{:\?\}.*claims|claims.*\{:\?\}|\{&?claims:\?\}|[?%]\s*&?claims\b' rustfs/src/admin/handlers/sts.rs || true)" +if [[ -n "$sts_claims_content_logs" ]]; then + echo "❌ logging guardrail violation: STS handlers must not interpolate JWT claims into logs or errors (GHSA-r54g-49rx-98cr / GHSA-8cm2-h255-v749 class); log derived metadata such as claims.len() instead" >&2 + echo "$sts_claims_content_logs" >&2 + exit 1 +fi + heal_hotpath_files=( "crates/ecstore/src/store/heal.rs" "crates/ecstore/src/store/mod.rs"