diff --git a/README.md b/README.md index 23567d4..73d1198 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,13 @@ No vote is involved — see [docs/wireguard.md](docs/wireguard.md#ipv4-allocated-signed-and-kept) and [docs/sync-model.md](docs/sync-model.md). +Because the address is allocated at run time rather than derived, it is not +known until the agent has started and agreed with its peers, so +`tsunagi tun-setup` cannot print it in advance. The agent prints the exact +`ip address add` command once it has one, and keeps saying so until the +address is actually on an interface — without it, packets leave with the +wrong source address and every peer drops them. + ## Running unprivileged The agent does not need to run as root. Creating a network interface and diff --git a/docs/wireguard.md b/docs/wireguard.md index 6ca99d1..30b1284 100644 --- a/docs/wireguard.md +++ b/docs/wireguard.md @@ -126,6 +126,15 @@ wins, and the agent adopts it. So the flag matters for whoever starts the network and is harmless afterwards. Pass `--ipv4-range none` for an IPv6-only overlay. +Putting the address on the interface still needs privileges, and the agent +cannot do it. Since the address is only known once the agent has agreed with +its peers, `tsunagi tun-setup` cannot print it up front either. So the agent +checks whether the address is assigned anywhere on the host — by binding a UDP +socket to it, which needs no privileges — and reports the exact command until +it is. This matters: with the wrong address on the interface, packets leave +with the wrong source and every peer drops them as not belonging to us, which +looks like a broken network rather than a missing command. + A release tombstone exists in the record type and merges correctly, but nothing emits one yet, so an address stays claimed until the network is forgotten. diff --git a/src/bin/tsunagi.rs b/src/bin/tsunagi.rs index 3ddf544..e1b8973 100644 --- a/src/bin/tsunagi.rs +++ b/src/bin/tsunagi.rs @@ -378,7 +378,6 @@ async fn status(args: StatusArgs) -> Result<(), Box> { async fn tun_setup(args: TunSetupArgs) -> Result<(), Box> { use tsunagi::dataplane::wireguard::{ DEFAULT_MTU, OVERLAY_PREFIX_LEN, WgKeyStore, interface_name, overlay_address, - overlay_address_v4, }; use tsunagi::identity::NetworkKeys; @@ -407,10 +406,12 @@ async fn tun_setup(args: TunSetupArgs) -> Result<(), Box> println!("# Network {name} ({network})"); println!("# Interface {interface}, address {address}/{OVERLAY_PREFIX_LEN}, mtu {mtu}"); - if let Some(range) = ipv4_range - && let Some(v4) = overlay_address_v4(network, &key.public(), range) - { - println!("# IPv4 overlay address {v4}/{}", range.prefix_len); + if ipv4_range.is_some() { + println!( + "# IPv4 is allocated once the agent runs and agrees with its peers, so it\n\ + # cannot be printed here. Start `tsunagi up`; it prints the exact\n\ + # `ip address add` command for the address it was given." + ); } println!("# Run once as root; then run `tsunagi up` as {user}."); println!( @@ -425,14 +426,7 @@ async fn tun_setup(args: TunSetupArgs) -> Result<(), Box> println!("sudo ip link set dev {interface} mtu {mtu} up"); println!("sudo sysctl -qw net.ipv6.conf.{interface}.keep_addr_on_down=1"); println!("sudo ip -6 address add {address}/{OVERLAY_PREFIX_LEN} dev {interface} nodad"); - if let Some(range) = ipv4_range - && let Some(v4) = overlay_address_v4(network, &key.public(), range) - { - println!( - "sudo ip address add {v4}/{} dev {interface}", - range.prefix_len - ); - } + println!("\n# To check it afterwards:"); println!("ip -6 addr show dev {interface}"); println!("\n# To remove it again:"); @@ -728,6 +722,7 @@ async fn build_report( .collect(), unroutable_packets: view.unroutable_packets, multicast_packets: view.multicast_packets, + unroutable_sample: view.unroutable_sample.map(|address| address.to_string()), }); NetworkReport { diff --git a/src/dataplane/wireguard/device.rs b/src/dataplane/wireguard/device.rs index a11aae1..45ff3e2 100644 --- a/src/dataplane/wireguard/device.rs +++ b/src/dataplane/wireguard/device.rs @@ -200,6 +200,8 @@ struct Inner { routes: RwLock>, next_index: AtomicU32, unroutable: AtomicU64, + /// One destination nobody owned, kept so the counter can be acted on. + unroutable_sample: Mutex>, multicast: AtomicU64, ipv4_conflicts: AtomicU64, } @@ -237,6 +239,7 @@ impl WireguardDevice { routes: RwLock::new(HashMap::new()), next_index: AtomicU32::new(1), unroutable: AtomicU64::new(0), + unroutable_sample: Mutex::new(None), multicast: AtomicU64::new(0), ipv4_conflicts: AtomicU64::new(0), }); @@ -427,6 +430,17 @@ impl WireguardDevice { self.inner.unroutable.load(Ordering::Relaxed) } + /// One destination that nobody owned, if there was one. + /// + /// A bare count says something is wrong but not what; the address usually + /// says it outright. + pub fn unroutable_sample(&self) -> Option { + match self.inner.unroutable_sample.lock() { + Ok(guard) => *guard, + Err(poisoned) => *poisoned.into_inner(), + } + } + /// Multicast packets dropped. /// /// Expected and harmless: Linux emits multicast listener and router @@ -518,12 +532,12 @@ async fn read_from_os(inner: Arc) { } let target = read_lock(&inner.routes).get(&destination).copied(); let Some(target) = target else { - inner.unroutable.fetch_add(1, Ordering::Relaxed); + note_unroutable(&inner, destination); continue; }; let peer = read_lock(&inner.peers).get(&target).cloned(); let Some(peer) = peer else { - inner.unroutable.fetch_add(1, Ordering::Relaxed); + note_unroutable(&inner, destination); continue; }; @@ -557,6 +571,16 @@ async fn read_from_os(inner: Arc) { } } +/// Counts a packet nobody owned the destination of, keeping one example. +fn note_unroutable(inner: &Inner, destination: IpAddr) { + inner.unroutable.fetch_add(1, Ordering::Relaxed); + let mut sample = match inner.unroutable_sample.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + *sample = Some(destination); +} + /// Peer -> operating system. async fn read_from_link(inner: Arc, peer: Arc) { loop { diff --git a/src/dataplane/wireguard/mod.rs b/src/dataplane/wireguard/mod.rs index 9530003..eba2c32 100644 --- a/src/dataplane/wireguard/mod.rs +++ b/src/dataplane/wireguard/mod.rs @@ -62,7 +62,7 @@ pub use plugin::{ WireguardConfig, WireguardPlugin, }; pub use store::WgKeyStore; -pub use tun::{MemoryTun, MemoryTunFactory, TunDevice, TunFactory, TunRequest}; +pub use tun::{MemoryTun, MemoryTunFactory, TunDevice, TunFactory, TunRequest, address_is_local}; #[cfg(feature = "tun-device")] pub use tun::{ diff --git a/src/dataplane/wireguard/plugin.rs b/src/dataplane/wireguard/plugin.rs index 238517f..e52a045 100644 --- a/src/dataplane/wireguard/plugin.rs +++ b/src/dataplane/wireguard/plugin.rs @@ -161,6 +161,8 @@ pub struct NetworkOverview { pub unroutable_packets: u64, /// Multicast packets dropped. Expected, not a fault. pub multicast_packets: u64, + /// One destination nobody owned, if there was one. + pub unroutable_sample: Option, } impl NetworkOverview { @@ -207,6 +209,8 @@ struct NetworkState { allocations: HashMap, /// The range those allocations came from. ipv4_range: Option, + /// The address last reported as missing, so it is said once, not forever. + reported_missing_v4: Option, } #[derive(Debug, Default)] @@ -351,6 +355,10 @@ impl WireguardPlugin { .as_ref() .map(|device| device.multicast_packets()) .unwrap_or(0), + unroutable_sample: state + .device + .as_ref() + .and_then(|device| device.unroutable_sample()), }) } @@ -435,6 +443,7 @@ impl Worker { links: HashMap::new(), allocations: HashMap::new(), ipv4_range: None, + reported_missing_v4: None, }); } @@ -495,6 +504,10 @@ impl Worker { }; let allocations = state.allocations.clone(); + let own_v4 = state.allocations.get(&self.local_id()).copied(); + let interface = state.device.as_ref().map(|_| state.interface.clone()); + let range = state.ipv4_range; + let state_reported = state.reported_missing_v4; let mut wanted: Vec = Vec::new(); let mut too_small: Vec<(usize, usize)> = Vec::new(); for (endpoint_id, announcement) in &state.announcements { @@ -534,8 +547,45 @@ impl Worker { } } device.retain_peers(&wanted); + + // The address is allocated at run time, but putting it on the + // interface needs privileges we do not have. Without it the kernel + // sends our packets with the wrong source address and every peer + // drops them, which looks like a broken network rather than a missing + // command. So say exactly what is wrong. + let missing_v4 = match (own_v4, interface.as_deref(), range) { + (Some(address), Some(interface), Some(range)) + if !super::tun::address_is_local(IpAddr::V4(address)) => + { + let already = state_reported == Some(address); + if let Some(state) = shared.networks.get_mut(&network) { + state.reported_missing_v4 = Some(address); + } + (!already).then_some((address, interface.to_string(), range)) + } + _ => { + if let Some(state) = shared.networks.get_mut(&network) { + state.reported_missing_v4 = None; + } + None + } + }; drop(shared); + if let Some((address, interface, range)) = missing_v4 { + self.report( + network, + format!( + "this agent was allocated {address} but that address is not on any \ + interface, so IPv4 cannot work: packets would leave with the wrong \ + source and every peer would drop them. Run:\n \ + sudo ip address add {address}/{} dev {interface}\n \ + and remove any other address of that range from it.", + range.prefix_len + ), + ); + } + for (available, needed) in too_small { self.report( network, diff --git a/src/dataplane/wireguard/tun.rs b/src/dataplane/wireguard/tun.rs index a7078a9..3318972 100644 --- a/src/dataplane/wireguard/tun.rs +++ b/src/dataplane/wireguard/tun.rs @@ -130,6 +130,17 @@ impl TunDevice for MemoryTun { } } +/// Whether an address is assigned to some interface on this host. +/// +/// Binding a UDP socket to a specific address only succeeds when the address +/// is local, which makes this a cheap check that needs no privileges and no +/// platform-specific code. It does not say *which* interface has it, which is +/// enough here: the agent chose the address, so anything else holding it is a +/// problem in its own right. +pub fn address_is_local(address: std::net::IpAddr) -> bool { + std::net::UdpSocket::bind((address, 0)).is_ok() +} + /// Creates [`MemoryTun`] devices. #[derive(Debug, Clone, Default)] pub struct MemoryTunFactory { diff --git a/src/ipc/mod.rs b/src/ipc/mod.rs index 196b8c1..a979449 100644 --- a/src/ipc/mod.rs +++ b/src/ipc/mod.rs @@ -148,6 +148,8 @@ pub struct OverlayReport { pub unroutable_packets: u64, /// Multicast packets dropped. Expected, not a fault. pub multicast_packets: u64, + /// One destination nobody owned, if there was one. + pub unroutable_sample: Option, } /// One overlay peer. @@ -272,8 +274,12 @@ impl StatusReport { if overlay.unroutable_packets > 0 { let _ = writeln!( out, - " {} packet(s) to addresses nobody owns", - overlay.unroutable_packets + " {} packet(s) to addresses nobody owns{}", + overlay.unroutable_packets, + match &overlay.unroutable_sample { + Some(sample) => format!(", most recently {sample}"), + None => String::new(), + } ); } } diff --git a/tests/wireguard.rs b/tests/wireguard.rs index 0dda8e3..9b82a84 100644 --- a/tests/wireguard.rs +++ b/tests/wireguard.rs @@ -504,6 +504,48 @@ async fn a_joining_member_adopts_the_range_the_network_already_uses() { b.shutdown().await; } +#[tokio::test] +async fn an_allocated_address_missing_from_the_host_is_reported() { + let discovery = SharedMemoryDiscovery::new(); + let (name, secret) = network("wg-missing-address"); + + // The in-memory interface never carries the address, which is exactly + // the situation of a real interface the operator has not configured yet. + // Left unsaid, packets leave with the wrong source and every peer drops + // them, which looks like a broken network rather than a missing command. + let a = WgAgent::spawn(&discovery, "ta").await; + let b = WgAgent::spawn(&discovery, "tb").await; + + let mut events = a.agent.subscribe(); + let network_id = a.agent.join_network(&name, &secret).await.unwrap(); + b.agent.join_network(&name, &secret).await.unwrap(); + a.wait_for_tunnels(network_id, 1).await; + + let reason = wait_event(&mut events, |event| match event { + Event::PluginError { reason, .. } if reason.contains("not on any") => Some(reason.clone()), + _ => None, + }) + .await; + + let allocated = a + .plugin + .overview(network_id) + .unwrap() + .overlay_address_v4 + .unwrap(); + assert!( + reason.contains(&allocated.to_string()), + "unexpected: {reason}" + ); + assert!( + reason.contains("ip address add"), + "must name the fix: {reason}" + ); + + a.shutdown().await; + b.shutdown().await; +} + #[tokio::test] async fn an_address_is_kept_across_a_restart() { let discovery = SharedMemoryDiscovery::new();