From 1357179578a6ec17628706bc19d6220c96f98356 Mon Sep 17 00:00:00 2001 From: Frando Date: Tue, 30 Sep 2025 12:37:27 +0200 Subject: [PATCH] fix: remove final dot in relay urls when doing https requests --- iroh-base/src/relay_url.rs | 48 ++++++++++++++++++++++++++++++++ iroh/src/net_report.rs | 4 +-- iroh/src/net_report/reportgen.rs | 14 ++++++---- 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/iroh-base/src/relay_url.rs b/iroh-base/src/relay_url.rs index 5925a39f09..76a30b0f4b 100644 --- a/iroh-base/src/relay_url.rs +++ b/iroh-base/src/relay_url.rs @@ -38,6 +38,43 @@ impl From for RelayUrl { } } +impl RelayUrl { + /// Returns the URL while removing the final dot in the relay URL's domain name. + /// + /// By default, we add a final dot to relay URLs to make sure that DNS resolution always + /// considers them as top-level domains without appending a search suffix. When using + /// the URL for TLS hostname verification, usually a domain name without a final + /// dot is expected. So when using the URL in the context of HTTPS or TLS, use + /// this function to get the URL without the final dot. + pub fn without_final_dot(&self) -> Url { + let mut url = self.0.deref().clone(); + if let Some(domain) = url.domain() { + if let Some(stripped) = domain.strip_suffix('.') { + let stripped = stripped.to_string(); + url.set_host(Some(&stripped)).ok(); + } + } + url + } + + /// Return the string representation of the host (domain or IP address) for this URL, if any. + /// + /// If the host is a domain, and the domain ends with a final dot, the final dot is removed. + /// + /// See [`Self::without_final_dot`] for details on when you might want to use this. + pub fn host_str_without_final_dot(&self) -> Option<&str> { + if let Some(domain) = self.0.domain() { + if let Some(stripped) = domain.strip_suffix('.') { + Some(stripped) + } else { + Some(domain) + } + } else { + self.0.host_str() + } + } +} + /// Can occur when parsing a string into a [`RelayUrl`]. #[stack_error(derive, add_meta)] #[error("Failed to parse relay URL")] @@ -124,5 +161,16 @@ mod tests { let url3 = RelayUrl::from(Url::parse("https://example.com/").unwrap()); assert_eq!(url, url3); + + // tests `RelayUrl::without_final_dot` + assert_eq!(url.deref(), &Url::parse("https://example.com.").unwrap()); + assert_eq!( + url.without_final_dot(), + Url::parse("https://example.com").unwrap() + ); + + // tests `RelayUrl::host_str_without_final_dot` + assert_eq!(url.host_str(), Some("example.com.")); + assert_eq!(url.host_str_without_final_dot(), Some("example.com")); } } diff --git a/iroh/src/net_report.rs b/iroh/src/net_report.rs index 24484bcaea..19941fd858 100644 --- a/iroh/src/net_report.rs +++ b/iroh/src/net_report.rs @@ -779,7 +779,7 @@ async fn run_probe_v4( debug!(?relay_addr_orig, ?relay_addr, "relay addr v4"); let host = relay .url - .host_str() + .host_str_without_final_dot() .ok_or_else(|| e!(QadProbeError::MissingHost))?; let conn = quic_client .create_conn(relay_addr, host) @@ -853,7 +853,7 @@ async fn run_probe_v6( debug!(?relay_addr_orig, ?relay_addr, "relay addr v6"); let host = relay .url - .host_str() + .host_str_without_final_dot() .ok_or_else(|| e!(QadProbeError::MissingHost))?; let conn = quic_client .create_conn(relay_addr, host) diff --git a/iroh/src/net_report/reportgen.rs b/iroh/src/net_report/reportgen.rs index 8b9fe40642..3e86ab3839 100644 --- a/iroh/src/net_report/reportgen.rs +++ b/iroh/src/net_report/reportgen.rs @@ -606,7 +606,7 @@ async fn check_captive_portal( // length is limited; see is_challenge_char in bin/iroh-relay for more // details. - let host_name = url.host_str().unwrap_or_default(); + let host_name = url.host_str_without_final_dot().unwrap_or_default(); let challenge = format!("ts_{host_name}"); let portal_url = format!("http://{host_name}/generate_204"); let res = client @@ -789,7 +789,9 @@ async fn run_https_probe( #[cfg(any(test, feature = "test-utils"))] insecure_skip_relay_cert_verify: bool, ) -> Result { trace!("HTTPS probe start"); - let url = relay.join(RELAY_PROBE_PATH)?; + // Convert the relay URL to a URL that has no final dot, because the final dot + // may trip up the certificate verification. + let url = relay.without_final_dot().join(RELAY_PROBE_PATH)?; // This should also use same connection establishment as relay client itself, which // needs to be more configurable so users can do more crazy things: @@ -802,7 +804,9 @@ async fn run_https_probe( } #[cfg(not(wasm_browser))] - if let Some(Host::Domain(domain)) = url.host() { + if let (Some(Host::Domain(domain_for_dns)), Some(Host::Domain(domain_for_reqwest))) = + (relay.host(), url.host()) + { // Use our own resolver rather than getaddrinfo // // Be careful, a non-zero port will override the port in the URI. @@ -811,12 +815,12 @@ async fn run_https_probe( // but staggered for reliability. Ideally this tries to resolve **both** IPv4 and // IPv6 though. But our resolver does not have a function for that yet. let addrs: Vec<_> = dns_resolver - .lookup_ipv4_ipv6_staggered(domain, DNS_TIMEOUT, DNS_STAGGERING_MS) + .lookup_ipv4_ipv6_staggered(domain_for_dns, DNS_TIMEOUT, DNS_STAGGERING_MS) .await? .map(|ipaddr| SocketAddr::new(ipaddr, 0)) .collect(); trace!(?addrs, "resolved addrs"); - builder = builder.resolve_to_addrs(domain, &addrs); + builder = builder.resolve_to_addrs(domain_for_reqwest, &addrs); } #[cfg(all(not(wasm_browser), any(test, feature = "test-utils")))]