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() {