From bdffa3a697d026bcfa6313e9982b4d0243f4ebe3 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Fri, 14 Nov 2025 08:31:22 +1030 Subject: [PATCH] fix(apple): prevent utun increments from IPC calls (#10855) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macOS, IPC calls to the network extension can wake it whilst not connected, causing the system to create a utun device. If startTunnel() is not subsequently called, these devices persist and accumulate over time. The existing dryStartStopCycle() mechanism was introduced to wake the extension after upgrades, but other IPC operations (log management functions) could also wake the extension without proper cleanup. Solution -------- Add wrapper functions in IPCClient that automatically handle wake-up and cleanup lifecycle for IPC calls made whilst disconnected: - Check VPN connection status - If connected: execute IPC operation directly (utun already exists) - If disconnected: wake extension → wait 500ms → execute IPC → cleanup Implementation -------------- For async IPC operations (clearLogs, getLogFolderSize): Created free functions in IPCClient that wrap low-level IPC calls with wrapIPCCallIfNeeded(): - clearLogsWithCleanup(store:session:) - getLogFolderSizeWithCleanup(store:session:) For callback-based exportLogs: We cannot use wrapper because exportLogs returns immediately and uses callbacks for streaming chunks. Wrapper would call stop() before export finishes, killing the extension mid-stream. Solution: Manual wake-up/cleanup in LogExporter where we already have continuation that waits for chunk.done signal: 1. Check if extension needs waking (vpnStatus != .connected) 2. If yes: wake extension, wait 500ms 3. Start export with callbacks 4. When chunk.done=true: cleanup utun device, then resume continuation 5. On error: cleanup utun device, then resume with error Fixes #10580 --------- Co-authored-by: Jamil Bou Kheir --- .../FirezoneKit/Helpers/IPCClient.swift | 239 ++++++++---------- .../FirezoneKit/Helpers/LogExporter.swift | 27 +- .../Managers/VPNConfigurationManager.swift | 2 +- .../FirezoneKit/Models/Configuration.swift | 8 + .../Sources/FirezoneKit/Stores/Store.swift | 67 ++--- .../PacketTunnelProvider.swift | 23 +- website/src/components/Changelog/Apple.tsx | 4 + 7 files changed, 157 insertions(+), 213 deletions(-) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift index 31fd01c78..3f5d76085 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift @@ -28,148 +28,116 @@ enum IPCClient { } // Encoder used to send messages to the tunnel - static let encoder = PropertyListEncoder() - static let decoder = PropertyListDecoder() + private static let encoder = PropertyListEncoder() + private static let decoder = PropertyListDecoder() // Auto-connect @MainActor - static func start(session: NETunnelProviderSession, configuration: Configuration) throws { - let tunnelConfiguration = configuration.toTunnelConfiguration() - let configData = try encoder.encode(tunnelConfiguration) - let options: [String: NSObject] = [ - "configuration": configData as NSObject - ] - try validateSession(session).startTunnel(options: options) + static func start(session: NETunnelProviderSession) throws { + try session.startTunnel() } // Sign in @MainActor - static func start(session: NETunnelProviderSession, token: String, configuration: Configuration) - throws - { - let tunnelConfiguration = configuration.toTunnelConfiguration() - let configData = try encoder.encode(tunnelConfiguration) + static func start(session: NETunnelProviderSession, token: String) throws { let options: [String: NSObject] = [ - "token": token as NSObject, - "configuration": configData as NSObject, + "token": token as NSObject ] - try validateSession(session).startTunnel(options: options) + try session.startTunnel(options: options) } + @MainActor static func signOut(session: NETunnelProviderSession) async throws { - try await sendMessageWithoutResponse(session: session, message: ProviderMessage.signOut) - try stop(session: session) + let message = ProviderMessage.signOut + let _ = try await sendProviderMessage(session: session, message: message) + + session.stopTunnel() } - static func stop(session: NETunnelProviderSession) throws { - try validateSession(session).stopTunnel() + @MainActor + static func fetchResources( + session: NETunnelProviderSession, currentHash: Data + ) async throws -> Data? { + let message = ProviderMessage.getResourceList(currentHash) + + // Get data from the provider - if hash matches, provider returns nil + return try await sendProviderMessage(session: session, message: message) } - #if os(macOS) - // On macOS, IPC calls to the system extension won't work after it's been upgraded, until the startTunnel call. - // Since we rely on IPC for the GUI to function, we need to send a dummy `startTunnel` that doesn't actually - // start the tunnel, but causes the system to wake the extension. - @MainActor - static func dryStartStopCycle(session: NETunnelProviderSession, configuration: Configuration) - throws - { - let tunnelConfiguration = configuration.toTunnelConfiguration() - let configData = try encoder.encode(tunnelConfiguration) - let options: [String: NSObject] = [ - "dryRun": true as NSObject, - "configuration": configData as NSObject, - ] - try validateSession(session).startTunnel(options: options) - } - #endif - - static func setConfiguration(session: NETunnelProviderSession, _ configuration: Configuration) - async throws - { - let tunnelConfiguration = await configuration.toTunnelConfiguration() - let message = ProviderMessage.setConfiguration(tunnelConfiguration) - - if session.status != .connected { - Log.trace("Not setting configuration whilst not connected") - return - } - try await sendMessageWithoutResponse(session: session, message: message) + @MainActor + static func setConfiguration( + session: NETunnelProviderSession, _ configuration: TunnelConfiguration + ) async throws { + let message = ProviderMessage.setConfiguration(configuration) + let _ = try await sendProviderMessage(session: session, message: message) } + + // MARK: - Low-level IPC operations + + @MainActor static func clearLogs(session: NETunnelProviderSession) async throws { - try await sendMessageWithoutResponse(session: session, message: ProviderMessage.clearLogs) + let message = ProviderMessage.clearLogs + let _ = try await sendProviderMessage(session: session, message: message) } + @MainActor static func getLogFolderSize(session: NETunnelProviderSession) async throws -> Int64 { - return try await withCheckedThrowingContinuation { continuation in + let message = ProviderMessage.getLogFolderSize + guard let data = try await sendProviderMessage(session: session, message: message) + else { + throw Error.noIPCData + } - do { - try validateSession(session).sendProviderMessage( - encoder.encode(ProviderMessage.getLogFolderSize) - ) { data in - - guard let data = data - else { - continuation - .resume(throwing: Error.noIPCData) - - return - } - data.withUnsafeBytes { rawBuffer in - continuation.resume(returning: rawBuffer.load(as: Int64.self)) - } - } - } catch { - continuation.resume(throwing: error) - } + return data.withUnsafeBytes { rawBuffer in + rawBuffer.load(as: Int64.self) } } - // Call this with a closure that will append each chunk to a buffer - // of some sort, like a file. The completed buffer is a valid Apple Archive - // in AAR format. - static func exportLogs( - session: NETunnelProviderSession, - appender: @escaping (LogChunk) -> Void, - errorHandler: @escaping (Error) -> Void - ) { - func loop() { - do { - try validateSession(session).sendProviderMessage( - encoder.encode(ProviderMessage.exportLogs) - ) { data in - guard let data = data - else { - errorHandler(Error.noIPCData) + @MainActor + static func exportLogs(session: NETunnelProviderSession, fileHandle: FileHandle) async throws { + let isCycleStart = try await maybeCycleStart(session) + defer { + if isCycleStart { session.stopTunnel() } + } - return - } - - guard - let chunk = try? decoder.decode( - LogChunk.self, from: data - ) - else { - errorHandler(Error.decodeIPCDataFailed) - - return - } - - appender(chunk) - - if !chunk.done { - // Continue - loop() + let message = ProviderMessage.exportLogs + let encodedMessage = try encoder.encode(message) + + func loop() async throws { + try await withCheckedThrowingContinuation { + (continuation: CheckedContinuation) in + do { + try session.sendProviderMessage(encodedMessage) { data in + guard let data = data else { + return continuation.resume(throwing: Error.noIPCData) + } + guard let chunk = try? decoder.decode(LogChunk.self, from: data) else { + return continuation.resume(throwing: Error.decodeIPCDataFailed) + } + + do { + try fileHandle.seekToEnd() + fileHandle.write(chunk.data) + + continuation.resume() + + if !chunk.done { + Task { try await loop() } + } + } catch { + return continuation.resume(throwing: error) + } } + } catch { + continuation.resume(throwing: error) } - } catch { - Log.error(error) } } // Start exporting - loop() + try await loop() } // Subscribe to system notifications about our VPN status changing @@ -195,33 +163,50 @@ enum IPCClient { } } - static func sessionStatus(session: NETunnelProviderSession) -> NEVPNStatus { - return session.status - } + private static func sendProviderMessage( + session: NETunnelProviderSession, + message: ProviderMessage, + ) async throws -> Data? { + let isCycleStart = try await maybeCycleStart(session) - private static func validateSession( - _ session: NETunnelProviderSession, - requiredStatuses: Set = [] - ) throws -> NETunnelProviderSession { - if requiredStatuses.isEmpty || requiredStatuses.contains(session.status) { - return session + defer { + if isCycleStart { session.stopTunnel() } } - throw Error.invalidStatus(session.status) - } - - private static func sendMessageWithoutResponse( - session: NETunnelProviderSession, - message: ProviderMessage - ) async throws { - try await withCheckedThrowingContinuation { continuation in + return try await withCheckedThrowingContinuation { continuation in do { - try validateSession(session).sendProviderMessage(encoder.encode(message)) { _ in - continuation.resume() + try session.sendProviderMessage(encoder.encode(message)) { data in + continuation.resume(returning: data) } } catch { continuation.resume(throwing: error) } } } + + /// On macOS, the tunnel needs to be in a connected, connecting, or reasserting state for the utun to be removed + /// upon stopTunnel. We do this by ensuring the tunnel is "started" prior to any IPC call. If so, we return true + /// so that the caller may stop the tunnel afterwards. + private static func maybeCycleStart(_ session: NETunnelProviderSession) async throws -> Bool { + if session.status == .invalid { + throw Error.invalidStatus(session.status) + } + + #if os(macOS) + if [.disconnected, .disconnecting].contains(session.status) { + let options: [String: NSObject] = [ + "cycleStart": true as NSObject + ] + + try session.startTunnel(options: options) + + // Give the system some time to start the tunnel (100ms) + try await Task.sleep(nanoseconds: 100_000_000) + + return true + } + #endif + + return false + } } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift index 868c25fa3..c0a2c78f8 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift @@ -52,33 +52,10 @@ import System .appendingPathComponent("tunnel.zip") fileManager.createFile(atPath: tunnelLogURL.path, contents: nil) let fileHandle = try FileHandle(forWritingTo: tunnelLogURL) + defer { try? fileHandle.close() } // 3. Await tunnel log export from tunnel process - try await withCheckedThrowingContinuation { continuation in - IPCClient.exportLogs( - session: session, - appender: { chunk in - do { - // Append each chunk to the archive - try fileHandle.seekToEnd() - fileHandle.write(chunk.data) - - if chunk.done { - try fileHandle.close() - continuation.resume() - } - - } catch { - try? fileHandle.close() - - continuation.resume(throwing: error) - } - }, - errorHandler: { error in - continuation.resume(throwing: error) - } - ) - } + try await IPCClient.exportLogs(session: session, fileHandle: fileHandle) // 4. Create app log archive let appLogURL = sharedLogFolderURL.appendingPathComponent("app.zip") diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift index 3dd2f806b..a83052c07 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift @@ -130,7 +130,7 @@ public class VPNConfigurationManager: @unchecked Sendable { configuration.internetResourceEnabled = internetResourceEnabled == "true" } - try await IPCClient.setConfiguration(session: session, configuration) + try await IPCClient.setConfiguration(session: session, configuration.toTunnelConfiguration()) // Remove fields to prevent confusion if the user sees these in System Settings and wonders why they're stale. if let protocolConfiguration = manager.protocolConfiguration as? NETunnelProviderProtocol { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift index 6ea0347ef..b5f429022 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift @@ -203,3 +203,11 @@ public struct TunnelConfiguration: Codable, Sendable { self.internetResourceEnabled = internetResourceEnabled } } + +extension TunnelConfiguration: Equatable { + public static func == (lhs: TunnelConfiguration, rhs: TunnelConfiguration) -> Bool { + return lhs.apiURL == rhs.apiURL && lhs.accountSlug == rhs.accountSlug + && lhs.logFilter == rhs.logFilter + && lhs.internetResourceEnabled == rhs.internetResourceEnabled + } +} diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index 5c0644094..e1ef2796c 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -44,6 +44,7 @@ public final class Store: ObservableObject { private var resourcesTimer: Timer? private var resourceUpdateTask: Task? public let configuration: Configuration + private var lastSavedConfiguration: TunnelConfiguration? private var vpnConfigurationManager: VPNConfigurationManager? private var cancellables: Set = [] @@ -69,15 +70,26 @@ public final class Store: ObservableObject { .debounce(for: .seconds(0.3), scheduler: DispatchQueue.main) // These happen quite frequently .sink(receiveValue: { [weak self] _ in guard let self = self else { return } + let current = self.configuration.toTunnelConfiguration() - if self.vpnConfigurationManager != nil { - Task { - do { - guard let session = try self.manager().session() else { return } - try await IPCClient.setConfiguration(session: session, self.configuration) - } catch { - Log.error(error) - } + if self.vpnConfigurationManager == nil { + // No manager yet, nothing to update + return + } + + if self.lastSavedConfiguration == current { + // No changes + return + } + + self.lastSavedConfiguration = current + + Task { + do { + guard let session = try self.manager().session() else { return } + try await IPCClient.setConfiguration(session: session, current) + } catch { + Log.error(error) } } }) @@ -126,7 +138,7 @@ public final class Store: ObservableObject { IPCClient.subscribeToVPNStatusUpdates(session: session, handler: vpnStatusChangeHandler) - let initialStatus = IPCClient.sessionStatus(session: session) + let initialStatus = session.status // Handle initial status to ensure resources start loading if already connected try await handleVPNStatusChange(newVPNStatus: initialStatus) @@ -210,7 +222,7 @@ public final class Store: ObservableObject { guard let session = try manager().session() else { throw VPNConfigurationManagerError.managerNotInitialized } - try IPCClient.start(session: session, configuration: configuration) + try IPCClient.start(session: session) } } func installVPNConfiguration() async throws { @@ -238,17 +250,7 @@ public final class Store: ObservableObject { throw VPNConfigurationManagerError.managerNotInitialized } - #if os(macOS) - // On macOS, the system removes the utun interface on stop ONLY if the VPN is in a connected state. - // So we need to do a dry run start-then-stop if we're not connected, to ensure the interface is removed. - if vpnStatus == .connected || vpnStatus == .connecting || vpnStatus == .reasserting { - try IPCClient.stop(session: session) - } else { - try IPCClient.dryStartStopCycle(session: session, configuration: configuration) - } - #else - try IPCClient.stop(session: session) - #endif + session.stopTunnel() } func signIn(authResponse: AuthResponse) async throws { @@ -272,7 +274,7 @@ public final class Store: ObservableObject { guard let session = try manager().session() else { throw VPNConfigurationManagerError.managerNotInitialized } - try IPCClient.start(session: session, token: authResponse.token, configuration: configuration) + try IPCClient.start(session: session, token: authResponse.token) } func signOut() async throws { @@ -362,26 +364,9 @@ public final class Store: ObservableObject { // Capture current hash before IPC call let currentHash = resourceListHash - // Get data from the provider - if hash matches, provider returns nil - let data = try await withCheckedThrowingContinuation { - (continuation: CheckedContinuation) in - do { - guard session.status == .connected else { - throw IPCClient.Error.invalidStatus(session.status) - } - - try session.sendProviderMessage( - IPCClient.encoder.encode(ProviderMessage.getResourceList(currentHash)) - ) { data in - continuation.resume(returning: data) - } - } catch { - continuation.resume(throwing: error) - } - } - // If no data returned, resources haven't changed - no update needed - guard let data = data else { + guard let data = try await IPCClient.fetchResources(session: session, currentHash: currentHash) + else { return } diff --git a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift index 61155e3ff..b8d625353 100644 --- a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift +++ b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift @@ -13,7 +13,6 @@ enum PacketTunnelProviderError: Error { case tunnelConfigurationIsInvalid case firezoneIdIsInvalid case tokenNotFoundInKeychain - case dryStartStopCycle } class PacketTunnelProvider: NEPacketTunnelProvider { @@ -56,10 +55,10 @@ class PacketTunnelProvider: NEPacketTunnelProvider { options: [String: NSObject]?, completionHandler: @escaping @Sendable (Error?) -> Void ) { - // Dummy start to get the extension running on macOS after upgrade - if options?["dryRun"] as? Bool == true { - Log.info("Dry run startup requested - extension awakened but not starting tunnel") - return completionHandler(PacketTunnelProviderError.dryStartStopCycle) + // Dummy start to attach a utun for cleanup later + if options?["cycleStart"] as? Bool == true { + Log.info("Cycle start requested - extension awakened and temporarily starting tunnel") + return completionHandler(nil) } // Log version on actual tunnel start @@ -68,20 +67,6 @@ class PacketTunnelProvider: NEPacketTunnelProvider { let build = Bundle.main.object(forInfoDictionaryKey: "CFBundleVersion") as? String ?? "unknown" Log.info("Starting tunnel - Version: \(version), Build: \(build)") - // Try to load configuration from options first (passed from client at startup) - if let configData = options?["configuration"] as? Data { - do { - let decoder = PropertyListDecoder() - let configFromOptions = try decoder.decode(TunnelConfiguration.self, from: configData) - Log.info("Loaded configuration from startTunnel options") - // Save it for future fallback (e.g., system-initiated restarts) - configFromOptions.save() - self.tunnelConfiguration = configFromOptions - } catch { - Log.error(error) - } - } - // If the tunnel starts up before the GUI after an upgrade crossing the 1.4.15 version boundary, // the old system settings-based config will still be present and the new configuration will be empty. // So handle that edge case gracefully. diff --git a/website/src/components/Changelog/Apple.tsx b/website/src/components/Changelog/Apple.tsx index 8ace1064a..df2580b92 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 macOS where the utun index would + auto-increment by itself on configuration updates. + Fixes an issue where the reported client version was out of date.