From 28ce4d516435306b929aba4a7ccbed00bf16e309 Mon Sep 17 00:00:00 2001 From: Roopesh Chander Date: Fri, 21 Dec 2018 15:40:04 +0530 Subject: [PATCH] NE: Change handling of bad domain names and Activate On Demand The solution implemented in commit b8c331c causes the tunnel to remain in 'Activating' state, without the ability to cancel that. So, in this commit, instead of retrying DNS silently on Activated-On-Demand tunnels, we fail the startTunnel() silently. To summarize, if activate-on-demand is on: - If started from the WireGuard app, show error using lastErrorFile mechanism, suggesting a way to turn off Activate On Demand - If not started from WireGuard app, don't call displayMessage() (don't show error to user) and silently fail starting the tunnel Signed-off-by: Roopesh Chander --- .../ErrorNotifier.swift | 26 ++++++----- .../PacketTunnelProvider.swift | 46 +++++++------------ 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift b/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift index 02fbd4c..1b74d5d 100644 --- a/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift +++ b/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift @@ -8,6 +8,9 @@ class ErrorNotifier { let activationAttemptId: String? weak var tunnelProvider: NEPacketTunnelProvider? + var tunnelName: String? + var isActivateOnDemandEnabled = false + init(activationAttemptId: String?, tunnelProvider: NEPacketTunnelProvider) { self.activationAttemptId = activationAttemptId self.tunnelProvider = tunnelProvider @@ -17,17 +20,13 @@ class ErrorNotifier { func errorMessage(for error: PacketTunnelProviderError) -> (String, String)? { switch error { case .savedProtocolConfigurationIsInvalid: - return ("Activation failure", "Could not retrieve tunnel information from the saved configuration") - case .dnsResolutionFailure(let tunnelName, let isActivateOnDemandEnabled): - if isActivateOnDemandEnabled { - return ("DNS resolution failure", "This tunnel has Activate On Demand enabled, so activation might be retried. You may turn off Activate On Demand in the WireGuard app by navigating to: '\(tunnelName)' > Edit") - } else { - return ("DNS resolution failure", "One or more endpoint domains could not be resolved") - } + return ("Activation failure", "Could not retrieve tunnel information from the saved configuration.") + case .dnsResolutionFailure: + return ("DNS resolution failure", "One or more endpoint domains could not be resolved.") case .couldNotStartWireGuard: - return ("Activation failure", "WireGuard backend could not be started") + return ("Activation failure", "WireGuard backend could not be started.") case .coultNotSetNetworkSettings: - return ("Activation failure", "Error applying network settings on the tunnel") + return ("Activation failure", "Error applying network settings on the tunnel.") } } @@ -35,13 +34,16 @@ class ErrorNotifier { guard let (title, message) = errorMessage(for: error) else { return } if let activationAttemptId = activationAttemptId, let lastErrorFilePath = FileManager.networkExtensionLastErrorFileURL?.path { // The tunnel was started from the app - let errorMessageData = "\(activationAttemptId)\n\(title)\n\(message)".data(using: .utf8) + let onDemandMessage = isActivateOnDemandEnabled ? " This tunnel has Activate On Demand enabled, so this tunnel might be activated automatically. You may turn off Activate On Demand in the WireGuard app by navigating to: '\(tunnelName ?? "tunnel")' > Edit." : "" + let errorMessageData = "\(activationAttemptId)\n\(title)\n\(message)\(onDemandMessage)".data(using: .utf8) FileManager.default.createFile(atPath: lastErrorFilePath, contents: errorMessageData, attributes: nil) } else { - // The tunnel was probably started from iOS Settings app + // The tunnel was probably started from iOS Settings app or activated on-demand if let tunnelProvider = self.tunnelProvider { // displayMessage() is deprecated, but there's no better alternative if invoked from iOS Settings - tunnelProvider.displayMessage("\(title): \(message)") { _ in } + if !isActivateOnDemandEnabled { // If using activate-on-demand, don't use displayMessage + tunnelProvider.displayMessage("\(title): \(message)") { _ in } + } } } } diff --git a/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift b/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift index f0ae3e7..f678ca7 100644 --- a/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift +++ b/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift @@ -8,7 +8,7 @@ import os.log enum PacketTunnelProviderError: Error { case savedProtocolConfigurationIsInvalid - case dnsResolutionFailure(tunnelName: String, isActivateOnDemandEnabled: Bool) + case dnsResolutionFailure case couldNotStartWireGuard case coultNotSetNetworkSettings } @@ -54,9 +54,13 @@ class PacketTunnelProvider: NEPacketTunnelProvider { wg_log(.info, staticMessage: "Tunnel has Activate On Demand disabled") } + errorNotifier.isActivateOnDemandEnabled = isActivateOnDemandEnabled + errorNotifier.tunnelName = tunnelName + let endpoints = tunnelConfiguration.peers.map { $0.endpoint } - guard let resolvedEndpoints = resolveDomainNames(endpoints: endpoints, isActivateOnDemandEnabled: isActivateOnDemandEnabled) else { - let dnsError = PacketTunnelProviderError.dnsResolutionFailure(tunnelName: tunnelName, isActivateOnDemandEnabled: isActivateOnDemandEnabled) + guard let resolvedEndpoints = resolveDomainNames(endpoints: endpoints) else { + wg_log(.error, staticMessage: "Starting tunnel failed: DNS resolution failure") + let dnsError = PacketTunnelProviderError.dnsResolutionFailure errorNotifier.notify(dnsError) startTunnelCompletionHandler(dnsError) return @@ -148,34 +152,16 @@ class PacketTunnelProvider: NEPacketTunnelProvider { } } - private func resolveDomainNames(endpoints: [Endpoint?], isActivateOnDemandEnabled: Bool) -> [Endpoint?]? { - var resolvedEndpoints = [Endpoint?]() - let dnsResolutionAttemptsCount = isActivateOnDemandEnabled ? 10 : 1 - var isDNSResolved = false - - for attemptIndex in 0 ..< dnsResolutionAttemptsCount { - do { - resolvedEndpoints = try DNSResolver.resolveSync(endpoints: endpoints) - isDNSResolved = true - } catch DNSResolverError.dnsResolutionFailed(let hostnames) { - wg_log(.error, staticMessage: "Starting tunnel failed: DNS resolution failure") - wg_log(.error, message: "Hostnames for which DNS resolution failed: \(hostnames.joined(separator: ", "))") - } catch { - // There can be no other errors from DNSResolver.resolveSync() - fatalError() - } - if isDNSResolved { - break - } else { - let isLastAttempt = attemptIndex == dnsResolutionAttemptsCount - 1 - if !isLastAttempt { - Thread.sleep(forTimeInterval: 4 /* seconds */) - wg_log(.error, message: "Retrying DNS resolution (Attempt \(attemptIndex + 2))") - } - } + private func resolveDomainNames(endpoints: [Endpoint?]) -> [Endpoint?]? { + do { + return try DNSResolver.resolveSync(endpoints: endpoints) + } catch DNSResolverError.dnsResolutionFailed(let hostnames) { + wg_log(.error, message: "DNS resolution failed for the following hostnames: \(hostnames.joined(separator: ", "))") + } catch { + // There can be no other errors from DNSResolver.resolveSync() + fatalError() } - - return isDNSResolved ? resolvedEndpoints : nil + return nil } private func connect(interfaceName: String, settings: String, fileDescriptor: Int32) -> Int32 {