From cf18096a1138ccdfa34de173a08963dbc4d7f632 Mon Sep 17 00:00:00 2001 From: Andrew Bulhak Date: Wed, 20 Mar 2024 22:10:03 +0100 Subject: [PATCH] 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.