From 66336ae26f0f8483cc78390a31edc957562c210b Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Wed, 29 May 2024 08:33:42 +0200 Subject: [PATCH 1/2] Reuse the connection attempts count logic for PQ PSK negotiation --- .../PacketTunnelProvider.swift | 2 +- .../Actor/PacketTunnelActor+PostQuantum.swift | 6 ++-- .../Actor/PacketTunnelActor+Public.swift | 4 +-- .../Actor/PacketTunnelActor.swift | 30 +++++++----------- .../Actor/PacketTunnelActorCommand.swift | 2 +- .../Actor/PacketTunnelActorProtocol.swift | 2 +- .../Actor/PacketTunnelActorReducer.swift | 4 +-- ios/PacketTunnelCore/Actor/State.swift | 10 ++++++ .../IPC/AppMessageHandler.swift | 2 +- .../Mocks/PacketTunnelActorStub.swift | 2 +- .../Mocks/SettingsReaderStub.swift | 14 +++++++++ .../PacketTunnelActorTests.swift | 31 +++++++++++++++++++ 12 files changed, 79 insertions(+), 30 deletions(-) diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift index 70c92a8819d5..08e9c129fe92 100644 --- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift +++ b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift @@ -327,6 +327,6 @@ extension PacketTunnelProvider: PostQuantumKeyReceiving { postQuantumActor.endCurrentNegotiation() // Do not try reconnecting to the `.current` relay, else the actor's `State` equality check will fail // and it will not try to reconnect - actor.reconnect(to: .random) + actor.reconnect(to: .random, reconnectReason: .connectionLoss) } } diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift index d721f571defb..d8a5a8772c50 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+PostQuantum.swift @@ -17,10 +17,10 @@ extension PacketTunnelActor { internal func tryStartPostQuantumNegotiation( withSettings settings: Settings, nextRelay: NextRelay, - reason: ReconnectReason + reason: ActorReconnectReason ) async throws { - if let connectionState = try makeConnectionState(nextRelay: nextRelay, settings: settings, reason: reason) { - let selectedEndpoint = connectionState.selectedRelay.endpoint + if let connectionState = try obfuscateConnection(nextRelay: nextRelay, settings: settings, reason: reason) { + let selectedEndpoint = connectionState.connectedEndpoint let activeKey = activeKey(from: connectionState, in: settings) let configurationBuilder = ConfigurationBuilder( diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift index 545775732708..aaafa317fe5a 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift @@ -38,8 +38,8 @@ extension PacketTunnelActor { - Parameter nextRelay: next relay to connect to. */ - public nonisolated func reconnect(to nextRelay: NextRelay) { - eventChannel.send(.reconnect(nextRelay)) + public nonisolated func reconnect(to nextRelay: NextRelay, reconnectReason: ActorReconnectReason = .userInitiated) { + eventChannel.send(.reconnect(nextRelay, reason: reconnectReason)) } /** diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift index 360bccb46f94..f497df07ae3e 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift @@ -148,16 +148,6 @@ public actor PacketTunnelActor { // MARK: - extension PacketTunnelActor { - /// Describes the reason for reconnection request. - enum ReconnectReason: Equatable { - /// Initiated by user. - case userInitiated - - /// Initiated by tunnel monitor due to loss of connectivity. - /// Actor will increment the connection attempt counter before picking next relay. - case connectionLoss - } - /** Start the tunnel. @@ -221,7 +211,7 @@ extension PacketTunnelActor { - nextRelay: next relay to connect to - reason: reason for reconnect */ - private func reconnect(to nextRelay: NextRelay, reason: ReconnectReason) async { + private func reconnect(to nextRelay: NextRelay, reason: ActorReconnectReason) async { do { switch state { // There is no connection monitoring going on when exchanging keys. @@ -256,7 +246,7 @@ extension PacketTunnelActor { */ private func tryStart( nextRelay: NextRelay, - reason: ReconnectReason = .userInitiated + reason: ActorReconnectReason = .userInitiated ) async throws { let settings: Settings = try settingsReader.read() @@ -284,7 +274,7 @@ extension PacketTunnelActor { private func tryStartConnection( withSettings settings: Settings, nextRelay: NextRelay, - reason: ReconnectReason + reason: ActorReconnectReason ) async throws { guard let connectionState = try obfuscateConnection(nextRelay: nextRelay, settings: settings, reason: reason), let targetState = state.targetStateForReconnect else { return } @@ -341,7 +331,7 @@ extension PacketTunnelActor { internal func makeConnectionState( nextRelay: NextRelay, settings: Settings, - reason: ReconnectReason + reason: ActorReconnectReason ) throws -> State.ConnectionData? { var keyPolicy: State.KeyPolicy = .useCurrent var networkReachability = defaultPathObserver.defaultPath?.networkReachability ?? .undetermined @@ -359,12 +349,11 @@ extension PacketTunnelActor { switch state { case .initial: break - case var .connecting(connectionState), var .reconnecting(connectionState): + // Handle PQ PSK separately as it doesn't interfere with either the `.connecting` or `.reconnecting` states. + case var .negotiatingPostQuantumKey(connectionState, _): if reason == .connectionLoss { connectionState.incrementAttemptCount() } - fallthrough - case var .negotiatingPostQuantumKey(connectionState, _): let selectedRelay = try callRelaySelector( connectionState.selectedRelay, connectionState.connectionAttemptCount @@ -372,6 +361,11 @@ extension PacketTunnelActor { connectionState.selectedRelay = selectedRelay connectionState.relayConstraints = settings.relayConstraints return connectionState + case var .connecting(connectionState), var .reconnecting(connectionState): + if reason == .connectionLoss { + connectionState.incrementAttemptCount() + } + fallthrough case var .connected(connectionState): let selectedRelay = try callRelaySelector( connectionState.selectedRelay, @@ -416,7 +410,7 @@ extension PacketTunnelActor { internal func obfuscateConnection( nextRelay: NextRelay, settings: Settings, - reason: ReconnectReason + reason: ActorReconnectReason ) throws -> State.ConnectionData? { guard let connectionState = try makeConnectionState(nextRelay: nextRelay, settings: settings, reason: reason) else { return nil } diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift index 024431d993ad..21e2aca702f3 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift @@ -19,7 +19,7 @@ extension PacketTunnelActor { case stop /// Reconnect tunnel. - case reconnect(NextRelay, reason: ReconnectReason = .userInitiated) + case reconnect(NextRelay, reason: ActorReconnectReason = .userInitiated) /// Enter blocked state. case error(BlockedStateReason) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActorProtocol.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActorProtocol.swift index 749b30713a9b..df34f768cb80 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActorProtocol.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActorProtocol.swift @@ -11,6 +11,6 @@ import Foundation public protocol PacketTunnelActorProtocol { var observedState: ObservedState { get async } - func reconnect(to nextRelay: NextRelay) + func reconnect(to nextRelay: NextRelay, reconnectReason: ActorReconnectReason) func notifyKeyRotation(date: Date?) } diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift index 338677e6c997..200da62fffab 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift @@ -18,7 +18,7 @@ extension PacketTunnelActor { case stopTunnelMonitor case updateTunnelMonitorPath(NetworkPath) case startConnection(NextRelay) - case restartConnection(NextRelay, ReconnectReason) + case restartConnection(NextRelay, ActorReconnectReason) // trigger a reconnect, which becomes several effects depending on the state case reconnect(NextRelay) case stopTunnelAdapter @@ -123,7 +123,7 @@ extension PacketTunnelActor { fileprivate static func subreducerForReconnect( _ state: State, - _ reason: PacketTunnelActor.ReconnectReason, + _ reason: ActorReconnectReason, _ nextRelay: NextRelay ) -> [PacketTunnelActor.Effect] { switch state { diff --git a/ios/PacketTunnelCore/Actor/State.swift b/ios/PacketTunnelCore/Actor/State.swift index f99799201cea..1afc4ca76804 100644 --- a/ios/PacketTunnelCore/Actor/State.swift +++ b/ios/PacketTunnelCore/Actor/State.swift @@ -238,3 +238,13 @@ public enum NextRelay: Equatable, Codable { /// Use pre-selected relay. case preSelected(SelectedRelay) } + +/// Describes the reason for reconnection request. +public enum ActorReconnectReason: Equatable { + /// Initiated by user. + case userInitiated + + /// Initiated by tunnel monitor due to loss of connectivity, or if post quantum key negotiation times out. + /// Actor will increment the connection attempt counter before picking next relay. + case connectionLoss +} diff --git a/ios/PacketTunnelCore/IPC/AppMessageHandler.swift b/ios/PacketTunnelCore/IPC/AppMessageHandler.swift index 3a852660e135..b70cc56a1dda 100644 --- a/ios/PacketTunnelCore/IPC/AppMessageHandler.swift +++ b/ios/PacketTunnelCore/IPC/AppMessageHandler.swift @@ -54,7 +54,7 @@ public struct AppMessageHandler { return nil case let .reconnectTunnel(nextRelay): - packetTunnelActor.reconnect(to: nextRelay) + packetTunnelActor.reconnect(to: nextRelay, reconnectReason: ActorReconnectReason.userInitiated) return nil } } diff --git a/ios/PacketTunnelCoreTests/Mocks/PacketTunnelActorStub.swift b/ios/PacketTunnelCoreTests/Mocks/PacketTunnelActorStub.swift index 9dbc8f18fc51..1c3c1533ae6e 100644 --- a/ios/PacketTunnelCoreTests/Mocks/PacketTunnelActorStub.swift +++ b/ios/PacketTunnelCoreTests/Mocks/PacketTunnelActorStub.swift @@ -23,7 +23,7 @@ struct PacketTunnelActorStub: PacketTunnelActorProtocol { } } - func reconnect(to nextRelay: NextRelay) { + func reconnect(to nextRelay: PacketTunnelCore.NextRelay, reconnectReason: ActorReconnectReason) { reconnectExpectation?.fulfill() } diff --git a/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift b/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift index f5806b9a47f2..b334cdac65b9 100644 --- a/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift +++ b/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift @@ -38,4 +38,18 @@ extension SettingsReaderStub { return staticSettings } } + + static func postQuantumConfiguration() -> SettingsReaderStub { + let staticSettings = Settings( + privateKey: PrivateKey(), + interfaceAddresses: [IPAddressRange(from: "127.0.0.1/32")!], + relayConstraints: RelayConstraints(), + dnsServers: .gateway, + obfuscation: WireGuardObfuscationSettings(state: .off, port: .automatic), + quantumResistance: .on + ) + return SettingsReaderStub { + return staticSettings + } + } } diff --git a/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift b/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift index 277854ea8fea..33d6657801be 100644 --- a/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift +++ b/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift @@ -123,6 +123,37 @@ final class PacketTunnelActorTests: XCTestCase { await fulfillment(of: [connectingStateExpectation], timeout: 1) } + func testPostQuantumReconnectionTransition() async throws { + let tunnelMonitor = TunnelMonitorStub { _, _ in } + let actor = PacketTunnelActor.mock( + tunnelMonitor: tunnelMonitor, + settingsReader: SettingsReaderStub.postQuantumConfiguration() + ) + let negotiatingPostQuantumKeyStateExpectation = expectation(description: "Expect post quantum state") + negotiatingPostQuantumKeyStateExpectation.expectedFulfillmentCount = 5 + var nextAttemptCount: UInt = 0 + stateSink = await actor.$observedState + .receive(on: DispatchQueue.main) + .sink { newState in + switch newState { + case .initial: + break + case let .negotiatingPostQuantumKey(connState, _): + XCTAssertEqual(connState.connectionAttemptCount, nextAttemptCount) + nextAttemptCount += 1 + negotiatingPostQuantumKeyStateExpectation.fulfill() + if nextAttemptCount < negotiatingPostQuantumKeyStateExpectation.expectedFulfillmentCount { + actor.reconnect(to: .random, reconnectReason: .connectionLoss) + } + default: + XCTFail("Received invalid state: \(newState.name).") + } + } + + actor.start(options: StartOptions(launchSource: .app)) + await fulfillment(of: [negotiatingPostQuantumKeyStateExpectation], timeout: 1) + } + /** Each subsequent re-connection attempt should produce a single change to `state` containing the incremented attempt counter and new relay. .reconnecting (attempt: 0) → .reconnecting (attempt: 1) → .reconnecting (attempt: 2) → ... From 476afeca15373c82a2349ce72a6996b74964a7b0 Mon Sep 17 00:00:00 2001 From: Bug Magnet Date: Tue, 4 Jun 2024 12:58:40 +0200 Subject: [PATCH 2/2] Make the actor reconnect reason explicit --- ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift | 2 +- ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift | 3 ++- ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift index aaafa317fe5a..5ba42a0bd203 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift @@ -38,7 +38,7 @@ extension PacketTunnelActor { - Parameter nextRelay: next relay to connect to. */ - public nonisolated func reconnect(to nextRelay: NextRelay, reconnectReason: ActorReconnectReason = .userInitiated) { + public nonisolated func reconnect(to nextRelay: NextRelay, reconnectReason: ActorReconnectReason) { eventChannel.send(.reconnect(nextRelay, reason: reconnectReason)) } diff --git a/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift b/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift index b334cdac65b9..e202ad04e319 100644 --- a/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift +++ b/ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift @@ -46,7 +46,8 @@ extension SettingsReaderStub { relayConstraints: RelayConstraints(), dnsServers: .gateway, obfuscation: WireGuardObfuscationSettings(state: .off, port: .automatic), - quantumResistance: .on + quantumResistance: .on, + multihopState: .off ) return SettingsReaderStub { return staticSettings diff --git a/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift b/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift index 33d6657801be..539aee36c468 100644 --- a/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift +++ b/ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift @@ -383,7 +383,7 @@ final class PacketTunnelActorTests: XCTestCase { reconnectingStateExpectation.fulfill() } - actor.reconnect(to: .random) + actor.reconnect(to: .random, reconnectReason: .userInitiated) await fulfillment( of: [reconnectingStateExpectation], @@ -414,7 +414,7 @@ final class PacketTunnelActorTests: XCTestCase { let reconnectingState: (ObservedState) -> Bool = { if case .reconnecting = $0 { true } else { false } } await expect(reconnectingState, on: actor) { reconnectingStateExpectation.fulfill() } - actor.reconnect(to: .random) + actor.reconnect(to: .random, reconnectReason: .userInitiated) await fulfillment( of: [reconnectingStateExpectation], timeout: Duration.milliseconds(100).timeInterval @@ -445,7 +445,7 @@ final class PacketTunnelActorTests: XCTestCase { // Cancel the state sink to avoid overfulfilling the connected expectation stateSink?.cancel() - actor.reconnect(to: .random) + actor.reconnect(to: .random, reconnectReason: .userInitiated) await fulfillment(of: [stopMonitorExpectation], timeout: 1) } }