Create DynamicStore on startup in macOS route monitor

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 85b76377d4..0df96ab6b3 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<Family> 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<RouteMessage> {
-    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,
+}
+
+// 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<RouteMessage> {
+        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)
+            });
 
-        // Request ifscoped default route for this interface
-        let msg = RouteMessage::new_route(Destination::Network(IpNetwork::from(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<NetworkServiceDetails> {
+        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::<CFDictionary>())?;
+        let name = global_dict
+            .find(unsafe { kSCDynamicStorePropNetPrimaryInterface }.to_void())
+            .map(|s| unsafe { CFType::wrap_under_get_rule(*s) })
+            .and_then(|s| s.downcast::<CFString>())
+            .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::<CFString>())
+            .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<NetworkServiceDetails> {
+        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::<CFDictionary>())
+                    .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::<CFString>())
+                    .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::<CFString>())
+                    .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::<Vec<_>>()
+    }
 }
 
 /// Return a map from interface name to link addresses (AF_LINK)
@@ -92,82 +215,6 @@ pub fn get_interface_link_addresses() -> io::Result<BTreeMap<String, SockaddrSto
     Ok(gateway_link_addrs)
 }
 
-struct NetworkServiceDetails {
-    name: String,
-    router_ip: IpAddr,
-}
-
-fn network_service_order(family: Family) -> Vec<NetworkServiceDetails> {
-    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::<CFDictionary>());
-    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::<CFString>())
-            .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::<CFDictionary>())?;
-            let name = ip_dict
-                .find(unsafe { kSCPropInterfaceName }.to_void())
-                .map(|s| unsafe { CFType::wrap_under_get_rule(*s) })
-                .and_then(|s| s.downcast::<CFString>())
-                .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::<CFString>())
-                .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::<CFString>())
-                        .and_then(|ip| ip.to_string().parse().ok())
-                })?;
-
-            Some(NetworkServiceDetails { name, router_ip })
-        })
-        .collect::<Vec<_>>()
-}
-
 /// Return whether the given interface has an assigned (unicast) IP address.
 fn is_active_interface(interface_name: &str, family: Family) -> io::Result<bool> {
     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 24923ea9af..f2726a38dc 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<mpsc::UnboundedSender<DefaultRouteEvent>>,
     check_default_routes_restored: Pin<Box<dyn FusedStream<Item = ()> + 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<mpsc::UnboundedSender<RouteManagerCommand>>,
     ) -> Result<Self> {
+        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<bool> {
-        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(&current_default_route).await
         {
             // We're done if the route we're looking for is already here
-            if route_matches_interface(Some(&current_default), Some(&desired_default_route)) {
+            if route_matches_interface(&current_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()
 }