more tests and clean up

iroh-cli/Cargo.toml

@@ -43,7 +43,7 @@ iroh = { version = "0.28.1", path = "../iroh", features = ["metrics"] }
 iroh-blobs = { version = "0.28.1", features = ["cli"] }
 iroh-docs = { version = "0.28.0", features = ["cli"] }
 iroh-gossip = { version = "0.28.1", features = ["cli"] }
-iroh-relay = { version = "0.28.0", path = "../iroh-relay", features = ["test-utils"]}
+iroh-relay = { version = "0.28.0", path = "../iroh-relay", features = ["dangerous-certs"]}
 iroh-metrics = { version = "0.28.0" }
 net-report = { package = "iroh-net-report", path = "../iroh-net-report", version = "0.28" }
 iroh-node-util = { path = "../iroh-node-util", features = ["config", "logging", "cli"] }

iroh-cli/src/commands/doctor.rs

@@ -4,7 +4,7 @@
 use std::{
     collections::HashMap,
     io,
-    net::{Ipv4Addr, Ipv6Addr, SocketAddr},
+    net::{Ipv6Addr, SocketAddr},
     num::NonZeroU16,
     path::PathBuf,
     str::FromStr,
@@ -44,7 +44,7 @@ use iroh::{
 };
 use iroh_metrics::core::Core;
 use iroh_node_util::config::data_root;
-use net_report::QuicConfig;
+use net_report::QuicAddressDiscovery;
 use portable_atomic::AtomicU64;
 use postcard::experimental::max_size::MaxSize;
 use rand::Rng;
@@ -1269,14 +1269,14 @@ pub async fn run(command: Commands, config: &NodeConfig) -> anyhow::Result<()> {
 }
 
 /// Create a quinn Endpoint that has QUIC address discovery enabled.
-fn create_quic_endpoint(dangerous_certs: bool) -> anyhow::Result<QuicConfig> {
+fn create_quic_endpoint(dangerous_certs: bool) -> anyhow::Result<QuicAddressDiscovery> {
     let client_config = match dangerous_certs {
         true => iroh_relay::client::make_dangerous_client_config(),
         false => iroh::net::tls::make_client_config_pki()?,
     };
     let client_config = quinn::ClientConfig::new(Arc::new(client_config));
     let ep = quinn::Endpoint::client(SocketAddr::new(Ipv6Addr::UNSPECIFIED.into(), 0))?;
-    Ok(QuicConfig { ep, client_config })
+    Ok(QuicAddressDiscovery { ep, client_config })
 }
 
 /// Runs the [`PlotterApp`].

iroh-net-report/src/lib.rs

@@ -20,7 +20,7 @@ use iroh_base::relay_map::{RelayMap, RelayNode, RelayUrl};
 use iroh_metrics::inc;
 use iroh_relay::protos::stun;
 use netwatch::{IpFamily, UdpSocket};
-pub use reportgen::QuicConfig;
+pub use reportgen::QuicAddressDiscovery;
 use tokio::{
     sync::{self, mpsc, oneshot},
     time::{Duration, Instant},
@@ -248,10 +248,10 @@ impl Client {
         dm: RelayMap,
         stun_conn4: Option<Arc<UdpSocket>>,
         stun_conn6: Option<Arc<UdpSocket>>,
-        quic_config: Option<QuicConfig>,
+        quic_addr_disc: Option<QuicAddressDiscovery>,
     ) -> Result<Arc<Report>> {
         let rx = self
-            .get_report_channel(dm, stun_conn4, stun_conn6, quic_config)
+            .get_report_channel(dm, stun_conn4, stun_conn6, quic_addr_disc)
             .await?;
         match rx.await {
             Ok(res) => res,
@@ -265,7 +265,7 @@ impl Client {
         dm: RelayMap,
         stun_conn4: Option<Arc<UdpSocket>>,
         stun_conn6: Option<Arc<UdpSocket>>,
-        quic_config: Option<QuicConfig>,
+        quic_addr_disc: Option<QuicAddressDiscovery>,
     ) -> Result<oneshot::Receiver<Result<Arc<Report>>>> {
         // TODO: consider if RelayMap should be made to easily clone?  It seems expensive
         // right now.
@@ -275,7 +275,7 @@ impl Client {
                 relay_map: dm,
                 stun_sock_v4: stun_conn4,
                 stun_sock_v6: stun_conn6,
-                quic_config,
+                quic_addr_disc,
                 response_tx: tx,
             })
             .await?;
@@ -319,7 +319,7 @@ pub(crate) enum Message {
         /// connection to do QUIC address discovery.
         ///
         /// If not provided, will not do QUIC address discovery.
-        quic_config: Option<QuicConfig>,
+        quic_addr_disc: Option<QuicAddressDiscovery>,
         /// Channel to receive the response.
         response_tx: oneshot::Sender<Result<Arc<Report>>>,
     },
@@ -459,14 +459,14 @@ impl Actor {
                     relay_map,
                     stun_sock_v4,
                     stun_sock_v6,
-                    quic_config,
+                    quic_addr_disc,
                     response_tx,
                 } => {
                     self.handle_run_check(
                         relay_map,
                         stun_sock_v4,
                         stun_sock_v6,
-                        quic_config,
+                        quic_addr_disc,
                         response_tx,
                     );
                 }
@@ -496,7 +496,7 @@ impl Actor {
         relay_map: RelayMap,
         stun_sock_v4: Option<Arc<UdpSocket>>,
         stun_sock_v6: Option<Arc<UdpSocket>>,
-        quic_config: Option<QuicConfig>,
+        quic_addr_disc: Option<QuicAddressDiscovery>,
         response_tx: oneshot::Sender<Result<Arc<Report>>>,
     ) {
         if self.current_report_run.is_some() {
@@ -547,7 +547,7 @@ impl Actor {
             relay_map,
             stun_sock_v4,
             stun_sock_v6,
-            quic_config,
+            quic_addr_disc,
             self.dns_resolver.clone(),
         );
 
@@ -803,7 +803,11 @@ mod test_utils {
 
     use std::sync::Arc;
 
-    use iroh_relay::server;
+    use iroh_relay::server::{
+        self,
+        testing::{quic_config, relay_config},
+        ServerConfig,
+    };
 
     use crate::RelayNode;
 
@@ -825,6 +829,30 @@ mod test_utils {
         (server, Arc::new(node_desc))
     }
 
+    pub(crate) async fn relay_with_quic() -> (server::Server, Arc<RelayNode>) {
+        let server_config = ServerConfig {
+            relay: Some(relay_config()),
+            stun: None,
+            quic: Some(quic_config()),
+            metrics_addr: None,
+        };
+        let server = server::Server::spawn(server_config)
+            .await
+            .expect("should serve relay");
+        let node_desc = RelayNode {
+            url: server.https_url().expect("should work as relay"),
+            stun_only: false,
+            stun_port: 0,
+            quic_only: false,
+            quic_port: server
+                .quic_addr()
+                .expect("server should serve quic address discovery")
+                .port(),
+        };
+
+        (server, Arc::new(node_desc))
+    }
+
     /// Create a [`crate::RelayMap`] of the given size.
     ///
     /// This function uses [`relay`]. Note that the returned map uses internal order that will
@@ -1315,4 +1343,41 @@ mod tests {
         task.abort();
         Ok(())
     }
+
+    #[tokio::test]
+    async fn test_quic_basic() -> Result<()> {
+        let _logging_guard = iroh_test::logging::setup();
+        // set up relay server that has quic enabled, but not stun
+        let (server, relay) = test_utils::relay_with_quic().await;
+
+        // set up quic client endpoint to use in the report
+        let client_config = iroh_relay::client::make_dangerous_client_config();
+        let client_config = quinn::ClientConfig::new(Arc::new(client_config));
+        let ep = quinn::Endpoint::client(SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 0))?;
+        let addr = match ep.local_addr()? {
+            SocketAddr::V4(ipp) => ipp,
+            SocketAddr::V6(_) => unreachable!(),
+        };
+        let quic_addr_disc = QuicAddressDiscovery {
+            ep: ep.clone(),
+            client_config,
+        };
+
+        // create a net report client
+        let resolver = crate::dns::tests::resolver();
+        let mut client = Client::new(None, resolver.clone())?;
+
+        let relay_map = RelayMap::from_nodes(vec![relay])?;
+        let r = client
+            .get_report(relay_map, None, None, Some(quic_addr_disc))
+            .await?;
+        assert!(r.ipv4);
+        assert!(r.ipv4_can_send);
+        assert_eq!(r.global_v4, Some(addr));
+
+        // cleanup
+        ep.wait_idle().await;
+        server.shutdown().await?;
+        Ok(())
+    }
 }

iroh-net-report/src/reportgen.rs

@@ -83,7 +83,7 @@ impl Client {
         relay_map: RelayMap,
         stun_sock4: Option<Arc<UdpSocket>>,
         stun_sock6: Option<Arc<UdpSocket>>,
-        quic_config: Option<QuicConfig>,
+        quic_addr_disc: Option<QuicAddressDiscovery>,
         dns_resolver: DnsResolver,
     ) -> Self {
         let (msg_tx, msg_rx) = mpsc::channel(32);
@@ -99,7 +99,7 @@ impl Client {
             relay_map,
             stun_sock4,
             stun_sock6,
-            quic_config,
+            quic_addr_disc,
             report: Report::default(),
             hairpin_actor: hairpin::Client::new(net_report, addr),
             outstanding_tasks: OutstandingTasks::default(),
@@ -174,7 +174,7 @@ struct Actor {
     /// Socket so send IPv6 STUN requests from.
     stun_sock6: Option<Arc<UdpSocket>>,
     /// QUIC configuration to do QUIC address Discovery
-    quic_config: Option<QuicConfig>,
+    quic_addr_disc: Option<QuicAddressDiscovery>,
 
     // Internal state.
     /// The report being built.
@@ -548,7 +548,7 @@ impl Actor {
                 let reportstate = self.addr();
                 let stun_sock4 = self.stun_sock4.clone();
                 let stun_sock6 = self.stun_sock6.clone();
-                let quic_config = self.quic_config.clone();
+                let quic_addr_disc = self.quic_addr_disc.clone();
                 let relay_node = probe.node().clone();
                 let probe = probe.clone();
                 let net_report = self.net_report.clone();
@@ -560,7 +560,7 @@ impl Actor {
                         reportstate,
                         stun_sock4,
                         stun_sock6,
-                        quic_config,
+                        quic_addr_disc,
                         relay_node,
                         probe.clone(),
                         net_report,
@@ -676,7 +676,7 @@ enum ProbeError {
 
 /// Pieces needed to do QUIC address discovery.
 #[derive(Debug, Clone)]
-pub struct QuicConfig {
+pub struct QuicAddressDiscovery {
     /// A QUIC Endpoint
     pub ep: quinn::Endpoint,
     /// A client config.
@@ -691,7 +691,7 @@ async fn run_probe(
     reportstate: Addr,
     stun_sock4: Option<Arc<UdpSocket>>,
     stun_sock6: Option<Arc<UdpSocket>>,
-    quic_config: Option<QuicConfig>,
+    quic_addr_disc: Option<QuicAddressDiscovery>,
     relay_node: Arc<RelayNode>,
     probe: Probe,
     net_report: net_report::Addr,
@@ -781,9 +781,9 @@ async fn run_probe(
         Probe::QuicAddrIpv4 { ref node, .. } | Probe::QuicAddrIpv6 { ref node, .. } => {
             debug!("sending QUIC address discovery prob");
             let url = node.url.clone();
-            match quic_config {
-                Some(quic_config) => {
-                    result = run_quic_addr_probe(quic_config, url, relay_addr, probe).await?
+            match quic_addr_disc {
+                Some(quic_addr_disc) => {
+                    result = run_quic_probe(quic_addr_disc, url, relay_addr, probe).await?
                 }
                 None => {
                     return Err(ProbeError::AbortSet(
@@ -801,9 +801,9 @@ async fn run_probe(
 
 /// Run a QUIC address discovery probe.
 // TODO(ramfox): if this probe is aborted, then the connection will never be
-// properly, causing errors on the server.
-async fn run_quic_addr_probe(
-    quic_config: QuicConfig,
+// properly, possibly causing errors on the server.
+async fn run_quic_probe(
+    quic_addr_disc: QuicAddressDiscovery,
     url: RelayUrl,
     relay_addr: SocketAddr,
     probe: Probe,
@@ -815,7 +815,8 @@ async fn run_quic_addr_probe(
     }
     // TODO(ramfox): what to put here if no host is given?
     let host = url.host_str().unwrap_or("localhost");
-    let quic_client = iroh_relay::quic::QuicClient::new(quic_config.ep, quic_config.client_config);
+    let quic_client =
+        iroh_relay::quic::QuicClient::new(quic_addr_disc.ep, quic_addr_disc.client_config);
     let (addr, latency) = quic_client
         .get_addr_and_latency(relay_addr, host)
         .await
@@ -1552,4 +1553,44 @@ mod tests {
         assert_eq!(ip, relay_url_ip);
         Ok(())
     }
+
+    #[tokio::test]
+    async fn test_quic_probe() -> TestResult {
+        let _logging_guard = iroh_test::logging::setup();
+        let (server, relay) = test_utils::relay().await;
+        let client_config = iroh_relay::client::make_dangerous_client_config();
+        let client_config = quinn::ClientConfig::new(Arc::new(client_config));
+        let ep = quinn::Endpoint::client(SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 0))?;
+        let client_addr = ep.local_addr()?;
+        let quic_addr_disc = QuicAddressDiscovery {
+            ep: ep.clone(),
+            client_config,
+        };
+        let url = relay.url.clone();
+        let port = server.quic_addr().unwrap().port();
+        let probe = Probe::QuicAddrIpv4 {
+            delay: Duration::from_secs(0),
+            node: relay.clone(),
+        };
+        let probe = match run_quic_probe(
+            quic_addr_disc,
+            url,
+            (Ipv4Addr::LOCALHOST, port).into(),
+            probe,
+        )
+        .await
+        {
+            Ok(probe) => probe,
+            Err(e) => match e {
+                ProbeError::AbortSet(err, _) | ProbeError::Error(err, _) => {
+                    return Err(err.into());
+                }
+            },
+        };
+        assert!(probe.ipv4_can_send);
+        assert_eq!(probe.addr.unwrap(), client_addr);
+        ep.wait_idle().await;
+        server.shutdown().await?;
+        Ok(())
+    }
 }

iroh-net/src/magicsock.rs

@@ -2222,10 +2222,10 @@ impl Actor {
         let pconn4 = Some(self.pconn4.as_socket());
         let pconn6 = self.pconn6.as_ref().map(|p| p.as_socket());
         let quic_endpoint = self.msock.quic_endpoint.read().expect("poisoned").clone();
-        let quic_config = match quic_endpoint {
+        let quic_addr_disc = match quic_endpoint {
             Some(ep) => {
                 match crate::tls::make_client_config(&self.msock.secret_key, None, vec![], false) {
-                    Ok(client_config) => Some(net_report::QuicConfig {
+                    Ok(client_config) => Some(net_report::QuicAddressDiscovery {
                         ep,
                         client_config: quinn::ClientConfig::new(Arc::new(client_config)),
                     }),
@@ -2241,7 +2241,7 @@ impl Actor {
         debug!("requesting net_report report");
         match self
             .net_reporter
-            .get_report_channel(relay_map, pconn4, pconn6, quic_config)
+            .get_report_channel(relay_map, pconn4, pconn6, quic_addr_disc)
             .await
         {
             Ok(rx) => {

iroh-net/src/test_utils.rs

@@ -62,7 +62,7 @@ pub async fn run_relay_server_with(
     stun: Option<StunConfig>,
     quic: bool,
 ) -> Result<(RelayMap, RelayUrl, Server)> {
-    let (certs, server_config) = iroh_relay::server::testing::tls_certs_and_config();
+    let (certs, server_config) = iroh_relay::server::testing::self_signed_tls_certs_and_config();
     let quic = if quic {
         Some(QuicConfig::new(
             server_config.clone(),