Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VPN now recovers from WireGuard closing utun #931

Merged
merged 5 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ protocol NetworkProtectionErrorConvertible {
}

public enum NetworkProtectionError: LocalizedError, CustomNSError {

// Tunnel configuration errors
case noServerRegistrationInfo
case couldNotSelectClosestServer
Expand Down Expand Up @@ -60,7 +61,8 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
case wireGuardInvalidState(reason: String)
case wireGuardDnsResolution
case wireGuardSetNetworkSettings(Error)
case startWireGuardBackend(Int32)
case startWireGuardBackend(Error)
case setWireguardConfig(Error)
Comment on lines +64 to +65
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's cleaner and easier to manage if we just send underlying errors.


// Auth errors
case noAuthTokenFound
Expand Down Expand Up @@ -110,6 +112,7 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
case .wireGuardDnsResolution: return 302
case .wireGuardSetNetworkSettings: return 303
case .startWireGuardBackend: return 304
case .setWireguardConfig: return 305
// 400+ Auth errors
case .noAuthTokenFound: return 400
// 500+ Subscription errors
Expand Down Expand Up @@ -140,7 +143,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
.wireGuardCannotLocateTunnelFileDescriptor,
.wireGuardInvalidState,
.wireGuardDnsResolution,
.startWireGuardBackend,
.noAuthTokenFound,
.vpnAccessRevoked:
return [:]
Expand All @@ -160,6 +162,8 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
.failedToParseRegisteredServersResponse(let error),
.failedToParseRedeemResponse(let error),
.wireGuardSetNetworkSettings(let error),
.startWireGuardBackend(let error),
.setWireguardConfig(let error),
.unhandledError(_, _, let error),
.failedToFetchServerStatus(let error),
.failedToParseServerStatusResponse(let error):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ extension WireGuardAdapterError: NetworkProtectionErrorConvertible {
return .wireGuardDnsResolution
case .setNetworkSettings(let error):
return .wireGuardSetNetworkSettings(error)
case .startWireGuardBackend(let code):
return .startWireGuardBackend(code)
case .startWireGuardBackend(let error):
return .startWireGuardBackend(error)
case .setWireguardConfig(let error):
return .setWireguardConfig(error)
}
}
}
26 changes: 21 additions & 5 deletions Sources/NetworkProtection/PacketTunnelProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,17 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {

// MARK: - WireGuard

private lazy var adapter: WireGuardAdapter = {
func makeAdapter(with packetTunnelProvider: PacketTunnelProvider) -> WireGuardAdapter {
WireGuardAdapter(with: self) { logLevel, message in
if logLevel == .error {
os_log("🔴 Received error from adapter: %{public}@", log: .networkProtection, type: .error, message)
} else {
os_log("Received message from adapter: %{public}@", log: .networkProtection, message)
}
}
}()
}

private lazy var adapter = makeAdapter(with: self)

// MARK: - Timers Support

Expand Down Expand Up @@ -913,9 +915,9 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
await stopMonitors()
}

do {
let tunnelConfiguration: TunnelConfiguration
var tunnelConfiguration: TunnelConfiguration?

do {
switch updateMethod {
case .selectServer(let serverSelectionMethod):
tunnelConfiguration = try await generateTunnelConfiguration(serverSelectionMethod: serverSelectionMethod,
Expand All @@ -928,7 +930,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
tunnelConfiguration = newTunnelConfiguration
}

try await updateAdapterConfiguration(tunnelConfiguration: tunnelConfiguration, reassert: reassert)
try await updateAdapterConfiguration(tunnelConfiguration: tunnelConfiguration!, reassert: reassert)

if reassert {
try await handleAdapterStarted(startReason: .reconnected)
Expand All @@ -937,21 +939,32 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
providerEvents.fire(.tunnelUpdateAttempt(.success))
} catch {
providerEvents.fire(.tunnelUpdateAttempt(.failure(error)))

switch error {
case WireGuardAdapterError.setWireguardConfig:
await cancelTunnel(with: error)
default:
break
}

throw error
}
}

@MainActor
private func updateAdapterConfiguration(tunnelConfiguration: TunnelConfiguration, reassert: Bool) async throws {

try await withCheckedThrowingContinuation { [weak self] (continuation: CheckedContinuation<Void, Error>) in
guard let self = self else {
continuation.resume()
return
}

self.adapter.update(tunnelConfiguration: tunnelConfiguration, reassert: reassert) { [weak self] error in

if let error = error {
self?.debugEvents?.fire(error.networkProtectionError)

continuation.resume(throwing: error)
return
}
Expand Down Expand Up @@ -1803,6 +1816,9 @@ extension WireGuardAdapterError: LocalizedError, CustomDebugStringConvertible {
case .startWireGuardBackend(let errorCode):
return "Starting tunnel failed with wgTurnOn returning: \(errorCode)"

case .setWireguardConfig(let errorCode):
return "Update tunnel failed with wgSetConfig returning: \(errorCode)"

case .invalidState:
return "Starting tunnel failed with invalid error"
}
Expand Down
26 changes: 21 additions & 5 deletions Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ public enum WireGuardAdapterError: CustomNSError {
case setNetworkSettings(Error)

/// Failure to start WireGuard backend.
case startWireGuardBackend(Int32)
case startWireGuardBackend(Error)

/// Failure to set the configuration for the WireGuard adapter
case setWireguardConfig(Error)

static let wireguardAdapterDomain = "WireGuardAdapter"

public var errorCode: Int {
switch self {
Expand All @@ -36,6 +41,7 @@ public enum WireGuardAdapterError: CustomNSError {
case .dnsResolution: return 102
case .setNetworkSettings: return 103
case .startWireGuardBackend: return 104
case .setWireguardConfig: return 105
}
}

Expand All @@ -52,8 +58,9 @@ public enum WireGuardAdapterError: CustomNSError {
return [NSUnderlyingErrorKey: firstError as NSError]
case .setNetworkSettings(let error):
return [NSUnderlyingErrorKey: error as NSError]
case .startWireGuardBackend(let code):
let error = NSError(domain: "startWireGuardBackend", code: Int(code))
case .startWireGuardBackend(let error):
return [NSUnderlyingErrorKey: error as NSError]
case .setWireguardConfig(let error):
return [NSUnderlyingErrorKey: error as NSError]
}
}
Expand Down Expand Up @@ -423,7 +430,14 @@ public class WireGuardAdapter {
let (wgConfig, resolutionResults) = settingsGenerator.uapiConfiguration()
self.logEndpointResolutionResults(resolutionResults)

wgSetConfig(handle, wgConfig)
let result = wgSetConfig(handle, wgConfig)

if result < 0 {
let error = NSError(domain: WireGuardAdapterError.wireguardAdapterDomain,
code: Int(result))
throw WireGuardAdapterError.setWireguardConfig(error)
}

#if os(iOS)
wgDisableSomeRoamingForBrokenMobileSemantics(handle)
#endif
Expand Down Expand Up @@ -539,7 +553,9 @@ public class WireGuardAdapter {

let handle = wgTurnOn(wgConfig, tunnelFileDescriptor)
if handle < 0 {
throw WireGuardAdapterError.startWireGuardBackend(handle)
let error = NSError(domain: WireGuardAdapterError.wireguardAdapterDomain,
code: Int(handle))
throw WireGuardAdapterError.startWireGuardBackend(error)
}
#if os(iOS)
wgDisableSomeRoamingForBrokenMobileSemantics(handle)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ final class NetworkProtectionErrorTests: XCTestCase {
.keychainUpdateError(field: "test", status: 1),
.keychainDeleteError(status: 1),
.wireGuardInvalidState(reason: "test"),
.startWireGuardBackend(1),

]

for error in errorsWithoutUnderlyingError {
Expand All @@ -77,6 +75,7 @@ final class NetworkProtectionErrorTests: XCTestCase {
.failedToRedeemInviteCode(underlyingError),
.failedToParseRedeemResponse(underlyingError),
.wireGuardSetNetworkSettings(underlyingError),
.startWireGuardBackend(underlyingError),
.unhandledError(function: #function, line: #line, error: underlyingError),
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ final class FailureRecoveryHandlerTests: XCTestCase {
dnsSettings: .default,
isKillSwitchEnabled: false
) { _ in
throw WireGuardAdapterError.startWireGuardBackend(0)
let underlyingError = NSError(domain: "test", code: 1)
throw WireGuardAdapterError.startWireGuardBackend(underlyingError)
}
}

Expand Down
Loading