From f588dea6b3cb5ec2ba0851289e0be7881a74128d Mon Sep 17 00:00:00 2001 From: "Franz Heinzmann (Frando)" Date: Wed, 28 Jun 2023 10:28:10 +0200 Subject: [PATCH 1/5] change: use DERP port from host_name URL --- iroh-net/src/defaults.rs | 1 - iroh-net/src/hp/derp/http.rs | 6 ++---- iroh-net/src/hp/derp/http/client.rs | 17 +++++++++++------ iroh-net/src/hp/derp/map.rs | 3 --- iroh-net/src/hp/magicsock/conn.rs | 8 +++++--- iroh-net/src/hp/netcheck.rs | 5 ++--- iroh-net/src/hp/stun.rs | 1 - iroh/src/commands/doctor.rs | 7 +++---- 8 files changed, 23 insertions(+), 25 deletions(-) diff --git a/iroh-net/src/defaults.rs b/iroh-net/src/defaults.rs index 60a5b14523..a064d8756b 100644 --- a/iroh-net/src/defaults.rs +++ b/iroh-net/src/defaults.rs @@ -18,7 +18,6 @@ pub fn default_derp_region() -> DerpRegion { stun_port: 3478, ipv4: UseIpv4::Some([35, 175, 99, 113].into()), ipv6: UseIpv6::None, - derp_port: 443, stun_test_ip: None, }; DerpRegion { diff --git a/iroh-net/src/hp/derp/http.rs b/iroh-net/src/hp/derp/http.rs index 464d0f0a13..39f8221c9b 100644 --- a/iroh-net/src/hp/derp/http.rs +++ b/iroh-net/src/hp/derp/http.rs @@ -80,13 +80,12 @@ mod tests { nodes: vec![DerpNode { name: "test_node".to_string(), region_id: 1, - host_name: "http://localhost".parse().unwrap(), + host_name: format!("http://localhost:{port}").parse().unwrap(), stun_only: false, stun_port: 0, stun_test_ip: None, ipv4: UseIpv4::Some(addr), ipv6: UseIpv6::Disabled, - derp_port: port, }], region_code: "test_region".to_string(), }; @@ -221,13 +220,12 @@ mod tests { nodes: vec![DerpNode { name: "test_node".to_string(), region_id: 1, - host_name: "https://localhost".parse().unwrap(), + host_name: format!("https://localhost:{port}").parse().unwrap(), stun_only: false, stun_port: 0, stun_test_ip: None, ipv4: UseIpv4::Some(addr), ipv6: UseIpv6::Disabled, - derp_port: port, }], region_code: "test_region".to_string(), }; diff --git a/iroh-net/src/hp/derp/http/client.rs b/iroh-net/src/hp/derp/http/client.rs index e9bd112964..05226a229b 100644 --- a/iroh-net/src/hp/derp/http/client.rs +++ b/iroh-net/src/hp/derp/http/client.rs @@ -683,10 +683,16 @@ impl Client { } } }; - let port = if node.derp_port != 0 { - node.derp_port - } else { - 443 + let port = match node.host_name.port() { + Some(port) => port, + None => match node.host_name.scheme() { + "http" => 80, + "https" => 443, + _ => return Err(ClientError::InvalidUrl( + "Invalid scheme in DERP hostname, only http: and https: schemes are supported." + .into(), + )), + }, }; let dst = format!("{host}:{port}"); debug!("dialing {}", dst); @@ -1123,13 +1129,12 @@ mod tests { nodes: vec![DerpNode { name: "test_node".to_string(), region_id: 1, - host_name: "http://bad.url".parse().unwrap(), + host_name: "https://bad.url".parse().unwrap(), stun_only: false, stun_port: 0, stun_test_ip: None, ipv4: UseIpv4::Some("35.175.99.112".parse().unwrap()), ipv6: UseIpv6::Disabled, - derp_port: 443, }], region_code: "test_region".to_string(), }; diff --git a/iroh-net/src/hp/derp/map.rs b/iroh-net/src/hp/derp/map.rs index ec1179c295..06b7063577 100644 --- a/iroh-net/src/hp/derp/map.rs +++ b/iroh-net/src/hp/derp/map.rs @@ -27,7 +27,6 @@ impl DerpMap { pub fn default_from_node( host_name: Url, stun_port: u16, - derp_port: u16, derp_ipv4: UseIpv4, derp_ipv6: UseIpv6, ) -> Self { @@ -47,7 +46,6 @@ impl DerpMap { stun_port, ipv4: derp_ipv4, ipv6: derp_ipv6, - derp_port, stun_test_ip: None, }], avoid: false, @@ -91,7 +89,6 @@ pub struct DerpNode { /// If `None`, A record(s) from DNS lookups of HostName are used. /// If `Disabled`, IPv6 is not used; pub ipv6: UseIpv6, - pub derp_port: u16, } #[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/iroh-net/src/hp/magicsock/conn.rs b/iroh-net/src/hp/magicsock/conn.rs index f2a9774599..8c45233f15 100644 --- a/iroh-net/src/hp/magicsock/conn.rs +++ b/iroh-net/src/hp/magicsock/conn.rs @@ -2654,13 +2654,15 @@ pub(crate) mod tests { nodes: vec![DerpNode { name: "t1".into(), region_id: 1, - host_name: "https://test-node.invalid".parse().unwrap(), + // In test mode, the DERP client does not validate HTTPS certs, so the host + // name is irrelevant, but the port is used. + host_name: format!("https://test-node.invalid:${}", https_addr.port()) + .parse() + .unwrap(), stun_only: false, stun_port: stun_addr.port(), ipv4: UseIpv4::Some("127.0.0.1".parse().unwrap()), ipv6: UseIpv6::None, - - derp_port: https_addr.port(), stun_test_ip: Some(stun_addr.ip()), }], avoid: false, diff --git a/iroh-net/src/hp/netcheck.rs b/iroh-net/src/hp/netcheck.rs index 7fc3f3c27c..456814c143 100644 --- a/iroh-net/src/hp/netcheck.rs +++ b/iroh-net/src/hp/netcheck.rs @@ -1788,7 +1788,7 @@ mod tests { .await .context("failed to create netcheck client")?; - let stun_servers = vec![("https://derp.iroh.network.", 3478, 0)]; + let stun_servers = vec![("https://derp.iroh.network.", 3478)]; let mut dm = DerpMap::default(); dm.regions.insert( @@ -1798,7 +1798,7 @@ mod tests { nodes: stun_servers .into_iter() .enumerate() - .map(|(i, (host_name, stun_port, derp_port))| DerpNode { + .map(|(i, (host_name, stun_port))| DerpNode { name: format!("default-{}", i), region_id: 1, host_name: host_name.parse().unwrap(), @@ -1806,7 +1806,6 @@ mod tests { stun_port, ipv4: UseIpv4::None, ipv6: UseIpv6::None, - derp_port, stun_test_ip: None, }) .collect(), diff --git a/iroh-net/src/hp/stun.rs b/iroh-net/src/hp/stun.rs index 255bfab9d8..707c443c05 100644 --- a/iroh-net/src/hp/stun.rs +++ b/iroh-net/src/hp/stun.rs @@ -184,7 +184,6 @@ pub mod test { stun_port: port, stun_only: true, stun_test_ip: None, - derp_port: 0, }; m.regions.insert( region_id, diff --git a/iroh/src/commands/doctor.rs b/iroh/src/commands/doctor.rs index c6433b264d..16b58eee63 100644 --- a/iroh/src/commands/doctor.rs +++ b/iroh/src/commands/doctor.rs @@ -197,7 +197,7 @@ async fn report(stun_host: Option, stun_port: u16, config: &Config) -> a Some(host_name) => { let host_name = host_name.parse()?; // creating a derp map from host name and stun port - DerpMap::default_from_node(host_name, stun_port, 0, UseIpv4::None, UseIpv6::None) + DerpMap::default_from_node(host_name, stun_port, UseIpv4::None, UseIpv6::None) } None => config.derp_map().expect("derp map not configured"), }; @@ -434,11 +434,10 @@ async fn passive_side(connection: quinn::Connection) -> anyhow::Result<()> { fn configure_local_derp_map() -> DerpMap { let stun_port = 3478; - let host_name = "http://derp.invalid".parse().unwrap(); - let derp_port = 3340; + let host_name = "http://derp.invalid:3340".parse().unwrap(); let derp_ipv4 = UseIpv4::Some("127.0.0.1".parse().unwrap()); let derp_ipv6 = UseIpv6::None; - DerpMap::default_from_node(host_name, stun_port, derp_port, derp_ipv4, derp_ipv6) + DerpMap::default_from_node(host_name, stun_port, derp_ipv4, derp_ipv6) } const DR_DERP_ALPN: [u8; 11] = *b"n0/drderp/1"; From 090b0d1753802dbe80e503639856a2ca47c303f0 Mon Sep 17 00:00:00 2001 From: "Franz Heinzmann (Frando)" Date: Fri, 30 Jun 2023 14:07:40 +0200 Subject: [PATCH 2/5] fix: typo --- iroh-net/src/hp/magicsock/conn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iroh-net/src/hp/magicsock/conn.rs b/iroh-net/src/hp/magicsock/conn.rs index 8c45233f15..d0d93733ef 100644 --- a/iroh-net/src/hp/magicsock/conn.rs +++ b/iroh-net/src/hp/magicsock/conn.rs @@ -2656,7 +2656,7 @@ pub(crate) mod tests { region_id: 1, // In test mode, the DERP client does not validate HTTPS certs, so the host // name is irrelevant, but the port is used. - host_name: format!("https://test-node.invalid:${}", https_addr.port()) + host_name: format!("https://test-node.invalid:{}", https_addr.port()) .parse() .unwrap(), stun_only: false, From 65fc7651eb3b46b383a2adac99e1db8169549cf7 Mon Sep 17 00:00:00 2001 From: "Franz Heinzmann (Frando)" Date: Mon, 3 Jul 2023 23:30:43 +0200 Subject: [PATCH 3/5] change: DerpRegion::host_name -> DerpRegion::url --- iroh-net/src/defaults.rs | 2 +- iroh-net/src/hp/derp/http.rs | 4 ++-- iroh-net/src/hp/derp/http/client.rs | 12 ++++++------ iroh-net/src/hp/derp/http/mesh_clients.rs | 2 +- iroh-net/src/hp/derp/map.rs | 6 +++--- iroh-net/src/hp/magicsock/conn.rs | 2 +- iroh-net/src/hp/netcheck.rs | 8 ++++---- iroh-net/src/hp/stun.rs | 2 +- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/iroh-net/src/defaults.rs b/iroh-net/src/defaults.rs index a064d8756b..a06a23a6c9 100644 --- a/iroh-net/src/defaults.rs +++ b/iroh-net/src/defaults.rs @@ -13,7 +13,7 @@ pub fn default_derp_region() -> DerpRegion { let default_n0_derp = DerpNode { name: "default-1".into(), region_id: 1, - host_name: "https://derp.iroh.network".parse().unwrap(), + url: "https://derp.iroh.network".parse().unwrap(), stun_only: false, stun_port: 3478, ipv4: UseIpv4::Some([35, 175, 99, 113].into()), diff --git a/iroh-net/src/hp/derp/http.rs b/iroh-net/src/hp/derp/http.rs index 39f8221c9b..8b779ac985 100644 --- a/iroh-net/src/hp/derp/http.rs +++ b/iroh-net/src/hp/derp/http.rs @@ -80,7 +80,7 @@ mod tests { nodes: vec![DerpNode { name: "test_node".to_string(), region_id: 1, - host_name: format!("http://localhost:{port}").parse().unwrap(), + url: format!("http://localhost:{port}").parse().unwrap(), stun_only: false, stun_port: 0, stun_test_ip: None, @@ -220,7 +220,7 @@ mod tests { nodes: vec![DerpNode { name: "test_node".to_string(), region_id: 1, - host_name: format!("https://localhost:{port}").parse().unwrap(), + url: format!("https://localhost:{port}").parse().unwrap(), stun_only: false, stun_port: 0, stun_test_ip: None, diff --git a/iroh-net/src/hp/derp/http/client.rs b/iroh-net/src/hp/derp/http/client.rs index 05226a229b..8494848fd0 100644 --- a/iroh-net/src/hp/derp/http/client.rs +++ b/iroh-net/src/hp/derp/http/client.rs @@ -351,7 +351,7 @@ impl Client { .and_then(|s| rustls::ServerName::try_from(s).ok()); } if let Some(node) = node { - if let Some(host) = node.host_name.host_str() { + if let Some(host) = node.url.host_str() { return rustls::ServerName::try_from(host).ok(); } } @@ -380,7 +380,7 @@ impl Client { return false; } if let Some(node) = node { - if node.host_name.scheme() == "http" { + if node.url.scheme() == "http" { return false; } } @@ -664,7 +664,7 @@ impl Client { UseIp::Ipv6(UseIpv6::Some(addr)) => addr.into(), _ => { let host = node - .host_name + .url .host() .ok_or_else(|| ClientError::InvalidUrl("missing host".into()))?; match host { @@ -683,9 +683,9 @@ impl Client { } } }; - let port = match node.host_name.port() { + let port = match node.url.port() { Some(port) => port, - None => match node.host_name.scheme() { + None => match node.url.scheme() { "http" => 80, "https" => 443, _ => return Err(ClientError::InvalidUrl( @@ -1129,7 +1129,7 @@ mod tests { nodes: vec![DerpNode { name: "test_node".to_string(), region_id: 1, - host_name: "https://bad.url".parse().unwrap(), + url: "https://bad.url".parse().unwrap(), stun_only: false, stun_port: 0, stun_test_ip: None, diff --git a/iroh-net/src/hp/derp/http/mesh_clients.rs b/iroh-net/src/hp/derp/http/mesh_clients.rs index dd9e7e8b96..2506904284 100644 --- a/iroh-net/src/hp/derp/http/mesh_clients.rs +++ b/iroh-net/src/hp/derp/http/mesh_clients.rs @@ -50,7 +50,7 @@ impl MeshClients { for (_, region) in derp_map.regions.iter() { for node in region.nodes.iter() { // note: `node.host_name` is expected to include the scheme - let mut url = node.host_name.clone(); + let mut url = node.url.clone(); url.set_path("/derp"); urls.push(url); } diff --git a/iroh-net/src/hp/derp/map.rs b/iroh-net/src/hp/derp/map.rs index 06b7063577..c30beb42c5 100644 --- a/iroh-net/src/hp/derp/map.rs +++ b/iroh-net/src/hp/derp/map.rs @@ -25,7 +25,7 @@ impl DerpMap { /// Creates a new [`DerpMap`] with a single Derp server configured. pub fn default_from_node( - host_name: Url, + url: Url, stun_port: u16, derp_ipv4: UseIpv4, derp_ipv6: UseIpv6, @@ -41,7 +41,7 @@ impl DerpMap { nodes: vec![DerpNode { name: "default-1".into(), region_id: 1, - host_name, + url: url, stun_only: !derp_ipv4.is_enabled() && !derp_ipv6.is_enabled(), stun_port, ipv4: derp_ipv4, @@ -77,7 +77,7 @@ pub struct DerpRegion { pub struct DerpNode { pub name: String, pub region_id: u16, - pub host_name: Url, + pub url: Url, pub stun_only: bool, pub stun_port: u16, pub stun_test_ip: Option, diff --git a/iroh-net/src/hp/magicsock/conn.rs b/iroh-net/src/hp/magicsock/conn.rs index d0d93733ef..7a7771f160 100644 --- a/iroh-net/src/hp/magicsock/conn.rs +++ b/iroh-net/src/hp/magicsock/conn.rs @@ -2656,7 +2656,7 @@ pub(crate) mod tests { region_id: 1, // In test mode, the DERP client does not validate HTTPS certs, so the host // name is irrelevant, but the port is used. - host_name: format!("https://test-node.invalid:{}", https_addr.port()) + url: format!("https://test-node.invalid:{}", https_addr.port()) .parse() .unwrap(), stun_only: false, diff --git a/iroh-net/src/hp/netcheck.rs b/iroh-net/src/hp/netcheck.rs index 456814c143..1876f35be1 100644 --- a/iroh-net/src/hp/netcheck.rs +++ b/iroh-net/src/hp/netcheck.rs @@ -335,7 +335,7 @@ async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option) -> Resu let node = &dm.regions.get(&preferred_derp).unwrap().nodes[0]; if node - .host_name + .url .host_str() .map(|s| s.ends_with(&DOT_INVALID)) .unwrap_or_default() @@ -354,7 +354,7 @@ async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option) -> Resu // length is limited; see is_challenge_char in bin/derper for more // details. - let host_name = node.host_name.host_str().unwrap_or_default(); + let host_name = node.url.host_str().unwrap_or_default(); let challenge = format!("ts_{}", host_name); let portal_url = format!("http://{}/generate_204", host_name); let res = client @@ -453,7 +453,7 @@ async fn get_node_addr(n: &DerpNode, proto: ProbeProto) -> Result { } } - match n.host_name.host() { + match n.url.host() { Some(url::Host::Domain(hostname)) => { async move { debug!(?proto, %hostname, "Performing DNS lookup for derp addr"); @@ -1801,7 +1801,7 @@ mod tests { .map(|(i, (host_name, stun_port))| DerpNode { name: format!("default-{}", i), region_id: 1, - host_name: host_name.parse().unwrap(), + url: host_name.parse().unwrap(), stun_only: true, stun_port, ipv4: UseIpv4::None, diff --git a/iroh-net/src/hp/stun.rs b/iroh-net/src/hp/stun.rs index 707c443c05..804419c358 100644 --- a/iroh-net/src/hp/stun.rs +++ b/iroh-net/src/hp/stun.rs @@ -178,7 +178,7 @@ pub mod test { let node = DerpNode { name: format!("{region_id}a"), region_id, - host_name: format!("http://{region_id}.invalid").parse().unwrap(), + url: format!("http://{region_id}.invalid").parse().unwrap(), ipv4, ipv6, stun_port: port, From b8a3f8cf1b5f31bee80e63ee6ea484e611d56de3 Mon Sep 17 00:00:00 2001 From: "Franz Heinzmann (Frando)" Date: Mon, 3 Jul 2023 23:42:42 +0200 Subject: [PATCH 4/5] fix: leftover renames --- iroh-net/examples/magic.rs | 15 +-------------- iroh/src/commands/doctor.rs | 8 ++++---- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/iroh-net/examples/magic.rs b/iroh-net/examples/magic.rs index 0bcbbf6ab8..9686e080cf 100644 --- a/iroh-net/examples/magic.rs +++ b/iroh-net/examples/magic.rs @@ -52,20 +52,7 @@ async fn main() -> anyhow::Result<()> { let derp_map = match args.derp_url { None => default_derp_map(), - Some(url) => { - // TODO: This should be done by the DERP client. - let derp_port = match url.port() { - Some(port) => port, - None => match url.scheme() { - "http" => 80, - "https" => 443, - _ => anyhow::bail!( - "Invalid scheme in DERP URL, only http: and https: schemes are supported." - ), - }, - }; - DerpMap::default_from_node(url, 3478, derp_port, UseIpv4::None, UseIpv6::None) - } + Some(url) => DerpMap::default_from_node(url, 3478, UseIpv4::None, UseIpv6::None), }; let endpoint = MagicEndpoint::builder() diff --git a/iroh/src/commands/doctor.rs b/iroh/src/commands/doctor.rs index 16b58eee63..5db5a3a4b2 100644 --- a/iroh/src/commands/doctor.rs +++ b/iroh/src/commands/doctor.rs @@ -195,9 +195,9 @@ async fn report(stun_host: Option, stun_port: u16, config: &Config) -> a let dm = match stun_host { Some(host_name) => { - let host_name = host_name.parse()?; + let url = host_name.parse()?; // creating a derp map from host name and stun port - DerpMap::default_from_node(host_name, stun_port, UseIpv4::None, UseIpv6::None) + DerpMap::default_from_node(url, stun_port, UseIpv4::None, UseIpv6::None) } None => config.derp_map().expect("derp map not configured"), }; @@ -434,10 +434,10 @@ async fn passive_side(connection: quinn::Connection) -> anyhow::Result<()> { fn configure_local_derp_map() -> DerpMap { let stun_port = 3478; - let host_name = "http://derp.invalid:3340".parse().unwrap(); + let url = "http://derp.invalid:3340".parse().unwrap(); let derp_ipv4 = UseIpv4::Some("127.0.0.1".parse().unwrap()); let derp_ipv6 = UseIpv6::None; - DerpMap::default_from_node(host_name, stun_port, derp_ipv4, derp_ipv6) + DerpMap::default_from_node(url, stun_port, derp_ipv4, derp_ipv6) } const DR_DERP_ALPN: [u8; 11] = *b"n0/drderp/1"; From e4f7ba1cdde065aa221817b869855748eb75622d Mon Sep 17 00:00:00 2001 From: "Franz Heinzmann (Frando)" Date: Mon, 3 Jul 2023 23:59:43 +0200 Subject: [PATCH 5/5] chore: clippy --- iroh-net/src/hp/derp/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iroh-net/src/hp/derp/map.rs b/iroh-net/src/hp/derp/map.rs index c30beb42c5..baa2840acb 100644 --- a/iroh-net/src/hp/derp/map.rs +++ b/iroh-net/src/hp/derp/map.rs @@ -41,7 +41,7 @@ impl DerpMap { nodes: vec![DerpNode { name: "default-1".into(), region_id: 1, - url: url, + url, stun_only: !derp_ipv4.is_enabled() && !derp_ipv6.is_enabled(), stun_port, ipv4: derp_ipv4,