From 86e6d5ce15842d775e33d85c2d535df671ecc9c5 Mon Sep 17 00:00:00 2001
From: Andrey Mnatsakanov <andrey@3ig.dev>
Date: Tue, 4 Aug 2026 11:08:28 +0200
Subject: [PATCH] fix(net): treat empty SSL_CERT_FILE/SSL_CERT_DIR as unset in
 the stealth client

The stealth (wreq) client gated its custom-trust-store build on
`var_os(...).is_some()`, which is true for a set-but-empty variable. Supplying
a store to wreq's `tls_cert_store` REPLACES the bundled webpki roots, so an
empty `SSL_CERT_FILE` (a common shell accident, e.g. `export SSL_CERT_FILE=$UNSET`)
built a near-empty default-paths store and made every HTTPS request fail with
CERTIFICATE_VERIFY_FAILED -- while the reqwest client, which already filters
empty values in configured_root_paths(), kept working. Same input, opposite
behavior.

Extract a shared `custom_cert_store_requested` predicate in client.rs that
applies the same `!is_empty()` filter, and call it from the stealth client's
guard. The predicate takes the values as arguments so it is unit-tested under
default features (the stealth feature pulls in boringssl and is not built in
the normal test job).

Fixes #551
---
 crates/obscura-net/src/client.rs      | 47 +++++++++++++++++++++++++++
 crates/obscura-net/src/wreq_client.rs |  7 ++--
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/crates/obscura-net/src/client.rs b/crates/obscura-net/src/client.rs
index c86c1f2..ce97854 100644
--- a/crates/obscura-net/src/client.rs
+++ b/crates/obscura-net/src/client.rs
@@ -60,6 +60,23 @@ fn configured_root_certificates() -> &'static [reqwest::Certificate] {
     })
 }
 
+/// Whether SSL_CERT_FILE / SSL_CERT_DIR request a custom TLS trust store. A
+/// variable that is set but empty (e.g. `SSL_CERT_FILE=""`, a common shell
+/// accident) is treated as unset, matching the `!is_empty()` filter in
+/// `configured_root_paths` above. The stealth client (wreq) relies on this:
+/// supplying a store to `tls_cert_store` REPLACES the bundled webpki roots, so
+/// an empty value would otherwise build a near-empty store and break all HTTPS.
+///
+/// The only non-test caller is the stealth (wreq) client, so a plain build
+/// without the `stealth` feature sees it as unused.
+#[cfg_attr(not(feature = "stealth"), allow(dead_code))]
+pub(crate) fn custom_cert_store_requested(
+    cert_file: Option<&std::ffi::OsStr>,
+    cert_dir: Option<&std::ffi::OsStr>,
+) -> bool {
+    cert_file.is_some_and(|v| !v.is_empty()) || cert_dir.is_some_and(|v| !v.is_empty())
+}
+
 #[derive(Debug, Clone)]
 pub struct Response {
     pub url: Url,
@@ -1014,3 +1031,33 @@ mod ssrf_tests {
         assert!(client.fetch(&url).await.is_err(), "unknown CA must be rejected");
     }
 }
+
+#[cfg(test)]
+mod cert_env_tests {
+    use super::custom_cert_store_requested;
+    use std::ffi::OsStr;
+
+    #[test]
+    fn empty_ssl_cert_env_is_treated_as_unset() {
+        // Set-but-empty must NOT request a custom store: for the stealth client
+        // that would replace the webpki roots with a near-empty default-paths
+        // store and break all HTTPS.
+        assert!(!custom_cert_store_requested(Some(OsStr::new("")), None));
+        assert!(!custom_cert_store_requested(None, Some(OsStr::new(""))));
+        assert!(!custom_cert_store_requested(
+            Some(OsStr::new("")),
+            Some(OsStr::new(""))
+        ));
+        // Genuinely unset: no custom store.
+        assert!(!custom_cert_store_requested(None, None));
+        // Set and non-empty: build the custom store (behavior unchanged).
+        assert!(custom_cert_store_requested(
+            Some(OsStr::new("/etc/corp/ca.pem")),
+            None
+        ));
+        assert!(custom_cert_store_requested(
+            None,
+            Some(OsStr::new("/etc/ssl/certs"))
+        ));
+    }
+}
diff --git a/crates/obscura-net/src/wreq_client.rs b/crates/obscura-net/src/wreq_client.rs
index 397e1c1..1af6158 100644
--- a/crates/obscura-net/src/wreq_client.rs
+++ b/crates/obscura-net/src/wreq_client.rs
@@ -77,9 +77,10 @@ impl StealthHttpClient {
         //    `tls/conn/ext.rs`), it does not add to them. Applying it unconditionally would break
         //    every ordinary site whenever the bundle is incomplete. With neither variable set,
         //    behaviour is byte-for-byte what it was before.
-        if std::env::var_os("SSL_CERT_FILE").is_some()
-            || std::env::var_os("SSL_CERT_DIR").is_some()
-        {
+        if crate::client::custom_cert_store_requested(
+            std::env::var_os("SSL_CERT_FILE").as_deref(),
+            std::env::var_os("SSL_CERT_DIR").as_deref(),
+        ) {
             match wreq::tls::trust::CertStore::builder().set_default_paths().build() {
                 Ok(store) => builder = builder.tls_cert_store(store),
                 Err(error) => tracing::warn!(