Remediate TunnelVision, TunnelCrack and fix "Exclude Local Networks" (#…

…3460) Task/Issue URL: https://app.asana.com/0/1206580121312550/1208686409805161/f Tech Design URL: https://app.asana.com/0/481882893211075/1208643192597095/f macOS PR: duckduckgo/macos-browser#3422 BSK PR: duckduckgo/BrowserServicesKit#1039 ## Description Remediate TunnelVision, TunnelCrack and fix "Exclude Local Networks".
duckduckgo · Nov 11, 2024 · f8fc6c5 · f8fc6c5
1 parent a18bfbb
commit f8fc6c5
Show file tree

Hide file tree

Showing 10 changed files with 103 additions and 24 deletions.
diff --git a/Core/FeatureFlag.swift b/Core/FeatureFlag.swift
@@ -49,6 +49,9 @@ public enum FeatureFlag: String {
 
     /// https://app.asana.com/0/72649045549333/1208231259093710/f
     case networkProtectionUserTips
+
+    /// https://app.asana.com/0/72649045549333/1208617860225199/f
+    case networkProtectionEnforceRoutes
 }
 
 extension FeatureFlag: FeatureFlagSourceProviding {
@@ -104,6 +107,8 @@ extension FeatureFlag: FeatureFlagSourceProviding {
             return .remoteReleasable(.feature(.autocompleteTabs))
         case .networkProtectionUserTips:
             return .remoteReleasable(.subfeature(NetworkProtectionSubfeature.userTips))
+        case .networkProtectionEnforceRoutes:
+            return .remoteDevelopment(.subfeature(NetworkProtectionSubfeature.enforceRoutes))
         case .adAttributionReporting:
             return .remoteReleasable(.feature(.adAttributionReporting))
         }

diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj
@@ -10980,7 +10980,7 @@
 			repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit";
 			requirement = {
 				kind = exactVersion;
-				version = 208.1.0;
+				version = 209.0.0;
 			};
 		};
 		9F8FE9472BAE50E50071E372 /* XCRemoteSwiftPackageReference "lottie-spm" */ = {

diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -32,8 +32,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/DuckDuckGo/BrowserServicesKit",
       "state" : {
-        "revision" : "6be781530a2516c703b8e1bcf0c90e6e763d3300",
-        "version" : "208.1.0"
+        "revision" : "614ea57db48db644ce7f3a3de9c20c9a7fbb08ff",
+        "version" : "209.0.0"
       }
     },
     {

diff --git a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo.xcscheme b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/DuckDuckGo.xcscheme
@@ -65,6 +65,9 @@
                <Test
                   Identifier = "AutoconsentMessageProtocolTests/testEval()">
                </Test>
+               <Test
+                  Identifier = "FireButtonReferenceTests/testClearDataUsingLegacyContainer()">
+               </Test>
                <Test
                   Identifier = "SyncSettingsViewControllerErrorTests/test_WhenAccountRemoved_ErrorHandlerSyncDidTurnOffCalled()">
                </Test>

diff --git a/DuckDuckGo/AppDependencyProvider.swift b/DuckDuckGo/AppDependencyProvider.swift
@@ -113,7 +113,7 @@ final class AppDependencyProvider: DependencyProvider {
                                                    entitlementsCache: entitlementsCache,
                                                    subscriptionEndpointService: subscriptionService,
                                                    authEndpointService: authService)
-        
+
         let subscriptionManager = DefaultSubscriptionManager(storePurchaseManager: DefaultStorePurchaseManager(),
                                                              accountManager: accountManager,
                                                              subscriptionEndpointService: subscriptionService,
@@ -126,17 +126,14 @@ final class AppDependencyProvider: DependencyProvider {
         let accessTokenProvider: () -> String? = {
             return { accountManager.accessToken }
         }()
-#if os(macOS)
-        networkProtectionKeychainTokenStore = NetworkProtectionKeychainTokenStore(keychainType: .dataProtection(.unspecified),
-                                                                                  serviceName: "\(Bundle.main.bundleIdentifier!).authToken",
-                                                                                  errorEvents: .networkProtectionAppDebugEvents,
-                                                                                  accessTokenProvider: accessTokenProvider)
-#else
+
         networkProtectionKeychainTokenStore = NetworkProtectionKeychainTokenStore(accessTokenProvider: accessTokenProvider)
-#endif
+
         networkProtectionTunnelController = NetworkProtectionTunnelController(accountManager: accountManager,
                                                                               tokenStore: networkProtectionKeychainTokenStore,
-                                                                              persistentPixel: persistentPixel)
+                                                                              featureFlagger: featureFlagger,
+                                                                              persistentPixel: persistentPixel,
+                                                                              settings: vpnSettings)
         vpnFeatureVisibility = DefaultNetworkProtectionVisibility(userDefaults: .networkProtectionGroupDefaults,
                                                                   accountManager: accountManager)
     }

diff --git a/DuckDuckGo/NetworkProtectionConvenienceInitialisers.swift b/DuckDuckGo/NetworkProtectionConvenienceInitialisers.swift
@@ -59,6 +59,7 @@ extension NetworkProtectionVPNSettingsViewModel {
     convenience init() {
         self.init(
             notificationsAuthorization: NotificationsAuthorizationController(),
+            controller: AppDependencyProvider.shared.networkProtectionTunnelController,
             settings: AppDependencyProvider.shared.vpnSettings
         )
     }

diff --git a/DuckDuckGo/NetworkProtectionDebugViewController.swift b/DuckDuckGo/NetworkProtectionDebugViewController.swift
@@ -63,6 +63,7 @@ final class NetworkProtectionDebugViewController: UITableViewController {
 
     enum DebugFeatureRows: Int, CaseIterable {
         case toggleAlwaysOn
+        case enforceRoutes
     }
 
     enum SimulateFailureRows: Int, CaseIterable {
@@ -324,6 +325,14 @@ final class NetworkProtectionDebugViewController: UITableViewController {
             } else {
                 cell.accessoryType = .checkmark
             }
+        case .enforceRoutes:
+            cell.textLabel?.text = "Enforce Routes"
+
+            if !AppDependencyProvider.shared.vpnSettings.enforceRoutes {
+                cell.accessoryType = .none
+            } else {
+                cell.accessoryType = .checkmark
+            }
         default:
             break
         }
@@ -334,6 +343,9 @@ final class NetworkProtectionDebugViewController: UITableViewController {
         case .toggleAlwaysOn:
             debugFeatures.alwaysOnDisabled.toggle()
             tableView.reloadRows(at: [indexPath], with: .none)
+        case .enforceRoutes:
+            AppDependencyProvider.shared.vpnSettings.enforceRoutes.toggle()
+            tableView.reloadRows(at: [indexPath], with: .none)
         default:
             break
         }

diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift
@@ -17,9 +17,10 @@
 //  limitations under the License.
 //
 
-import Foundation
+import BrowserServicesKit
 import Combine
 import Core
+import Foundation
 import NetworkExtension
 import NetworkProtection
 import Subscription
@@ -34,6 +35,7 @@ enum VPNConfigurationRemovalReason: String {
 final class NetworkProtectionTunnelController: TunnelController, TunnelSessionProvider {
     static var shouldSimulateFailure: Bool = false
 
+    private let featureFlagger: FeatureFlagger
     private var internalManager: NETunnelProviderManager?
     private let debugFeatures = NetworkProtectionDebugFeatures()
     private let tokenStore: NetworkProtectionKeychainTokenStore
@@ -42,6 +44,7 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
     private let notificationCenter: NotificationCenter = .default
     private var previousStatus: NEVPNStatus = .invalid
     private let persistentPixel: PersistentPixelFiring
+    private let settings: VPNSettings
     private var cancellables = Set<AnyCancellable>()
 
     // MARK: - Manager, Session, & Connection
@@ -119,9 +122,25 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
         }
     }
 
-    init(accountManager: AccountManager, tokenStore: NetworkProtectionKeychainTokenStore, persistentPixel: PersistentPixelFiring) {
-        self.tokenStore = tokenStore
+    // MARK: - Enforce Routes
+
+    private var enforceRoutes: Bool {
+        featureFlagger.isFeatureOn(.networkProtectionEnforceRoutes)
+    }
+
+    // MARK: - Initializers
+
+    init(accountManager: AccountManager,
+         tokenStore: NetworkProtectionKeychainTokenStore,
+         featureFlagger: FeatureFlagger,
+         persistentPixel: PersistentPixelFiring,
+         settings: VPNSettings) {
+
+        self.featureFlagger = featureFlagger
         self.persistentPixel = persistentPixel
+        self.settings = settings
+        self.tokenStore = tokenStore
+
         subscribeToSnoozeTimingChanges()
         subscribeToStatusChanges()
         subscribeToConfigurationChanges()
@@ -180,6 +199,16 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
         tunnelManager.connection.stopVPNTunnel()
     }
 
+    func command(_ command: VPNCommand) async throws {
+        guard let activeSession = await AppDependencyProvider.shared.networkProtectionTunnelController.activeSession(),
+            activeSession.status == .connected else {
+
+            return
+        }
+
+        try? await activeSession.sendProviderRequest(.command(command))
+    }
+
     func removeVPN(reason: VPNConfigurationRemovalReason) async {
         do {
             try await tunnelManager?.removeFromPreferences()
@@ -293,6 +322,7 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
         return tunnelManager
     }
 
+    @MainActor
     private func setupAndSave(_ tunnelManager: NETunnelProviderManager) async throws {
         setup(tunnelManager)
 
@@ -319,6 +349,7 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
 
     /// Setups the tunnel manager if it's not set up already.
     ///
+    @MainActor
     private func setup(_ tunnelManager: NETunnelProviderManager) {
         tunnelManager.localizedDescription = "DuckDuckGo VPN"
         tunnelManager.isEnabled = true
@@ -327,9 +358,24 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
             let protocolConfiguration = NETunnelProviderProtocol()
             protocolConfiguration.serverAddress = "127.0.0.1" // Dummy address... the NetP service will take care of grabbing a real server
 
+            protocolConfiguration.providerConfiguration = [:]
+
             // always-on
             protocolConfiguration.disconnectOnSleep = false
 
+            // Enforce routes
+            protocolConfiguration.enforceRoutes = enforceRoutes
+
+            // We will control excluded networks through includedRoutes / excludedRoutes
+            protocolConfiguration.excludeLocalNetworks = false
+
+            #if DEBUG
+            if #available(iOS 17.4, *) {
+                // This is useful to ensure debugging is never blocked by the VPN
+                protocolConfiguration.excludeDeviceCommunication = true
+            }
+            #endif
+
             return protocolConfiguration
         }()
 

diff --git a/DuckDuckGo/NetworkProtectionVPNSettingsView.swift b/DuckDuckGo/NetworkProtectionVPNSettingsView.swift
@@ -50,9 +50,6 @@ struct NetworkProtectionVPNSettingsView: View {
                     footerText: UserText.netPExcludeLocalNetworksSettingFooter
                 ) {
                     Toggle("", isOn: $viewModel.excludeLocalNetworks)
-                        .onTapGesture {
-                            viewModel.toggleExcludeLocalNetworks()
-                        }
                 }
 
                 dnsSection()

diff --git a/DuckDuckGo/NetworkProtectionVPNSettingsViewModel.swift b/DuckDuckGo/NetworkProtectionVPNSettingsViewModel.swift
@@ -29,6 +29,7 @@ enum NetworkProtectionNotificationsViewKind: Equatable {
 }
 
 final class NetworkProtectionVPNSettingsViewModel: ObservableObject {
+    private let controller: TunnelController
     private let settings: VPNSettings
     private var cancellables: Set<AnyCancellable> = []
 
@@ -39,11 +40,32 @@ final class NetworkProtectionVPNSettingsViewModel: ObservableObject {
         self.settings.notifyStatusChanges
     }
 
-    @Published public var excludeLocalNetworks: Bool = true
+    @Published public var excludeLocalNetworks: Bool {
+        didSet {
+            guard oldValue != excludeLocalNetworks else {
+                return
+            }
+
+            settings.excludeLocalNetworks = excludeLocalNetworks
+
+            Task {
+                // We need to allow some time for the setting to propagate
+                // But ultimately this should actually be a user choice
+                try await Task.sleep(interval: 0.1)
+                try await controller.command(.restartAdapter)
+            }
+        }
+    }
+
     @Published public var usesCustomDNS = false
     @Published public var dnsServers: String = UserText.vpnSettingDNSServerDefaultValue
 
-    init(notificationsAuthorization: NotificationsAuthorizationControlling, settings: VPNSettings) {
+    init(notificationsAuthorization: NotificationsAuthorizationControlling,
+         controller: TunnelController,
+         settings: VPNSettings) {
+
+        self.controller = controller
+        self.excludeLocalNetworks = settings.excludeLocalNetworks
         self.settings = settings
         self.notificationsAuthorization = notificationsAuthorization
 
@@ -77,10 +99,6 @@ final class NetworkProtectionVPNSettingsViewModel: ObservableObject {
         settings.notifyStatusChanges = enabled
     }
 
-    func toggleExcludeLocalNetworks() {
-        settings.excludeLocalNetworks.toggle()
-    }
-
     private static func localizedString(forRegionCode: String) -> String {
         Locale.current.localizedString(forRegionCode: forRegionCode) ?? forRegionCode.capitalized
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -65,6 +65,9 @@ @@
                    <Test
                       Identifier = "AutoconsentMessageProtocolTests/testEval()">
                    </Test>
+                   <Test
+                      Identifier = "FireButtonReferenceTests/testClearDataUsingLegacyContainer()">
+                   </Test>
                    <Test
                       Identifier = "SyncSettingsViewControllerErrorTests/test_WhenAccountRemoved_ErrorHandlerSyncDidTurnOffCalled()">
                    </Test>
@@ Expand Down @@