From 290b0c298f7d3345d3376f6b20c362825298019f Mon Sep 17 00:00:00 2001 From: Jamil Date: Fri, 1 Aug 2025 02:20:24 -0400 Subject: [PATCH] fix(apple/ios): less aggressive setDns to avoid update loops (#10075) In https://github.com/firezone/firezone/pull/10022/files#diff-a84e8f62a17ac67f781019e6ac0456567fd18ffa7c13b3248609d78debb6480eL342 we removed the path connectivity filter that prevented path update loops on iOS. This was done to try and respond more aggressively to path updates in order to set system DNS resolvers, because we can't glean from the path's instance properties that any DNS configuration has changed - we simply have to assume so. Unfortunately, that caused an issue where we now enter a path update loop and effectively never fully bring the tunnel up. I've spent lots of time looking for a reliable work around that would allow us to both, (1) respond to path updates for DNS configuration changes on the system (we have to blindly react to these), and (2) avoid path update loops, but alas, without a significant time investment, there doesn't seem to be a way. So, we only set system resolvers on iOS in the path update handler if there was _also_ a detectable connectivity change, and settle on the assumption that **most** DNS configuration changes will be accompanied by a network connectivity change as well. --- .../FirezoneNetworkExtension/Adapter.swift | 32 ++++++++++++------- website/src/components/Changelog/Apple.tsx | 4 +++ 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index 1e49b15f2..b15237a46 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -128,18 +128,17 @@ class Adapter { self.packetTunnelProvider?.reasserting = false } - if lastPath?.connectivityDifferentFrom(path: path) != false { + if path.connectivityDifferentFrom(path: lastPath) { // Tell connlib to reset network state and DNS resolvers, 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. - reset(reason: "primary network path changed", path: path) - } else { - // Reset only resolvers - setSystemDefaultResolvers(path) + session?.reset("primary network path changed") } + setSystemDefaultResolvers(path) + lastPath = path } } @@ -419,6 +418,17 @@ extension Adapter: CallbackHandlerDelegate { let resolvers = self.systemConfigurationResolvers.getDefaultDNSServers( interfaceName: path.availableInterfaces.first?.name) #elseif os(iOS) + + // DNS server updates don't necessarily trigger a connectivity change, but we'll get a path update callback + // nevertheless. Unfortunately there's no visible difference in instance properties between the two path + // objects. On macOS this isn't an issue because setting new resolvers here doesn't trigger a change. + // On iOS, however, we need to prevent path update loops by not reacting to path updates that we ourselves + // triggered by the network settings apply. + + // TODO: Find a hackier hack to avoid this on iOS + if !path.connectivityDifferentFrom(path: lastPath) { + return + } let resolvers = resetToSystemDNSGettingBindResolvers() #endif @@ -515,13 +525,13 @@ extension Adapter: CallbackHandlerDelegate { #endif extension Network.NWPath { - func connectivityDifferentFrom(path: Network.NWPath) -> Bool { + func connectivityDifferentFrom(path: Network.NWPath? = nil) -> Bool { // We define a path as different from another if the following properties change - return path.supportsIPv4 != self.supportsIPv4 || path.supportsIPv6 != self.supportsIPv6 - || path.supportsDNS != self.supportsDNS - || path.status != self.status - || path.availableInterfaces.first != self.availableInterfaces.first - || path.gateways != self.gateways + return path?.supportsIPv4 != self.supportsIPv4 || path?.supportsIPv6 != self.supportsIPv6 + || path?.supportsDNS != self.supportsDNS + || path?.status != self.status + || path?.availableInterfaces.first != self.availableInterfaces.first + || path?.gateways != self.gateways } } diff --git a/website/src/components/Changelog/Apple.tsx b/website/src/components/Changelog/Apple.tsx index 479da630f..10dd1664b 100644 --- a/website/src/components/Changelog/Apple.tsx +++ b/website/src/components/Changelog/Apple.tsx @@ -25,6 +25,10 @@ export default function Apple() { {/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */} + + Fixes an issue on iOS where the tunnel may never fully come up after + signing in due to a network connectivity reset loop. + Fixes an issue where connectivity could be lost for up to 20 seconds after waking from sleep.