feat(iroh-net): ICMPv6 probe support in netcheck (#2057)

## Description

Now that we support IPv6 it becomes worth it to add an ICMPv6 probe.
This adds the probe, the two main features are:
- latency from the probe is used for the derper selection.
- the information is used in network change detection.

Note that this also tweaks the definition of this for network change
detection a bit.  If one of the netcheck reports did not include any
information about ICMP availability then it is ignored in the
comparison.  This makes sense because the report stops before ICMP
probes are run if things are decent, so just because in one report
some STUN UDP packets got lost and the ICMP probe did get to run
does not mean anything changed.

This also improves respecting the STUN-only setting of derp servers,
all probes now respect this.

## Notes & open questions

Some other changes that are more incidental:

- DerpMap::nodes() now returns just the values of the map.  Each node 
  knows it's URL anyway.

- Add separate DNS lookup functions for IPv4 and IPv6.  Probes want
  that level of control anyway.

- Use stricter types for global_v4 and global_v6 fields in netcheck
  report.

- Minor refactor in reportgen to split off processing probe reports
  and making probe logic decisions.  This separates concerns better
  and makes testing purely functional bits easier.

- Individual probes no longer have a timeout.  The global probe timeout
  is all that matters.  The individual probe timeout was already applied
  irregularly between the different probe types.

Closes #1853 

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

iroh-net/src/config.rs

@@ -63,8 +63,11 @@ pub struct NetInfo {
     /// Whether the host has UDP internet connectivity.
     pub working_udp: Option<bool>,
 
-    /// Whether ICMPv4 works. Empty means not checked.
-    pub working_icm_pv4: Option<bool>,
+    /// Whether ICMPv4 works, `None` means not checked.
+    pub working_icmp_v4: Option<bool>,
+
+    /// Whether ICMPv6 works, `None` means not checked.
+    pub working_icmp_v6: Option<bool>,
 
     /// Whether we have an existing portmap open (UPnP, PMP, or PCP).
     pub have_port_map: bool,
@@ -91,12 +94,21 @@ pub struct NetInfo {
 impl NetInfo {
     /// reports whether `self` and `other` are basically equal, ignoring changes in DERP ServerLatency & DerpLatency.
     pub fn basically_equal(&self, other: &Self) -> bool {
+        let eq_icmp_v4 = match (self.working_icmp_v4, other.working_icmp_v4) {
+            (Some(slf), Some(other)) => slf == other,
+            _ => true, // ignore for comparison if only one report had this info
+        };
+        let eq_icmp_v6 = match (self.working_icmp_v6, other.working_icmp_v6) {
+            (Some(slf), Some(other)) => slf == other,
+            _ => true, // ignore for comparison if only one report had this info
+        };
         self.mapping_varies_by_dest_ip == other.mapping_varies_by_dest_ip
             && self.hair_pinning == other.hair_pinning
             && self.working_ipv6 == other.working_ipv6
             && self.os_has_ipv6 == other.os_has_ipv6
             && self.working_udp == other.working_udp
-            && self.working_icm_pv4 == other.working_icm_pv4
+            && eq_icmp_v4
+            && eq_icmp_v6
             && self.have_port_map == other.have_port_map
             && self.portmap_probe == other.portmap_probe
             && self.preferred_derp == other.preferred_derp

iroh-net/src/derp/map.rs

@@ -41,8 +41,8 @@ impl DerpMap {
     }
 
     /// Returns an `Iterator` over all known nodes.
-    pub fn nodes(&self) -> impl Iterator<Item = (&DerpUrl, &Arc<DerpNode>)> {
-        self.nodes.iter()
+    pub fn nodes(&self) -> impl Iterator<Item = &Arc<DerpNode>> {
+        self.nodes.values()
     }
 
     /// Is this a known node?

iroh-net/src/dns.rs

@@ -23,6 +23,22 @@ fn get_resolver() -> Result<TokioAsyncResolver> {
     Ok(resolver)
 }
 
+pub(crate) async fn lookup_ipv4<N: IntoName + TryParseIp + Clone>(
+    host: N,
+    timeout: Duration,
+) -> Result<Vec<IpAddr>> {
+    let addrs = tokio::time::timeout(timeout, DNS_RESOLVER.ipv4_lookup(host)).await??;
+    Ok(addrs.into_iter().map(|ip| IpAddr::V4(ip.0)).collect())
+}
+
+pub(crate) async fn lookup_ipv6<N: IntoName + TryParseIp + Clone>(
+    host: N,
+    timeout: Duration,
+) -> Result<Vec<IpAddr>> {
+    let addrs = tokio::time::timeout(timeout, DNS_RESOLVER.ipv6_lookup(host)).await??;
+    Ok(addrs.into_iter().map(|ip| IpAddr::V6(ip.0)).collect())
+}
+
 /// Resolve IPv4 and IPv6 in parallel.
 ///
 /// `LookupIpStrategy::Ipv4AndIpv6` will wait for ipv6 resolution timeout, even if it is

iroh-net/src/magicsock.rs

@@ -1922,7 +1922,7 @@ impl Actor {
 
         if let Some(nr) = nr {
             if let Some(global_v4) = nr.global_v4 {
-                add_addr!(already, eps, global_v4, config::EndpointType::Stun);
+                add_addr!(already, eps, global_v4.into(), config::EndpointType::Stun);
 
                 // If they're behind a hard NAT and are using a fixed
                 // port locally, assume they might've added a static
@@ -1932,11 +1932,16 @@ impl Actor {
                 if nr.mapping_varies_by_dest_ip.unwrap_or_default() && port != 0 {
                     let mut addr = global_v4;
                     addr.set_port(port);
-                    add_addr!(already, eps, addr, config::EndpointType::Stun4LocalPort);
+                    add_addr!(
+                        already,
+                        eps,
+                        addr.into(),
+                        config::EndpointType::Stun4LocalPort
+                    );
                 }
             }
             if let Some(global_v6) = nr.global_v6 {
-                add_addr!(already, eps, global_v6, config::EndpointType::Stun);
+                add_addr!(already, eps, global_v6.into(), config::EndpointType::Stun);
             }
         }
         let local_addr_v4 = self.pconn4.local_addr().ok();
@@ -2163,7 +2168,8 @@ impl Actor {
                 working_ipv6: Some(r.ipv6),
                 os_has_ipv6: Some(r.os_has_ipv6),
                 working_udp: Some(r.udp),
-                working_icm_pv4: Some(r.icmpv4),
+                working_icmp_v4: r.icmpv4,
+                working_icmp_v6: r.icmpv6,
                 preferred_derp: r.preferred_derp.clone(),
                 link_type: None,
             };

iroh-net/src/net/interfaces.rs

@@ -272,7 +272,7 @@ impl State {
         let ifname = fake.iface.name.clone();
         Self {
             interfaces: [(ifname.clone(), fake)].into_iter().collect(),
-            have_v6: false,
+            have_v6: true,
             have_v4: true,
             is_expensive: false,
             default_route_interface: Some(ifname),

iroh-net/src/netcheck.rs

@@ -4,7 +4,7 @@
 
 use std::collections::{BTreeMap, HashMap};
 use std::fmt::{self, Debug};
-use std::net::SocketAddr;
+use std::net::{SocketAddr, SocketAddrV4, SocketAddrV6};
 use std::sync::Arc;
 
 use anyhow::{anyhow, Context as _, Result};
@@ -58,10 +58,17 @@ pub struct Report {
     pub ipv4_can_send: bool,
     /// could bind a socket to ::1
     pub os_has_ipv6: bool,
-    /// an ICMPv4 round trip completed
-    pub icmpv4: bool,
-    /// Whether STUN results depend which STUN server you're talking to (on IPv4).
+    /// An ICMPv4 round trip completed, `None` if not checked.
+    pub icmpv4: Option<bool>,
+    /// An ICMPv6 round trip completed, `None` if not checked.
+    pub icmpv6: Option<bool>,
+    /// Whether STUN results depend on which STUN server you're talking to (on IPv4).
     pub mapping_varies_by_dest_ip: Option<bool>,
+    /// Whether STUN results depend on which STUN server you're talking to (on IPv6).
+    ///
+    /// Note that we don't really expect this to happen and are merely logging this if
+    /// detecting rather than using it.  For now.
+    pub mapping_varies_by_dest_ipv6: Option<bool>,
     /// Whether the router supports communicating between two local devices through the NATted
     /// public IP address (on IPv4).
     pub hair_pinning: Option<bool>,
@@ -76,9 +83,9 @@ pub struct Report {
     /// keyed by DERP Url
     pub derp_v6_latency: DerpLatencies,
     /// ip:port of global IPv4
-    pub global_v4: Option<SocketAddr>,
+    pub global_v4: Option<SocketAddrV4>,
     /// `[ip]:port` of global IPv6
-    pub global_v6: Option<SocketAddr>,
+    pub global_v6: Option<SocketAddrV6>,
     /// CaptivePortal is set when we think there's a captive portal that is
     /// intercepting HTTP traffic.
     pub captive_portal: Option<bool>,
@@ -763,6 +770,8 @@ pub(crate) fn os_has_ipv6() -> bool {
 
 #[cfg(test)]
 mod tests {
+    use std::net::Ipv4Addr;
+
     use bytes::BytesMut;
     use tokio::time;
     use tracing::info;
@@ -871,7 +880,7 @@ mod tests {
         // the STUN server being blocked will look like from the client's perspective.
         let blackhole = tokio::net::UdpSocket::bind("127.0.0.1:0").await?;
         let stun_addr = blackhole.local_addr()?;
-        let dm = stun::test::derp_map_of_opts([(stun_addr, true)].into_iter());
+        let dm = stun::test::derp_map_of_opts([(stun_addr, false)].into_iter());
 
         // Now create a client and generate a report.
         let mut client = Client::new(None)?;
@@ -884,34 +893,25 @@ mod tests {
         // blocked.  Unfortunately on some systems we simply don't have permissions to
         // create raw ICMP pings and we'll have to silently accept this test is useless (if
         // we could, this would be a skip instead).
-        let have_pinger = Pinger::new().is_ok();
-        if !have_pinger {
-            error!("pinger disabled, test effectively skipped");
-        } else {
-            info!("pinger enabled");
-        }
-
-        // This is the test: we will fall back to sending ICMP pings.  These should
-        // succeed when we have a working pinger.
-        // TODO: fix the test on all environments
-        let icmpv4 = r.icmpv4; // have_pinger;
-        dbg!(have_pinger, r.icmpv4);
+        let pinger = Pinger::new();
+        let can_ping = pinger.send(Ipv4Addr::LOCALHOST.into(), b"aa").await.is_ok();
+        let want_icmpv4 = match can_ping {
+            true => Some(true),
+            false => None,
+        };
 
         let want = Report {
-            // The ip_v4_can_send flag gets set differently across platforms.
-            // On Windows this test detects false, while on Linux detects true.
-            // That's not relevant to this test, so just accept what we're given.
-            ipv4_can_send: r.ipv4_can_send,
+            // The ICMP probe sets the can_ping flag.
+            ipv4_can_send: can_ping,
             // OS IPv6 test is irrelevant here, accept whatever the current machine has.
             os_has_ipv6: r.os_has_ipv6,
             // Captive portal test is irrelevant; accept what the current report has.
             captive_portal: r.captive_portal,
-            icmpv4,
+            // If we can ping we expect to have this.
+            icmpv4: want_icmpv4,
             // If we had a pinger, we'll have some latencies filled in and a preferred derp
-            derp_latency: have_pinger
-                .then(|| r.derp_latency.clone())
-                .unwrap_or_default(),
-            preferred_derp: have_pinger
+            derp_latency: can_ping.then(|| r.derp_latency.clone()).unwrap_or_default(),
+            preferred_derp: can_ping
                 .then_some(r.preferred_derp.clone())
                 .unwrap_or_default(),
             ..Default::default()