mirror of
https://github.com/outbackdingo/firezone.git
synced 2026-01-27 10:18:54 +00:00
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.
This commit is contained in:
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,10 @@ export default function Apple() {
|
||||
<Entries downloadLinks={downloadLinks} title="macOS / iOS">
|
||||
{/* 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. */}
|
||||
<Unreleased>
|
||||
<ChangeItem pull="10075">
|
||||
Fixes an issue on iOS where the tunnel may never fully come up after
|
||||
signing in due to a network connectivity reset loop.
|
||||
</ChangeItem>
|
||||
<ChangeItem pull="10056">
|
||||
Fixes an issue where connectivity could be lost for up to 20 seconds
|
||||
after waking from sleep.
|
||||
|
||||
Reference in New Issue
Block a user