Improve PacketTunnelPathObserver

Author: buggmagnet

ios/MullvadVPN/TunnelManager/TunnelManager.swift

@@ -876,7 +876,7 @@ final class TunnelManager: StorePaymentObserver, @unchecked Sendable {
     private func startNetworkMonitor() {
         cancelNetworkMonitor()
 
-        networkMonitor = NWPathMonitor()
+        networkMonitor = NWPathMonitor(prohibitedInterfaceTypes: [.other])
         networkMonitor?.pathUpdateHandler = { [weak self] path in
             self?.didUpdateNetworkPath(path)
         }
@@ -885,6 +885,7 @@ final class TunnelManager: StorePaymentObserver, @unchecked Sendable {
     }
 
     private func cancelNetworkMonitor() {
+        networkMonitor?.pathUpdateHandler = nil
         networkMonitor?.cancel()
         networkMonitor = nil
     }

ios/PacketTunnel/PacketTunnelProvider/PacketTunnelPathObserver.swift

@@ -7,48 +7,50 @@
 //
 
 import Combine
+import MullvadLogging
+import MullvadTypes
+import Network
 import NetworkExtension
 import PacketTunnelCore
 
-final class PacketTunnelPathObserver: DefaultPathObserverProtocol, @unchecked Sendable {
-    private weak var packetTunnelProvider: NEPacketTunnelProvider?
-    private let stateLock = NSLock()
-    private var pathUpdatePublisher: AnyCancellable?
+final class PacketTunnelPathObserver: DefaultPathObserverProtocol, Sendable {
     private let eventQueue: DispatchQueue
+    private let pathMonitor: NWPathMonitor
+    nonisolated(unsafe) let logger = Logger(label: "PacketTunnelPathObserver")
+    private let stateLock = NSLock()
 
-    init(packetTunnelProvider: NEPacketTunnelProvider, eventQueue: DispatchQueue) {
-        self.packetTunnelProvider = packetTunnelProvider
-        self.eventQueue = eventQueue
+    nonisolated(unsafe) private var started = false
+
+    public var currentPathStatus: Network.NWPath.Status {
+        stateLock.withLock {
+            pathMonitor.currentPath.status
+        }
     }
 
-    var defaultPath: NetworkPath? {
-        return packetTunnelProvider?.defaultPath
+    init(eventQueue: DispatchQueue) {
+        self.eventQueue = eventQueue
+
+        pathMonitor = NWPathMonitor(prohibitedInterfaceTypes: [.other])
     }
 
-    func start(_ body: @escaping @Sendable (NetworkPath) -> Void) {
+    func start(_ body: @escaping @Sendable (Network.NWPath.Status) -> Void) {
         stateLock.withLock {
-            pathUpdatePublisher?.cancel()
-
-            // Normally packet tunnel provider should exist throughout the network extension lifetime.
-            pathUpdatePublisher = packetTunnelProvider?.publisher(for: \.defaultPath)
-                .removeDuplicates(by: { oldPath, newPath in
-                    oldPath?.status == newPath?.status
-                })
-                .throttle(for: .seconds(2), scheduler: eventQueue, latest: true)
-                .sink { change in
-                    if let change {
-                        body(change)
-                    }
-                }
+            guard started == false else { return }
+            defer { started = true }
+            pathMonitor.pathUpdateHandler = { updatedPath in
+                body(updatedPath.status)
+            }
+
+            pathMonitor.start(queue: eventQueue)
         }
     }
 
     func stop() {
         stateLock.withLock {
-            pathUpdatePublisher?.cancel()
-            pathUpdatePublisher = nil
+            guard started == true else { return }
+            defer { started = false }
+            pathMonitor.pathUpdateHandler = nil
+            pathMonitor.cancel()
         }
     }
 }
-
-extension NetworkExtension.NWPath: NetworkPath {}

ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift

@@ -91,7 +91,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
             timings: PacketTunnelActorTimings(),
             tunnelAdapter: adapter,
             tunnelMonitor: tunnelMonitor,
-            defaultPathObserver: PacketTunnelPathObserver(packetTunnelProvider: self, eventQueue: internalQueue),
+            defaultPathObserver: PacketTunnelPathObserver(eventQueue: internalQueue),
             blockedStateErrorMapper: BlockedStateErrorMapper(),
             relaySelector: relaySelector,
             settingsReader: TunnelSettingsManager(settingsReader: SettingsReader()) { [weak self] settings in

ios/PacketTunnelCore/Actor/NetworkPath+NetworkReachability.swift

@@ -7,22 +7,20 @@
 //
 
 import Foundation
+import Network
 
-extension NetworkPath {
+extension Network.NWPath.Status {
     /// Converts `NetworkPath.status` into `NetworkReachability`.
     var networkReachability: NetworkReachability {
-        switch status {
-        case .satisfiable, .satisfied:
-            return .reachable
-
+        switch self {
+        case .satisfied:
+            .reachable
         case .unsatisfied:
-            return .unreachable
-
-        case .invalid:
-            return .undetermined
-
+            .unreachable
+        case .requiresConnection:
+            .reachable
         @unknown default:
-            return .undetermined
+            .undetermined
         }
     }
 }

ios/PacketTunnelCore/Actor/PacketTunnelActor+ConnectionMonitoring.swift

@@ -16,45 +16,4 @@ extension PacketTunnelActor {
             self?.eventChannel.send(.monitorEvent(event))
         }
     }
-
-    /**
-     Handle tunnel monitor event.
-
-     Invoked by comand consumer.
-
-     - Important: this method will suspend and must only be invoked as a part of channel consumer to guarantee transactional execution.
-     */
-    func handleMonitorEvent(_ event: TunnelMonitorEvent) async {
-        switch event {
-        case .connectionEstablished:
-            onEstablishConnection()
-
-        case .connectionLost:
-            await onHandleConnectionRecovery()
-        }
-    }
-
-    /// Reset connection attempt counter and update actor state to `connected` state once connection is established.
-    private func onEstablishConnection() {
-        switch state {
-        case var .connecting(connState), var .reconnecting(connState):
-            // Reset connection attempt once successfully connected.
-            connState.connectionAttemptCount = 0
-            state = .connected(connState)
-
-        case .initial, .connected, .disconnecting, .disconnected, .error, .negotiatingEphemeralPeer:
-            break
-        }
-    }
-
-    /// Tell the tunnel to reconnect providing the correct reason to ensure that the attempt counter is incremented before reconnect.
-    private func onHandleConnectionRecovery() async {
-        switch state {
-        case .connecting, .reconnecting, .connected:
-            eventChannel.send(.reconnect(.random, reason: .connectionLoss))
-
-        case .initial, .disconnected, .disconnecting, .error, .negotiatingEphemeralPeer:
-            break
-        }
-    }
 }

ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift

@@ -62,7 +62,7 @@ extension PacketTunnelActor {
                 relayConstraints: nil,
                 currentKey: nil,
                 keyPolicy: .useCurrent,
-                networkReachability: defaultPathObserver.defaultPath?.networkReachability ?? .undetermined,
+                networkReachability: defaultPathObserver.currentPathStatus.networkReachability,
                 recoveryTask: startRecoveryTaskIfNeeded(reason: reason),
                 priorState: .initial
             )

ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift

@@ -7,23 +7,20 @@
 //
 
 import Foundation
+import Network
 
 extension PacketTunnelActor {
     /**
      Start observing changes to default path.
 
      - Parameter notifyObserverWithCurrentPath: immediately notifies path observer with the current path when set to `true`.
      */
-    func startDefaultPathObserver(notifyObserverWithCurrentPath: Bool = false) {
+    func startDefaultPathObserver() {
         logger.trace("Start default path observer.")
 
         defaultPathObserver.start { [weak self] networkPath in
             self?.eventChannel.send(.networkReachability(networkPath))
         }
-
-        if notifyObserverWithCurrentPath, let currentPath = defaultPathObserver.defaultPath {
-            eventChannel.send(.networkReachability(currentPath))
-        }
     }
 
     /// Stop observing changes to default path.
@@ -38,7 +35,7 @@ extension PacketTunnelActor {
 
      - Parameter networkPath: new default path
      */
-    func handleDefaultPathChange(_ networkPath: NetworkPath) {
+    func handleDefaultPathChange(_ networkPath: Network.NWPath.Status) {
         tunnelMonitor.handleNetworkPathUpdate(networkPath)
 
         let newReachability = networkPath.networkReachability

ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift

@@ -36,15 +36,13 @@ extension PacketTunnelActor {
             return
         }
 
-        stopDefaultPathObserver()
-
         state = .connecting(connectionData)
 
         // Resume tunnel monitoring and use IPv4 gateway as a probe address.
         tunnelMonitor.start(probeAddress: connectionData.selectedRelays.exit.endpoint.ipv4Gateway)
         // Restart default path observer and notify the observer with the current path that might have changed while
         // path observer was paused.
-        startDefaultPathObserver(notifyObserverWithCurrentPath: false)
+        startDefaultPathObserver()
     }
 
     /**

ios/PacketTunnelCore/Actor/PacketTunnelActor.swift

@@ -294,17 +294,10 @@ extension PacketTunnelActor {
             connectionData: connectionState
         ).make()
 
-        /*
-         Stop default path observer while updating WireGuard configuration since it will call the system method
-         `NEPacketTunnelProvider.setTunnelNetworkSettings()` which may cause active interfaces to go down making it look
-         like network connectivity is not available, but only for a brief moment.
-         */
-        stopDefaultPathObserver()
-
         defer {
             // Restart default path observer and notify the observer with the current path that might have changed while
             // path observer was paused.
-            startDefaultPathObserver(notifyObserverWithCurrentPath: true)
+            startDefaultPathObserver()
         }
 
         // Daita parameters are gotten from an ephemeral peer
@@ -342,7 +335,7 @@ extension PacketTunnelActor {
         reason: ActorReconnectReason
     ) throws -> State.ConnectionData? {
         var keyPolicy: State.KeyPolicy = .useCurrent
-        var networkReachability = defaultPathObserver.defaultPath?.networkReachability ?? .undetermined
+        var networkReachability = defaultPathObserver.currentPathStatus.networkReachability
         var lastKeyRotation: Date?
 
         let callRelaySelector = { [self] maybeCurrentRelays, connectionCount in

ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift

@@ -8,6 +8,7 @@
 
 import Foundation
 import MullvadTypes
+import Network
 import WireGuardKitTypes
 
 extension PacketTunnelActor {
@@ -35,7 +36,7 @@ extension PacketTunnelActor {
         case monitorEvent(_ event: TunnelMonitorEvent)
 
         /// Network reachability events.
-        case networkReachability(NetworkPath)
+        case networkReachability(Network.NWPath.Status)
 
         /// Update the device private key, as per post-quantum protocols
         case ephemeralPeerNegotiationStateChanged(EphemeralPeerNegotiationState, OneshotChannel)

ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift

@@ -8,6 +8,7 @@
 
 import Foundation
 import MullvadTypes
+import Network
 import WireGuardKitTypes
 
 extension PacketTunnelActor {
@@ -17,7 +18,7 @@ extension PacketTunnelActor {
         case stopDefaultPathObserver
         case startTunnelMonitor
         case stopTunnelMonitor
-        case updateTunnelMonitorPath(NetworkPath)
+        case updateTunnelMonitorPath(Network.NWPath.Status)
         case startConnection(NextRelays)
         case restartConnection(NextRelays, ActorReconnectReason)
 
@@ -39,7 +40,7 @@ extension PacketTunnelActor {
             case (.stopDefaultPathObserver, .stopDefaultPathObserver): true
             case (.startTunnelMonitor, .startTunnelMonitor): true
             case (.stopTunnelMonitor, .stopTunnelMonitor): true
-            case let (.updateTunnelMonitorPath(lp), .updateTunnelMonitorPath(rp)): lp.status == rp.status
+            case let (.updateTunnelMonitorPath(lp), .updateTunnelMonitorPath(rp)): lp == rp
             case let (.startConnection(nr0), .startConnection(nr1)): nr0 == nr1
             case let (.restartConnection(nr0, rr0), .restartConnection(nr1, rr1)): nr0 == nr1 && rr0 == rr1
             case let (.reconnect(nr0), .reconnect(nr1)): nr0 == nr1

ios/PacketTunnelCore/TunnelMonitor/DefaultPathObserverProtocol.swift

@@ -7,22 +7,17 @@
 //
 
 import Foundation
-import NetworkExtension
+import Network
 
 /// A type providing default path access and observation.
 public protocol DefaultPathObserverProtocol: Sendable {
     /// Returns current default path or `nil` if unknown yet.
-    var defaultPath: NetworkPath? { get }
+    var currentPathStatus: Network.NWPath.Status { get }
 
     /// Start observing changes to `defaultPath`.
     /// This call must be idempotent. Multiple calls to start should replace the existing handler block.
-    func start(_ body: @escaping @Sendable (NetworkPath) -> Void)
+    func start(_ body: @escaping @Sendable (Network.NWPath.Status) -> Void)
 
     /// Stop observing changes to `defaultPath`.
     func stop()
 }
-
-/// A type that represents a network path.
-public protocol NetworkPath: Sendable {
-    var status: NetworkExtension.NWPathStatus { get }
-}

ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift

@@ -122,10 +122,9 @@ public final class TunnelMonitor: TunnelMonitorProtocol {
         stopConnectivityCheckTimer()
     }
 
-    public func handleNetworkPathUpdate(_ networkPath: NetworkPath) {
+    public func handleNetworkPathUpdate(_ networkPath: Network.NWPath.Status) {
         nslock.withLock {
-            let pathStatus = networkPath.status
-            let isReachable = pathStatus == .satisfiable || pathStatus == .satisfied
+            let isReachable = networkPath == .satisfied || networkPath == .requiresConnection
 
             switch state.connectionState {
             case .pendingStart:

ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorProtocol.swift

@@ -40,5 +40,5 @@ public protocol TunnelMonitorProtocol: AnyObject, Sendable {
     func onSleep()
 
     /// Handle changes in network path, eg. update connection state and monitoring.
-    func handleNetworkPathUpdate(_ networkPath: NetworkPath)
+    func handleNetworkPathUpdate(_ networkPath: Network.NWPath.Status)
 }