Improve PacketTunnelPathObserver

17 files changed, 68 insertions, 143 deletions

diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
index ad6deeb3d7..af1a41f124 100644
--- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift
+++ b/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
     }
diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelPathObserver.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelPathObserver.swift
index 327bcf2042..97066add3d 100644
--- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelPathObserver.swift
+++ b/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()
+            guard started == false else { return }
+            defer { started = true }
+            pathMonitor.pathUpdateHandler = { updatedPath in
+                body(updatedPath.status)
+            }
 
-            // 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)
-                    }
-                }
+            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 {}
diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift
index 5ce1d92eff..93520bd97b 100644
--- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift
+++ b/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
diff --git a/ios/PacketTunnelCore/Actor/NetworkPath+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/NetworkPath+NetworkReachability.swift
index 95557c45a2..4c4cd719f8 100644
--- a/ios/PacketTunnelCore/Actor/NetworkPath+NetworkReachability.swift
+++ b/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
         }
     }
 }
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+ConnectionMonitoring.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+ConnectionMonitoring.swift
index 22bc809f0b..9207543aa1 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+ConnectionMonitoring.swift
+++ b/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
-        }
-    }
 }
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift
index 5a0958b238..46c98c122c 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift
+++ b/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
             )
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift
index 07a0b907eb..989fcc56a3 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift
+++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift
@@ -7,6 +7,7 @@
 //
 
 import Foundation
+import Network
 
 extension PacketTunnelActor {
     /**
@@ -14,16 +15,12 @@ extension PacketTunnelActor {
 
      - 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
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift
index 54ba0e8501..69e1501026 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift
+++ b/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()
     }
 
     /**
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift
index 8c92cbec4f..e55cf1e8cb 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift
+++ b/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
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift
index 7900ada8a4..9cf3833bb8 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift
+++ b/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)
diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift
index 228583602c..e268d6a8b3 100644
--- a/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift
+++ b/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
diff --git a/ios/PacketTunnelCore/TunnelMonitor/DefaultPathObserverProtocol.swift b/ios/PacketTunnelCore/TunnelMonitor/DefaultPathObserverProtocol.swift
index fda89af08d..ea15c85a78 100644
--- a/ios/PacketTunnelCore/TunnelMonitor/DefaultPathObserverProtocol.swift
+++ b/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 }
-}
diff --git a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift
index 778b8ac417..c1160d25f2 100644
--- a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitor.swift
+++ b/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:
diff --git a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorProtocol.swift b/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorProtocol.swift
index 5bc9d8e5f5..c10b9e6f75 100644
--- a/ios/PacketTunnelCore/TunnelMonitor/TunnelMonitorProtocol.swift
+++ b/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)
 }
diff --git a/ios/PacketTunnelCoreTests/Mocks/DefaultPathObserverFake.swift b/ios/PacketTunnelCoreTests/Mocks/DefaultPathObserverFake.swift
index baee852366..c80001b506 100644
--- a/ios/PacketTunnelCoreTests/Mocks/DefaultPathObserverFake.swift
+++ b/ios/PacketTunnelCoreTests/Mocks/DefaultPathObserverFake.swift
@@ -7,46 +7,28 @@
 //
 
 import Foundation
+import Network
 import NetworkExtension
 import PacketTunnelCore
 
-struct NetworkPathStub: NetworkPath {
-    var status: NetworkExtension.NWPathStatus = .satisfied
-}
-
 /// Default path observer fake that uses in-memory storage to keep current path and provides a method to simulate path change from tests.
 class DefaultPathObserverFake: DefaultPathObserverProtocol, @unchecked Sendable {
-    var defaultPath: NetworkPath? {
-        return stateLock.withLock { innerPath }
-    }
-
-    private var innerPath: NetworkPath = NetworkPathStub()
-    private var stateLock = NSLock()
-    private var defaultPathHandler: ((NetworkPath) -> Void)?
+    var currentPathStatus: Network.NWPath.Status { .satisfied }
+    private var defaultPathHandler: ((Network.NWPath.Status) -> Void)?
 
     public var onStart: (() -> Void)?
     public var onStop: (() -> Void)?
 
-    func start(_ body: @escaping (NetworkPath) -> Void) {
-        stateLock.withLock {
-            defaultPathHandler = body
-            onStart?()
-        }
+    func start(_ body: @escaping (Network.NWPath.Status) -> Void) {
+        defaultPathHandler = body
+        onStart?()
     }
 
     func stop() {
-        stateLock.withLock {
-            defaultPathHandler = nil
-            onStop?()
-        }
+        defaultPathHandler = nil
+        onStop?()
     }
 
     /// Simulate network path update.
-    func updatePath(_ newPath: NetworkPath) {
-        let pathHandler = stateLock.withLock {
-            innerPath = newPath
-            return defaultPathHandler
-        }
-        pathHandler?(newPath)
-    }
+    func updatePath(_ newPath: Network.NWPath.Status) {}
 }
diff --git a/ios/PacketTunnelCoreTests/Mocks/TunnelMonitorStub.swift b/ios/PacketTunnelCoreTests/Mocks/TunnelMonitorStub.swift
index 1de36fb64e..7fb2bc1dff 100644
--- a/ios/PacketTunnelCoreTests/Mocks/TunnelMonitorStub.swift
+++ b/ios/PacketTunnelCoreTests/Mocks/TunnelMonitorStub.swift
@@ -64,7 +64,7 @@ class TunnelMonitorStub: TunnelMonitorProtocol, @unchecked Sendable {
 
     func onSleep() {}
 
-    func handleNetworkPathUpdate(_ networkPath: NetworkPath) {}
+    func handleNetworkPathUpdate(_ networkPath: Network.NWPath.Status) {}
 
     func dispatch(_ event: TunnelMonitorEvent, after delay: DispatchTimeInterval = .never) {
         if case .never = delay {
diff --git a/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift b/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift
index 7c200a7156..484e69ffe1 100644
--- a/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift
+++ b/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift
@@ -311,7 +311,6 @@ final class PacketTunnelActorTests: XCTestCase {
 
         let connectedStateExpectation = expectation(description: "Connected state")
         let didStopObserverExpectation = expectation(description: "Did stop path observer")
-        didStopObserverExpectation.expectedFulfillmentCount = 2
         pathObserver.onStop = { didStopObserverExpectation.fulfill() }
 
         let expression: (ObservedState) -> Bool = { if case .connected = $0 { true } else { false } }