fix(apple): Ensure system extension loads after upgrade (#9166)

On macOS, after upgrading the client, the new system extension fails to
respond to IPC commands until it receives a `startTunnel` call. After
that, subsequent IPC calls will succeed across relaunches of the app.

To fix this, we introduce a dummy `startTunnel` call on macOS that
attempts to bring life into the System extension whenever we receive
`nil` configuration.

We also tidy up a few other things to make this easier to follow.


Fixes #9156 
Fixes #8476

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift

@@ -79,6 +79,16 @@ class IPCClient {
     try session([.connected, .connecting, .reasserting]).stopTunnel()
   }
 
+#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 start the extension.
+  func startSystemExtension() throws {
+    let options: [String: NSObject] = ["dryRun": true as NSObject]
+    try session().startTunnel(options: options)
+  }
+#endif
+
   func getConfiguration() async throws -> Configuration? {
     return try await withCheckedThrowingContinuation { continuation in
       do {

swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/SystemExtensionManager.swift

@@ -7,7 +7,7 @@
 #if os(macOS)
 import SystemExtensions
 
-public enum SystemExtensionError: Error {
+enum SystemExtensionError: Error {
   case unknownResult(OSSystemExtensionRequest.Result)
 
   var description: String {
@@ -18,7 +18,7 @@ public enum SystemExtensionError: Error {
   }
 }
 
-public enum SystemExtensionStatus {
+enum SystemExtensionStatus {
   // Not installed or enabled at all
   case needsInstall
 
@@ -30,43 +30,41 @@ public enum SystemExtensionStatus {
   case installed
 }
 
-public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, ObservableObject {
+enum SystemExtensionRequestType {
+  case install
+  case check
+}
+
+class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate, ObservableObject {
   // Delegate methods complete with either a true or false outcome or an Error
   private var continuation: CheckedContinuation<SystemExtensionStatus, Error>?
 
-  public func installSystemExtension(
+  func sendRequest(
+    requestType: SystemExtensionRequestType,
     identifier: String,
     continuation: CheckedContinuation<SystemExtensionStatus, Error>
   ) {
     self.continuation = continuation
 
-    let request = OSSystemExtensionRequest.activationRequest(forExtensionWithIdentifier: identifier, queue: .main)
+    let request = switch requestType {
+    case .install:
+      OSSystemExtensionRequest.activationRequest(forExtensionWithIdentifier: identifier, queue: .main)
+    case .check:
+      OSSystemExtensionRequest.propertiesRequest(
+        forExtensionWithIdentifier: identifier,
+        queue: .main
+      )
+    }
+
     request.delegate = self
 
-    // Install extension
-    OSSystemExtensionManager.shared.submitRequest(request)
-  }
-
-  public func checkStatus(
-    identifier: String,
-    continuation: CheckedContinuation<SystemExtensionStatus, Error>
-  ) {
-    self.continuation = continuation
-
-    let request = OSSystemExtensionRequest.propertiesRequest(
-      forExtensionWithIdentifier: identifier,
-      queue: .main
-    )
-    request.delegate = self
-
-    // Send request
     OSSystemExtensionManager.shared.submitRequest(request)
   }
 
   // MARK: - OSSystemExtensionRequestDelegate
 
   // Result of system extension installation
-  public func request(
+  func request(
     _ request: OSSystemExtensionRequest,
     didFinishWithResult result: OSSystemExtensionRequest.Result
   ) {
@@ -81,7 +79,7 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate,
   }
 
   // Result of properties request
-  public func request(
+  func request(
     _ request: OSSystemExtensionRequest,
     foundProperties properties: [OSSystemExtensionProperties]
   ) {
@@ -116,15 +114,15 @@ public class SystemExtensionManager: NSObject, OSSystemExtensionRequestDelegate,
     resume(returning: .needsInstall)
   }
 
-  public func request(_ request: OSSystemExtensionRequest, didFailWithError error: Error) {
+  func request(_ request: OSSystemExtensionRequest, didFailWithError error: Error) {
     resume(throwing: error)
   }
 
-  public func requestNeedsUserApproval(_ request: OSSystemExtensionRequest) {
+  func requestNeedsUserApproval(_ request: OSSystemExtensionRequest) {
     // We assume this state until we receive a success response.
   }
 
-  public func request(
+  func request(
     _ request: OSSystemExtensionRequest,
     actionForReplacingExtension existing: OSSystemExtensionProperties,
     withExtension ext: OSSystemExtensionProperties

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift

@@ -15,6 +15,7 @@ import AppKit
 
 @MainActor
 // TODO: Move some state logic to view models
+// swiftlint:disable:next type_body_length
 public final class Store: ObservableObject {
   @Published private(set) var actorName: String
   @Published private(set) var favorites = Favorites()
@@ -27,7 +28,7 @@ public final class Store: ObservableObject {
   @Published private(set) var configuration: Configuration?
 
   // Enacapsulate Tunnel status here to make it easier for other components to observe
-  @Published private(set) var status: NEVPNStatus?
+  @Published private(set) var vpnStatus: NEVPNStatus?
 
   // User notifications
   @Published private(set) var decision: UNAuthorizationStatus?
@@ -59,74 +60,48 @@ public final class Store: ObservableObject {
     }
 
     // Load our state from the system. Based on what's loaded, we may need to ask the user for permission for things.
-    initNotifications()
-    initSystemExtension()
-    initVPNConfiguration()
-  }
-
-  func initNotifications() {
+    // When everything loads correctly, we attempt to start the tunnel if connectOnStart is enabled.
     Task {
-      self.decision = await self.sessionNotification.loadAuthorizationStatus()
+      do {
+        await initNotifications()
+        try await initSystemExtension()
+        try await initVPNConfiguration()
+        try await setupTunnelObservers()
+        try await initConfiguration()
+        try await maybeAutoConnect()
+      } catch {
+        Log.error(error)
+      }
     }
   }
 
-  func initSystemExtension() {
 #if os(macOS)
-    Task {
-      do {
-        self.systemExtensionStatus = try await self.checkSystemExtensionStatus()
-      } catch {
-        Log.error(error)
-      }
+  func systemExtensionRequest(_ requestType: SystemExtensionRequestType) async throws {
+    let manager = SystemExtensionManager()
+
+    self.systemExtensionStatus =
+    try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<SystemExtensionStatus, Error>) in
+      manager.sendRequest(
+        requestType: requestType,
+        identifier: VPNConfigurationManager.bundleIdentifier,
+        continuation: continuation
+      )
     }
+  }
 #endif
+
+  private func setupTunnelObservers() async throws {
+    let vpnStatusChangeHandler: (NEVPNStatus) async throws -> Void = { [weak self] status in
+      try await self?.handleVPNStatusChange(newVPNStatus: status)
+    }
+    try ipcClient().subscribeToVPNStatusUpdates(handler: vpnStatusChangeHandler)
+    self.vpnStatus = try ipcClient().sessionStatus()
   }
 
-  func initVPNConfiguration() {
-    Task {
-      do {
-        // Try to load existing configuration
-        if let manager = try await VPNConfigurationManager.load() {
-          try await manager.maybeMigrateConfiguration()
-          self.vpnConfigurationManager = manager
-          try await setupTunnelObservers()
-          self.configuration = try await ipcClient().getConfiguration()
-          Telemetry.firezoneId = configuration?.firezoneId
+  private func handleVPNStatusChange(newVPNStatus: NEVPNStatus) async throws {
+    self.vpnStatus = newVPNStatus
 
-          try await manager.enableConfiguration()
-          if configuration?.connectOnStart ?? true {
-            try ipcClient().start()
-          }
-        } else {
-          status = .invalid
-        }
-      } catch {
-        Log.error(error)
-      }
-    }
-  }
-
-  func setupTunnelObservers() async throws {
-    let statusChangeHandler: (NEVPNStatus) async throws -> Void = { [weak self] status in
-      try await self?.handleStatusChange(newStatus: status)
-    }
-
-    try ipcClient().subscribeToVPNStatusUpdates(handler: statusChangeHandler)
-    try await handleStatusChange(newStatus: ipcClient().sessionStatus())
-  }
-
-  func handleStatusChange(newStatus: NEVPNStatus) async throws {
-    status = newStatus
-
-    if status == .invalid {
-      // VPN configuration was yanked from system settings
-      endConfigurationPolling()
-    } else {
-      // This is a no-op if the timer is already active
-      beginConfigurationPolling()
-    }
-
-    if status == .connected {
+    if newVPNStatus == .connected {
       beginUpdatingResources()
     } else {
       endUpdatingResources()
@@ -135,7 +110,7 @@ public final class Store: ObservableObject {
 #if os(macOS)
     // On macOS we must show notifications from the UI process. On iOS, we've already initiated the notification
     // from the tunnel process, because the UI process is not guaranteed to be alive.
-    if status == .disconnected {
+    if vpnStatus == .disconnected {
       do {
         let reason = try await ipcClient().consumeStopReason()
         if reason == .authenticationCanceled {
@@ -148,54 +123,65 @@ public final class Store: ObservableObject {
 
     // When this happens, it's because either our VPN configuration or System Extension (or both) were removed.
     // So load the system extension status again to determine which view to load.
-    if status == .invalid {
-      self.systemExtensionStatus = try await checkSystemExtensionStatus()
+    if vpnStatus == .invalid {
+      try await systemExtensionRequest(.check)
     }
 #endif
   }
 
+  private func initNotifications() async {
+    self.decision = await self.sessionNotification.loadAuthorizationStatus()
+  }
+
+  private func initSystemExtension() async throws {
 #if os(macOS)
-  func checkSystemExtensionStatus() async throws -> SystemExtensionStatus {
-    let checker = SystemExtensionManager()
+    try await systemExtensionRequest(.check)
 
-    let status =
-    try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<SystemExtensionStatus, Error>) in
-      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 systemExtensionStatus == .needsReplacement {
+      try await systemExtensionRequest(.install)
     }
-
-    // If already installed but the wrong version, go ahead and install.
-    // This shouldn't prompt the user.
-    if status == .needsReplacement {
-      try await installSystemExtension()
-    }
-
-    return status
-  }
-
-  func installSystemExtension() async throws {
-    let installer = SystemExtensionManager()
-
-    // 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.systemExtensionStatus =
-    try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<SystemExtensionStatus, Error>) in
-      installer.installSystemExtension(
-        identifier: VPNConfigurationManager.bundleIdentifier,
-        continuation: continuation
-      )
-    }
-  }
 #endif
+  }
 
+  private func initVPNConfiguration() async throws {
+    // Try to load existing configuration
+    if let manager = try await VPNConfigurationManager.load() {
+      try await manager.maybeMigrateConfiguration()
+      self.vpnConfigurationManager = manager
+    } else {
+      self.vpnStatus = .invalid
+    }
+  }
+
+  // On macOS, after upgrading Firezone, we need to issue a startTunnel to start the IPC service so that we
+  // can fetch configuration. We try a few times here to do that so that we can determine connectOnStart, before
+  // giving up and polling configuration anyway.
+  private func initConfiguration() async throws {
+    var configuration: Configuration?
+    let end = Date().addingTimeInterval(3)
+
+    while configuration == nil && Date() < end {
+      configuration = try await getConfigurationStartingSystemExtension()
+      try await Task.sleep(nanoseconds: 100_000_000)
+    }
+
+    self.configuration = configuration
+
+    beginConfigurationPolling()
+  }
+
+  private func maybeAutoConnect() async throws {
+    if configuration?.connectOnStart == true {
+      try await manager().enableConfiguration()
+      try ipcClient().start()
+    }
+  }
   func installVPNConfiguration() async throws {
     // Create a new VPN configuration in system settings.
     self.vpnConfigurationManager = try await VPNConfigurationManager()
 
     self.configuration = try await ipcClient().getConfiguration()
-    Telemetry.firezoneId = configuration?.firezoneId
 
     try await setupTunnelObservers()
   }
@@ -281,7 +267,14 @@ public final class Store: ObservableObject {
           self.configurationUpdateTask = Task {
             if !Task.isCancelled {
               do {
-                self.configuration = try await self.ipcClient().getConfiguration()
+                self.configuration = try await self.getConfigurationStartingSystemExtension()
+              } catch let error as NSError {
+                // https://developer.apple.com/documentation/networkextension/nevpnerror-swift.struct/code
+                if error.domain == "NEVPNErrorDomain" && error.code == 1 {
+                  // not initialized yet
+                } else {
+                  Log.error(error)
+                }
               } catch {
                 Log.error(error)
               }
@@ -298,11 +291,21 @@ public final class Store: ObservableObject {
     self.configurationTimer = timer
   }
 
-  private func endConfigurationPolling() {
-    configurationUpdateTask?.cancel()
-    configurationTimer?.invalidate()
-    configurationTimer = nil
-    self.configuration = nil
+  private func getConfigurationStartingSystemExtension() async throws -> Configuration? {
+    var configuration = try await ipcClient().getConfiguration()
+
+#if os(macOS)
+    if configuration == nil {
+      try ipcClient().startSystemExtension()
+      configuration = try await ipcClient().getConfiguration()
+    }
+#endif
+
+    if Telemetry.firezoneId == nil {
+      Telemetry.firezoneId = configuration?.firezoneId
+    }
+
+    return configuration
   }
 
   // Network Extensions don't have a 2-way binding up to the GUI process,
@@ -323,6 +326,13 @@ public final class Store: ObservableObject {
             if !Task.isCancelled {
               do {
                 self.resourceList = try await self.ipcClient().fetchResources()
+              } catch let error as NSError {
+                // https://developer.apple.com/documentation/networkextension/nevpnerror-swift.struct/code
+                if error.domain == "NEVPNErrorDomain" && error.code == 1 {
+                  // not initialized yet
+                } else {
+                  Log.error(error)
+                }
               } catch {
                 Log.error(error)
               }

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift

@@ -27,18 +27,18 @@ public struct AppView: View {
   // handle.
   private static var cancellables: Set<AnyCancellable> = []
   public static func subscribeToGlobalEvents(store: Store) {
-    store.$status
+    store.$vpnStatus
       .combineLatest(store.$systemExtensionStatus)
       .receive(on: DispatchQueue.main)
       .debounce(for: .seconds(0.3), scheduler: DispatchQueue.main) // Prevents flurry of windows from opening
-      .sink(receiveValue: { status, systemExtensionStatus in
+      .sink(receiveValue: { vpnStatus, systemExtensionStatus in
         // Open window in case permissions are revoked
-        if status == .invalid || systemExtensionStatus != .installed {
+        if vpnStatus == .invalid || systemExtensionStatus != .installed {
           WindowDefinition.main.openWindow()
         }
 
         // Close window for day to day use
-        if status != .invalid && systemExtensionStatus == .installed && launchedBefore() {
+        if vpnStatus != .invalid && systemExtensionStatus == .installed && launchedBefore() {
           WindowDefinition.main.window()?.close()
         }
       })
@@ -89,7 +89,7 @@ public struct AppView: View {
   @ViewBuilder
   public var body: some View {
 #if os(iOS)
-    switch (store.status, store.decision) {
+    switch (store.vpnStatus, store.decision) {
     case (nil, _), (_, nil):
       ProgressView()
     case (.invalid, _):
@@ -106,9 +106,12 @@ public struct AppView: View {
       }
     }
 #elseif os(macOS)
-    switch (store.systemExtensionStatus, store.status) {
+    switch (store.systemExtensionStatus, store.vpnStatus) {
     case (nil, nil):
-      ProgressView()
+      VStack {
+        ProgressView()
+        Text("Getting things ready... this should only take a few seconds.")
+      }
     case (.needsInstall, _), (_, .invalid):
       GrantVPNView()
     default:

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift

@@ -136,7 +136,7 @@ struct GrantVPNView: View {
   func installSystemExtension() {
     Task {
       do {
-        try await store.installSystemExtension()
+        try await store.systemExtensionRequest(.install)
 
         // The window has a tendency to go to the background after installing
         // the system extension

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift

@@ -203,7 +203,7 @@ public final class MenuBar: NSObject, ObservableObject {
         self.handleResourceListChanged()
       }).store(in: &cancellables)
 
-    store.$status
+    store.$vpnStatus
       .receive(on: DispatchQueue.main)
       .sink(receiveValue: { [weak self] _ in
         guard let self = self else { return }
@@ -242,7 +242,7 @@ public final class MenuBar: NSObject, ObservableObject {
     updateStatusItemIcon()
     updateSignInMenuItems()
     quitMenuItem.title = {
-      switch store.status {
+      switch store.vpnStatus {
       case .connected, .connecting:
         return "Disconnect and Quit"
       default:
@@ -324,13 +324,13 @@ public final class MenuBar: NSObject, ObservableObject {
   }
 
   func updateStatusItemIcon() {
-    updateAnimation(status: store.status)
-    statusItem.button?.image = getStatusIcon(status: store.status, notification: updateChecker.updateAvailable)
+    updateAnimation(status: store.vpnStatus)
+    statusItem.button?.image = getStatusIcon(status: store.vpnStatus, notification: updateChecker.updateAvailable)
   }
 
   func updateSignInMenuItems() {
     // Update "Sign In" / "Sign Out" menu items
-    switch store.status {
+    switch store.vpnStatus {
     case nil:
       signInMenuItem.title = "Loading VPN configurations from system settings…"
       signInMenuItem.action = nil
@@ -374,7 +374,7 @@ public final class MenuBar: NSObject, ObservableObject {
 
   func updateResourcesMenuItems() {
     // Update resources "header" menu items
-    switch store.status {
+    switch store.vpnStatus {
     case .connecting:
       resourcesTitleMenuItem.isHidden = true
       resourcesUnavailableMenuItem.isHidden = false
@@ -727,7 +727,7 @@ public final class MenuBar: NSObject, ObservableObject {
         // our VPN configuration got removed. Since we don't know which, reinstall
         // the system extension here too just in case. It's a no-op if already
         // installed.
-        try await store.installSystemExtension()
+        try await store.systemExtensionRequest(.install)
         try await store.installVPNConfiguration()
       } catch let error as NSError {
         if error.domain == "NEVPNErrorDomain" && error.code == 5 {

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/SessionView.swift

@@ -15,7 +15,7 @@ struct SessionView: View {
   @EnvironmentObject var store: Store
 
   var body: some View {
-    switch store.status {
+    switch store.vpnStatus {
     case .connected:
       switch store.resourceList {
       case .loaded(let resources):

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/SettingsView.swift

@@ -245,7 +245,7 @@ public struct SettingsView: View {
           ToolbarItem(placement: .navigationBarTrailing) {
             Button("Save") {
               let action = ConfirmationAlertContinueAction.saveAllSettingsAndDismiss
-              if case .connected = store.status {
+              if case .connected = store.vpnStatus {
                 self.confirmationAlertContinueAction = action
                 self.isShowingConfirmationAlert = true
               } else {
@@ -313,7 +313,7 @@ public struct SettingsView: View {
             "Apply",
             action: {
               let action = ConfirmationAlertContinueAction.saveSettings
-              if [.connected, .connecting, .reasserting].contains(store.status) {
+              if [.connected, .connecting, .reasserting].contains(store.vpnStatus) {
                 self.confirmationAlertContinueAction = action
                 self.isShowingConfirmationAlert = true
               } else {
@@ -758,7 +758,7 @@ public struct SettingsView: View {
   private func saveSettings() async throws {
     try await viewModel.save()
 
-    if [.connected, .connecting, .reasserting].contains(store.status) {
+    if [.connected, .connecting, .reasserting].contains(store.vpnStatus) {
       // TODO: Warn user instead of signing out
       try await self.store.signOut()
     }

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/iOSNavigationView.swift

@@ -54,12 +54,12 @@ struct iOSNavigationView<Content: View>: View { // swiftlint:disable:this type_n
         Label("Settings", systemImage: "gear")
       }
     )
-    .disabled(store.status == .invalid)
+    .disabled(store.vpnStatus == .invalid)
   }
 
   private var authMenu: some View {
     Menu {
-      if store.status == .connected {
+      if store.vpnStatus == .connected {
         Text("Signed in as \(store.actorName)")
         Button(
           action: {

swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift

@@ -42,7 +42,12 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
     completionHandler: @escaping (Error?) -> Void
   ) {
     super.startTunnel(options: options, completionHandler: completionHandler)
-    Log.log("\(#function)")
+
+    // Dummy start to get the extension running on macOS after upgrade
+    if options?["dryRun"] as? Bool == true {
+      completionHandler(nil)
+      return
+    }
 
     // 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.