diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f0cf70f..27d04eeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,8 +78,21 @@ its heading and collects entries; the date and the link go on with the tag. itself if it was built with the updater, so 0.3.4 is the first that can, and 0.3.5 is the first update it will install. +### Changed + +- **Discovery reads the Windows device table directly** instead of running + `arp -a` and parsing its text. On an idle machine that makes no measurable + difference (`arp -a` took 65 ms), but it removes a program start from every + discover and sweep, and under heavy CPU load `arp -a` took about 4 seconds + on the same machine. + ### Fixed +- **Discovery reported the broadcast address as a device.** The network's + device table on Windows lists `x.x.x.255` with the MAC + `ff:ff:ff:ff:ff:ff`, and discovery listed it as a host that ignored ping + (and a sweep then scanned it). The network and broadcast addresses, and broadcast and + multicast MACs, are no longer reported. - **The macOS app could be refused as broken on Apple Silicon.** Its only signature was the one Apple's linker puts on every arm64 program, which claims the app's files are sealed when nothing sealed them. macOS's own diff --git a/crates/netscli-core/Cargo.toml b/crates/netscli-core/Cargo.toml index 1b1aeda0..785cb66e 100644 --- a/crates/netscli-core/Cargo.toml +++ b/crates/netscli-core/Cargo.toml @@ -48,7 +48,7 @@ pnet_datalink = "0.35" [target.'cfg(windows)'.dependencies] ipconfig = "0.3" -windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_NetworkManagement_IpHelper", "Win32_Networking_WinSock"] } +windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_NetworkManagement_IpHelper", "Win32_NetworkManagement_Ndis", "Win32_Networking_WinSock"] } [features] # Lean by default: scan, discover, DNS, ARP, OUI lookup, stats. diff --git a/crates/netscli-core/src/arp/platform.rs b/crates/netscli-core/src/arp/platform.rs index 3629d86f..5932a7f3 100644 --- a/crates/netscli-core/src/arp/platform.rs +++ b/crates/netscli-core/src/arp/platform.rs @@ -2,6 +2,8 @@ mod command; mod interfaces; mod mutate; mod table; +#[cfg(target_os = "windows")] +mod windows_table; use std::net::IpAddr; diff --git a/crates/netscli-core/src/arp/platform/table.rs b/crates/netscli-core/src/arp/platform/table.rs index e14d2200..8d02ac43 100644 --- a/crates/netscli-core/src/arp/platform/table.rs +++ b/crates/netscli-core/src/arp/platform/table.rs @@ -1,12 +1,15 @@ +#[cfg(any(target_os = "linux", target_os = "macos"))] use mac_address::MacAddress; +#[cfg(any(target_os = "linux", target_os = "macos"))] use std::net::IpAddr; -#[cfg(any(target_os = "windows", target_os = "macos"))] +#[cfg(target_os = "macos")] use super::command; use crate::arp::types::ArpEntry; #[cfg(not(any(target_os = "linux", target_os = "windows", target_os = "macos")))] use crate::error::Error; use crate::error::Result; +#[cfg(any(target_os = "linux", target_os = "macos"))] use crate::oui::lookup_vendor; #[cfg(target_os = "linux")] @@ -42,57 +45,7 @@ pub(super) fn get_arp_table() -> Result> { #[cfg(target_os = "windows")] pub(super) fn get_arp_table() -> Result> { - use std::str::FromStr; - - let output = command::arp_command().arg("-a").output()?; - let text = String::from_utf8_lossy(&output.stdout); - let mut entries = Vec::new(); - - // `arp -a` on Windows groups entries by interface. Each group begins with - // a header like `Interface: 192.168.1.2 --- 0x5` followed by a column - // header (`Internet Address Physical Address Type`) and then - // rows. Track the current interface IP so entries are correctly attributed - // instead of all being flattened to "unknown". - let mut current_interface = String::from("unknown"); - - for line in text.lines() { - let trimmed = line.trim(); - if trimmed.is_empty() { - continue; - } - - if let Some(rest) = trimmed.strip_prefix("Interface:") { - if let Some(iface_ip) = rest.split_whitespace().next() { - current_interface = iface_ip.to_string(); - } - continue; - } - - if trimmed.starts_with("Internet Address") { - continue; - } - - let parts: Vec<&str> = trimmed.split_whitespace().collect(); - if parts.len() < 3 { - continue; - } - - let Ok(ip) = IpAddr::from_str(parts[0]) else { - continue; - }; - let Ok(mac) = MacAddress::from_str(parts[1]) else { - continue; - }; - - let vendor = lookup_vendor(parts[1]); - entries.push(ArpEntry { - ip, - mac, - interface: current_interface.clone(), - vendor, - }); - } - Ok(entries) + super::windows_table::read() } #[cfg(target_os = "macos")] diff --git a/crates/netscli-core/src/arp/platform/windows_table.rs b/crates/netscli-core/src/arp/platform/windows_table.rs new file mode 100644 index 00000000..fcd9ca22 --- /dev/null +++ b/crates/netscli-core/src/arp/platform/windows_table.rs @@ -0,0 +1,164 @@ +//! The Windows neighbor table, read with `GetIpNetTable2` instead of running +//! `arp -a`. +//! +//! Discovery reads this table on every run. `arp -a` took 65 ms on an idle +//! Windows 11 machine but about 4 s on the same machine under heavy CPU load +//! (measured 2026-10-01). The API returns the same table in-process, with +//! no program to start. +//! +//! Only entries for a real neighbor are kept: reachable, stale, delay, probe +//! or permanent states, with a unicast MAC. `arp -a` also lists the subnet +//! broadcast address (ff-ff-ff-ff-ff-ff, "static") and multicast groups +//! (01-00-5e-...). Discovery used to report the broadcast address as a host. + +use std::collections::HashMap; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + +use mac_address::MacAddress; +use windows_sys::Win32::Foundation::NO_ERROR; +use windows_sys::Win32::NetworkManagement::IpHelper::{ + FreeMibTable, GetIpNetTable2, GetUnicastIpAddressTable, MIB_IPNET_TABLE2, + MIB_UNICASTIPADDRESS_TABLE, +}; +use windows_sys::Win32::Networking::WinSock::{ + NlnsDelay, NlnsPermanent, NlnsProbe, NlnsReachable, NlnsStale, AF_INET, AF_INET6, + NL_NEIGHBOR_STATE, SOCKADDR_INET, +}; + +use crate::arp::types::ArpEntry; +use crate::error::{Error, Result}; +use crate::oui::lookup_vendor; + +pub(super) fn read() -> Result> { + let interfaces = interface_ipv4_by_index(); + let mut table: *mut MIB_IPNET_TABLE2 = std::ptr::null_mut(); + // SAFETY: GetIpNetTable2 allocates the table and writes its pointer. + // IPv4 only, like `arp -a` here and the Linux and macOS tables. + let status = unsafe { GetIpNetTable2(AF_INET, &mut table) }; + if status != NO_ERROR || table.is_null() { + return Err(Error::Network(format!("GetIpNetTable2 failed: {status}"))); + } + // SAFETY: on success `table` points at NumEntries rows; freed below. + let rows = unsafe { + std::slice::from_raw_parts((*table).Table.as_ptr(), (*table).NumEntries as usize) + }; + let mut entries = Vec::new(); + for row in rows { + if row.PhysicalAddressLength != 6 { + continue; + } + let bytes: [u8; 6] = row.PhysicalAddress[..6].try_into().unwrap_or([0; 6]); + if !is_neighbor(row.State, bytes) { + continue; + } + let Some(ip) = sockaddr_ip(&row.Address) else { + continue; + }; + let mac = MacAddress::new(bytes); + let vendor = lookup_vendor(&mac.to_string()); + let interface = interfaces + .get(&row.InterfaceIndex) + .map(|ip| ip.to_string()) + .unwrap_or_else(|| format!("if{}", row.InterfaceIndex)); + entries.push(ArpEntry { + ip, + mac, + interface, + vendor, + }); + } + // SAFETY: allocated by GetIpNetTable2. + unsafe { FreeMibTable(table as *const _) }; + Ok(entries) +} + +/// A row worth reporting: a state that means a real neighbor, and a unicast +/// MAC. All zeros is an unresolved entry, and the low bit of the first octet +/// marks broadcast (ff:ff:ff:ff:ff:ff) and multicast (01:00:5e:...). +fn is_neighbor(state: NL_NEIGHBOR_STATE, mac: [u8; 6]) -> bool { + const LIVE: [NL_NEIGHBOR_STATE; 5] = [ + NlnsReachable, + NlnsStale, + NlnsDelay, + NlnsProbe, + NlnsPermanent, + ]; + LIVE.contains(&state) && mac != [0; 6] && mac[0] & 1 == 0 +} + +/// Each interface's first IPv4 address, so entries keep the `interface` +/// value `arp -a` gave them (its "Interface: 192.168.1.10" header). +fn interface_ipv4_by_index() -> HashMap { + let mut map = HashMap::new(); + let mut table: *mut MIB_UNICASTIPADDRESS_TABLE = std::ptr::null_mut(); + // SAFETY: as for GetIpNetTable2. + let status = unsafe { GetUnicastIpAddressTable(AF_INET, &mut table) }; + if status != NO_ERROR || table.is_null() { + return map; + } + // SAFETY: on success `table` points at NumEntries rows; freed below. + let rows = unsafe { + std::slice::from_raw_parts((*table).Table.as_ptr(), (*table).NumEntries as usize) + }; + for row in rows { + if let Some(IpAddr::V4(ip)) = sockaddr_ip(&row.Address) { + map.entry(row.InterfaceIndex).or_insert(ip); + } + } + // SAFETY: allocated by GetUnicastIpAddressTable. + unsafe { FreeMibTable(table as *const _) }; + map +} + +fn sockaddr_ip(addr: &SOCKADDR_INET) -> Option { + // SAFETY: si_family says which union member is valid. + unsafe { + match addr.si_family { + AF_INET => { + let raw = addr.Ipv4.sin_addr.S_un.S_addr; + Some(IpAddr::V4(Ipv4Addr::from(u32::from_be(raw)))) + } + AF_INET6 => Some(IpAddr::V6(Ipv6Addr::from(addr.Ipv6.sin6_addr.u.Byte))), + _ => None, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use windows_sys::Win32::Networking::WinSock::{NlnsIncomplete, NlnsUnreachable}; + + const UNICAST: [u8; 6] = [0x2c, 0xcf, 0x67, 0x26, 0xff, 0x96]; + + #[test] + fn a_live_unicast_neighbor_is_kept() { + assert!(is_neighbor(NlnsReachable, UNICAST)); + assert!(is_neighbor(NlnsStale, UNICAST)); + assert!(is_neighbor(NlnsPermanent, UNICAST)); + } + + #[test] + fn the_broadcast_and_multicast_entries_are_not_hosts() { + // `arp -a` lists x.x.x.255 as ff-ff-ff-ff-ff-ff "static"; discovery + // reported it as a host. + assert!(!is_neighbor(NlnsPermanent, [0xff; 6])); + assert!(!is_neighbor( + NlnsPermanent, + [0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb] + )); + } + + #[test] + fn unresolved_entries_are_not_hosts() { + assert!(!is_neighbor(NlnsUnreachable, UNICAST)); + assert!(!is_neighbor(NlnsIncomplete, UNICAST)); + assert!(!is_neighbor(NlnsReachable, [0; 6])); + } + + #[test] + fn the_table_reads_and_includes_no_broadcast_entry() { + let entries = read().expect("GetIpNetTable2"); + assert!(entries.iter().all(|e| e.mac.bytes() != [0xff; 6])); + } +} diff --git a/crates/netscli-core/src/discover.rs b/crates/netscli-core/src/discover.rs index b1ea3807..e8837c40 100644 --- a/crates/netscli-core/src/discover.rs +++ b/crates/netscli-core/src/discover.rs @@ -242,9 +242,10 @@ impl DiscoverEngine { // Only within the range that was asked for. The neighbour // table spans every interface, so it holds addresses from // other subnets entirely. - IpAddr::V4(v4) => subnet.contains(v4), + IpAddr::V4(v4) => is_host_address(&subnet, *v4), IpAddr::V6(_) => false, }) + .filter(|ip| arp_map.get(ip).is_none_or(|e| e.mac.bytes()[0] & 1 == 0)) .collect(); neighbors.sort_unstable(); hosts.extend( @@ -256,3 +257,36 @@ impl DiscoverEngine { Ok(hosts) } } + +/// An address in `subnet` that a device can hold: not the network address +/// and not the broadcast address, except on /31 and /32 where every address +/// is usable. The Windows neighbour table lists x.x.x.255 +/// (ff:ff:ff:ff:ff:ff), and discovery reported it as a host. +fn is_host_address(subnet: &Ipv4Net, ip: std::net::Ipv4Addr) -> bool { + subnet.contains(&ip) + && (subnet.prefix_len() >= 31 || (ip != subnet.network() && ip != subnet.broadcast())) +} + +#[cfg(test)] +mod host_address_tests { + use super::is_host_address; + + #[test] + fn network_and_broadcast_are_not_hosts() { + let net = "192.168.1.0/24".parse().unwrap(); + assert!(!is_host_address(&net, "192.168.1.0".parse().unwrap())); + assert!(!is_host_address(&net, "192.168.1.255".parse().unwrap())); + assert!(is_host_address(&net, "192.168.1.1".parse().unwrap())); + assert!(is_host_address(&net, "192.168.1.254".parse().unwrap())); + assert!(!is_host_address(&net, "192.168.2.1".parse().unwrap())); + } + + #[test] + fn every_address_counts_on_a_31_or_32() { + let p2p = "10.0.0.0/31".parse().unwrap(); + assert!(is_host_address(&p2p, "10.0.0.0".parse().unwrap())); + assert!(is_host_address(&p2p, "10.0.0.1".parse().unwrap())); + let single = "10.0.0.5/32".parse().unwrap(); + assert!(is_host_address(&single, "10.0.0.5".parse().unwrap())); + } +}