fix: improve connectivity

* fix(conn): send full ping if no addrs are not available

* fix: handle disco messages on derp

* fix: stop stayinalive once session times out

* cleanup pings more regularly

* fix typo

* ci: enable skipped tests

* ci: skip for real

* fix: ensure derp hostnames are treated consistently

* ci: debug

* less trust

* revert trust

* fix(netcheck): hostname handling for captive portal

* fix(derp): don't panic on region check

* refactor ping handling

* ci: add back integration flag

* refactor: remove unused fields from `cfg::Node`

* refactor: improve initial udp addr selection

* fix: derp backup addr

* fix: track received ips as candidate endpoints

* fix: change when pings are sent

* always directly ping best_addr

* rm log file

* improve trust setup

* fix: ensure full pings are done and best_addr is cleaned up

* happy clippy

* fix(endpoint): use recent pong instead of now

* fix: improve full ping and unknown key handling

- send full pings on best_addr ping if needed
- only store endpoints with known keys

* show logging on failed cli tests & adjust `candidate_endpoint`

* simplify ping logic

* refactor: use a timer to handle ping timeouts

* ci: remove debug flag

* test: do not assume no packets get lost

---------

Co-authored-by: Kasey <klhuizinga@gmail.com>

.github/workflows/ci.yml

@@ -331,7 +331,7 @@ jobs:
         sudo kill -9 $(pgrep ovs)
         sudo mn --clean
         sudo python3 main.py --integration sims/iroh/iroh.json
-        sudo python3 main.py --integration --skip intg_derper__1_to_1_NAT_provide,intg_derper__1_to_1_NAT_both sims/integration
+        sudo python3 main.py --integration sims/integration
 
     - name: Setup Environment (PR)  
       if: ${{ github.event_name == 'pull_request' }}  

.github/workflows/netsim.yml

@@ -95,7 +95,7 @@ jobs:
         if [ -z "${c}" ];
         then
           sudo python3 main.py sims/iroh
-          sudo python3 main.py --skip intg_derper__1_to_1_NAT_provide,intg_derper__1_to_1_NAT_both sims/integration
+          sudo python3 main.py sims/integration
         else
           echo $c >> custom_sim.json
           sudo python3 main.py custom_sim.json
@@ -197,4 +197,4 @@ jobs:
         curl -X POST -H  "Content-Type: application/json" -H "Authorization: Bearer ${{secrets.METRO_TOKEN}}" --data "$metro_data" ${{secrets.METRO_ENDPOINT}}
         d=$(cat report_metro_integration.txt)
         metro_data=$(printf "%s\n " "$d")
-        curl -X POST -H  "Content-Type: application/json" -H "Authorization: Bearer ${{secrets.METRO_TOKEN}}" --data "$metro_data" ${{secrets.METRO_ENDPOINT}}
+        curl -X POST -H  "Content-Type: application/json" -H "Authorization: Bearer ${{secrets.METRO_TOKEN}}" --data "$metro_data" ${{secrets.METRO_ENDPOINT}}

iroh-net/Cargo.toml

@@ -52,6 +52,7 @@ tokio = { version = "1", features = ["io-util", "sync", "rt", "net", "fs"] }
 tokio-util = { version = "0.7", features = ["io-util", "io"] }
 tokio-rustls = { version = "0.24" }
 tokio-rustls-acme = { version = "0.1" }
+url = { version = "2.4", features = ["serde"] }
 webpki = { version = "0.22", features = ["std"] }
 webpki-roots = "0.23.0"
 wg = "0.3.1"

iroh-net/src/client.rs

@@ -1,7 +1,7 @@
 use std::{
     net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6},
     sync::Arc,
-    time::{Duration, Instant},
+    time::Duration,
 };
 
 use anyhow::{Context, Result};
@@ -115,12 +115,6 @@ pub async fn dial_peer(
                 key: node_key.clone(),
                 endpoints,
                 derp: Some(SocketAddr::new(DERP_MAGIC_IP, DEFAULT_DERP_REGION)),
-                created: Instant::now(),
-                hostinfo: hp::hostinfo::Hostinfo::default(),
-                keep_alive: false,
-                expired: false,
-                online: None,
-                last_seen: None,
             }],
         })
         .await?;

iroh-net/src/hp/cfg.rs

@@ -4,10 +4,9 @@ use std::{
     collections::HashMap,
     fmt::Display,
     net::{IpAddr, Ipv4Addr, SocketAddr},
-    time::Instant,
 };
 
-use super::{hostinfo::Hostinfo, key};
+use super::key;
 
 /// Fake WireGuard endpoint IP address that means to
 /// use DERP. When used (in the Node.DERP field), the port number of
@@ -169,20 +168,4 @@ pub struct Node {
     ///
     /// If this nodes expected to be reachable via DERP relaying.
     pub derp: Option<SocketAddr>,
-    pub hostinfo: Hostinfo,
-    pub created: Instant,
-
-    /// When the node was last online. It is not
-    /// updated when Online is true. It is nil if the current
-    /// node doesn't have permission to know, or the node has never been online.
-    pub last_seen: Option<Instant>,
-
-    /// Whether the node is currently connected to the
-    /// coordination server. A value of None means unknown, or the current node doesn't have permission to know.
-    pub online: Option<bool>,
-
-    /// Open and keep open a connection to this peer
-    pub keep_alive: bool,
-
-    pub expired: bool,
 }

iroh-net/src/hp/derp/http.rs

@@ -78,7 +78,7 @@ mod tests {
             nodes: vec![DerpNode {
                 name: "test_node".to_string(),
                 region_id: 1,
-                host_name: "localhost".to_string(),
+                host_name: "http://localhost".parse().unwrap(),
                 stun_only: false,
                 stun_port: 0,
                 stun_test_ip: None,
@@ -216,7 +216,7 @@ mod tests {
             nodes: vec![DerpNode {
                 name: "test_node".to_string(),
                 region_id: 1,
-                host_name: "localhost".to_string(),
+                host_name: "https://localhost".parse().unwrap(),
                 stun_only: false,
                 stun_port: 0,
                 stun_test_ip: None,

iroh-net/src/hp/derp/http/client.rs

@@ -11,12 +11,12 @@ use futures::future::BoxFuture;
 use hyper::upgrade::Upgraded;
 use hyper::{header::UPGRADE, Body, Request};
 use rand::Rng;
-use reqwest::Url;
 use tokio::net::TcpStream;
 use tokio::sync::oneshot;
 use tokio::sync::Mutex;
 use tokio::task::JoinSet;
 use tracing::{debug, instrument, warn};
+use url::Url;
 
 use crate::hp::derp::client_conn::Io;
 use crate::hp::derp::{
@@ -70,8 +70,8 @@ pub enum ClientError {
     PingTimeout,
     #[error("cannot acknowledge pings")]
     CannotAckPings,
-    #[error("invalid url")]
-    InvalidUrl,
+    #[error("invalid url: {0}")]
+    InvalidUrl(String),
     #[error("dns: {0:?}")]
     Dns(Option<trust_dns_resolver::error::ResolveError>),
 }
@@ -157,7 +157,7 @@ impl ClientBuilder {
         self
     }
 
-    /// S returns if we should prefer ipv6
+    /// Returns if we should prefer ipv6
     /// it replaces the derphttp.AddressFamilySelector we pass
     /// It provides the hint as to whether in an IPv4-vs-IPv6 race that
     /// IPv4 should be held back a bit to give IPv6 a better-than-50/50
@@ -306,7 +306,8 @@ impl Client {
         if let Some(get_region) = &self.inner.get_region {
             let region = get_region()
                 .await
-                .expect("Cannot connection client: DERP region is unknown");
+                .ok_or_else(|| ClientError::DerpRegionNotAvail)?;
+
             return Ok(region);
         }
         Err(ClientError::DerpRegionNotAvail)
@@ -323,7 +324,9 @@ impl Client {
                 .and_then(|s| rustls::ServerName::try_from(s).ok());
         }
         if let Some(node) = node {
-            return rustls::ServerName::try_from(node.host_name.as_str()).ok();
+            if let Some(host) = node.host_name.host_str() {
+                return rustls::ServerName::try_from(host).ok();
+            }
         }
 
         None
@@ -344,22 +347,33 @@ impl Client {
         None
     }
 
-    fn use_https(&self) -> bool {
+    fn use_https(&self, node: Option<&DerpNode>) -> bool {
         // only disable https if we are explicitly dialing a http url
         if let Some(true) = self.inner.url.as_ref().map(|url| url.scheme() == "http") {
             return false;
         }
+        if let Some(node) = node {
+            if node.host_name.scheme() == "http" {
+                return false;
+            }
+        }
         true
     }
 
     async fn connect_0(&self) -> Result<DerpClient, ClientError> {
         debug!("connect_0");
-        let region = self.current_region().await?;
-        debug!("connect_0 region: {:?}", region);
+        let url = self.url();
+        let is_test_url = url
+            .as_ref()
+            .map(|url| url.as_str().ends_with(".invalid"))
+            .unwrap_or_default();
+
+        debug!("connect_0 url: {:?}, is_test_url: {}", url, is_test_url);
 
-        let (tcp_stream, derp_node) = if self.url().is_some() {
+        let (tcp_stream, derp_node) = if url.is_some() && !is_test_url {
             (self.dial_url().await?, None)
         } else {
+            let region = self.current_region().await?;
             let (tcp_stream, derp_node) = self.dial_region(region).await?;
             (tcp_stream, Some(derp_node))
         };
@@ -374,7 +388,7 @@ impl Client {
             .body(Body::empty())
             .unwrap();
 
-        let res = if self.use_https() {
+        let res = if self.use_https(derp_node.as_ref()) {
             debug!("Starting TLS handshake");
             // TODO: review TLS config
             let mut roots = rustls::RootCertStore::empty();
@@ -398,7 +412,7 @@ impl Client {
             let tls_connector: tokio_rustls::TlsConnector = Arc::new(config).into();
             let hostname = self
                 .tls_servername(derp_node.as_ref())
-                .ok_or_else(|| ClientError::InvalidUrl)?;
+                .ok_or_else(|| ClientError::InvalidUrl("no tls servername".into()))?;
             let tls_stream = tls_connector.connect(hostname, tcp_stream).await?;
             debug!("tls_connector connect success");
             let (mut request_sender, connection) = hyper::client::conn::Builder::new()
@@ -494,27 +508,29 @@ impl Client {
     }
 
     async fn dial_url(&self) -> Result<TcpStream, ClientError> {
-        let host = if let Some(host_str) = self.url().and_then(|url| url.host_str()) {
-            host_str
-        } else {
-            return Err(ClientError::InvalidUrl);
-        };
+        let host = self.url().and_then(|url| url.host()).ok_or_else(|| {
+            ClientError::InvalidUrl(format!("missing host from {:?}", self.url()))
+        })?;
+
         debug!("dial url: {}", host);
-        let dst_ip: IpAddr = if let Ok(ip) = host.parse() {
-            // Already a valid IP address
-            ip
-        } else {
-            // Need to do a DNS lookup
-            let addr = DNS_RESOLVER
-                .lookup_ip(host)
-                .await
-                .map_err(|e| ClientError::Dns(Some(e)))?
-                .iter()
-                .next();
-            addr.ok_or_else(|| ClientError::Dns(None))?
+        let dst_ip = match host {
+            url::Host::Domain(hostname) => {
+                // Need to do a DNS lookup
+                let addr = DNS_RESOLVER
+                    .lookup_ip(hostname)
+                    .await
+                    .map_err(|e| ClientError::Dns(Some(e)))?
+                    .iter()
+                    .next();
+                addr.ok_or_else(|| ClientError::Dns(None))?
+            }
+            url::Host::Ipv4(ip) => IpAddr::V4(ip),
+            url::Host::Ipv6(ip) => IpAddr::V6(ip),
         };
 
-        let port = self.url_port().ok_or_else(|| ClientError::InvalidUrl)?;
+        let port = self
+            .url_port()
+            .ok_or_else(|| ClientError::InvalidUrl("missing url port".into()))?;
         let addr = SocketAddr::new(dst_ip, port);
 
         tracing::error!("connecting to {}", addr);
@@ -620,18 +636,23 @@ impl Client {
             UseIp::Ipv4(UseIpv4::Some(addr)) => addr.into(),
             UseIp::Ipv6(UseIpv6::Some(addr)) => addr.into(),
             _ => {
-                if let Ok(ip) = node.host_name.parse() {
-                    // Already a valid IP address
-                    ip
-                } else {
-                    // Need to do a DNS lookup
-                    let addr = DNS_RESOLVER
-                        .lookup_ip(&node.host_name)
-                        .await
-                        .map_err(|e| ClientError::Dns(Some(e)))?
-                        .iter()
-                        .next();
-                    addr.ok_or_else(|| ClientError::Dns(None))?
+                let host = node
+                    .host_name
+                    .host()
+                    .ok_or_else(|| ClientError::InvalidUrl("missing host".into()))?;
+                match host {
+                    url::Host::Domain(domain) => {
+                        // Need to do a DNS lookup
+                        let addr = DNS_RESOLVER
+                            .lookup_ip(domain)
+                            .await
+                            .map_err(|e| ClientError::Dns(Some(e)))?
+                            .iter()
+                            .next();
+                        addr.ok_or_else(|| ClientError::Dns(None))?
+                    }
+                    url::Host::Ipv4(ip) => IpAddr::V4(ip),
+                    url::Host::Ipv6(ip) => IpAddr::V6(ip),
                 }
             }
         };
@@ -900,7 +921,7 @@ mod tests {
             nodes: vec![DerpNode {
                 name: "test_node".to_string(),
                 region_id: 1,
-                host_name: "bad.url".into(),
+                host_name: "http://bad.url".parse().unwrap(),
                 stun_only: false,
                 stun_port: 0,
                 stun_test_ip: None,

iroh-net/src/hp/derp/map.rs

@@ -7,6 +7,7 @@ use std::{
 };
 
 use serde::{Deserialize, Serialize};
+use url::Url;
 
 /// Configuration of all the Derp servers that can be used.
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
@@ -24,7 +25,7 @@ impl DerpMap {
 
     /// Creates a new [`DerpMap`] with a single Derp server configured.
     pub fn default_from_node(
-        host_name: String,
+        host_name: Url,
         stun_port: u16,
         derp_port: u16,
         derp_ipv4: UseIpv4,
@@ -78,7 +79,7 @@ pub struct DerpRegion {
 pub struct DerpNode {
     pub name: String,
     pub region_id: usize,
-    pub host_name: String,
+    pub host_name: Url,
     pub stun_only: bool,
     pub stun_port: u16,
     pub stun_test_ip: Option<IpAddr>,