Add option to use includeAllNetworks.

In OS-independant parts of the code I called this `strictLeakPrevention`.

My main concern here is that we are just tracking the macOS property. I'm worried that if saving fails we may show the wrong value. Probably the best option would be to have our own vairable that we track before attempting changes and clear only when we are sure we know the current state again. But that can be added before shipping the end-user UI, for now at least the core functionality works.

Bug: https://linear.app/soveng/issue/OBS-234/consider-setting-the-includeallnetworks-network-extension-flag

Author: kevincox

apple/client/OsStatus.swift

@@ -10,16 +10,29 @@ class OsStatus: Encodable {
     var internetAvailable: Bool = false
     var osVpnStatus: NEVPNStatus
     let srcVersion = sourceVersion()
+    var strictLeakPrevention: Bool
     var updaterStatus = UpdaterStatus()
     var debugBundleStatus = DebugBundleStatus()
 
-    init(osVpnStatus: NEVPNStatus) {
+    init(
+        strictLeakPrevention: Bool,
+        osVpnStatus: NEVPNStatus,
+    ) {
         self.osVpnStatus = osVpnStatus
+        self.strictLeakPrevention = strictLeakPrevention
     }
 
-    static func watchable(connection: NEVPNConnection) -> WatchableValue<OsStatus> {
-        let notifications = NotificationCenter.default.notifications(named: .NEVPNStatusDidChange, object: connection)
-        let w = WatchableValue(OsStatus(osVpnStatus: connection.status))
+    static func watchable(
+        manager: NEVPNManager,
+    ) -> WatchableValue<OsStatus> {
+        var lastIncludeAllNetworks = switch manager.protocolConfiguration {
+        case let .some(proto): proto.includeAllNetworks
+        case nil: false // Report safe default.
+        }
+        let w = WatchableValue(OsStatus(
+            strictLeakPrevention: lastIncludeAllNetworks,
+            osVpnStatus: manager.connection.status
+        ))
         Task {
             for await path in NWPathMonitor().stream() {
                 logger.info("NWPathMonitor event: \(path.debugDescription, privacy: .public)")
@@ -29,16 +42,44 @@ class OsStatus: Encodable {
                 }
             }
         }
+
+        let vpnConfigNotifications = NotificationCenter.default.notifications(named: .NEVPNConfigurationChange, object: manager)
         Task {
-            for await _ in notifications {
-                let osVpnStatus = connection.status
+            for await _ in vpnConfigNotifications {
+                let includeAllNetworks: Bool
+                if let proto = manager.protocolConfiguration {
+                    includeAllNetworks = proto.includeAllNetworks
+                } else {
+                    logger.warning("NEVPNManager.protocolConfiguration is nil")
+                    includeAllNetworks = false // Safe default
+                }
+
+                logger.info("NEVPNConfigurationChangeNotification includeAllNetworks \(includeAllNetworks, privacy: .public)")
+
+                if includeAllNetworks == lastIncludeAllNetworks {
+                    continue
+                }
+
+                lastIncludeAllNetworks = includeAllNetworks
+                _ = w.update { value in
+                    value.strictLeakPrevention = includeAllNetworks
+                    value.version = UUID()
+                }
+            }
+        }
+
+        let vpnStatusNotifications = NotificationCenter.default.notifications(named: .NEVPNStatusDidChange, object: manager.connection)
+        Task {
+            for await _ in vpnStatusNotifications {
+                let osVpnStatus = manager.connection.status
                 logger.info("NEVPNStatus event: \(osVpnStatus, privacy: .public)")
                 _ = w.update { value in
                     value.osVpnStatus = osVpnStatus
                     value.version = UUID()
                 }
             }
         }
+
         return w
     }
 }

apple/client/TunnelProvider.swift

@@ -77,6 +77,7 @@ class TunnelProviderInit {
             let proto = NETunnelProviderProtocol()
             proto.providerBundleIdentifier = extensionBundleID()
             proto.serverAddress = "obscura.net"
+            proto.includeAllNetworks = manager.protocolConfiguration?.includeAllNetworks ?? false
             manager.protocolConfiguration = proto
             manager.isEnabled = true
 

apple/client/app_state.swift

@@ -16,7 +16,7 @@ class AppState: ObservableObject {
     ) {
         self.manager = manager
         self.status = initialStatus
-        self.osStatus = OsStatus.watchable(connection: manager.connection)
+        self.osStatus = OsStatus.watchable(manager: manager)
 
         Task { @MainActor in
             var version: UUID = initialStatus.version
@@ -63,6 +63,36 @@ class AppState: ObservableObject {
         }
     }
 
+    func setIncludeAllNetworks(enable: Bool) async throws {
+        guard let proto = self.manager.protocolConfiguration else {
+            throw "NEVPNManager.protocolConfiguration is nil"
+        }
+
+        Self.logger.info("setIncludeAllNetworks \(proto.includeAllNetworks, privacy: .public) → \(enable, privacy: .public)")
+
+        if proto.includeAllNetworks == enable { return }
+
+        proto.includeAllNetworks = enable
+        do {
+            try await self.manager.saveToPreferences()
+            return
+        } catch {
+            Self.logger.error("Failed to save NEVPNManager: \(error.localizedDescription)")
+        }
+
+        do {
+            try await self.manager.loadFromPreferences()
+            return
+        } catch {
+            Self.logger.error("Failed to reload NEVPNManager: \(error.localizedDescription)")
+        }
+
+        proto.includeAllNetworks = false
+        Self.logger.warning("Marking local includeAllNetworks to false as a safe default.")
+
+        throw "Unable to save VPN configuration."
+    }
+
     func enableTunnel(_ tunnelArgs: TunnelArgs) async throws(String) {
         for _ in 1 ..< 3 {
             // Checking the status to decide whether to use `startVPNTunnel` or the `setTunnelArgs` command is not necessary for correct behavior. Handling of the `errorCodeTunnelInactive` error code is sufficient to always do the right thing eventually. However, this does require an app message round-trip to the NE, which can be a little slow at times.

apple/client/command.swift

@@ -10,6 +10,7 @@ enum Command: Codable {
     case stopTunnel
     case debuggingArchive
     case revealItemInDir(path: String)
+    case setStrictLeakPrevention(enable: Bool)
     case registerAsLoginItem
     case unregisterAsLoginItem
     case isRegisteredAsLoginItem
@@ -55,6 +56,13 @@ func handleWebViewCommand(command: Command) async throws(String) -> String {
     case .resetUserDefaults:
         // NOTE: only shown in the Developer View
         appState.resetUserDefaults()
+    case .setStrictLeakPrevention(let enable):
+        do {
+            try await appState.setIncludeAllNetworks(enable: enable)
+        } catch {
+            logger.error("Could not set includeAllNetworks \(error, privacy: .public)")
+            throw errorCodeOther
+        }
     case .jsonFfiCmd(cmd: let jsonCmd, let timeoutMs):
         let attemptTimeout: Duration? = switch timeoutMs {
         case .some(let ms): .milliseconds(ms)

obscura-ui/src/bridge/commands.ts

@@ -73,6 +73,10 @@ export function logout() {
     return jsonFfiCmd('logout');
 }
 
+export async function setStrictLeakPrevention(enable: boolean): Promise<void> {
+    await invoke('setStrictLeakPrevention', { enable });
+}
+
 export function connect(exit: string | null = null) {
     let jsonTunnelArgs = JSON.stringify(({ exit }));
     return invoke('startTunnel', { tunnelArgs: jsonTunnelArgs });

obscura-ui/src/common/appContext.ts

@@ -38,6 +38,7 @@ export interface OsStatus {
     internetAvailable: boolean,
     osVpnStatus: NEVPNStatus,
     srcVersion: string
+    strictLeakPrevention: boolean,
     updaterStatus: UpdaterStatus,
     debugBundleStatus: {
         inProgress?: boolean,

obscura-ui/src/common/utils.ts

@@ -110,6 +110,14 @@ export function percentEncodeQuery(params: Record<string, string>) {
 
 const DEFAULT_ERROR_MSG = "An unexpected error has occurred.";
 
+export function errMsg(error: unknown): string {
+    if (error instanceof Error) {
+        return error.message;
+    }
+    console.warn(fmt`errMsg: error = ${error} is not an instance of Error`);
+    return DEFAULT_ERROR_MSG;
+}
+
 export function normalizeError(error: unknown): Error {
     if (error instanceof Error) {
         return error;

obscura-ui/src/views/DeveloperView.tsx

@@ -1,4 +1,4 @@
-import { Accordion, Autocomplete, Button, Group, JsonInput, Stack, Text, TextInput, Title } from '@mantine/core';
+import { Accordion, Autocomplete, Button, Group, JsonInput, Stack, Switch, Text, TextInput, Title } from '@mantine/core';
 import { useInterval } from '@mantine/hooks';
 import { notifications } from '@mantine/notifications';
 import Cookies from 'js-cookie';
@@ -8,7 +8,7 @@ import * as commands from '../bridge/commands';
 import { Exit } from '../common/api';
 import { AppContext } from '../common/appContext';
 import { localStorageGet, LocalStorageKey } from '../common/localStorage';
-import { IS_WK_WEB_VIEW } from '../common/utils';
+import { errMsg, IS_WK_WEB_VIEW } from '../common/utils';
 import DevSendCommand from '../components/DevSendCommand';
 import DevSetApiUrl from '../components/DevSetApiUrl';
 
@@ -36,6 +36,9 @@ export default function DeveloperViewer() {
 
     const [accordionValues, setAccordionValues] = useState<string[]>([]);
 
+    const [strictLeakLoading, setStrictLeakLoading] = useState(false);
+    const [strictLeakError, setStrictLeakError] = useState<string>();
+
     return <Stack p={20} mb={50}>
         <Title order={3}>Developer View</Title>
         <Title order={4}>Statuses</Title>
@@ -78,6 +81,22 @@ export default function DeveloperViewer() {
           <Autocomplete onChange={setLocalStorageKey} label='local storage key' data={Object.values(LocalStorageKey)} />
           <Button onClick={() => setLocalStorageValue(localStorageGet(localStorageKey as LocalStorageKey))}>Get</Button>
         </Group>
+        <Switch
+          error={strictLeakError}
+          checked={osStatus.strictLeakPrevention}
+          disabled={strictLeakLoading}
+          onChange={async event => {
+            try {
+              setStrictLeakLoading(true);
+              setStrictLeakError(undefined);
+              await commands.setStrictLeakPrevention(event.currentTarget.checked);
+            } catch (err) {
+              setStrictLeakError(errMsg(err));
+            } finally {
+              setStrictLeakLoading(false);
+            }
+          }}
+          label="Enable Strict Leak Prevention" />
         <JsonInput value={localStorageValue ?? 'null'} contentEditable={false} />
     </Stack>;
 }