feat(ecstore): Skip rustls provider install if already present (#2145)

Signed-off-by: houseme <housemecn@gmail.com>
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

@@ -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.

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<aws_sdk_s3::config::SharedHttpClient> {
-    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;
     }

crates/ecstore/src/client/transition_api.rs

@@ -139,7 +139,11 @@ pub enum BucketLookupType {
 fn load_root_store_from_tls_path() -> Option<rustls::RootCertStore> {
     // 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<T, F>(build: F) -> Result<T, std::io::Error>
 where
     F: FnOnce() -> Result<T, std::io::Error>,
 {
-    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<rustls::ClientConfig, std::io::Error> {
@@ -198,15 +199,25 @@ fn build_tls_config() -> Result<rustls::ClientConfig, std::io::Error> {
 
 impl TransitionClient {
     pub async fn new(endpoint: &str, opts: Options, tier_type: &str) -> Result<TransitionClient, std::io::Error> {
-        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<TransitionClient, std::io::Error> {
+        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");
+    }
 }