From 3722c81eca83a68786be4c65d369ae9cfa218143 Mon Sep 17 00:00:00 2001 From: Jamil Date: Tue, 14 Jan 2025 16:13:34 -0800 Subject: [PATCH] fix(apple/macOS): Handle outdated system extensions (#7759) When a user launches the macOS app, we check if the system extension is installed. If it was, we assumed it would function properly. However, an older version of the extension can be installed from our current app version, so we would erroneously consider the extension as "installed" even though it needed to be updated. To fix this, we introduced an enum for tracking the system extension state with `installed`, `needsReplacement`, and `needsInstall` states. These track whether the extension is up-to-date, needs upgrade (or downgrade), or needs to be approved and enabled by the user altogether respectively. Importantly, this also gracefully handles downgrades, not just upgrades since we already return a `.replace` action in our request callback that the system calls when installing an extension with the same bundle ID as one that exists. --- .../Managers/SystemExtensionManager.swift | 57 ++++++++++++++++--- .../Sources/FirezoneKit/Stores/Store.swift | 20 +++++-- .../Sources/FirezoneKit/Views/AppView.swift | 9 +-- .../FirezoneKit/Views/GrantVPNView.swift | 8 ++- 4 files changed, 72 insertions(+), 22 deletions(-) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift index 3505728f1..bb8a1e7c6 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift @@ -18,13 +18,25 @@ public enum SystemExtensionError: Error { } } +public enum SystemExtensionStatus { + // Not installed or enabled at all + case needsInstall + + // Version of the extension is installed that differs from our bundle version. + // "Installing" it will replace it without prompting the user. + case needsReplacement + + // Installed and version is current with our app bundle + case installed +} + public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, ObservableObject { // Delegate methods complete with either a true or false outcome or an Error - private var continuation: CheckedContinuation? + private var continuation: CheckedContinuation? public func installSystemExtension( identifier: String, - continuation: CheckedContinuation + continuation: CheckedContinuation ) { self.continuation = continuation @@ -35,9 +47,9 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, OSSystemExtensionManager.shared.submitRequest(request) } - public func isInstalled( + public func checkStatus( identifier: String, - continuation: CheckedContinuation + continuation: CheckedContinuation ) { self.continuation = continuation @@ -62,7 +74,7 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, } // Installation succeeded - resume(returning: true) + resume(returning: .installed) } // Result of properties request @@ -70,9 +82,36 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, _ request: OSSystemExtensionRequest, foundProperties properties: [OSSystemExtensionProperties] ) { - // Returns true if we find any extension installed matching the bundle id - // Otherwise false - continuation?.resume(returning: properties.contains { $0.isEnabled }) + // Standard keys in any bundle. If missing, we've got bigger issues. + let ourBundleVersion = Bundle.main.object( + forInfoDictionaryKey: "CFBundleVersion" + ) as! String + let ourBundleShortVersion = Bundle.main.object( + forInfoDictionaryKey: "CFBundleShortVersionString" + ) as! String + + // Up to date if version and build number match + let isCurrentVersionInstalled = properties.contains { sysex in + sysex.isEnabled + && sysex.bundleVersion == ourBundleVersion + && sysex.bundleShortVersion == ourBundleShortVersion + } + if isCurrentVersionInstalled { + resume(returning: .installed) + + return + } + + // Needs replacement if we found our extension, but its version doesn't match + // Note this can happen for upgrades _or_ downgrades + let isAnyVersionInstalled = properties.contains { $0.isEnabled } + if isAnyVersionInstalled { + resume(returning: .needsReplacement) + + return + } + + resume(returning: .needsInstall) } public func request(_ request: OSSystemExtensionRequest, didFailWithError error: Error) { @@ -92,7 +131,7 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, self.continuation = nil } - private func resume(returning val: Bool) { + private func resume(returning val: SystemExtensionStatus) { self.continuation?.resume(returning: val) self.continuation = nil } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index 9e14a4471..c7e23e722 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -24,8 +24,10 @@ public final class Store: ObservableObject { // to observe @Published private(set) var status: NEVPNStatus? +#if os(macOS) // Track whether our system extension has been installed (macOS) - @Published private(set) var isInstalled: Bool = false + @Published private(set) var systemExtensionStatus: SystemExtensionStatus? +#endif // This is not currently updated after it is initialized, but // we could periodically update it if we need to. @@ -117,14 +119,20 @@ public final class Store: ObservableObject { #if os(macOS) let checker = SystemExtensionManager() - self.isInstalled = try await withCheckedThrowingContinuation { - (continuation: CheckedContinuation) in + self.systemExtensionStatus = try await withCheckedThrowingContinuation { + (continuation: CheckedContinuation) in - checker.isInstalled( + checker.checkStatus( identifier: VPNConfigurationManager.bundleIdentifier, continuation: continuation ) } + + // If already installed but the wrong version, go ahead and install. + // This shouldn't prompt the user. + if self.systemExtensionStatus == .needsReplacement { + try await installSystemExtension() + } #endif } @@ -134,8 +142,8 @@ public final class Store: ObservableObject { // 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 - self.isInstalled = try await withCheckedThrowingContinuation { - (continuation: CheckedContinuation) in + self.systemExtensionStatus = try await withCheckedThrowingContinuation { + (continuation: CheckedContinuation) in installer.installSystemExtension( identifier: VPNConfigurationManager.bundleIdentifier, diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift index b7353feb0..d2d7fbfab 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift @@ -38,8 +38,9 @@ public class AppViewModel: ObservableObject { #if os(macOS) try await self.store.checkedIfInstalled() + let isInstalled = self.store.systemExtensionStatus == .installed - if !self.store.isInstalled || self.store.status == .invalid { + if !isInstalled || self.store.status == .invalid { // Show the main Window if VPN permission needs to be granted AppViewModel.WindowDefinition.main.openWindow() @@ -106,10 +107,10 @@ public struct AppView: View { } } #elseif os(macOS) - switch (model.store.isInstalled, model.status) { - case (_, nil): + switch (model.store.systemExtensionStatus, model.status) { + case (nil, nil): ProgressView() - case (false, _), (_, .invalid): + case (.needsInstall, _), (_, .invalid): GrantVPNView(model: GrantVPNViewModel(store: model.store)) default: FirstTimeView() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift index 45df13854..edcc9f207 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift @@ -18,11 +18,13 @@ final class GrantVPNViewModel: ObservableObject { init(store: Store) { self.store = store - store.$isInstalled +#if os(macOS) + store.$systemExtensionStatus .receive(on: DispatchQueue.main) - .sink(receiveValue: { [weak self] isInstalled in - self?.isInstalled = isInstalled + .sink(receiveValue: { [weak self] status in + self?.isInstalled = status == .installed }).store(in: &cancellables) +#endif } func installSystemExtensionButtonTapped() {