From e60c2aa30930dac0af39a92ee53f8cb7e3da987d Mon Sep 17 00:00:00 2001 From: Sebastian Zivota Date: Wed, 6 Mar 2024 17:41:40 +0100 Subject: [PATCH 1/4] feat: Create a no_ssl client and add it to HTTP and GCS downloaders --- .../symbolicator-service/src/download/gcs.rs | 34 +++++++++++++++---- .../symbolicator-service/src/download/http.rs | 14 +++++--- .../symbolicator-service/src/download/mod.rs | 22 +++++++++--- 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/crates/symbolicator-service/src/download/gcs.rs b/crates/symbolicator-service/src/download/gcs.rs index 746db693d..74d9ae97e 100644 --- a/crates/symbolicator-service/src/download/gcs.rs +++ b/crates/symbolicator-service/src/download/gcs.rs @@ -17,16 +17,24 @@ type GcsTokenCache = moka::future::Cache, CacheEntry pub struct GcsDownloader { token_cache: GcsTokenCache, client: reqwest::Client, + #[allow(unused)] + no_ssl_client: reqwest::Client, timeouts: DownloadTimeouts, } impl GcsDownloader { - pub fn new(client: reqwest::Client, timeouts: DownloadTimeouts, token_capacity: u64) -> Self { + pub fn new( + client: reqwest::Client, + no_ssl_client: reqwest::Client, + timeouts: DownloadTimeouts, + token_capacity: u64, + ) -> Self { Self { token_cache: GcsTokenCache::builder() .max_capacity(token_capacity) .build(), client, + no_ssl_client, timeouts, } } @@ -106,8 +114,12 @@ mod tests { test::setup(); let source = gcs_source(test::gcs_source_key!()); - let downloader = - GcsDownloader::new(Client::new(), Default::default(), 100.try_into().unwrap()); + let downloader = GcsDownloader::new( + Client::new(), + Client::new(), + Default::default(), + 100.try_into().unwrap(), + ); let tempdir = test::tempdir(); let target_path = tempdir.path().join("myfile"); @@ -134,8 +146,12 @@ mod tests { test::setup(); let source = gcs_source(test::gcs_source_key!()); - let downloader = - GcsDownloader::new(Client::new(), Default::default(), 100.try_into().unwrap()); + let downloader = GcsDownloader::new( + Client::new(), + Client::new(), + Default::default(), + 100.try_into().unwrap(), + ); let tempdir = test::tempdir(); let target_path = tempdir.path().join("myfile"); @@ -161,8 +177,12 @@ mod tests { }; let source = gcs_source(broken_credentials); - let downloader = - GcsDownloader::new(Client::new(), Default::default(), 100.try_into().unwrap()); + let downloader = GcsDownloader::new( + Client::new(), + Client::new(), + Default::default(), + 100.try_into().unwrap(), + ); let tempdir = test::tempdir(); let target_path = tempdir.path().join("myfile"); diff --git a/crates/symbolicator-service/src/download/http.rs b/crates/symbolicator-service/src/download/http.rs index bcc3c355a..5d092ac6e 100644 --- a/crates/symbolicator-service/src/download/http.rs +++ b/crates/symbolicator-service/src/download/http.rs @@ -14,12 +14,18 @@ use super::USER_AGENT; #[derive(Debug)] pub struct HttpDownloader { client: Client, + #[allow(unused)] + no_ssl_client: Client, timeouts: DownloadTimeouts, } impl HttpDownloader { - pub fn new(client: Client, timeouts: DownloadTimeouts) -> Self { - Self { client, timeouts } + pub fn new(client: Client, no_ssl_client: Client, timeouts: DownloadTimeouts) -> Self { + Self { + client, + no_ssl_client, + timeouts, + } } /// Downloads a source hosted on an HTTP server. @@ -73,7 +79,7 @@ mod tests { let loc = SourceLocation::new("hello.txt"); let file_source = HttpRemoteFile::new(http_source, loc); - let downloader = HttpDownloader::new(Client::new(), Default::default()); + let downloader = HttpDownloader::new(Client::new(), Client::new(), Default::default()); let mut destination = tokio::fs::File::create(&dest).await.unwrap(); let download_status = downloader .download_source("", &file_source, &mut destination) @@ -100,7 +106,7 @@ mod tests { let loc = SourceLocation::new("i-do-not-exist"); let file_source = HttpRemoteFile::new(http_source, loc); - let downloader = HttpDownloader::new(Client::new(), Default::default()); + let downloader = HttpDownloader::new(Client::new(), Client::new(), Default::default()); let mut destination = tokio::fs::File::create(&dest).await.unwrap(); let download_status = downloader .download_source("", &file_source, &mut destination) diff --git a/crates/symbolicator-service/src/download/mod.rs b/crates/symbolicator-service/src/download/mod.rs index c09573f5e..d0db514ca 100644 --- a/crates/symbolicator-service/src/download/mod.rs +++ b/crates/symbolicator-service/src/download/mod.rs @@ -231,11 +231,16 @@ impl DownloadService { pub fn new(config: &Config, runtime: tokio::runtime::Handle) -> Arc { let timeouts = DownloadTimeouts::from_config(config); - // The trusted client can always connect to reserved IPs. The restricted client only can if it's - // explicitly allowed in the config. + // | client | can connect to reserved IPs | accepts invalid SSL certs | + // | -----------| ----------------------------|---------------------------| + // | trusted | yes | no | + // | restricted | according to config setting | no | + // | no_ssl | according to config setting | yes | let trusted_client = crate::utils::http::create_client(&timeouts, true, false); let restricted_client = crate::utils::http::create_client(&timeouts, config.connect_to_reserved_ips, false); + let no_ssl_client = + crate::utils::http::create_client(&timeouts, config.connect_to_reserved_ips, true); let in_memory = &config.caches.in_memory; Arc::new(Self { @@ -243,9 +248,18 @@ impl DownloadService { timeouts, trusted_client: trusted_client.clone(), sentry: sentry::SentryDownloader::new(trusted_client, runtime, timeouts, in_memory), - http: http::HttpDownloader::new(restricted_client.clone(), timeouts), + http: http::HttpDownloader::new( + restricted_client.clone(), + no_ssl_client.clone(), + timeouts, + ), s3: s3::S3Downloader::new(timeouts, in_memory.s3_client_capacity), - gcs: gcs::GcsDownloader::new(restricted_client, timeouts, in_memory.gcs_token_capacity), + gcs: gcs::GcsDownloader::new( + restricted_client, + no_ssl_client, + timeouts, + in_memory.gcs_token_capacity, + ), fs: filesystem::FilesystemDownloader::new(), host_deny_list: HostDenyList::from_config(config), connect_to_reserved_ips: config.connect_to_reserved_ips, From 83dc347763b91f48e98612039f51b0721401340c Mon Sep 17 00:00:00 2001 From: Sebastian Zivota Date: Wed, 6 Mar 2024 18:00:05 +0100 Subject: [PATCH 2/4] Remove no-ssl client from GCS --- .../symbolicator-service/src/download/gcs.rs | 34 ++++--------------- .../symbolicator-service/src/download/mod.rs | 13 ++----- 2 files changed, 9 insertions(+), 38 deletions(-) diff --git a/crates/symbolicator-service/src/download/gcs.rs b/crates/symbolicator-service/src/download/gcs.rs index 74d9ae97e..746db693d 100644 --- a/crates/symbolicator-service/src/download/gcs.rs +++ b/crates/symbolicator-service/src/download/gcs.rs @@ -17,24 +17,16 @@ type GcsTokenCache = moka::future::Cache, CacheEntry pub struct GcsDownloader { token_cache: GcsTokenCache, client: reqwest::Client, - #[allow(unused)] - no_ssl_client: reqwest::Client, timeouts: DownloadTimeouts, } impl GcsDownloader { - pub fn new( - client: reqwest::Client, - no_ssl_client: reqwest::Client, - timeouts: DownloadTimeouts, - token_capacity: u64, - ) -> Self { + pub fn new(client: reqwest::Client, timeouts: DownloadTimeouts, token_capacity: u64) -> Self { Self { token_cache: GcsTokenCache::builder() .max_capacity(token_capacity) .build(), client, - no_ssl_client, timeouts, } } @@ -114,12 +106,8 @@ mod tests { test::setup(); let source = gcs_source(test::gcs_source_key!()); - let downloader = GcsDownloader::new( - Client::new(), - Client::new(), - Default::default(), - 100.try_into().unwrap(), - ); + let downloader = + GcsDownloader::new(Client::new(), Default::default(), 100.try_into().unwrap()); let tempdir = test::tempdir(); let target_path = tempdir.path().join("myfile"); @@ -146,12 +134,8 @@ mod tests { test::setup(); let source = gcs_source(test::gcs_source_key!()); - let downloader = GcsDownloader::new( - Client::new(), - Client::new(), - Default::default(), - 100.try_into().unwrap(), - ); + let downloader = + GcsDownloader::new(Client::new(), Default::default(), 100.try_into().unwrap()); let tempdir = test::tempdir(); let target_path = tempdir.path().join("myfile"); @@ -177,12 +161,8 @@ mod tests { }; let source = gcs_source(broken_credentials); - let downloader = GcsDownloader::new( - Client::new(), - Client::new(), - Default::default(), - 100.try_into().unwrap(), - ); + let downloader = + GcsDownloader::new(Client::new(), Default::default(), 100.try_into().unwrap()); let tempdir = test::tempdir(); let target_path = tempdir.path().join("myfile"); diff --git a/crates/symbolicator-service/src/download/mod.rs b/crates/symbolicator-service/src/download/mod.rs index d0db514ca..470d1d1da 100644 --- a/crates/symbolicator-service/src/download/mod.rs +++ b/crates/symbolicator-service/src/download/mod.rs @@ -248,18 +248,9 @@ impl DownloadService { timeouts, trusted_client: trusted_client.clone(), sentry: sentry::SentryDownloader::new(trusted_client, runtime, timeouts, in_memory), - http: http::HttpDownloader::new( - restricted_client.clone(), - no_ssl_client.clone(), - timeouts, - ), + http: http::HttpDownloader::new(restricted_client.clone(), no_ssl_client, timeouts), s3: s3::S3Downloader::new(timeouts, in_memory.s3_client_capacity), - gcs: gcs::GcsDownloader::new( - restricted_client, - no_ssl_client, - timeouts, - in_memory.gcs_token_capacity, - ), + gcs: gcs::GcsDownloader::new(restricted_client, timeouts, in_memory.gcs_token_capacity), fs: filesystem::FilesystemDownloader::new(), host_deny_list: HostDenyList::from_config(config), connect_to_reserved_ips: config.connect_to_reserved_ips, From 60d32c266343f23f9b02763b58823574804e0777 Mon Sep 17 00:00:00 2001 From: Sebastian Zivota Date: Thu, 7 Mar 2024 10:41:03 +0100 Subject: [PATCH 3/4] feat: Add `accept_invalid_certs` flag to HttpSource --- crates/symbolicator-service/src/download/http.rs | 9 +++++++-- crates/symbolicator-sources/src/sources/http.rs | 7 +++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/symbolicator-service/src/download/http.rs b/crates/symbolicator-service/src/download/http.rs index 5d092ac6e..d69c70c8c 100644 --- a/crates/symbolicator-service/src/download/http.rs +++ b/crates/symbolicator-service/src/download/http.rs @@ -14,7 +14,6 @@ use super::USER_AGENT; #[derive(Debug)] pub struct HttpDownloader { client: Client, - #[allow(unused)] no_ssl_client: Client, timeouts: DownloadTimeouts, } @@ -38,7 +37,13 @@ impl HttpDownloader { let download_url = file_source.url().map_err(|_| CacheError::NotFound)?; tracing::debug!("Fetching debug file from `{}`", download_url); - let mut builder = self.client.get(download_url); + + // Use `self.no_ssl_client` if the source is configured to accept invalid SSL certs + let mut builder = if file_source.source.accept_invalid_certs { + self.no_ssl_client.get(download_url) + } else { + self.client.get(download_url) + }; let headers = file_source .source diff --git a/crates/symbolicator-sources/src/sources/http.rs b/crates/symbolicator-sources/src/sources/http.rs index 8e37b00a6..69d330820 100644 --- a/crates/symbolicator-sources/src/sources/http.rs +++ b/crates/symbolicator-sources/src/sources/http.rs @@ -22,6 +22,13 @@ pub struct HttpSourceConfig { /// Configuration common to all sources. #[serde(flatten)] pub files: CommonSourceConfig, + + /// If true, it should be possible to download from this source + /// even if SSL certificates can't be verified. + /// + /// Don't use this lightly! + #[serde(default)] + pub accept_invalid_certs: bool, } /// The HTTP-specific [`RemoteFile`]. From efaa4efe73ccaa03057684bd20ebdd3030c7cbee Mon Sep 17 00:00:00 2001 From: Sebastian Zivota Date: Thu, 7 Mar 2024 11:54:22 +0100 Subject: [PATCH 4/4] Set new field everywhere --- crates/symbolicator-native/tests/integration/e2e.rs | 3 +++ crates/symbolicator-native/tests/integration/public_sources.rs | 2 ++ crates/symbolicator-sources/src/sources/http.rs | 1 + crates/symbolicator-test/src/lib.rs | 3 +++ 4 files changed, 9 insertions(+) diff --git a/crates/symbolicator-native/tests/integration/e2e.rs b/crates/symbolicator-native/tests/integration/e2e.rs index 92b25a4d1..7f31e0003 100644 --- a/crates/symbolicator-native/tests/integration/e2e.rs +++ b/crates/symbolicator-native/tests/integration/e2e.rs @@ -205,6 +205,7 @@ async fn test_reserved_ip_addresses() { url: url.clone(), headers: Default::default(), files: files.clone(), + accept_invalid_certs: false, }))); url.set_host(Some("127.0.0.1")).unwrap(); @@ -213,6 +214,7 @@ async fn test_reserved_ip_addresses() { url: url.clone(), headers: Default::default(), files: files.clone(), + accept_invalid_certs: false, }))); url.set_host(Some("localhost")).unwrap(); @@ -221,6 +223,7 @@ async fn test_reserved_ip_addresses() { url, headers: Default::default(), files, + accept_invalid_certs: false, }))); let request = example_request(sources); diff --git a/crates/symbolicator-native/tests/integration/public_sources.rs b/crates/symbolicator-native/tests/integration/public_sources.rs index ac8fc3360..24f65127c 100644 --- a/crates/symbolicator-native/tests/integration/public_sources.rs +++ b/crates/symbolicator-native/tests/integration/public_sources.rs @@ -23,6 +23,7 @@ async fn test_nuget_source() { .unwrap(), headers: Default::default(), files: source_config(DirectoryLayoutType::Symstore, vec![FileType::PortablePdb]), + accept_invalid_certs: false, })); let request = make_symbolication_request( @@ -60,6 +61,7 @@ async fn test_ubuntu_source() { DirectoryLayoutType::Debuginfod, vec![FileType::ElfCode, FileType::ElfDebug], ), + accept_invalid_certs: false, })); let request = make_symbolication_request( diff --git a/crates/symbolicator-sources/src/sources/http.rs b/crates/symbolicator-sources/src/sources/http.rs index 69d330820..e7059b599 100644 --- a/crates/symbolicator-sources/src/sources/http.rs +++ b/crates/symbolicator-sources/src/sources/http.rs @@ -65,6 +65,7 @@ impl HttpRemoteFile { url, headers: Default::default(), files: Default::default(), + accept_invalid_certs: false, }); let location = SourceLocation::new(""); diff --git a/crates/symbolicator-test/src/lib.rs b/crates/symbolicator-test/src/lib.rs index b98c83475..e42b8b42f 100644 --- a/crates/symbolicator-test/src/lib.rs +++ b/crates/symbolicator-test/src/lib.rs @@ -127,6 +127,7 @@ pub fn microsoft_symsrv() -> SourceConfig { }, ..Default::default() }, + accept_invalid_certs: false, })) } @@ -305,6 +306,7 @@ impl Server { url: self.url(path), headers: Default::default(), files, + accept_invalid_certs: false, })) } } @@ -340,6 +342,7 @@ pub fn symbol_server() -> (Server, SourceConfig) { url: server.url("symbols/"), headers: Default::default(), files: Default::default(), + accept_invalid_certs: false, })); (server, source)