Merge branch 'fix-port-selection-algorithm-ios-142'

8 files changed, 94 insertions, 26 deletions

diff --git a/ios/MullvadTypes/PacketTunnelStatus.swift b/ios/MullvadTypes/PacketTunnelStatus.swift
index 6f38c25c78..bceed0c2fe 100644
--- a/ios/MullvadTypes/PacketTunnelStatus.swift
+++ b/ios/MullvadTypes/PacketTunnelStatus.swift
@@ -46,15 +46,20 @@ public struct PacketTunnelStatus: Codable, Equatable {
     /// Current relay.
     public var tunnelRelay: PacketTunnelRelay?
 
+    /// Number of consecutive connection failure attempts.
+    public var numberOfFailedAttempts: UInt
+
     public init(
         lastErrors: [PacketTunnelErrorWrapper] = [],
         isNetworkReachable: Bool = true,
         deviceCheck: DeviceCheck? = nil,
-        tunnelRelay: PacketTunnelRelay? = nil
+        tunnelRelay: PacketTunnelRelay? = nil,
+        numberOfFailedAttempts: UInt = 0
     ) {
         self.lastErrors = lastErrors
         self.isNetworkReachable = isNetworkReachable
         self.deviceCheck = deviceCheck
         self.tunnelRelay = tunnelRelay
+        self.numberOfFailedAttempts = numberOfFailedAttempts
     }
 }
diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
index 0b44c7f84a..a9cc31cb51 100644
--- a/ios/MullvadVPN.xcodeproj/project.pbxproj
+++ b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -1192,29 +1192,29 @@
 		581943F228F8014500B0CB5E /* MullvadTypes */ = {
 			isa = PBXGroup;
 			children = (
-				58561C98239A5D1500BD6B5E /* IPv4Endpoint.swift */,
-				586A95112901321B007BAF2B /* IPv6Endpoint.swift */,
-				586A951329013235007BAF2B /* AnyIPEndpoint.swift */,
 				584D26BE270C550B004EA533 /* AnyIPAddress.swift */,
-				06AC115628F848D00037AF9A /* IPAddress+Codable.swift */,
+				586A951329013235007BAF2B /* AnyIPEndpoint.swift */,
 				06AC113628F83FD70037AF9A /* Cancellable.swift */,
-				58E511E028DDB7F100B0BCDE /* WrappingError.swift */,
 				58E511E328DDDE8900B0BCDE /* CustomErrorDescriptionProtocol.swift */,
-				58E511EA28DDE18400B0BCDE /* Error+Chain.swift */,
 				586168682976F6BD00EF8598 /* DisplayError.swift */,
+				58E511EA28DDE18400B0BCDE /* Error+Chain.swift */,
+				58900D0228BBDCC70094E4F0 /* FixedWidthInteger+Arithmetics.swift */,
+				06AC115628F848D00037AF9A /* IPAddress+Codable.swift */,
+				58561C98239A5D1500BD6B5E /* IPv4Endpoint.swift */,
+				586A95112901321B007BAF2B /* IPv6Endpoint.swift */,
 				58AEEF642344A36000C9BBD5 /* KeychainError.swift */,
 				58A1AA8623F43901009F7EA6 /* Location.swift */,
 				5840250322B11AB700E4CFEC /* MullvadEndpoint.swift */,
+				58D223D7294C8E5E0029F5F8 /* MullvadTypes.h */,
+				06410E172934F43B00AFC18C /* PacketTunnelErrorWrapper.swift */,
+				5898D2B62902A9EA00EB5EBA /* PacketTunnelRelay.swift */,
+				585DA89826B0329200B8C587 /* PacketTunnelStatus.swift */,
+				58CAFA01298530DC00BE19F7 /* Promise.swift */,
 				5898D2B12902A6DE00EB5EBA /* RelayConstraint.swift */,
 				58781CC822AE7CA8009B9D8E /* RelayConstraints.swift */,
 				5898D2AF2902A67C00EB5EBA /* RelayLocation.swift */,
-				585DA89826B0329200B8C587 /* PacketTunnelStatus.swift */,
-				5898D2B62902A9EA00EB5EBA /* PacketTunnelRelay.swift */,
-				58900D0228BBDCC70094E4F0 /* FixedWidthInteger+Arithmetics.swift */,
-				06410E172934F43B00AFC18C /* PacketTunnelErrorWrapper.swift */,
-				58CAFA01298530DC00BE19F7 /* Promise.swift */,
 				58F1311427E0B2AB007AC5BC /* Result+Extensions.swift */,
-				58D223D7294C8E5E0029F5F8 /* MullvadTypes.h */,
+				58E511E028DDB7F100B0BCDE /* WrappingError.swift */,
 			);
 			path = MullvadTypes;
 			sourceTree = "<group>";
diff --git a/ios/MullvadVPN/SettingsManager/SettingsManager.swift b/ios/MullvadVPN/SettingsManager/SettingsManager.swift
index 96bed23c45..795175ea06 100644
--- a/ios/MullvadVPN/SettingsManager/SettingsManager.swift
+++ b/ios/MullvadVPN/SettingsManager/SettingsManager.swift
@@ -444,7 +444,7 @@ enum SettingsKey: String, CaseIterable {
     case settings = "Settings"
     case deviceState = "DeviceState"
     case lastUsedAccount = "LastUsedAccount"
-    case shouldWipeSettings = "shouldWipeSettings"
+    case shouldWipeSettings = "ShouldWipeSettings"
 }
 
 /// An error type describing a failure to read or parse settings version.
diff --git a/ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProviderHost.swift b/ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProviderHost.swift
index 3c2c8f85d0..7d2331b92d 100644
--- a/ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProviderHost.swift
+++ b/ios/MullvadVPN/SimulatorTunnelProvider/SimulatorTunnelProviderHost.swift
@@ -155,7 +155,8 @@ final class SimulatorTunnelProviderHost: SimulatorTunnelProviderDelegate {
 
         return try RelaySelector.evaluate(
             relays: cachedRelays.relays,
-            constraints: tunnelSettings.relayConstraints
+            constraints: tunnelSettings.relayConstraints,
+            numberOfFailedAttempts: 0
         )
     }
 }
diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
index 3dcb412734..bdf1dc5369 100644
--- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift
+++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift
@@ -753,7 +753,8 @@ final class TunnelManager: StorePaymentObserver {
 
         return try RelaySelector.evaluate(
             relays: cachedRelays.relays,
-            constraints: settings.relayConstraints
+            constraints: settings.relayConstraints,
+            numberOfFailedAttempts: tunnelStatus.packetTunnelStatus.numberOfFailedAttempts
         )
     }
 
diff --git a/ios/MullvadVPNTests/RelaySelectorTests.swift b/ios/MullvadVPNTests/RelaySelectorTests.swift
index 4b55c6647f..f0276f5ba0 100644
--- a/ios/MullvadVPNTests/RelaySelectorTests.swift
+++ b/ios/MullvadVPNTests/RelaySelectorTests.swift
@@ -12,18 +12,29 @@ import Network
 import RelaySelector
 import XCTest
 
+private let portRanges: [[UInt16]] = [[4000, 4001], [5000, 5001]]
+private let defaultPort: UInt16 = 53
+
 class RelaySelectorTests: XCTestCase {
     func testCountryConstraint() throws {
         let constraints = RelayConstraints(location: .only(.country("es")))
 
-        let result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints)
+        let result = try RelaySelector.evaluate(
+            relays: sampleRelays,
+            constraints: constraints,
+            numberOfFailedAttempts: 0
+        )
 
         XCTAssertEqual(result.relay.hostname, "es1-wireguard")
     }
 
     func testCityConstraint() throws {
         let constraints = RelayConstraints(location: .only(.city("se", "got")))
-        let result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints)
+        let result = try RelaySelector.evaluate(
+            relays: sampleRelays,
+            constraints: constraints,
+            numberOfFailedAttempts: 0
+        )
 
         XCTAssertEqual(result.relay.hostname, "se10-wireguard")
     }
@@ -31,10 +42,38 @@ class RelaySelectorTests: XCTestCase {
     func testHostnameConstraint() throws {
         let constraints = RelayConstraints(location: .only(.hostname("se", "sto", "se6-wireguard")))
 
-        let result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints)
+        let result = try RelaySelector.evaluate(
+            relays: sampleRelays,
+            constraints: constraints,
+            numberOfFailedAttempts: 0
+        )
 
         XCTAssertEqual(result.relay.hostname, "se6-wireguard")
     }
+
+    func testRandomPortSelectionWithFailedAttempts() throws {
+        let constraints = RelayConstraints(location: .only(.hostname("se", "sto", "se6-wireguard")))
+        let allPorts = portRanges.flatMap { $0 }
+
+        var result = try RelaySelector.evaluate(
+            relays: sampleRelays,
+            constraints: constraints,
+            numberOfFailedAttempts: 0
+        )
+        XCTAssertTrue(allPorts.contains(result.endpoint.ipv4Relay.port))
+
+        result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints, numberOfFailedAttempts: 1)
+        XCTAssertTrue(allPorts.contains(result.endpoint.ipv4Relay.port))
+
+        result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints, numberOfFailedAttempts: 2)
+        XCTAssertEqual(result.endpoint.ipv4Relay.port, defaultPort)
+
+        result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints, numberOfFailedAttempts: 3)
+        XCTAssertEqual(result.endpoint.ipv4Relay.port, defaultPort)
+
+        result = try RelaySelector.evaluate(relays: sampleRelays, constraints: constraints, numberOfFailedAttempts: 4)
+        XCTAssertTrue(allPorts.contains(result.endpoint.ipv4Relay.port))
+    }
 }
 
 private let sampleRelays = REST.ServerRelaysResponse(
@@ -61,7 +100,7 @@ private let sampleRelays = REST.ServerRelaysResponse(
     wireguard: REST.ServerWireguardTunnels(
         ipv4Gateway: .loopback,
         ipv6Gateway: .loopback,
-        portRanges: [[53, 53]],
+        portRanges: portRanges,
         relays: [
             REST.ServerRelay(
                 hostname: "es1-wireguard",
diff --git a/ios/PacketTunnel/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider.swift
index c70d5e0d35..52220cdf49 100644
--- a/ios/PacketTunnel/PacketTunnelProvider.swift
+++ b/ios/PacketTunnel/PacketTunnelProvider.swift
@@ -117,7 +117,8 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate {
             lastErrors: errors.compactMap { $0 },
             isNetworkReachable: isNetworkReachable,
             deviceCheck: deviceCheck,
-            tunnelRelay: selectorResult?.packetTunnelRelay
+            tunnelRelay: selectorResult?.packetTunnelRelay,
+            numberOfFailedAttempts: numberOfFailedAttempts
         )
     }
 
@@ -674,7 +675,8 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate {
 
         return try RelaySelector.evaluate(
             relays: cachedRelayList.relays,
-            constraints: relayConstraints
+            constraints: relayConstraints,
+            numberOfFailedAttempts: packetTunnelStatus.numberOfFailedAttempts
         )
     }
 
diff --git a/ios/RelaySelector/RelaySelector.swift b/ios/RelaySelector/RelaySelector.swift
index 392aee7334..0feed32cf4 100644
--- a/ios/RelaySelector/RelaySelector.swift
+++ b/ios/RelaySelector/RelaySelector.swift
@@ -10,6 +10,8 @@ import Foundation
 import MullvadREST
 import MullvadTypes
 
+private let defaultPort: UInt16 = 53
+
 public enum RelaySelector {
     /**
      Returns random shadowsocks TCP bridge, otherwise `nil` if there are no shadowdsocks bridges.
@@ -24,13 +26,17 @@ public enum RelaySelector {
      */
     public static func evaluate(
         relays: REST.ServerRelaysResponse,
-        constraints: RelayConstraints
+        constraints: RelayConstraints,
+        numberOfFailedAttempts: UInt
     ) throws -> RelaySelectorResult {
         let filteredRelays = applyConstraints(constraints, relays: Self.parseRelaysResponse(relays))
+        let port = applyConstraints(
+            constraints,
+            rawPortRanges: relays.wireguard.portRanges,
+            numberOfFailedAttempts: numberOfFailedAttempts
+        )
 
-        guard let relayWithLocation = pickRandomRelay(relays: filteredRelays),
-              let port = pickRandomPort(rawPortRanges: relays.wireguard.portRanges)
-        else {
+        guard let relayWithLocation = pickRandomRelay(relays: filteredRelays), let port = port else {
             throw NoRelaysSatisfyingConstraintsError()
         }
 
@@ -82,6 +88,20 @@ public enum RelaySelector {
         }
     }
 
+    /// Produce a port that is either user provided or randomly selected, satisfying the given constraints.
+    private static func applyConstraints(
+        _ constraints: RelayConstraints,
+        rawPortRanges: [[UInt16]],
+        numberOfFailedAttempts: UInt
+    ) -> UInt16? {
+        // 1. First two attempts should pick a random port.
+        // 2. The next two should pick port 53.
+        // 3. Repeat steps 1 and 2.
+        let useDefaultPort = (numberOfFailedAttempts % 4 == 2) || (numberOfFailedAttempts % 4 == 3)
+
+        return useDefaultPort ? defaultPort : pickRandomPort(rawPortRanges: rawPortRanges)
+    }
+
     private static func pickRandomRelay(relays: [RelayWithLocation]) -> RelayWithLocation? {
         let totalWeight = relays.reduce(0) { accummulatedWeight, relayWithLocation in
             return accummulatedWeight + relayWithLocation.relay.weight