From d25ccd4c1798cb18976509301c68ea1f795f7637 Mon Sep 17 00:00:00 2001 From: KKRainbow <5665404+KKRainbow@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:43:35 +0800 Subject: [PATCH] fix(windows): reuse numbered TUN names after complete shutdown (#2694) * fix(windows): reuse stable TUN identities after complete shutdown Wait for NIC forwarding and packet-drain tasks to release their handles before stopping or replacing the interface. Stop the old static-IP NIC before entering its recreation path. Store typed NIC state so shutdown can await the owned tasks instead of only cancelling them on drop. Use the instance UUID for the default Windows interface name and derive a stable Wintun GUID from the device name. Keep explicit names and their existing sharing behavior, while generated names remain dedicated. Keep resolved names out of the input flags so recreation preserves that choice. Pin rust-tun to the fix that joins its reader during session drop. Remove the global NetworkList cleanup that deleted profiles based only on their names. Stable identities avoid creating a new profile for each restart without guessing which existing records are safe to delete. Add regression tests for receiver release and Windows instance restarts with another dedicated or shared-NIC member still running. Validation: formatting and workspace Clippy pass; 66 NIC, shared-NIC and TUN runtime tests pass in the privileged rust container. Fixes #2682 Related: #110 * fix(windows): allocate the first unused numbered TUN name Use et_1, et_2 and subsequent vacant slots for default Windows NICs. Reserve each name across threads and processes with a named kernel object, retaining it through firewall cleanup and releasing it on exit. Check the current device tree to skip disabled adapters without treating removed adapters in the IP interface tables as occupied slots. Keep GUIDs derived from names and preserve explicit NIC sharing. Cover concurrent allocation, case-insensitive reservations and slot reuse. * fix(windows): release idle TUN names across cached and service owners Release a shared NIC name after its dispatcher has stopped and its firewall rules have been removed, even when the registry caches the NIC. Clear the old interface name so a later cache drop cannot remove rules belonging to its replacement. Await dispatcher teardown on control failure and timeout as well as on successful shutdown. Grant only SYNCHRONIZE access to name reservations across accounts so SYSTEM services and desktop processes can skip occupied slots. Cover idle registry reuse, firewall ownership and failed-control cleanup with regression tests. * test(tun): use the renamed dispatcher task handle on Unix Update the two Unix-only teardown tests to match the task field used by explicit dispatcher shutdown. * fix(windows): report firewall cleanup failures and verify all rules Inspect all eight interface rules, including the ALL protocol pair. Treat only a missing rule as absent and propagate other lookup errors. Attempt every rule deletion and report failures with rule names instead of silently returning success after a permission or service error. Validate rule protocols, directions, profiles, interface binding and allow settings on Windows. Cover repeated add/remove, unprivileged failures, full cleanup and shared-NIC rule ownership across restarts. Pin tun-easytier to merged v2 commit 5ba6be073e5b486b68c12f80fac5cfb50601f024 and update Cargo.lock. Its tree matches the previously tested PR commit. --- Cargo.lock | 9 +- easytier/Cargo.toml | 7 +- easytier/src/arch/windows.rs | 156 ++++++++++---- easytier/src/arch/windows/tun_name.rs | 154 ++++++++++++++ easytier/src/common/ifcfg/windows.rs | 74 ------- .../src/instance/runtime_host/tun_common.rs | 66 ++++-- .../src/instance/runtime_host/tun_desktop.rs | 1 + easytier/src/instance/shared_virtual_nic.rs | 7 +- .../instance/shared_virtual_nic/dispatcher.rs | 63 ++++-- .../src/instance/shared_virtual_nic/tests.rs | 74 +++++++ easytier/src/instance/virtual_nic.rs | 192 ++++++++++++++---- 11 files changed, 613 insertions(+), 190 deletions(-) create mode 100644 easytier/src/arch/windows/tun_name.rs diff --git a/Cargo.lock b/Cargo.lock index ebff0a4a..f17937e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8756,6 +8756,12 @@ dependencies = [ "digest 0.11.3", ] +[[package]] +name = "sha1_smol" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbfa15b3dddfee50a0fff136974b3e1bde555604ba463834a7eb7deb6417705d" + [[package]] name = "sha2" version = "0.10.9" @@ -10640,7 +10646,7 @@ checksum = "d2df906b07856748fa3f6e0ad0cbaa047052d4a7dd609e231c4f72cee8c36f31" [[package]] name = "tun-easytier" version = "1.1.1" -source = "git+https://github.com/EasyTier/rust-tun#12378839e7985283df0e4fb536b7137230356db5" +source = "git+https://github.com/EasyTier/rust-tun?rev=5ba6be073e5b486b68c12f80fac5cfb50601f024#5ba6be073e5b486b68c12f80fac5cfb50601f024" dependencies = [ "bytes", "cfg-if", @@ -10881,6 +10887,7 @@ dependencies = [ "js-sys", "rand 0.10.2", "serde_core", + "sha1_smol", "wasm-bindgen", ] diff --git a/easytier/Cargo.toml b/easytier/Cargo.toml index ab774522..2560eb56 100644 --- a/easytier/Cargo.toml +++ b/easytier/Cargo.toml @@ -113,7 +113,7 @@ rcgen = { version = "0.14.10", optional = true } tokio-rustls = { workspace = true, optional = true } # for tap device -tun = { package = "tun-easytier", git = "https://github.com/EasyTier/rust-tun", features = [ +tun = { package = "tun-easytier", git = "https://github.com/EasyTier/rust-tun", rev = "5ba6be073e5b486b68c12f80fac5cfb50601f024", features = [ "async", ], optional = true } # for net ns @@ -127,6 +127,7 @@ nix = { version = "0.31.3", features = [ uuid = { workspace = true, features = [ "v4", + "v5", "fast-rng", "macro-diagnostics", "serde", @@ -257,6 +258,10 @@ windows = { workspace = true, features = [ "Win32_System_Variant", "Win32_Networking_WinSock", "Win32_System_IO", + "Win32_System_Threading", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Devices_DeviceAndDriverInstallation", ] } encoding = "0.2" winreg = "0.56" diff --git a/easytier/src/arch/windows.rs b/easytier/src/arch/windows.rs index d8e113f0..019bb52b 100644 --- a/easytier/src/arch/windows.rs +++ b/easytier/src/arch/windows.rs @@ -4,7 +4,7 @@ use anyhow::Context; use network_interface::NetworkInterfaceConfig; use windows::{ Win32::{ - Foundation::FALSE, + Foundation::{ERROR_FILE_NOT_FOUND, FALSE}, NetworkManagement::WindowsFirewall::{ INetFwPolicy2, INetFwRule, NET_FW_ACTION_ALLOW, NET_FW_PROFILE2_DOMAIN, NET_FW_PROFILE2_PRIVATE, NET_FW_PROFILE2_PUBLIC, NET_FW_RULE_DIR_IN, @@ -23,6 +23,9 @@ use windows::{ core::{BOOL, BSTR}, }; +mod tun_name; +pub(crate) use tun_name::reserve_tun_name; + pub fn disable_connection_reset(socket: &S) -> io::Result<()> { let handle = SOCKET(socket.as_raw_socket() as usize); @@ -61,16 +64,6 @@ pub fn disable_connection_reset(socket: &S) -> io::Result<()> { Ok(()) } -pub fn interface_count() -> io::Result { - let ifaces = network_interface::NetworkInterface::show().map_err(|e| { - io::Error::new( - io::ErrorKind::NotFound, - format!("Failed to get interfaces. error: {}", e), - ) - })?; - Ok(ifaces.len()) -} - pub fn find_interface_index(iface_name: &str) -> io::Result { let ifaces = network_interface::NetworkInterface::show().map_err(|e| { io::Error::new( @@ -382,6 +375,7 @@ pub fn remove_interface_firewall_rules(interface_name: &str) -> anyhow::Result<( }?; let rules = unsafe { policy.Rules()? }; + let mut failures = Vec::new(); for protocol_name in ["TCP", "UDP", "ICMP", "ALL"] { for direction in ["Inbound", "Outbound"] { @@ -390,12 +384,19 @@ pub fn remove_interface_firewall_rules(interface_name: &str) -> anyhow::Result<( interface_name, protocol_name, direction ); let name_bstr = BSTR::from(&rule_name); - unsafe { - let _ = rules.Remove(&name_bstr); // Ignore errors, rule might not exist + // Removing a missing rule succeeds. Keep trying the remaining + // rules on failure, but report failures to the caller. + if let Err(error) = unsafe { rules.Remove(&name_bstr) } { + failures.push(format!("{rule_name}: {error}")); } } } + anyhow::ensure!( + failures.is_empty(), + "failed to remove firewall rules: {}", + failures.join("; ") + ); Ok(()) } @@ -415,8 +416,8 @@ pub fn list_interface_firewall_rules(interface_name: &str) -> anyhow::Result anyhow::Result Ok(true), - Err(_) => Ok(false), + Err(error) if error.code() == ERROR_FILE_NOT_FOUND.to_hresult() => Ok(false), + Err(error) => { + Err(error).with_context(|| format!("failed to inspect firewall rule {rule_name}")) + } } } } @@ -466,10 +459,32 @@ mod tests { assert!(res.is_ok()); } - #[test] - #[ignore] // Requires administrator privileges, ignored by default - fn test_interface_firewall_rules() { - let test_interface = "test_interface"; + #[cfg(feature = "tun")] + #[tokio::test] + #[ignore = "requires Administrator privileges and wintun.dll"] + async fn test_interface_firewall_rules() { + use crate::{ + common::netns::NetNS, + instance::virtual_nic::{VirtualNic, VirtualNicConfig}, + }; + use windows::Win32::System::Ole::{ + SafeArrayGetElement, SafeArrayGetLBound, SafeArrayGetUBound, + }; + + let test_interface = "et_test_firewall"; + let mut nic = VirtualNic::new(VirtualNicConfig::new( + test_interface.to_owned(), + 1500, + NetNS::new(None), + )); + let _tunnel = nic.create_dev().await.unwrap(); + guarden::defer! { + remove_interface_firewall_rules(test_interface).unwrap(); + } + + // Removing missing rules is successful, including on repeated cleanup. + remove_interface_firewall_rules(test_interface).unwrap(); + remove_interface_firewall_rules(test_interface).unwrap(); // Add firewall rules let add_result = add_interface_to_firewall_allowlist(test_interface); @@ -486,17 +501,31 @@ mod tests { // Verify rules were created let rules = list_interface_firewall_rules(test_interface).unwrap(); + assert_eq!(rules.len(), 8); println!("Created {} firewall rules:", rules.len()); for rule in &rules { println!(" - {}", rule); } // Verify required protocol rules are all created - let expected_protocols = ["TCP", "UDP", "ICMP"]; - let expected_directions = ["Inbound", "Outbound"]; + let expected_protocols = [("TCP", 6), ("UDP", 17), ("ICMP", 1), ("ALL", 256)]; + let expected_directions = [ + ("Inbound", NET_FW_RULE_DIR_IN), + ("Outbound", NET_FW_RULE_DIR_OUT), + ]; + let _com = ComInitializer::new().unwrap(); + let policy: INetFwPolicy2 = unsafe { + CoCreateInstance( + &windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2, + None, + CLSCTX_ALL, + ) + .unwrap() + }; + let collection = unsafe { policy.Rules().unwrap() }; - for protocol in &expected_protocols { - for direction in &expected_directions { + for (protocol, number) in expected_protocols { + for (direction, value) in expected_directions { let rule_name = format!( "EasyTier {} - {} Protocol ({})", test_interface, protocol, direction @@ -506,10 +535,46 @@ mod tests { "Missing required rule: {}", rule_name ); + unsafe { + let rule = collection.Item(&BSTR::from(&rule_name)).unwrap(); + assert_eq!(rule.Protocol().unwrap(), number); + assert_eq!(rule.Direction().unwrap(), value); + assert_eq!(rule.Action().unwrap(), NET_FW_ACTION_ALLOW); + assert_eq!( + rule.Enabled().unwrap(), + windows::Win32::Foundation::VARIANT_TRUE + ); + assert_eq!(rule.Profiles().unwrap(), 7); + assert!(rule.ApplicationName().unwrap().is_empty()); + assert_eq!(rule.LocalAddresses().unwrap().to_string(), "*"); + assert_eq!(rule.RemoteAddresses().unwrap().to_string(), "*"); + let interfaces = rule.Interfaces().unwrap(); + assert_eq!( + interfaces.Anonymous.Anonymous.vt, + VARENUM(VT_ARRAY.0 | VT_VARIANT.0) + ); + let array = interfaces.Anonymous.Anonymous.Anonymous.parray; + let index = SafeArrayGetLBound(array, 1).unwrap(); + assert_eq!(SafeArrayGetUBound(array, 1).unwrap(), index); + let mut interface = VARIANT::default(); + SafeArrayGetElement(array, &index, &mut interface as *mut _ as *mut _).unwrap(); + assert_eq!(interface.Anonymous.Anonymous.vt, VT_BSTR); + assert_eq!( + interface.Anonymous.Anonymous.Anonymous.bstrVal.to_string(), + test_interface + ); + } } } - println!("✓ All required protocol rules (TCP/UDP/ICMP) are present"); + println!("✓ All eight protocol rules and their properties are correct"); + + // Repeated starts retain the same rule names. + add_interface_to_firewall_allowlist(test_interface).unwrap(); + assert_eq!( + list_interface_firewall_rules(test_interface).unwrap(), + rules + ); // Remove firewall rules let remove_result = remove_interface_firewall_rules(test_interface); @@ -532,4 +597,23 @@ mod tests { test_interface ); } + + #[test] + #[ignore = "requires a non-administrator token and preinstalled interface rules"] + fn firewall_permission_failures_are_reported() { + let interface = std::env::var("EASYTIER_TEST_FIREWALL_INTERFACE").unwrap(); + assert!(interface.starts_with("et_test_")); + let before = list_interface_firewall_rules(&interface).unwrap(); + assert!( + !before.is_empty(), + "an administrator must install test rules first" + ); + + assert!(add_interface_to_firewall_allowlist(&interface).is_err()); + let error = remove_interface_firewall_rules(&interface).unwrap_err(); + for name in &before { + assert!(error.to_string().contains(name)); + } + assert_eq!(list_interface_firewall_rules(&interface).unwrap(), before); + } } diff --git a/easytier/src/arch/windows/tun_name.rs b/easytier/src/arch/windows/tun_name.rs new file mode 100644 index 00000000..9ba4d40e --- /dev/null +++ b/easytier/src/arch/windows/tun_name.rs @@ -0,0 +1,154 @@ +use std::os::windows::io::{FromRawHandle, OwnedHandle}; + +use windows::{ + Win32::{ + Devices::DeviceAndDriverInstallation::{ + CM_LOCATE_DEVNODE_NORMAL, CM_Locate_DevNodeW, CR_NO_SUCH_DEVNODE, CR_SUCCESS, + }, + Foundation::{ERROR_ALREADY_EXISTS, GetLastError, HLOCAL, LocalFree}, + Security::{ + Authorization::{ + ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, + }, + PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES, + }, + System::Threading::{CreateMutexExW, SYNCHRONIZATION_SYNCHRONIZE}, + }, + core::{HSTRING, w}, +}; +use winreg::{RegKey, enums::HKEY_LOCAL_MACHINE}; + +fn reserve_name(name: &str) -> anyhow::Result> { + let identity = uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_URL, name.to_lowercase().as_bytes()); + let object_name = HSTRING::from(format!("Global\\EasyTier.TunName.{identity}")); + // Reserve the name by the object's lifetime, without thread ownership or + // waiting. Windows also closes the handle if the process is terminated. + let (handle, existed) = unsafe { + // Service and desktop processes must be able to detect each other's + // reservations. Grant SYNCHRONIZE only, without modification rights. + let mut descriptor = PSECURITY_DESCRIPTOR::default(); + ConvertStringSecurityDescriptorToSecurityDescriptorW( + w!("D:(A;;0x00100000;;;WD)"), + SDDL_REVISION_1, + &mut descriptor, + None, + )?; + let attributes = SECURITY_ATTRIBUTES { + nLength: std::mem::size_of::() as u32, + lpSecurityDescriptor: descriptor.0, + ..Default::default() + }; + let result = CreateMutexExW( + Some(&attributes), + &object_name, + 0, + SYNCHRONIZATION_SYNCHRONIZE.0, + ); + let existed = GetLastError() == ERROR_ALREADY_EXISTS; + LocalFree(Some(HLOCAL(descriptor.0))); + let handle = result?; + (OwnedHandle::from_raw_handle(handle.0), existed) + }; + Ok((!existed).then_some(handle)) +} + +fn interface_exists(name: &str) -> anyhow::Result { + // IP interface tables can retain removed adapters after process exit. + // Check the device tree: disabled devices still occupy their names, but + // nonpresent devices left by a terminated process do not. + let network = RegKey::predef(HKEY_LOCAL_MACHINE).open_subkey( + r"SYSTEM\CurrentControlSet\Control\Network\{4D36E972-E325-11CE-BFC1-08002BE10318}", + )?; + for guid in network.enum_keys() { + let connection = match network.open_subkey(format!(r"{}\Connection", guid?)) { + Ok(key) => key, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => continue, + Err(err) => return Err(err.into()), + }; + let existing_name: String = connection.get_value("Name")?; + if !existing_name.eq_ignore_ascii_case(name) { + continue; + } + let instance_id: String = connection.get_value("PnpInstanceID")?; + let mut dev_inst = 0; + let result = unsafe { + CM_Locate_DevNodeW( + &mut dev_inst, + &HSTRING::from(instance_id), + CM_LOCATE_DEVNODE_NORMAL, + ) + }; + match result { + CR_SUCCESS => return Ok(true), + CR_NO_SUCH_DEVNODE => continue, + _ => anyhow::bail!("failed to locate network device {name}: {result:?}"), + } + } + Ok(false) +} + +pub(crate) fn reserve_tun_name(requested: &str) -> anyhow::Result<(String, OwnedHandle)> { + if !requested.is_empty() { + let handle = reserve_name(requested)? + .ok_or_else(|| anyhow::anyhow!("TUN name {requested} is already in use"))?; + return Ok((requested.to_owned(), handle)); + } + + for index in 1..=u32::MAX { + let name = format!("et_{index}"); + let Some(handle) = reserve_name(&name)? else { + continue; + }; + if !interface_exists(&name)? { + return Ok((name, handle)); + } + } + anyhow::bail!("no unused TUN name available") +} + +#[cfg(test)] +mod tests { + use super::reserve_tun_name; + use std::sync::{Arc, Barrier}; + + #[test] + fn explicit_name_reservation_is_case_insensitive_and_released() { + let name = format!("et_test_reserve_{}", uuid::Uuid::new_v4().simple()); + let reservation = reserve_tun_name(&name).unwrap(); + let error = reserve_tun_name(&name.to_uppercase()).unwrap_err(); + assert!(error.to_string().contains("is already in use")); + drop(reservation); + assert!(reserve_tun_name(&name).is_ok()); + } + + #[test] + fn concurrent_default_names_are_unique_and_reuse_vacant_slots() { + let barrier = Arc::new(Barrier::new(8)); + let mut reservations = std::thread::scope(|scope| { + let threads: Vec<_> = (0..8) + .map(|_| { + let barrier = barrier.clone(); + scope.spawn(move || { + barrier.wait(); + reserve_tun_name("").unwrap() + }) + }) + .collect(); + threads + .into_iter() + .map(|thread| thread.join().unwrap()) + .collect::>() + }); + reservations.sort_by_key(|(name, _)| name[3..].parse::().unwrap()); + assert!( + reservations + .windows(2) + .all(|names| names[0].0 != names[1].0) + ); + + let (name, handle) = reservations.remove(0); + drop(handle); + let reused = reserve_tun_name("").unwrap(); + assert_eq!(reused.0, name); + } +} diff --git a/easytier/src/common/ifcfg/windows.rs b/easytier/src/common/ifcfg/windows.rs index fc14e966..a8d7419a 100644 --- a/easytier/src/common/ifcfg/windows.rs +++ b/easytier/src/common/ifcfg/windows.rs @@ -333,80 +333,6 @@ impl RegistryManager { pub const NETBT_INTERFACE_PREFIX: &str = r"SYSTEM\CurrentControlSet\Services\NetBT\Parameters\Interfaces\Tcpip_"; - pub fn reg_delete_obsoleted_items(dev_name: &str) -> io::Result<()> { - use winreg::{RegKey, enums::HKEY_LOCAL_MACHINE, enums::KEY_ALL_ACCESS}; - let hklm = RegKey::predef(HKEY_LOCAL_MACHINE); - let profiles_key = hklm.open_subkey_with_flags( - "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\NetworkList\\Profiles", - KEY_ALL_ACCESS, - )?; - let unmanaged_key = hklm.open_subkey_with_flags( - "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\NetworkList\\Signatures\\Unmanaged", - KEY_ALL_ACCESS, - )?; - // collect subkeys to delete - let mut keys_to_delete = Vec::new(); - let mut keys_to_delete_unmanaged = Vec::new(); - for subkey_name in profiles_key.enum_keys().filter_map(Result::ok) { - let subkey = profiles_key.open_subkey(&subkey_name)?; - // check if ProfileName contains "et" - match subkey.get_value::("ProfileName") { - Ok(profile_name) => { - if profile_name.contains("et_") - || (!dev_name.is_empty() && dev_name == profile_name) - { - keys_to_delete.push(subkey_name); - } - } - Err(e) => { - tracing::error!( - "Failed to read ProfileName for subkey {}: {}", - subkey_name, - e - ); - } - } - } - for subkey_name in unmanaged_key.enum_keys().filter_map(Result::ok) { - let subkey = unmanaged_key.open_subkey(&subkey_name)?; - // check if ProfileName contains "et" - match subkey.get_value::("Description") { - Ok(profile_name) => { - if profile_name.contains("et_") - || (!dev_name.is_empty() && dev_name == profile_name) - { - keys_to_delete_unmanaged.push(subkey_name); - } - } - Err(e) => { - tracing::error!( - "Failed to read ProfileName for subkey {}: {}", - subkey_name, - e - ); - } - } - } - // delete collected subkeys - if !keys_to_delete.is_empty() { - for subkey_name in keys_to_delete { - match profiles_key.delete_subkey_all(&subkey_name) { - Ok(_) => tracing::trace!("Successfully deleted subkey: {}", subkey_name), - Err(e) => tracing::error!("Failed to delete subkey {}: {}", subkey_name, e), - } - } - } - if !keys_to_delete_unmanaged.is_empty() { - for subkey_name in keys_to_delete_unmanaged { - match unmanaged_key.delete_subkey_all(&subkey_name) { - Ok(_) => tracing::trace!("Successfully deleted subkey: {}", subkey_name), - Err(e) => tracing::error!("Failed to delete subkey {}: {}", subkey_name, e), - } - } - } - Ok(()) - } - pub fn reg_change_catrgory_in_profile(dev_name: &str) -> io::Result<()> { use winreg::{RegKey, enums::HKEY_LOCAL_MACHINE, enums::KEY_ALL_ACCESS}; let hklm = RegKey::predef(HKEY_LOCAL_MACHINE); diff --git a/easytier/src/instance/runtime_host/tun_common.rs b/easytier/src/instance/runtime_host/tun_common.rs index af981041..80c3d2ba 100644 --- a/easytier/src/instance/runtime_host/tun_common.rs +++ b/easytier/src/instance/runtime_host/tun_common.rs @@ -1,7 +1,4 @@ -use std::{ - any::Any, - sync::{Arc, OnceLock}, -}; +use std::sync::{Arc, OnceLock}; #[cfg(not(mobile))] use easytier_core::instance::CorePacketPlane; @@ -18,24 +15,24 @@ use super::MagicDnsRuntime; use crate::instance::shared_virtual_nic::ArcSharedVirtualNicRegistry; use crate::{common::global_ctx::ArcGlobalCtx, instance::virtual_nic::NicCtx}; -struct NicCtxContainer { - _nic_ctx: Option>, - magic_dns: MagicDnsRuntime, +enum NicCtxContainer { + Active { + nic_ctx: Box, + magic_dns: MagicDnsRuntime, + }, + PacketDrain(JoinSet<()>), } impl NicCtxContainer { fn new(nic_ctx: NicCtx, magic_dns: MagicDnsRuntime) -> Self { - Self { - _nic_ctx: Some(Box::new(nic_ctx)), + Self::Active { + nic_ctx: Box::new(nic_ctx), magic_dns, } } fn packet_drain(tasks: JoinSet<()>) -> Self { - Self { - _nic_ctx: Some(Box::new(tasks)), - magic_dns: MagicDnsRuntime::default(), - } + Self::PacketDrain(tasks) } } @@ -67,11 +64,18 @@ impl TunNicState { } pub(super) async fn stop(&self) { - let mut old = self.nic_ctx.lock().await.take(); - if let Some(nic) = old.as_mut() { - nic.magic_dns.stop().await; + let old = self.nic_ctx.lock().await.take(); + match old { + Some(NicCtxContainer::Active { + mut nic_ctx, + mut magic_dns, + }) => { + magic_dns.stop().await; + nic_ctx.shutdown().await; + } + Some(NicCtxContainer::PacketDrain(mut tasks)) => tasks.shutdown().await, + None => {} } - drop(old); } #[cfg(mobile)] @@ -79,8 +83,10 @@ impl TunNicState { &self, ) -> Option { let nic = self.nic_ctx.lock().await; - let nic = nic.as_ref()?._nic_ctx.as_ref()?.downcast_ref::()?; - nic.current_mobile_tun_claims().await + match nic.as_ref()? { + NicCtxContainer::Active { nic_ctx, .. } => nic_ctx.current_mobile_tun_claims().await, + NicCtxContainer::PacketDrain(_) => None, + } } pub(super) async fn drain(&self) { @@ -194,6 +200,28 @@ mod tests { use super::*; use crate::{common::global_ctx::GlobalCtxEvent, instance::test_instance::TestInstance}; + #[tokio::test] + async fn stop_releases_packet_receiver_before_returning() { + let state = TunNicState::empty(); + let (_sender, receiver) = tokio::sync::mpsc::channel(1); + state + .install_receiver(HostPacketReceiver::new(receiver)) + .unwrap(); + let receiver = state.receiver(); + state.drain().await; + // Let the drain acquire the receiver and block waiting for a packet. + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while receiver.try_lock().is_ok() { + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + + state.stop().await; + assert!(receiver.try_lock().is_ok()); + } + #[tokio::test] async fn dhcp_preserves_publication_permit_and_cancelled_address() { let instance = TestInstance::new_with_process_runtime( diff --git a/easytier/src/instance/runtime_host/tun_desktop.rs b/easytier/src/instance/runtime_host/tun_desktop.rs index 8b575d6a..b4bcb670 100644 --- a/easytier/src/instance/runtime_host/tun_desktop.rs +++ b/easytier/src/instance/runtime_host/tun_desktop.rs @@ -78,6 +78,7 @@ impl NativeTunRuntime { Self::report_static_ip_cancelled(&mut output); return; } + nic_state.stop().await; let closed = Arc::new(Notify::new()); let mut nic = NicCtx::new( global_ctx.clone(), diff --git a/easytier/src/instance/shared_virtual_nic.rs b/easytier/src/instance/shared_virtual_nic.rs index f8041100..e732291e 100644 --- a/easytier/src/instance/shared_virtual_nic.rs +++ b/easytier/src/instance/shared_virtual_nic.rs @@ -246,6 +246,8 @@ impl SharedVirtualNic { if let Some(dispatcher) = self.dispatcher.take() { dispatcher.shutdown_without_invalidation().await; } + #[cfg(target_os = "windows")] + self.nic.lock().await.clear_windows_device(); } async fn apply_ifcfg_update(&self, next_ifcfg: &SharedIfConfig) -> Result<(), Error> { @@ -362,7 +364,10 @@ impl SharedVirtualNic { self.member_tunnel_table.clone(), self.valid.clone(), ); - self.sync_dispatcher_sources(&dispatcher).await?; + if let Err(err) = self.sync_dispatcher_sources(&dispatcher).await { + dispatcher.shutdown_without_invalidation().await; + return Err(err); + } self.dispatcher = Some(dispatcher); Ok(()) } diff --git a/easytier/src/instance/shared_virtual_nic/dispatcher.rs b/easytier/src/instance/shared_virtual_nic/dispatcher.rs index c06188a2..cf1de981 100644 --- a/easytier/src/instance/shared_virtual_nic/dispatcher.rs +++ b/easytier/src/instance/shared_virtual_nic/dispatcher.rs @@ -508,7 +508,7 @@ impl SharedVirtualNicNatTable { } pub(super) struct SharedVirtualNicDispatcher { - _task: AbortOnDropHandle<()>, + task: AbortOnDropHandle<()>, control_sender: mpsc::UnboundedSender, } @@ -534,7 +534,7 @@ impl SharedVirtualNicDispatcher { }; Self { - _task: AbortOnDropHandle::new(SHARED_VIRTUAL_NIC_RUNTIME.spawn(task.run())), + task: AbortOnDropHandle::new(SHARED_VIRTUAL_NIC_RUNTIME.spawn(task.run())), control_sender, } } @@ -566,7 +566,7 @@ impl SharedVirtualNicDispatcher { let task = SHARED_VIRTUAL_NIC_RUNTIME.spawn(task.run_for_mobile(first_open_sender)); let dispatcher = Self { - _task: AbortOnDropHandle::new(task), + task: AbortOnDropHandle::new(task), control_sender, }; await_mobile_open(first_open_receiver) @@ -631,23 +631,25 @@ impl SharedVirtualNicDispatcher { pub(super) async fn shutdown_without_invalidation(self) { let (ack, rx) = oneshot::channel(); - if self + let sent = self .control_sender .send(SharedVirtualNicControl::Shutdown { invalidate: false, ack, }) - .is_err() - { - return; - } + .is_ok(); - if tokio::time::timeout(DISPATCHER_SHUTDOWN_TIMEOUT, rx) - .await - .is_err() + if sent + && tokio::time::timeout(DISPATCHER_SHUTDOWN_TIMEOUT, rx) + .await + .is_err() { tracing::warn!("timed out shutting down shared virtual nic dispatcher"); } + // Even on send failure or timeout, wait for the task's TUN halves to + // be dropped before releasing the device name for another owner. + self.task.abort(); + let _ = self.task.await; } } @@ -2025,7 +2027,7 @@ mod tests { }, }; let dispatcher = SharedVirtualNicDispatcher { - _task: AbortOnDropHandle::new(tokio::spawn(task.run())), + task: AbortOnDropHandle::new(tokio::spawn(task.run())), control_sender: control_sender.clone(), }; (dispatcher, control_sender) @@ -2104,7 +2106,7 @@ mod tests { } assert!(!valid.load(Ordering::Acquire)); assert!(member_tunnel_table.control_sender().is_none()); - tokio::time::timeout(Duration::from_secs(1), dispatcher._task) + tokio::time::timeout(Duration::from_secs(1), dispatcher.task) .await .unwrap() .unwrap(); @@ -2170,7 +2172,7 @@ mod tests { .await .is_err() ); - tokio::time::timeout(Duration::from_secs(1), dispatcher._task) + tokio::time::timeout(Duration::from_secs(1), dispatcher.task) .await .unwrap() .unwrap(); @@ -2210,7 +2212,7 @@ mod tests { } assert!(!valid.load(Ordering::Acquire)); assert!(member_tunnel_table.control_sender().is_none()); - tokio::time::timeout(Duration::from_secs(1), dispatcher._task) + tokio::time::timeout(Duration::from_secs(1), dispatcher.task) .await .unwrap() .unwrap(); @@ -3077,6 +3079,35 @@ mod tests { assert!(member_tunnel_table.control_sender().is_none()); } + #[tokio::test] + async fn dispatcher_shutdown_waits_for_resources_when_control_fails() { + for receiver_closed in [true, false] { + let (control_sender, control_receiver) = mpsc::unbounded_channel(); + let resource = Arc::new(()); + let weak = Arc::downgrade(&resource); + let (ready, started) = oneshot::channel(); + let task = tokio::spawn(async move { + let _resource = resource; + let _receiver = if receiver_closed { + drop(control_receiver); + None + } else { + Some(control_receiver) + }; + ready.send(()).unwrap(); + std::future::pending::<()>().await; + }); + started.await.unwrap(); + let dispatcher = SharedVirtualNicDispatcher { + task: AbortOnDropHandle::new(task), + control_sender, + }; + + dispatcher.shutdown_without_invalidation().await; + assert!(weak.upgrade().is_none()); + } + } + #[tokio::test] async fn cancelled_member_updates_finish_before_registration_cleanup() { use super::super::{SharedVirtualNic, SharedVirtualNicMember}; @@ -3222,7 +3253,7 @@ mod tests { } }); shared_nic.dispatcher = Some(SharedVirtualNicDispatcher { - _task: AbortOnDropHandle::new(task), + task: AbortOnDropHandle::new(task), control_sender, }); let table = shared_nic.member_tunnel_table.clone(); diff --git a/easytier/src/instance/shared_virtual_nic/tests.rs b/easytier/src/instance/shared_virtual_nic/tests.rs index 03aa470a..e228144b 100644 --- a/easytier/src/instance/shared_virtual_nic/tests.rs +++ b/easytier/src/instance/shared_virtual_nic/tests.rs @@ -7,6 +7,80 @@ use super::*; const MEMBER: SharedVirtualNicMemberId = uuid::Uuid::from_u128(1); const REGISTRATION: SharedVirtualNicMemberRegistrationId = uuid::Uuid::from_u128(10); +#[cfg(target_os = "windows")] +#[tokio::test] +#[ignore = "requires Administrator privileges and wintun.dll"] +async fn idle_shared_nic_releases_name_while_registry_remains_alive() { + use crate::{ + arch::windows::list_interface_firewall_rules, + common::config::{ConfigLoader, TomlConfigLoader}, + instance::test_instance::TestInstance, + }; + use easytier_core::process_runtime::CoreProcessRuntime; + use std::time::Duration; + + let name = "et_test_idle_shared"; + let config = || { + let config = + TomlConfigLoader::new_from_str("ipv4 = \"198.18.28.1/30\"\nlisteners = []").unwrap(); + let mut flags = config.get_flags(); + flags.dev_name = name.to_owned(); + config.set_flags(flags); + config + }; + let registry = TestInstance::new_shared_virtual_nic_registry(); + let mut first = TestInstance::new_with_process_runtime_and_shared_virtual_nic_registry( + config(), + CoreProcessRuntime::new(), + registry.clone(), + ); + first.run().await.unwrap(); + assert_eq!(list_interface_firewall_rules(name).unwrap().len(), 8); + let cached = registry + .lock() + .await + .get_by_dev_name_for_test(name) + .unwrap(); + first.clear_resources().await; + tokio::time::timeout(Duration::from_secs(5), async { + loop { + let nic = cached.lock().await; + if nic.member_registrations.is_empty() && nic.dispatcher.is_none() { + break; + } + drop(nic); + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + assert!(list_interface_firewall_rules(name).unwrap().is_empty()); + + // An independent registry has the same name-allocation boundary as a + // separate process. The previous registry deliberately remains alive. + let mut second = TestInstance::new_with_process_runtime_and_shared_virtual_nic_registry( + config(), + CoreProcessRuntime::new(), + TestInstance::new_shared_virtual_nic_registry(), + ); + second.run().await.unwrap(); + assert_eq!(second.get_global_ctx().get_tun_device_name().unwrap(), name); + let rules = list_interface_firewall_rules(name).unwrap(); + assert_eq!(rules.len(), 8); + drop(first); + drop(registry); + drop(cached); + assert_eq!(list_interface_firewall_rules(name).unwrap(), rules); + second.clear_resources().await; + tokio::time::timeout(Duration::from_secs(5), async { + while !list_interface_firewall_rules(name).unwrap().is_empty() { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); +} + fn virtual_nic_config(mtu: u32, netns: Option<&str>) -> VirtualNicConfig { VirtualNicConfig::new("et0".to_owned(), mtu, NetNS::new(netns.map(str::to_owned))) } diff --git a/easytier/src/instance/virtual_nic.rs b/easytier/src/instance/virtual_nic.rs index 69c0e260..f37266f7 100644 --- a/easytier/src/instance/virtual_nic.rs +++ b/easytier/src/instance/virtual_nic.rs @@ -272,32 +272,43 @@ pub struct VirtualNic { ifname: Option, ifcfg: Box, + + // Keep the name reserved through firewall cleanup. The device + // itself may already have disappeared when its forwarding tasks stop. + #[cfg(target_os = "windows")] + name_reservation: Option, } impl Drop for VirtualNic { fn drop(&mut self) { #[cfg(target_os = "windows")] - { - if let Some(ref ifname) = self.ifname { - // Try to clean up firewall rules, but don't panic in destructor - if let Err(error) = crate::arch::windows::remove_interface_firewall_rules(ifname) { - tracing::warn!( - %error, - "failed to remove firewall rules for interface {}", - ifname - ); - } - } - } + self.clear_windows_device(); } } impl VirtualNic { + #[cfg(target_os = "windows")] + pub(crate) fn clear_windows_device(&mut self) { + if let Some(ifname) = self.ifname.take() { + // Try to clean up firewall rules, but don't panic in destructor + if let Err(error) = crate::arch::windows::remove_interface_firewall_rules(&ifname) { + tracing::warn!( + %error, + "failed to remove firewall rules for interface {}", + ifname + ); + } + } + self.name_reservation.take(); + } + pub fn new(config: VirtualNicConfig) -> Self { Self { config, ifname: None, ifcfg: Box::new(IfConfiger::default()), + #[cfg(target_os = "windows")] + name_reservation: None, } } @@ -548,6 +559,12 @@ impl VirtualNic { #[cfg(target_os = "windows")] { + if self.name_reservation.is_none() { + let (name, reservation) = + crate::arch::windows::reserve_tun_name(&self.config.dev_name)?; + self.config.dev_name = name; + self.name_reservation = Some(reservation); + } let dev_name = self.config.dev_name.clone(); match crate::arch::windows::add_self_to_firewall_allowlist() { @@ -560,30 +577,14 @@ impl VirtualNic { } } - match RegistryManager::reg_delete_obsoleted_items(&dev_name) { - Ok(_) => tracing::trace!("delete successful!"), - Err(e) => tracing::error!("An error occurred: {}", e), - } - - if !dev_name.is_empty() { - config.tun_name(&dev_name); - } else { - use rand::distributions::Distribution as _; - let c = crate::arch::windows::interface_count()?; - let mut rng = rand::thread_rng(); - let s: String = rand::distributions::Alphanumeric - .sample_iter(&mut rng) - .take(4) - .map(char::from) - .collect::() - .to_lowercase(); - - let random_dev_name = format!("et_{}_{}", c, s); - config.tun_name(random_dev_name.clone()); - self.config.dev_name = random_dev_name; - } - + config.tun_name(&dev_name); config.platform_config(|config| { + // Wintun uses the GUID for Windows network profile identity. + // Include the name so explicitly shared NICs use the same GUID. + let identity = format!("https://easytier.rs/wintun/{}", dev_name.to_lowercase()); + config.device_guid( + uuid::Uuid::new_v5(&uuid::Uuid::NAMESPACE_URL, identity.as_bytes()).as_u128(), + ); config.skip_config(true); config.ring_cap(Some(std::cmp::min( config.min_ring_cap() * 32, @@ -1080,7 +1081,9 @@ impl NicCtx { mtu -= 20; } - VirtualNicConfig::new(flags.dev_name, mtu, global_ctx.net_ns.clone()) + let dev_name = flags.dev_name; + + VirtualNicConfig::new(dev_name, mtu, global_ctx.net_ns.clone()) } pub(crate) fn shared_route_backend_for_dns(&self) -> Option { @@ -1126,7 +1129,8 @@ impl NicCtx { net_ns: NetNS::new(None), ..nic_config }; - let backend = if cfg!(mobile) || !nic_config.dev_name.is_empty() { + // Only an explicitly configured name opts into sharing. + let backend = if cfg!(mobile) || !global_ctx.get_flags().dev_name.is_empty() { NicBackend::Shared(registry.lock().await.create_member( nic_config.dev_name.clone(), nic_config, @@ -1150,6 +1154,12 @@ impl NicCtx { self.backend.ifname().await } + pub(crate) async fn shutdown(&mut self) { + // The forwarding tasks own the TUN stream and sink. Wait until both + // release the device before another NIC with the same name is created. + self.tasks.shutdown().await; + } + async fn tun_ifname(&self) -> Result { self.backend .ifname() @@ -1561,11 +1571,6 @@ impl NicCtx { #[cfg(target_os = "windows")] { - let mut flags = self.global_ctx.get_flags(); - if flags.dev_name.is_empty() { - flags.dev_name = ifname.clone(); - self.global_ctx.set_flags(flags); - } let _ = RegistryManager::reg_change_catrgory_in_profile(&ifname); } @@ -1710,6 +1715,109 @@ mod tests { use super::{NicCtx, VirtualNic}; + #[cfg(target_os = "windows")] + #[rstest::rstest] + #[case(None)] + #[case(Some("et_test_restart"))] + #[tokio::test] + #[ignore = "requires Administrator privileges and wintun.dll"] + async fn windows_device_identity_survives_recreation(#[case] dev_name: Option<&str>) { + use crate::{ + arch::windows::list_interface_firewall_rules, + common::{ + config::{ConfigLoader, TomlConfigLoader}, + ifcfg::RegistryManager, + }, + instance::test_instance::TestInstance, + }; + use easytier_core::process_runtime::CoreProcessRuntime; + + let process = CoreProcessRuntime::new(); + let registry = TestInstance::new_shared_virtual_nic_registry(); + let new_instance = |config: TomlConfigLoader| { + if let Some(dev_name) = dev_name { + let mut flags = config.get_flags(); + flags.dev_name = dev_name.to_owned(); + config.set_flags(flags); + } + TestInstance::new_with_process_runtime_and_shared_virtual_nic_registry( + config, + process.clone(), + registry.clone(), + ) + }; + let config = + TomlConfigLoader::new_from_str("ipv4 = \"198.18.26.1/30\"\nlisteners = []").unwrap(); + let mut first = new_instance(config); + let saved_config = first.get_global_ctx().config.dump(); + first.run().await.unwrap(); + let name = first.get_global_ctx().get_tun_device_name().unwrap(); + let guid = RegistryManager::find_interface_guid(&name).unwrap(); + assert_eq!(list_interface_firewall_rules(&name).unwrap().len(), 8); + // The resolved name is runtime state, not an explicit request to share + // the device when this GlobalCtx next recreates its NIC. + assert_eq!( + first.get_global_ctx().get_flags().dev_name, + dev_name.unwrap_or_default(), + ); + assert_eq!( + registry + .lock() + .await + .get_by_dev_name_for_test(&name) + .is_some(), + dev_name.is_some(), + ); + + // Keep B running while A is stopped and recreated, as in issue #110. + let other_config = + TomlConfigLoader::new_from_str("ipv4 = \"198.18.27.1/30\"\nlisteners = []").unwrap(); + let mut other = new_instance(other_config); + other.run().await.unwrap(); + let other_name = other.get_global_ctx().get_tun_device_name().unwrap(); + let other_guid = RegistryManager::find_interface_guid(&other_name).unwrap(); + if dev_name.is_some() { + assert_eq!(name, other_name); + assert_eq!(guid, other_guid); + } else { + assert_ne!(name, other_name); + assert_ne!(guid, other_guid); + } + first.clear_resources().await; + + for _ in 0..10 { + let config = TomlConfigLoader::new_from_str(&saved_config).unwrap(); + let mut restarted = new_instance(config); + restarted.run().await.unwrap(); + assert_eq!( + restarted.get_global_ctx().get_tun_device_name().unwrap(), + name + ); + assert_eq!(RegistryManager::find_interface_guid(&name).unwrap(), guid); + assert_eq!( + RegistryManager::find_interface_guid(&other_name).unwrap(), + other_guid, + ); + assert_eq!(list_interface_firewall_rules(&name).unwrap().len(), 8); + restarted.clear_resources().await; + assert_eq!(list_interface_firewall_rules(&other_name).unwrap().len(), 8); + if dev_name.is_none() { + assert!(list_interface_firewall_rules(&name).unwrap().is_empty()); + } + } + other.clear_resources().await; + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while !list_interface_firewall_rules(&other_name) + .unwrap() + .is_empty() + { + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + }) + .await + .unwrap(); + } + #[cfg(target_os = "linux")] #[tokio::test] async fn mobile_configuration_registers_ipv6_and_tracks_public_updates() {