From 45466e3b78a8f5a80f333651b7669e8e57779190 Mon Sep 17 00:00:00 2001 From: Jamil Date: Sat, 25 Jan 2025 07:01:00 -0800 Subject: [PATCH] fix(apple): Ensure Adapter state is `started` in queue (#7860) When processing the items in the Adapter's workQueue, it's possible the Adapter has stopped by the time the queued closure begins execution. This can possibly lead to obscure failures if for example we're trying to apply network settings to a disconnected tunnel. Supersedes: #7858 --- .../FirezoneNetworkExtension/Adapter.swift | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index c12f98f3e..ee99f8c49 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -200,6 +200,11 @@ class Adapter { // This is async to avoid blocking the main UI thread workQueue.async { [weak self] in guard let self = self else { return } + guard case .tunnelStarted(let _session) = self.state + else { + Log.debug("\(#function): Invalid state \(self.state)") + return + } if hash == Data(SHA256.hash(data: Data((resourceListJSON ?? "").utf8))) { // nothing changed @@ -220,6 +225,12 @@ class Adapter { public func setInternetResourceEnabled(_ enabled: Bool) { workQueue.async { [weak self] in guard let self = self else { return } + guard case .tunnelStarted(let _session) = self.state + else { + Log.debug("\(#function): Invalid state \(self.state)") + return + } + self.internetResourceEnabled = enabled self.resourcesUpdated() } @@ -364,32 +375,29 @@ extension Adapter: CallbackHandlerDelegate { // This is a queued callback to ensure ordering workQueue.async { [weak self] in guard let self = self else { return } - if networkSettings == nil { - // First time receiving this callback, so initialize our network settings - networkSettings = NetworkSettings( - packetTunnelProvider: packetTunnelProvider) + guard case .tunnelStarted(let _session) = self.state + else { + Log.debug("\(#function): Invalid state \(self.state)") + return } + let networkSettings = self.networkSettings + ?? NetworkSettings(packetTunnelProvider: packetTunnelProvider) + Log.log( "\(#function): \(tunnelAddressIPv4) \(tunnelAddressIPv6) \(dnsAddresses) \(routeListv4) \(routeListv6)") - switch state { - case .tunnelStarted(session: _): - guard let networkSettings = networkSettings else { return } - networkSettings.tunnelAddressIPv4 = tunnelAddressIPv4 - networkSettings.tunnelAddressIPv6 = tunnelAddressIPv6 - networkSettings.dnsAddresses = dnsAddresses - networkSettings.routes4 = try! JSONDecoder().decode( - [NetworkSettings.Cidr].self, from: routeListv4.data(using: .utf8)! - ).compactMap { $0.asNEIPv4Route } - networkSettings.routes6 = try! JSONDecoder().decode( - [NetworkSettings.Cidr].self, from: routeListv6.data(using: .utf8)! - ).compactMap { $0.asNEIPv6Route } + networkSettings.tunnelAddressIPv4 = tunnelAddressIPv4 + networkSettings.tunnelAddressIPv6 = tunnelAddressIPv6 + networkSettings.dnsAddresses = dnsAddresses + networkSettings.routes4 = try! JSONDecoder().decode( + [NetworkSettings.Cidr].self, from: routeListv4.data(using: .utf8)! + ).compactMap { $0.asNEIPv4Route } + networkSettings.routes6 = try! JSONDecoder().decode( + [NetworkSettings.Cidr].self, from: routeListv6.data(using: .utf8)! + ).compactMap { $0.asNEIPv6Route } - networkSettings.apply() - case .tunnelStopped: - Log.error(AdapterError.invalidState(self.state)) - } + networkSettings.apply() } } @@ -397,6 +405,11 @@ extension Adapter: CallbackHandlerDelegate { // This is a queued callback to ensure ordering workQueue.async { [weak self] in guard let self = self else { return } + guard case .tunnelStarted(let _session) = self.state + else { + Log.debug("Tried to call \(#function) while state is \(self.state)") + return + } Log.log("\(#function)") @@ -412,6 +425,11 @@ extension Adapter: CallbackHandlerDelegate { // to ensure that we can clean up even if connlib exits before we are done. workQueue.async { [weak self] in guard let self = self else { return } + guard case .tunnelStarted(let _session) = self.state + else { + Log.debug("\(#function): Invalid state \(self.state)") + return + } Log.log("\(#function)") // Set a default stop reason. In the future, we may have more to act upon in