From 2087800f46766df575ebd9acbbb912aeca8f7ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Fri, 6 Oct 2023 23:04:23 +0200 Subject: [PATCH 1/8] Make BurstGuard configurable --- talpid-routing/src/debounce.rs | 46 +++++++++++++------ talpid-routing/src/unix/macos/mod.rs | 21 ++++++--- .../src/windows/default_route_monitor.rs | 14 ++++-- 3 files changed, 57 insertions(+), 24 deletions(-) diff --git a/talpid-routing/src/debounce.rs b/talpid-routing/src/debounce.rs index ba1e52250ce2..2463b2c429fb 100644 --- a/talpid-routing/src/debounce.rs +++ b/talpid-routing/src/debounce.rs @@ -14,34 +14,39 @@ use std::{ /// `buffer_period`. At which point the wrapped function will be called. pub struct BurstGuard { sender: Sender, + /// This is the period of time the `BurstGuard` will wait for a new trigger to be sent + /// before it calls the callback. + buffer_period: Duration, + /// This is the longest period that the `BurstGuard` will wait from the first trigger till + /// it calls the callback. + longest_buffer_period: Duration, } enum BurstGuardEvent { - Trigger, + Trigger(Duration), Shutdown(Sender<()>), } impl BurstGuard { - pub fn new(callback: F) -> Self { - /// This is the period of time the `BurstGuard` will wait for a new trigger to be sent - /// before it calls the callback. - const BURST_BUFFER_PERIOD: Duration = Duration::from_millis(200); - /// This is the longest period that the `BurstGuard` will wait from the first trigger till - /// it calls the callback. - const BURST_LONGEST_BUFFER_PERIOD: Duration = Duration::from_secs(2); - + pub fn new( + buffer_period: Duration, + longest_buffer_period: Duration, + callback: F, + ) -> Self { let (sender, listener) = channel(); std::thread::spawn(move || { // The `stop` implementation assumes that this thread will not call `callback` again // if the listener has been dropped. while let Ok(message) = listener.recv() { match message { - BurstGuardEvent::Trigger => { + BurstGuardEvent::Trigger(mut period) => { let start = Instant::now(); loop { - match listener.recv_timeout(BURST_BUFFER_PERIOD) { - Ok(BurstGuardEvent::Trigger) => { - if start.elapsed() >= BURST_LONGEST_BUFFER_PERIOD { + match listener.recv_timeout(period) { + Ok(BurstGuardEvent::Trigger(new_period)) => { + period = new_period; + let max_period = std::cmp::max(longest_buffer_period, period); + if start.elapsed() >= max_period { callback(); break; } @@ -67,7 +72,11 @@ impl BurstGuard { } } }); - Self { sender } + Self { + sender, + buffer_period, + longest_buffer_period, + } } /// When `stop` returns an then the `BurstGuard` thread is guaranteed to not make any further @@ -90,6 +99,13 @@ impl BurstGuard { /// Asynchronously trigger burst pub fn trigger(&self) { - self.sender.send(BurstGuardEvent::Trigger).unwrap(); + self.trigger_with_period(self.buffer_period) + } + + /// Asynchronously trigger burst + pub fn trigger_with_period(&self, buffer_period: Duration) { + self.sender + .send(BurstGuardEvent::Trigger(buffer_period)) + .unwrap(); } } diff --git a/talpid-routing/src/unix/macos/mod.rs b/talpid-routing/src/unix/macos/mod.rs index 8cca3594f90e..24923ea9af99 100644 --- a/talpid-routing/src/unix/macos/mod.rs +++ b/talpid-routing/src/unix/macos/mod.rs @@ -25,6 +25,9 @@ mod watch; pub type Result = std::result::Result; +const BURST_BUFFER_PERIOD: Duration = Duration::from_millis(200); +const BURST_LONGEST_BUFFER_PERIOD: Duration = Duration::from_secs(2); + /// Errors that can happen in the macOS routing integration. #[derive(err_derive::Error, Debug)] #[error(no_from)] @@ -93,12 +96,18 @@ impl RouteManagerImpl { manage_tx: Weak>, ) -> Result { let routing_table = RoutingTable::new().map_err(Error::RoutingTable)?; - let update_trigger = BurstGuard::new(move || { - let Some(manage_tx) = manage_tx.upgrade() else { - return; - }; - let _ = manage_tx.unbounded_send(RouteManagerCommand::RefreshRoutes); - }); + + let update_trigger = BurstGuard::new( + BURST_BUFFER_PERIOD, + BURST_LONGEST_BUFFER_PERIOD, + move || { + let Some(manage_tx) = manage_tx.upgrade() else { + return; + }; + let _ = manage_tx.unbounded_send(RouteManagerCommand::RefreshRoutes); + }, + ); + Ok(Self { routing_table, non_tunnel_routes: HashSet::new(), diff --git a/talpid-routing/src/windows/default_route_monitor.rs b/talpid-routing/src/windows/default_route_monitor.rs index d42dbc91de7e..0f7d64e3a892 100644 --- a/talpid-routing/src/windows/default_route_monitor.rs +++ b/talpid-routing/src/windows/default_route_monitor.rs @@ -7,6 +7,7 @@ use crate::debounce::BurstGuard; use std::{ ffi::c_void, sync::{Arc, Mutex}, + time::Duration, }; use talpid_types::win32_err; use windows_sys::Win32::{ @@ -173,10 +174,17 @@ impl DefaultRouteMonitor { family, ))); + const BURST_BUFFER_PERIOD: Duration = Duration::from_millis(200); + const BURST_LONGEST_BUFFER_PERIOD: Duration = Duration::from_secs(2); + let moved_context = context.clone(); - let burst_guard = Mutex::new(BurstGuard::new(move || { - moved_context.lock().unwrap().evaluate_routes(); - })); + let burst_guard = Mutex::new(BurstGuard::new( + BURST_BUFFER_PERIOD, + BURST_LONGEST_BUFFER_PERIOD, + move || { + moved_context.lock().unwrap().evaluate_routes(); + }, + )); // SAFETY: We need to send the ContextAndBurstGuard to the windows notification functions as // a raw pointer. This imposes the requirement it is not mutated or dropped until From a96a16c8329c11804840b769c5ef2c7c618ee587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Fri, 6 Oct 2023 23:14:10 +0200 Subject: [PATCH 2/8] Create DynamicStore on startup in macOS route monitor --- talpid-routing/src/unix/macos/interface.rs | 251 ++++++++++++--------- talpid-routing/src/unix/macos/mod.rs | 71 +++--- 2 files changed, 189 insertions(+), 133 deletions(-) diff --git a/talpid-routing/src/unix/macos/interface.rs b/talpid-routing/src/unix/macos/interface.rs index 85b76377d4d7..0df96ab6b377 100644 --- a/talpid-routing/src/unix/macos/interface.rs +++ b/talpid-routing/src/unix/macos/interface.rs @@ -15,7 +15,7 @@ use system_configuration::{ dictionary::CFDictionary, string::CFString, }, - dynamic_store::SCDynamicStoreBuilder, + dynamic_store::{SCDynamicStore, SCDynamicStoreBuilder}, network_configuration::SCNetworkSet, preferences::SCPreferences, sys::schema_definitions::{ @@ -41,42 +41,165 @@ impl std::fmt::Display for Family { } } -impl From for IpNetwork { - fn from(fam: Family) -> Self { - match fam { +impl Family { + pub fn default_network(self) -> IpNetwork { + match self { Family::V4 => IpNetwork::new(Ipv4Addr::UNSPECIFIED.into(), 0).unwrap(), Family::V6 => IpNetwork::new(Ipv6Addr::UNSPECIFIED.into(), 0).unwrap(), } } } -/// Retrieve the best current default route. That is the first scoped default route, ordered by -/// network service order, and with interfaces filtered out if they do not have valid IP addresses -/// assigned. -/// -/// # Note -/// -/// The tunnel interface is not even listed in the service order, so it will be skipped. -pub fn get_best_default_route(family: Family) -> Option { - for iface in network_service_order(family) { - let Ok(index) = if_nametoindex(iface.name.as_str()) else { - continue; - }; +struct NetworkServiceDetails { + name: String, + router_ip: IpAddr, +} + +pub struct PrimaryInterfaceMonitor { + store: SCDynamicStore, + set: SCNetworkSet, +} - // Request ifscoped default route for this interface - let msg = RouteMessage::new_route(Destination::Network(IpNetwork::from(family))) +// FIXME: Implement Send on SCDynamicStore, if it's safe +unsafe impl Send for PrimaryInterfaceMonitor {} + +impl PrimaryInterfaceMonitor { + pub fn new() -> Self { + let store = SCDynamicStoreBuilder::new("talpid-routing").build(); + let prefs = SCPreferences::default(&CFString::new("talpid-routing")); + let set = SCNetworkSet::new(&prefs); + Self { store, set } + } + + /// Retrieve the best current default route. This is based on the primary interface, or else + /// the first active interface in the network service order. + pub fn get_route(&self, family: Family) -> Option { + let ifaces = self + .get_primary_interface(family) + .map(|iface| { + log::debug!("Found primary interface for {family}"); + vec![iface] + }) + .unwrap_or_else(|| { + log::debug!("No primary interface for {family}. Checking service order"); + self.network_services(family) + }); + + let (iface, index) = ifaces + .into_iter() + .filter_map(|iface| { + let index = if_nametoindex(iface.name.as_str()).map_err(|error| { + log::error!("Failed to retrieve interface index for \"{}\": {error}", iface.name); + error + }).ok()?; + + let active = is_active_interface(&iface.name, family).unwrap_or_else(|error| { + log::error!("is_active_interface() returned an error for interface \"{}\", assuming active. Error: {error}", iface.name); + true + }); + if !active { + log::debug!("Skipping inactive interface {}, router IP {}", iface.name, iface.router_ip); + return None; + } + Some((iface, index)) + }) + .next()?; + + // Synthesize a scoped route for the interface + let msg = RouteMessage::new_route(Destination::Network(family.default_network())) .set_gateway_addr(iface.router_ip) .set_interface_index(u16::try_from(index).unwrap()); - let active = is_active_interface(&iface.name, family).unwrap_or_else(|error| { - log::error!("is_active_interface() returned an error for interface \"{}\", assuming active. Error: {error}", iface.name); - true - }); - if active { - return Some(msg); - } + Some(msg) + } + + fn get_primary_interface(&self, family: Family) -> Option { + let global_name = if family == Family::V4 { + "State:/Network/Global/IPv4" + } else { + "State:/Network/Global/IPv6" + }; + let global_dict = self + .store + .get(CFString::new(global_name)) + .and_then(|v| v.downcast_into::())?; + let name = global_dict + .find(unsafe { kSCDynamicStorePropNetPrimaryInterface }.to_void()) + .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) + .and_then(|s| s.downcast::()) + .map(|s| s.to_string()) + .or_else(|| { + log::debug!("Missing name for primary interface ({family})"); + None + })?; + + let router_key = if family == Family::V4 { + unsafe { kSCPropNetIPv4Router.to_void() } + } else { + unsafe { kSCPropNetIPv6Router.to_void() } + }; + + let router_ip = global_dict + .find(router_key) + .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) + .and_then(|s| s.downcast::()) + .and_then(|ip| ip.to_string().parse().ok()) + .or_else(|| { + log::debug!("Missing router IP for primary interface \"{name}\""); + None + })?; + + Some(NetworkServiceDetails { name, router_ip }) } - None + fn network_services(&self, family: Family) -> Vec { + let router_key = if family == Family::V4 { + unsafe { kSCPropNetIPv4Router.to_void() } + } else { + unsafe { kSCPropNetIPv6Router.to_void() } + }; + + self.set + .service_order() + .iter() + .filter_map(|service_id| { + let service_id_s = service_id.to_string(); + let key = if family == Family::V4 { + format!("State:/Network/Service/{service_id_s}/IPv4") + } else { + format!("State:/Network/Service/{service_id_s}/IPv6") + }; + + let ip_dict = self + .store + .get(CFString::new(&key)) + .and_then(|v| v.downcast_into::()) + .or_else(|| { + log::debug!("No {family} dict for {service_id_s}"); + None + })?; + let name = ip_dict + .find(unsafe { kSCPropInterfaceName }.to_void()) + .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) + .and_then(|s| s.downcast::()) + .map(|s| s.to_string()) + .or_else(|| { + log::debug!("Missing name for service {service_id_s} ({family})"); + None + })?; + let router_ip = ip_dict + .find(router_key) + .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) + .and_then(|s| s.downcast::()) + .and_then(|ip| ip.to_string().parse().ok()) + .or_else(|| { + log::debug!("Missing router IP for {service_id_s} ({name}, {family})"); + None + })?; + + Some(NetworkServiceDetails { name, router_ip }) + }) + .collect::>() + } } /// Return a map from interface name to link addresses (AF_LINK) @@ -92,82 +215,6 @@ pub fn get_interface_link_addresses() -> io::Result Vec { - let prefs = SCPreferences::default(&CFString::new("talpid-routing")); - let set = SCNetworkSet::new(&prefs); - let service_order = set.service_order(); - let store = SCDynamicStoreBuilder::new("talpid-routing").build(); - - let global_dict = if family == Family::V4 { - "State:/Network/Global/IPv4" - } else { - "State:/Network/Global/IPv6" - }; - let global_dict = store - .get(CFString::new(global_dict)) - .and_then(|v| v.downcast_into::()); - let primary_interface = if let Some(ref dict) = global_dict { - dict.find(unsafe { kSCDynamicStorePropNetPrimaryInterface }.to_void()) - .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) - .and_then(|s| s.downcast::()) - .map(|s| s.to_string()) - } else { - None - }; - - let router_key = if family == Family::V4 { - unsafe { kSCPropNetIPv4Router.to_void() } - } else { - unsafe { kSCPropNetIPv6Router.to_void() } - }; - - service_order - .iter() - .filter_map(|service_id| { - let service_id_s = service_id.to_string(); - let key = if family == Family::V4 { - format!("State:/Network/Service/{service_id_s}/IPv4") - } else { - format!("State:/Network/Service/{service_id_s}/IPv6") - }; - - let ip_dict = store - .get(CFString::new(&key)) - .and_then(|v| v.downcast_into::())?; - let name = ip_dict - .find(unsafe { kSCPropInterfaceName }.to_void()) - .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) - .and_then(|s| s.downcast::()) - .map(|s| s.to_string())?; - let router_ip = ip_dict - .find(router_key) - .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) - .and_then(|s| s.downcast::()) - .and_then(|ip| ip.to_string().parse().ok()) - .or_else(|| { - if Some(&name) != primary_interface.as_ref() { - return None; - } - let Some(ref dict) = global_dict else { - return None; - }; - // Sometimes only the primary interface contains the router IPv6 addr - dict.find(router_key) - .map(|s| unsafe { CFType::wrap_under_get_rule(*s) }) - .and_then(|s| s.downcast::()) - .and_then(|ip| ip.to_string().parse().ok()) - })?; - - Some(NetworkServiceDetails { name, router_ip }) - }) - .collect::>() -} - /// Return whether the given interface has an assigned (unicast) IP address. fn is_active_interface(interface_name: &str, family: Family) -> io::Result { let required_link_flags: InterfaceFlags = InterfaceFlags::IFF_UP | InterfaceFlags::IFF_RUNNING; diff --git a/talpid-routing/src/unix/macos/mod.rs b/talpid-routing/src/unix/macos/mod.rs index 24923ea9af99..f2726a38dc42 100644 --- a/talpid-routing/src/unix/macos/mod.rs +++ b/talpid-routing/src/unix/macos/mod.rs @@ -87,6 +87,8 @@ pub struct RouteManagerImpl { update_trigger: BurstGuard, default_route_listeners: Vec>, check_default_routes_restored: Pin + Send>>, + unhandled_default_route_changes: bool, + primary_interface_monitor: interface::PrimaryInterfaceMonitor, } impl RouteManagerImpl { @@ -95,6 +97,7 @@ impl RouteManagerImpl { pub(crate) async fn new( manage_tx: Weak>, ) -> Result { + let primary_interface_monitor = interface::PrimaryInterfaceMonitor::new(); let routing_table = RoutingTable::new().map_err(Error::RoutingTable)?; let update_trigger = BurstGuard::new( @@ -119,6 +122,8 @@ impl RouteManagerImpl { update_trigger, default_route_listeners: vec![], check_default_routes_restored: Box::pin(futures::stream::pending()), + unhandled_default_route_changes: false, + primary_interface_monitor, }) } @@ -183,7 +188,7 @@ impl RouteManagerImpl { device: None, ip: route.gateway_ip(), }, - prefix: IpNetwork::from(interface::Family::V4), + prefix: interface::Family::V4.default_network(), metric: None, } }); @@ -193,7 +198,7 @@ impl RouteManagerImpl { device: None, ip: route.gateway_ip(), }, - prefix: IpNetwork::from(interface::Family::V6), + prefix: interface::Family::V6.default_network(), metric: None, } }); @@ -206,11 +211,6 @@ impl RouteManagerImpl { log::debug!("Cancelling restoration of default routes"); self.check_default_routes_restored = Box::pin(futures::stream::pending()); } - - // Reset known best route - let _ = self.update_best_default_route(interface::Family::V4); - let _ = self.update_best_default_route(interface::Family::V6); - log::debug!("Adding routes: {routes:?}"); let _ = tx.send(self.add_required_routes(routes).await); } @@ -221,7 +221,7 @@ impl RouteManagerImpl { }, Some(RouteManagerCommand::RefreshRoutes) => { if let Err(error) = self.refresh_routes().await { - log::error!("Failed to refresh routes: {error}") + log::error!("Failed to refresh routes: {error}"); } }, None => { @@ -319,15 +319,23 @@ impl RouteManagerImpl { log::error!("Failed to process deleted route: {err}"); } } - if route.errno() == 0 && route.is_default().unwrap_or(true) { - self.update_trigger.trigger(); + if route.errno() != 0 { + return; } + if route.is_default().unwrap_or(true) { + self.unhandled_default_route_changes = true; + } + self.update_trigger.trigger(); } Ok(RouteSocketMessage::AddRoute(route)) | Ok(RouteSocketMessage::ChangeRoute(route)) => { - if route.errno() == 0 && route.is_default().unwrap_or(true) { - self.update_trigger.trigger(); + if route.errno() != 0 { + return; + } + if route.is_default().unwrap_or(true) { + self.unhandled_default_route_changes = true; } + self.update_trigger.trigger(); } Ok(RouteSocketMessage::AddAddress(_) | RouteSocketMessage::DeleteAddress(_)) => { self.update_trigger.trigger(); @@ -350,6 +358,10 @@ impl RouteManagerImpl { self.update_best_default_route(interface::Family::V4)?; self.update_best_default_route(interface::Family::V6)?; + if !self.unhandled_default_route_changes { + return Ok(()); + } + // Remove any existing ifscope route that we've added self.remove_applied_routes(|route| { route.is_ifscope() && route.is_default().unwrap_or(false) @@ -360,7 +372,11 @@ impl RouteManagerImpl { self.apply_tunnel_default_route().await?; // Update routes using default interface - self.apply_non_tunnel_routes().await + self.apply_non_tunnel_routes().await?; + + self.unhandled_default_route_changes = false; + + Ok(()) } /// Figure out what the best default routes to use are, and send updates to default route change @@ -372,7 +388,7 @@ impl RouteManagerImpl { /// /// On success, the function returns whether the previously known best default changed. fn update_best_default_route(&mut self, family: interface::Family) -> Result { - let best_route = interface::get_best_default_route(family); + let best_route = self.primary_interface_monitor.get_route(family); let current_route = get_current_best_default_route!(self, family); log::trace!("Best route ({family:?}): {best_route:?}"); @@ -380,13 +396,15 @@ impl RouteManagerImpl { return Ok(false); } + self.unhandled_default_route_changes = true; + let old_pair = current_route .as_ref() .map(|r| (r.interface_index(), r.gateway_ip())); let new_pair = best_route .as_ref() .map(|r| (r.interface_index(), r.gateway_ip())); - log::debug!("Best default route changed from {old_pair:?} to {new_pair:?}"); + log::debug!("Best default route ({family}) changed from {old_pair:?} to {new_pair:?}"); let _ = std::mem::replace(current_route, best_route); let changed = current_route.is_some(); @@ -444,7 +462,7 @@ impl RouteManagerImpl { self.replace_with_scoped_route(family).await?; // Make sure there is really no other unscoped default route - let mut msg = RouteMessage::new_route(IpNetwork::from(family).into()); + let mut msg = RouteMessage::new_route(family.default_network().into()); msg = msg.set_gateway_route(true); let old_route = self.routing_table.get_route(&msg).await; if let Ok(Some(old_route)) = old_route { @@ -610,16 +628,16 @@ impl RouteManagerImpl { /// Add back unscoped default route for the given `family`, if it is still missing. This /// function returns true when no route had to be added. async fn restore_default_route(&mut self, family: interface::Family) -> bool { - let Some(desired_default_route) = interface::get_best_default_route(family) else { + let Some(desired_default_route) = self.primary_interface_monitor.get_route(family) else { return true; }; - let current_default_route = RouteMessage::new_route(IpNetwork::from(family).into()); + let current_default_route = RouteMessage::new_route(family.default_network().into()); if let Ok(Some(current_default)) = self.routing_table.get_route(¤t_default_route).await { // We're done if the route we're looking for is already here - if route_matches_interface(Some(¤t_default), Some(&desired_default_route)) { + if route_matches_interface(¤t_default, &desired_default_route) { return true; } let _ = self @@ -638,16 +656,7 @@ impl RouteManagerImpl { } } -fn route_matches_interface( - default_route: Option<&RouteMessage>, - interface_route: Option<&RouteMessage>, -) -> bool { - match (default_route, interface_route) { - (Some(default_route), Some(interface_route)) => { - default_route.gateway_ip() == interface_route.gateway_ip() - && default_route.interface_index() == interface_route.interface_index() - } - (None, None) => true, - _ => false, - } +fn route_matches_interface(default_route: &RouteMessage, interface_route: &RouteMessage) -> bool { + default_route.gateway_ip() == interface_route.gateway_ip() + && default_route.interface_index() == interface_route.interface_index() } From 108cfc2c8c0ed3aa9664841377bbafcb1a0fd14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Mon, 9 Oct 2023 16:00:08 +0200 Subject: [PATCH 3/8] React to any network service change in dynamic store --- talpid-routing/src/unix/macos/interface.rs | 70 +++++++++++++++++++--- talpid-routing/src/unix/macos/mod.rs | 11 +++- 2 files changed, 70 insertions(+), 11 deletions(-) diff --git a/talpid-routing/src/unix/macos/interface.rs b/talpid-routing/src/unix/macos/interface.rs index 0df96ab6b377..1ed65aadea80 100644 --- a/talpid-routing/src/unix/macos/interface.rs +++ b/talpid-routing/src/unix/macos/interface.rs @@ -1,3 +1,4 @@ +use futures::channel::mpsc::{self, UnboundedReceiver, UnboundedSender}; use ipnetwork::IpNetwork; use nix::{ net::if_::{if_nametoindex, InterfaceFlags}, @@ -9,13 +10,16 @@ use std::{ net::{IpAddr, Ipv4Addr, Ipv6Addr}, }; +use super::data::{Destination, RouteMessage}; use system_configuration::{ core_foundation::{ + array::CFArray, base::{CFType, TCFType, ToVoid}, dictionary::CFDictionary, + runloop::{kCFRunLoopCommonModes, CFRunLoop}, string::CFString, }, - dynamic_store::{SCDynamicStore, SCDynamicStoreBuilder}, + dynamic_store::{SCDynamicStore, SCDynamicStoreBuilder, SCDynamicStoreCallBackContext}, network_configuration::SCNetworkSet, preferences::SCPreferences, sys::schema_definitions::{ @@ -24,7 +28,9 @@ use system_configuration::{ }, }; -use super::data::{Destination, RouteMessage}; +const STATE_IPV4_KEY: &str = "State:/Network/Global/IPv4"; +const STATE_IPV6_KEY: &str = "State:/Network/Global/IPv6"; +const STATE_SERVICE_PATTERN: &str = "State:/Network/Service/.*/IP.*"; #[derive(Debug, PartialEq, Clone, Copy)] pub enum Family { @@ -57,18 +63,64 @@ struct NetworkServiceDetails { pub struct PrimaryInterfaceMonitor { store: SCDynamicStore, - set: SCNetworkSet, + prefs: SCPreferences, } // FIXME: Implement Send on SCDynamicStore, if it's safe unsafe impl Send for PrimaryInterfaceMonitor {} +pub enum InterfaceEvent { + Update, +} + impl PrimaryInterfaceMonitor { - pub fn new() -> Self { + pub fn new() -> (Self, UnboundedReceiver) { let store = SCDynamicStoreBuilder::new("talpid-routing").build(); let prefs = SCPreferences::default(&CFString::new("talpid-routing")); - let set = SCNetworkSet::new(&prefs); - Self { store, set } + + let (tx, rx) = mpsc::unbounded(); + Self::start_listener(tx); + + (Self { store, prefs }, rx) + } + + fn start_listener(tx: UnboundedSender) { + std::thread::spawn(|| { + let listener_store = SCDynamicStoreBuilder::new("talpid-routing-listener") + .callback_context(SCDynamicStoreCallBackContext { + callout: Self::store_change_handler, + info: tx, + }) + .build(); + + let watch_keys: CFArray = CFArray::from_CFTypes(&[ + CFString::new(STATE_IPV4_KEY), + CFString::new(STATE_IPV6_KEY), + ]); + let watch_patterns = CFArray::from_CFTypes(&[CFString::new(STATE_SERVICE_PATTERN)]); + + if !listener_store.set_notification_keys(&watch_keys, &watch_patterns) { + log::error!("Failed to start interface listener"); + return; + } + + let run_loop_source = listener_store.create_run_loop_source(); + CFRunLoop::get_current().add_source(&run_loop_source, unsafe { kCFRunLoopCommonModes }); + CFRunLoop::run_current(); + + log::debug!("Interface listener exiting"); + }); + } + + fn store_change_handler( + _store: SCDynamicStore, + changed_keys: CFArray, + tx: &mut UnboundedSender, + ) { + for k in changed_keys.iter() { + log::debug!("Interface change, key {}", k.to_string()); + } + let _ = tx.unbounded_send(InterfaceEvent::Update); } /// Retrieve the best current default route. This is based on the primary interface, or else @@ -114,9 +166,9 @@ impl PrimaryInterfaceMonitor { fn get_primary_interface(&self, family: Family) -> Option { let global_name = if family == Family::V4 { - "State:/Network/Global/IPv4" + STATE_IPV4_KEY } else { - "State:/Network/Global/IPv6" + STATE_IPV6_KEY }; let global_dict = self .store @@ -158,7 +210,7 @@ impl PrimaryInterfaceMonitor { unsafe { kSCPropNetIPv6Router.to_void() } }; - self.set + SCNetworkSet::new(&self.prefs) .service_order() .iter() .filter_map(|service_id| { diff --git a/talpid-routing/src/unix/macos/mod.rs b/talpid-routing/src/unix/macos/mod.rs index f2726a38dc42..351265146d17 100644 --- a/talpid-routing/src/unix/macos/mod.rs +++ b/talpid-routing/src/unix/macos/mod.rs @@ -1,7 +1,7 @@ use crate::{debounce::BurstGuard, NetNode, Node, RequiredRoute, Route}; use futures::{ - channel::mpsc, + channel::mpsc::{self, UnboundedReceiver}, future::FutureExt, stream::{FusedStream, StreamExt}, }; @@ -89,6 +89,7 @@ pub struct RouteManagerImpl { check_default_routes_restored: Pin + Send>>, unhandled_default_route_changes: bool, primary_interface_monitor: interface::PrimaryInterfaceMonitor, + interface_change_rx: UnboundedReceiver, } impl RouteManagerImpl { @@ -97,7 +98,8 @@ impl RouteManagerImpl { pub(crate) async fn new( manage_tx: Weak>, ) -> Result { - let primary_interface_monitor = interface::PrimaryInterfaceMonitor::new(); + let (primary_interface_monitor, interface_change_rx) = + interface::PrimaryInterfaceMonitor::new(); let routing_table = RoutingTable::new().map_err(Error::RoutingTable)?; let update_trigger = BurstGuard::new( @@ -124,6 +126,7 @@ impl RouteManagerImpl { check_default_routes_restored: Box::pin(futures::stream::pending()), unhandled_default_route_changes: false, primary_interface_monitor, + interface_change_rx, }) } @@ -167,6 +170,10 @@ impl RouteManagerImpl { } } + _event = self.interface_change_rx.next() => { + self.update_trigger.trigger(); + } + command = manage_rx.next() => { match command { Some(RouteManagerCommand::Shutdown(tx)) => { From 864503f26701a0001f53204c9363265bd4c2df6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Mon, 9 Oct 2023 16:05:29 +0200 Subject: [PATCH 4/8] Print network services when offline --- talpid-routing/src/unix/macos/interface.rs | 14 ++++++++++++++ talpid-routing/src/unix/macos/mod.rs | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/talpid-routing/src/unix/macos/interface.rs b/talpid-routing/src/unix/macos/interface.rs index 1ed65aadea80..a6e0bed540a3 100644 --- a/talpid-routing/src/unix/macos/interface.rs +++ b/talpid-routing/src/unix/macos/interface.rs @@ -56,6 +56,7 @@ impl Family { } } +#[derive(Debug)] struct NetworkServiceDetails { name: String, router_ip: IpAddr, @@ -252,6 +253,19 @@ impl PrimaryInterfaceMonitor { }) .collect::>() } + + pub fn debug(&self) { + for family in [Family::V4, Family::V6] { + log::debug!( + "Primary interface ({family}): {:?}", + self.get_primary_interface(family) + ); + log::debug!( + "Network services ({family}): {:?}", + self.network_services(family) + ); + } + } } /// Return a map from interface name to link addresses (AF_LINK) diff --git a/talpid-routing/src/unix/macos/mod.rs b/talpid-routing/src/unix/macos/mod.rs index 351265146d17..5b694029397b 100644 --- a/talpid-routing/src/unix/macos/mod.rs +++ b/talpid-routing/src/unix/macos/mod.rs @@ -152,6 +152,8 @@ impl RouteManagerImpl { false }); + self.debug_offline(); + let mut completion_tx = None; loop { @@ -365,6 +367,8 @@ impl RouteManagerImpl { self.update_best_default_route(interface::Family::V4)?; self.update_best_default_route(interface::Family::V6)?; + self.debug_offline(); + if !self.unhandled_default_route_changes { return Ok(()); } @@ -386,6 +390,12 @@ impl RouteManagerImpl { Ok(()) } + fn debug_offline(&self) { + if self.v4_default_route.is_none() && self.v6_default_route.is_none() { + self.primary_interface_monitor.debug(); + } + } + /// Figure out what the best default routes to use are, and send updates to default route change /// subscribers. The "best routes" are used by the tunnel device to send packets to the VPN /// relay. From 806ec7e38ad7a778a7aeb70772fad3f3d83dc6ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Mon, 9 Oct 2023 16:40:16 +0200 Subject: [PATCH 5/8] Refresh routes when connecting in offline states --- talpid-core/src/tunnel_state_machine/connecting_state.rs | 6 ++++++ talpid-routing/src/unix/mod.rs | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs index 2bfc83e55872..28731d617d6d 100644 --- a/talpid-core/src/tunnel_state_machine/connecting_state.rs +++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs @@ -539,6 +539,12 @@ impl TunnelState for ConnectingState { retry_attempt: u32, ) -> (TunnelStateWrapper, TunnelStateTransition) { if shared_values.is_offline { + // FIXME: Temporary: Nudge route manager to update the default interface + #[cfg(target_os = "macos")] + if let Ok(handle) = shared_values.route_manager.handle() { + log::debug!("Poking route manager to update default routes"); + let _ = handle.refresh_routes(); + } return ErrorState::enter(shared_values, ErrorStateCause::IsOffline); } match shared_values.runtime.block_on( diff --git a/talpid-routing/src/unix/mod.rs b/talpid-routing/src/unix/mod.rs index 02dac8ac0fcd..863e3a3aac42 100644 --- a/talpid-routing/src/unix/mod.rs +++ b/talpid-routing/src/unix/mod.rs @@ -93,6 +93,14 @@ impl RouteManagerHandle { response_rx.await.map_err(|_| Error::ManagerChannelDown) } + /// Get current non-tunnel default routes. + #[cfg(target_os = "macos")] + pub fn refresh_routes(&self) -> Result<(), Error> { + self.tx + .unbounded_send(RouteManagerCommand::RefreshRoutes) + .map_err(|_| Error::RouteManagerDown) + } + /// Ensure that packets are routed using the correct tables. #[cfg(target_os = "linux")] pub async fn create_routing_rules(&self, enable_ipv6: bool) -> Result<(), Error> { From a2cef15e7b109b1b42fa54357b942485d65140ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Mon, 9 Oct 2023 22:51:22 +0200 Subject: [PATCH 6/8] Refuse to resolve IPv6 (panic) --- talpid-core/src/resolver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/talpid-core/src/resolver.rs b/talpid-core/src/resolver.rs index 19311a38ccea..ed703c86fdaa 100644 --- a/talpid-core/src/resolver.rs +++ b/talpid-core/src/resolver.rs @@ -30,7 +30,7 @@ use trust_dns_server::{ ServerFuture, }; -const ALLOWED_RECORD_TYPES: &[RecordType] = &[RecordType::A, RecordType::AAAA, RecordType::CNAME]; +const ALLOWED_RECORD_TYPES: &[RecordType] = &[RecordType::A, RecordType::CNAME]; const CAPTIVE_PORTAL_DOMAINS: &[&str] = &["captive.apple.com", "netcts.cdn-apple.com"]; static ALLOWED_DOMAINS: Lazy> = Lazy::new(|| { From 58dc6f10bcd2f8f5768d44eac2a79731243c19a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Mon, 9 Oct 2023 22:52:05 +0200 Subject: [PATCH 7/8] Synthesize offline state on macOS --- talpid-core/src/offline/macos.rs | 112 +++++++++++++++++++++---------- 1 file changed, 78 insertions(+), 34 deletions(-) diff --git a/talpid-core/src/offline/macos.rs b/talpid-core/src/offline/macos.rs index b679a52b226e..926d68918c46 100644 --- a/talpid-core/src/offline/macos.rs +++ b/talpid-core/src/offline/macos.rs @@ -3,10 +3,24 @@ //! user from connecting to a relay. //! //! See [RouteManagerHandle::default_route_listener]. -use futures::{channel::mpsc::UnboundedSender, StreamExt}; -use std::sync::{Arc, Mutex}; +//! +//! This offline monitor synthesizes an offline state between network switches and before coming +//! online from an offline state. This is done to work around issues with DNS being blocked due +//! to macOS's connectivity check. In the offline state, a DNS server on localhost prevents the +//! connectivity check from being blocked. +use futures::{ + channel::mpsc::UnboundedSender, + future::{Fuse, FutureExt}, + select, StreamExt, +}; +use std::{ + sync::{Arc, Mutex}, + time::Duration, +}; use talpid_routing::{DefaultRouteEvent, RouteManagerHandle}; +const SYNTHETIC_OFFLINE_DURATION: Duration = Duration::from_secs(1); + #[derive(err_derive::Error, Debug)] pub enum Error { #[error(display = "Failed to initialize route monitor")] @@ -18,6 +32,7 @@ pub struct MonitorHandle { _notify_tx: Arc>, } +#[derive(Clone)] struct ConnectivityState { v4_connectivity: bool, v6_connectivity: bool, @@ -45,7 +60,7 @@ pub async fn spawn_monitor( let notify_tx = Arc::new(notify_tx); // note: begin observing before initializing the state - let mut route_listener = route_manager_handle.default_route_listener().await?; + let route_listener = route_manager_handle.default_route_listener().await?; let (v4_connectivity, v6_connectivity) = match route_manager_handle.get_default_routes().await { Ok((v4_route, v6_route)) => (v4_route.is_some(), v6_route.is_some()), @@ -61,6 +76,8 @@ pub async fn spawn_monitor( v4_connectivity, v6_connectivity, }; + let mut real_state = state.clone(); + let state = Arc::new(Mutex::new(state)); let weak_state = Arc::downgrade(&state); @@ -68,43 +85,70 @@ pub async fn spawn_monitor( // Detect changes to the default route tokio::spawn(async move { - while let Some(event) = route_listener.next().await { - let Some(state) = weak_state.upgrade() else { - break; - }; - let mut state = state.lock().unwrap(); + let mut timeout = Fuse::terminated(); + let mut route_listener = route_listener.fuse(); + + loop { + select! { + _ = timeout => { + // Update shared state + let Some(state) = weak_state.upgrade() else { + break; + }; + let mut state = state.lock().unwrap(); + *state = real_state.clone(); + + if state.get_connectivity() { + log::info!("Connectivity changed: Connected"); + let Some(tx) = weak_notify_tx.upgrade() else { + break; + }; + let _ = tx.unbounded_send(false); + } + } - let previous_connectivity = state.get_connectivity(); + route_event = route_listener.next() => { + let Some(event) = route_event else { + break; + }; + + // Update real state + match event { + DefaultRouteEvent::AddedOrChangedV4 => { + real_state.v4_connectivity = true; + } + DefaultRouteEvent::AddedOrChangedV6 => { + real_state.v6_connectivity = true; + } + DefaultRouteEvent::RemovedV4 => { + real_state.v4_connectivity = false; + } + DefaultRouteEvent::RemovedV6 => { + real_state.v6_connectivity = false; + } + } - match event { - DefaultRouteEvent::AddedOrChangedV4 => { - state.v4_connectivity = true; - } - DefaultRouteEvent::AddedOrChangedV6 => { - state.v6_connectivity = true; - } - DefaultRouteEvent::RemovedV4 => { + // Synthesize offline state + // Update shared state + let Some(state) = weak_state.upgrade() else { + break; + }; + let mut state = state.lock().unwrap(); + let previous_connectivity = state.get_connectivity(); state.v4_connectivity = false; - } - DefaultRouteEvent::RemovedV6 => { state.v6_connectivity = false; - } - } - let new_connectivity = state.get_connectivity(); - if previous_connectivity != new_connectivity { - log::info!( - "Connectivity changed: {}", - if new_connectivity { - "Connected" - } else { - "Offline" + if previous_connectivity { + let Some(tx) = weak_notify_tx.upgrade() else { + break; + }; + let _ = tx.unbounded_send(true); + log::info!("Connectivity changed: Offline"); + } + if real_state.get_connectivity() { + timeout = Box::pin(tokio::time::sleep(SYNTHETIC_OFFLINE_DURATION)).fuse(); } - ); - let Some(tx) = weak_notify_tx.upgrade() else { - break; - }; - let _ = tx.unbounded_send(!new_connectivity); + } } } From 65b00bab8aafc7bbb2d11bab55ee96a8e762b3ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Mon, 9 Oct 2023 18:56:38 +0200 Subject: [PATCH 8/8] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaa69f91842e..92e4c7573530 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,9 @@ Line wrap the file at 100 chars. Th #### Windows - Correctly detect whether OS is Windows Server (primarily for logging in daemon.log). +#### macOS +- Fix connectivity issues when switching between networks or disconnecting. + ## [android/2023.6] - 2023-09-25 ### Fixed