From f83bf95b049608b7854fb17ddd803e37e3042f1b Mon Sep 17 00:00:00 2001 From: houseme Date: Thu, 12 Mar 2026 18:02:19 +0800 Subject: [PATCH] feat(ecstore): Skip rustls provider install if already present (#2145) Signed-off-by: houseme Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: houseme <4829346+houseme@users.noreply.github.com> --- crates/config/src/constants/app.rs | 6 ++ .../ecstore/src/bucket/bucket_target_sys.rs | 2 +- crates/ecstore/src/client/transition_api.rs | 92 ++++++++++++++----- rustfs/src/main.rs | 6 ++ rustfs/src/server/cert.rs | 2 - 5 files changed, 81 insertions(+), 27 deletions(-) diff --git a/crates/config/src/constants/app.rs b/crates/config/src/constants/app.rs index 0b6cb2b85..627019dfb 100644 --- a/crates/config/src/constants/app.rs +++ b/crates/config/src/constants/app.rs @@ -131,6 +131,12 @@ pub const ENV_RUSTFS_ADDRESS: &str = "RUSTFS_ADDRESS"; /// Environment variable for server volumes. pub const ENV_RUSTFS_VOLUMES: &str = "RUSTFS_VOLUMES"; +/// Environment variable for server tls path. +pub const ENV_RUSTFS_TLS_PATH: &str = "RUSTFS_TLS_PATH"; + +/// Default value for the server TLS path if `ENV_RUSTFS_TLS_PATH` is not set. +pub const DEFAULT_RUSTFS_TLS_PATH: &str = ""; + /// Default port for rustfs /// This is the default port for rustfs. /// This is used to bind the server to a specific port. diff --git a/crates/ecstore/src/bucket/bucket_target_sys.rs b/crates/ecstore/src/bucket/bucket_target_sys.rs index b52de247e..e840b1f1a 100644 --- a/crates/ecstore/src/bucket/bucket_target_sys.rs +++ b/crates/ecstore/src/bucket/bucket_target_sys.rs @@ -804,7 +804,7 @@ impl BucketTargetSys { } async fn build_aws_s3_http_client_from_tls_path() -> Option { - let tls_path = std::env::var("RUSTFS_TLS_PATH").ok()?; + let tls_path = rustfs_utils::get_env_str(rustfs_config::ENV_RUSTFS_TLS_PATH, rustfs_config::DEFAULT_RUSTFS_TLS_PATH); if tls_path.is_empty() { return None; } diff --git a/crates/ecstore/src/client/transition_api.rs b/crates/ecstore/src/client/transition_api.rs index 04dd8f1a8..d23c6fb3a 100644 --- a/crates/ecstore/src/client/transition_api.rs +++ b/crates/ecstore/src/client/transition_api.rs @@ -139,7 +139,11 @@ pub enum BucketLookupType { fn load_root_store_from_tls_path() -> Option { // Load the root certificate bundle from the path specified by the // RUSTFS_TLS_PATH environment variable. - let tp = std::env::var("RUSTFS_TLS_PATH").ok()?; + let tp = rustfs_utils::get_env_str(rustfs_config::ENV_RUSTFS_TLS_PATH, rustfs_config::DEFAULT_RUSTFS_TLS_PATH); + // If no TLS path is configured, do not fall back to a CA bundle in the current directory. + if tp.is_empty() { + return None; + } let ca = std::path::Path::new(&tp).join(rustfs_config::RUSTFS_CA_CERT); if !ca.exists() { return None; @@ -171,15 +175,12 @@ fn with_rustls_init_guard(build: F) -> Result where F: FnOnce() -> Result, { - match std::panic::catch_unwind(std::panic::AssertUnwindSafe(build)) { - Ok(result) => result, - Err(payload) => { - let panic_message = panic_payload_to_message(payload); - Err(std::io::Error::other(format!( - "failed to initialize rustls crypto provider: {panic_message}. Ensure exactly one rustls crypto provider feature is enabled (aws-lc-rs or ring), or install one with CryptoProvider::install_default()" - ))) - } - } + std::panic::catch_unwind(std::panic::AssertUnwindSafe(build)).unwrap_or_else(|payload| { + let panic_message = panic_payload_to_message(payload); + Err(std::io::Error::other(format!( + "failed to initialize rustls crypto provider: {panic_message}. Ensure exactly one rustls crypto provider feature is enabled (aws-lc-rs or ring), or install one with CryptoProvider::install_default()" + ))) + }) } fn build_tls_config() -> Result { @@ -198,15 +199,25 @@ fn build_tls_config() -> Result { impl TransitionClient { pub async fn new(endpoint: &str, opts: Options, tier_type: &str) -> Result { - let clnt = Self::private_new(endpoint, opts, tier_type).await?; + let client = Self::private_new(endpoint, opts, tier_type).await?; - Ok(clnt) + Ok(client) } async fn private_new(endpoint: &str, opts: Options, tier_type: &str) -> Result { + if rustls::crypto::CryptoProvider::get_default().is_none() { + // No default provider is set yet; try to install aws-lc-rs. + // `install_default` can only fail if another thread races us and installs a provider + // between our check and this call, which is still safe to ignore. + if rustls::crypto::aws_lc_rs::default_provider().install_default().is_err() { + debug!("rustls crypto provider was installed concurrently, skipping aws-lc-rs install"); + } + } else { + debug!("rustls crypto provider already installed, skipping aws-lc-rs install"); + } + let endpoint_url = get_endpoint_url(endpoint, opts.secure)?; - let client; let tls = build_tls_config()?; let https = hyper_rustls::HttpsConnectorBuilder::new() @@ -215,14 +226,14 @@ impl TransitionClient { .enable_http1() .enable_http2() .build(); - client = Client::builder(TokioExecutor::new()).build(https); + let http_client = Client::builder(TokioExecutor::new()).build(https); - let mut clnt = TransitionClient { + let mut client = TransitionClient { endpoint_url, creds_provider: Arc::new(Mutex::new(opts.creds)), override_signer_type: SignatureType::SignatureDefault, secure: opts.secure, - http_client: client, + http_client, bucket_loc_cache: Arc::new(Mutex::new(BucketLocationCache::new())), is_trace_enabled: Arc::new(Mutex::new(false)), trace_errors_only: Arc::new(Mutex::new(false)), @@ -240,23 +251,23 @@ impl TransitionClient { }; { - let mut md5_hasher = clnt.md5_hasher.lock().unwrap(); + let mut md5_hasher = client.md5_hasher.lock().unwrap(); if md5_hasher.is_none() { *md5_hasher = Some(HashAlgorithm::Md5); } } - if clnt.sha256_hasher.is_none() { - clnt.sha256_hasher = Some(HashAlgorithm::SHA256); + if client.sha256_hasher.is_none() { + client.sha256_hasher = Some(HashAlgorithm::SHA256); } - clnt.trailing_header_support = opts.trailing_headers && clnt.override_signer_type == SignatureType::SignatureV4; + client.trailing_header_support = opts.trailing_headers && client.override_signer_type == SignatureType::SignatureV4; - clnt.max_retries = MAX_RETRY; + client.max_retries = MAX_RETRY; if opts.max_retries > 0 { - clnt.max_retries = opts.max_retries; + client.max_retries = opts.max_retries; } - Ok(clnt) + Ok(client) } fn endpoint_url(&self) -> Url { @@ -1315,7 +1326,7 @@ pub struct CreateBucketConfiguration { #[cfg(test)] mod tests { - use super::{build_tls_config, with_rustls_init_guard}; + use super::{build_tls_config, load_root_store_from_tls_path, with_rustls_init_guard}; #[test] fn rustls_guard_converts_panics_to_io_errors() { @@ -1332,4 +1343,37 @@ mod tests { let outcome = std::panic::catch_unwind(build_tls_config); assert!(outcome.is_ok(), "TLS config creation should not panic"); } + + /// When RUSTFS_TLS_PATH is not set, `load_root_store_from_tls_path` must return `None` + /// (i.e. it must not silently look for a CA bundle in the current working directory). + #[test] + fn tls_path_unset_returns_none() { + let result = temp_env::with_var_unset(rustfs_config::ENV_RUSTFS_TLS_PATH, || load_root_store_from_tls_path()); + assert!(result.is_none(), "expected None when RUSTFS_TLS_PATH is unset, but got a root store"); + } + + /// When RUSTFS_TLS_PATH is set to an empty string, `load_root_store_from_tls_path` must + /// return `None` to avoid accidentally trusting a CA bundle in the current directory. + #[test] + fn tls_path_empty_returns_none() { + let result = temp_env::with_var(rustfs_config::ENV_RUSTFS_TLS_PATH, Some(""), || load_root_store_from_tls_path()); + assert!(result.is_none(), "expected None when RUSTFS_TLS_PATH is empty, but got a root store"); + } + + /// Installing the rustls crypto provider when one is already set must not panic or return + /// an error that surfaces to callers (the race-safe `get_default` check guards the install). + #[test] + fn provider_install_is_idempotent() { + // Install once (may already be set by another test in this binary — that's fine). + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + + // A second install attempt on an already-set provider must not panic. + let outcome = std::panic::catch_unwind(|| { + if rustls::crypto::CryptoProvider::get_default().is_none() { + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + } + // If a default is already present, the branch above is simply skipped. + }); + assert!(outcome.is_ok(), "provider install guard must not panic when a provider is already set"); + } } diff --git a/rustfs/src/main.rs b/rustfs/src/main.rs index 30c5d2213..f4e1921aa 100644 --- a/rustfs/src/main.rs +++ b/rustfs/src/main.rs @@ -67,6 +67,7 @@ use rustfs_metrics::init_metrics_system; use rustfs_obs::{init_obs, set_global_guard}; use rustfs_scanner::init_data_scanner; use rustfs_utils::{get_env_bool_with_aliases, net::parse_and_resolve_address}; +use rustls::crypto::aws_lc_rs::default_provider; use std::io::{Error, Result}; use std::sync::Arc; use tokio_util::sync::CancellationToken; @@ -141,6 +142,11 @@ async fn async_main() -> Result<()> { // Initialize trusted proxies system rustfs_trusted_proxies::init(); + // Make sure to use a modern encryption suite + if default_provider().install_default().is_err() { + // A crypto provider is already installed (e.g. by the host process); this is fine. + debug!("rustls crypto provider already installed, skipping aws-lc-rs default install"); + } // Initialize TLS if a certificate path is provided if let Some(tls_path) = &config.tls_path { match init_cert(tls_path).await { diff --git a/rustfs/src/server/cert.rs b/rustfs/src/server/cert.rs index 937bc9284..21c4954d9 100644 --- a/rustfs/src/server/cert.rs +++ b/rustfs/src/server/cert.rs @@ -102,8 +102,6 @@ pub(crate) async fn init_cert(tls_path: &str) -> Result<(), RustFSError> { info!("No TLS path configured; skipping certificate initialization"); return Ok(()); } - // Make sure to use a modern encryption suite - let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); let tls_dir = PathBuf::from(tls_path);