Skip to content

Commit

Permalink
VPN server failure detection recovery (#786)
Browse files Browse the repository at this point in the history
* Small tidy up to remove dependency cycle

* Inject settings rather than environment to client

* Add failure recovery register request

* Add log category for failure recovery

* Label the config+server tuple values

* Store server not just info

* Abstract reasserting from PacketTunnelProvider

* Create failurerecoveryHandler + empty tests

* Pull updating adapter config into helper function

* Pull deviceManager into a lazy var

* Add failure monitoring and recovery to PTP

* Remove location from request

* Don't regenerate the key on failure recovery

* Fix broken existing tests

* Write tests for failureRecoveryHandler

* Add comment about todo pixel

* Update function name to be more consistent

* Make sure tunnelUpdateAttempt pixels are fired

* Fix swiftlint

* Add initial retry logic

* Add more logging

* Fixed no recovery errors

* Update tests for failure recovery

* Stop failure monitor when handshake succeeds

* Tidy up from PR feedback

* Add events for pixels

* Tidy up tests

* Swiftlint fixes

* Tidy up error handling

* Fix reasserting problem

* Swiftlint

* Another swiftlint fix

* Make devicemanager property private

* Remove... x? (wtf)

* Use convenience serverInfo property

Co-authored-by: Anh Do <[email protected]>

* Use the typealiased tuple

Co-authored-by: Anh Do <[email protected]>

* Revert to injecting environment not settings

* Always stop reasserting regardless of result

* Fix retry logic

* Better handle task cancellation

* Improve readability of GenrateTunnelConfigurationResult

* Make sure netP convertible errors are converted

---------

Co-authored-by: Anh Do <[email protected]>
  • Loading branch information
graeme and quanganhdo authored Apr 30, 2024
1 parent 8d8f54f commit 2681b52
Show file tree
Hide file tree
Showing 15 changed files with 922 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public actor NetworkProtectionTunnelFailureMonitor {
task?.isCancelled == false
}

private weak var tunnelProvider: PacketTunnelProvider?
private let handshakeReporter: HandshakeReporting

private let networkMonitor = NWPathMonitor()

Expand All @@ -61,8 +61,8 @@ public actor NetworkProtectionTunnelFailureMonitor {

// MARK: - Init & deinit

init(tunnelProvider: PacketTunnelProvider) {
self.tunnelProvider = tunnelProvider
init(handshakeReporter: HandshakeReporting) {
self.handshakeReporter = handshakeReporter
self.networkMonitor.start(queue: .global())

os_log("[+] %{public}@", log: .networkProtectionMemoryLog, type: .debug, String(describing: self))
Expand Down Expand Up @@ -113,7 +113,7 @@ public actor NetworkProtectionTunnelFailureMonitor {
return
}

let mostRecentHandshake = await tunnelProvider?.mostRecentHandshake() ?? 0
let mostRecentHandshake = (try? await handshakeReporter.getMostRecentHandshake()) ?? 0

guard mostRecentHandshake > 0 else {
os_log("⚫️ Got handshake timestamp at or below 0, skipping check", log: .networkProtectionTunnelFailureMonitorLog, type: .debug)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
//
// WireGuardAdapter+HandshakeReporting.swift
//
// Copyright © 2023 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation

protocol HandshakeReporting {
/// Retrieves the number of seconds of the most recent handshake for the previously added peer entry, expressed relative to the Unix epoch.
///
/// - Throws: ConfigReadingError
/// - Returns: Interval between the most recent handshake and the Unix epoch.
///
func getMostRecentHandshake() async throws -> TimeInterval
}

extension WireGuardAdapter: HandshakeReporting {}
7 changes: 7 additions & 0 deletions Sources/NetworkProtection/Logging/Logging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ extension OSLog {
Logging.networkProtectionTunnelFailureMonitorLoggingEnabled ? Logging.networkProtectionTunnelFailureMonitor : .disabled
}

public static var networkProtectionServerFailureRecoveryLog: OSLog {
Logging.networkProtectionServerFailureRecoveryLoggingEnabled ? Logging.networkProtectionServerFailureRecovery : .disabled
}

public static var networkProtectionConnectionTesterLog: OSLog {
Logging.networkProtectionConnectionTesterLoggingEnabled ? Logging.networkProtectionConnectionTesterLog : .disabled
}
Expand Down Expand Up @@ -90,6 +94,9 @@ struct Logging {
fileprivate static let networkProtectionTunnelFailureMonitorLoggingEnabled = true
fileprivate static let networkProtectionTunnelFailureMonitor: OSLog = OSLog(subsystem: subsystem, category: "Network Protection: Tunnel Failure Monitor")

fileprivate static let networkProtectionServerFailureRecoveryLoggingEnabled = true
fileprivate static let networkProtectionServerFailureRecovery: OSLog = OSLog(subsystem: subsystem, category: "Network Protection: Server Failure Recovery")

fileprivate static let networkProtectionConnectionTesterLoggingEnabled = true
fileprivate static let networkProtectionConnectionTesterLog: OSLog = OSLog(subsystem: subsystem, category: "Network Protection: Connection Tester")

Expand Down
13 changes: 10 additions & 3 deletions Sources/NetworkProtection/NetworkProtectionDeviceManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,26 @@ public enum NetworkProtectionServerSelectionMethod: CustomDebugStringConvertible
"avoidServer: \(serverName)"
case .preferredLocation(let location):
"preferredLocation: \(location)"
case .failureRecovery(serverName: let serverName):
"failureRecovery: \(serverName)"
}
}

case automatic
case preferredServer(serverName: String)
case avoidServer(serverName: String)
case preferredLocation(NetworkProtectionSelectedLocation)
case failureRecovery(serverName: String)
}

public protocol NetworkProtectionDeviceManagement {
typealias GenerateTunnelConfigurationResult = (tunnelConfiguration: TunnelConfiguration, server: NetworkProtectionServer)

func generateTunnelConfiguration(selectionMethod: NetworkProtectionServerSelectionMethod,
includedRoutes: [IPAddressRange],
excludedRoutes: [IPAddressRange],
isKillSwitchEnabled: Bool,
regenerateKey: Bool) async throws -> (TunnelConfiguration, NetworkProtectionServerInfo)
regenerateKey: Bool) async throws -> GenerateTunnelConfigurationResult

}

Expand Down Expand Up @@ -115,7 +119,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
includedRoutes: [IPAddressRange],
excludedRoutes: [IPAddressRange],
isKillSwitchEnabled: Bool,
regenerateKey: Bool) async throws -> (TunnelConfiguration, NetworkProtectionServerInfo) {
regenerateKey: Bool) async throws -> GenerateTunnelConfigurationResult {
var keyPair: KeyPair

if regenerateKey {
Expand Down Expand Up @@ -153,7 +157,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
includedRoutes: includedRoutes,
excludedRoutes: excludedRoutes,
isKillSwitchEnabled: isKillSwitchEnabled)
return (configuration, selectedServer.serverInfo)
return (configuration, selectedServer)
} catch let error as NetworkProtectionError {
errorEvents?.fire(error)
throw error
Expand Down Expand Up @@ -194,6 +198,9 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement {
case .preferredLocation(let location):
serverSelection = .location(country: location.country, city: location.city)
excludedServerName = nil
case .failureRecovery(serverName: let serverName):
serverSelection = .recovery(server: serverName)
excludedServerName = nil
}

let requestBody = RegisterKeyRequestBody(publicKey: keyPair.publicKey,
Expand Down
16 changes: 16 additions & 0 deletions Sources/NetworkProtection/Networking/NetworkProtectionClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ struct RegisterKeyRequestBody: Encodable {
let server: String?
let country: String?
let city: String?
let mode: String?

init(publicKey: PublicKey,
serverSelection: RegisterServerSelection) {
Expand All @@ -103,14 +104,29 @@ struct RegisterKeyRequestBody: Encodable {
server = nil
self.country = country
self.city = city
case .recovery(server: let server):
self.server = server
self.country = nil
self.city = nil
}
mode = serverSelection.mode
}
}

enum RegisterServerSelection {
case automatic
case server(name: String)
case location(country: String, city: String?)
case recovery(server: String)

var mode: String? {
switch self {
case .recovery:
"failureRecovery"
case .automatic, .location, .server:
nil
}
}
}

struct RedeemInviteCodeRequestBody: Encodable {
Expand Down
Loading

0 comments on commit 2681b52

Please sign in to comment.