Fixes tunnel startup options handling (#489)

Task/Issue URL: https://app.asana.com/0/0/1205466155643247/f
iOS PR: duckduckgo/iOS#2003
macOS PR: duckduckgo/macos-browser#1581
What kind of version bump will this require?: Patch

## Description

Fixes tunnel startup options handling, and improves the options loading mechanism to make it unit testable.

---

* Fixes startup options reset

* Makes a correction to my previous commits

* WIP

* WIP

* Adds unit tests for StartupOptions

* Fixes some swiftlint warnings and improves the code

Sources/NetworkProtection/ExtensionMessage/ExtensionMessage.swift

@@ -69,7 +69,10 @@ public enum ExtensionMessage: RawRepresentable {
         case .isHavingConnectivityIssues:
             self = .isHavingConnectivityIssues
         case .setSelectedServer:
-            guard data.count > 1 else { return nil }
+            guard data.count > 1 else {
+                self = .setSelectedServer(nil)
+                return
+            }
             let serverName = ExtensionMessageString(rawValue: data[1...])
             self = .setSelectedServer(serverName?.value)
 
@@ -160,6 +163,7 @@ public enum ExtensionMessage: RawRepresentable {
                     assertionFailure("could not encode routes: \(error)")
                 }
             }
+
         case .setSelectedServer(.none),
              .setKeyValidity(.none),
              .resetAllState,

Sources/NetworkProtection/NetworkProtectionOptionKey.swift

@@ -31,6 +31,3 @@ public enum NetworkProtectionOptionKey {
     public static let excludedRoutes = "excludedRoutes"
     public static let connectionTesterEnabled = "connectionTesterEnabled"
 }
-public enum NetworkProtectionOptionValue {
-    public static let `true` = "true" as NSString
-}

Sources/NetworkProtection/PacketTunnelProvider.swift

@@ -37,12 +37,15 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
     // MARK: - Error Handling
 
     enum TunnelError: LocalizedError {
+        case startingTunnelWithoutAuthToken
         case couldNotGenerateTunnelConfiguration(internalError: Error)
         case couldNotFixConnection
         case simulateTunnelFailureError
 
         var errorDescription: String? {
             switch self {
+            case .startingTunnelWithoutAuthToken:
+                return "Missing auth token at startup"
             case .couldNotGenerateTunnelConfiguration(let internalError):
                 return "Failed to generate a tunnel configuration: \(internalError.localizedDescription)"
             case .simulateTunnelFailureError:
@@ -184,19 +187,22 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
     }
 
     private func setKeyValidity(_ interval: TimeInterval?) {
-        guard keyValidity != interval,
-            let interval = interval else {
-
+        guard keyValidity != interval else {
             return
         }
 
-        let firstExpirationDate = Date().addingTimeInterval(interval)
+        if let interval {
+            let firstExpirationDate = Date().addingTimeInterval(interval)
+
+            os_log("Setting key validity interval to %{public}@ seconds (next expiration date %{public}@)",
+                   log: .networkProtectionKeyManagement,
+                   String(describing: interval),
+                   String(describing: firstExpirationDate))
+        } else {
+            os_log("Resetting key validity interval",
+                   log: .networkProtectionKeyManagement)
+        }
 
-        os_log("Setting key validity to %{public}@ seconds (next expiration date %{public}@)",
-               log: .networkProtectionKeyManagement,
-               type: .info,
-               String(describing: interval),
-               String(describing: firstExpirationDate))
         keyStore.setValidityInterval(interval)
     }
 
@@ -327,12 +333,33 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         protocolConfiguration as? NETunnelProviderProtocol
     }
 
-    private func load(options: [String: NSObject]?) throws {
-        guard let options = options else {
-            os_log("🔵 Tunnel options are not set", log: .networkProtection)
+    private func runDebugSimulations(options: StartupOptions) throws {
+        if options.simulateError {
+            throw TunnelError.simulateTunnelFailureError
+        }
+
+        if options.simulateCrash {
+            DispatchQueue.main.asyncAfter(deadline: .now() + TimeInterval.seconds(2)) {
+                fatalError("Simulated PacketTunnelProvider crash")
+            }
+
+            return
+        }
+
+        if options.simulateMemoryCrash {
+            Task {
+                var array = [String]()
+                while true {
+                    array.append("Crash")
+                }
+            }
+
             return
         }
+    }
 
+    private func load(options: StartupOptions) throws {
+        isConnectionTesterEnabled = options.enableTester
         loadKeyValidity(from: options)
         loadSelectedServer(from: options)
         try loadAuthToken(from: options)
@@ -342,32 +369,42 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
         let vendorOptions = provider?.providerConfiguration
 
         loadRoutes(from: vendorOptions)
-        self.isConnectionTesterEnabled = vendorOptions?[NetworkProtectionOptionKey.connectionTesterEnabled] as? Bool ?? true
     }
 
-    private func loadKeyValidity(from options: [String: AnyObject]) {
-        guard let keyValidityString = options[NetworkProtectionOptionKey.keyValidity] as? String,
-              let keyValidity = TimeInterval(keyValidityString) else {
-            return
+    private func loadKeyValidity(from options: StartupOptions) {
+        switch options.keyValidity {
+        case .set(let validity):
+            setKeyValidity(validity)
+        case .useExisting:
+            break
+        case .reset:
+            setKeyValidity(nil)
         }
-
-        setKeyValidity(keyValidity)
     }
 
-    private func loadSelectedServer(from options: [String: AnyObject]) {
-        guard let serverName = options[NetworkProtectionOptionKey.selectedServer] as? String else {
-            return
+    private func loadSelectedServer(from options: StartupOptions) {
+        switch options.selectedServer {
+        case .set(let selectedServer):
+            selectedServerStore.selectedServer = selectedServer
+        case .useExisting:
+            break
+        case .reset:
+            selectedServerStore.selectedServer = .automatic
         }
-
-        selectedServerStore.selectedServer = .endpoint(serverName)
     }
 
-    private func loadAuthToken(from options: [String: AnyObject]) throws {
-        guard let authToken = options[NetworkProtectionOptionKey.authToken] as? String else {
-            return
+    private func loadAuthToken(from options: StartupOptions) throws {
+        switch options.authToken {
+        case .set(let authToken):
+            try tokenStore.store(authToken)
+        case .useExisting:
+            break
+        case .reset:
+            // This case should in theory not be possible, but it's ideal to have this in place
+            // in case an error in the controller on the client side allows it.
+            try tokenStore.deleteToken()
+            throw TunnelError.startingTunnelWithoutAuthToken
         }
-
-        try tokenStore.store(authToken)
     }
 
     private func loadRoutes(from options: [String: Any]?) {
@@ -389,55 +426,41 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
 
     open override func startTunnel(options: [String: NSObject]?, completionHandler: @escaping (Error?) -> Void) {
         connectionStatus = .connecting
-
-        // when activated by system "on-demand" the option is set
-        var isOnDemand: Bool {
-            options?[NetworkProtectionOptionKey.isOnDemand] as? Bool == true
-        }
-        var isActivatedFromSystemSettings: Bool {
-            options?[NetworkProtectionOptionKey.activationAttemptId] == nil && !isOnDemand
-        }
+        tunnelHealth.isHavingConnectivityIssues = false
+        controllerErrorStore.lastErrorMessage = nil
 
         let internalCompletionHandler = { [weak self] (error: Error?) in
-            if error != nil {
-                self?.connectionStatus = .disconnected
-                completionHandler(error)
+            guard let error else {
+                completionHandler(nil)
                 return
             }
 
-            completionHandler(nil)
-        }
-
-        tunnelHealth.isHavingConnectivityIssues = false
-        controllerErrorStore.lastErrorMessage = nil
+            let errorDescription = (error as? LocalizedError)?.localizedDescription ?? String(describing: error)
 
-        os_log("🔵 Will load options\n%{public}@", log: .networkProtection, String(describing: options))
-
-        if options?[NetworkProtectionOptionKey.tunnelFailureSimulation] == NetworkProtectionOptionValue.true {
-            internalCompletionHandler(TunnelError.simulateTunnelFailureError)
-            controllerErrorStore.lastErrorMessage = TunnelError.simulateTunnelFailureError.localizedDescription
-            return
-        }
+            os_log("Tunnel startup error: %{public}@", type: .error, errorDescription)
+            self?.controllerErrorStore.lastErrorMessage = errorDescription
+            self?.connectionStatus = .disconnected
 
-        if options?[NetworkProtectionOptionKey.tunnelFatalErrorCrashSimulation] == NetworkProtectionOptionValue.true {
-            simulateTunnelFatalError()
+            completionHandler(error)
         }
 
-        if options?[NetworkProtectionOptionKey.tunnelMemoryCrashSimulation] == NetworkProtectionOptionValue.true {
-            simulateTunnelMemoryOveruse()
-        }
+        os_log("Will load options\n%{public}@", log: .networkProtection, String(describing: options))
+        let startupOptions = StartupOptions(options: options ?? [:], log: .networkProtection)
+        startTunnel(options: startupOptions, completionHandler: internalCompletionHandler)
+    }
 
+    private func startTunnel(options: StartupOptions, completionHandler: @escaping (Error?) -> Void) {
         do {
+            try runDebugSimulations(options: options)
             try load(options: options)
             try loadVendorOptions(from: tunnelProviderProtocol)
         } catch {
-            internalCompletionHandler(error)
+            completionHandler(error)
             return
         }
 
-        os_log("🔵 Done! Starting tunnel from the %{public}@", log: .networkProtection, type: .info, (isActivatedFromSystemSettings ? "settings" : (isOnDemand ? "on-demand" : "app")))
-
-        startTunnel(selectedServer: selectedServerStore.selectedServer, completionHandler: internalCompletionHandler)
+        os_log("Starting tunnel %{public}@", log: .networkProtection, options.startupMethod.debugDescription)
+        startTunnel(selectedServer: selectedServerStore.selectedServer, completionHandler: completionHandler)
     }
 
     private func startTunnel(selectedServer: SelectedNetworkProtectionServer, completionHandler: @escaping (Error?) -> Void) {
@@ -703,7 +726,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
     private func handleSetSelectedServer(_ serverName: String?, completionHandler: ((Data?) -> Void)? = nil) {
         Task {
             guard let serverName else {
-
                 if case .endpoint = selectedServerStore.selectedServer {
                     selectedServerStore.selectedServer = .automatic
                     try? await updateTunnelConfiguration(serverSelectionMethod: .automatic)