From 0cdfd1fd4f12100ddd45f864e0a6dc898e843ce6 Mon Sep 17 00:00:00 2001 From: Jamil Date: Thu, 5 Dec 2024 21:51:22 -0800 Subject: [PATCH] fix(apple/macos): Install system extension on app launch (#7459) - Installs the system extension on app launch instead of each time we start the tunnel, as [recommended by Apple](https://developer.apple.com/documentation/systemextensions/installing-system-extensions-and-drivers). This will typically happen when the app is installed for the first time, or upgraded / downgraded. - Changes the completion handler functionality for observing the system extension status to an observed property on the class. This allows us to update the MenuBar based on the status of the installation, preventing the user from attempting to sign in unless the system extension has been installed. ~~This PR exposes a new, subtle issue - since we don't reinstall the system extension on each startTunnel, the process stays running. This is expected. However, now the logging handle needs to be maintained across connlib sessions, similar to the Android tunnel lifetime.~~ Fixed in #7460 Expect one or two more PRs to handle further edge cases with improved UX as more testing with the release build and upgrade/downgrade workflows are attempted. --- .../Firezone/Application/FirezoneApp.swift | 4 ++ .../Managers/SystemExtensionManager.swift | 33 ++++++------- .../FirezoneKit/Managers/TunnelManager.swift | 33 ++----------- .../Sources/FirezoneKit/Views/MenuBar.swift | 48 +++++++++++++++---- .../PacketTunnelProvider.swift | 2 + 5 files changed, 64 insertions(+), 56 deletions(-) diff --git a/swift/apple/Firezone/Application/FirezoneApp.swift b/swift/apple/Firezone/Application/FirezoneApp.swift index 43993ed26..d85853823 100644 --- a/swift/apple/Firezone/Application/FirezoneApp.swift +++ b/swift/apple/Firezone/Application/FirezoneApp.swift @@ -73,6 +73,10 @@ struct FirezoneApp: App { menuBar = MenuBar(model: SessionViewModel(favorites: favorites, store: store)) } + // Apple recommends installing the system extension as early as possible after app launch. + // See https://developer.apple.com/documentation/systemextensions/installing-system-extensions-and-drivers + SystemExtensionManager.shared.installSystemExtension(identifier: TunnelManager.bundleIdentifier) + // SwiftUI will show the first window group, so close it on launch _ = AppViewModel.WindowDefinition.allCases.map { $0.window()?.close() } } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift index ee7c48681..496142bc8 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift @@ -7,20 +7,19 @@ #if os(macOS) import SystemExtensions -enum SystemExtensionError: Error { - case UnexpectedResult(result: OSSystemExtensionRequest.Result) - case NeedsUserApproval -} +public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, ObservableObject { + // Maintain a static handle to the extension manager for tracking the state of the extension activation. + public static let shared = SystemExtensionManager() -public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate { - private var completionHandler: ((Error?) -> Void)? + @Published public var status: ExtensionStatus = .unknown - public func installSystemExtension( - identifier: String?, - completionHandler: @escaping (Error?) -> Void - ) { - guard let identifier = identifier else { return } - self.completionHandler = completionHandler + public enum ExtensionStatus { + case awaitingUserApproval + case installed + case unknown + } + + public func installSystemExtension(identifier: String) { let request = OSSystemExtensionRequest.activationRequest(forExtensionWithIdentifier: identifier, queue: .main) request.delegate = self @@ -33,23 +32,21 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate public func request(_ request: OSSystemExtensionRequest, didFinishWithResult result: OSSystemExtensionRequest.Result) { guard result == .completed else { - completionHandler?(SystemExtensionError.UnexpectedResult(result: result)) + status = .awaitingUserApproval return } // Success - completionHandler?(nil) + status = .installed } public func request(_ request: OSSystemExtensionRequest, didFailWithError error: Error) { - completionHandler?(error) + status = .awaitingUserApproval } public func requestNeedsUserApproval(_ request: OSSystemExtensionRequest) { - completionHandler?(SystemExtensionError.NeedsUserApproval) - - // TODO: Inform the user to approve the system extension in System Preferences > Security & Privacy. + status = .awaitingUserApproval } public func request(_ request: OSSystemExtensionRequest, actionForReplacingExtension existing: OSSystemExtensionProperties, withExtension ext: OSSystemExtensionProperties) -> OSSystemExtensionRequest.ReplacementAction { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/TunnelManager.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/TunnelManager.swift index 62e98d327..a19f88eaf 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/TunnelManager.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/TunnelManager.swift @@ -93,11 +93,7 @@ public class TunnelManager { // Encoder used to send messages to the tunnel private let encoder = PropertyListEncoder() -#if os(macOS) - private let systemExtensionManager = SystemExtensionManager() -#endif - - private let bundleIdentifier = "\(Bundle.main.bundleIdentifier!).network-extension" + public static let bundleIdentifier: String = "\(Bundle.main.bundleIdentifier!).network-extension" private let bundleDescription = "Firezone" init() { @@ -111,7 +107,7 @@ public class TunnelManager { let settings = Settings.defaultValue protocolConfiguration.providerConfiguration = settings.toProviderConfiguration() - protocolConfiguration.providerBundleIdentifier = bundleIdentifier + protocolConfiguration.providerBundleIdentifier = TunnelManager.bundleIdentifier protocolConfiguration.serverAddress = settings.apiURL manager.localizedDescription = bundleDescription manager.protocolConfiguration = protocolConfiguration @@ -142,7 +138,7 @@ public class TunnelManager { Log.app.log("\(#function): \(managers.count) tunnel managers found") for manager in managers { if let protocolConfiguration = manager.protocolConfiguration as? NETunnelProviderProtocol, - protocolConfiguration.providerBundleIdentifier == bundleIdentifier, + protocolConfiguration.providerBundleIdentifier == TunnelManager.bundleIdentifier, let providerConfiguration = protocolConfiguration.providerConfiguration as? [String: String] { // Found it @@ -230,29 +226,6 @@ public class TunnelManager { options = ["token": token as NSObject] } -#if os(macOS) - // On macOS we use System Extensions, and we need to wait for them to be activated - // before we can continue starting the tunnel. Otherwise, the tunnel will come up, - // but then be killed immediately after since the system will reap its process as - // the system extension is moved in place. This is more of an issue in development - // where the system will replace the extension on each call to this API, ignoring the - // version check that typically prevents extensions from being reactivated if they have the - // same marketing version. - systemExtensionManager.installSystemExtension(identifier: bundleIdentifier) { error in - if let error = error { - Log.app.error("\(#function): Installing system extension failed! \(error.localizedDescription)") - - return - } - - self.startTunnel(options: options) - } -#else - startTunnel(options: options) -#endif - } - - func startTunnel(options: [String: NSObject]?) { do { try session().startTunnel(options: options) } catch { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift index 3ef29084a..d6c07c77c 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift @@ -27,6 +27,7 @@ public final class MenuBar: NSObject, ObservableObject { private var cancellables: Set = [] private var vpnStatus: NEVPNStatus = .disconnected + private var extensionStatus: SystemExtensionManager.ExtensionStatus = .unknown private var updateChecker: UpdateChecker = UpdateChecker() private var updateMenuDisplayed: Bool = false @@ -96,6 +97,16 @@ public final class MenuBar: NSObject, ObservableObject { self.updateStatusItemIcon() self.refreshUpdateItem() }).store(in: &cancellables) + + SystemExtensionManager.shared.$status + .receive(on: DispatchQueue.main) + .sink(receiveValue: { [weak self] status in + guard let self = self else { return } + + self.extensionStatus = status + self.handleTunnelStatusOrResourcesChanged() + }) + .store(in: &cancellables) } private lazy var menu = NSMenu() @@ -271,6 +282,18 @@ public final class MenuBar: NSObject, ObservableObject { } } + @objc private func installSystemExtensionButtonTapped() { + Task { + SystemExtensionManager.shared.installSystemExtension(identifier: TunnelManager.bundleIdentifier) + } + } + + @objc private func grantVPNPermissionButtonTapped() { + Task { + model.store.createVPNProfile() + } + } + @objc private func settingsButtonTapped() { AppViewModel.WindowDefinition.settings.openWindow() } @@ -366,25 +389,34 @@ public final class MenuBar: NSObject, ObservableObject { let resources = model.resources let status = model.status // Update "Sign In" / "Sign Out" menu items - switch status { - case .invalid: - signInMenuItem.title = "Requires VPN permission" - signInMenuItem.target = nil + switch (extensionStatus, status) { + case (.awaitingUserApproval, _): + signInMenuItem.title = "Enable the system extension to sign in…" + signInMenuItem.target = self + signInMenuItem.action = #selector(installSystemExtensionButtonTapped) signOutMenuItem.isHidden = true settingsMenuItem.target = nil - case .disconnected: + case (_, .invalid): + signInMenuItem.title = "Allow the VPN permission to sign in…" + signInMenuItem.target = self + signInMenuItem.action = #selector(grantVPNPermissionButtonTapped) + signOutMenuItem.isHidden = true + settingsMenuItem.target = nil + case (_, .disconnected): signInMenuItem.title = "Sign In" signInMenuItem.target = self + signInMenuItem.action = #selector(signInButtonTapped) signInMenuItem.isEnabled = true signOutMenuItem.isHidden = true settingsMenuItem.target = self - case .disconnecting: - signInMenuItem.title = "Signing out..." + case (_, .disconnecting): + signInMenuItem.title = "Signing out…" signInMenuItem.target = self + signInMenuItem.action = #selector(signInButtonTapped) signInMenuItem.isEnabled = false signOutMenuItem.isHidden = true settingsMenuItem.target = self - case .connected, .reasserting, .connecting: + case (_, .connected), (_, .reasserting), (_, .connecting): let title = "Signed in as \(model.store.actorName ?? "Unknown User")" signInMenuItem.title = title signInMenuItem.target = nil diff --git a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift index 9262098c4..9125ae518 100644 --- a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift +++ b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift @@ -133,6 +133,8 @@ class PacketTunnelProvider: NEPacketTunnelProvider { adapter?.stop() cancelTunnelWithError(nil) + super.stopTunnel(with: reason, completionHandler: completionHandler) + completionHandler() } override func handleAppMessage(_ message: Data, completionHandler: ((Data?) -> Void)? = nil) {