Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse the connection attempts count logic for PQ PSK negotiation #6295

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
private var adapter: WgAdapter!
private var relaySelector: RelaySelectorWrapper!

override init() {

Check warning on line 37 in ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift

View workflow job for this annotation

GitHub Actions / End to end tests

Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 53 lines (function_body_length)

Check warning on line 37 in ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift

View workflow job for this annotation

GitHub Actions / End to end tests

Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 53 lines (function_body_length)
Self.configureLogging()

providerLogger = Logger(label: "PacketTunnelProvider")
Expand Down Expand Up @@ -327,6 +327,6 @@
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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions ios/PacketTunnelCore/Actor/PacketTunnelActor+Public.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
eventChannel.send(.reconnect(nextRelay, reason: reconnectReason))
}

/**
Expand Down
30 changes: 12 additions & 18 deletions ios/PacketTunnelCore/Actor/PacketTunnelActor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,6 @@
// 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.

Expand Down Expand Up @@ -221,7 +211,7 @@
- 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.
Expand Down Expand Up @@ -256,7 +246,7 @@
*/
private func tryStart(
nextRelay: NextRelay,
reason: ReconnectReason = .userInitiated
reason: ActorReconnectReason = .userInitiated
) async throws {
let settings: Settings = try settingsReader.read()

Expand Down Expand Up @@ -284,7 +274,7 @@
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 }
Expand Down Expand Up @@ -338,10 +328,10 @@

- Returns: New connection state or `nil` if current state is at or past `.disconnecting` phase.
*/
internal func makeConnectionState(

Check warning on line 331 in ios/PacketTunnelCore/Actor/PacketTunnelActor.swift

View workflow job for this annotation

GitHub Actions / End to end tests

Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 60 lines (function_body_length)
nextRelay: NextRelay,
settings: Settings,
reason: ReconnectReason
reason: ActorReconnectReason
) throws -> State.ConnectionData? {
var keyPolicy: State.KeyPolicy = .useCurrent
var networkReachability = defaultPathObserver.defaultPath?.networkReachability ?? .undetermined
Expand All @@ -359,19 +349,23 @@
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
)
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,
Expand Down Expand Up @@ -416,7 +410,7 @@
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 }
Expand Down
2 changes: 1 addition & 1 deletion ios/PacketTunnelCore/Actor/PacketTunnelActorCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion ios/PacketTunnelCore/Actor/PacketTunnelActorProtocol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?)
}
4 changes: 2 additions & 2 deletions ios/PacketTunnelCore/Actor/PacketTunnelActorReducer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -123,7 +123,7 @@ extension PacketTunnelActor {

fileprivate static func subreducerForReconnect(
_ state: State,
_ reason: PacketTunnelActor.ReconnectReason,
_ reason: ActorReconnectReason,
_ nextRelay: NextRelay
) -> [PacketTunnelActor.Effect] {
switch state {
Expand Down
10 changes: 10 additions & 0 deletions ios/PacketTunnelCore/Actor/State.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion ios/PacketTunnelCore/IPC/AppMessageHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct PacketTunnelActorStub: PacketTunnelActorProtocol {
}
}

func reconnect(to nextRelay: NextRelay) {
func reconnect(to nextRelay: PacketTunnelCore.NextRelay, reconnectReason: ActorReconnectReason) {
reconnectExpectation?.fulfill()
}

Expand Down
15 changes: 15 additions & 0 deletions ios/PacketTunnelCoreTests/Mocks/SettingsReaderStub.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,19 @@ 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,
multihopState: .off
)
return SettingsReaderStub {
return staticSettings
}
}
}
37 changes: 34 additions & 3 deletions ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,37 @@
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) → ...
Expand Down Expand Up @@ -176,7 +207,7 @@
3. The issue goes away on the second attempt to read settings.
4. An actor should transition through `.connecting` towards`.connected` state.
*/
func testLockedDeviceErrorOnBoot() async throws {

Check warning on line 210 in ios/PacketTunnelCoreTests/PacketTunnelActorTests.swift

View workflow job for this annotation

GitHub Actions / End to end tests

Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 52 lines (function_body_length)
let initialStateExpectation = expectation(description: "Expect initial state")
let errorStateExpectation = expectation(description: "Expect error state")
let connectingStateExpectation = expectation(description: "Expect connecting state")
Expand Down Expand Up @@ -352,7 +383,7 @@
reconnectingStateExpectation.fulfill()
}

actor.reconnect(to: .random)
actor.reconnect(to: .random, reconnectReason: .userInitiated)

await fulfillment(
of: [reconnectingStateExpectation],
Expand Down Expand Up @@ -383,7 +414,7 @@
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
Expand Down Expand Up @@ -414,7 +445,7 @@
// 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)
}
}
Expand Down
Loading