Skip to content

Commit

Permalink
fix(iroh-net): Fix in detecting globally routable IPv6 addresses (#2030)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
flub authored Feb 19, 2024
1 parent ac667bb commit c3aa17e
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 9 deletions.
43 changes: 35 additions & 8 deletions iroh-net/src/net/interfaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,10 @@ impl State {
}
}

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

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

Expand Down Expand Up @@ -383,6 +393,8 @@ impl HomeRouter {

#[cfg(test)]
mod tests {
use std::net::Ipv6Addr;

use super::*;

#[tokio::test]
Expand All @@ -398,4 +410,19 @@ mod tests {
let home_router = HomeRouter::new().expect("missing home router");
println!("home router: {:#?}", home_router);
}

#[test]
fn test_is_usable_v6() {
let loopback = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0x1);
assert!(!is_usable_v6(&loopback.into()));

let link_local = Ipv6Addr::new(0xfe80, 0, 0, 0, 0xcbc9, 0x6aff, 0x5b07, 0x4a9e);
assert!(!is_usable_v6(&link_local.into()));

let derp_use1 = Ipv6Addr::new(0x2a01, 0x4ff, 0xf0, 0xc4a1, 0, 0, 0, 0x1);
assert!(is_usable_v6(&derp_use1.into()));

let random_2603 = Ipv6Addr::new(0x2603, 0x3ff, 0xf1, 0xc3aa, 0x1, 0x2, 0x3, 0x1);
assert!(is_usable_v6(&random_2603.into()));
}
}
5 changes: 4 additions & 1 deletion iroh-net/src/netcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,10 @@ mod tests {
"expected key 1 in DERPLatency; got {:?}",
r.derp_latency
);
assert!(r.global_v4.is_some(), "expected globalV4 set");
assert!(
r.global_v4.is_some() || r.global_v6.is_some(),
"expected at least one of global_v4 or global_v6"
);
assert!(r.preferred_derp.is_some());
} else {
eprintln!("missing UDP, probe not returned by network");
Expand Down
1 change: 1 addition & 0 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ impl Actor {
/// aborted. That is, the main actor loop stops polling them.
async fn spawn_probes_task(&mut self) -> Result<JoinSet<Result<ProbeReport>>> {
let if_state = interfaces::State::new().await;
debug!(?if_state, "Local interfaces");
let plan = match self.last_report {
Some(ref report) => ProbePlan::with_last_report(&self.derp_map, &if_state, report),
None => ProbePlan::initial(&self.derp_map, &if_state),
Expand Down

0 comments on commit c3aa17e

Please sign in to comment.