diff options
| author | Bug Magnet <marco.nikic@mullvad.net> | 2024-11-27 12:04:46 +0100 |
|---|---|---|
| committer | Bug Magnet <marco.nikic@mullvad.net> | 2024-11-27 12:04:46 +0100 |
| commit | 1db50d297755346e01bbea889ab5234c54d2f70a (patch) | |
| tree | 081a2a624d63654821122c0c9c1cf43f29ac00be | |
| parent | 6d4a83cac7b2f646a1656686ddf1358eb3e82339 (diff) | |
| parent | 12174a710c9f3b103e52384a7fbe50351bf7ad8f (diff) | |
| download | mullvadvpn-1db50d297755346e01bbea889ab5234c54d2f70a.tar.xz mullvadvpn-1db50d297755346e01bbea889ab5234c54d2f70a.zip | |
Merge branch 'fix-broken-retry-strategy-tests-ios-938'
| -rw-r--r-- | docs/relay-selector.md | 18 | ||||
| -rw-r--r-- | ios/MullvadREST/Relay/ObfuscatorPortSelector.swift | 2 | ||||
| -rw-r--r-- | ios/MullvadREST/Relay/RelaySelector.swift | 8 | ||||
| -rw-r--r-- | ios/MullvadVPNTests/MullvadREST/Relay/ObfuscatorPortSelectorTests.swift | 23 | ||||
| -rw-r--r-- | ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift | 6 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/Networking/FirewallAPIClient.swift | 34 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/Networking/FirewallRule.swift | 6 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/Networking/Networking.swift | 46 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/Pages/TunnelControlPage.swift | 60 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/Pages/VPNSettingsPage.swift | 8 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/RelayTests.swift | 43 | ||||
| -rw-r--r-- | ios/MullvadVPNUITests/SettingsMigrationTests.swift | 2 |
12 files changed, 139 insertions, 117 deletions
diff --git a/docs/relay-selector.md b/docs/relay-selector.md index 811a13f6ba..8433d69dae 100644 --- a/docs/relay-selector.md +++ b/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. diff --git a/ios/MullvadREST/Relay/ObfuscatorPortSelector.swift b/ios/MullvadREST/Relay/ObfuscatorPortSelector.swift index 597a8fdc96..8cb2776d9b 100644 --- a/ios/MullvadREST/Relay/ObfuscatorPortSelector.swift +++ b/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: diff --git a/ios/MullvadREST/Relay/RelaySelector.swift b/ios/MullvadREST/Relay/RelaySelector.swift index 20e6153532..96118e6ae5 100644 --- a/ios/MullvadREST/Relay/RelaySelector.swift +++ b/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) } diff --git a/ios/MullvadVPNTests/MullvadREST/Relay/ObfuscatorPortSelectorTests.swift b/ios/MullvadVPNTests/MullvadREST/Relay/ObfuscatorPortSelectorTests.swift index 32d0eeade2..f26bb149ae 100644 --- a/ios/MullvadVPNTests/MullvadREST/Relay/ObfuscatorPortSelectorTests.swift +++ b/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)) } } diff --git a/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift b/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift index 7a3f6a7d73..af14199e3b 100644 --- a/ios/MullvadVPNTests/MullvadREST/Relay/RelaySelectorTests.swift +++ b/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) diff --git a/ios/MullvadVPNUITests/Networking/FirewallAPIClient.swift b/ios/MullvadVPNUITests/Networking/FirewallAPIClient.swift index b53d0b15e0..da7f2482ac 100644 --- a/ios/MullvadVPNUITests/Networking/FirewallAPIClient.swift +++ b/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)") diff --git a/ios/MullvadVPNUITests/Networking/FirewallRule.swift b/ios/MullvadVPNUITests/Networking/FirewallRule.swift index 84f9a56820..02f1811e87 100644 --- a/ios/MullvadVPNUITests/Networking/FirewallRule.swift +++ b/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, diff --git a/ios/MullvadVPNUITests/Networking/Networking.swift b/ios/MullvadVPNUITests/Networking/Networking.swift index 6e75925d94..9e00f48d02 100644 --- a/ios/MullvadVPNUITests/Networking/Networking.swift +++ b/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) diff --git a/ios/MullvadVPNUITests/Pages/TunnelControlPage.swift b/ios/MullvadVPNUITests/Pages/TunnelControlPage.swift index cf90555e5a..fc8f6c044a 100644 --- a/ios/MullvadVPNUITests/Pages/TunnelControlPage.swift +++ b/ios/MullvadVPNUITests/Pages/TunnelControlPage.swift @@ -127,31 +127,31 @@ 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 @@ -159,35 +159,25 @@ class TunnelControlPage: Page { /// 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)) } } diff --git a/ios/MullvadVPNUITests/Pages/VPNSettingsPage.swift b/ios/MullvadVPNUITests/Pages/VPNSettingsPage.swift index 570f32ab08..7f463069b8 100644 --- a/ios/MullvadVPNUITests/Pages/VPNSettingsPage.swift +++ b/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() diff --git a/ios/MullvadVPNUITests/RelayTests.swift b/ios/MullvadVPNUITests/RelayTests.swift index b000d2fe50..319a1b0a23 100644 --- a/ios/MullvadVPNUITests/RelayTests.swift +++ b/ios/MullvadVPNUITests/RelayTests.swift @@ -151,7 +151,48 @@ class RelayTests: LoggedInWithTimeUITestCase { VPNSettingsPage(app) .tapWireGuardObfuscationExpandButton() - .tapWireGuardObfuscationOnCell() + .tapWireGuardObfuscationUdpOverTcpCell() + .tapBackButton() + + SettingsPage(app) + .tapDoneButton() + + TunnelControlPage(app) + .tapSecureConnectionButton() + + allowAddVPNConfigurationsIfAsked() + + TunnelControlPage(app) + .waitForSecureConnectionLabel() + + try Networking.verifyCanAccessInternet() + + TunnelControlPage(app) + .tapDisconnectButton() + } + + func testWireGuardOverShadowsocksManually() throws { + addTeardownBlock { + HeaderBar(self.app) + .tapSettingsButton() + + SettingsPage(self.app) + .tapVPNSettingsCell() + + VPNSettingsPage(self.app) + .tapWireGuardObfuscationExpandButton() + .tapWireGuardObfuscationOffCell() + } + + HeaderBar(app) + .tapSettingsButton() + + SettingsPage(app) + .tapVPNSettingsCell() + + VPNSettingsPage(app) + .tapWireGuardObfuscationExpandButton() + .tapWireGuardObfuscationShadowsocksCell() .tapBackButton() SettingsPage(app) diff --git a/ios/MullvadVPNUITests/SettingsMigrationTests.swift b/ios/MullvadVPNUITests/SettingsMigrationTests.swift index 85ab71bafa..0de0e59319 100644 --- a/ios/MullvadVPNUITests/SettingsMigrationTests.swift +++ b/ios/MullvadVPNUITests/SettingsMigrationTests.swift @@ -116,7 +116,7 @@ class SettingsMigrationTests: BaseUITestCase { .enterText(wireGuardPort) .dismissKeyboard() .tapWireGuardObfuscationExpandButton() - .tapWireGuardObfuscationOnCell() + .tapWireGuardObfuscationUdpOverTcpCell() .tapUDPOverTCPPortExpandButton() .tapUDPOverTCPPort80Cell() .tapQuantumResistantTunnelExpandButton() |
