mirror of
https://github.com/outbackdingo/firezone.git
synced 2026-01-27 10:18:54 +00:00
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
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -34,6 +34,10 @@ export default function Apple() {
|
||||
flaky connections, requiring signing out and signin back in to
|
||||
recover.
|
||||
</ChangeItem>
|
||||
<ChangeItem pull="9993">
|
||||
Fixes an issue where DNS resolvers could be lost upon waking from
|
||||
sleep, leading to broken Internet connectivity.
|
||||
</ChangeItem>
|
||||
<ChangeItem pull="9891">
|
||||
Fixes an issue where connections would sometimes take up to 90s to
|
||||
establish.
|
||||
|
||||
Reference in New Issue
Block a user