Skip to content

Commit c3aa17e

Browse files
authored
fix(iroh-net): Fix in detecting globally routable IPv6 addresses (#2030)
## Description We were incorrectly identifying globally routable IPv6 addresses as not routable. This make netcheck skip IPv6 addresses. ## Notes & open questions Closes #2022 The fix in the netcheck, test is curious, it fails on this PR fairly consistently yet it is really a flaky test introduced in combination with #2027. Now that we have IPv6 resolving properly we might get an IPv6 result. Since this test only has a single DerpUrl it only needs one result to complete. This could be an IPv4 OR an IPv6 result, we don't know which one will be faster. And if the other is slow enough then it won't be received. I think this behaviour is fine, we simply can't guarantee that both must have a result and the point of netcheck is to return something working quickly. ## Change checklist - [x] Self-review. - [x] Documentation updates if relevant. - [x] Tests if relevant.
1 parent ac667bb commit c3aa17e

File tree

3 files changed

+40
-9
lines changed

3 files changed

+40
-9
lines changed

iroh-net/src/net/interfaces.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,10 @@ impl State {
282282
}
283283
}
284284

285-
/// Reports whether ip is a usable IPv4 address which could
286-
/// conceivably be used to get Internet connectivity. Globally routable and
287-
/// private IPv4 addresses are always Usable, and link local 169.254.x.x
288-
/// addresses are in some environments.
285+
/// Reports whether ip is a usable IPv4 address which should have Internet connectivity.
286+
///
287+
/// Globally routable and private IPv4 addresses are always Usable, and link local
288+
/// 169.254.x.x addresses are in some environments.
289289
fn is_usable_v4(ip: &IpAddr) -> bool {
290290
if !ip.is_ipv4() || ip.is_loopback() {
291291
return false;
@@ -294,15 +294,25 @@ fn is_usable_v4(ip: &IpAddr) -> bool {
294294
true
295295
}
296296

297-
/// Reports whether ip is a usable IPv6 address which could
298-
/// conceivably be used to get Internet connectivity. Globally routable
299-
/// IPv6 addresses are always Usable, and Unique Local Addresses
297+
/// Reports whether ip is a usable IPv6 address which should have Internet connectivity.
298+
///
299+
/// Globally routable IPv6 addresses are always Usable, and Unique Local Addresses
300300
/// (fc00::/7) are in some environments used with address translation.
301+
///
302+
/// We consider all 2000::/3 addresses to be routable, which is the interpretation of
303+
/// <https://www.iana.org/assignments/ipv6-unicast-address-assignments/ipv6-unicast-address-assignments.xhtml>
304+
/// as well. However this probably includes some addresses which should not be routed,
305+
/// e.g. documentation addresses. See also
306+
/// <https://doc.rust-lang.org/std/net/struct.Ipv6Addr.html#method.is_global> for an
307+
/// alternative implementation which is both stricter and laxer in some regards.
301308
fn is_usable_v6(ip: &IpAddr) -> bool {
302309
match ip {
303310
IpAddr::V6(ip) => {
304311
// V6 Global1 2000::/3
305-
if matches!(ip.segments(), [0x2000, _, _, _, _, _, _, _]) {
312+
let mask: u16 = 0b1110_0000_0000_0000;
313+
let base: u16 = 0x2000;
314+
let segment1 = ip.segments()[0];
315+
if (base & mask) == (segment1 & mask) {
306316
return true;
307317
}
308318

@@ -383,6 +393,8 @@ impl HomeRouter {
383393

384394
#[cfg(test)]
385395
mod tests {
396+
use std::net::Ipv6Addr;
397+
386398
use super::*;
387399

388400
#[tokio::test]
@@ -398,4 +410,19 @@ mod tests {
398410
let home_router = HomeRouter::new().expect("missing home router");
399411
println!("home router: {:#?}", home_router);
400412
}
413+
414+
#[test]
415+
fn test_is_usable_v6() {
416+
let loopback = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0x1);
417+
assert!(!is_usable_v6(&loopback.into()));
418+
419+
let link_local = Ipv6Addr::new(0xfe80, 0, 0, 0, 0xcbc9, 0x6aff, 0x5b07, 0x4a9e);
420+
assert!(!is_usable_v6(&link_local.into()));
421+
422+
let derp_use1 = Ipv6Addr::new(0x2a01, 0x4ff, 0xf0, 0xc4a1, 0, 0, 0, 0x1);
423+
assert!(is_usable_v6(&derp_use1.into()));
424+
425+
let random_2603 = Ipv6Addr::new(0x2603, 0x3ff, 0xf1, 0xc3aa, 0x1, 0x2, 0x3, 0x1);
426+
assert!(is_usable_v6(&random_2603.into()));
427+
}
401428
}

iroh-net/src/netcheck.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,10 @@ mod tests {
847847
"expected key 1 in DERPLatency; got {:?}",
848848
r.derp_latency
849849
);
850-
assert!(r.global_v4.is_some(), "expected globalV4 set");
850+
assert!(
851+
r.global_v4.is_some() || r.global_v6.is_some(),
852+
"expected at least one of global_v4 or global_v6"
853+
);
851854
assert!(r.preferred_derp.is_some());
852855
} else {
853856
eprintln!("missing UDP, probe not returned by network");

iroh-net/src/netcheck/reportgen.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ impl Actor {
581581
/// aborted. That is, the main actor loop stops polling them.
582582
async fn spawn_probes_task(&mut self) -> Result<JoinSet<Result<ProbeReport>>> {
583583
let if_state = interfaces::State::new().await;
584+
debug!(?if_state, "Local interfaces");
584585
let plan = match self.last_report {
585586
Some(ref report) => ProbePlan::with_last_report(&self.derp_map, &if_state, report),
586587
None => ProbePlan::initial(&self.derp_map, &if_state),

0 commit comments

Comments
 (0)