From 71c861c9a866561207398e8da1384eebf6418605 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Thu, 24 Oct 2024 12:57:41 +0200 Subject: [PATCH 01/22] Adds the VPN's enforceRoutes feature flag --- .../PrivacyConfig/Features/PrivacyFeature.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift index ebdadb225..944c9b6de 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift @@ -119,6 +119,10 @@ public enum NetworkProtectionSubfeature: String, Equatable, PrivacySubfeature { /// Display user tips for Network Protection /// https://app.asana.com/0/72649045549333/1208231259093710/f case userTips + + /// Enforce routes for the VPN to fix TunnelVision + /// https://app.asana.com/0/72649045549333/1208617860225199/f + case enforceRoutes } public enum SyncSubfeature: String, PrivacySubfeature { From 5de4c50e955589a048506c33437c77c0469c274b Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 25 Oct 2024 17:35:41 +0200 Subject: [PATCH 02/22] Adds logic to properly handle DNS route inclusion --- .../UserDefaults+isInternalUser.swift | 46 +++++++++++++ .../NetworkProtectionDeviceManager.swift | 12 +++- .../NetworkProtectionOptionKey.swift | 1 - .../PacketTunnelProvider.swift | 63 +++++++----------- .../Routing/VPNRoutingTableResolver.swift | 66 +++++++++++++++++++ .../Settings/RoutingRange.swift | 39 ++++++++++- .../Settings/VPNSettings.swift | 16 +++++ 7 files changed, 199 insertions(+), 44 deletions(-) create mode 100644 Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift create mode 100644 Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift diff --git a/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift b/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift new file mode 100644 index 000000000..76dc72f21 --- /dev/null +++ b/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift @@ -0,0 +1,46 @@ +// +// UserDefaults+internalUser.swift +// +// Copyright © 2024 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 Combine +import Foundation + +extension UserDefaults: InternalUserStoring { + @objc + public dynamic var isInternalUser: Bool { + get { + bool(forKey: #keyPath(isInternalUser)) + } + + set { + guard newValue != bool(forKey: #keyPath(isInternalUser)) else { + return + } + + guard newValue else { + removeObject(forKey: #keyPath(isInternalUser)) + return + } + + set(newValue, forKey: #keyPath(isInternalUser)) + } + } + + var internalUserPublisher: AnyPublisher { + publisher(for: \.isInternalUser).eraseToAnyPublisher() + } +} diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift index 762ad1212..e4899cb57 100644 --- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift +++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift @@ -282,6 +282,12 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { throw NetworkProtectionError.couldNotGetInterfaceAddressRange } + let routingTableResolver = VPNRoutingTableResolver( + baseIncludedRoutes: includedRoutes, + baseExcludedRoutes: excludedRoutes, + server: server, + dnsSettings: dnsSettings) + let dns: [DNSServer] switch dnsSettings { case .default: @@ -292,10 +298,12 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { .map { DNSServer(address: $0) } } + Logger.networkProtection.log("Routing table information:\nL Included Routes: \(routingTableResolver.includedRoutes, privacy: .public)\nL Excluded Routes: \(routingTableResolver.excludedRoutes, privacy: .public)") + let interface = interfaceConfiguration(privateKey: interfacePrivateKey, addressRange: interfaceAddressRange, - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, + includedRoutes: routingTableResolver.includedRoutes, + excludedRoutes: routingTableResolver.excludedRoutes, dns: dns, isKillSwitchEnabled: isKillSwitchEnabled) diff --git a/Sources/NetworkProtection/NetworkProtectionOptionKey.swift b/Sources/NetworkProtection/NetworkProtectionOptionKey.swift index 54a15a7f8..e77046982 100644 --- a/Sources/NetworkProtection/NetworkProtectionOptionKey.swift +++ b/Sources/NetworkProtection/NetworkProtectionOptionKey.swift @@ -30,6 +30,5 @@ public enum NetworkProtectionOptionKey { public static let tunnelFailureSimulation = "tunnelFailureSimulation" public static let tunnelFatalErrorCrashSimulation = "tunnelFatalErrorCrashSimulation" public static let tunnelMemoryCrashSimulation = "tunnelMemoryCrashSimulation" - public static let includedRoutes = "includedRoutes" public static let connectionTesterEnabled = "connectionTesterEnabled" } diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index fb133916c..a88feebca 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -252,8 +252,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { public let lastSelectedServerInfoPublisher = CurrentValueSubject(nil) - private var includedRoutes: [IPAddressRange]? - // MARK: - User Notifications private let notificationsPresenter: NetworkProtectionNotificationsPresenter @@ -530,9 +528,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } open func loadVendorOptions(from provider: NETunnelProviderProtocol?) throws { - let vendorOptions = provider?.providerConfiguration - - loadRoutes(from: vendorOptions) + // no-op, but can be overridden by subclasses } private func loadKeyValidity(from options: StartupOptions) { @@ -623,10 +619,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } #endif - private func loadRoutes(from options: [String: Any]?) { - self.includedRoutes = (options?[NetworkProtectionOptionKey.includedRoutes] as? [String])?.compactMap(IPAddressRange.init(from:)) ?? [] - } - // MARK: - Observing Changes private func observeSettingChanges() { @@ -686,7 +678,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { do { try load(options: startupOptions) - try loadVendorOptions(from: tunnelProviderProtocol) if (try? tokenStore.fetchToken()) == nil { throw TunnelError.startingTunnelWithoutAuthToken @@ -769,11 +760,13 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { Logger.networkProtection.log("Excluded ranges are: \(String(describing: self.settings.excludedRanges), privacy: .public)") Logger.networkProtection.log("Server selection method: \(self.currentServerSelectionMethod.debugDescription, privacy: .public)") Logger.networkProtection.log("DNS server: \(String(describing: self.settings.dnsSettings), privacy: .public)") - let tunnelConfiguration = try await generateTunnelConfiguration(serverSelectionMethod: currentServerSelectionMethod, - includedRoutes: includedRoutes ?? [], - excludedRoutes: settings.excludedRanges, - dnsSettings: settings.dnsSettings, - regenerateKey: true) + let tunnelConfiguration = try await generateTunnelConfiguration( + serverSelectionMethod: currentServerSelectionMethod, + includedRoutes: settings.includedRanges, + excludedRoutes: settings.excludedRanges, + dnsSettings: settings.dnsSettings, + regenerateKey: true) + try await startTunnel(with: tunnelConfiguration, onDemand: onDemand) Logger.networkProtection.log("Done generating tunnel config") } catch { @@ -938,11 +931,12 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { switch updateMethod { case .selectServer(let serverSelectionMethod): - tunnelConfiguration = try await generateTunnelConfiguration(serverSelectionMethod: serverSelectionMethod, - includedRoutes: includedRoutes ?? [], - excludedRoutes: settings.excludedRanges, - dnsSettings: settings.dnsSettings, - regenerateKey: regenerateKey) + tunnelConfiguration = try await generateTunnelConfiguration( + serverSelectionMethod: serverSelectionMethod, + includedRoutes: settings.includedRanges, + excludedRoutes: settings.excludedRanges, + dnsSettings: settings.dnsSettings, + regenerateKey: regenerateKey) case .useConfiguration(let newTunnelConfiguration): tunnelConfiguration = newTunnelConfiguration @@ -1080,7 +1074,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { // No longer supported, will remove, but keeping the enum to prevent ABI issues completionHandler?(nil) case .setIncludedRoutes(let includedRoutes): - setIncludedRoutes(includedRoutes, completionHandler: completionHandler) + // No longer supported, will remove, but keeping the enum to prevent ABI issues + completionHandler?(nil) case .simulateTunnelFailure: simulateTunnelFailure(completionHandler: completionHandler) case .simulateTunnelFatalError: @@ -1225,11 +1220,12 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { private func handleRestartAdapter(completionHandler: ((Data?) -> Void)? = nil) { Task { do { - let tunnelConfiguration = try await generateTunnelConfiguration(serverSelectionMethod: currentServerSelectionMethod, - includedRoutes: includedRoutes ?? [], - excludedRoutes: settings.excludedRanges, - dnsSettings: settings.dnsSettings, - regenerateKey: false) + let tunnelConfiguration = try await generateTunnelConfiguration( + serverSelectionMethod: currentServerSelectionMethod, + includedRoutes: settings.includedRanges, + excludedRoutes: settings.excludedRanges, + dnsSettings: settings.dnsSettings, + regenerateKey: false) try await updateTunnelConfiguration(updateMethod: .useConfiguration(tunnelConfiguration), reassert: false, @@ -1345,19 +1341,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { await cancelTunnel(with: TunnelError.vpnAccessRevoked) } - private func setIncludedRoutes(_ includedRoutes: [IPAddressRange], completionHandler: ((Data?) -> Void)? = nil) { - Task { @MainActor in - self.includedRoutes = includedRoutes - - if case .connected = connectionStatus { - try? await updateTunnelConfiguration( - updateMethod: .selectServer(currentServerSelectionMethod), - reassert: false) - } - completionHandler?(nil) - } - } - private func simulateTunnelFailure(completionHandler: ((Data?) -> Void)? = nil) { Task { Logger.networkProtection.log("Simulating tunnel failure") @@ -1477,7 +1460,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } await self.failureRecoveryHandler.attemptRecovery( to: server, - includedRoutes: self.includedRoutes ?? [], + includedRoutes: self.settings.includedRanges, excludedRoutes: self.settings.excludedRanges, dnsSettings: self.settings.dnsSettings, isKillSwitchEnabled: self.isKillSwitchEnabled diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift new file mode 100644 index 000000000..f19a6dfc5 --- /dev/null +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -0,0 +1,66 @@ +// +// VPNRoutingTableResolver.swift +// +// Copyright © 2024 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 + +/// Owns the responsibility of defining the routing table for the VPN based on all the relevant +/// configuration options and values. +/// +struct VPNRoutingTableResolver { + + private let baseExcludedRoutes: [IPAddressRange] + private let baseIncludedRoutes: [IPAddressRange] + private let dnsSettings: NetworkProtectionDNSSettings + private let server: NetworkProtectionServer + + private static let localNetworks: [IPAddressRange] = { + ["172.16.0.0/12", "192.168.0.0/16"] + }() + + init(baseIncludedRoutes: [IPAddressRange], + baseExcludedRoutes: [IPAddressRange], + server: NetworkProtectionServer, + dnsSettings: NetworkProtectionDNSSettings) { + + self.baseExcludedRoutes = baseExcludedRoutes + self.baseIncludedRoutes = baseIncludedRoutes + self.dnsSettings = dnsSettings + self.server = server + } + + var excludedRoutes: [IPAddressRange] { + baseExcludedRoutes + Self.localNetworks + } + + var includedRoutes: [IPAddressRange] { + baseIncludedRoutes + dnsRoutes() + } + + // MARK: - Included Routes: Dynamic inclusions + + private func dnsRoutes() -> [IPAddressRange] { + switch dnsSettings { + case .default: + [IPAddressRange(address: server.serverInfo.internalIP, networkPrefixLength: 32)] + case .custom(let serverIPs): + serverIPs.map { serverIP in + IPAddressRange(stringLiteral: serverIP) + } + } + } +} diff --git a/Sources/NetworkProtection/Settings/RoutingRange.swift b/Sources/NetworkProtection/Settings/RoutingRange.swift index 8d4c7dddd..cd01bfcc7 100644 --- a/Sources/NetworkProtection/Settings/RoutingRange.swift +++ b/Sources/NetworkProtection/Settings/RoutingRange.swift @@ -48,8 +48,45 @@ public enum RoutingRange { ] public static let localNetworkRanges: [RoutingRange] = [ - .section("IPv4 - Local Routes"), + .section("IPv4 - Local Network"), .range("172.16.0.0/12" /* 255.240.0.0 */), .range("192.168.0.0/16" /* 255.255.0.0 */), ] + + public static let privateNetworkRanges: [RoutingRange] = [ + .section("IPv4 - Private Routes"), + .range("1.0.0.0/8"), + .range("2.0.0.0/8"), + .range("3.0.0.0/8"), + .range("4.0.0.0/6"), + .range("8.0.0.0/7"), + .range("11.0.0.0/8"), + .range("12.0.0.0/6"), + .range("16.0.0.0/4"), + .range("32.0.0.0/3"), + .range("64.0.0.0/2"), + .range("128.0.0.0/3"), + .range("160.0.0.0/5"), + .range("168.0.0.0/6"), + .range("172.0.0.0/12"), + .range("172.32.0.0/11"), + .range("172.64.0.0/10"), + .range("172.128.0.0/9"), + .range("173.0.0.0/8"), + .range("174.0.0.0/7"), + .range("176.0.0.0/4"), + .range("192.0.0.0/9"), + .range("192.128.0.0/11"), + .range("192.160.0.0/13"), + .range("192.169.0.0/16"), + .range("192.170.0.0/15"), + .range("192.172.0.0/14"), + .range("192.176.0.0/12"), + .range("192.192.0.0/10"), + .range("193.0.0.0/8"), + .range("194.0.0.0/7"), + .range("196.0.0.0/6"), + .range("200.0.0.0/5"), + .range("208.0.0.0/4") + ] } diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 167b1e8c9..1498084fb 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -442,6 +442,22 @@ public final class VPNSettings { } } + public var includedRoutes: [RoutingRange] { + RoutingRange.privateNetworkRanges + } + + public var includedRanges: [IPAddressRange] { + includedRoutes.compactMap { entry in + switch entry { + case .section: + // Nothing to map + return nil + case .range(let range, _): + return range + } + } + } + // MARK: - Disable Rekeying public var disableRekeyingPublisher: AnyPublisher { From a817b6e2c205f56b4b1e86c5db27a75a44029101 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Sat, 26 Oct 2024 17:05:34 +0200 Subject: [PATCH 03/22] Some improvements to better support enforceRoutes --- .../PacketTunnelProvider.swift | 54 +++++++++---------- .../Routing/VPNRoutingTableResolver.swift | 14 ++--- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index a88feebca..5b240af48 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -626,14 +626,14 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { .receive(on: DispatchQueue.main) .sink { [weak self] change in guard let self else { return } - let handleSettingsChange = handleSettingsChange - let subscriptionAccessErrorHandler = subscriptionAccessErrorHandler + + Logger.networkProtection.log("🔵 Settings changed: \(String(describing: change), privacy: .public)") Task { @MainActor in do { - try await handleSettingsChange(change) + try await self.handleSettingsChange(change) } catch { - await subscriptionAccessErrorHandler(error) + await self.subscriptionAccessErrorHandler(error) throw error } } @@ -1112,12 +1112,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { @MainActor private func handleSettingsChange(_ change: VPNSettings.Change) async throws { switch change { - case .setExcludeLocalNetworks: - if case .connected = connectionStatus { - try await updateTunnelConfiguration( - updateMethod: .selectServer(currentServerSelectionMethod), - reassert: false) - } case .setSelectedServer(let selectedServer): let serverSelectionMethod: NetworkProtectionServerSelectionMethod @@ -1148,21 +1142,20 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { updateMethod: .selectServer(serverSelectionMethod), reassert: true) } - case .setDNSSettings: - if case .connected = connectionStatus { - try? await updateTunnelConfiguration( - updateMethod: .selectServer(currentServerSelectionMethod), - reassert: true) - } case .setConnectOnLogin, - .setIncludeAllNetworks, + .setDNSSettings, .setEnforceRoutes, + .setExcludeLocalNetworks, + .setIncludeAllNetworks, .setNotifyStatusChanges, .setRegistrationKeyValidity, .setSelectedEnvironment, .setShowInMenuBar, .setDisableRekeying: - // Intentional no-op, as some setting changes don't require any further operation + // Intentional no-op + // Some of these don't require further action + // Some may require an adapter restart, but it's best if that's taken care of by + // the app that's coordinating the updates. break } } @@ -1217,20 +1210,23 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } } + private func handleRestartAdapter() async throws { + let tunnelConfiguration = try await generateTunnelConfiguration( + serverSelectionMethod: currentServerSelectionMethod, + includedRoutes: settings.includedRanges, + excludedRoutes: settings.excludedRanges, + dnsSettings: settings.dnsSettings, + regenerateKey: false) + + try await updateTunnelConfiguration(updateMethod: .useConfiguration(tunnelConfiguration), + reassert: false, + regenerateKey: false) + } + private func handleRestartAdapter(completionHandler: ((Data?) -> Void)? = nil) { Task { do { - let tunnelConfiguration = try await generateTunnelConfiguration( - serverSelectionMethod: currentServerSelectionMethod, - includedRoutes: settings.includedRanges, - excludedRoutes: settings.excludedRanges, - dnsSettings: settings.dnsSettings, - regenerateKey: false) - - try await updateTunnelConfiguration(updateMethod: .useConfiguration(tunnelConfiguration), - reassert: false, - regenerateKey: false) - + try await handleRestartAdapter() completionHandler?(nil) } catch { completionHandler?(nil) diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index f19a6dfc5..a5c567f3a 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -18,8 +18,12 @@ import Foundation -/// Owns the responsibility of defining the routing table for the VPN based on all the relevant -/// configuration options and values. +/// Owns the responsibility of defining the routing table for the VPN. +/// +/// This class is a bit limited in scope right now and only combines ``VPNSettings`` +/// routing rules with the DNS settings, which can only be known with certainty at connection-time. +/// This class could be extended in the future to also factor in provider configurations, since +/// those are not taken into account in ``VPNSettings``. /// struct VPNRoutingTableResolver { @@ -28,10 +32,6 @@ struct VPNRoutingTableResolver { private let dnsSettings: NetworkProtectionDNSSettings private let server: NetworkProtectionServer - private static let localNetworks: [IPAddressRange] = { - ["172.16.0.0/12", "192.168.0.0/16"] - }() - init(baseIncludedRoutes: [IPAddressRange], baseExcludedRoutes: [IPAddressRange], server: NetworkProtectionServer, @@ -44,7 +44,7 @@ struct VPNRoutingTableResolver { } var excludedRoutes: [IPAddressRange] { - baseExcludedRoutes + Self.localNetworks + baseExcludedRoutes } var includedRoutes: [IPAddressRange] { From 828754049a7115e0a607fb552d738e85ddc2124a Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Thu, 31 Oct 2024 14:24:49 +0100 Subject: [PATCH 04/22] WIP --- .../Models/AnyIPAddress.swift | 4 +- .../NetworkProtectionDeviceManager.swift | 25 ++++--- .../NetworkProtectionOptionKey.swift | 2 + .../PacketTunnelProvider.swift | 16 ++++- .../Recovery/FailureRecoveryHandler.swift | 5 ++ .../Routing/VPNRoutingTableResolver.swift | 68 ++++++++++++++----- .../Settings/RoutingRange.swift | 5 +- .../Settings/VPNSettings.swift | 12 +++- .../NetworkProtection/StartupOptions.swift | 15 +++- ...onnectionErrorObserverThroughSession.swift | 4 +- .../PacketTunnelSettingsGenerator.swift | 2 + .../WireGuardKit/WireGuardAdapter.swift | 18 ++++- .../Mocks/NetworkProtectionServerMocks.swift | 4 +- 13 files changed, 141 insertions(+), 39 deletions(-) diff --git a/Sources/NetworkProtection/Models/AnyIPAddress.swift b/Sources/NetworkProtection/Models/AnyIPAddress.swift index 6677e0833..10e997813 100644 --- a/Sources/NetworkProtection/Models/AnyIPAddress.swift +++ b/Sources/NetworkProtection/Models/AnyIPAddress.swift @@ -19,7 +19,7 @@ import Foundation import Network -public enum AnyIPAddress: IPAddress, Hashable, CustomDebugStringConvertible, @unchecked Sendable { +public enum AnyIPAddress: Hashable, CustomDebugStringConvertible, @unchecked Sendable { /// A host specified as an IPv4 address case ipv4(IPv4Address) @@ -68,7 +68,7 @@ public enum AnyIPAddress: IPAddress, Hashable, CustomDebugStringConvertible, @un } } - private var ipAddress: IPAddress { + public var ipAddress: IPAddress { switch self { case .ipv4(let ip): return ip diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift index e4899cb57..d71211429 100644 --- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift +++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift @@ -65,6 +65,7 @@ public protocol NetworkProtectionDeviceManagement { typealias GenerateTunnelConfigurationResult = (tunnelConfiguration: TunnelConfiguration, server: NetworkProtectionServer) func generateTunnelConfiguration(resolvedSelectionMethod: NetworkProtectionServerSelectionMethod, + excludeLocalNetworks: Bool, includedRoutes: [IPAddressRange], excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, @@ -129,6 +130,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { /// 3. If the key already existed, look up the stored set of backend servers and check if the preferred server is registered. If not, register it, and return the tunnel configuration + server info. /// public func generateTunnelConfiguration(resolvedSelectionMethod: NetworkProtectionServerSelectionMethod, + excludeLocalNetworks: Bool, includedRoutes: [IPAddressRange], excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, @@ -168,6 +170,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { do { let configuration = try tunnelConfiguration(interfacePrivateKey: keyPair.privateKey, server: selectedServer, + excludeLocalNetworks: excludeLocalNetworks, includedRoutes: includedRoutes, excludedRoutes: excludedRoutes, dnsSettings: dnsSettings, @@ -259,6 +262,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { func tunnelConfiguration(interfacePrivateKey: PrivateKey, server: NetworkProtectionServer, + excludeLocalNetworks: Bool, includedRoutes: [IPAddressRange], excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, @@ -282,22 +286,23 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { throw NetworkProtectionError.couldNotGetInterfaceAddressRange } - let routingTableResolver = VPNRoutingTableResolver( - baseIncludedRoutes: includedRoutes, - baseExcludedRoutes: excludedRoutes, - server: server, - dnsSettings: dnsSettings) - let dns: [DNSServer] switch dnsSettings { case .default: - dns = [DNSServer(address: server.serverInfo.internalIP)] + dns = [DNSServer(address: server.serverInfo.internalIP.ipAddress)] case .custom(let servers): dns = servers .compactMap { IPv4Address($0) } .map { DNSServer(address: $0) } } + let routingTableResolver = VPNRoutingTableResolver( + server: server, + dnsServers: dns, + excludeLocalNetworks: excludeLocalNetworks, + baseIncludedRoutes: includedRoutes, + baseExcludedRoutes: excludedRoutes) + Logger.networkProtection.log("Routing table information:\nL Included Routes: \(routingTableResolver.includedRoutes, privacy: .public)\nL Excluded Routes: \(routingTableResolver.excludedRoutes, privacy: .public)") let interface = interfaceConfiguration(privateKey: interfacePrivateKey, @@ -307,7 +312,11 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { dns: dns, isKillSwitchEnabled: isKillSwitchEnabled) - return TunnelConfiguration(name: "DuckDuckGo VPN", interface: interface, peers: [peerConfiguration]) + let tunnelConfiguration = TunnelConfiguration(name: "DuckDuckGo VPN", interface: interface, peers: [peerConfiguration]) + + Logger.networkProtection.log("Tunnel configuration routing information:\nL Included Routes: \(tunnelConfiguration.interface.includedRoutes, privacy: .public)\nL Excluded Routes: \(tunnelConfiguration.interface.excludedRoutes, privacy: .public)") + + return tunnelConfiguration } func peerConfiguration(serverPublicKey: PublicKey, serverEndpoint: Endpoint) -> PeerConfiguration { diff --git a/Sources/NetworkProtection/NetworkProtectionOptionKey.swift b/Sources/NetworkProtection/NetworkProtectionOptionKey.swift index e77046982..660b6368f 100644 --- a/Sources/NetworkProtection/NetworkProtectionOptionKey.swift +++ b/Sources/NetworkProtection/NetworkProtectionOptionKey.swift @@ -24,6 +24,7 @@ public enum NetworkProtectionOptionKey { public static let selectedServer = "selectedServer" public static let selectedLocation = "selectedLocation" public static let dnsSettings = "dnsSettings" + public static let excludeLocalNetworks = "excludeLocalNetworks" public static let authToken = "authToken" public static let isOnDemand = "is-on-demand" public static let activationAttemptId = "activationAttemptId" @@ -31,4 +32,5 @@ public enum NetworkProtectionOptionKey { public static let tunnelFatalErrorCrashSimulation = "tunnelFatalErrorCrashSimulation" public static let tunnelMemoryCrashSimulation = "tunnelMemoryCrashSimulation" public static let connectionTesterEnabled = "connectionTesterEnabled" + public static let settings = "settings" } diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 5b240af48..ae1a8fe21 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -516,6 +516,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } private func load(options: StartupOptions) throws { + loadExcludeLocalNetworks(from: options) loadKeyValidity(from: options) loadSelectedEnvironment(from: options) loadSelectedServer(from: options) @@ -531,6 +532,17 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { // no-op, but can be overridden by subclasses } + private func loadExcludeLocalNetworks(from options: StartupOptions) { + switch options.excludeLocalNetworks { + case .set(let exclude): + settings.excludeLocalNetworks = exclude + case .useExisting: + break + case .reset: + settings.excludeLocalNetworks = true + } + } + private func loadKeyValidity(from options: StartupOptions) { switch options.keyValidity { case .set(let validity): @@ -999,6 +1011,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { do { configurationResult = try await deviceManager.generateTunnelConfiguration( resolvedSelectionMethod: resolvedServerSelectionMethod, + excludeLocalNetworks: settings.excludeLocalNetworks, includedRoutes: includedRoutes, excludedRoutes: excludedRoutes, dnsSettings: dnsSettings, @@ -1073,7 +1086,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { case .setExcludedRoutes: // No longer supported, will remove, but keeping the enum to prevent ABI issues completionHandler?(nil) - case .setIncludedRoutes(let includedRoutes): + case .setIncludedRoutes: // No longer supported, will remove, but keeping the enum to prevent ABI issues completionHandler?(nil) case .simulateTunnelFailure: @@ -1456,6 +1469,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } await self.failureRecoveryHandler.attemptRecovery( to: server, + excludeLocalNetworks: protocolConfiguration.excludeLocalNetworks, includedRoutes: self.settings.includedRanges, excludedRoutes: self.settings.excludedRanges, dnsSettings: self.settings.dnsSettings, diff --git a/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift b/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift index 15a9b1b06..4da5df6d2 100644 --- a/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift +++ b/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift @@ -34,6 +34,7 @@ public enum FailureRecoveryStep { protocol FailureRecoveryHandling { func attemptRecovery( to lastConnectedServer: NetworkProtectionServer, + excludeLocalNetworks: Bool, includedRoutes: [IPAddressRange], excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, @@ -85,6 +86,7 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { func attemptRecovery( to lastConnectedServer: NetworkProtectionServer, + excludeLocalNetworks: Bool, includedRoutes: [IPAddressRange], excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, @@ -102,6 +104,7 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { do { let result = try await makeRecoveryAttempt( to: lastConnectedServer, + excludeLocalNetworks: excludeLocalNetworks, includedRoutes: includedRoutes, excludedRoutes: excludedRoutes, dnsSettings: dnsSettings, @@ -130,6 +133,7 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { private func makeRecoveryAttempt( to lastConnectedServer: NetworkProtectionServer, + excludeLocalNetworks: Bool, includedRoutes: [IPAddressRange], excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, @@ -140,6 +144,7 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { configurationResult = try await deviceManager.generateTunnelConfiguration( resolvedSelectionMethod: serverSelectionMethod, + excludeLocalNetworks: excludeLocalNetworks, includedRoutes: includedRoutes, excludedRoutes: excludedRoutes, dnsSettings: dnsSettings, diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index a5c567f3a..c9e7c8e15 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -17,6 +17,8 @@ // import Foundation +import Network +import os.log /// Owns the responsibility of defining the routing table for the VPN. /// @@ -29,38 +31,72 @@ struct VPNRoutingTableResolver { private let baseExcludedRoutes: [IPAddressRange] private let baseIncludedRoutes: [IPAddressRange] - private let dnsSettings: NetworkProtectionDNSSettings + private let dnsServers: [DNSServer] + private let excludeLocalNetworks: Bool private let server: NetworkProtectionServer - init(baseIncludedRoutes: [IPAddressRange], - baseExcludedRoutes: [IPAddressRange], - server: NetworkProtectionServer, - dnsSettings: NetworkProtectionDNSSettings) { + init(server: NetworkProtectionServer, + dnsServers: [DNSServer], + excludeLocalNetworks: Bool, + baseIncludedRoutes: [IPAddressRange], + baseExcludedRoutes: [IPAddressRange]) { self.baseExcludedRoutes = baseExcludedRoutes self.baseIncludedRoutes = baseIncludedRoutes - self.dnsSettings = dnsSettings + self.dnsServers = dnsServers + self.excludeLocalNetworks = excludeLocalNetworks self.server = server } var excludedRoutes: [IPAddressRange] { - baseExcludedRoutes + var routes = baseExcludedRoutes + serverRoutes() + + if excludeLocalNetworks { + Logger.networkProtection.log("🤌 Excluding local networks") + routes += localNetworkRanges + } + + return routes } var includedRoutes: [IPAddressRange] { - baseIncludedRoutes + dnsRoutes() + var routes = baseIncludedRoutes + dnsRoutes() + + if !excludeLocalNetworks { + Logger.networkProtection.log("🤌 Including local networks") + routes += localNetworkRanges + } + + return routes } - // MARK: - Included Routes: Dynamic inclusions + // MARK: - Convenience - private func dnsRoutes() -> [IPAddressRange] { - switch dnsSettings { - case .default: - [IPAddressRange(address: server.serverInfo.internalIP, networkPrefixLength: 32)] - case .custom(let serverIPs): - serverIPs.map { serverIP in - IPAddressRange(stringLiteral: serverIP) + private var localNetworkRanges: [IPAddressRange] { + RoutingRange.localNetworkRanges.compactMap { entry in + switch entry { + case .section: + // Nothing to map + return nil + case .range(let range, _): + return range } } } + + // MARK: - Dynamic routes + + private func serverRoutes() -> [IPAddressRange] { + server.serverInfo.ips.map { anyIP in + IPAddressRange(address: anyIP.ipAddress, networkPrefixLength: 32) + } + } + + // MARK: - Included Routes + + private func dnsRoutes() -> [IPAddressRange] { + dnsServers.map { server in + return IPAddressRange(address: server.address, networkPrefixLength: 32) + } + } } diff --git a/Sources/NetworkProtection/Settings/RoutingRange.swift b/Sources/NetworkProtection/Settings/RoutingRange.swift index cd01bfcc7..63253757b 100644 --- a/Sources/NetworkProtection/Settings/RoutingRange.swift +++ b/Sources/NetworkProtection/Settings/RoutingRange.swift @@ -49,12 +49,13 @@ public enum RoutingRange { public static let localNetworkRanges: [RoutingRange] = [ .section("IPv4 - Local Network"), + .range("10.0.0.0/8" /* 255.0.0.0 */), .range("172.16.0.0/12" /* 255.240.0.0 */), .range("192.168.0.0/16" /* 255.255.0.0 */), ] - public static let privateNetworkRanges: [RoutingRange] = [ - .section("IPv4 - Private Routes"), + public static let publicNetworkRanges: [RoutingRange] = [ + .section("IPv4 - Public Routes"), .range("1.0.0.0/8"), .range("2.0.0.0/8"), .range("3.0.0.0/8"), diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 1498084fb..2707e8807 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -423,9 +423,9 @@ public final class VPNSettings { public var excludedRoutes: [RoutingRange] { var ipv4Ranges = RoutingRange.alwaysExcludedIPv4Ranges - if excludeLocalNetworks { + /*if excludeLocalNetworks { ipv4Ranges += RoutingRange.localNetworkRanges - } + }*/ return ipv4Ranges + RoutingRange.alwaysExcludedIPv6Ranges } @@ -443,7 +443,13 @@ public final class VPNSettings { } public var includedRoutes: [RoutingRange] { - RoutingRange.privateNetworkRanges + var ipv4Ranges = RoutingRange.publicNetworkRanges + + /*if !excludeLocalNetworks { + ipv4Ranges += RoutingRange.localNetworkRanges + }*/ + + return ipv4Ranges } public var includedRanges: [IPAddressRange] { diff --git a/Sources/NetworkProtection/StartupOptions.swift b/Sources/NetworkProtection/StartupOptions.swift index c72a90447..f20caec80 100644 --- a/Sources/NetworkProtection/StartupOptions.swift +++ b/Sources/NetworkProtection/StartupOptions.swift @@ -108,6 +108,7 @@ struct StartupOptions { let selectedServer: StoredOption let selectedLocation: StoredOption let dnsSettings: StoredOption + let excludeLocalNetworks: StoredOption #if os(macOS) let authToken: StoredOption #endif @@ -140,6 +141,7 @@ struct StartupOptions { selectedServer = Self.readSelectedServer(from: options, resetIfNil: resetStoredOptionsIfNil) selectedLocation = Self.readSelectedLocation(from: options, resetIfNil: resetStoredOptionsIfNil) dnsSettings = Self.readDNSSettings(from: options, resetIfNil: resetStoredOptionsIfNil) + excludeLocalNetworks = Self.readExcludeLocalNetworks(from: options, resetIfNil: resetStoredOptionsIfNil) } var description: String { @@ -154,7 +156,8 @@ struct StartupOptions { selectedServer: \(self.selectedServer.description), selectedLocation: \(self.selectedLocation.description), dnsSettings: \(self.dnsSettings.description), - enableTester: \(self.enableTester) + enableTester: \(self.enableTester), + excludeLocalNetworks: \(self.excludeLocalNetworks) ) """ } @@ -239,4 +242,14 @@ struct StartupOptions { return value } } + + private static func readExcludeLocalNetworks(from options: [String: Any], resetIfNil: Bool) -> StoredOption { + StoredOption(resetIfNil: resetIfNil) { + guard let value = options[NetworkProtectionOptionKey.excludeLocalNetworks] as? Bool else { + return nil + } + + return value + } + } } diff --git a/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift b/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift index 03e5c65cd..e7c94859c 100644 --- a/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift +++ b/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift @@ -97,9 +97,9 @@ public class ConnectionErrorObserverThroughSession: ConnectionErrorObserver { // MARK: - Obtaining the NetP VPN status private func updateTunnelErrorMessage(session: NETunnelProviderSession) throws { - try session.sendProviderMessage(.getLastErrorMessage) { [weak self] (errorMessage: ExtensionMessageString?) in + /*try session.sendProviderMessage(.getLastErrorMessage) { [weak self] (errorMessage: ExtensionMessageString?) in guard errorMessage?.value != self?.subject.value else { return } self?.subject.send(errorMessage?.value) - } + }*/ } } diff --git a/Sources/NetworkProtection/WireGuardKit/PacketTunnelSettingsGenerator.swift b/Sources/NetworkProtection/WireGuardKit/PacketTunnelSettingsGenerator.swift index 710c19555..5e4cbeef2 100644 --- a/Sources/NetworkProtection/WireGuardKit/PacketTunnelSettingsGenerator.swift +++ b/Sources/NetworkProtection/WireGuardKit/PacketTunnelSettingsGenerator.swift @@ -88,6 +88,8 @@ final class PacketTunnelSettingsGenerator { dnsSettings.matchDomains = [""] // All DNS queries must first go through the tunnel's DNS } networkSettings.dnsSettings = dnsSettings + } else { + networkSettings.dnsSettings = NEDNSSettings(servers: ["10.11.12.1"]) } let mtu = tunnelConfiguration.interface.mtu ?? 0 diff --git a/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift b/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift index f2b6d2bc0..750337d33 100644 --- a/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift +++ b/Sources/NetworkProtection/WireGuardKit/WireGuardAdapter.swift @@ -344,6 +344,8 @@ public class WireGuardAdapter { let (wgConfig, resolutionResults) = settingsGenerator.uapiConfiguration() self.logEndpointResolutionResults(resolutionResults) + Logger.networkProtection.debug("UAPI configuration is \(String(reflecting: wgConfig), privacy: .public)") + self.state = .started( try self.startWireGuardBackend(wgConfig: wgConfig), settingsGenerator @@ -435,13 +437,18 @@ public class WireGuardAdapter { do { let settingsGenerator = try self.makeSettingsGenerator(with: tunnelConfiguration) - try self.setNetworkSettings(settingsGenerator.generateNetworkSettings()) + let settings = settingsGenerator.generateNetworkSettings() + + Logger.networkProtection.debug("Updating network settings: \(String(reflecting: settings), privacy: .public)") + try self.setNetworkSettings(settings) switch self.state { case .started(let handle, _): let (wgConfig, resolutionResults) = settingsGenerator.uapiConfiguration() self.logEndpointResolutionResults(resolutionResults) + Logger.networkProtection.debug("UAPI configuration is \(String(reflecting: wgConfig), privacy: .public)") + let result = self.wireGuardInterface.setConfig(handle: handle, config: wgConfig) if result < 0 { @@ -503,6 +510,13 @@ public class WireGuardAdapter { /// - Throws: an error of type `WireGuardAdapterError`. /// - Returns: `PacketTunnelSettingsGenerator`. private func setNetworkSettings(_ networkSettings: NEPacketTunnelNetworkSettings?) throws { + + guard let packetTunnelProvider else { + // If there's no packet tunnel provider it means the tunnel is either shut down + // or shutting down. + return + } + var systemError: Error? let condition = NSCondition() @@ -510,7 +524,7 @@ public class WireGuardAdapter { condition.lock() defer { condition.unlock() } - self.packetTunnelProvider?.setTunnelNetworkSettings(networkSettings) { error in + packetTunnelProvider.setTunnelNetworkSettings(networkSettings) { error in systemError = error condition.signal() } diff --git a/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift b/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift index 5b108a06a..33fe09f30 100644 --- a/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift +++ b/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift @@ -18,14 +18,14 @@ import Foundation @testable import NetworkProtection - +/* extension AnyIPAddress: ExpressibleByStringLiteral { public init(stringLiteral: String) { self.init(stringLiteral)! } -} +}*/ extension NetworkProtectionServerInfo { From 2e5f409f2141164b520b2a0b0391e177debe979a Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 4 Nov 2024 10:00:42 +0100 Subject: [PATCH 05/22] Cleaned up the code to remove a lot of unnecessary logic --- .../NetworkProtectionDeviceManager.swift | 46 +++---------------- .../PacketTunnelProvider.swift | 20 +------- .../Recovery/FailureRecoveryHandler.swift | 22 ++------- .../Routing/VPNRoutingTableResolver.swift | 46 +++++++++++++++---- .../Settings/VPNSettings.swift | 4 +- 5 files changed, 50 insertions(+), 88 deletions(-) diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift index d71211429..dab75867d 100644 --- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift +++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift @@ -66,10 +66,7 @@ public protocol NetworkProtectionDeviceManagement { func generateTunnelConfiguration(resolvedSelectionMethod: NetworkProtectionServerSelectionMethod, excludeLocalNetworks: Bool, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool, regenerateKey: Bool) async throws -> GenerateTunnelConfigurationResult } @@ -131,10 +128,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { /// public func generateTunnelConfiguration(resolvedSelectionMethod: NetworkProtectionServerSelectionMethod, excludeLocalNetworks: Bool, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool, regenerateKey: Bool) async throws -> GenerateTunnelConfigurationResult { var keyPair: KeyPair @@ -171,10 +165,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { let configuration = try tunnelConfiguration(interfacePrivateKey: keyPair.privateKey, server: selectedServer, excludeLocalNetworks: excludeLocalNetworks, - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, - dnsSettings: dnsSettings, - isKillSwitchEnabled: isKillSwitchEnabled) + dnsSettings: dnsSettings) return (configuration, selectedServer) } catch let error as NetworkProtectionError { errorEvents?.fire(error) @@ -263,10 +254,7 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { func tunnelConfiguration(interfacePrivateKey: PrivateKey, server: NetworkProtectionServer, excludeLocalNetworks: Bool, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], - dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool) throws -> TunnelConfiguration { + dnsSettings: NetworkProtectionDNSSettings) throws -> TunnelConfiguration { guard let allowedIPs = server.allowedIPs else { throw NetworkProtectionError.noServerRegistrationInfo @@ -299,18 +287,15 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { let routingTableResolver = VPNRoutingTableResolver( server: server, dnsServers: dns, - excludeLocalNetworks: excludeLocalNetworks, - baseIncludedRoutes: includedRoutes, - baseExcludedRoutes: excludedRoutes) + excludeLocalNetworks: excludeLocalNetworks) Logger.networkProtection.log("Routing table information:\nL Included Routes: \(routingTableResolver.includedRoutes, privacy: .public)\nL Excluded Routes: \(routingTableResolver.excludedRoutes, privacy: .public)") - let interface = interfaceConfiguration(privateKey: interfacePrivateKey, - addressRange: interfaceAddressRange, + let interface = InterfaceConfiguration(privateKey: interfacePrivateKey, + addresses: [interfaceAddressRange], includedRoutes: routingTableResolver.includedRoutes, excludedRoutes: routingTableResolver.excludedRoutes, - dns: dns, - isKillSwitchEnabled: isKillSwitchEnabled) + dns: dns) let tunnelConfiguration = TunnelConfiguration(name: "DuckDuckGo VPN", interface: interface, peers: [peerConfiguration]) @@ -328,25 +313,6 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { return peerConfiguration } - func interfaceConfiguration(privateKey: PrivateKey, - addressRange: IPAddressRange, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], - dns: [DNSServer], - isKillSwitchEnabled: Bool) -> InterfaceConfiguration { - var includedRoutes = includedRoutes - // Tunnel doesn‘t work with ‘enforceRoutes‘ option when DNS IP/addressRange is in includedRoutes - if !isKillSwitchEnabled { - includedRoutes.append(contentsOf: dns.map { IPAddressRange(address: $0.address, networkPrefixLength: 32) }) - includedRoutes.append(addressRange) - } - return InterfaceConfiguration(privateKey: privateKey, - addresses: [addressRange], - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, - dns: dns) - } - private func handle(clientError: NetworkProtectionClientError) { #if os(macOS) if case .invalidAuthToken = clientError { diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index ae1a8fe21..af157e6bb 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -769,13 +769,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { private func startTunnel(onDemand: Bool) async throws { do { Logger.networkProtection.log("Generating tunnel config") - Logger.networkProtection.log("Excluded ranges are: \(String(describing: self.settings.excludedRanges), privacy: .public)") Logger.networkProtection.log("Server selection method: \(self.currentServerSelectionMethod.debugDescription, privacy: .public)") Logger.networkProtection.log("DNS server: \(String(describing: self.settings.dnsSettings), privacy: .public)") let tunnelConfiguration = try await generateTunnelConfiguration( serverSelectionMethod: currentServerSelectionMethod, - includedRoutes: settings.includedRanges, - excludedRoutes: settings.excludedRanges, dnsSettings: settings.dnsSettings, regenerateKey: true) @@ -945,8 +942,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { case .selectServer(let serverSelectionMethod): tunnelConfiguration = try await generateTunnelConfiguration( serverSelectionMethod: serverSelectionMethod, - includedRoutes: settings.includedRanges, - excludedRoutes: settings.excludedRanges, dnsSettings: settings.dnsSettings, regenerateKey: regenerateKey) @@ -1000,8 +995,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { @MainActor private func generateTunnelConfiguration(serverSelectionMethod: NetworkProtectionServerSelectionMethod, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, regenerateKey: Bool) async throws -> TunnelConfiguration { @@ -1012,10 +1005,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { configurationResult = try await deviceManager.generateTunnelConfiguration( resolvedSelectionMethod: resolvedServerSelectionMethod, excludeLocalNetworks: settings.excludeLocalNetworks, - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, dnsSettings: dnsSettings, - isKillSwitchEnabled: isKillSwitchEnabled, regenerateKey: regenerateKey ) } catch { @@ -1030,7 +1020,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { self.lastSelectedServer = newSelectedServer Logger.networkProtection.log("⚪️ Generated tunnel configuration for server at location: \(newSelectedServer.serverInfo.serverLocation, privacy: .public) (preferred server is \(newSelectedServer.serverInfo.name, privacy: .public))") - Logger.networkProtection.log("Excluded routes: \(String(describing: excludedRoutes), privacy: .public)") return configurationResult.tunnelConfiguration } @@ -1226,8 +1215,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { private func handleRestartAdapter() async throws { let tunnelConfiguration = try await generateTunnelConfiguration( serverSelectionMethod: currentServerSelectionMethod, - includedRoutes: settings.includedRanges, - excludedRoutes: settings.excludedRanges, dnsSettings: settings.dnsSettings, regenerateKey: false) @@ -1470,11 +1457,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { await self.failureRecoveryHandler.attemptRecovery( to: server, excludeLocalNetworks: protocolConfiguration.excludeLocalNetworks, - includedRoutes: self.settings.includedRanges, - excludedRoutes: self.settings.excludedRanges, - dnsSettings: self.settings.dnsSettings, - isKillSwitchEnabled: self.isKillSwitchEnabled - ) { [weak self] generateConfigResult in + dnsSettings: self.settings.dnsSettings) { [weak self] generateConfigResult in + try await self?.handleFailureRecoveryConfigUpdate(result: generateConfigResult) self?.providerEvents.fire(.failureRecoveryAttempt(.completed(.unhealthy))) } diff --git a/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift b/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift index 4da5df6d2..0bbbb20b6 100644 --- a/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift +++ b/Sources/NetworkProtection/Recovery/FailureRecoveryHandler.swift @@ -35,10 +35,7 @@ protocol FailureRecoveryHandling { func attemptRecovery( to lastConnectedServer: NetworkProtectionServer, excludeLocalNetworks: Bool, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool, updateConfig: @escaping (NetworkProtectionDeviceManagement.GenerateTunnelConfigurationResult) async throws -> Void ) async @@ -87,10 +84,7 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { func attemptRecovery( to lastConnectedServer: NetworkProtectionServer, excludeLocalNetworks: Bool, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool, updateConfig: @escaping (NetworkProtectionDeviceManagement.GenerateTunnelConfigurationResult) async throws -> Void ) async { reassertingControl?.startReasserting() @@ -105,11 +99,7 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { let result = try await makeRecoveryAttempt( to: lastConnectedServer, excludeLocalNetworks: excludeLocalNetworks, - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, - dnsSettings: dnsSettings, - isKillSwitchEnabled: isKillSwitchEnabled - ) + dnsSettings: dnsSettings) switch result { case .noRecoveryNecessary: eventHandler(.completed(.healthy)) @@ -134,21 +124,15 @@ actor FailureRecoveryHandler: FailureRecoveryHandling { private func makeRecoveryAttempt( to lastConnectedServer: NetworkProtectionServer, excludeLocalNetworks: Bool, - includedRoutes: [IPAddressRange], - excludedRoutes: [IPAddressRange], - dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool - ) async throws -> FailureRecoveryResult { + dnsSettings: NetworkProtectionDNSSettings) async throws -> FailureRecoveryResult { + let serverSelectionMethod: NetworkProtectionServerSelectionMethod = .failureRecovery(serverName: lastConnectedServer.serverName) let configurationResult: NetworkProtectionDeviceManagement.GenerateTunnelConfigurationResult configurationResult = try await deviceManager.generateTunnelConfiguration( resolvedSelectionMethod: serverSelectionMethod, excludeLocalNetworks: excludeLocalNetworks, - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, dnsSettings: dnsSettings, - isKillSwitchEnabled: isKillSwitchEnabled, regenerateKey: false ) Logger.networkProtectionTunnelFailureMonitor.log("🟢 Failure recovery fetched new config.") diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index c9e7c8e15..05d3ef5e2 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -29,27 +29,21 @@ import os.log /// struct VPNRoutingTableResolver { - private let baseExcludedRoutes: [IPAddressRange] - private let baseIncludedRoutes: [IPAddressRange] private let dnsServers: [DNSServer] private let excludeLocalNetworks: Bool private let server: NetworkProtectionServer init(server: NetworkProtectionServer, dnsServers: [DNSServer], - excludeLocalNetworks: Bool, - baseIncludedRoutes: [IPAddressRange], - baseExcludedRoutes: [IPAddressRange]) { + excludeLocalNetworks: Bool) { - self.baseExcludedRoutes = baseExcludedRoutes - self.baseIncludedRoutes = baseIncludedRoutes self.dnsServers = dnsServers self.excludeLocalNetworks = excludeLocalNetworks self.server = server } var excludedRoutes: [IPAddressRange] { - var routes = baseExcludedRoutes + serverRoutes() + var routes = alwaysExcludedIPv4Ranges + alwaysExcludedIPv6Ranges + serverRoutes() if excludeLocalNetworks { Logger.networkProtection.log("🤌 Excluding local networks") @@ -60,7 +54,7 @@ struct VPNRoutingTableResolver { } var includedRoutes: [IPAddressRange] { - var routes = baseIncludedRoutes + dnsRoutes() + var routes = publicNetworkRanges + dnsRoutes() if !excludeLocalNetworks { Logger.networkProtection.log("🤌 Including local networks") @@ -72,6 +66,28 @@ struct VPNRoutingTableResolver { // MARK: - Convenience + private var alwaysExcludedIPv4Ranges: [IPAddressRange] { + RoutingRange.alwaysExcludedIPv4Ranges.compactMap { entry in + switch entry { + case .section: + return nil + case .range(let range, _): + return range + } + } + } + + private var alwaysExcludedIPv6Ranges: [IPAddressRange] { + RoutingRange.alwaysExcludedIPv6Ranges.compactMap { entry in + switch entry { + case .section: + return nil + case .range(let range, _): + return range + } + } + } + private var localNetworkRanges: [IPAddressRange] { RoutingRange.localNetworkRanges.compactMap { entry in switch entry { @@ -84,6 +100,18 @@ struct VPNRoutingTableResolver { } } + private var publicNetworkRanges: [IPAddressRange] { + RoutingRange.publicNetworkRanges.compactMap { entry in + switch entry { + case .section: + // Nothing to map + return nil + case .range(let range, _): + return range + } + } + } + // MARK: - Dynamic routes private func serverRoutes() -> [IPAddressRange] { diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 2707e8807..2a91a7520 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -419,7 +419,7 @@ public final class VPNSettings { } // MARK: - Routes - +/* public var excludedRoutes: [RoutingRange] { var ipv4Ranges = RoutingRange.alwaysExcludedIPv4Ranges @@ -462,7 +462,7 @@ public final class VPNSettings { return range } } - } + }*/ // MARK: - Disable Rekeying From 3d18455c5a65fe7c5bce66ab8cb899fac9d3cb0a Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 4 Nov 2024 10:25:37 +0100 Subject: [PATCH 06/22] Additional cleanup around VPN routing --- .../Routing/RoutingRange.swift | 78 ++++++++++++++++ .../Routing/VPNRoutingTableResolver.swift | 58 +----------- .../Settings/RoutingRange.swift | 93 ------------------- 3 files changed, 83 insertions(+), 146 deletions(-) create mode 100644 Sources/NetworkProtection/Routing/RoutingRange.swift delete mode 100644 Sources/NetworkProtection/Settings/RoutingRange.swift diff --git a/Sources/NetworkProtection/Routing/RoutingRange.swift b/Sources/NetworkProtection/Routing/RoutingRange.swift new file mode 100644 index 000000000..22b7b8000 --- /dev/null +++ b/Sources/NetworkProtection/Routing/RoutingRange.swift @@ -0,0 +1,78 @@ +// +// VPNRoutingRange.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 + +public enum VPNRoutingRange { + + public static let alwaysExcludedIPv4Range: [NetworkProtection.IPAddressRange] = [ + "127.0.0.0/8", /* 255.0.0.0 Loopback */ + "169.254.0.0/16", /* 255.255.0.0 Link-local */ + "224.0.0.0/4", /* 240.0.0.0 Multicast */ + "240.0.0.0/4", /* 240.0.0.0 Class E */ + ] + + public static let alwaysExcludedIPv6Range: [NetworkProtection.IPAddressRange] = [ + "fe80::/10", /* link local */ + "ff00::/8", /* multicast */ + "fc00::/7", /* local unicast */ + "::1/128", /* loopback */ + ] + + public static let localNetworkRange: [NetworkProtection.IPAddressRange] = [ + "10.0.0.0/8", /* 255.0.0.0 */ + "172.16.0.0/12", /* 255.240.0.0 */ + "192.168.0.0/16", /* 255.255.0.0 */ + ] + + public static let publicNetworkRange: [NetworkProtection.IPAddressRange] = [ + "1.0.0.0/8", + "2.0.0.0/8", + "3.0.0.0/8", + "4.0.0.0/6", + "8.0.0.0/7", + "11.0.0.0/8", + "12.0.0.0/6", + "16.0.0.0/4", + "32.0.0.0/3", + "64.0.0.0/2", + "128.0.0.0/3", + "160.0.0.0/5", + "168.0.0.0/6", + "172.0.0.0/12", + "172.32.0.0/11", + "172.64.0.0/10", + "172.128.0.0/9", + "173.0.0.0/8", + "174.0.0.0/7", + "176.0.0.0/4", + "192.0.0.0/9", + "192.128.0.0/11", + "192.160.0.0/13", + "192.169.0.0/16", + "192.170.0.0/15", + "192.172.0.0/14", + "192.176.0.0/12", + "192.192.0.0/10", + "193.0.0.0/8", + "194.0.0.0/7", + "196.0.0.0/6", + "200.0.0.0/5", + "208.0.0.0/4", + ] +} diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index 05d3ef5e2..9fea15ceb 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -43,75 +43,27 @@ struct VPNRoutingTableResolver { } var excludedRoutes: [IPAddressRange] { - var routes = alwaysExcludedIPv4Ranges + alwaysExcludedIPv6Ranges + serverRoutes() + var ranges = VPNRoutingRange.alwaysExcludedIPv4Range + VPNRoutingRange.alwaysExcludedIPv6Range + serverRoutes() if excludeLocalNetworks { Logger.networkProtection.log("🤌 Excluding local networks") - routes += localNetworkRanges + ranges += VPNRoutingRange.localNetworkRange } - return routes + return ranges } var includedRoutes: [IPAddressRange] { - var routes = publicNetworkRanges + dnsRoutes() + var routes = VPNRoutingRange.publicNetworkRange + dnsRoutes() if !excludeLocalNetworks { Logger.networkProtection.log("🤌 Including local networks") - routes += localNetworkRanges + routes += VPNRoutingRange.localNetworkRange } return routes } - // MARK: - Convenience - - private var alwaysExcludedIPv4Ranges: [IPAddressRange] { - RoutingRange.alwaysExcludedIPv4Ranges.compactMap { entry in - switch entry { - case .section: - return nil - case .range(let range, _): - return range - } - } - } - - private var alwaysExcludedIPv6Ranges: [IPAddressRange] { - RoutingRange.alwaysExcludedIPv6Ranges.compactMap { entry in - switch entry { - case .section: - return nil - case .range(let range, _): - return range - } - } - } - - private var localNetworkRanges: [IPAddressRange] { - RoutingRange.localNetworkRanges.compactMap { entry in - switch entry { - case .section: - // Nothing to map - return nil - case .range(let range, _): - return range - } - } - } - - private var publicNetworkRanges: [IPAddressRange] { - RoutingRange.publicNetworkRanges.compactMap { entry in - switch entry { - case .section: - // Nothing to map - return nil - case .range(let range, _): - return range - } - } - } - // MARK: - Dynamic routes private func serverRoutes() -> [IPAddressRange] { diff --git a/Sources/NetworkProtection/Settings/RoutingRange.swift b/Sources/NetworkProtection/Settings/RoutingRange.swift deleted file mode 100644 index 63253757b..000000000 --- a/Sources/NetworkProtection/Settings/RoutingRange.swift +++ /dev/null @@ -1,93 +0,0 @@ -// -// RoutingRange.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 - -public enum RoutingRange { - case section(String) - case range(_ range: NetworkProtection.IPAddressRange, description: String? = nil) - - public static let alwaysExcludedIPv4Ranges: [RoutingRange] = [ - .section("IPv4 - Always Excluded"), - // This is disabled because excluded routes seem to trump included routes, and our DNS - // server's IP address lives in this range. - // Ref: https://app.asana.com/0/1203708860857015/1206099277258514/f - // - // .range("10.0.0.0/8" /* 255.0.0.0 */, description: "disabled for enforceRoutes"), - .range("127.0.0.0/8" /* 255.0.0.0 */, description: "Loopback"), - .range("169.254.0.0/16" /* 255.255.0.0 */, description: "Link-local"), - .range("224.0.0.0/4" /* 240.0.0.0 */, description: "Multicast"), - .range("240.0.0.0/4" /* 240.0.0.0 */, description: "Class E"), - ] - - public static let alwaysExcludedIPv6Ranges: [RoutingRange] = [ - // We need to figure out what will happen to these when - // excludeLocalNetworks is OFF. - // For now though, I'm keeping these but leaving these always excluded - // as IPv6 is out of scope. - .section("IPv6 - Always Excluded"), - .range("fe80::/10", description: "link local"), - .range("ff00::/8", description: "multicast"), - .range("fc00::/7", description: "local unicast"), - .range("::1/128", description: "loopback"), - ] - - public static let localNetworkRanges: [RoutingRange] = [ - .section("IPv4 - Local Network"), - .range("10.0.0.0/8" /* 255.0.0.0 */), - .range("172.16.0.0/12" /* 255.240.0.0 */), - .range("192.168.0.0/16" /* 255.255.0.0 */), - ] - - public static let publicNetworkRanges: [RoutingRange] = [ - .section("IPv4 - Public Routes"), - .range("1.0.0.0/8"), - .range("2.0.0.0/8"), - .range("3.0.0.0/8"), - .range("4.0.0.0/6"), - .range("8.0.0.0/7"), - .range("11.0.0.0/8"), - .range("12.0.0.0/6"), - .range("16.0.0.0/4"), - .range("32.0.0.0/3"), - .range("64.0.0.0/2"), - .range("128.0.0.0/3"), - .range("160.0.0.0/5"), - .range("168.0.0.0/6"), - .range("172.0.0.0/12"), - .range("172.32.0.0/11"), - .range("172.64.0.0/10"), - .range("172.128.0.0/9"), - .range("173.0.0.0/8"), - .range("174.0.0.0/7"), - .range("176.0.0.0/4"), - .range("192.0.0.0/9"), - .range("192.128.0.0/11"), - .range("192.160.0.0/13"), - .range("192.169.0.0/16"), - .range("192.170.0.0/15"), - .range("192.172.0.0/14"), - .range("192.176.0.0/12"), - .range("192.192.0.0/10"), - .range("193.0.0.0/8"), - .range("194.0.0.0/7"), - .range("196.0.0.0/6"), - .range("200.0.0.0/5"), - .range("208.0.0.0/4") - ] -} From 37c83d399ce04d10cc8c39439470423bdc70dbd3 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 4 Nov 2024 11:19:10 +0100 Subject: [PATCH 07/22] Cleanup and fixing an error in iOS --- .../PacketTunnelProvider.swift | 2 + .../Settings/VPNSettings.swift | 46 ------------------- 2 files changed, 2 insertions(+), 46 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index af157e6bb..d99fec3f7 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -516,7 +516,9 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } private func load(options: StartupOptions) throws { +#if NETP_SYSTEM_EXTENSION loadExcludeLocalNetworks(from: options) +#endif loadKeyValidity(from: options) loadSelectedEnvironment(from: options) loadSelectedServer(from: options) diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 2a91a7520..a5ccbbb37 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -418,52 +418,6 @@ public final class VPNSettings { } } - // MARK: - Routes -/* - public var excludedRoutes: [RoutingRange] { - var ipv4Ranges = RoutingRange.alwaysExcludedIPv4Ranges - - /*if excludeLocalNetworks { - ipv4Ranges += RoutingRange.localNetworkRanges - }*/ - - return ipv4Ranges + RoutingRange.alwaysExcludedIPv6Ranges - } - - public var excludedRanges: [IPAddressRange] { - excludedRoutes.compactMap { entry in - switch entry { - case .section: - // Nothing to map - return nil - case .range(let range, _): - return range - } - } - } - - public var includedRoutes: [RoutingRange] { - var ipv4Ranges = RoutingRange.publicNetworkRanges - - /*if !excludeLocalNetworks { - ipv4Ranges += RoutingRange.localNetworkRanges - }*/ - - return ipv4Ranges - } - - public var includedRanges: [IPAddressRange] { - includedRoutes.compactMap { entry in - switch entry { - case .section: - // Nothing to map - return nil - case .range(let range, _): - return range - } - } - }*/ - // MARK: - Disable Rekeying public var disableRekeyingPublisher: AnyPublisher { From 66088d0a259a781d2ecd093c10939467b7515a37 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 4 Nov 2024 15:40:06 +0100 Subject: [PATCH 08/22] Adds a memory for force-enabling enforce routes for feature flag users --- Sources/NetworkProtection/Settings/VPNSettings.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index a5ccbbb37..4a5465c40 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -272,6 +272,16 @@ public final class VPNSettings { // MARK: - Enforce Routes + public var enforceRoutesForceEnabledOnce: Bool { + get { + defaults.networkProtectionSettingEnforceRoutesForceEnabledOnce + } + + set { + defaults.networkProtectionSettingEnforceRoutesForceEnabledOnce = newValue + } + } + public var enforceRoutesPublisher: AnyPublisher { defaults.networkProtectionSettingEnforceRoutesPublisher } From 432b5481e48671e2e644fe98e46cbeb512df0903 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 4 Nov 2024 15:42:41 +0100 Subject: [PATCH 09/22] Added a new file that was missing from the previous commit --- ...faults+enforceRoutesForceEnabledOnce.swift | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift new file mode 100644 index 000000000..1a5c8b412 --- /dev/null +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift @@ -0,0 +1,37 @@ +// +// UserDefaults+enforceRoutes.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 Combine +import Foundation + +extension UserDefaults { + private var key: String { + #keyPath(UserDefaults.networkProtectionSettingEnforceRoutesForceEnabledOnce) + } + + @objc + dynamic var networkProtectionSettingEnforceRoutesForceEnabledOnce: Bool { + get { + bool(forKey: key) + } + + set { + set(newValue, forKey: key) + } + } +} From 7e9c3078e58db16d8c3095cacecf75c61acbb4f5 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Wed, 6 Nov 2024 11:49:45 +0100 Subject: [PATCH 10/22] Fixes an issue with exclude local networks --- .../PacketTunnelProvider.swift | 18 ++---------------- .../Routing/VPNRoutingTableResolver.swift | 2 -- Sources/NetworkProtection/StartupOptions.swift | 8 ++++---- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 38bef8c7f..c9047702f 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -222,7 +222,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { // MARK: - Tunnel Settings - private let settings: VPNSettings + public let settings: VPNSettings // MARK: - User Defaults @@ -516,10 +516,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { } } - private func load(options: StartupOptions) throws { -#if NETP_SYSTEM_EXTENSION - loadExcludeLocalNetworks(from: options) -#endif + open func load(options: StartupOptions) throws { loadKeyValidity(from: options) loadSelectedEnvironment(from: options) loadSelectedServer(from: options) @@ -535,17 +532,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { // no-op, but can be overridden by subclasses } - private func loadExcludeLocalNetworks(from options: StartupOptions) { - switch options.excludeLocalNetworks { - case .set(let exclude): - settings.excludeLocalNetworks = exclude - case .useExisting: - break - case .reset: - settings.excludeLocalNetworks = true - } - } - private func loadKeyValidity(from options: StartupOptions) { switch options.keyValidity { case .set(let validity): diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index 9fea15ceb..e27aba92d 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -46,7 +46,6 @@ struct VPNRoutingTableResolver { var ranges = VPNRoutingRange.alwaysExcludedIPv4Range + VPNRoutingRange.alwaysExcludedIPv6Range + serverRoutes() if excludeLocalNetworks { - Logger.networkProtection.log("🤌 Excluding local networks") ranges += VPNRoutingRange.localNetworkRange } @@ -57,7 +56,6 @@ struct VPNRoutingTableResolver { var routes = VPNRoutingRange.publicNetworkRange + dnsRoutes() if !excludeLocalNetworks { - Logger.networkProtection.log("🤌 Including local networks") routes += VPNRoutingRange.localNetworkRange } diff --git a/Sources/NetworkProtection/StartupOptions.swift b/Sources/NetworkProtection/StartupOptions.swift index f20caec80..dcc9ef4c6 100644 --- a/Sources/NetworkProtection/StartupOptions.swift +++ b/Sources/NetworkProtection/StartupOptions.swift @@ -21,7 +21,7 @@ import Common /// This class handles the proper parsing of the startup options for our tunnel. /// -struct StartupOptions { +public struct StartupOptions { enum StartupMethod: CustomDebugStringConvertible { /// Case started up manually from the main app. @@ -53,7 +53,7 @@ struct StartupOptions { /// /// Since these options are stored, the logic can allow for /// - enum StoredOption: Equatable { + public enum StoredOption: Equatable { case set(_ value: T) case reset case useExisting @@ -85,7 +85,7 @@ struct StartupOptions { // MARK: - Equatable - static func == (lhs: StartupOptions.StoredOption, rhs: StartupOptions.StoredOption) -> Bool { + public static func == (lhs: StartupOptions.StoredOption, rhs: StartupOptions.StoredOption) -> Bool { switch (lhs, rhs) { case (.reset, .reset): return true @@ -108,7 +108,7 @@ struct StartupOptions { let selectedServer: StoredOption let selectedLocation: StoredOption let dnsSettings: StoredOption - let excludeLocalNetworks: StoredOption + public let excludeLocalNetworks: StoredOption #if os(macOS) let authToken: StoredOption #endif From 86ddb4c779ce064d4a15abd5709b279ff08b2bc1 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Wed, 6 Nov 2024 14:54:06 +0100 Subject: [PATCH 11/22] Fixes several swiftlint warnings --- .../InternalUserDecider/UserDefaults+isInternalUser.swift | 2 +- .../Routing/{RoutingRange.swift => VPNRoutingRange.swift} | 0 .../UserDefaults+enforceRoutesForceEnabledOnce.swift | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) rename Sources/NetworkProtection/Routing/{RoutingRange.swift => VPNRoutingRange.swift} (100%) diff --git a/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift b/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift index 76dc72f21..a40e0887c 100644 --- a/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift +++ b/Sources/BrowserServicesKit/InternalUserDecider/UserDefaults+isInternalUser.swift @@ -1,5 +1,5 @@ // -// UserDefaults+internalUser.swift +// UserDefaults+isInternalUser.swift // // Copyright © 2024 DuckDuckGo. All rights reserved. // diff --git a/Sources/NetworkProtection/Routing/RoutingRange.swift b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift similarity index 100% rename from Sources/NetworkProtection/Routing/RoutingRange.swift rename to Sources/NetworkProtection/Routing/VPNRoutingRange.swift diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift index 1a5c8b412..d0f592a19 100644 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift +++ b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift @@ -1,7 +1,7 @@ // -// UserDefaults+enforceRoutes.swift +// UserDefaults+enforceRoutesForceEnabledOnce.swift // -// Copyright © 2023 DuckDuckGo. All rights reserved. +// Copyright © 2024 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. From 9b038e2b82495e4bf30fefdcb794b148957eab02 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Wed, 6 Nov 2024 15:11:32 +0100 Subject: [PATCH 12/22] Fixes unit tests --- .../MockNetworkProtectionDeviceManagement.swift | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift b/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift index fbbe427ef..608d2eb8d 100644 --- a/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift +++ b/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift @@ -20,6 +20,7 @@ import Foundation import NetworkProtection public final class MockNetworkProtectionDeviceManagement: NetworkProtectionDeviceManagement { + enum MockError: Error { case noStubSet } @@ -27,9 +28,8 @@ public final class MockNetworkProtectionDeviceManagement: NetworkProtectionDevic // swiftlint:disable:next large_tuple public var spyGenerateTunnelConfiguration: ( selectionMethod: NetworkProtection.NetworkProtectionServerSelectionMethod, - includedRoutes: [NetworkProtection.IPAddressRange], - excludedRoutes: [NetworkProtection.IPAddressRange], - isKillSwitchEnabled: Bool, + excludeLocalNetworks: Bool, + dnsSettings: NetworkProtectionDNSSettings, regenerateKey: Bool )? @@ -44,16 +44,13 @@ public final class MockNetworkProtectionDeviceManagement: NetworkProtectionDevic public func generateTunnelConfiguration( resolvedSelectionMethod: NetworkProtection.NetworkProtectionServerSelectionMethod, - includedRoutes: [NetworkProtection.IPAddressRange], - excludedRoutes: [NetworkProtection.IPAddressRange], + excludeLocalNetworks: Bool, dnsSettings: NetworkProtectionDNSSettings, - isKillSwitchEnabled: Bool, regenerateKey: Bool) async throws -> (tunnelConfiguration: NetworkProtection.TunnelConfiguration, server: NetworkProtection.NetworkProtectionServer) { spyGenerateTunnelConfiguration = ( selectionMethod: resolvedSelectionMethod, - includedRoutes: includedRoutes, - excludedRoutes: excludedRoutes, - isKillSwitchEnabled: isKillSwitchEnabled, + excludeLocalNetworks: excludeLocalNetworks, + dnsSettings: dnsSettings, regenerateKey: regenerateKey ) if let stubGenerateTunnelConfiguration { From 725579da41bced7ef27969c982f7c057b99f28c4 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Wed, 6 Nov 2024 15:22:17 +0100 Subject: [PATCH 13/22] Re-enabled some code that was disabled by mistake --- .../ConnectionErrorObserverThroughSession.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift b/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift index e7c94859c..03e5c65cd 100644 --- a/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift +++ b/Sources/NetworkProtection/Status/ConnectionErrorObserver/ConnectionErrorObserverThroughSession.swift @@ -97,9 +97,9 @@ public class ConnectionErrorObserverThroughSession: ConnectionErrorObserver { // MARK: - Obtaining the NetP VPN status private func updateTunnelErrorMessage(session: NETunnelProviderSession) throws { - /*try session.sendProviderMessage(.getLastErrorMessage) { [weak self] (errorMessage: ExtensionMessageString?) in + try session.sendProviderMessage(.getLastErrorMessage) { [weak self] (errorMessage: ExtensionMessageString?) in guard errorMessage?.value != self?.subject.value else { return } self?.subject.send(errorMessage?.value) - }*/ + } } } From d9cac684a7c488f39f85bb099e40e58428ef5ace Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Wed, 6 Nov 2024 16:12:08 +0100 Subject: [PATCH 14/22] Fixes unit tests --- ...ockNetworkProtectionDeviceManagement.swift | 1 - .../Mocks/NetworkProtectionServerMocks.swift | 14 ++++---- .../NetworkProtectionDeviceManagerTests.swift | 4 +-- .../NetworkProtectionServerInfoTests.swift | 4 +-- .../FailureRecoveryHandlerTests.swift | 34 ++++++------------- 5 files changed, 21 insertions(+), 36 deletions(-) diff --git a/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift b/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift index 608d2eb8d..faf94b4d8 100644 --- a/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift +++ b/Sources/NetworkProtectionTestUtils/MockNetworkProtectionDeviceManagement.swift @@ -25,7 +25,6 @@ public final class MockNetworkProtectionDeviceManagement: NetworkProtectionDevic case noStubSet } - // swiftlint:disable:next large_tuple public var spyGenerateTunnelConfiguration: ( selectionMethod: NetworkProtection.NetworkProtectionServerSelectionMethod, excludeLocalNetworks: Bool, diff --git a/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift b/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift index 33fe09f30..61b5787a3 100644 --- a/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift +++ b/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift @@ -32,8 +32,8 @@ extension NetworkProtectionServerInfo { static let mock = NetworkProtectionServerInfo(name: "Mock Server", publicKey: "ovn9RpzUuvQ4XLQt6B3RKuEXGIxa5QpTnehjduZlcSE=", hostNames: ["duckduckgo.com"], - ips: ["192.168.1.1"], - internalIP: "10.11.12.1", + ips: [AnyIPAddress("192.168.1.1")!], + internalIP: AnyIPAddress("10.11.12.1")!, port: 443, attributes: .init(city: "City", country: "Country", state: "State")) @@ -41,15 +41,15 @@ extension NetworkProtectionServerInfo { publicKey: "ovn9RpzUuvQ4XLQt6B3RKuEXGIxa5QpTnehjduZlcSE=", hostNames: ["duckduckgo.com"], ips: [], - internalIP: "10.11.12.1", + internalIP: AnyIPAddress("10.11.12.1")!, port: 443, attributes: .init(city: "City", country: "Country", state: "State")) static let ipAddressOnly = NetworkProtectionServerInfo(name: "Mock Server", publicKey: "ovn9RpzUuvQ4XLQt6B3RKuEXGIxa5QpTnehjduZlcSE=", hostNames: [], - ips: ["192.168.1.1"], - internalIP: "10.11.12.1", + ips: [AnyIPAddress("192.168.1.1")!], + internalIP: AnyIPAddress("10.11.12.1")!, port: 443, attributes: .init(city: "City", country: "Country", state: "State")) @@ -57,8 +57,8 @@ extension NetworkProtectionServerInfo { NetworkProtectionServerInfo(name: name, publicKey: publicKey, hostNames: ["duckduckgo.com"], - ips: ["192.168.1.1"], - internalIP: "10.11.12.1", + ips: [AnyIPAddress("192.168.1.1")!], + internalIP: AnyIPAddress("10.11.12.1")!, port: 443, attributes: .init(city: "City", country: "Country", state: "State")) } diff --git a/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift b/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift index 5bb3befc4..21b28e346 100644 --- a/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift +++ b/Tests/NetworkProtectionTests/NetworkProtectionDeviceManagerTests.swift @@ -212,10 +212,8 @@ extension NetworkProtectionDeviceManager { regenerateKey: Bool) async throws -> NetworkProtectionDeviceManager.GenerateTunnelConfigurationResult { try await generateTunnelConfiguration( resolvedSelectionMethod: selectionMethod, - includedRoutes: [], - excludedRoutes: [], + excludeLocalNetworks: false, dnsSettings: .default, - isKillSwitchEnabled: false, regenerateKey: regenerateKey ) } diff --git a/Tests/NetworkProtectionTests/NetworkProtectionServerInfoTests.swift b/Tests/NetworkProtectionTests/NetworkProtectionServerInfoTests.swift index 99cb9a4cd..0cd593edb 100644 --- a/Tests/NetworkProtectionTests/NetworkProtectionServerInfoTests.swift +++ b/Tests/NetworkProtectionTests/NetworkProtectionServerInfoTests.swift @@ -27,7 +27,7 @@ final class NetworkProtectionServerInfoTests: XCTestCase { publicKey: "", hostNames: [], ips: [], - internalIP: "10.11.12.1", + internalIP: AnyIPAddress("10.11.12.1")!, port: 42, attributes: .init(city: "Amsterdam", country: "nl", state: "na")) @@ -39,7 +39,7 @@ final class NetworkProtectionServerInfoTests: XCTestCase { publicKey: "", hostNames: [], ips: [], - internalIP: "10.11.12.1", + internalIP: AnyIPAddress("10.11.12.1")!, port: 42, attributes: .init(city: "New York", country: "us", state: "ny")) diff --git a/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift b/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift index f8742732b..6ec6711d9 100644 --- a/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift +++ b/Tests/NetworkProtectionTests/Recovery/FailureRecoveryHandlerTests.swift @@ -54,23 +54,17 @@ final class FailureRecoveryHandlerTests: XCTestCase { func testAttemptRecovery_callsDeviceManagerWithExpectedValues() async { let expectedServerName = "expectedServerName" let server = NetworkProtectionServer.registeredServer(named: expectedServerName) - let expectedIncludedRoutes: [IPAddressRange] = ["1.2.3.4/5"] - let expectedExcludedRoutes: [IPAddressRange] = ["10.9.8.7/6"] - let expectedKillSwitchEnabledValue = false + let expectedExcludeLocalNetworks = false await failureRecoveryHandler.attemptRecovery( to: server, - includedRoutes: expectedIncludedRoutes, - excludedRoutes: expectedExcludedRoutes, - dnsSettings: .default, - isKillSwitchEnabled: expectedKillSwitchEnabledValue + excludeLocalNetworks: expectedExcludeLocalNetworks, + dnsSettings: .default ) {_ in } guard let spyGenerateTunnelConfiguration = deviceManager.spyGenerateTunnelConfiguration else { XCTFail("attemptRecovery not called") return } - XCTAssertEqual(spyGenerateTunnelConfiguration.includedRoutes, expectedIncludedRoutes) - XCTAssertEqual(spyGenerateTunnelConfiguration.excludedRoutes, expectedExcludedRoutes) - XCTAssertEqual(spyGenerateTunnelConfiguration.isKillSwitchEnabled, expectedKillSwitchEnabledValue) + XCTAssertEqual(spyGenerateTunnelConfiguration.excludeLocalNetworks, expectedExcludeLocalNetworks) guard case .failureRecovery(let serverName) = spyGenerateTunnelConfiguration.selectionMethod else { XCTFail("Expected selectionMethod to equal failureRecover. Got \(spyGenerateTunnelConfiguration.selectionMethod)") @@ -127,10 +121,8 @@ final class FailureRecoveryHandlerTests: XCTestCase { ) await failureRecoveryHandler.attemptRecovery( to: .mockRegisteredServer, - includedRoutes: [], - excludedRoutes: [], - dnsSettings: .default, - isKillSwitchEnabled: false + excludeLocalNetworks: false, + dnsSettings: .default ) {_ in } XCTAssertEqual(startedCount, 1) @@ -314,10 +306,8 @@ final class FailureRecoveryHandlerTests: XCTestCase { deviceManager.stubGenerateTunnelConfigurationError = NetworkProtectionError.noServerRegistrationInfo await failureRecoveryHandler.attemptRecovery( to: .mockRegisteredServer, - includedRoutes: [], - excludedRoutes: [], - dnsSettings: .default, - isKillSwitchEnabled: false + excludeLocalNetworks: false, + dnsSettings: .default ) {_ in } } @@ -332,10 +322,8 @@ final class FailureRecoveryHandlerTests: XCTestCase { await failureRecoveryHandler.attemptRecovery( to: .mockRegisteredServer, - includedRoutes: [], - excludedRoutes: [], - dnsSettings: .default, - isKillSwitchEnabled: false + excludeLocalNetworks: false, + dnsSettings: .default ) { _ in let underlyingError = NSError(domain: "test", code: 1) throw WireGuardAdapterError.startWireGuardBackend(underlyingError) @@ -354,7 +342,7 @@ final class FailureRecoveryHandlerTests: XCTestCase { var newConfigResult: NetworkProtectionDeviceManagement.GenerateTunnelConfigurationResult? - await failureRecoveryHandler.attemptRecovery(to: lastServer, includedRoutes: [], excludedRoutes: [], dnsSettings: .default, isKillSwitchEnabled: true) { configResult in + await failureRecoveryHandler.attemptRecovery(to: lastServer, excludeLocalNetworks: false, dnsSettings: .default) { configResult in newConfigResult = configResult } return newConfigResult From 0ba1e7077f7ed4410497193c008d04ada3d91f69 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 8 Nov 2024 00:00:24 +0100 Subject: [PATCH 15/22] Updates the tunnel configuration to work best against TunnelCrack --- .../NetworkProtectionDeviceManager.swift | 1 - .../Routing/VPNRoutingTableResolver.swift | 25 +++++-------------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift index dab75867d..a599e150a 100644 --- a/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift +++ b/Sources/NetworkProtection/NetworkProtectionDeviceManager.swift @@ -285,7 +285,6 @@ public actor NetworkProtectionDeviceManager: NetworkProtectionDeviceManagement { } let routingTableResolver = VPNRoutingTableResolver( - server: server, dnsServers: dns, excludeLocalNetworks: excludeLocalNetworks) diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index e27aba92d..c461cb853 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -31,25 +31,20 @@ struct VPNRoutingTableResolver { private let dnsServers: [DNSServer] private let excludeLocalNetworks: Bool - private let server: NetworkProtectionServer - init(server: NetworkProtectionServer, - dnsServers: [DNSServer], + init(dnsServers: [DNSServer], excludeLocalNetworks: Bool) { self.dnsServers = dnsServers self.excludeLocalNetworks = excludeLocalNetworks - self.server = server } var excludedRoutes: [IPAddressRange] { - var ranges = VPNRoutingRange.alwaysExcludedIPv4Range + VPNRoutingRange.alwaysExcludedIPv6Range + serverRoutes() - - if excludeLocalNetworks { - ranges += VPNRoutingRange.localNetworkRange - } - - return ranges + // We currently don't define excluded routes, only included. Our testing show that this + // is what works best. Please see the task below for more technical details. + // + // Ref: https://app.asana.com/0/481882893211075/1208643192597095/f + return [] } var includedRoutes: [IPAddressRange] { @@ -62,14 +57,6 @@ struct VPNRoutingTableResolver { return routes } - // MARK: - Dynamic routes - - private func serverRoutes() -> [IPAddressRange] { - server.serverInfo.ips.map { anyIP in - IPAddressRange(address: anyIP.ipAddress, networkPrefixLength: 32) - } - } - // MARK: - Included Routes private func dnsRoutes() -> [IPAddressRange] { From 2c34dbde39f1d13326901743312574b69a17ae69 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 8 Nov 2024 01:21:31 +0100 Subject: [PATCH 16/22] Included local networks again --- .../Routing/VPNRoutingRange.swift | 2 +- .../Routing/VPNRoutingTableResolver.swift | 4 ++ ...faults+enforceRoutesForceEnabledOnce.swift | 37 ------------------- .../Settings/VPNSettings.swift | 10 ----- 4 files changed, 5 insertions(+), 48 deletions(-) delete mode 100644 Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift diff --git a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift index 22b7b8000..6693087b9 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift @@ -35,7 +35,7 @@ public enum VPNRoutingRange { ] public static let localNetworkRange: [NetworkProtection.IPAddressRange] = [ - "10.0.0.0/8", /* 255.0.0.0 */ + //"10.0.0.0/8", /* 255.0.0.0 */ "172.16.0.0/12", /* 255.240.0.0 */ "192.168.0.0/16", /* 255.255.0.0 */ ] diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index c461cb853..4f2e371d5 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -40,6 +40,10 @@ struct VPNRoutingTableResolver { } var excludedRoutes: [IPAddressRange] { + guard !excludeLocalNetworks else { + return VPNRoutingRange.localNetworkRange + } + // We currently don't define excluded routes, only included. Our testing show that this // is what works best. Please see the task below for more technical details. // diff --git a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift b/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift deleted file mode 100644 index d0f592a19..000000000 --- a/Sources/NetworkProtection/Settings/Extensions/UserDefaults+enforceRoutesForceEnabledOnce.swift +++ /dev/null @@ -1,37 +0,0 @@ -// -// UserDefaults+enforceRoutesForceEnabledOnce.swift -// -// Copyright © 2024 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 Combine -import Foundation - -extension UserDefaults { - private var key: String { - #keyPath(UserDefaults.networkProtectionSettingEnforceRoutesForceEnabledOnce) - } - - @objc - dynamic var networkProtectionSettingEnforceRoutesForceEnabledOnce: Bool { - get { - bool(forKey: key) - } - - set { - set(newValue, forKey: key) - } - } -} diff --git a/Sources/NetworkProtection/Settings/VPNSettings.swift b/Sources/NetworkProtection/Settings/VPNSettings.swift index 4a5465c40..a5ccbbb37 100644 --- a/Sources/NetworkProtection/Settings/VPNSettings.swift +++ b/Sources/NetworkProtection/Settings/VPNSettings.swift @@ -272,16 +272,6 @@ public final class VPNSettings { // MARK: - Enforce Routes - public var enforceRoutesForceEnabledOnce: Bool { - get { - defaults.networkProtectionSettingEnforceRoutesForceEnabledOnce - } - - set { - defaults.networkProtectionSettingEnforceRoutesForceEnabledOnce = newValue - } - } - public var enforceRoutesPublisher: AnyPublisher { defaults.networkProtectionSettingEnforceRoutesPublisher } From b717437c1e06e8e49a76d8d62cf19cac9a58588e Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 8 Nov 2024 02:18:04 +0100 Subject: [PATCH 17/22] Updates the excluded routes --- .../Routing/VPNRoutingTableResolver.swift | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift index 4f2e371d5..505aa455a 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingTableResolver.swift @@ -40,15 +40,13 @@ struct VPNRoutingTableResolver { } var excludedRoutes: [IPAddressRange] { - guard !excludeLocalNetworks else { - return VPNRoutingRange.localNetworkRange + var routes = VPNRoutingRange.alwaysExcludedIPv4Range + + if excludeLocalNetworks { + routes += VPNRoutingRange.localNetworkRange } - // We currently don't define excluded routes, only included. Our testing show that this - // is what works best. Please see the task below for more technical details. - // - // Ref: https://app.asana.com/0/481882893211075/1208643192597095/f - return [] + return routes } var includedRoutes: [IPAddressRange] { From e89ceb7c59fcb4d803c5fd1fee6167ffb2020fa5 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 8 Nov 2024 02:21:43 +0100 Subject: [PATCH 18/22] Fixes a swiflint warning --- Sources/NetworkProtection/Routing/VPNRoutingRange.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift index 6693087b9..d72f63628 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift @@ -35,7 +35,7 @@ public enum VPNRoutingRange { ] public static let localNetworkRange: [NetworkProtection.IPAddressRange] = [ - //"10.0.0.0/8", /* 255.0.0.0 */ + // "10.0.0.0/8", /* 255.0.0.0 */ "172.16.0.0/12", /* 255.240.0.0 */ "192.168.0.0/16", /* 255.255.0.0 */ ] From f05ad5ee4cc36cf719ea94c23395d0e57e975698 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 8 Nov 2024 03:25:26 +0100 Subject: [PATCH 19/22] Rolls back an unintentional change --- .../Routing/VPNRoutingRange.swift | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift index d72f63628..292d5cc9b 100644 --- a/Sources/NetworkProtection/Routing/VPNRoutingRange.swift +++ b/Sources/NetworkProtection/Routing/VPNRoutingRange.swift @@ -21,10 +21,10 @@ import Foundation public enum VPNRoutingRange { public static let alwaysExcludedIPv4Range: [NetworkProtection.IPAddressRange] = [ - "127.0.0.0/8", /* 255.0.0.0 Loopback */ - "169.254.0.0/16", /* 255.255.0.0 Link-local */ - "224.0.0.0/4", /* 240.0.0.0 Multicast */ - "240.0.0.0/4", /* 240.0.0.0 Class E */ + "127.0.0.1/8", /* 255.0.0.0 Loopback */ + "169.254.0.1/16", /* 255.255.0.0 Link-local */ + "224.0.0.1/4", /* 240.0.0.0 Multicast */ + "240.0.0.1/4", /* 240.0.0.0 Class E */ ] public static let alwaysExcludedIPv6Range: [NetworkProtection.IPAddressRange] = [ @@ -36,43 +36,43 @@ public enum VPNRoutingRange { public static let localNetworkRange: [NetworkProtection.IPAddressRange] = [ // "10.0.0.0/8", /* 255.0.0.0 */ - "172.16.0.0/12", /* 255.240.0.0 */ - "192.168.0.0/16", /* 255.255.0.0 */ + "172.16.0.1/12", /* 255.240.0.0 */ + "192.168.0.1/16", /* 255.255.0.0 */ ] public static let publicNetworkRange: [NetworkProtection.IPAddressRange] = [ - "1.0.0.0/8", - "2.0.0.0/8", - "3.0.0.0/8", - "4.0.0.0/6", - "8.0.0.0/7", - "11.0.0.0/8", - "12.0.0.0/6", - "16.0.0.0/4", - "32.0.0.0/3", - "64.0.0.0/2", - "128.0.0.0/3", - "160.0.0.0/5", - "168.0.0.0/6", - "172.0.0.0/12", - "172.32.0.0/11", - "172.64.0.0/10", - "172.128.0.0/9", - "173.0.0.0/8", - "174.0.0.0/7", - "176.0.0.0/4", - "192.0.0.0/9", - "192.128.0.0/11", - "192.160.0.0/13", - "192.169.0.0/16", - "192.170.0.0/15", - "192.172.0.0/14", - "192.176.0.0/12", - "192.192.0.0/10", - "193.0.0.0/8", - "194.0.0.0/7", - "196.0.0.0/6", - "200.0.0.0/5", - "208.0.0.0/4", + "1.0.0.1/8", + "2.0.0.1/8", + "3.0.0.1/8", + "4.0.0.1/6", + "8.0.0.1/7", + "11.0.0.1/8", + "12.0.0.1/6", + "16.0.0.1/4", + "32.0.0.1/3", + "64.0.0.1/2", + "128.0.0.1/3", + "160.0.0.1/5", + "168.0.0.1/6", + "172.0.0.1/12", + "172.32.0.1/11", + "172.64.0.1/10", + "172.128.0.1/9", + "173.0.0.1/8", + "174.0.0.1/7", + "176.0.0.1/4", + "192.0.0.1/9", + "192.128.0.1/11", + "192.160.0.1/13", + "192.169.0.1/16", + "192.170.0.1/15", + "192.172.0.1/14", + "192.176.0.1/12", + "192.192.0.1/10", + "193.0.0.1/8", + "194.0.0.1/7", + "196.0.0.1/6", + "200.0.0.1/5", + "208.0.0.1/4", ] } From 0dead1bd202a7e77014e0982a439a1d42324977e Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 8 Nov 2024 14:08:48 +0100 Subject: [PATCH 20/22] Updates a protocol --- Sources/NetworkProtection/Controllers/TunnelController.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/NetworkProtection/Controllers/TunnelController.swift b/Sources/NetworkProtection/Controllers/TunnelController.swift index be56206de..abc4d50be 100644 --- a/Sources/NetworkProtection/Controllers/TunnelController.swift +++ b/Sources/NetworkProtection/Controllers/TunnelController.swift @@ -33,6 +33,10 @@ public protocol TunnelController { /// func stop() async + /// Sends a command to the adapter + /// + func command(_ command: VPNCommand) async throws + /// Whether the tunnel is connected /// var isConnected: Bool { get async } From bc7bf68f41b03da3a86563d2de98bee0e47f9957 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 11 Nov 2024 20:05:52 +0100 Subject: [PATCH 21/22] Fixes an issue with the unit tests --- .../Controllers/MockTunnelController.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Sources/NetworkProtectionTestUtils/Controllers/MockTunnelController.swift b/Sources/NetworkProtectionTestUtils/Controllers/MockTunnelController.swift index 0e822db2e..01f818276 100644 --- a/Sources/NetworkProtectionTestUtils/Controllers/MockTunnelController.swift +++ b/Sources/NetworkProtectionTestUtils/Controllers/MockTunnelController.swift @@ -34,6 +34,11 @@ public final class MockTunnelController: TunnelController, TunnelSessionProvider didCallStop = true } + public var calledCommand: VPNCommand? + public func command(_ command: VPNCommand) async throws { + calledCommand = command + } + public var isConnected: Bool { true } From 2ded3f80384e6a05b6427e2920b3ca1f2497207a Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Mon, 11 Nov 2024 20:59:52 +0100 Subject: [PATCH 22/22] Removes some commented out code --- .../Mocks/NetworkProtectionServerMocks.swift | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift b/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift index 61b5787a3..a28b5c03b 100644 --- a/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift +++ b/Tests/NetworkProtectionTests/Mocks/NetworkProtectionServerMocks.swift @@ -18,14 +18,6 @@ import Foundation @testable import NetworkProtection -/* -extension AnyIPAddress: ExpressibleByStringLiteral { - - public init(stringLiteral: String) { - self.init(stringLiteral)! - } - -}*/ extension NetworkProtectionServerInfo {