Skip to content

Commit

Permalink
Allow settings migration from packet tunnel
Browse files Browse the repository at this point in the history
  • Loading branch information
@buggmagnet
buggmagnet committed Oct 8, 2024
1 parent b54a6c3 commit 3ec12b7
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 23 deletions.
10 changes: 10 additions & 0 deletions ios/MullvadREST/RetryStrategy/RetryStrategy.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ extension REST {
),
applyJitter: true
)

public static var failedMigrationRecovery = RetryStrategy(
maxRetryCount: .max,
delay: .exponentialBackoff(
initial: .seconds(10),
multiplier: UInt64(2),
maxDelay: .minutes(5)
),
applyJitter: true
)
}

public enum RetryDelay: Equatable {
Expand Down
50 changes: 33 additions & 17 deletions ios/MullvadSettings/MigrationManager.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,38 +23,54 @@ public enum SettingsMigrationResult {

public struct MigrationManager {
private let logger = Logger(label: "MigrationManager")
private let cacheDirectory: URL

public init() {}
public init(cacheDirectory: URL) {
self.cacheDirectory = cacheDirectory.appendingPathComponent("migrationState.json")
}

/// Migrate settings store if needed.
///
/// Reads the current settings, upgrades them to the latest version if needed
/// and writes back to `store` when settings are updated.
///
/// In order to avoid migration happening from both the VPN and the host processes at the same time,
/// a non existant file path is used as a lock to synchronize access between the processes.
/// This file is accessed by `NSFileCoordinator` in order to prevent multiple processes accessing at the same time.
/// - Parameters:
/// - store: The store to from which settings are read and written to.
/// - proxyFactory: Factory used for migrations that involve API calls.
/// - migrationCompleted: Completion handler called with a migration result.
public func migrateSettings(
store: SettingsStore,
migrationCompleted: @escaping (SettingsMigrationResult) -> Void
) {
let resetStoreHandler = { (result: SettingsMigrationResult) in
// Reset store upon failure to migrate settings.
if case .failure = result {
SettingsManager.resetStore()
let fileCoordinator = NSFileCoordinator(filePresenter: nil)
var error: NSError?

// This will block the calling thread if another process is currently running the same code.
// This is intentional to avoid TOCTOU issues, and guaranteeing settings cannot be read
// in a half written state.
// The resulting effect is that only one process at a time can do settings migrations.
// The other process will be blocked, and will have nothing to do as long as settings were successfully upgraded.
fileCoordinator.coordinate(writingItemAt: cacheDirectory, error: &error) { _ in
let resetStoreHandler = { (result: SettingsMigrationResult) in
// Reset store upon failure to migrate settings.
if case .failure = result {
SettingsManager.resetStore()
}
migrationCompleted(result)
}
migrationCompleted(result)
}

do {
try upgradeSettingsToLatestVersion(
store: store,
migrationCompleted: migrationCompleted
)
} catch .itemNotFound as KeychainError {
migrationCompleted(.nothing)
} catch {
resetStoreHandler(.failure(error))
do {
try upgradeSettingsToLatestVersion(
store: store,
migrationCompleted: migrationCompleted
)
} catch .itemNotFound as KeychainError {
migrationCompleted(.nothing)
} catch {
resetStoreHandler(.failure(error))
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions ios/MullvadVPN.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@
A932D9F32B5EB61100999395 /* HeadRequestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A932D9F22B5EB61100999395 /* HeadRequestTests.swift */; };
A932D9F52B5EBB9D00999395 /* RESTTransportStub.swift in Sources */ = {isa = PBXBuildFile; fileRef = A932D9F42B5EBB9D00999395 /* RESTTransportStub.swift */; };
A935594C2B4C2DA900D5D524 /* APIAvailabilityTestRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = A935594B2B4C2DA900D5D524 /* APIAvailabilityTestRequest.swift */; };
A939661B2CAE6CE1008128CA /* MigrationManagerMultiProcessUpgradeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A939661A2CAE6CE1008128CA /* MigrationManagerMultiProcessUpgradeTests.swift */; };
A94D691A2ABAD66700413DD4 /* WireGuardKitTypes in Frameworks */ = {isa = PBXBuildFile; productRef = 58FE25E22AA72AE9003D1918 /* WireGuardKitTypes */; };
A94D691B2ABAD66700413DD4 /* WireGuardKitTypes in Frameworks */ = {isa = PBXBuildFile; productRef = 58FE25E72AA7399D003D1918 /* WireGuardKitTypes */; };
A95EEE362B722CD600A8A39B /* TunnelMonitorState.swift in Sources */ = {isa = PBXBuildFile; fileRef = A95EEE352B722CD600A8A39B /* TunnelMonitorState.swift */; };
Expand Down Expand Up @@ -2020,6 +2021,7 @@
A932D9F42B5EBB9D00999395 /* RESTTransportStub.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RESTTransportStub.swift; sourceTree = "<group>"; };
A935594B2B4C2DA900D5D524 /* APIAvailabilityTestRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = APIAvailabilityTestRequest.swift; sourceTree = "<group>"; };
A935594D2B4E919F00D5D524 /* Api.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Api.xcconfig; sourceTree = "<group>"; };
A939661A2CAE6CE1008128CA /* MigrationManagerMultiProcessUpgradeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MigrationManagerMultiProcessUpgradeTests.swift; sourceTree = "<group>"; };
A944F2712B8E02E800473F4C /* libmullvad_ios.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libmullvad_ios.a; path = "../target/aarch64-apple-ios/debug/libmullvad_ios.a"; sourceTree = "<group>"; };
A9467E7E2A29DEFE000DC21F /* RelayCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayCacheTests.swift; sourceTree = "<group>"; };
A948809A2BC9308D0090A44C /* EphemeralPeerExchangeActor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EphemeralPeerExchangeActor.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2574,6 +2576,7 @@
7A5869C22B5820CE00640D27 /* IPOverrideRepositoryTests.swift */,
7AB4CCB82B69097E006037F5 /* IPOverrideTests.swift */,
7A516C3B2B712F0B00BBD33D /* IPOverrideWrapperTests.swift */,
A939661A2CAE6CE1008128CA /* MigrationManagerMultiProcessUpgradeTests.swift */,
A9B6AC172ADE8F4300F7802A /* MigrationManagerTests.swift */,
F072D3CE2C07122400906F64 /* SettingsUpdaterTests.swift */,
449872E32B7CB96300094DDC /* TunnelSettingsUpdateTests.swift */,
Expand Down Expand Up @@ -5377,6 +5380,7 @@
7A9BE5A52B90760C00E2A7D0 /* CustomListsDataSourceTests.swift in Sources */,
A9A5FA1B2ACB05160083449F /* Tunnel.swift in Sources */,
A9A5FA1C2ACB05160083449F /* Tunnel+Messaging.swift in Sources */,
A939661B2CAE6CE1008128CA /* MigrationManagerMultiProcessUpgradeTests.swift in Sources */,
7A9BE5A92B90806800E2A7D0 /* CustomListsRepositoryStub.swift in Sources */,
F09D04BB2AE95396003D4F89 /* URLSessionStub.swift in Sources */,
A9A5FA1D2ACB05160083449F /* TunnelBlockObserver.swift in Sources */,
Expand Down
8 changes: 5 additions & 3 deletions ios/MullvadVPN/AppDelegate.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
private(set) var storePaymentManager: StorePaymentManager!
private var transportMonitor: TransportMonitor!
private var settingsObserver: TunnelBlockObserver!
private let migrationManager = MigrationManager()
private var migrationManager: MigrationManager!

private(set) var accessMethodRepository = AccessMethodRepository()
private(set) var shadowsocksLoader: ShadowsocksLoaderProtocol!
Expand All @@ -67,7 +67,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
}

let containerURL = ApplicationConfiguration.containerURL

migrationManager = MigrationManager(cacheDirectory: containerURL)
configureLogging()

addressCache = REST.AddressCache(canWriteToCache: true, cacheDirectory: containerURL)
Expand Down Expand Up @@ -478,14 +478,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
switch migrationResult {
case .success:
// Tell the tunnel to re-read tunnel configuration after migration.
logger.debug("Reconnect the tunnel after settings migration.")
logger.debug("Successful migration from UI Process")
tunnelManager.reconnectTunnel(selectNewRelay: true)
fallthrough

case .nothing:
logger.debug("Attempted migration from UI Process, but found nothing to do")
finish(nil)

case let .failure(error):
logger.error("Failed migration from UI Process: \(error)")
let migrationUIHandler = application.connectedScenes
.first { $0 is SettingsMigrationUIHandler } as? SettingsMigrationUIHandler

Expand Down
58 changes: 58 additions & 0 deletions ios/MullvadVPNTests/MullvadSettings/MigrationManagerMultiProcessUpgradeTests.swift
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
//
// MigrationManagerMultiProcessUpgradeTests.swift
// MullvadVPNTests
//
// Created by Marco Nikic on 2024-10-03.
// Copyright © 2024 Mullvad VPN AB. All rights reserved.
//

@testable import MullvadMockData
@testable import MullvadREST
@testable import MullvadSettings
@testable import MullvadTypes
import XCTest

extension MigrationManagerTests {
func testMigrationDoesNothingIfAnotherProcessIsRunningUpdates() throws {
let hostProcess = DispatchQueue(label: "net.tests.HostMigration")
let packetTunnelProcess = DispatchQueue(label: "net.tests.PacketTunnelMigration")
let osakaRelayConstraints = RelayConstraints(
exitLocations: .only(UserSelectedRelays(locations: [.city("jp", "osa")]))
)
var settingsV1 = TunnelSettingsV1()
settingsV1.relayConstraints = osakaRelayConstraints

try write(settings: settingsV1, version: SchemaVersion.v1.rawValue, in: Self.store)

let backgroundMigrationExpectation = expectation(description: "Migration from packet tunnel")
let foregroundMigrationExpectation = expectation(description: "Migration from host")
var migrationHappenedInPacketTunnel = false
var migrationHappenedInHost = false

packetTunnelProcess.async { [unowned self] in
manager.migrateSettings(store: MigrationManagerTests.store) { backgroundMigrationResult in
if case .success = backgroundMigrationResult {
migrationHappenedInPacketTunnel = true
}
backgroundMigrationExpectation.fulfill()
}
}

hostProcess.async { [unowned self] in
manager.migrateSettings(store: MigrationManagerTests.store) { foregroundMigrationResult in
if case .success = foregroundMigrationResult {
migrationHappenedInHost = true
}
foregroundMigrationExpectation.fulfill()
}
}

wait(for: [backgroundMigrationExpectation, foregroundMigrationExpectation], timeout: .UnitTest.invertedTimeout)

// Migration happens either in one process, or the other.
// This check guarantees it didn't happen in both simultaneously.
XCTAssertNotEqual(migrationHappenedInPacketTunnel, migrationHappenedInHost)
let latestSettings = try SettingsManager.readSettings()
XCTAssertEqual(osakaRelayConstraints, latestSettings.relayConstraints)
}
}
12 changes: 10 additions & 2 deletions ios/MullvadVPNTests/MullvadSettings/MigrationManagerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ final class MigrationManagerTests: XCTestCase {
static let store = InMemorySettingsStore<SettingNotFound>()

var manager: MigrationManager!
var testFileURL: URL!
override static func setUp() {
SettingsManager.unitTestStore = store
}
Expand All @@ -25,7 +26,14 @@ final class MigrationManagerTests: XCTestCase {
}

override func setUpWithError() throws {
manager = MigrationManager()
testFileURL = FileManager.default.temporaryDirectory
.appendingPathComponent("MigrationManagerTests-\(UUID().uuidString)", isDirectory: true)
try FileManager.default.createDirectory(at: testFileURL, withIntermediateDirectories: true)
manager = MigrationManager(cacheDirectory: testFileURL)
}

override func tearDownWithError() throws {
try FileManager.default.removeItem(at: testFileURL)
}

func testNothingToMigrate() throws {
Expand Down Expand Up @@ -224,7 +232,7 @@ final class MigrationManagerTests: XCTestCase {
wait(for: [successfulMigrationExpectation], timeout: .UnitTest.timeout)
}

private func write(settings: any TunnelSettings, version: Int, in store: SettingsStore) throws {
func write(settings: any TunnelSettings, version: Int, in store: SettingsStore) throws {
let parser = SettingsParser(decoder: JSONDecoder(), encoder: JSONEncoder())
let payload = try parser.producePayload(settings, version: version)
try store.write(payload, for: .settings)
Expand Down
2 changes: 1 addition & 1 deletion ios/MullvadVPNTests/MullvadTypes/FileCacheTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FileCacheTests: XCTestCase {

override func setUp() {
testFileURL = FileManager.default.temporaryDirectory
.appendingPathComponent("FileCacheTest-\(UUID().uuidString)", isDirectory: true)
.appendingPathComponent("FileCacheTest-\(UUID().uuidString)", isDirectory: false)
}

override func tearDown() {
Expand Down
29 changes: 29 additions & 0 deletions ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
private var ephemeralPeerExchangingPipeline: EphemeralPeerExchangingPipeline!
private let tunnelSettingsUpdater: SettingsUpdater!
private var encryptedDNSTransport: EncryptedDNSTransport!
private var migrationManager: MigrationManager!
let migrationFailureIterator = REST.RetryStrategy.failedMigrationRecovery.makeDelayIterator()

private let tunnelSettingsListener = TunnelSettingsListener()
private lazy var ephemeralPeerReceiver = {
Expand All @@ -49,9 +51,12 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
ipOverrideRepository: IPOverrideRepository()
)
tunnelSettingsUpdater = SettingsUpdater(listener: tunnelSettingsListener)
migrationManager = MigrationManager(cacheDirectory: containerURL)

super.init()

performSettingsMigration()

let transportProvider = setUpTransportProvider(
appContainerURL: containerURL,
ipOverrideWrapper: ipOverrideWrapper,
Expand Down Expand Up @@ -168,6 +173,30 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
actor.onWake()
}

private func performSettingsMigration() {
migrationManager.migrateSettings(
store: SettingsManager.store,
migrationCompleted: { [unowned self] migrationResult in
switch migrationResult {
case .success:
providerLogger.debug("Successful migration from PacketTunnel")
case .nothing:
providerLogger.debug("Attempted migration from PacketTunnel, but found nothing to do")
case let .failure(error):
// `next` returns an Optional value, but this iterator is guaranteed to always have a next value
guard let delay = migrationFailureIterator.next() else { return }
providerLogger
.error(
"Failed migration from PacketTunnel: \(error), retrying in \(delay.timeInterval) seconds"
)
DispatchQueue.main.asyncAfter(deadline: .now() + delay.timeInterval) { [unowned self] in
performSettingsMigration()
}
}
}
)
}

private func setUpTransportProvider(
appContainerURL: URL,
ipOverrideWrapper: IPOverrideWrapper,
Expand Down

0 comments on commit 3ec12b7

Please sign in to comment.