From 67b4dd86eae2c2b5f4dcf95515e529890370c69f Mon Sep 17 00:00:00 2001 From: Jamil Date: Thu, 24 Jul 2025 18:35:01 -0400 Subject: [PATCH] fix(apple): increase sensitivity of network reset (#9993) On Apple platforms, we tried to be clever about filtering path updates from the network connectivity change monitor, because there can be a flurry of them upon waking from sleep or network roaming. However, because of this, we had a bug that could occur in certain situations (such as waking from sleep) where we could effectively "land" on an empty DNS resolver list. This could happen if: 1. We receive a path update handler that meaningfully changes connectivity, but its `supportsDNS` property is `false`. This means it hasn't received any resolvers from DHCP yet. We would then setDns with an empty resolver list. 2. We then receive a path update handler with the _only_ change being `supportDNS=true`. Since we didn't count this change as a meaningful path change, we skipped the `setDns` call, and connlib would be stuck without DNS resolution. To fix the above, we stop trying to be clever about connectivity changes, and just use `oldPath != path`. That will increase reset a bit, but it will now handle other edge cases such as an IP address changing on the primary interface, any other interfaces change, and the like. Fixes #9866 --- .../FirezoneNetworkExtension/Adapter.swift | 36 +++++++++---------- website/src/components/Changelog/Apple.tsx | 4 +++ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index 2ba653118..b051b9914 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -68,9 +68,6 @@ class Adapter { private let systemConfigurationResolvers = SystemConfigurationResolvers() #endif - /// Track our last fetched DNS resolvers to know whether to tell connlib they've updated - private var lastFetchedResolvers: [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. @@ -127,6 +124,10 @@ class Adapter { self.packetTunnelProvider?.reasserting = true } } else { + if self.packetTunnelProvider?.reasserting == true { + self.packetTunnelProvider?.reasserting = false + } + // 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. @@ -142,27 +143,21 @@ class Adapter { let resolvers = getSystemDefaultResolvers( interfaceName: path.availableInterfaces.first?.name) - if self.lastFetchedResolvers != resolvers, - let encoded = try? JSONEncoder().encode(resolvers), - let jsonResolvers = String(data: encoded, encoding: .utf8)?.intoRustString() - { - - do { - try session?.setDns(jsonResolvers) - } catch let error { - // `toString` needed to deep copy the string and avoid a possible dangling pointer - let msg = (error as? RustString)?.toString() ?? "Unknown error" - Log.error(AdapterError.setDnsError(msg)) + do { + let encoded = try JSONEncoder().encode(resolvers) + guard let jsonResolvers = String(data: encoded, encoding: .utf8) + else { + Log.warning("jsonResolvers conversion failed: \(resolvers)") + return } - // Update our state tracker - self.lastFetchedResolvers = resolvers + try session?.setDns(jsonResolvers.intoRustString()) + } catch let error { + // `toString` needed to deep copy the string and avoid a possible dangling pointer + let msg = (error as? RustString)?.toString() ?? "Unknown error" + Log.error(AdapterError.setDnsError(msg)) } } - - if self.packetTunnelProvider?.reasserting == true { - self.packetTunnelProvider?.reasserting = false - } } } @@ -525,6 +520,7 @@ 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.supportsDNS != self.supportsDNS || 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 67732314f..cb4e89660 100644 --- a/website/src/components/Changelog/Apple.tsx +++ b/website/src/components/Changelog/Apple.tsx @@ -34,6 +34,10 @@ export default function Apple() { flaky connections, requiring signing out and signin back in to recover. + + Fixes an issue where DNS resolvers could be lost upon waking from + sleep, leading to broken Internet connectivity. + Fixes an issue where connections would sometimes take up to 90s to establish.