diff --git a/clients/android/native/src/discovery.rs b/clients/android/native/src/discovery.rs index 5c49d5bc5..9e236bd4d 100644 --- a/clients/android/native/src/discovery.rs +++ b/clients/android/native/src/discovery.rs @@ -178,11 +178,12 @@ fn resolve(info: &ResolvedService) -> Option { if !proto.is_empty() && proto != PROTO { return None; // some other DNS-SD service sharing the type — ignore } - let addr = info - .get_addresses_v4() - .iter() - .next() - .map(|a| a.to_string())?; + // Deterministic pick from the union of per-interface answers (the host OS's responder + // contributes VPN/overlay addresses; `iter().next()` on the HashSet dialed an arbitrary + // one) — same policy as the desktop client, shared in `punktfunk_core::discovery`. + let candidates: Vec = info.get_addresses_v4().into_iter().collect(); + let addr = punktfunk_core::discovery::pick_host_addr(&candidates, val("addr").parse().ok())? + .to_string(); let id = val("id"); let fullname = info.get_fullname(); Some(Host { diff --git a/crates/pf-client-core/src/discovery.rs b/crates/pf-client-core/src/discovery.rs index 4432a6cb4..546a67cf3 100644 --- a/crates/pf-client-core/src/discovery.rs +++ b/crates/pf-client-core/src/discovery.rs @@ -141,9 +141,19 @@ pub fn browse() -> (async_channel::Receiver, Rescan) { // responder often answers AAAA for its hostname) would render a host card // that fails on every click. A v6-only advert is dropped — the honest // "not found" — until the stack actually speaks IPv6. - let Some(addr) = - info.get_addresses_v4().iter().next().map(|a| a.to_string()) - else { + // + // Among the v4 addresses, pick deterministically: the set is a union of + // per-interface answers from EVERY responder (a host on ZeroTier/… + // contributes its overlay address via the OS responder), and taking + // `iter().next()` of the HashSet dialed an arbitrary one — a field + // client streamed over the host's VPN while both machines shared a LAN. + let candidates: Vec = + info.get_addresses_v4().into_iter().collect(); + let Some(addr) = punktfunk_core::discovery::pick_host_addr( + &candidates, + val("addr").parse().ok(), + ) + .map(|a| a.to_string()) else { continue; }; let id = val("id"); diff --git a/crates/punktfunk-core/src/discovery.rs b/crates/punktfunk-core/src/discovery.rs new file mode 100644 index 000000000..5687a37bc --- /dev/null +++ b/crates/punktfunk-core/src/discovery.rs @@ -0,0 +1,171 @@ +//! Shared discovery-address selection: which A record to dial when an mDNS advert resolves to +//! several. +//! +//! The resolved set is a UNION of answers from every responder on every interface. The host's +//! own advert registers exactly one address (its routed primary — see the host crate's +//! `discovery.rs`), but the host OS's built-in mDNS responder also answers A queries for the +//! same `.local.` label per interface, with that interface's address — so a host running +//! an overlay network (ZeroTier, Tailscale, …) whose multicast reaches this client contributes +//! its overlay address to the set. Field case: a client dialed the host's ZeroTier address +//! while both machines shared a LAN, because the pick was `HashSet::iter().next()` — arbitrary, +//! and re-rolled on every re-announce. +//! +//! [`rank_host_addr`] is the pure policy (testable); [`pick_host_addr`] applies it with this +//! machine's live context. + +use std::net::{IpAddr, Ipv4Addr, UdpSocket}; + +/// Common leading bits of two addresses — the "how on-link is this" proxy the ranking runs on. +/// No netmasks: a longer shared prefix with one of our own addresses is monotonically "more +/// likely on this segment", which is all a RANKING needs. +fn prefix_bits(a: Ipv4Addr, b: Ipv4Addr) -> u32 { + (u32::from(a) ^ u32::from(b)).leading_zeros() +} + +/// The address to dial, chosen deterministically. Score, best wins, in order: +/// +/// 1. longest common prefix with ANY of this machine's unicast addresses — an address on one of +/// our own subnets beats one we would have to route. This alone settles the overlay case in +/// both directions: on a shared LAN the host's LAN address out-prefixes its overlay address, +/// and a client that can ONLY reach the host through the overlay has no LAN interface for +/// the host's LAN address to match, so the overlay address wins instead; +/// 2. the address the host itself declared (mDNS TXT `addr`, its routed primary) — settles a +/// multi-NIC host's tie without ever overriding reachability, because a declared address we +/// cannot see on-link already lost rung 1; +/// 3. longest common prefix with OUR routed (default-route) source address — a host that +/// predates the `addr` TXT still resolves the common ties here; +/// 4. the numerically lowest address — pure determinism, so a re-announce cannot flap the pick. +pub fn rank_host_addr( + candidates: &[Ipv4Addr], + host_declared: Option, + local_ips: &[Ipv4Addr], + routed_local: Option, +) -> Option { + candidates.iter().copied().max_by_key(|&c| { + ( + local_ips + .iter() + .map(|&l| prefix_bits(c, l)) + .max() + .unwrap_or(0), + host_declared == Some(c), + routed_local.map_or(0, |r| prefix_bits(c, r)), + std::cmp::Reverse(u32::from(c)), + ) + }) +} + +/// [`rank_host_addr`] with this machine's live context: every non-loopback unicast IPv4, plus +/// the source address the OS routes toward the internet. Gathered per call — discovery events +/// are rare, and interfaces change (VPN up/down) between them. +pub fn pick_host_addr( + candidates: &[Ipv4Addr], + host_declared: Option, +) -> Option { + rank_host_addr( + candidates, + host_declared, + &local_ipv4s(), + routed_local_ipv4(), + ) +} + +fn local_ipv4s() -> Vec { + if_addrs::get_if_addrs() + .map(|ifs| { + ifs.into_iter() + .filter_map(|i| match i.ip() { + IpAddr::V4(v) if !v.is_loopback() => Some(v), + _ => None, + }) + .collect() + }) + .unwrap_or_default() +} + +/// Same trick as the host's `primary_local_ip`: a UDP `connect()` performs the route lookup +/// without sending a packet, and `local_addr` is the source address the OS chose. +fn routed_local_ipv4() -> Option { + let sock = UdpSocket::bind("0.0.0.0:0").ok()?; + sock.connect("8.8.8.8:80").ok()?; + match sock.local_addr().ok()?.ip() { + IpAddr::V4(v) if !v.is_loopback() => Some(v), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::rank_host_addr; + use std::net::Ipv4Addr; + + fn ip(s: &str) -> Ipv4Addr { + s.parse().unwrap() + } + + // The 2026-08-28 field case: host advertises from its LAN address, the OS responder adds + // the ZeroTier address over the overlay's multicast, and both machines are on both + // networks. The LAN address must win — with or without the host's TXT declaration. + #[test] + fn shared_lan_beats_shared_overlay() { + let candidates = [ip("192.168.196.206"), ip("192.168.1.170")]; + let locals = [ip("192.168.1.150"), ip("192.168.196.57")]; + for declared in [None, Some(ip("192.168.1.170"))] { + assert_eq!( + rank_host_addr(&candidates, declared, &locals, Some(ip("192.168.1.150"))), + Some(ip("192.168.1.170")) + ); + } + } + + // A client that can ONLY reach the host through the overlay (different site): the host's + // declared LAN address is not on any of our subnets, so it must NOT win — the overlay + // address is the reachable one. + #[test] + fn overlay_only_client_ignores_the_declared_lan_address() { + let candidates = [ip("192.168.1.170"), ip("192.168.196.206")]; + let locals = [ip("10.1.2.3"), ip("192.168.196.57")]; + assert_eq!( + rank_host_addr( + &candidates, + Some(ip("192.168.1.170")), + &locals, + Some(ip("10.1.2.3")) + ), + Some(ip("192.168.196.206")) + ); + } + + // A multi-NIC host (Ethernet + Wi-Fi on the same LAN) ties on every reachability rung; + // its own declaration settles which of ITS addresses we dial. Without the declaration + // (older host) the pick is still deterministic. + #[test] + fn declared_addr_settles_a_multi_nic_tie() { + let candidates = [ip("192.168.1.170"), ip("192.168.1.171")]; + let locals = [ip("192.168.1.150")]; + assert_eq!( + rank_host_addr( + &candidates, + Some(ip("192.168.1.171")), + &locals, + Some(ip("192.168.1.150")) + ), + Some(ip("192.168.1.171")) + ); + assert_eq!( + rank_host_addr(&candidates, None, &locals, Some(ip("192.168.1.150"))), + Some(ip("192.168.1.170")), + "no declaration: lowest address, never a hash-order roll" + ); + } + + #[test] + fn no_context_is_still_deterministic() { + let candidates = [ip("10.0.0.9"), ip("10.0.0.5")]; + assert_eq!( + rank_host_addr(&candidates, None, &[], None), + Some(ip("10.0.0.5")) + ); + assert_eq!(rank_host_addr(&[], None, &[], None), None); + } +} diff --git a/crates/punktfunk-core/src/lib.rs b/crates/punktfunk-core/src/lib.rs index 08f74f5c4..a7b9f2255 100644 --- a/crates/punktfunk-core/src/lib.rs +++ b/crates/punktfunk-core/src/lib.rs @@ -61,6 +61,7 @@ pub mod client; pub mod clipboard; pub mod config; pub mod crypto; +pub mod discovery; pub mod error; pub mod fec; pub mod input; diff --git a/crates/punktfunk-host/src/discovery.rs b/crates/punktfunk-host/src/discovery.rs index bfdb808f1..19405e3ff 100644 --- a/crates/punktfunk-host/src/discovery.rs +++ b/crates/punktfunk-host/src/discovery.rs @@ -21,6 +21,11 @@ //! - `os` — the host's OS identity chain (`windows` | `macos` | `linux[/][/]`, e.g. //! `linux/fedora/bazzite` — see [`crate::osinfo`]), so a client can show an OS icon on the host //! card. Advisory/unauthenticated like `mac`: a wrong value only draws a wrong icon. +//! - `addr` — the IPv4 this advert was registered for (the host's routed primary). The A-record +//! set a client resolves is a union polluted by OTHER responders answering per-interface (the +//! host OS's own mDNS stack answers for `.local.` on a VPN/overlay interface with that +//! interface's address); the client's picker (`punktfunk_core::discovery`) uses this declared +//! value to break ties among addresses it resolved anyway. Advisory like the rest. use anyhow::{Context, Result}; use mdns_sd::{ServiceDaemon, ServiceInfo}; @@ -220,6 +225,9 @@ pub fn advertise_native( if !macs.is_empty() { props.insert("mac".into(), macs.join(",")); } + // `addr` — which of the host's addresses this advert is FOR (see module doc): the + // client-side picker's tie-breaker against per-interface answers from other responders. + props.insert("addr".into(), ip.to_string()); // Detect & warn (never modifies) if the routed NIC isn't armed to wake — the usual reason // WoL silently fails. Re-checked on an address change because the routed NIC may be a // different one now.