Merge branch 'fix-broken-retry-strategy-tests-ios-938'

docs/relay-selector.md

@@ -62,6 +62,20 @@ constraints the following default ones will take effect:
   - The seventh attempt will connect to an OpenVPN relay over TCP on port 443
   - The eighth attempt will connect to an OpenVPN relay over a bridge on a random port
 
+### Default constraints for tunnel endpoints on iOS
+
+The iOS platform does not support OpenVPN, or connecting to a relay over IPv6.
+As such, the above algorithm is simplified to the following version:
+  - The first attempt will connect to a Wireguard relay on a random port
+  - The second attempt will connect to a Wireguard relay on port 443
+  - The third attempt will connect to a Wireguard relay on a random port using Shadowsocks for obfuscation
+  - The fourth attempt will connect to a Wireguard relay on a random port using [UDP2TCP obfuscation](https://github.com/mullvad/udp-over-tcp)
+
+### Random Ports for UDP2TCP and Shadowsocks
+
+- The UDP2TCP random port is **either** 80 **or** 5001
+- The Shadowsocks port is random within a certain range of ports defined by the relay list
+
 If no tunnel has been established after exhausting this list of attempts, the relay selector will
 loop back to the first default constraint and continue its search from there.
 
@@ -117,5 +131,5 @@ will indirectly change the bridge state to _Auto_ if it was previously set to _O
 
 ### Obfuscator caveats
 
-Currently, there is only a single type of obfuscator - _udp2tcp_, and it's only used if it's mode is
-set to _On_ or _Auto_ and the user has selected WireGuard to be the only tunnel protocol to be used.
+There are two type of obfuscators - _udp2tcp_, and _shadowsocks_.
+They are used if the obfuscation mode is set _Auto_ and the user has selected WireGuard to be the only tunnel protocol to be used.

ios/MullvadREST/Relay/ObfuscatorPortSelector.swift

@@ -93,7 +93,7 @@ struct ObfuscatorPortSelector {
     ) -> RelayConstraint<UInt16> {
         switch tunnelSettings.wireGuardObfuscation.udpOverTcpPort {
         case .automatic:
-            return (connectionAttemptCount % 2 == 0) ? .only(80) : .only(5001)
+            return [.only(80), .only(5001)].randomElement()!
         case .port5001:
             return .only(5001)
         case .port80:

ios/MullvadREST/Relay/RelaySelector.swift

@@ -9,7 +9,7 @@
 import MullvadSettings
 import MullvadTypes
 
-private let defaultPort: UInt16 = 53
+private let defaultPort: UInt16 = 443
 
 public enum RelaySelector {
     // MARK: - public
@@ -98,10 +98,10 @@ public enum RelaySelector {
             return port
 
         case .any:
-            // 1. First two attempts should pick a random port.
-            // 2. The next two should pick port 53.
+            // 1. First attempt should pick a random port.
+            // 2. The second should pick port 443.
             // 3. Repeat steps 1 and 2.
-            let useDefaultPort = (numberOfFailedAttempts % 4 == 2) || (numberOfFailedAttempts % 4 == 3)
+            let useDefaultPort = numberOfFailedAttempts.isOrdered(nth: 2, forEverySetOf: 2)
 
             return useDefaultPort ? defaultPort : pickRandomPort(rawPortRanges: rawPortRanges)
         }

ios/MullvadVPNTests/MullvadREST/Relay/ObfuscatorPortSelectorTests.swift

@@ -68,7 +68,7 @@ final class ObfuscatorPortSelectorTests: XCTestCase {
         XCTAssertEqual(obfuscationResult.port, .only(5001))
     }
 
-    func testObfuscateUpdOverTcpPortAutomaticIsPort80OnEvenRetryAttempts() throws {
+    func testObfuscateUpdOverTcpPortAutomaticIsRandomPort() throws {
         tunnelSettings.wireGuardObfuscation = WireGuardObfuscationSettings(
             state: .udpOverTcp,
             udpOverTcpPort: .automatic
@@ -82,25 +82,8 @@ final class ObfuscatorPortSelectorTests: XCTestCase {
                 connectionAttemptCount: UInt(attempt)
             )
 
-            XCTAssertEqual(obfuscationResult.port, .only(80))
-        }
-    }
-
-    func testObfuscateUpdOverTcpPortAutomaticIsPort5001OnOddRetryAttempts() throws {
-        tunnelSettings.wireGuardObfuscation = WireGuardObfuscationSettings(
-            state: .udpOverTcp,
-            udpOverTcpPort: .automatic
-        )
-
-        try (0 ... 10).filter { !$0.isMultiple(of: 2) }.forEach { attempt in
-            let obfuscationResult = try ObfuscatorPortSelector(
-                relays: sampleRelays
-            ).obfuscate(
-                tunnelSettings: tunnelSettings,
-                connectionAttemptCount: UInt(attempt)
-            )
-
-            XCTAssertEqual(obfuscationResult.port, .only(5001))
+            let validPorts: [RelayConstraint<UInt16>] = [.only(80), .only(5001)]
+            XCTAssertTrue(validPorts.contains(obfuscationResult.port))
         }
     }
 

ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift

@@ -14,7 +14,7 @@ import Network
 import XCTest
 
 private let portRanges: [[UInt16]] = [[4000, 4001], [5000, 5001]]
-private let defaultPort: UInt16 = 53
+private let defaultPort: UInt16 = 443
 
 class RelaySelectorTests: XCTestCase {
     let sampleRelays = ServerRelaysResponseStubs.sampleRelays
@@ -124,10 +124,10 @@ class RelaySelectorTests: XCTestCase {
         XCTAssertTrue(allPorts.contains(result.endpoint.ipv4Relay.port))
 
         result = try pickRelay(by: constraints, in: sampleRelays, failedAttemptCount: 1)
-        XCTAssertTrue(allPorts.contains(result.endpoint.ipv4Relay.port))
+        XCTAssertEqual(result.endpoint.ipv4Relay.port, defaultPort)
 
         result = try pickRelay(by: constraints, in: sampleRelays, failedAttemptCount: 2)
-        XCTAssertEqual(result.endpoint.ipv4Relay.port, defaultPort)
+        XCTAssertTrue(allPorts.contains(result.endpoint.ipv4Relay.port))
 
         result = try pickRelay(by: constraints, in: sampleRelays, failedAttemptCount: 3)
         XCTAssertEqual(result.endpoint.ipv4Relay.port, defaultPort)

ios/MullvadVPNUITests/Networking/FirewallAPIClient.swift

@@ -77,6 +77,40 @@ class FirewallAPIClient {
         }
     }
 
+    /// Gets the IP address of the device under test
+    public func getDeviceIPAddress() throws -> String {
+        let deviceIPURL = baseURL.appendingPathComponent("own-ip")
+        let request = URLRequest(url: deviceIPURL)
+        let completionHandlerInvokedExpectation = XCTestExpectation(
+            description: "Completion handler for the request is invoked"
+        )
+        var deviceIPAddress = ""
+        var requestError: Error?
+
+        let dataTask = URLSession.shared.dataTask(with: request) { data, _, _ in
+            defer { completionHandlerInvokedExpectation.fulfill() }
+            guard let data else {
+                requestError = NetworkingError.internalError(reason: "Could not get device IP")
+                return
+            }
+
+            deviceIPAddress = String(data: data, encoding: .utf8)!
+        }
+
+        dataTask.resume()
+
+        let waitResult = XCTWaiter.wait(for: [completionHandlerInvokedExpectation], timeout: 30)
+        if waitResult != .completed {
+            XCTFail("Failed to get device IP address - timeout")
+        }
+
+        if let requestError {
+            throw requestError
+        }
+
+        return deviceIPAddress
+    }
+
     /// Remove all firewall rules associated to this device under test
     public func removeRules() {
         let removeRulesURL = baseURL.appendingPathComponent("remove-rules/\(sessionIdentifier)")

ios/MullvadVPNUITests/Networking/FirewallRule.swift

@@ -36,7 +36,7 @@ struct FirewallRule {
 
     /// Make a firewall rule blocking API access for the current device under test
     public static func makeBlockAPIAccessFirewallRule() throws -> FirewallRule {
-        let deviceIPAddress = try Networking.getIPAddress()
+        let deviceIPAddress = try FirewallAPIClient().getDeviceIPAddress()
         let apiIPAddress = try MullvadAPIWrapper.getAPIIPAddress()
         return FirewallRule(
             fromIPAddress: deviceIPAddress,
@@ -46,7 +46,7 @@ struct FirewallRule {
     }
 
     public static func makeBlockAllTrafficRule(toIPAddress: String) throws -> FirewallRule {
-        let deviceIPAddress = try Networking.getIPAddress()
+        let deviceIPAddress = try FirewallAPIClient().getDeviceIPAddress()
 
         return FirewallRule(
             fromIPAddress: deviceIPAddress,
@@ -56,7 +56,7 @@ struct FirewallRule {
     }
 
     public static func makeBlockUDPTrafficRule(toIPAddress: String) throws -> FirewallRule {
-        let deviceIPAddress = try Networking.getIPAddress()
+        let deviceIPAddress = try FirewallAPIClient().getDeviceIPAddress()
 
         return FirewallRule(
             fromIPAddress: deviceIPAddress,

ios/MullvadVPNUITests/Networking/Networking.swift

@@ -22,52 +22,6 @@ struct DNSServerEntry: Decodable {
 
 /// Class with methods for verifying network connectivity
 class Networking {
-    /// Get IP address of the iOS device under test
-    static func getIPAddress() throws -> String {
-        var ipAddress: String
-        // Get list of all interfaces on the local machine:
-        var interfaceList: UnsafeMutablePointer<ifaddrs>?
-        guard getifaddrs(&interfaceList) == 0, let firstInterfaceAddress = interfaceList else {
-            throw NetworkingError.internalError(reason: "Failed to locate local networking interface")
-        }
-
-        // For each interface
-        for interfacePointer in sequence(first: firstInterfaceAddress, next: { $0.pointee.ifa_next }) {
-            let flags = Int32(interfacePointer.pointee.ifa_flags)
-            let interfaceAddress = interfacePointer.pointee.ifa_addr.pointee
-
-            // Check for running IPv4 interfaces. Skip the loopback interface.
-            if (
-                flags &
-                    (IFF_UP | IFF_RUNNING | IFF_LOOPBACK)
-            ) == (IFF_UP | IFF_RUNNING),
-                interfaceAddress.sa_family == UInt8(AF_INET) {
-                // Check if interface is en0 which is the WiFi connection on the iPhone
-                let name = String(cString: interfacePointer.pointee.ifa_name)
-                if name == "en0" {
-                    // Convert interface address to a human readable string:
-                    var hostname = [CChar](repeating: 0, count: Int(NI_MAXHOST))
-                    if getnameinfo(
-                        interfacePointer.pointee.ifa_addr,
-                        socklen_t(interfaceAddress.sa_len),
-                        &hostname,
-                        socklen_t(hostname.count),
-                        nil,
-                        socklen_t(0),
-                        NI_NUMERICHOST
-                    ) == 0 {
-                        ipAddress = String(cString: hostname)
-                        return ipAddress
-                    }
-                }
-            }
-        }
-
-        freeifaddrs(interfaceList)
-
-        throw NetworkingError.internalError(reason: "Failed to determine device's IP address")
-    }
-
     /// Get configured ad serving domain
     private static func getAdServingDomain() throws -> String {
         guard let adServingDomain = Bundle(for: Networking.self)

ios/MullvadVPNUITests/Pages/TunnelControlPage.swift

@@ -127,67 +127,57 @@ class TunnelControlPage: Page {
 
     /// Verify that the app attempts to connect over UDP before switching to TCP. For testing blocked UDP traffic.
     @discardableResult func verifyConnectingOverTCPAfterUDPAttempts() -> Self {
-        let connectionAttempts = waitForConnectionAttempts(3, timeout: 15)
+        let connectionAttempts = waitForConnectionAttempts(4, timeout: 30)
 
-        // Should do three connection attempts but due to UI bug sometimes only two are displayed, sometimes all three
-        if connectionAttempts.count == 3 { // Expected retries flow
+        // Should do four connection attempts but due to UI bug sometimes only two are displayed, sometimes all four
+        if connectionAttempts.count == 4 { // Expected retries flow
             for (attemptIndex, attempt) in connectionAttempts.enumerated() {
-                if attemptIndex == 0 || attemptIndex == 1 {
+                if attemptIndex < 3 {
                     XCTAssertEqual(attempt.protocolName, "UDP")
-                } else if attemptIndex == 2 {
+                } else if attemptIndex == 3 {
                     XCTAssertEqual(attempt.protocolName, "TCP")
                 } else {
                     XCTFail("Unexpected connection attempt")
                 }
             }
-        } else if connectionAttempts.count == 2 { // Most of the times this incorrect flow is shown
+        } else if connectionAttempts.count == 3 { // Most of the times this incorrect flow is shown
             for (attemptIndex, attempt) in connectionAttempts.enumerated() {
-                if attemptIndex == 0 {
+                if attemptIndex == 0 || attemptIndex == 1 {
                     XCTAssertEqual(attempt.protocolName, "UDP")
-                } else if attemptIndex == 1 {
+                } else if attemptIndex == 2 {
                     XCTAssertEqual(attempt.protocolName, "TCP")
                 } else {
                     XCTFail("Unexpected connection attempt")
                 }
             }
         } else {
-            XCTFail("Unexpected number of connection attempts")
+            XCTFail("Unexpected number of connection attempts, expected 3~4, got \(connectionAttempts.count)")
         }
 
         return self
     }
 
     /// Verify that connection attempts are made in the correct order
     @discardableResult func verifyConnectionAttemptsOrder() -> Self {
-        let connectionAttempts = waitForConnectionAttempts(4, timeout: 50)
+        var connectionAttempts = waitForConnectionAttempts(4, timeout: 80)
+        var totalAttemptsOffset = 0
         XCTAssertEqual(connectionAttempts.count, 4)
 
+        /// Sometimes, the UI will only show an IP address for the first connection attempt, which gets skipped by
+        /// `waitForConnectionAttempts`, and offsets expected attempts count by 1, but still counts towards
+        /// total connection attempts. Remove that last attempt which would be the first one of a new series
+        /// of connection attempts.
         if connectionAttempts.last?.protocolName == "UDP" {
-            // If last attempt is over UDP it means we have encountered the UI bug where only one UDP attempt is shown and then the two TCP attempts
-            for (attemptIndex, attempt) in connectionAttempts.enumerated() {
-                if attemptIndex == 0 {
-                    XCTAssertEqual(attempt.protocolName, "UDP")
-                } else if attemptIndex == 1 {
-                    XCTAssertEqual(attempt.protocolName, "TCP")
-                    XCTAssertEqual(attempt.port, "80")
-                } else if attemptIndex == 2 {
-                    XCTAssertEqual(attempt.protocolName, "TCP")
-                    XCTAssertEqual(attempt.port, "5001")
-                } // Ignore the 4th attempt which is the first attempt of new attempt cycle
-            }
-        } else {
-            for (attemptIndex, attempt) in connectionAttempts.enumerated() {
-                if attemptIndex == 0 {
-                    XCTAssertEqual(attempt.protocolName, "UDP")
-                } else if attemptIndex == 1 {
-                    XCTAssertEqual(attempt.protocolName, "UDP")
-                } else if attemptIndex == 2 {
-                    XCTAssertEqual(attempt.protocolName, "TCP")
-                    XCTAssertEqual(attempt.port, "80")
-                } else if attemptIndex == 3 {
-                    XCTAssertEqual(attempt.protocolName, "TCP")
-                    XCTAssertEqual(attempt.port, "5001")
-                }
+            connectionAttempts.removeLast()
+            totalAttemptsOffset = 1
+        }
+        for (attemptIndex, attempt) in connectionAttempts.enumerated() {
+            if attemptIndex < 3 - totalAttemptsOffset {
+                XCTAssertEqual(attempt.protocolName, "UDP")
+            } else {
+                XCTAssertEqual(attempt.protocolName, "TCP")
+                let validPorts = ["80", "5001"]
+                XCTAssertTrue(validPorts.contains(attempt.port))
             }
         }
 

ios/MullvadVPNUITests/Pages/VPNSettingsPage.swift

@@ -102,12 +102,18 @@ class VPNSettingsPage: Page {
         return self
     }
 
-    @discardableResult func tapWireGuardObfuscationOnCell() -> Self {
+    @discardableResult func tapWireGuardObfuscationUdpOverTcpCell() -> Self {
         app.cells[AccessibilityIdentifier.wireGuardObfuscationUdpOverTcp].tap()
 
         return self
     }
 
+    @discardableResult func tapWireGuardObfuscationShadowsocksCell() -> Self {
+        app.cells[AccessibilityIdentifier.wireGuardObfuscationShadowsocks].tap()
+
+        return self
+    }
+
     @discardableResult func tapWireGuardObfuscationOffCell() -> Self {
         app.cells[AccessibilityIdentifier.wireGuardObfuscationOff].tap()