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

Prevent iOS from stopping the tunnel if it remains in connecting state for too long #5329

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
8 changes: 0 additions & 8 deletions ios/MullvadVPN.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,6 @@
58C7A4702A8649ED0060C66F /* PingerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58C7A46F2A8649ED0060C66F /* PingerTests.swift */; };
58C7AF112ABD8480007EDD7A /* TunnelProviderMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 585DA89226B0323E00B8C587 /* TunnelProviderMessage.swift */; };
58C7AF122ABD8480007EDD7A /* TunnelProviderReply.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5898D2A7290182B000EB5EBA /* TunnelProviderReply.swift */; };
58C7AF132ABD8480007EDD7A /* PacketTunnelStatus.swift in Sources */ = {isa = PBXBuildFile; fileRef = 585DA89826B0329200B8C587 /* PacketTunnelStatus.swift */; };
58C7AF152ABD8480007EDD7A /* PacketTunnelRelay.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5898D2B62902A9EA00EB5EBA /* PacketTunnelRelay.swift */; };
58C7AF162ABD84A8007EDD7A /* URLRequestProxy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58D229B6298D1D5200BB5A2D /* URLRequestProxy.swift */; };
58C7AF172ABD84AA007EDD7A /* ProxyURLRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 063687AF28EB083800BE7161 /* ProxyURLRequest.swift */; };
58C7AF182ABD84AB007EDD7A /* ProxyURLResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5898D2AD290185D200EB5EBA /* ProxyURLResponse.swift */; };
Expand Down Expand Up @@ -1268,7 +1266,6 @@
585CA70E25F8C44600B47C62 /* UIMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIMetrics.swift; sourceTree = "<group>"; };
585DA87626B024A600B8C587 /* CachedRelays.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CachedRelays.swift; sourceTree = "<group>"; };
585DA89226B0323E00B8C587 /* TunnelProviderMessage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelProviderMessage.swift; sourceTree = "<group>"; };
585DA89826B0329200B8C587 /* PacketTunnelStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PacketTunnelStatus.swift; sourceTree = "<group>"; };
585E820227F3285E00939F0E /* SendStoreReceiptOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SendStoreReceiptOperation.swift; sourceTree = "<group>"; };
58607A4C2947287800BC467D /* AccountExpiryInAppNotificationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountExpiryInAppNotificationProvider.swift; sourceTree = "<group>"; };
586168682976F6BD00EF8598 /* DisplayError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DisplayError.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1358,7 +1355,6 @@
5898D2AD290185D200EB5EBA /* ProxyURLResponse.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProxyURLResponse.swift; sourceTree = "<group>"; };
5898D2AF2902A67C00EB5EBA /* RelayLocation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayLocation.swift; sourceTree = "<group>"; };
5898D2B12902A6DE00EB5EBA /* RelayConstraint.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayConstraint.swift; sourceTree = "<group>"; };
5898D2B62902A9EA00EB5EBA /* PacketTunnelRelay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PacketTunnelRelay.swift; sourceTree = "<group>"; };
589A455228E094B300565204 /* OperationsTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = OperationsTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
589C6A7A2A45ACCA00DAD3EF /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
589C6A7B2A45AE0100DAD3EF /* TunnelObfuscation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TunnelObfuscation.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2599,8 +2595,6 @@
children = (
7AEF7F192AD00F52006FE45D /* AppMessageHandler.swift */,
587C575226D2615F005EF767 /* PacketTunnelOptions.swift */,
5898D2B62902A9EA00EB5EBA /* PacketTunnelRelay.swift */,
585DA89826B0329200B8C587 /* PacketTunnelStatus.swift */,
585DA89226B0323E00B8C587 /* TunnelProviderMessage.swift */,
5898D2A7290182B000EB5EBA /* TunnelProviderReply.swift */,
);
Expand Down Expand Up @@ -4206,7 +4200,6 @@
58C7AF112ABD8480007EDD7A /* TunnelProviderMessage.swift in Sources */,
58C7AF162ABD84A8007EDD7A /* URLRequestProxy.swift in Sources */,
58FE25D72AA72A8F003D1918 /* State.swift in Sources */,
58C7AF132ABD8480007EDD7A /* PacketTunnelStatus.swift in Sources */,
58C7A4592A863FB90060C66F /* WgStats.swift in Sources */,
7AD0AA1F2AD6C8B900119E10 /* URLRequestProxyProtocol.swift in Sources */,
7A6B4F592AB8412E00123853 /* TunnelMonitorTimings.swift in Sources */,
Expand All @@ -4223,7 +4216,6 @@
58342C042AAB61FB003BA12D /* State+Extensions.swift in Sources */,
583832272AC3193600EA2071 /* PacketTunnelActor+SleepCycle.swift in Sources */,
58FE25DC2AA72A8F003D1918 /* AnyTask.swift in Sources */,
58C7AF152ABD8480007EDD7A /* PacketTunnelRelay.swift in Sources */,
58FE25D92AA72A8F003D1918 /* AutoCancellingTask.swift in Sources */,
58FE25E12AA72A9B003D1918 /* SettingsReaderProtocol.swift in Sources */,
58C7A4582A863FB90060C66F /* TunnelMonitorProtocol.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import RelayCache
import RelaySelector

final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
private var observedState: ObservedState = .disconnected
private var selectedRelay: SelectedRelay?
private let urlRequestProxy: URLRequestProxy
private let relayCacheTracker: RelayCacheTracker
Expand All @@ -39,15 +40,20 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
options: [String: NSObject]?,
completionHandler: @escaping (Error?) -> Void
) {
dispatchQueue.async {
dispatchQueue.async { [weak self] in
guard let self else {
completionHandler(nil)
return
}

var selectedRelay: SelectedRelay?

do {
let tunnelOptions = PacketTunnelOptions(rawOptions: options ?? [:])

selectedRelay = try tunnelOptions.getSelectedRelay()
} catch {
self.providerLogger.error(
providerLogger.error(
error: error,
message: """
Failed to decode selected relay passed from the app. \
Expand All @@ -57,11 +63,10 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
}

do {
self.selectedRelay = try selectedRelay ?? self.pickRelay()

setInternalStateConnected(with: try selectedRelay ?? pickRelay())
completionHandler(nil)
} catch {
self.providerLogger.error(
providerLogger.error(
error: error,
message: "Failed to pick relay."
)
Expand All @@ -71,8 +76,9 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
}

override func stopTunnel(with reason: NEProviderStopReason, completionHandler: @escaping () -> Void) {
dispatchQueue.async {
self.selectedRelay = nil
dispatchQueue.async { [weak self] in
self?.selectedRelay = nil
self?.observedState = .disconnected

completionHandler()
}
Expand All @@ -98,12 +104,9 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
private func handleProviderMessage(_ message: TunnelProviderMessage, completionHandler: ((Data?) -> Void)?) {
switch message {
case .getTunnelStatus:
var tunnelStatus = PacketTunnelStatus()
tunnelStatus.tunnelRelay = self.selectedRelay?.packetTunnelRelay

var reply: Data?
do {
reply = try TunnelProviderReply(tunnelStatus).encode()
reply = try TunnelProviderReply(observedState).encode()
} catch {
self.providerLogger.error(
error: error,
Expand All @@ -115,6 +118,7 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {

case let .reconnectTunnel(nextRelay):
reasserting = true

switch nextRelay {
case let .preSelected(selectedRelay):
self.selectedRelay = selectedRelay
Expand All @@ -125,7 +129,10 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
case .current:
break
}

setInternalStateConnected(with: selectedRelay)
reasserting = false

completionHandler?(nil)

case let .sendURLRequest(proxyRequest):
Expand Down Expand Up @@ -166,6 +173,26 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
location: selectorResult.location
)
}

private func setInternalStateConnected(with selectedRelay: SelectedRelay?) {
guard let selectedRelay = selectedRelay else { return }

do {
observedState = .connected(
ObservedConnectionState(
selectedRelay: selectedRelay,
relayConstraints: try SettingsManager.readSettings().relayConstraints,
networkReachability: .reachable,
connectionAttemptCount: 0
)
)
} catch {
providerLogger.error(
error: error,
message: "Failed to read device settings."
)
}
}
}

#endif
83 changes: 28 additions & 55 deletions ios/MullvadVPN/TunnelManager/MapConnectionStatusOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,25 @@ class MapConnectionStatusOperation: AsyncOperation {
let tunnelState = interactor.tunnelStatus.state

switch connectionStatus {
case .connecting:
handleConnectingState(tunnelState, tunnel)
return

case .reasserting:
fetchTunnelStatus(tunnel: tunnel) { packetTunnelStatus in
if let blockedStateReason = packetTunnelStatus.blockedStateReason {
return .error(blockedStateReason)
} else if packetTunnelStatus.isNetworkReachable {
return packetTunnelStatus.tunnelRelay.map { .reconnecting($0) }
} else {
return .waitingForConnectivity(.noConnection)
}
}
return

case .connected:
fetchTunnelStatus(tunnel: tunnel) { packetTunnelStatus in
if let blockedStateReason = packetTunnelStatus.blockedStateReason {
return .error(blockedStateReason)
} else if packetTunnelStatus.isNetworkReachable {
return packetTunnelStatus.tunnelRelay.map { .connected($0) }
} else {
return .waitingForConnectivity(.noConnection)
case .connecting, .reasserting, .connected:
fetchTunnelStatus(tunnel: tunnel) { observedState in
switch observedState {
case let .connected(connectionState):
return connectionState.isNetworkReachable
? .connected(connectionState.selectedRelay)
: .waitingForConnectivity(.noConnection)
case let .connecting(connectionState):
return connectionState.isNetworkReachable
? .connecting(connectionState.selectedRelay)
: .waitingForConnectivity(.noConnection)
case let .reconnecting(connectionState):
return connectionState.isNetworkReachable
? .reconnecting(connectionState.selectedRelay)
: .waitingForConnectivity(.noConnection)
case let .error(blockedState):
return .error(blockedState.reason)
case .initial, .disconnecting, .disconnected:
return .none
}
}
return
Expand All @@ -78,7 +73,7 @@ class MapConnectionStatusOperation: AsyncOperation {
handleDisconnectedState(tunnelState)

case .disconnecting:
handleDisconnectionState(tunnelState)
handleDisconnectingState(tunnelState)

case .invalid:
setTunnelDisconnectedStatus()
Expand All @@ -94,38 +89,16 @@ class MapConnectionStatusOperation: AsyncOperation {
request?.cancel()
}

private func handleConnectingState(_ tunnelState: TunnelState, _ tunnel: any TunnelProtocol) {
switch tunnelState {
case .connecting:
break

default:
interactor.updateTunnelStatus { tunnelStatus in
tunnelStatus.state = .connecting(nil)
}
}

fetchTunnelStatus(tunnel: tunnel) { packetTunnelStatus in
if let blockedStateReason = packetTunnelStatus.blockedStateReason {
return .error(blockedStateReason)
} else if packetTunnelStatus.isNetworkReachable {
return packetTunnelStatus.tunnelRelay.map { .connecting($0) }
} else {
return .waitingForConnectivity(.noConnection)
}
}
}

private func handleDisconnectionState(_ tunnelState: TunnelState) {
private func handleDisconnectingState(_ tunnelState: TunnelState) {
switch tunnelState {
case .disconnecting:
break
default:
interactor.updateTunnelStatus { tunnelStatus in
let packetTunnelStatus = tunnelStatus.packetTunnelStatus
let isNetworkReachable = tunnelStatus.observedState.connectionState?.isNetworkReachable ?? false

tunnelStatus = TunnelStatus()
tunnelStatus.state = packetTunnelStatus.isNetworkReachable
tunnelStatus.state = isNetworkReachable
? .disconnecting(.nothing)
: .waitingForConnectivity(.noNetwork)
}
Expand Down Expand Up @@ -161,17 +134,17 @@ class MapConnectionStatusOperation: AsyncOperation {

private func fetchTunnelStatus(
tunnel: any TunnelProtocol,
mapToState: @escaping (PacketTunnelStatus) -> TunnelState?
mapToState: @escaping (ObservedState) -> TunnelState?
) {
request = tunnel.getTunnelStatus { [weak self] completion in
request = tunnel.getTunnelStatus { [weak self] result in
guard let self else { return }

dispatchQueue.async {
if case let .success(packetTunnelStatus) = completion, !self.isCancelled {
if case let .success(observedState) = result, !self.isCancelled {
self.interactor.updateTunnelStatus { tunnelStatus in
tunnelStatus.packetTunnelStatus = packetTunnelStatus
tunnelStatus.observedState = observedState

if let newState = mapToState(packetTunnelStatus) {
if let newState = mapToState(observedState) {
tunnelStatus.state = newState
}
}
Expand Down
3 changes: 1 addition & 2 deletions ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ class StartTunnelOperation: ResultOperation<Void> {

interactor.updateTunnelStatus { tunnelStatus in
tunnelStatus = TunnelStatus()
tunnelStatus.packetTunnelStatus.tunnelRelay = selectedRelay?.packetTunnelRelay
tunnelStatus.state = .connecting(selectedRelay?.packetTunnelRelay)
tunnelStatus.state = .connecting(selectedRelay)
}

try tunnel.start(options: tunnelOptions.rawOptions())
Expand Down
2 changes: 1 addition & 1 deletion ios/MullvadVPN/TunnelManager/Tunnel+Messaging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ extension TunnelProtocol {

/// Request status from packet tunnel process.
func getTunnelStatus(
completionHandler: @escaping (Result<PacketTunnelStatus, Error>) -> Void
completionHandler: @escaping (Result<ObservedState, Error>) -> Void
) -> Cancellable {
let operation = SendTunnelProviderMessageOperation(
dispatchQueue: dispatchQueue,
Expand Down
4 changes: 2 additions & 2 deletions ios/MullvadVPN/TunnelManager/TunnelManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ final class TunnelManager: StorePaymentObserver {

// Packet tunnel may have attempted or rotated the key.
// In that case we have to reload device state from Keychain as it's likely was modified by packet tunnel.
let newPacketTunnelKeyRotation = newTunnelStatus.packetTunnelStatus.lastKeyRotation
let newPacketTunnelKeyRotation = newTunnelStatus.observedState.connectionState?.lastKeyRotation
if lastPacketTunnelKeyRotation != newPacketTunnelKeyRotation {
lastPacketTunnelKeyRotation = newPacketTunnelKeyRotation
refreshDeviceState()
Expand Down Expand Up @@ -816,7 +816,7 @@ final class TunnelManager: StorePaymentObserver {
let selectorResult = try RelaySelector.evaluate(
relays: cachedRelays.relays,
constraints: settings.relayConstraints,
numberOfFailedAttempts: tunnelStatus.packetTunnelStatus.numberOfFailedAttempts
numberOfFailedAttempts: tunnelStatus.observedState.connectionState?.connectionAttemptCount ?? 0
)

return SelectedRelay(
Expand Down
20 changes: 12 additions & 8 deletions ios/MullvadVPN/TunnelManager/TunnelState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,22 @@ import PacketTunnelCore
/// A struct describing the tunnel status.
struct TunnelStatus: Equatable, CustomStringConvertible {
/// Tunnel status returned by tunnel process.
var packetTunnelStatus = PacketTunnelStatus()
var observedState: ObservedState = .disconnected

/// Tunnel state.
var state: TunnelState = .disconnected

var description: String {
var s = "\(state), network "

if packetTunnelStatus.isNetworkReachable {
s += "reachable"
if let connectionState = observedState.connectionState {
if connectionState.isNetworkReachable {
s += "reachable"
} else {
s += "unreachable"
}
} else {
s += "unreachable"
s += "reachability unknown"
}

return s
Expand All @@ -44,10 +48,10 @@ enum TunnelState: Equatable, CustomStringConvertible {
case pendingReconnect

/// Connecting the tunnel.
case connecting(PacketTunnelRelay?)
case connecting(SelectedRelay?)

/// Connected the tunnel
case connected(PacketTunnelRelay)
case connected(SelectedRelay)

/// Disconnecting the tunnel
case disconnecting(ActionAfterDisconnect)
Expand All @@ -60,7 +64,7 @@ enum TunnelState: Equatable, CustomStringConvertible {
/// 1. Asking the running tunnel to reconnect to new relay via IPC.
/// 2. Tunnel attempts to reconnect to new relay as the current relay appears to be
/// dysfunctional.
case reconnecting(PacketTunnelRelay)
case reconnecting(SelectedRelay)

/// Waiting for connectivity to come back up.
case waitingForConnectivity(WaitingForConnectionReason)
Expand Down Expand Up @@ -102,7 +106,7 @@ enum TunnelState: Equatable, CustomStringConvertible {
}
}

var relay: PacketTunnelRelay? {
var relay: SelectedRelay? {
switch self {
case let .connected(relay), let .reconnecting(relay):
return relay
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ final class TunnelControlView: UIView {
)

connectionPanel.dataSource = ConnectionPanelData(
inAddress: "\(tunnelRelay.ipv4Relay) UDP",
inAddress: "\(tunnelRelay.endpoint.ipv4Relay) UDP",
outAddress: nil
)
connectionPanel.isHidden = false
Expand Down
Loading
Loading