Cache the transportStrategy in the UserDefaults

ios/MullvadREST/RESTTransportStrategy.swift → ios/MullvadREST/TransportStrategy.swift

@@ -1,14 +1,15 @@
 //
-//  RESTTransportStrategy.swift
+//  TransportStrategy.swift
 //  MullvadREST
 //
 //  Created by Marco Nikic on 2023-04-27.
 //  Copyright © 2023 Mullvad VPN AB. All rights reserved.
 //
 
 import Foundation
+import MullvadTypes
 
-public struct TransportStrategy: Codable, Equatable {
+public struct TransportStrategy: Equatable {
     /// The different transports suggested by the strategy
     public enum Transport {
         /// Suggests using a direct connection
@@ -23,19 +24,25 @@ public struct TransportStrategy: Codable, Equatable {
     /// suggestion.
     ///
     /// `internal` instead of `private` for testing purposes.
-    internal var connectionAttempts: UInt
+    internal var connectionAttempts: Int
 
-    public init() {
-        connectionAttempts = 0
+    /// Enables recording of failed connection attempts.
+    private let attemptsRecorder: AttemptsRecording
+
+    public init(connectionAttempts: Int = 0, attemptsRecorder: AttemptsRecording) {
+        self.connectionAttempts = connectionAttempts
+        self.attemptsRecorder = attemptsRecorder
     }
 
     /// Instructs the strategy that a network connection failed
     ///
     /// Every third failure results in a direct transport suggestion.
     public mutating func didFail() {
         let (partial, isOverflow) = connectionAttempts.addingReportingOverflow(1)
-        // UInt.max is a multiple of 3, go directly to 1 when overflowing
-        connectionAttempts = isOverflow ? 1 : partial
+        // (Int.max - 1) is a multiple of 3, go directly to 2 when overflowing
+        // to keep the "every third failure" algorithm correct
+        connectionAttempts = isOverflow ? 2 : partial
+        attemptsRecorder.record(connectionAttempts)
     }
 
     /// The suggested connection transport
@@ -44,4 +51,8 @@ public struct TransportStrategy: Codable, Equatable {
     public func connectionTransport() -> Transport {
         connectionAttempts.isMultiple(of: 3) ? .useURLSession : .useShadowsocks
     }
+
+    public static func == (lhs: TransportStrategy, rhs: TransportStrategy) -> Bool {
+        lhs.connectionAttempts == rhs.connectionAttempts
+    }
 }

ios/MullvadRESTTests/TransportStrategyTests.swift

@@ -7,45 +7,48 @@
 //
 
 @testable import MullvadREST
+@testable import MullvadTypes
 import XCTest
 
 final class TransportStrategyTests: XCTestCase {
     func testEveryThirdConnectionAttemptsIsDirect() {
-        loopStrategyTest(with: TransportStrategy())
+        loopStrategyTest(with: TransportStrategy(attemptsRecorder: MockRecorder()), in: 0 ... 12)
     }
 
     func testOverflowingConnectionAttempts() {
-        var strategy = TransportStrategy()
-        strategy.connectionAttempts = UInt.max
+        let strategy = TransportStrategy(connectionAttempts: Int.max, attemptsRecorder: MockRecorder())
 
-        loopStrategyTest(with: strategy)
+        // (Int.max - 1) is a multiple of 3, so skip the first iteration
+        loopStrategyTest(with: strategy, in: 1 ... 12)
     }
 
-    func testLoadingFromCacheDoesNotImpactStrategy() throws {
-        var strategy = TransportStrategy()
+    func testConnectionAttemptsAreRecordedAfterFailure() {
+        var recorder = MockRecorder()
+        var strategy = TransportStrategy(attemptsRecorder: recorder)
 
-        // Fail twice, the next suggested transport mode should be via Shadowsocks proxy
-        strategy.didFail()
-        strategy.didFail()
-        XCTAssertEqual(strategy.connectionTransport(), .useShadowsocks)
-
-        // Serialize the strategy and reload it from memory to simulate an application restart
-        let encodedRawStrategy = try JSONEncoder().encode(strategy)
-        var reloadedStrategy = try JSONDecoder().decode(TransportStrategy.self, from: encodedRawStrategy)
+        recorder.didRecord = { connectionAttempt in
+            XCTAssertEqual(connectionAttempt, 1)
+        }
 
-        // This should be the third failure, the next suggested transport will be a direct one
-        reloadedStrategy.didFail()
-        XCTAssertEqual(reloadedStrategy.connectionTransport(), .useURLSession)
+        strategy.didFail()
     }
 
-    private func loopStrategyTest(with strategy: TransportStrategy) {
+    private func loopStrategyTest(with strategy: TransportStrategy, in range: ClosedRange<Int>) {
         var strategy = strategy
 
-        for index in 0 ... 12 {
+        for index in range {
             let expectedResult: TransportStrategy.Transport
             expectedResult = index.isMultiple(of: 3) ? .useURLSession : .useShadowsocks
             XCTAssertEqual(strategy.connectionTransport(), expectedResult)
             strategy.didFail()
         }
     }
 }
+
+struct MockRecorder: AttemptsRecording {
+    var didRecord: ((Int) -> Void)?
+
+    func record(_ attempts: Int) {
+        didRecord?(attempts)
+    }
+}

ios/MullvadTransport/TransportProvider.swift

@@ -31,7 +31,7 @@ public final class TransportProvider: RESTTransport {
         relayCache: RelayCache,
         addressCache: REST.AddressCache,
         shadowsocksCache: ShadowsocksConfigurationCache,
-        transportStrategy: TransportStrategy = .init(),
+        transportStrategy: TransportStrategy,
         constraintsUpdater: RelayConstraintsUpdater
     ) {
         self.urlSessionTransport = urlSessionTransport

ios/MullvadTypes/AttemptsRecording.swift

@@ -0,0 +1,16 @@
+//
+//  AttemptsRecording.swift
+//  MullvadTypes
+//
+//  Created by Marco Nikic on 2023-07-24.
+//  Copyright © 2023 Mullvad VPN AB. All rights reserved.
+//
+
+import Foundation
+
+/// Defines a generic way to record an attempt.
+///
+/// Used by `TransportStrategy` to record failed connection attempts in cache.
+public protocol AttemptsRecording {
+    func record(_ attempts: Int)
+}

ios/MullvadVPN.xcodeproj/project.pbxproj

@@ -399,7 +399,10 @@
 		7ABE318D2A1CDD4500DF4963 /* UIFont+Weight.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7ABE318C2A1CDD4500DF4963 /* UIFont+Weight.swift */; };
 		7AE47E522A17972A000418DA /* CustomAlertViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AE47E512A17972A000418DA /* CustomAlertViewController.swift */; };
 		7AF0419E29E957EB00D492DD /* AccountCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AF0419D29E957EB00D492DD /* AccountCoordinator.swift */; };
-		A917351F29FAA9C400D5DCFD /* RESTTransportStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = A917351E29FAA9C400D5DCFD /* RESTTransportStrategy.swift */; };
+		A90F9DEC2A6E91A5009DC8B3 /* AttemptsRecording.swift in Sources */ = {isa = PBXBuildFile; fileRef = A90F9DEB2A6E91A5009DC8B3 /* AttemptsRecording.swift */; };
+		A90F9DED2A6E929E009DC8B3 /* UserDefaults+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = A90F9DE92A6E8B1F009DC8B3 /* UserDefaults+Extensions.swift */; };
+		A90F9DEE2A6E92AD009DC8B3 /* UserDefaults+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = A90F9DE92A6E8B1F009DC8B3 /* UserDefaults+Extensions.swift */; };
+		A917351F29FAA9C400D5DCFD /* TransportStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = A917351E29FAA9C400D5DCFD /* TransportStrategy.swift */; };
 		A917352129FAAA5200D5DCFD /* TransportStrategyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A917352029FAAA5200D5DCFD /* TransportStrategyTests.swift */; };
 		A93D13782A1F60A6001EB0B1 /* shadowsocks.h in Headers */ = {isa = PBXBuildFile; fileRef = 586F2BE129F6916F009E6924 /* shadowsocks.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		A9467E7F2A29DEFE000DC21F /* RelayCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A9467E7E2A29DEFE000DC21F /* RelayCacheTests.swift */; };
@@ -418,9 +421,9 @@
 		A9D99BA52A1F808900DE27D3 /* RelayCache.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 063F02732902B63F001FA09F /* RelayCache.framework */; };
 		A9D99BA62A1F809C00DE27D3 /* libRelaySelector.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 5898D29829017DAC00EB5EBA /* libRelaySelector.a */; };
 		A9D99BA92A1F81B700DE27D3 /* MullvadTransport.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = A97F1F412A1F4E1A00ECEFDE /* MullvadTransport.framework */; };
-		A9EC20F02A5D79ED0040D56E /* TunnelObfuscation.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 5840231F2A406BF5007B27AC /* TunnelObfuscation.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		A9EC20E62A5C488D0040D56E /* Haversine.swift in Sources */ = {isa = PBXBuildFile; fileRef = A9EC20E52A5C488D0040D56E /* Haversine.swift */; };
 		A9EC20E82A5D3A8C0040D56E /* CoordinatesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A9EC20E72A5D3A8C0040D56E /* CoordinatesTests.swift */; };
+		A9EC20F02A5D79ED0040D56E /* TunnelObfuscation.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 5840231F2A406BF5007B27AC /* TunnelObfuscation.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		A9EC20F42A5D96030040D56E /* Midpoint.swift in Sources */ = {isa = PBXBuildFile; fileRef = A9EC20F32A5D96030040D56E /* Midpoint.swift */; };
 		E1187ABC289BBB850024E748 /* OutOfTimeViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1187ABA289BBB850024E748 /* OutOfTimeViewController.swift */; };
 		E1187ABD289BBB850024E748 /* OutOfTimeContentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1187ABB289BBB850024E748 /* OutOfTimeContentView.swift */; };
@@ -1184,7 +1187,9 @@
 		7ABE318C2A1CDD4500DF4963 /* UIFont+Weight.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIFont+Weight.swift"; sourceTree = "<group>"; };
 		7AE47E512A17972A000418DA /* CustomAlertViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomAlertViewController.swift; sourceTree = "<group>"; };
 		7AF0419D29E957EB00D492DD /* AccountCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountCoordinator.swift; sourceTree = "<group>"; };
-		A917351E29FAA9C400D5DCFD /* RESTTransportStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RESTTransportStrategy.swift; sourceTree = "<group>"; };
+		A90F9DE92A6E8B1F009DC8B3 /* UserDefaults+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UserDefaults+Extensions.swift"; sourceTree = "<group>"; };
+		A90F9DEB2A6E91A5009DC8B3 /* AttemptsRecording.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttemptsRecording.swift; sourceTree = "<group>"; };
+		A917351E29FAA9C400D5DCFD /* TransportStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TransportStrategy.swift; sourceTree = "<group>"; };
 		A917352029FAAA5200D5DCFD /* TransportStrategyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TransportStrategyTests.swift; sourceTree = "<group>"; };
 		A9467E7E2A29DEFE000DC21F /* RelayCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayCacheTests.swift; sourceTree = "<group>"; };
 		A9467E872A2DCD57000DC21F /* ShadowsocksConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShadowsocksConfiguration.swift; sourceTree = "<group>"; };
@@ -1435,10 +1440,10 @@
 				5897F1732913EAF800AF5695 /* ExponentialBackoff.swift */,
 				06FAE67528F83CA40033DD93 /* RESTTaskIdentifier.swift */,
 				06FAE67D28F83CA50033DD93 /* RESTTransport.swift */,
-				A917351E29FAA9C400D5DCFD /* RESTTransportStrategy.swift */,
 				06FAE66528F83CA30033DD93 /* RESTURLSession.swift */,
 				06FAE67728F83CA40033DD93 /* ServerRelaysResponse.swift */,
 				06FAE66B28F83CA30033DD93 /* SSLPinningURLSessionDelegate.swift */,
+				A917351E29FAA9C400D5DCFD /* TransportStrategy.swift */,
 			);
 			path = MullvadREST;
 			sourceTree = "<group>";
@@ -1472,6 +1477,7 @@
 			children = (
 				584D26BE270C550B004EA533 /* AnyIPAddress.swift */,
 				586A951329013235007BAF2B /* AnyIPEndpoint.swift */,
+				A90F9DEB2A6E91A5009DC8B3 /* AttemptsRecording.swift */,
 				06AC113628F83FD70037AF9A /* Cancellable.swift */,
 				A9A8A8EA2A262AB30086D569 /* FileCache.swift */,
 				58E511E328DDDE8900B0BCDE /* CustomErrorDescriptionProtocol.swift */,
@@ -1523,6 +1529,7 @@
 				5803B4B12940A48700C23744 /* TunnelStore.swift */,
 				5842102F282D8A3C00F24E46 /* UpdateAccountDataOperation.swift */,
 				58421031282E42B000F24E46 /* UpdateDeviceDataOperation.swift */,
+				A90F9DE92A6E8B1F009DC8B3 /* UserDefaults+Extensions.swift */,
 				581DA2742A1E283E0046ED47 /* WgKeyRotation.swift */,
 			);
 			path = TunnelManager;
@@ -3092,7 +3099,7 @@
 				06799ADF28F98E4800ACD94E /* RESTDevicesProxy.swift in Sources */,
 				06799ADA28F98E4800ACD94E /* RESTResponseHandler.swift in Sources */,
 				062B45BC28FD8C3B00746E77 /* RESTDefaults.swift in Sources */,
-				A917351F29FAA9C400D5DCFD /* RESTTransportStrategy.swift in Sources */,
+				A917351F29FAA9C400D5DCFD /* TransportStrategy.swift in Sources */,
 				06799AE428F98E4800ACD94E /* RESTAccountsProxy.swift in Sources */,
 				5897F1742913EAF800AF5695 /* ExponentialBackoff.swift in Sources */,
 				06799AE328F98E4800ACD94E /* RESTNetworkOperation.swift in Sources */,
@@ -3267,6 +3274,7 @@
 				582AE3102440A6CA00E6733A /* InputTextFormatter.swift in Sources */,
 				5820EDAB288FF0D2006BF4E4 /* DeviceRowView.swift in Sources */,
 				F0E8CC0C2A4EE672007ED3B4 /* SetupAccountCompletedViewController.swift in Sources */,
+				A90F9DEE2A6E92AD009DC8B3 /* UserDefaults+Extensions.swift in Sources */,
 				5846227726E22A7C0035F7C2 /* StorePaymentManagerDelegate.swift in Sources */,
 				58EF581125D69DB400AEBA94 /* StatusImageView.swift in Sources */,
 				58907D9524D17B4E00CFC3F5 /* DisconnectSplitButton.swift in Sources */,
@@ -3435,6 +3443,7 @@
 				5877D70F282137E8002FCFC7 /* SettingsManager.swift in Sources */,
 				58CE38C728992C8700A6D6E5 /* WireGuardAdapterError+Localization.swift in Sources */,
 				58E511E828DDDF2400B0BCDE /* CodingErrors+CustomErrorDescription.swift in Sources */,
+				A90F9DED2A6E929E009DC8B3 /* UserDefaults+Extensions.swift in Sources */,
 				58FDF2D92A0BA11A00C2B061 /* DeviceCheckOperation.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
@@ -3475,6 +3484,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				58D22406294C90210029F5F8 /* IPv4Endpoint.swift in Sources */,
+				A90F9DEC2A6E91A5009DC8B3 /* AttemptsRecording.swift in Sources */,
 				58D22407294C90210029F5F8 /* IPv6Endpoint.swift in Sources */,
 				58CAFA032985367600BE19F7 /* Promise.swift in Sources */,
 				A97FF5502A0D2FFC00900996 /* NSFileCoordinator+Extensions.swift in Sources */,

ios/MullvadVPN/AppDelegate.swift

@@ -100,11 +100,22 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
 
         let urlSessionTransport = URLSessionTransport(urlSession: REST.makeURLSession())
         let shadowsocksCache = ShadowsocksConfigurationCache(cacheDirectory: containerURL)
+
+        // This init cannot fail as long as the security group identifier is valid
+        let sharedUserDefaults = UserDefaults(suiteName: ApplicationConfiguration.securityGroupIdentifier)!
+        let connectionAttempts = sharedUserDefaults
+            .integer(forKey: ApplicationConfiguration.connectionAttemptsSharedCacheKey)
+        let transportStrategy = TransportStrategy(
+            connectionAttempts: connectionAttempts,
+            attemptsRecorder: sharedUserDefaults
+        )
+
         let transportProvider = TransportProvider(
             urlSessionTransport: urlSessionTransport,
             relayCache: relayCache,
             addressCache: addressCache,
             shadowsocksCache: shadowsocksCache,
+            transportStrategy: transportStrategy,
             constraintsUpdater: constraintsUpdater
         )
 

ios/MullvadVPN/TunnelManager/UserDefaults+Extensions.swift

@@ -0,0 +1,16 @@
+//
+//  UserDefaults+Extensions.swift
+//  TunnelManager
+//
+//  Created by Marco Nikic on 2023-07-24.
+//  Copyright © 2023 Mullvad VPN AB. All rights reserved.
+//
+
+import Foundation
+import MullvadTypes
+
+extension UserDefaults: AttemptsRecording {
+    public func record(_ attempts: Int) {
+        set(attempts, forKey: ApplicationConfiguration.connectionAttemptsSharedCacheKey)
+    }
+}

ios/PacketTunnel/PacketTunnelProvider.swift

@@ -145,11 +145,22 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate {
         let urlSession = REST.makeURLSession()
         let urlSessionTransport = URLSessionTransport(urlSession: urlSession)
         let shadowsocksCache = ShadowsocksConfigurationCache(cacheDirectory: containerURL)
+
+        // This init cannot fail as long as the security group identifier is valid
+        let sharedUserDefaults = UserDefaults(suiteName: ApplicationConfiguration.securityGroupIdentifier)!
+        let connectionAttempts = sharedUserDefaults
+            .integer(forKey: ApplicationConfiguration.connectionAttemptsSharedCacheKey)
+        let transportStrategy = TransportStrategy(
+            connectionAttempts: connectionAttempts,
+            attemptsRecorder: sharedUserDefaults
+        )
+
         let transportProvider = TransportProvider(
             urlSessionTransport: urlSessionTransport,
             relayCache: relayCache,
             addressCache: addressCache,
             shadowsocksCache: shadowsocksCache,
+            transportStrategy: transportStrategy,
             constraintsUpdater: constraintsUpdater
         )
 

ios/Shared/ApplicationConfiguration.swift

@@ -36,4 +36,7 @@ enum ApplicationConfiguration {
 
     /// Maximum number of devices per account.
     static let maxAllowedDevices = 5
+
+    /// `UserDefaults` key shared by both processes. Used to cache and synchronize connection attempts between them.
+    static let connectionAttemptsSharedCacheKey = "ConnectionAttemptsSharedCacheKey"
 }