From fa51e3f1d1636374341b7bf07967626af9e97b91 Mon Sep 17 00:00:00 2001 From: Roopesh Chander Date: Wed, 19 Dec 2018 15:38:00 +0530 Subject: [PATCH] NE: Handle bad domain names and Activate On Demand This combination causes iOS to keep trying to bring up the tunnel, leading to a lot of displayMessage() alerts. In this fix, if we get a DNS resolution error in an Activate On Demand enabled tunnel, we silently retry 9 times (with a 4-second delay before each retry) and then show the displayMessage() alert. Signed-off-by: Roopesh Chander --- .../NETunnelProviderProtocol+Extension.swift | 9 ++- .../WireGuard/Tunnel/TunnelsManager.swift | 4 +- .../ErrorNotifier.swift | 8 ++- .../PacketTunnelProvider.swift | 57 ++++++++++++++----- 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/WireGuard/Shared/NETunnelProviderProtocol+Extension.swift b/WireGuard/Shared/NETunnelProviderProtocol+Extension.swift index 960bf22..9f4af77 100644 --- a/WireGuard/Shared/NETunnelProviderProtocol+Extension.swift +++ b/WireGuard/Shared/NETunnelProviderProtocol+Extension.swift @@ -4,7 +4,7 @@ import NetworkExtension extension NETunnelProviderProtocol { - convenience init?(tunnelConfiguration: TunnelConfiguration) { + convenience init?(tunnelConfiguration: TunnelConfiguration, isActivateOnDemandEnabled: Bool) { assert(!tunnelConfiguration.interface.name.isEmpty) guard let serializedTunnelConfiguration = try? JSONEncoder().encode(tunnelConfiguration) else { return nil } @@ -14,7 +14,8 @@ extension NETunnelProviderProtocol { providerBundleIdentifier = "\(appId).network-extension" providerConfiguration = [ "tunnelConfiguration": serializedTunnelConfiguration, - "tunnelConfigurationVersion": 1 + "tunnelConfigurationVersion": 1, + "isActivateOnDemandEnabled": isActivateOnDemandEnabled ] let endpoints = tunnelConfiguration.peers.compactMap {$0.endpoint} @@ -32,4 +33,8 @@ extension NETunnelProviderProtocol { guard let serializedTunnelConfiguration = providerConfiguration?["tunnelConfiguration"] as? Data else { return nil } return try? JSONDecoder().decode(TunnelConfiguration.self, from: serializedTunnelConfiguration) } + + var isActivateOnDemandEnabled: Bool { + return (providerConfiguration?["isActivateOnDemandEnabled"] as? Bool) ?? false + } } diff --git a/WireGuard/WireGuard/Tunnel/TunnelsManager.swift b/WireGuard/WireGuard/Tunnel/TunnelsManager.swift index 26e134f..10ef1ef 100644 --- a/WireGuard/WireGuard/Tunnel/TunnelsManager.swift +++ b/WireGuard/WireGuard/Tunnel/TunnelsManager.swift @@ -59,7 +59,7 @@ class TunnelsManager { } let tunnelProviderManager = NETunnelProviderManager() - tunnelProviderManager.protocolConfiguration = NETunnelProviderProtocol(tunnelConfiguration: tunnelConfiguration) + tunnelProviderManager.protocolConfiguration = NETunnelProviderProtocol(tunnelConfiguration: tunnelConfiguration, isActivateOnDemandEnabled: activateOnDemandSetting.isActivateOnDemandEnabled) tunnelProviderManager.localizedDescription = tunnelName tunnelProviderManager.isEnabled = true @@ -115,7 +115,7 @@ class TunnelsManager { } tunnel.name = tunnelName } - tunnelProviderManager.protocolConfiguration = NETunnelProviderProtocol(tunnelConfiguration: tunnelConfiguration) + tunnelProviderManager.protocolConfiguration = NETunnelProviderProtocol(tunnelConfiguration: tunnelConfiguration, isActivateOnDemandEnabled: activateOnDemandSetting.isActivateOnDemandEnabled) tunnelProviderManager.localizedDescription = tunnelName tunnelProviderManager.isEnabled = true diff --git a/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift b/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift index c392233..02fbd4c 100644 --- a/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift +++ b/WireGuard/WireGuardNetworkExtension/ErrorNotifier.swift @@ -18,8 +18,12 @@ class ErrorNotifier { switch error { case .savedProtocolConfigurationIsInvalid: 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 .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") + } case .couldNotStartWireGuard: return ("Activation failure", "WireGuard backend could not be started") case .coultNotSetNetworkSettings: diff --git a/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift b/WireGuard/WireGuardNetworkExtension/PacketTunnelProvider.swift index 12a2e43..9a3aede 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(hostnames: [String]) + case dnsResolutionFailure(tunnelName: String, isActivateOnDemandEnabled: Bool) case couldNotStartWireGuard case coultNotSetNetworkSettings } @@ -38,21 +38,22 @@ class PacketTunnelProvider: NEPacketTunnelProvider { configureLogger() - wg_log(.info, message: "Starting tunnel '\(tunnelConfiguration.interface.name)'") + let tunnelName = tunnelConfiguration.interface.name + wg_log(.info, message: "Starting tunnel '\(tunnelName)'") + + let isActivateOnDemandEnabled = tunnelProviderProtocol.isActivateOnDemandEnabled + if isActivateOnDemandEnabled { + wg_log(.info, staticMessage: "Tunnel has Activate On Demand enabled") + } else { + wg_log(.info, staticMessage: "Tunnel has Activate On Demand disabled") + } let endpoints = tunnelConfiguration.peers.map { $0.endpoint } - var resolvedEndpoints = [Endpoint?]() - do { - resolvedEndpoints = try DNSResolver.resolveSync(endpoints: endpoints) - } 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: ", "))") - errorNotifier.notify(PacketTunnelProviderError.dnsResolutionFailure(hostnames: hostnames)) - startTunnelCompletionHandler(PacketTunnelProviderError.dnsResolutionFailure(hostnames: hostnames)) + guard let resolvedEndpoints = resolveDomainNames(endpoints: endpoints, isActivateOnDemandEnabled: isActivateOnDemandEnabled) else { + let dnsError = PacketTunnelProviderError.dnsResolutionFailure(tunnelName: tunnelName, isActivateOnDemandEnabled: isActivateOnDemandEnabled) + errorNotifier.notify(dnsError) + startTunnelCompletionHandler(dnsError) return - } catch { - // There can be no other errors from DNSResolver.resolveSync() - fatalError() } assert(endpoints.count == resolvedEndpoints.count) @@ -143,6 +144,36 @@ 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))") + } + } + } + + return isDNSResolved ? resolvedEndpoints : nil + } + private func connect(interfaceName: String, settings: String, fileDescriptor: Int32) -> Int32 { return withStringsAsGoStrings(interfaceName, settings) { return wgTurnOn($0.0, $0.1, fileDescriptor) } }