fix: remove final dot in relay urls when doing https requests

Author: Frando

iroh-base/src/relay_url.rs

@@ -38,6 +38,43 @@ impl From<Url> 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"));
     }
 }

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)

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<HttpsProbeReport, MeasureHttpsLatencyError> {
     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")))]