From cb061bf9bab169f75b6b342a0fbc4a744a91731e Mon Sep 17 00:00:00 2001
From: Jamil <jamilbk@users.noreply.github.com>
Date: Sat, 31 Aug 2024 13:49:54 -0700
Subject: [PATCH] fix(apple): Trigger connlib reset() when IPv4, IPv6, or
 available network gateways has changed (#6521)

On Apple, we try to be smart about triggering connlib's `reset()` in
order to keep from triggering endless update loops. This can happen
because connlib itself triggers path monitoring updates through
onUpdateRoutes and such.

Before, we only kept track of whether our primary interface changed in
order to consider the path updated. Now, we also track IPv4/IPv6
connectivity and the network's available gateways (read: routers) to
trigger changes. This fixes the case where our interface loses or gains
IPv4 / IPv6 connectivity, or the router address changes.

Fixes #6515

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
---
 .../FirezoneNetworkExtension/Adapter.swift    | 28 ++++++++++++-------
 website/src/components/Changelog/Apple.tsx    | 10 +++++++
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift
index 28eeb8287..91f22b46a 100644
--- a/swift/apple/FirezoneNetworkExtension/Adapter.swift
+++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift
@@ -52,8 +52,9 @@ class Adapter {
   /// Track our last fetched DNS resolvers to know whether to tell connlib they've updated
   private var lastFetchedResolvers: [String] = []
 
-  /// Used to avoid needlessly sending resets to connlib
-  private var primaryInterfaceName: String?
+  /// Remembers the last _relevant_ path update.
+  /// A path update is considered relevant if certain properties change that require us to reset connlib's network state.
+  private var lastRelevantPath: Network.NWPath?
 
   /// Private queue used to ensure consistent ordering among path update and connlib callbacks
   /// This is the primary async primitive used in this class.
@@ -281,24 +282,20 @@ extension Adapter {
     guard case .tunnelStarted(let session) = state else { return }
 
     if path.status == .unsatisfied {
-      Log.tunnel.log("\(#function): Detected network change: Offline.")
-
       // Check if we need to set reasserting, avoids OS log spam and potentially other side effects
       if packetTunnelProvider?.reasserting == false {
         // Tell the UI we're not connected
         packetTunnelProvider?.reasserting = true
       }
     } else {
-      Log.tunnel.log("\(#function): Detected network change: Online.")
-
-      // Hint to connlib we're back online, but only do so if our primary interface changes,
-      // and therefore we need to bump sockets. On darwin, this is needed to send packets
+      // Tell connlib to reset network state, but only do so if our connectivity has
+      // meaningfully changed. On darwin, this is needed to send packets
       // out of a different interface even when 0.0.0.0 is used as the source.
       // If our primary interface changes, we can be certain the old socket shouldn't be
       // used anymore.
-      if path.availableInterfaces.first?.name != primaryInterfaceName {
+      if lastRelevantPath?.connectivityDifferentFrom(path: path) != false {
+        lastRelevantPath = path
         session.reset()
-        primaryInterfaceName = path.availableInterfaces.first?.name
       }
 
       if shouldFetchSystemResolvers(path: path) {
@@ -492,3 +489,14 @@ extension Adapter: CallbackHandlerDelegate {
     }
   }
 #endif
+
+extension Network.NWPath {
+  func connectivityDifferentFrom(path: Network.NWPath) -> Bool {
+    // We define a path as different from another if the following properties change
+    return path.supportsIPv4 != self.supportsIPv4 ||
+      path.supportsIPv6 != self.supportsIPv6 ||
+      path.availableInterfaces.first?.name != self.availableInterfaces.first?.name ||
+      // Apple provides no documentation on whether order is meaningful, so assume it isn't.
+      Set(self.gateways) != Set(path.gateways)
+  }
+}
diff --git a/website/src/components/Changelog/Apple.tsx b/website/src/components/Changelog/Apple.tsx
index ff940c82f..99b167398 100644
--- a/website/src/components/Changelog/Apple.tsx
+++ b/website/src/components/Changelog/Apple.tsx
@@ -9,6 +9,16 @@ export default function Apple() {
       href="https://apps.apple.com/us/app/firezone/id6443661826"
       title="macOS / iOS"
     >
+      {/*
+      <Entry version="1.3.1" date={new Date("2024-08-31")}>
+        <ul className="list-disc space-y-2 pl-4 mb-4">
+          <ChangeItem pull="6521">
+            Gracefully handles cases where the device's local interface IPv4/IPv6 address or
+            local network gateway changes while the client is connected.
+          </ChangeItem>
+        </ul>
+      </Entry>
+      */}
       <Entry version="1.3.0" date={new Date("2024-08-30")}>
         <ul className="list-disc space-y-2 pl-4 mb-4">
           <ChangeItem pull="6434">