From 2108c4ad2810418ba418ea67e75615fe6048ed46 Mon Sep 17 00:00:00 2001 From: houseme Date: Tue, 27 Jan 2026 01:47:39 +0800 Subject: [PATCH] fix: remove plaintext credential logging (#1619) --- crates/credentials/src/credentials.rs | 132 ++++++++++++++++++++++---- crates/utils/src/lib.rs | 1 - rustfs/src/config/mod.rs | 16 +--- rustfs/src/main.rs | 4 +- 4 files changed, 121 insertions(+), 32 deletions(-) diff --git a/crates/credentials/src/credentials.rs b/crates/credentials/src/credentials.rs index 169905995..6c3813c44 100644 --- a/crates/credentials/src/credentials.rs +++ b/crates/credentials/src/credentials.rs @@ -18,6 +18,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use std::collections::HashMap; use std::env; +use std::fmt; use std::io::Error; use std::sync::OnceLock; use time::OffsetDateTime; @@ -28,6 +29,37 @@ static GLOBAL_ACTIVE_CRED: OnceLock = OnceLock::new(); /// Global RPC authentication token pub static GLOBAL_RUSTFS_RPC_SECRET: OnceLock = OnceLock::new(); +/// Error type for credentials operations +#[derive(Debug)] +pub enum CredentialsError { + /// Credentials already initialized + AlreadyInitialized, + /// Failed to generate access key + AccessKeyGenerationFailed(Error), + /// Failed to generate secret key + SecretKeyGenerationFailed(Error), +} + +impl fmt::Display for CredentialsError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CredentialsError::AlreadyInitialized => write!(f, "Credentials already initialized"), + CredentialsError::AccessKeyGenerationFailed(e) => write!(f, "Failed to generate access key: {}", e), + CredentialsError::SecretKeyGenerationFailed(e) => write!(f, "Failed to generate secret key: {}", e), + } + } +} + +impl std::error::Error for CredentialsError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + CredentialsError::AccessKeyGenerationFailed(e) => Some(e), + CredentialsError::SecretKeyGenerationFailed(e) => Some(e), + _ => None, + } + } +} + /// Initialize the global action credentials /// /// # Arguments @@ -35,15 +67,17 @@ pub static GLOBAL_RUSTFS_RPC_SECRET: OnceLock = OnceLock::new(); /// * `sk` - Optional secret key /// /// # Returns -/// * `Result<(), Box>` - Ok if successful, Err with existing credentials if already initialized -/// -/// # Panics -/// This function panics if automatic credential generation fails when `ak` or `sk` -/// are `None`, for example if the random number generator fails while calling -/// `gen_access_key` or `gen_secret_key`. -pub fn init_global_action_credentials(ak: Option, sk: Option) -> Result<(), Box> { - let ak = ak.unwrap_or_else(|| gen_access_key(20).expect("Failed to generate access key")); - let sk = sk.unwrap_or_else(|| gen_secret_key(32).expect("Failed to generate secret key")); +/// * `Result<(), CredentialsError>` - Ok if successful, Err if already initialized or generation failed +pub fn init_global_action_credentials(ak: Option, sk: Option) -> Result<(), CredentialsError> { + let ak = match ak { + Some(k) => k, + None => gen_access_key(20).map_err(CredentialsError::AccessKeyGenerationFailed)?, + }; + + let sk = match sk { + Some(k) => k, + None => gen_secret_key(32).map_err(CredentialsError::SecretKeyGenerationFailed)?, + }; let cred = Credentials { access_key: ak, @@ -51,12 +85,7 @@ pub fn init_global_action_credentials(ak: Option, sk: Option) -> ..Default::default() }; - GLOBAL_ACTIVE_CRED.set(cred).map_err(|e| { - Box::new(Credentials { - access_key: e.access_key.clone(), - ..Default::default() - }) - }) + GLOBAL_ACTIVE_CRED.set(cred).map_err(|_| CredentialsError::AlreadyInitialized) } /// Get the global action credentials @@ -195,6 +224,36 @@ pub fn get_rpc_token() -> String { .clone() } +/// A wrapper struct for masking sensitive strings in Debug implementations. +/// It avoids allocating a new String just for formatting. +pub struct Masked<'a>(pub Option<&'a str>); + +impl<'a> fmt::Debug for Masked<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.0 { + None => Ok(()), + Some(s) => { + let len = s.len(); + if len == 0 { + Ok(()) + } else if len == 1 { + write!(f, "***") + } else if len == 2 { + write!(f, "{}***|{}", &s[0..1], len) + } else { + write!(f, "{}***{}|{}", &s[0..1], &s[len - 1..], len) + } + } + } + } +} + +impl<'a> fmt::Display for Masked<'a> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(self, f) + } +} + /// Credentials structure /// /// Fields: @@ -209,7 +268,7 @@ pub fn get_rpc_token() -> String { /// - name: Optional name string /// - description: Optional description string /// -#[derive(Serialize, Deserialize, Clone, Default, Debug)] +#[derive(Serialize, Deserialize, Clone, Default)] pub struct Credentials { pub access_key: String, pub secret_key: String, @@ -223,6 +282,23 @@ pub struct Credentials { pub description: Option, } +impl fmt::Debug for Credentials { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Credentials") + .field("access_key", &self.access_key) + .field("secret_key", &Masked(Some(&self.secret_key))) + .field("session_token", &self.session_token) + .field("expiration", &self.expiration) + .field("status", &self.status) + .field("parent_user", &self.parent_user) + .field("groups", &self.groups) + .field("claims", &self.claims) + .field("name", &self.name) + .field("description", &self.description) + .finish() + } +} + impl Credentials { pub fn is_expired(&self) -> bool { if self.expiration.is_none() { @@ -383,4 +459,28 @@ mod tests { assert_eq!(sk.len(), 32); } } + + #[test] + fn test_masked_debug() { + // Test None + assert_eq!(format!("{:?}", Masked(None)), ""); + + // Test empty string + assert_eq!(format!("{:?}", Masked(Some(""))), ""); + + // Test length 1 + assert_eq!(format!("{:?}", Masked(Some("a"))), "***"); + + // Test length 2 + assert_eq!(format!("{:?}", Masked(Some("ab"))), "a***|2"); + + // Test length 3 + assert_eq!(format!("{:?}", Masked(Some("abc"))), "a***c|3"); + + // Test length 4 + assert_eq!(format!("{:?}", Masked(Some("abcd"))), "a***d|4"); + + // Test longer string + assert_eq!(format!("{:?}", Masked(Some("secretpassword"))), "s***d|14"); + } } diff --git a/crates/utils/src/lib.rs b/crates/utils/src/lib.rs index 5bf82476b..1bb007fa7 100644 --- a/crates/utils/src/lib.rs +++ b/crates/utils/src/lib.rs @@ -90,5 +90,4 @@ mod dunce; #[cfg(feature = "path")] pub use dunce::*; mod envs; - pub use envs::*; diff --git a/rustfs/src/config/mod.rs b/rustfs/src/config/mod.rs index 87462eb2c..9a2bb9a40 100644 --- a/rustfs/src/config/mod.rs +++ b/rustfs/src/config/mod.rs @@ -168,18 +168,18 @@ impl std::fmt::Debug for Opt { .field("address", &self.address) .field("server_domains", &self.server_domains) .field("access_key", &self.access_key) - .field("secret_key", &Opt::mask_sensitive(Some(&self.secret_key))) // Hide sensitive values + .field("secret_key", &rustfs_credentials::Masked(Some(&self.secret_key))) // Hide sensitive values .field("console_enable", &self.console_enable) .field("console_address", &self.console_address) .field("obs_endpoint", &self.obs_endpoint) .field("tls_path", &self.tls_path) - .field("license", &Opt::mask_sensitive(self.license.as_ref())) + .field("license", &rustfs_credentials::Masked(self.license.as_deref())) .field("region", &self.region) .field("kms_enable", &self.kms_enable) .field("kms_backend", &self.kms_backend) .field("kms_key_dir", &self.kms_key_dir) .field("kms_vault_address", &self.kms_vault_address) - .field("kms_vault_token", &Opt::mask_sensitive(self.kms_vault_token.as_ref())) + .field("kms_vault_token", &rustfs_credentials::Masked(self.kms_vault_token.as_deref())) .field("kms_default_key_id", &self.kms_default_key_id) .field("buffer_profile_disable", &self.buffer_profile_disable) .field("buffer_profile", &self.buffer_profile) @@ -187,16 +187,6 @@ impl std::fmt::Debug for Opt { } } -impl Opt { - /// Mask sensitive information in Option - fn mask_sensitive(s: Option<&String>) -> String { - match s { - None => "".to_string(), - Some(s) => format!("{}***{}|{}", s.chars().next().unwrap_or('*'), s.chars().last().unwrap_or('*'), s.len()), - } - } -} - // lazy_static::lazy_static! { // pub(crate) static ref OPT: OnceLock = OnceLock::new(); // } diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index f4bb67bf0..c07459eb9 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -168,8 +168,8 @@ async fn run(opt: config::Opt) -> Result<()> { info!(target: "rustfs::main::run", "Global action credentials initialized successfully."); } Err(e) => { - let msg = format!("init_global_action_credentials failed: {e:?}"); - error!("{msg}"); + let msg = format!("init global action credentials failed: {e:?}"); + error!(target: "rustfs::main::run","{msg}"); return Err(Error::other(msg)); } };