From db8ed404e9843c39ea6ed6d38735e4a0b94a84d5 Mon Sep 17 00:00:00 2001
From: Peter Gerber <peter@arbitrary.ch>
Date: Fri, 14 Jun 2024 15:18:54 +0000
Subject: [PATCH] Deal with SSL_CERT_{FILE,DIR} set by Cargo

Cargo sets these env. vars. internally, at least here, on Linux:

$ cargo init --bin env-conflict
$ cd env-conflict
$ cat >src/main.rs <<EOF
fn main() {
    let _ = dbg!(std::env::var("SSL_CERT_FILE"));
    let _ = dbg!(std::env::var("SSL_CERT_DIR"));
}
EOF
$ cargo -q run
[src/main.rs:2:13] std::env::var("SSL_CERT_FILE") = Ok(
    "/usr/lib/ssl/cert.pem",
)
[src/main.rs:3:13] std::env::var("SSL_CERT_DIR") = Ok(
    "/usr/lib/ssl/certs",
)
---
 tests/common/mod.rs      | 17 +++++++++++++++++
 tests/compare_mozilla.rs |  9 +++++++++
 tests/smoketests.rs      | 38 +++++++++++++++++++++++++++++++++++---
 3 files changed, 61 insertions(+), 3 deletions(-)
 create mode 100644 tests/common/mod.rs

diff --git a/tests/common/mod.rs b/tests/common/mod.rs
new file mode 100644
index 0000000..2e21d99
--- /dev/null
+++ b/tests/common/mod.rs
@@ -0,0 +1,17 @@
+use std::env;
+
+/// Cargo, at least sometimes, sets SSL_CERT_FILE and SSL_CERT_DIR internally,
+/// it uses OpenSSL. So, always unset both at the beginning of a test even if
+/// the test doesn't use either.
+///
+/// # Safety
+///
+/// This is only safe if used together with `#[serial]` because calling
+/// `[env::remove_var()]` is unsafe if another thread is running.
+///
+/// Note that `env::remove_var()` is scheduled to become unsafe in Rust
+/// Edition 2024.
+pub(crate) unsafe fn clear_env() {
+    env::remove_var("SSL_CERT_FILE");
+    env::remove_var("SSL_CERT_DIR");
+}
diff --git a/tests/compare_mozilla.rs b/tests/compare_mozilla.rs
index ffb16ad..e01614e 100644
--- a/tests/compare_mozilla.rs
+++ b/tests/compare_mozilla.rs
@@ -4,10 +4,13 @@
 //! as expressed by the `webpki-roots` crate.
 //!
 //! This is, obviously, quite a heuristic test.
+mod common;
+
 use std::collections::HashMap;
 
 use pki_types::Der;
 use ring::io::der;
+use serial_test::serial;
 use webpki::anchor_from_trusted_cert;
 
 fn stringify_x500name(subject: &Der<'_>) -> String {
@@ -139,7 +142,13 @@ fn test_contains_most_roots_known_by_mozilla() {
 }
 
 #[test]
+#[serial]
 fn util_list_certs() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
+
     let native = rustls_native_certs::load_native_certs().unwrap();
 
     for (i, cert) in native.iter().enumerate() {
diff --git a/tests/smoketests.rs b/tests/smoketests.rs
index 24c0382..0d9c075 100644
--- a/tests/smoketests.rs
+++ b/tests/smoketests.rs
@@ -1,3 +1,5 @@
+mod common;
+
 use std::io::{ErrorKind, Read, Write};
 use std::net::TcpStream;
 #[cfg(unix)]
@@ -63,42 +65,71 @@ fn check_site(domain: &str) -> Result<(), ()> {
 #[test]
 #[serial]
 fn google() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     check_site("google.com").unwrap();
 }
 
 #[test]
 #[serial]
 fn amazon() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     check_site("amazon.com").unwrap();
 }
 
 #[test]
 #[serial]
 fn facebook() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     check_site("facebook.com").unwrap();
 }
 
 #[test]
 #[serial]
 fn netflix() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     check_site("netflix.com").unwrap();
 }
 
 #[test]
 #[serial]
 fn ebay() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     check_site("ebay.com").unwrap();
 }
 
 #[test]
 #[serial]
 fn apple() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     check_site("apple.com").unwrap();
 }
 
 #[test]
 #[serial]
 fn badssl_with_env() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
+
     // Self-signed certs should never be trusted by default:
     assert!(check_site("self-signed.badssl.com").is_err());
 
@@ -109,12 +140,15 @@ fn badssl_with_env() {
         PathBuf::from("./tests/badssl-com-chain.pem"),
     );
     check_site("self-signed.badssl.com").unwrap();
-    env::remove_var("SSL_CERT_FILE");
 }
 
 #[test]
 #[serial]
 fn badssl_with_dir_from_env() {
+    unsafe {
+        // SAFETY: safe because of #[serial]
+        common::clear_env();
+    }
     let temp_dir = tempfile::TempDir::new().unwrap();
     let original = Path::new("tests/badssl-com-chain.pem")
         .canonicalize()
@@ -141,6 +175,4 @@ fn badssl_with_dir_from_env() {
     symlink("/a/path/which/does/not/exist/hopefully", link2).unwrap();
 
     check_site("self-signed.badssl.com").unwrap();
-
-    env::remove_var("SSL_CERT_DIR");
 }