Remediate TunnelVision, TunnelCrack and fix "Exclude Local Networks"

DuckDuckGo.xcodeproj/project.pbxproj

@@ -14683,8 +14683,8 @@
 			isa = XCRemoteSwiftPackageReference;
 			repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
 			requirement = {
-				kind = exactVersion;
-				version = 203.1.0;
+				kind = revision;
+				revision = 432b5481e48671e2e644fe98e46cbeb512df0903;
 			};
 		};
 		9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = {

DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

@@ -32,8 +32,7 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/duckduckgo/BrowserServicesKit",
       "state" : {
-        "revision" : "19f1e5c945aa92562ad2d087e8d6c99801edf656",
-        "version" : "203.1.0"
+        "revision" : "432b5481e48671e2e644fe98e46cbeb512df0903"
       }
     },
     {

DuckDuckGo/Application/AppDelegate.swift

@@ -76,6 +76,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
     private(set) var stateRestorationManager: AppStateRestorationManager!
     private var grammarFeaturesManager = GrammarFeaturesManager()
     let internalUserDecider: InternalUserDecider
+    private var isInternalUserSharingCancellable: AnyCancellable?
     let featureFlagger: FeatureFlagger
     private var appIconChanger: AppIconChanger!
     private var autoClearHandler: AutoClearHandler!
@@ -429,6 +430,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
 
         subscribeToEmailProtectionStatusNotifications()
         subscribeToDataImportCompleteNotification()
+        subscribeToInternalUserChanges()
 
         fireFailedCompilationsPixelIfNeeded()
 
@@ -742,6 +744,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
         NotificationCenter.default.addObserver(self, selector: #selector(dataImportCompleteNotification(_:)), name: .dataImportComplete, object: nil)
     }
 
+    private func subscribeToInternalUserChanges() {
+        UserDefaults.appConfiguration.isInternalUser = internalUserDecider.isInternalUser
+
+        isInternalUserSharingCancellable = internalUserDecider.isInternalUserPublisher
+            .assign(to: \.isInternalUser, onWeaklyHeld: UserDefaults.appConfiguration)
+    }
+
     private func emailDidSignInNotification(_ notification: Notification) {
         PixelKit.fire(NonStandardEvent(NonStandardPixel.emailEnabled))
         if AppDelegate.isNewUser {

DuckDuckGo/FeatureFlagging/Model/FeatureFlag.swift

@@ -41,6 +41,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 {
@@ -66,6 +69,8 @@ extension FeatureFlag: FeatureFlagSourceProviding {
             return .remoteReleasable(.subfeature(AutofillSubfeature.credentialsImportPromotionForExistingUsers))
         case .networkProtectionUserTips:
             return .remoteDevelopment(.subfeature(NetworkProtectionSubfeature.userTips))
+        case .networkProtectionEnforceRoutes:
+            return .remoteDevelopment(.subfeature(NetworkProtectionSubfeature.enforceRoutes))
         }
     }
 }

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionDebugMenu.swift

@@ -69,18 +69,23 @@ final class NetworkProtectionDebugMenu: NSMenu {
                 NSMenuItem(title: "Reset All State", action: #selector(NetworkProtectionDebugMenu.resetAllState))
                     .targetting(self)
 
-                NSMenuItem(title: "Reset Site Issue Alert", action: #selector(NetworkProtectionDebugMenu.resetSiteIssuesAlert(_:)))
+                NSMenuItem.separator() // Resetting single components should go below this point
+
+                NSMenuItem(title: "Remove Network Extension and Login Items", action: #selector(NetworkProtectionDebugMenu.removeSystemExtensionAndAgents))
+                    .targetting(self)
+
+                NSMenuItem(title: "Remove VPN configuration", action: #selector(NetworkProtectionDebugMenu.removeVPNConfiguration(_:)))
                     .targetting(self)
 
                 resetToDefaults
                     .targetting(self)
 
-                NSMenuItem.separator()
+                NSMenuItem.separator() // Resetting VPN subfeatures should go below this point
 
-                NSMenuItem(title: "Remove Network Extension and Login Items", action: #selector(NetworkProtectionDebugMenu.removeSystemExtensionAndAgents))
+                NSMenuItem(title: "Reset Enforce Routes force-enabled flag", action: #selector(NetworkProtectionDebugMenu.resetEnforceRoutesForceEnabledFlag(_:)))
                     .targetting(self)
 
-                NSMenuItem(title: "Remove VPN configuration", action: #selector(NetworkProtectionDebugMenu.removeVPNConfiguration(_:)))
+                NSMenuItem(title: "Reset Site Issue Alert", action: #selector(NetworkProtectionDebugMenu.resetSiteIssuesAlert(_:)))
                     .targetting(self)
             }
 
@@ -185,6 +190,10 @@ final class NetworkProtectionDebugMenu: NSMenu {
         }
     }
 
+    @objc func resetEnforceRoutesForceEnabledFlag(_ sender: Any?) {
+        settings.enforceRoutesForceEnabledOnce = false
+    }
+
     @objc func resetSiteIssuesAlert(_ sender: Any?) {
         debugUtilities.resetSiteIssuesAlert()
     }
@@ -310,14 +319,29 @@ final class NetworkProtectionDebugMenu: NSMenu {
 
     @objc func toggleEnforceRoutesAction(_ sender: Any?) {
         settings.enforceRoutes.toggle()
+
+        Task {
+            try await Task.sleep(interval: 0.1)
+            try await debugUtilities.restartAdapter()
+        }
     }
 
     @objc func toggleIncludeAllNetworks(_ sender: Any?) {
         settings.includeAllNetworks.toggle()
+
+        Task {
+            try await Task.sleep(interval: 0.1)
+            try await debugUtilities.restartAdapter()
+        }
     }
 
     @objc func toggleShouldExcludeLocalRoutes(_ sender: Any?) {
         settings.excludeLocalNetworks.toggle()
+
+        Task {
+            try await Task.sleep(interval: 0.1)
+            try await debugUtilities.restartAdapter()
+        }
     }
 
     @objc func openAppContainerInFinder(_ sender: Any?) {

DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionTunnelController.swift

@@ -34,18 +34,17 @@ import SystemExtensions
 #endif
 
 import Subscription
+import BrowserServicesKit
 
 typealias NetworkProtectionStatusChangeHandler = (NetworkProtection.ConnectionStatus) -> Void
 typealias NetworkProtectionConfigChangeHandler = () -> Void
 
 final class NetworkProtectionTunnelController: TunnelController, TunnelSessionProvider {
 
-    // MARK: - Settings
+    // MARK: - Configuration
 
+    private let internalUserDecider: InternalUserDecider
     let settings: VPNSettings
-
-    // MARK: - Defaults
-
     let defaults: UserDefaults
 
     // MARK: - Combine Cancellables
@@ -154,11 +153,13 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
     ///
     init(networkExtensionBundleID: String,
          networkExtensionController: NetworkExtensionController,
+         internalUserDecider: InternalUserDecider,
          settings: VPNSettings,
          defaults: UserDefaults,
          notificationCenter: NotificationCenter = .default,
          accessTokenStorage: SubscriptionTokenKeychainStorage) {
 
+        self.internalUserDecider = internalUserDecider
         self.networkExtensionBundleID = networkExtensionBundleID
         self.networkExtensionController = networkExtensionController
         self.notificationCenter = notificationCenter
@@ -295,8 +296,7 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
     }
 
     private func handleSetExcludeLocalNetworks(_ excludeLocalNetworks: Bool) async throws {
-        guard let tunnelManager = await manager,
-              tunnelManager.protocolConfiguration?.excludeLocalNetworks == !excludeLocalNetworks else {
+        guard let tunnelManager = await manager else {
             return
         }
 
@@ -349,23 +349,19 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
             protocolConfiguration.providerBundleIdentifier = Bundle.tunnelExtensionBundleID
             protocolConfiguration.providerConfiguration = [
                 NetworkProtectionOptionKey.defaultPixelHeaders: APIRequest.Headers().httpHeaders,
-                NetworkProtectionOptionKey.includedRoutes: includedRoutes().map(\.stringRepresentation) as NSArray
             ]
 
             // always-on
             protocolConfiguration.disconnectOnSleep = false
 
             // kill switch
-            protocolConfiguration.enforceRoutes = settings.enforceRoutes
+            protocolConfiguration.enforceRoutes = enforceRoutes
 
             // this setting breaks Connection Tester
             protocolConfiguration.includeAllNetworks = settings.includeAllNetworks
 
-            // This is intentionally not used but left here for documentation purposes.
-            // The reason for this is that we want to have full control of the routes that
-            // are excluded, so instead of using this setting we're just configuring the
-            // excluded routes through our VPNSettings class, which our extension reads directly.
-            // protocolConfiguration.excludeLocalNetworks = settings.excludeLocalNetworks
+            // This messes up the routing, so please keep it always disabled
+            protocolConfiguration.excludeLocalNetworks = false
 
             return protocolConfiguration
         }()
@@ -618,6 +614,8 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
         options[NetworkProtectionOptionKey.selectedEnvironment] = settings.selectedEnvironment.rawValue as NSString
         options[NetworkProtectionOptionKey.selectedServer] = settings.selectedServer.stringValue as? NSString
 
+        options[NetworkProtectionOptionKey.excludeLocalNetworks] = NSNumber(value: settings.excludeLocalNetworks)
+
 #if NETP_SYSTEM_EXTENSION
         if let data = try? JSONEncoder().encode(settings.selectedLocation) {
             options[NetworkProtectionOptionKey.selectedLocation] = NSData(data: data)
@@ -732,54 +730,25 @@ final class NetworkProtectionTunnelController: TunnelController, TunnelSessionPr
         try await tunnelManager.saveToPreferences()
     }
 
-    /* Temporarily disabled until we fix this menu: https://app.asana.com/0/0/1205766100762904/f
-    @MainActor
-    private func excludedRoutes() -> [NetworkProtection.IPAddressRange] {
-        settings.excludedRoutes.compactMap { [excludedRoutesPreferences] item -> NetworkProtection.IPAddressRange? in
-            guard case .exclusion(range: let range, description: _, default: let defaultValue) = item,
-                  excludedRoutesPreferences[range.stringRepresentation, default: defaultValue] == true
-            else { return nil }
-            // TO BE fixed:
-            // when 10.11.12.1 DNS is used 10.0.0.0/8 should be included (not excluded)
-            // but marking 10.11.12.1 as an Included Route breaks tunnel (probably these routes are conflicting)
-            if settings.enforceRoutes && range == "10.0.0.0/8" {
-                return nil
-            }
+    // MARK: - Routing
 
-            return range
-        }
-    }*/
+    private var enforceRoutes: Bool {
 
-    /// extra Included Routes appended to 0.0.0.0, ::/0 (peers) and interface.addresses
-    @MainActor
-    private func includedRoutes() -> [NetworkProtection.IPAddressRange] {
-        []
-    }
-
-    /* Temporarily disabled - https://app.asana.com/0/0/1205766100762904/f
-    @MainActor
-    func setExcludedRoute(_ route: String, enabled: Bool) {
-        excludedRoutesPreferences[route] = enabled
-    }
-
-    @MainActor
-    func isExcludedRouteEnabled(_ route: String) -> Bool {
-        guard let range = IPAddressRange(from: route),
-              let exclusionListItem = settings.exclusionList.first(where: {
-                  if case .exclusion(range: range, description: _, default: _) = $0 { return true }
-                  return false
-              }),
-              case .exclusion(range: _, description: _, default: let defaultValue) = exclusionListItem else {
-
-            assertionFailure("Invalid route \(route)")
+        guard internalUserDecider.isInternalUser else {
             return false
         }
-        // TO BE fixed: see excludedRoutes()
-        if settings.enforceRoutes && route == "10.0.0.0/8" {
-            return false
+
+        /// Even though there's a remote feature flag, we still want to allow internal users to disable enforceRoutes
+        /// manually in case they have trouble.  To achieve this we force the setting to ON once, but otherwise
+        /// allow the internal user to disable it again.
+        if !settings.enforceRoutesForceEnabledOnce {
+            settings.enforceRoutesForceEnabledOnce = true
+            settings.excludeLocalNetworks = true
+            settings.enforceRoutes = true
         }
-        return excludedRoutesPreferences[route, default: defaultValue]
-    }*/
+
+        return settings.enforceRoutes
+    }
 
     struct TunnelFailureError: LocalizedError {
         let errorDescription: String?

DuckDuckGo/Preferences/Model/VPNPreferencesModel.swift

@@ -39,7 +39,11 @@ final class VPNPreferencesModel: ObservableObject {
     @Published var excludeLocalNetworks: Bool {
         didSet {
             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 vpnXPCClient.command(.restartAdapter)
             }
         }

DuckDuckGoVPN/DuckDuckGoVPNAppDelegate.swift

@@ -220,10 +220,13 @@ final class DuckDuckGoVPNAppDelegate: NSObject, NSApplicationDelegate {
         return controller
     }()
 
+    private let internalUserDecider = DefaultInternalUserDecider(store: UserDefaults.appConfiguration)
+
     @MainActor
     private lazy var tunnelController = NetworkProtectionTunnelController(
         networkExtensionBundleID: tunnelExtensionBundleID,
         networkExtensionController: networkExtensionController,
+        internalUserDecider: internalUserDecider,
         settings: tunnelSettings,
         defaults: userDefaults,
         accessTokenStorage: accessTokenStorage)

DuckDuckGoVPN/VPNPrivacyConfigurationManager.swift

@@ -81,7 +81,8 @@ public final class VPNPrivacyConfigurationManager: PrivacyConfigurationManaging
         return privacyConfig
     }
 
-    public var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(store: InternalUserDeciderStoreMock())
+    public var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(
+        store: UserDefaults.appConfiguration)
 
     @discardableResult
     public func reload(etag: String?, data: Data?) -> PrivacyConfigurationManager.ReloadResult {