From c09e8decec3473aa4047591f0c3ae57a0d14ab17 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Tue, 19 Mar 2024 17:27:44 +0100 Subject: [PATCH 01/10] Add utility methods for mutating `State`'s data, replacing switch stmts --- .../PacketTunnelProvider.swift | 121 +++++++++--------- .../Actor/PacketTunnelActor+KeyPolicy.swift | 97 +++++++------- ...acketTunnelActor+NetworkReachability.swift | 35 +---- .../Actor/State+Extensions.swift | 59 ++++++++- 4 files changed, 164 insertions(+), 148 deletions(-) diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift index 0195cb16fbfe..0a7db12442fe 100644 --- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift +++ b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift @@ -108,6 +108,9 @@ class PacketTunnelProvider: NEPacketTunnelProvider { if connectionState.connectionAttemptCount > 1 { return } + case let .negotiatingKey(connectionState): + try await startPostQuantumKeyExchange() + return default: break } @@ -123,65 +126,65 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // // try await startPostQuantumKeyExchange() // } -// -// func selectGothenburgRelay() throws -> MullvadEndpoint { -// let constraints = RelayConstraints( -// locations: .only(UserSelectedRelays(locations: [.city("se", "got")])) -// ) -// let relay = try relaySelector.selectRelay(with: constraints, connectionAttemptFailureCount: 0) -// return relay.endpoint -// } -// -// var pqTCPConnection: NWTCPConnection? -// -// func startPostQuantumKeyExchange() async throws { -// let settingsReader = SettingsReader() -// let settings: Settings = try settingsReader.read() -// let privateKey = settings.privateKey -// let postQuantumSharedKey = PrivateKey() // This will become the new private key of the device -// -// let IPv4Gateway = IPv4Address("10.64.0.1")! -// let gothenburgRelay = try selectGothenburgRelay() -// -// let configurationBuilder = ConfigurationBuilder( -// privateKey: settings.privateKey, -// interfaceAddresses: settings.interfaceAddresses, -// dns: settings.dnsServers, -// endpoint: gothenburgRelay, -// allowedIPs: [ -// IPAddressRange(from: "10.64.0.1/8")!, -// ] -// ) -// -// try await adapter.start(configuration: configurationBuilder.makeConfiguration()) -// -// let negotiator = PostQuantumKeyNegotiatior() -// let gatewayEndpoint = NWHostEndpoint(hostname: "10.64.0.1", port: "1337") -// -// pqTCPConnection = createTCPConnectionThroughTunnel( -// to: gatewayEndpoint, -// enableTLS: false, -// tlsParameters: nil, -// delegate: nil -// ) -// guard let pqTCPConnection else { return } -// -// // This will work as long as there is a detached, top-level task here. -// // It might be due to the async runtime environment for `override func startTunnel(options: [String: NSObject]? = nil) async throws` -// // There is a strong chance that the function's async availability was not properly declared by Apple. -// Task.detached { -// for await isViable in pqTCPConnection.viability where isViable == true { -// negotiator.negotiateKey( -// gatewayIP: IPv4Gateway, -// devicePublicKey: privateKey.publicKey, -// presharedKey: postQuantumSharedKey.publicKey, -// packetTunnel: self, -// tcpConnection: self.pqTCPConnection! -// ) -// break -// } -// } -// } + + func selectGothenburgRelay() throws -> MullvadEndpoint { + let constraints = RelayConstraints( + locations: .only(UserSelectedRelays(locations: [.city("se", "got")])) + ) + let relay = try relaySelector.selectRelay(with: constraints, connectionAttemptFailureCount: 0) + return relay.endpoint + } + + var pqTCPConnection: NWTCPConnection? + + func startPostQuantumKeyExchange() async throws { + let settingsReader = SettingsReader() + let settings: Settings = try settingsReader.read() + let privateKey = settings.privateKey + let postQuantumSharedKey = PrivateKey() // This will become the new private key of the device + + let IPv4Gateway = IPv4Address("10.64.0.1")! + let gothenburgRelay = try selectGothenburgRelay() + + let configurationBuilder = ConfigurationBuilder( + privateKey: settings.privateKey, + interfaceAddresses: settings.interfaceAddresses, + dns: settings.dnsServers, + endpoint: gothenburgRelay, + allowedIPs: [ + IPAddressRange(from: "10.64.0.1/8")!, + ] + ) + + try await adapter.start(configuration: configurationBuilder.makeConfiguration()) + + let negotiator = PostQuantumKeyNegotiatior() + let gatewayEndpoint = NWHostEndpoint(hostname: "10.64.0.1", port: "1337") + + pqTCPConnection = createTCPConnectionThroughTunnel( + to: gatewayEndpoint, + enableTLS: false, + tlsParameters: nil, + delegate: nil + ) + guard let pqTCPConnection else { return } + + // This will work as long as there is a detached, top-level task here. + // It might be due to the async runtime environment for `override func startTunnel(options: [String: NSObject]? = nil) async throws` + // There is a strong chance that the function's async availability was not properly declared by Apple. + Task.detached { + for await isViable in pqTCPConnection.viability where isViable == true { + negotiator.negotiateKey( + gatewayIP: IPv4Gateway, + devicePublicKey: privateKey.publicKey, + presharedKey: postQuantumSharedKey.publicKey, + packetTunnel: self, + tcpConnection: self.pqTCPConnection! + ) + break + } + } + } // MARK: - End testing Post Quantum key exchange diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift index 5ab13353e915..91647e994478 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift @@ -23,7 +23,7 @@ extension PacketTunnelActor { - Parameter lastKeyRotation: date when last key rotation took place. */ func cacheActiveKey(lastKeyRotation: Date?) { - func mutateConnectionState(_ connState: inout ConnectionState) -> Bool { + func connectionStateMutator(_ connState: inout ConnectionState) -> Bool { switch connState.keyPolicy { case .useCurrent: if let currentKey = connState.currentKey { @@ -44,23 +44,7 @@ extension PacketTunnelActor { } } - switch state { - case var .connecting(connState): - if mutateConnectionState(&connState) { - state = .connecting(connState) - } - - case var .connected(connState): - if mutateConnectionState(&connState) { - state = .connected(connState) - } - - case var .reconnecting(connState): - if mutateConnectionState(&connState) { - state = .reconnecting(connState) - } - - case var .error(blockedState): + func blockedStateMutator(_ blockedState: inout BlockedState) -> Bool { switch blockedState.keyPolicy { case .useCurrent: // Key policy is preserved between states and key rotation may still happen while in blocked state. @@ -73,16 +57,17 @@ extension PacketTunnelActor { blockedState.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) blockedState.currentKey = nil - state = .error(blockedState) + return true } - case .usePrior: - break + default: + return false } - - case .initial, .disconnected, .disconnecting: - break + return false } + + _ = state.mutateConnectionState(connectionStateMutator) || + state.mutateBlockedState(blockedStateMutator) } /** @@ -123,37 +108,41 @@ extension PacketTunnelActor { - Returns: `true` if the tunnel should reconnect, otherwise `false`. */ private func switchToCurrentKeyInner() -> Bool { - switch state { - case var .connecting(connState): - if setCurrentKeyPolicy(&connState.keyPolicy) { - state = .connecting(connState) - return true - } - - case var .connected(connState): - if setCurrentKeyPolicy(&connState.keyPolicy) { - state = .connected(connState) - return true - } - - case var .reconnecting(connState): - if setCurrentKeyPolicy(&connState.keyPolicy) { - state = .reconnecting(connState) - return true - } - - case var .error(blockedState): - if setCurrentKeyPolicy(&blockedState.keyPolicy) { - state = .error(blockedState) - - // Prevent tunnel from reconnecting when in blocked state. - return false - } - - case .disconnected, .disconnecting, .initial: - break - } + let changed = state.mutateKeyPolicy(setCurrentKeyPolicy) + // Prevent tunnel from reconnecting when in blocked state. + guard case .error = state else { return changed } return false +// switch state { +// case var .connecting(connState): +// if setCurrentKeyPolicy(&connState.keyPolicy) { +// state = .connecting(connState) +// return true +// } +// +// case var .connected(connState): +// if setCurrentKeyPolicy(&connState.keyPolicy) { +// state = .connected(connState) +// return true +// } +// +// case var .reconnecting(connState): +// if setCurrentKeyPolicy(&connState.keyPolicy) { +// state = .reconnecting(connState) +// return true +// } +// +// case var .error(blockedState): +// if setCurrentKeyPolicy(&blockedState.keyPolicy) { +// state = .error(blockedState) +// +// // Prevent tunnel from reconnecting when in blocked state. +// return false +// } +// +// case .disconnected, .disconnecting, .initial: +// break +// } +// return false } /** diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift index 2d3b600239ae..7c44a3c17d45 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift @@ -51,35 +51,14 @@ extension PacketTunnelActor { return false } - switch state { - case var .connecting(connState): - if mutateConnectionState(&connState) { - state = .connecting(connState) + if !state.mutateConnectionState(mutateConnectionState) { + state.mutateBlockedState { blockedState in + if blockedState.networkReachability != newReachability { + blockedState.networkReachability = newReachability + return true + } + return false } - - case var .connected(connState): - if mutateConnectionState(&connState) { - state = .connected(connState) - } - - case var .reconnecting(connState): - if mutateConnectionState(&connState) { - state = .reconnecting(connState) - } - - case var .disconnecting(connState): - if mutateConnectionState(&connState) { - state = .disconnecting(connState) - } - - case var .error(blockedState): - if blockedState.networkReachability != newReachability { - blockedState.networkReachability = newReachability - state = .error(blockedState) - } - - case .initial, .disconnected: - break } } } diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift index dc7580fdf158..ee12bd5af7e8 100644 --- a/ios/PacketTunnelCore/Actor/State+Extensions.swift +++ b/ios/PacketTunnelCore/Actor/State+Extensions.swift @@ -62,21 +62,66 @@ extension State { var name: String { switch self { case .connected: - return "Connected" + "Connected" case .connecting: - return "Connecting" + "Connecting" case .reconnecting: - return "Reconnecting" + "Reconnecting" case .disconnecting: - return "Disconnecting" + "Disconnecting" case .disconnected: - return "Disconnected" + "Disconnected" case .initial: - return "Initial" + "Initial" case .error: - return "Error" + "Error" } } + + /// Apply a mutating function to the connection state if this state has one, and replace its value. If not, this is a no-op. + /// - parameter modifier: A function that takes an `inout ConnectionState` and returns `true`if it has been mutated + /// - returns: `true` if the state's value has been changed + @discardableResult mutating func mutateConnectionState(_ modifier: (inout ConnectionState) -> Bool) -> Bool { + switch self { + case var .connecting(connState): + defer { self = .connecting(connState) } + return modifier(&connState) + case var .connected(connState): + defer { self = .connected(connState) } + return modifier(&connState) + case var .reconnecting(connState): + defer { self = .reconnecting(connState) } + return modifier(&connState) + case var .disconnecting(connState): + defer { self = .disconnecting(connState) } + return modifier(&connState) + default: + return false + } + } + + /// Apply a mutating function to the blocked state if this state has one, and replace its value. If not, this is a no-op. + /// - parameter modifier: A function that takes an `inout ConnectionState` and returns `true`if it has been mutated + /// - returns: `true` if the state's value has been changed + + @discardableResult mutating func mutateBlockedState(_ modifier: (inout BlockedState) -> Bool) -> Bool { + switch self { + case var .error(blockedState): + defer { self = .error(blockedState) } + return modifier(&blockedState) + default: + return false + } + } + + /// Apply a mutating function to the state's key policy + /// - parameter modifier: A function that takes an `inout KeyPolicy` and returns `true`if it has been mutated + /// - returns: `true` if the state's value has been changed + @discardableResult mutating func mutateKeyPolicy(_ modifier: (inout KeyPolicy) -> Bool) -> Bool { + self.mutateConnectionState { modifier(&$0.keyPolicy) } + || self.mutateBlockedState { modifier(&$0.keyPolicy) } + || false + } } extension KeyPolicy { From a083bbd15e8f1f65a694de2b91465f4a4b86232c Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Tue, 19 Mar 2024 17:36:11 +0100 Subject: [PATCH 02/10] Remove premature changes not related to this refactoring --- .../PacketTunnelProvider.swift | 121 +++++++++--------- 1 file changed, 59 insertions(+), 62 deletions(-) diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift index 0a7db12442fe..0195cb16fbfe 100644 --- a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift +++ b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift @@ -108,9 +108,6 @@ class PacketTunnelProvider: NEPacketTunnelProvider { if connectionState.connectionAttemptCount > 1 { return } - case let .negotiatingKey(connectionState): - try await startPostQuantumKeyExchange() - return default: break } @@ -126,65 +123,65 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // // try await startPostQuantumKeyExchange() // } - - func selectGothenburgRelay() throws -> MullvadEndpoint { - let constraints = RelayConstraints( - locations: .only(UserSelectedRelays(locations: [.city("se", "got")])) - ) - let relay = try relaySelector.selectRelay(with: constraints, connectionAttemptFailureCount: 0) - return relay.endpoint - } - - var pqTCPConnection: NWTCPConnection? - - func startPostQuantumKeyExchange() async throws { - let settingsReader = SettingsReader() - let settings: Settings = try settingsReader.read() - let privateKey = settings.privateKey - let postQuantumSharedKey = PrivateKey() // This will become the new private key of the device - - let IPv4Gateway = IPv4Address("10.64.0.1")! - let gothenburgRelay = try selectGothenburgRelay() - - let configurationBuilder = ConfigurationBuilder( - privateKey: settings.privateKey, - interfaceAddresses: settings.interfaceAddresses, - dns: settings.dnsServers, - endpoint: gothenburgRelay, - allowedIPs: [ - IPAddressRange(from: "10.64.0.1/8")!, - ] - ) - - try await adapter.start(configuration: configurationBuilder.makeConfiguration()) - - let negotiator = PostQuantumKeyNegotiatior() - let gatewayEndpoint = NWHostEndpoint(hostname: "10.64.0.1", port: "1337") - - pqTCPConnection = createTCPConnectionThroughTunnel( - to: gatewayEndpoint, - enableTLS: false, - tlsParameters: nil, - delegate: nil - ) - guard let pqTCPConnection else { return } - - // This will work as long as there is a detached, top-level task here. - // It might be due to the async runtime environment for `override func startTunnel(options: [String: NSObject]? = nil) async throws` - // There is a strong chance that the function's async availability was not properly declared by Apple. - Task.detached { - for await isViable in pqTCPConnection.viability where isViable == true { - negotiator.negotiateKey( - gatewayIP: IPv4Gateway, - devicePublicKey: privateKey.publicKey, - presharedKey: postQuantumSharedKey.publicKey, - packetTunnel: self, - tcpConnection: self.pqTCPConnection! - ) - break - } - } - } +// +// func selectGothenburgRelay() throws -> MullvadEndpoint { +// let constraints = RelayConstraints( +// locations: .only(UserSelectedRelays(locations: [.city("se", "got")])) +// ) +// let relay = try relaySelector.selectRelay(with: constraints, connectionAttemptFailureCount: 0) +// return relay.endpoint +// } +// +// var pqTCPConnection: NWTCPConnection? +// +// func startPostQuantumKeyExchange() async throws { +// let settingsReader = SettingsReader() +// let settings: Settings = try settingsReader.read() +// let privateKey = settings.privateKey +// let postQuantumSharedKey = PrivateKey() // This will become the new private key of the device +// +// let IPv4Gateway = IPv4Address("10.64.0.1")! +// let gothenburgRelay = try selectGothenburgRelay() +// +// let configurationBuilder = ConfigurationBuilder( +// privateKey: settings.privateKey, +// interfaceAddresses: settings.interfaceAddresses, +// dns: settings.dnsServers, +// endpoint: gothenburgRelay, +// allowedIPs: [ +// IPAddressRange(from: "10.64.0.1/8")!, +// ] +// ) +// +// try await adapter.start(configuration: configurationBuilder.makeConfiguration()) +// +// let negotiator = PostQuantumKeyNegotiatior() +// let gatewayEndpoint = NWHostEndpoint(hostname: "10.64.0.1", port: "1337") +// +// pqTCPConnection = createTCPConnectionThroughTunnel( +// to: gatewayEndpoint, +// enableTLS: false, +// tlsParameters: nil, +// delegate: nil +// ) +// guard let pqTCPConnection else { return } +// +// // This will work as long as there is a detached, top-level task here. +// // It might be due to the async runtime environment for `override func startTunnel(options: [String: NSObject]? = nil) async throws` +// // There is a strong chance that the function's async availability was not properly declared by Apple. +// Task.detached { +// for await isViable in pqTCPConnection.viability where isViable == true { +// negotiator.negotiateKey( +// gatewayIP: IPv4Gateway, +// devicePublicKey: privateKey.publicKey, +// presharedKey: postQuantumSharedKey.publicKey, +// packetTunnel: self, +// tcpConnection: self.pqTCPConnection! +// ) +// break +// } +// } +// } // MARK: - End testing Post Quantum key exchange From d598397c2aff5ff5031ad2bcd165a99969c8b9aa Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Tue, 19 Mar 2024 17:37:28 +0100 Subject: [PATCH 03/10] Remove commented dead code --- .../Actor/PacketTunnelActor+KeyPolicy.swift | 31 ------------------- 1 file changed, 31 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift index 91647e994478..d8a5336b7b33 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift @@ -112,37 +112,6 @@ extension PacketTunnelActor { // Prevent tunnel from reconnecting when in blocked state. guard case .error = state else { return changed } return false -// switch state { -// case var .connecting(connState): -// if setCurrentKeyPolicy(&connState.keyPolicy) { -// state = .connecting(connState) -// return true -// } -// -// case var .connected(connState): -// if setCurrentKeyPolicy(&connState.keyPolicy) { -// state = .connected(connState) -// return true -// } -// -// case var .reconnecting(connState): -// if setCurrentKeyPolicy(&connState.keyPolicy) { -// state = .reconnecting(connState) -// return true -// } -// -// case var .error(blockedState): -// if setCurrentKeyPolicy(&blockedState.keyPolicy) { -// state = .error(blockedState) -// -// // Prevent tunnel from reconnecting when in blocked state. -// return false -// } -// -// case .disconnected, .disconnecting, .initial: -// break -// } -// return false } /** From 0fcf91caf2d19f79a1f8a9ffc03a5b4ccac9f037 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Wed, 20 Mar 2024 15:48:39 +0100 Subject: [PATCH 04/10] Make State Equatable, move change checks to didSet &c --- .../Actor/PacketTunnelActor.swift | 3 +- .../Actor/State+Extensions.swift | 58 ++++++++++++++----- ios/PacketTunnelCore/Actor/State.swift | 6 +- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift index 18ce82e08dc6..904054240def 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift @@ -27,7 +27,8 @@ import WireGuardKitTypes */ public actor PacketTunnelActor { var state: State = .initial { - didSet { + didSet(oldValue) { + guard state != oldValue else { return } logger.debug("\(state.logFormat())") observedState = state.observedState } diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift index ee12bd5af7e8..927ec41e785b 100644 --- a/ios/PacketTunnelCore/Actor/State+Extensions.swift +++ b/ios/PacketTunnelCore/Actor/State+Extensions.swift @@ -78,26 +78,33 @@ extension State { } } + /// Return a copy of this state with the associated value (if appropriate) replaced with a new value. + /// If the value does not apply, this just returns the state as is, ignoring it. + + internal func replacingConnectionState(with newValue: ConnectionState) -> State { + switch self { + case .connecting: .connecting(newValue) + case .connected: .connected(newValue) + case .reconnecting: .reconnecting(newValue) + case .disconnecting: .disconnecting(newValue) + default: self + } + } + /// Apply a mutating function to the connection state if this state has one, and replace its value. If not, this is a no-op. /// - parameter modifier: A function that takes an `inout ConnectionState` and returns `true`if it has been mutated /// - returns: `true` if the state's value has been changed @discardableResult mutating func mutateConnectionState(_ modifier: (inout ConnectionState) -> Bool) -> Bool { + var modified = false switch self { - case var .connecting(connState): - defer { self = .connecting(connState) } - return modifier(&connState) - case var .connected(connState): - defer { self = .connected(connState) } - return modifier(&connState) - case var .reconnecting(connState): - defer { self = .reconnecting(connState) } - return modifier(&connState) - case var .disconnecting(connState): - defer { self = .disconnecting(connState) } - return modifier(&connState) + case var .connecting(connState), var .connected(connState), var .reconnecting(connState), + var .disconnecting(connState): + modified = modifier(&connState) + self = self.replacingConnectionState(with: connState) default: return false } + return modified } /// Apply a mutating function to the blocked state if this state has one, and replace its value. If not, this is a no-op. @@ -107,8 +114,9 @@ extension State { @discardableResult mutating func mutateBlockedState(_ modifier: (inout BlockedState) -> Bool) -> Bool { switch self { case var .error(blockedState): - defer { self = .error(blockedState) } - return modifier(&blockedState) + let modified = modifier(&blockedState) + self = .error(blockedState) + return modified default: return false } @@ -135,6 +143,16 @@ extension KeyPolicy { } } +extension KeyPolicy: Equatable { + static func == (lhs: KeyPolicy, rhs: KeyPolicy) -> Bool { + switch (lhs, rhs) { + case (.useCurrent, .useCurrent): true + case let (.usePrior(priorA, _), .usePrior(priorB, _)): priorA == priorB + default: false + } + } +} + extension BlockedStateReason { /** Returns true if the tunnel should attempt to restart periodically to recover from error that does not require explicit restart to be initiated by user. @@ -156,3 +174,15 @@ extension BlockedStateReason { } } } + +extension BlockedState: Equatable { + static func == (lhs: BlockedState, rhs: BlockedState) -> Bool { + lhs.reason == rhs.reason + && lhs.relayConstraints == rhs.relayConstraints + && lhs.currentKey == rhs.currentKey + && lhs.keyPolicy == rhs.keyPolicy + && lhs.networkReachability == rhs.networkReachability + && lhs.lastKeyRotation == rhs.lastKeyRotation + && lhs.priorState == rhs.priorState + } +} diff --git a/ios/PacketTunnelCore/Actor/State.swift b/ios/PacketTunnelCore/Actor/State.swift index 3cca82d865d7..9b4cee8a62ac 100644 --- a/ios/PacketTunnelCore/Actor/State.swift +++ b/ios/PacketTunnelCore/Actor/State.swift @@ -54,7 +54,7 @@ import WireGuardKitTypes `.connecting`, `.reconnecting`, `.error` can be interrupted if the tunnel is requested to stop, which should segue actor towards `.disconnected` state. */ -enum State { +enum State: Equatable { /// Initial state at the time when actor is initialized but before the first connection attempt. case initial @@ -97,7 +97,7 @@ public enum NetworkReachability: Equatable, Codable { } /// Data associated with states that hold connection data. -struct ConnectionState { +struct ConnectionState: Equatable { /// Current selected relay. public var selectedRelay: SelectedRelay @@ -206,7 +206,7 @@ public enum BlockedStateReason: String, Codable, Equatable { } /// Legal states that can precede error state. -enum StatePriorToBlockedState { +enum StatePriorToBlockedState: Equatable { case initial, connecting, connected, reconnecting } From e234242a813911693f17455d69a0b9126e59730d Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Wed, 20 Mar 2024 16:23:19 +0100 Subject: [PATCH 05/10] Simplify network reachability change handling --- ...acketTunnelActor+NetworkReachability.swift | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift index 7c44a3c17d45..76b9ba2930f6 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift @@ -43,22 +43,16 @@ extension PacketTunnelActor { let newReachability = networkPath.networkReachability - func mutateConnectionState(_ connState: inout ConnectionState) -> Bool { - if connState.networkReachability != newReachability { - connState.networkReachability = newReachability - return true - } - return false + func connectionStateMutator(_ connState: inout ConnectionState) -> Bool { + connState.networkReachability = newReachability + return true } - if !state.mutateConnectionState(mutateConnectionState) { - state.mutateBlockedState { blockedState in - if blockedState.networkReachability != newReachability { - blockedState.networkReachability = newReachability - return true - } - return false - } + func blockedStateMutator(_ blockedState: inout BlockedState) -> Bool { + blockedState.networkReachability = newReachability + return true } + + _ = state.mutateConnectionState(connectionStateMutator) || state.mutateBlockedState(blockedStateMutator) } } From 6414f9bb1f0b9c69b42f06f6ba781983ef036bb9 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Wed, 20 Mar 2024 21:37:31 +0100 Subject: [PATCH 06/10] Implement protocol for common State connection/error data --- .../Actor/PacketTunnelActor+KeyPolicy.swift | 70 ++++------------- ...acketTunnelActor+NetworkReachability.swift | 12 +-- .../Actor/State+Extensions.swift | 77 +++++++++++-------- ios/PacketTunnelCore/Actor/State.swift | 12 ++- 4 files changed, 73 insertions(+), 98 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift index d8a5336b7b33..3148762abb40 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift @@ -23,51 +23,19 @@ extension PacketTunnelActor { - Parameter lastKeyRotation: date when last key rotation took place. */ func cacheActiveKey(lastKeyRotation: Date?) { - func connectionStateMutator(_ connState: inout ConnectionState) -> Bool { - switch connState.keyPolicy { - case .useCurrent: - if let currentKey = connState.currentKey { - connState.lastKeyRotation = lastKeyRotation - - // Move currentKey into keyPolicy. - connState.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) - connState.currentKey = nil - - return true - } else { - return false - } - - case .usePrior: - // It's unlikely that we'll see subsequent key rotations happen frequently. - return false - } + // There should be a way to rationalise these two identical functions into one. + func connectionStateMutator(_ connState: inout ConnectionOrBlockedState) { + guard connState.keyPolicy == .useCurrent, let currentKey = connState.currentKey else { return } + // Key policy is preserved between states and key rotation may still happen while in blocked state. + // Therefore perform the key switch as normal with one exception that it shouldn't reconnect the tunnel + // automatically. + connState.lastKeyRotation = lastKeyRotation + + // Move currentKey into keyPolicy. + connState.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) + connState.currentKey = nil } - - func blockedStateMutator(_ blockedState: inout BlockedState) -> Bool { - switch blockedState.keyPolicy { - case .useCurrent: - // Key policy is preserved between states and key rotation may still happen while in blocked state. - // Therefore perform the key switch as normal with one exception that it shouldn't reconnect the tunnel - // automatically. - if let currentKey = blockedState.currentKey { - blockedState.lastKeyRotation = lastKeyRotation - - // Move currentKey into keyPolicy. - blockedState.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) - blockedState.currentKey = nil - - return true - } - - default: - return false - } - return false - } - - _ = state.mutateConnectionState(connectionStateMutator) || - state.mutateBlockedState(blockedStateMutator) + state.mutateConnectionOrBlockedState(connectionStateMutator) } /** @@ -108,9 +76,10 @@ extension PacketTunnelActor { - Returns: `true` if the tunnel should reconnect, otherwise `false`. */ private func switchToCurrentKeyInner() -> Bool { - let changed = state.mutateKeyPolicy(setCurrentKeyPolicy) + let oldKeyPolicy = state.keyPolicy + state.mutateKeyPolicy(setCurrentKeyPolicy) // Prevent tunnel from reconnecting when in blocked state. - guard case .error = state else { return changed } + guard case .error = state else { return state.keyPolicy != oldKeyPolicy } return false } @@ -120,14 +89,9 @@ extension PacketTunnelActor { - Parameter keyPolicy: a reference to key policy held either in connection state or blocked state struct. - Returns: `true` when the policy was modified, otherwise `false`. */ - private func setCurrentKeyPolicy(_ keyPolicy: inout KeyPolicy) -> Bool { - switch keyPolicy { - case .useCurrent: - return false - - case .usePrior: + private func setCurrentKeyPolicy(_ keyPolicy: inout KeyPolicy) { + if case .usePrior = keyPolicy { keyPolicy = .useCurrent - return true } } } diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift index 76b9ba2930f6..13eed04e540c 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift @@ -43,16 +43,6 @@ extension PacketTunnelActor { let newReachability = networkPath.networkReachability - func connectionStateMutator(_ connState: inout ConnectionState) -> Bool { - connState.networkReachability = newReachability - return true - } - - func blockedStateMutator(_ blockedState: inout BlockedState) -> Bool { - blockedState.networkReachability = newReachability - return true - } - - _ = state.mutateConnectionState(connectionStateMutator) || state.mutateBlockedState(blockedStateMutator) + state.mutateConnectionOrBlockedState { $0.networkReachability = newReachability } } } diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift index 927ec41e785b..db989686e8a3 100644 --- a/ios/PacketTunnelCore/Actor/State+Extensions.swift +++ b/ios/PacketTunnelCore/Actor/State+Extensions.swift @@ -78,6 +78,32 @@ extension State { } } + var connectionState: ConnectionState? { + switch self { + case + let .connecting(connState), + let .connected(connState), + let .reconnecting(connState), + let .disconnecting(connState): connState + default: nil + } + } + + var blockedState: BlockedState? { + switch self { + case let .error(blockedState): blockedState + default: nil + } + } + + var connectionOrErrorState: ConnectionOrBlockedState? { + self.connectionState ?? self.blockedState + } + + var keyPolicy: KeyPolicy? { + connectionOrErrorState?.keyPolicy + } + /// Return a copy of this state with the associated value (if appropriate) replaced with a new value. /// If the value does not apply, this just returns the state as is, ignoring it. @@ -91,44 +117,31 @@ extension State { } } - /// Apply a mutating function to the connection state if this state has one, and replace its value. If not, this is a no-op. - /// - parameter modifier: A function that takes an `inout ConnectionState` and returns `true`if it has been mutated - /// - returns: `true` if the state's value has been changed - @discardableResult mutating func mutateConnectionState(_ modifier: (inout ConnectionState) -> Bool) -> Bool { - var modified = false - switch self { - case var .connecting(connState), var .connected(connState), var .reconnecting(connState), - var .disconnecting(connState): - modified = modifier(&connState) - self = self.replacingConnectionState(with: connState) - default: - return false - } - return modified - } - - /// Apply a mutating function to the blocked state if this state has one, and replace its value. If not, this is a no-op. - /// - parameter modifier: A function that takes an `inout ConnectionState` and returns `true`if it has been mutated - /// - returns: `true` if the state's value has been changed - - @discardableResult mutating func mutateBlockedState(_ modifier: (inout BlockedState) -> Bool) -> Bool { + /// Apply a mutating function to the connection/error state's associated data if this state has one, + /// and replace its value. If not, this is a no-op. + /// - parameter modifier: A function that takes an `inout ConnectionOrBlockedState` and modifies it + mutating func mutateConnectionOrBlockedState(_ modifier: (inout ConnectionOrBlockedState) -> Void) { switch self { - case var .error(blockedState): - let modified = modifier(&blockedState) - self = .error(blockedState) - return modified + case let .connecting(connState), + let .connected(connState), + let .reconnecting(connState), + let .disconnecting(connState): + var connOrErrorState: ConnectionOrBlockedState = connState + modifier(&connOrErrorState) + self = self.replacingConnectionState(with: connOrErrorState as! ConnectionState) + case let .error(blockedState): + var connOrErrorState: ConnectionOrBlockedState = blockedState + modifier(&connOrErrorState) + self = .error(connOrErrorState as! BlockedState) default: - return false + break } } /// Apply a mutating function to the state's key policy - /// - parameter modifier: A function that takes an `inout KeyPolicy` and returns `true`if it has been mutated - /// - returns: `true` if the state's value has been changed - @discardableResult mutating func mutateKeyPolicy(_ modifier: (inout KeyPolicy) -> Bool) -> Bool { - self.mutateConnectionState { modifier(&$0.keyPolicy) } - || self.mutateBlockedState { modifier(&$0.keyPolicy) } - || false + /// - parameter modifier: A function that takes an `inout KeyPolicy` and modifies it + mutating func mutateKeyPolicy(_ modifier: (inout KeyPolicy) -> Void) { + self.mutateConnectionOrBlockedState { modifier(&$0.keyPolicy) } } } diff --git a/ios/PacketTunnelCore/Actor/State.swift b/ios/PacketTunnelCore/Actor/State.swift index 9b4cee8a62ac..cd007be1932a 100644 --- a/ios/PacketTunnelCore/Actor/State.swift +++ b/ios/PacketTunnelCore/Actor/State.swift @@ -96,8 +96,16 @@ public enum NetworkReachability: Equatable, Codable { case undetermined, reachable, unreachable } +// perhaps this should have a better name? +protocol ConnectionOrBlockedState { + var currentKey: PrivateKey? { get set } + var keyPolicy: KeyPolicy { get set } + var networkReachability: NetworkReachability { get set } + var lastKeyRotation: Date? { get set } +} + /// Data associated with states that hold connection data. -struct ConnectionState: Equatable { +struct ConnectionState: Equatable, ConnectionOrBlockedState { /// Current selected relay. public var selectedRelay: SelectedRelay @@ -138,7 +146,7 @@ struct ConnectionState: Equatable { } /// Data associated with error state. -struct BlockedState { +struct BlockedState: ConnectionOrBlockedState { /// Reason why block state was entered. public var reason: BlockedStateReason From c8df0a4830683f39b959803ec38b6426ec1a4575 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Wed, 20 Mar 2024 22:10:03 +0100 Subject: [PATCH 07/10] Rename State component types for clarity --- .../Actor/ObservedState.swift | 8 +- .../Actor/PacketTunnelActor+ErrorState.swift | 10 +- .../Actor/PacketTunnelActor+KeyPolicy.swift | 16 +- ...acketTunnelActor+NetworkReachability.swift | 2 +- .../Actor/PacketTunnelActor.swift | 8 +- .../Actor/State+Extensions.swift | 34 +++-- ios/PacketTunnelCore/Actor/State.swift | 137 +++++++++--------- 7 files changed, 111 insertions(+), 104 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/ObservedState.swift b/ios/PacketTunnelCore/Actor/ObservedState.swift index 54b6c0d5f5e4..a913894f0e0d 100644 --- a/ios/PacketTunnelCore/Actor/ObservedState.swift +++ b/ios/PacketTunnelCore/Actor/ObservedState.swift @@ -86,8 +86,8 @@ extension State { } } -extension ConnectionState { - /// Map `ConnectionState` to `ObservedConnectionState`. +extension State.ConnectionData { + /// Map `State.ConnectionData` to `ObservedConnectionState`. var observedConnectionState: ObservedConnectionState { ObservedConnectionState( selectedRelay: selectedRelay, @@ -101,8 +101,8 @@ extension ConnectionState { } } -extension BlockedState { - /// Map `BlockedState` to `ObservedBlockedState` +extension State.BlockingData { + /// Map `State.BlockingData` to `ObservedBlockedState` var observedBlockedState: ObservedBlockedState { return ObservedBlockedState(reason: reason, relayConstraints: relayConstraints) } diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift index 94888c81157b..40d0431d1a72 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+ErrorState.swift @@ -54,10 +54,10 @@ extension PacketTunnelActor { - Returns: New blocked state that should be assigned to error state, otherwise `nil` when actor is past or at `disconnecting` phase or when actor is already in the error state and no changes need to be made. */ - private func makeBlockedState(reason: BlockedStateReason) -> BlockedState? { + private func makeBlockedState(reason: BlockedStateReason) -> State.BlockingData? { switch state { case .initial: - return BlockedState( + return State.BlockingData( reason: reason, relayConstraints: nil, currentKey: nil, @@ -93,11 +93,11 @@ extension PacketTunnelActor { Map connection state to blocked state. */ private func mapConnectionState( - _ connState: ConnectionState, + _ connState: State.ConnectionData, reason: BlockedStateReason, priorState: StatePriorToBlockedState - ) -> BlockedState { - BlockedState( + ) -> State.BlockingData { + State.BlockingData( reason: reason, relayConstraints: connState.relayConstraints, currentKey: connState.currentKey, diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift index 3148762abb40..a49afd82a552 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift @@ -24,18 +24,20 @@ extension PacketTunnelActor { */ func cacheActiveKey(lastKeyRotation: Date?) { // There should be a way to rationalise these two identical functions into one. - func connectionStateMutator(_ connState: inout ConnectionOrBlockedState) { - guard connState.keyPolicy == .useCurrent, let currentKey = connState.currentKey else { return } + state.mutateAssociatedData { stateData in + guard + stateData.keyPolicy == .useCurrent, + let currentKey = stateData.currentKey + else { return } // Key policy is preserved between states and key rotation may still happen while in blocked state. // Therefore perform the key switch as normal with one exception that it shouldn't reconnect the tunnel // automatically. - connState.lastKeyRotation = lastKeyRotation - + stateData.lastKeyRotation = lastKeyRotation + // Move currentKey into keyPolicy. - connState.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) - connState.currentKey = nil + stateData.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) + stateData.currentKey = nil } - state.mutateConnectionOrBlockedState(connectionStateMutator) } /** diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift index 13eed04e540c..e60a0081ffb4 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+NetworkReachability.swift @@ -43,6 +43,6 @@ extension PacketTunnelActor { let newReachability = networkPath.networkReachability - state.mutateConnectionOrBlockedState { $0.networkReachability = newReachability } + state.mutateAssociatedData { $0.networkReachability = newReachability } } } diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift index 904054240def..0405b3759222 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor.swift @@ -294,7 +294,7 @@ extension PacketTunnelActor { nextRelay: NextRelay, settings: Settings, reason: ReconnectReason - ) throws -> ConnectionState? { + ) throws -> State.ConnectionData? { var keyPolicy: KeyPolicy = .useCurrent var networkReachability = defaultPathObserver.defaultPath?.networkReachability ?? .undetermined var lastKeyRotation: Date? @@ -333,7 +333,7 @@ extension PacketTunnelActor { return nil } let selectedRelay = try callRelaySelector(nil, 0) - return ConnectionState( + return State.ConnectionData( selectedRelay: selectedRelay, relayConstraints: settings.relayConstraints, currentKey: settings.privateKey, @@ -351,7 +351,7 @@ extension PacketTunnelActor { nextRelay: NextRelay, settings: Settings, reason: ReconnectReason - ) throws -> ConnectionState? { + ) throws -> State.ConnectionData? { guard let connectionState = try makeConnectionState(nextRelay: nextRelay, settings: settings, reason: reason) else { return nil } @@ -362,7 +362,7 @@ extension PacketTunnelActor { ) let transportLayer = protocolObfuscator.transportLayer.map { $0 } ?? .udp - return ConnectionState( + return State.ConnectionData( selectedRelay: connectionState.selectedRelay, relayConstraints: connectionState.relayConstraints, currentKey: settings.privateKey, diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift index db989686e8a3..014c34bb2c4f 100644 --- a/ios/PacketTunnelCore/Actor/State+Extensions.swift +++ b/ios/PacketTunnelCore/Actor/State+Extensions.swift @@ -78,7 +78,7 @@ extension State { } } - var connectionState: ConnectionState? { + var connectionData: State.ConnectionData? { switch self { case let .connecting(connState), @@ -89,25 +89,25 @@ extension State { } } - var blockedState: BlockedState? { + var blockedData: State.BlockingData? { switch self { case let .error(blockedState): blockedState default: nil } } - var connectionOrErrorState: ConnectionOrBlockedState? { - self.connectionState ?? self.blockedState + var associatedData: StateAssociatedData? { + self.connectionData ?? self.blockedData } var keyPolicy: KeyPolicy? { - connectionOrErrorState?.keyPolicy + associatedData?.keyPolicy } /// Return a copy of this state with the associated value (if appropriate) replaced with a new value. /// If the value does not apply, this just returns the state as is, ignoring it. - internal func replacingConnectionState(with newValue: ConnectionState) -> State { + internal func replacingConnectionData(with newValue: State.ConnectionData) -> State { switch self { case .connecting: .connecting(newValue) case .connected: .connected(newValue) @@ -120,19 +120,21 @@ extension State { /// Apply a mutating function to the connection/error state's associated data if this state has one, /// and replace its value. If not, this is a no-op. /// - parameter modifier: A function that takes an `inout ConnectionOrBlockedState` and modifies it - mutating func mutateConnectionOrBlockedState(_ modifier: (inout ConnectionOrBlockedState) -> Void) { + mutating func mutateAssociatedData(_ modifier: (inout StateAssociatedData) -> Void) { switch self { case let .connecting(connState), let .connected(connState), let .reconnecting(connState), let .disconnecting(connState): - var connOrErrorState: ConnectionOrBlockedState = connState - modifier(&connOrErrorState) - self = self.replacingConnectionState(with: connOrErrorState as! ConnectionState) + var associatedData: StateAssociatedData = connState + modifier(&associatedData) + self = self.replacingConnectionData(with: associatedData as! ConnectionData) + case let .error(blockedState): - var connOrErrorState: ConnectionOrBlockedState = blockedState - modifier(&connOrErrorState) - self = .error(connOrErrorState as! BlockedState) + var associatedData: StateAssociatedData = blockedState + modifier(&associatedData) + self = .error(associatedData as! BlockingData) + default: break } @@ -141,7 +143,7 @@ extension State { /// Apply a mutating function to the state's key policy /// - parameter modifier: A function that takes an `inout KeyPolicy` and modifies it mutating func mutateKeyPolicy(_ modifier: (inout KeyPolicy) -> Void) { - self.mutateConnectionOrBlockedState { modifier(&$0.keyPolicy) } + self.mutateAssociatedData { modifier(&$0.keyPolicy) } } } @@ -188,8 +190,8 @@ extension BlockedStateReason { } } -extension BlockedState: Equatable { - static func == (lhs: BlockedState, rhs: BlockedState) -> Bool { +extension State.BlockingData: Equatable { + static func == (lhs: State.BlockingData, rhs: State.BlockingData) -> Bool { lhs.reason == rhs.reason && lhs.relayConstraints == rhs.relayConstraints && lhs.currentKey == rhs.currentKey diff --git a/ios/PacketTunnelCore/Actor/State.swift b/ios/PacketTunnelCore/Actor/State.swift index cd007be1932a..93484eb5c4e8 100644 --- a/ios/PacketTunnelCore/Actor/State.swift +++ b/ios/PacketTunnelCore/Actor/State.swift @@ -60,16 +60,16 @@ enum State: Equatable { /// Tunnel is attempting to connect. /// The actor should remain in this state until the very first connection is established, i.e determined by tunnel monitor. - case connecting(ConnectionState) + case connecting(ConnectionData) /// Tunnel is connected. - case connected(ConnectionState) + case connected(ConnectionData) /// Tunnel is attempting to reconnect. - case reconnecting(ConnectionState) + case reconnecting(ConnectionData) /// Tunnel is disconnecting. - case disconnecting(ConnectionState) + case disconnecting(ConnectionData) /// Tunnel is disconnected. /// Normally the process is shutdown after entering this state. @@ -79,7 +79,7 @@ enum State: Equatable { /// This state is normally entered when the tunnel is unable to start or reconnect. /// In this state the tunnel blocks all nework connectivity by setting up a peerless WireGuard tunnel, and either awaits user action or, in certain /// circumstances, attempts to recover automatically using a repeating timer. - case error(BlockedState) + case error(BlockingData) } /// Policy describing what WG key to use for tunnel communication. @@ -97,85 +97,88 @@ public enum NetworkReachability: Equatable, Codable { } // perhaps this should have a better name? -protocol ConnectionOrBlockedState { +protocol StateAssociatedData { var currentKey: PrivateKey? { get set } var keyPolicy: KeyPolicy { get set } var networkReachability: NetworkReachability { get set } var lastKeyRotation: Date? { get set } } -/// Data associated with states that hold connection data. -struct ConnectionState: Equatable, ConnectionOrBlockedState { - /// Current selected relay. - public var selectedRelay: SelectedRelay - - /// Last relay constraints read from settings. - /// This is primarily used by packet tunnel for updating constraints in tunnel provider. - public var relayConstraints: RelayConstraints - - /// Last WG key read from settings. - /// Can be `nil` if moved to `keyPolicy`. - public var currentKey: PrivateKey? - - /// Policy describing the current key that should be used by the tunnel. - public var keyPolicy: KeyPolicy - - /// Whether network connectivity outside of tunnel is available. - public var networkReachability: NetworkReachability - - /// Connection attempt counter. - /// Reset to zero once connection is established. - public var connectionAttemptCount: UInt - - /// Last time packet tunnel rotated the key. - public var lastKeyRotation: Date? - - /// Increment connection attempt counter by one, wrapping to zero on overflow. - public mutating func incrementAttemptCount() { - let (value, isOverflow) = connectionAttemptCount.addingReportingOverflow(1) - connectionAttemptCount = isOverflow ? 0 : value +extension State { + /// Data associated with states that hold connection data. + struct ConnectionData: Equatable, StateAssociatedData { + /// Current selected relay. + public var selectedRelay: SelectedRelay + + /// Last relay constraints read from settings. + /// This is primarily used by packet tunnel for updating constraints in tunnel provider. + public var relayConstraints: RelayConstraints + + /// Last WG key read from settings. + /// Can be `nil` if moved to `keyPolicy`. + public var currentKey: PrivateKey? + + /// Policy describing the current key that should be used by the tunnel. + public var keyPolicy: KeyPolicy + + /// Whether network connectivity outside of tunnel is available. + public var networkReachability: NetworkReachability + + /// Connection attempt counter. + /// Reset to zero once connection is established. + public var connectionAttemptCount: UInt + + /// Last time packet tunnel rotated the key. + public var lastKeyRotation: Date? + + /// Increment connection attempt counter by one, wrapping to zero on overflow. + public mutating func incrementAttemptCount() { + let (value, isOverflow) = connectionAttemptCount.addingReportingOverflow(1) + connectionAttemptCount = isOverflow ? 0 : value + } + + /// The actual endpoint fed to WireGuard, can be a local endpoint if obfuscation is used. + public let connectedEndpoint: MullvadEndpoint + /// Via which transport protocol was the connection made to the relay + public let transportLayer: TransportLayer + + /// The remote port that was chosen to connect to `connectedEndpoint` + public let remotePort: UInt16 } - /// The actual endpoint fed to WireGuard, can be a local endpoint if obfuscation is used. - public let connectedEndpoint: MullvadEndpoint - /// Via which transport protocol was the connection made to the relay - public let transportLayer: TransportLayer - - /// The remote port that was chosen to connect to `connectedEndpoint` - public let remotePort: UInt16 -} - -/// Data associated with error state. -struct BlockedState: ConnectionOrBlockedState { - /// Reason why block state was entered. - public var reason: BlockedStateReason + /// Data associated with error state. + struct BlockingData: StateAssociatedData { + /// Reason why block state was entered. + public var reason: BlockedStateReason - /// Last relay constraints read from settings. - /// This is primarily used by packet tunnel for updating constraints in tunnel provider. - public var relayConstraints: RelayConstraints? + /// Last relay constraints read from settings. + /// This is primarily used by packet tunnel for updating constraints in tunnel provider. + public var relayConstraints: RelayConstraints? - /// Last WG key read from setings. - /// Can be `nil` if moved to `keyPolicy` or when it's uknown. - public var currentKey: PrivateKey? + /// Last WG key read from setings. + /// Can be `nil` if moved to `keyPolicy` or when it's uknown. + public var currentKey: PrivateKey? - /// Policy describing the current key that should be used by the tunnel. - public var keyPolicy: KeyPolicy + /// Policy describing the current key that should be used by the tunnel. + public var keyPolicy: KeyPolicy - /// Whether network connectivity outside of tunnel is available. - public var networkReachability: NetworkReachability + /// Whether network connectivity outside of tunnel is available. + public var networkReachability: NetworkReachability - /// Last time packet tunnel rotated or attempted to rotate the key. - /// This is used by `TunnelManager` to detect when it needs to refresh device state from Keychain. - public var lastKeyRotation: Date? + /// Last time packet tunnel rotated or attempted to rotate the key. + /// This is used by `TunnelManager` to detect when it needs to refresh device state from Keychain. + public var lastKeyRotation: Date? - /// Task responsible for periodically calling actor to restart the tunnel. - /// Initiated based on the error that led to blocked state. - public var recoveryTask: AutoCancellingTask? + /// Task responsible for periodically calling actor to restart the tunnel. + /// Initiated based on the error that led to blocked state. + public var recoveryTask: AutoCancellingTask? - /// Prior state of the actor before entering blocked state - public var priorState: StatePriorToBlockedState + /// Prior state of the actor before entering blocked state + public var priorState: StatePriorToBlockedState + } } + /// Reason why packet tunnel entered error state. public enum BlockedStateReason: String, Codable, Equatable { /// Device is locked. From 213c112096ad1dfb6286828e7126b742cec6ed86 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Thu, 21 Mar 2024 10:45:53 +0100 Subject: [PATCH 08/10] Remove trailing spaces --- .../Actor/PacketTunnelActor+KeyPolicy.swift | 4 ++-- .../Actor/State+Extensions.swift | 4 ++-- ios/PacketTunnelCore/Actor/State.swift | 19 +++++++++---------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift index a49afd82a552..3084b21676f7 100644 --- a/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift +++ b/ios/PacketTunnelCore/Actor/PacketTunnelActor+KeyPolicy.swift @@ -25,7 +25,7 @@ extension PacketTunnelActor { func cacheActiveKey(lastKeyRotation: Date?) { // There should be a way to rationalise these two identical functions into one. state.mutateAssociatedData { stateData in - guard + guard stateData.keyPolicy == .useCurrent, let currentKey = stateData.currentKey else { return } @@ -33,7 +33,7 @@ extension PacketTunnelActor { // Therefore perform the key switch as normal with one exception that it shouldn't reconnect the tunnel // automatically. stateData.lastKeyRotation = lastKeyRotation - + // Move currentKey into keyPolicy. stateData.keyPolicy = .usePrior(currentKey, startKeySwitchTask()) stateData.currentKey = nil diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift index 014c34bb2c4f..024495c116da 100644 --- a/ios/PacketTunnelCore/Actor/State+Extensions.swift +++ b/ios/PacketTunnelCore/Actor/State+Extensions.swift @@ -129,12 +129,12 @@ extension State { var associatedData: StateAssociatedData = connState modifier(&associatedData) self = self.replacingConnectionData(with: associatedData as! ConnectionData) - + case let .error(blockedState): var associatedData: StateAssociatedData = blockedState modifier(&associatedData) self = .error(associatedData as! BlockingData) - + default: break } diff --git a/ios/PacketTunnelCore/Actor/State.swift b/ios/PacketTunnelCore/Actor/State.swift index 93484eb5c4e8..ff7c09307413 100644 --- a/ios/PacketTunnelCore/Actor/State.swift +++ b/ios/PacketTunnelCore/Actor/State.swift @@ -109,39 +109,39 @@ extension State { struct ConnectionData: Equatable, StateAssociatedData { /// Current selected relay. public var selectedRelay: SelectedRelay - + /// Last relay constraints read from settings. /// This is primarily used by packet tunnel for updating constraints in tunnel provider. public var relayConstraints: RelayConstraints - + /// Last WG key read from settings. /// Can be `nil` if moved to `keyPolicy`. public var currentKey: PrivateKey? - + /// Policy describing the current key that should be used by the tunnel. public var keyPolicy: KeyPolicy - + /// Whether network connectivity outside of tunnel is available. public var networkReachability: NetworkReachability - + /// Connection attempt counter. /// Reset to zero once connection is established. public var connectionAttemptCount: UInt - + /// Last time packet tunnel rotated the key. public var lastKeyRotation: Date? - + /// Increment connection attempt counter by one, wrapping to zero on overflow. public mutating func incrementAttemptCount() { let (value, isOverflow) = connectionAttemptCount.addingReportingOverflow(1) connectionAttemptCount = isOverflow ? 0 : value } - + /// The actual endpoint fed to WireGuard, can be a local endpoint if obfuscation is used. public let connectedEndpoint: MullvadEndpoint /// Via which transport protocol was the connection made to the relay public let transportLayer: TransportLayer - + /// The remote port that was chosen to connect to `connectedEndpoint` public let remotePort: UInt16 } @@ -178,7 +178,6 @@ extension State { } } - /// Reason why packet tunnel entered error state. public enum BlockedStateReason: String, Codable, Equatable { /// Device is locked. From d8af0af28891f2e5e043a9f2de63faa3c2dc0e9c Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Thu, 21 Mar 2024 15:29:46 +0100 Subject: [PATCH 09/10] Remove obsolete comment about StateAssociatedData's name --- ios/PacketTunnelCore/Actor/State.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/ios/PacketTunnelCore/Actor/State.swift b/ios/PacketTunnelCore/Actor/State.swift index ff7c09307413..198ac45c9660 100644 --- a/ios/PacketTunnelCore/Actor/State.swift +++ b/ios/PacketTunnelCore/Actor/State.swift @@ -96,7 +96,6 @@ public enum NetworkReachability: Equatable, Codable { case undetermined, reachable, unreachable } -// perhaps this should have a better name? protocol StateAssociatedData { var currentKey: PrivateKey? { get set } var keyPolicy: KeyPolicy { get set } From 4b580942421b673eb038603825585b1ca6ae0366 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Thu, 21 Mar 2024 15:34:06 +0100 Subject: [PATCH 10/10] Disable swiftlint warning when force-casting Connection/Blocking data --- ios/PacketTunnelCore/Actor/State+Extensions.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ios/PacketTunnelCore/Actor/State+Extensions.swift b/ios/PacketTunnelCore/Actor/State+Extensions.swift index 024495c116da..97d2deb38898 100644 --- a/ios/PacketTunnelCore/Actor/State+Extensions.swift +++ b/ios/PacketTunnelCore/Actor/State+Extensions.swift @@ -128,11 +128,13 @@ extension State { let .disconnecting(connState): var associatedData: StateAssociatedData = connState modifier(&associatedData) + // swiftlint:disable:next force_cast self = self.replacingConnectionData(with: associatedData as! ConnectionData) case let .error(blockedState): var associatedData: StateAssociatedData = blockedState modifier(&associatedData) + // swiftlint:disable:next force_cast self = .error(associatedData as! BlockingData) default: