From cf9cfd002cd27aec26a5cab493b18f0e4abf0758 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Feb 2025 16:19:54 +1100 Subject: [PATCH 1/7] chore: support operating the VPN without the app --- .../Coder Desktop/Coder_DesktopApp.swift | 4 +- .../Coder Desktop/NetworkExtension.swift | 28 ++------ .../Coder Desktop/SystemExtension.swift | 67 +++++++++++++++++-- Coder Desktop/Coder Desktop/VPNService.swift | 48 +++++++------ .../Coder Desktop/XPCInterface.swift | 8 +++ Coder Desktop/VPN/Manager.swift | 2 +- Coder Desktop/VPN/XPCInterface.swift | 9 ++- Coder Desktop/VPNLib/XPC.swift | 2 +- 8 files changed, 113 insertions(+), 55 deletions(-) diff --git a/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift b/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift index b952e98..8b65836 100644 --- a/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift +++ b/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift @@ -47,9 +47,11 @@ class AppDelegate: NSObject, NSApplicationDelegate { } } + // MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)` func applicationShouldTerminate(_: NSApplication) -> NSApplication.TerminateReply { Task { - await vpn.quit() + await vpn.stop() + NSApp.reply(toApplicationShouldTerminate: true) } return .terminateLater } diff --git a/Coder Desktop/Coder Desktop/NetworkExtension.swift b/Coder Desktop/Coder Desktop/NetworkExtension.swift index 28aa78f..39d4ef2 100644 --- a/Coder Desktop/Coder Desktop/NetworkExtension.swift +++ b/Coder Desktop/Coder Desktop/NetworkExtension.swift @@ -24,16 +24,6 @@ enum NetworkExtensionState: Equatable { /// An actor that handles configuring, enabling, and disabling the VPN tunnel via the /// NetworkExtension APIs. extension CoderVPNService { - // Updates the UI if a previous configuration exists - func loadNetworkExtension() async { - do { - try await getTunnelManager() - neState = .disabled - } catch { - neState = .unconfigured - } - } - func configureNetworkExtension(proto: NETunnelProviderProtocol) async { // removing the old tunnels, rather than reconfiguring ensures that configuration changes // are picked up. @@ -74,18 +64,13 @@ extension CoderVPNService { func enableNetworkExtension() async { do { let tm = try await getTunnelManager() - if !tm.isEnabled { - tm.isEnabled = true - try await tm.saveToPreferences() - logger.debug("saved tunnel with enabled=true") - } try tm.connection.startVPNTunnel() } catch { - logger.error("enable network extension: \(error)") + logger.error("start tunnel: \(error)") neState = .failed(error.localizedDescription) return } - logger.debug("enabled and started tunnel") + logger.debug("started tunnel") neState = .enabled } @@ -93,20 +78,17 @@ extension CoderVPNService { do { let tm = try await getTunnelManager() tm.connection.stopVPNTunnel() - tm.isEnabled = false - - try await tm.saveToPreferences() } catch { - logger.error("disable network extension: \(error)") + logger.error("stop tunnel: \(error)") neState = .failed(error.localizedDescription) return } - logger.debug("saved tunnel with enabled=false") + logger.debug("stopped tunnel") neState = .disabled } @discardableResult - private func getTunnelManager() async throws(VPNServiceError) -> NETunnelProviderManager { + func getTunnelManager() async throws(VPNServiceError) -> NETunnelProviderManager { var tunnels: [NETunnelProviderManager] = [] do { tunnels = try await NETunnelProviderManager.loadAllFromPreferences() diff --git a/Coder Desktop/Coder Desktop/SystemExtension.swift b/Coder Desktop/Coder Desktop/SystemExtension.swift index 934db09..dd4d53d 100644 --- a/Coder Desktop/Coder Desktop/SystemExtension.swift +++ b/Coder Desktop/Coder Desktop/SystemExtension.swift @@ -29,7 +29,16 @@ protocol SystemExtensionAsyncRecorder: Sendable { extension CoderVPNService: SystemExtensionAsyncRecorder { func recordSystemExtensionState(_ state: SystemExtensionState) async { sysExtnState = state + if state == .uninstalled { + installSystemExtension() + } if state == .installed { + do { + try await getTunnelManager() + neState = .disabled + } catch { + neState = .unconfigured + } // system extension was successfully installed, so we don't need the delegate any more systemExtnDelegate = nil } @@ -64,7 +73,21 @@ extension CoderVPNService: SystemExtensionAsyncRecorder { return extensionBundle } - func installSystemExtension() { + func checkSystemExtensionStatus() { + logger.info("checking SystemExtension status") + guard let bundleID = extensionBundle.bundleIdentifier else { + logger.error("Bundle has no identifier") + return + } + let request = OSSystemExtensionRequest.propertiesRequest(forExtensionWithIdentifier: bundleID, queue: .main) + let delegate = SystemExtensionDelegate(asyncDelegate: self) + request.delegate = delegate + systemExtnDelegate = delegate + OSSystemExtensionManager.shared.submitRequest(request) + logger.info("submitted SystemExtension properties request with bundleID: \(bundleID)") + } + + private func installSystemExtension() { logger.info("activating SystemExtension") guard let bundleID = extensionBundle.bundleIdentifier else { logger.error("Bundle has no identifier") @@ -74,11 +97,9 @@ extension CoderVPNService: SystemExtensionAsyncRecorder { forExtensionWithIdentifier: bundleID, queue: .main ) - let delegate = SystemExtensionDelegate(asyncDelegate: self) - systemExtnDelegate = delegate - request.delegate = delegate + request.delegate = systemExtnDelegate OSSystemExtensionManager.shared.submitRequest(request) - logger.info("submitted SystemExtension request with bundleID: \(bundleID)") + logger.info("submitted SystemExtension activate request with bundleID: \(bundleID)") } } @@ -88,6 +109,8 @@ class SystemExtensionDelegate: NSObject, OSSystemExtensionRequestDelegate { private var logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "vpn-installer") + // TODO: Refactor this to use a continuation, so the result of a request can be + // 'await'd for the determined state private var asyncDelegate: AsyncDelegate init(asyncDelegate: AsyncDelegate) { @@ -138,4 +161,38 @@ class SystemExtensionDelegate: logger.info("Replacing \(request.identifier) v\(existing.bundleShortVersion) with v\(`extension`.bundleShortVersion)") return .replace } + + public func request( + _: OSSystemExtensionRequest, + foundProperties properties: [OSSystemExtensionProperties] + ) { + // In debug builds we always replace the SE to test + // changes made without bumping the version + #if DEBUG + Task { [asyncDelegate] in + await asyncDelegate.recordSystemExtensionState(.uninstalled) + } + return + #else + let version = Bundle.main.object(forInfoDictionaryKey: "CFBundleVersion") as? String + let shortVersion = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String + + let versionMatches = properties.contains { sysex in + sysex.isEnabled + && sysex.bundleVersion == version + && sysex.bundleShortVersion == shortVersion + } + if versionMatches { + Task { [asyncDelegate] in + await asyncDelegate.recordSystemExtensionState(.installed) + } + return + } + + // Either uninstalled or needs replacing + Task { [asyncDelegate] in + await asyncDelegate.recordSystemExtensionState(.uninstalled) + } + #endif + } } diff --git a/Coder Desktop/Coder Desktop/VPNService.swift b/Coder Desktop/Coder Desktop/VPNService.swift index 60e7ace..16b6478 100644 --- a/Coder Desktop/Coder Desktop/VPNService.swift +++ b/Coder Desktop/Coder Desktop/VPNService.swift @@ -41,7 +41,6 @@ enum VPNServiceError: Error, Equatable { final class CoderVPNService: NSObject, VPNService { var logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "vpn") lazy var xpc: VPNXPCInterface = .init(vpn: self) - var terminating = false var workspaces: [UUID: String] = [:] @Published var tunnelState: VPNServiceState = .disabled @@ -66,10 +65,7 @@ final class CoderVPNService: NSObject, VPNService { override init() { super.init() - installSystemExtension() - Task { - await loadNetworkExtension() - } + checkSystemExtensionStatus() NotificationCenter.default.addObserver( self, selector: #selector(vpnDidUpdate(_:)), @@ -82,6 +78,11 @@ final class CoderVPNService: NSObject, VPNService { NotificationCenter.default.removeObserver(self) } + func clearPeers() { + agents = [:] + workspaces = [:] + } + func start() async { switch tunnelState { case .disabled, .failed: @@ -102,19 +103,6 @@ final class CoderVPNService: NSObject, VPNService { logger.info("network extension stopped") } - // Instructs the service to stop the VPN and then quit once the stop event - // is read over XPC. - // MUST only be called from `NSApplicationDelegate.applicationShouldTerminate` - // MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)` - func quit() async { - guard tunnelState == .connected else { - NSApp.reply(toApplicationShouldTerminate: true) - return - } - terminating = true - await stop() - } - func configureTunnelProviderProtocol(proto: NETunnelProviderProtocol?) { Task { if let proto { @@ -145,6 +133,22 @@ final class CoderVPNService: NSObject, VPNService { } } + func onExtensionPeerState(_ data: Data?) { + logger.info("network extension peer state") + guard let data else { + logger.error("could not retrieve peer state from network extension") + return + } + do { + let msg = try Vpn_PeerUpdate(serializedBytes: data) + debugPrint(msg) + clearPeers() + applyPeerUpdate(with: msg) + } catch { + logger.error("failed to decode peer update \(error)") + } + } + func applyPeerUpdate(with update: Vpn_PeerUpdate) { // Delete agents update.deletedAgents @@ -204,9 +208,6 @@ extension CoderVPNService { } switch connection.status { case .disconnected: - if terminating { - NSApp.reply(toApplicationShouldTerminate: true) - } connection.fetchLastDisconnectError { err in self.tunnelState = if let err { .failed(.internalError(err.localizedDescription)) @@ -217,6 +218,11 @@ extension CoderVPNService { case .connecting: tunnelState = .connecting case .connected: + // If we moved from disabled to connected, then the NE was already + // running, and we need to request the current peer state + if self.tunnelState == .disabled { + xpc.getPeerState() + } tunnelState = .connected case .reasserting: tunnelState = .connecting diff --git a/Coder Desktop/Coder Desktop/XPCInterface.swift b/Coder Desktop/Coder Desktop/XPCInterface.swift index 4bdc2b2..e8730a6 100644 --- a/Coder Desktop/Coder Desktop/XPCInterface.swift +++ b/Coder Desktop/Coder Desktop/XPCInterface.swift @@ -45,6 +45,14 @@ import VPNLib } } + func getPeerState() { + xpc.getPeerState { data in + Task { @MainActor in + self.svc.onExtensionPeerState(data) + } + } + } + func onPeerUpdate(_ data: Data) { Task { @MainActor in svc.onExtensionPeerUpdate(data) diff --git a/Coder Desktop/VPN/Manager.swift b/Coder Desktop/VPN/Manager.swift index ee2adc5..c938818 100644 --- a/Coder Desktop/VPN/Manager.swift +++ b/Coder Desktop/VPN/Manager.swift @@ -194,7 +194,7 @@ actor Manager { // Retrieves the current state of all peers, // as required when starting the app whilst the network extension is already running - func getPeerInfo() async throws(ManagerError) -> Vpn_PeerUpdate { + func getPeerState() async throws(ManagerError) -> Vpn_PeerUpdate { logger.info("sending peer state request") let resp: Vpn_TunnelMessage do { diff --git a/Coder Desktop/VPN/XPCInterface.swift b/Coder Desktop/VPN/XPCInterface.swift index 6ecb119..d83f7d7 100644 --- a/Coder Desktop/VPN/XPCInterface.swift +++ b/Coder Desktop/VPN/XPCInterface.swift @@ -20,9 +20,12 @@ import VPNLib } } - func getPeerInfo(with reply: @escaping () -> Void) { - // TODO: Retrieve from Manager - reply() + func getPeerState(with reply: @escaping (Data?) -> Void) { + let reply = CallbackWrapper(reply) + Task { + let data = try? await manager?.getPeerState().serializedData() + reply(data) + } } func ping(with reply: @escaping () -> Void) { diff --git a/Coder Desktop/VPNLib/XPC.swift b/Coder Desktop/VPNLib/XPC.swift index ffbf6d8..eda8ab0 100644 --- a/Coder Desktop/VPNLib/XPC.swift +++ b/Coder Desktop/VPNLib/XPC.swift @@ -2,7 +2,7 @@ import Foundation @preconcurrency @objc public protocol VPNXPCProtocol { - func getPeerInfo(with reply: @escaping () -> Void) + func getPeerState(with reply: @escaping (Data?) -> Void) func ping(with reply: @escaping () -> Void) } From 951fc1a1abd1a9c8b1bdce6a6cb21e665d16a38a Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Feb 2025 16:20:53 +1100 Subject: [PATCH 2/7] fmt --- Coder Desktop/Coder Desktop/SystemExtension.swift | 14 +++++++------- Coder Desktop/Coder Desktop/VPNService.swift | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Coder Desktop/Coder Desktop/SystemExtension.swift b/Coder Desktop/Coder Desktop/SystemExtension.swift index dd4d53d..5f9b358 100644 --- a/Coder Desktop/Coder Desktop/SystemExtension.swift +++ b/Coder Desktop/Coder Desktop/SystemExtension.swift @@ -168,12 +168,12 @@ class SystemExtensionDelegate: ) { // In debug builds we always replace the SE to test // changes made without bumping the version - #if DEBUG - Task { [asyncDelegate] in - await asyncDelegate.recordSystemExtensionState(.uninstalled) - } - return - #else + #if DEBUG + Task { [asyncDelegate] in + await asyncDelegate.recordSystemExtensionState(.uninstalled) + } + return + #else let version = Bundle.main.object(forInfoDictionaryKey: "CFBundleVersion") as? String let shortVersion = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String @@ -193,6 +193,6 @@ class SystemExtensionDelegate: Task { [asyncDelegate] in await asyncDelegate.recordSystemExtensionState(.uninstalled) } - #endif + #endif } } diff --git a/Coder Desktop/Coder Desktop/VPNService.swift b/Coder Desktop/Coder Desktop/VPNService.swift index 16b6478..d5794a8 100644 --- a/Coder Desktop/Coder Desktop/VPNService.swift +++ b/Coder Desktop/Coder Desktop/VPNService.swift @@ -220,7 +220,7 @@ extension CoderVPNService { case .connected: // If we moved from disabled to connected, then the NE was already // running, and we need to request the current peer state - if self.tunnelState == .disabled { + if tunnelState == .disabled { xpc.getPeerState() } tunnelState = .connected From f617f6f787c653d99822376ce68796c8bed6e199 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Feb 2025 16:45:41 +1100 Subject: [PATCH 3/7] rename misleading function --- Coder Desktop/Coder Desktop/SystemExtension.swift | 2 +- Coder Desktop/Coder Desktop/VPNService.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Coder Desktop/Coder Desktop/SystemExtension.swift b/Coder Desktop/Coder Desktop/SystemExtension.swift index 5f9b358..d085c85 100644 --- a/Coder Desktop/Coder Desktop/SystemExtension.swift +++ b/Coder Desktop/Coder Desktop/SystemExtension.swift @@ -73,7 +73,7 @@ extension CoderVPNService: SystemExtensionAsyncRecorder { return extensionBundle } - func checkSystemExtensionStatus() { + func attemptSystemExtensionInstall() { logger.info("checking SystemExtension status") guard let bundleID = extensionBundle.bundleIdentifier else { logger.error("Bundle has no identifier") diff --git a/Coder Desktop/Coder Desktop/VPNService.swift b/Coder Desktop/Coder Desktop/VPNService.swift index d5794a8..16b64bc 100644 --- a/Coder Desktop/Coder Desktop/VPNService.swift +++ b/Coder Desktop/Coder Desktop/VPNService.swift @@ -65,7 +65,7 @@ final class CoderVPNService: NSObject, VPNService { override init() { super.init() - checkSystemExtensionStatus() + attemptSystemExtensionInstall() NotificationCenter.default.addObserver( self, selector: #selector(vpnDidUpdate(_:)), From b7c9f582a6462a5e9500835a431add0b394e8933 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Feb 2025 13:57:28 +1100 Subject: [PATCH 4/7] add stop vpn on quit setting --- Coder Desktop/Coder Desktop/Coder_DesktopApp.swift | 2 ++ Coder Desktop/Coder Desktop/State.swift | 3 +++ Coder Desktop/Coder Desktop/Views/Settings/GeneralTab.swift | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift b/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift index 8b65836..d0c985a 100644 --- a/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift +++ b/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift @@ -48,7 +48,9 @@ class AppDelegate: NSObject, NSApplicationDelegate { } // MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)` + // or return `.terminateNow` func applicationShouldTerminate(_: NSApplication) -> NSApplication.TerminateReply { + if !settings.stopVPNOnQuit { return .terminateNow } Task { await vpn.stop() NSApp.reply(toApplicationShouldTerminate: true) diff --git a/Coder Desktop/Coder Desktop/State.swift b/Coder Desktop/Coder Desktop/State.swift index cb51450..c98a09f 100644 --- a/Coder Desktop/Coder Desktop/State.swift +++ b/Coder Desktop/Coder Desktop/State.swift @@ -104,6 +104,8 @@ class Settings: ObservableObject { } } + @AppStorage(Keys.stopVPNOnQuit) var stopVPNOnQuit = true + init(store: UserDefaults = .standard) { self.store = store _literalHeaders = Published( @@ -116,6 +118,7 @@ class Settings: ObservableObject { enum Keys { static let useLiteralHeaders = "UseLiteralHeaders" static let literalHeaders = "LiteralHeaders" + static let stopVPNOnQuit = "StopVPNOnQuit" } } diff --git a/Coder Desktop/Coder Desktop/Views/Settings/GeneralTab.swift b/Coder Desktop/Coder Desktop/Views/Settings/GeneralTab.swift index 0c1bb9e..1dc1cf9 100644 --- a/Coder Desktop/Coder Desktop/Views/Settings/GeneralTab.swift +++ b/Coder Desktop/Coder Desktop/Views/Settings/GeneralTab.swift @@ -2,11 +2,17 @@ import LaunchAtLogin import SwiftUI struct GeneralTab: View { + @EnvironmentObject var settings: Settings var body: some View { Form { Section { LaunchAtLogin.Toggle("Launch at Login") } + Section { + Toggle(isOn: $settings.stopVPNOnQuit) { + Text("Stop VPN on Quit") + } + } }.formStyle(.grouped) } } From 27ab4a730b78cfdc28e31cd55584225c189b0751 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 4 Feb 2025 19:19:01 +1100 Subject: [PATCH 5/7] review --- .../Coder Desktop/Coder_DesktopApp.swift | 2 +- .../Coder Desktop/NetworkExtension.swift | 15 ++++- .../Coder Desktop/SystemExtension.swift | 67 ++----------------- Coder Desktop/Coder Desktop/VPNService.swift | 23 ++++--- 4 files changed, 31 insertions(+), 76 deletions(-) diff --git a/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift b/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift index d0c985a..4e7cebc 100644 --- a/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift +++ b/Coder Desktop/Coder Desktop/Coder_DesktopApp.swift @@ -47,7 +47,7 @@ class AppDelegate: NSObject, NSApplicationDelegate { } } - // MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)` + // This function MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)` // or return `.terminateNow` func applicationShouldTerminate(_: NSApplication) -> NSApplication.TerminateReply { if !settings.stopVPNOnQuit { return .terminateNow } diff --git a/Coder Desktop/Coder Desktop/NetworkExtension.swift b/Coder Desktop/Coder Desktop/NetworkExtension.swift index 39d4ef2..16d18bb 100644 --- a/Coder Desktop/Coder Desktop/NetworkExtension.swift +++ b/Coder Desktop/Coder Desktop/NetworkExtension.swift @@ -24,6 +24,15 @@ enum NetworkExtensionState: Equatable { /// An actor that handles configuring, enabling, and disabling the VPN tunnel via the /// NetworkExtension APIs. extension CoderVPNService { + func hasNetworkExtensionConfig() async -> Bool { + do { + _ = try await getTunnelManager() + return true + } catch { + return false + } + } + func configureNetworkExtension(proto: NETunnelProviderProtocol) async { // removing the old tunnels, rather than reconfiguring ensures that configuration changes // are picked up. @@ -61,7 +70,7 @@ extension CoderVPNService { } } - func enableNetworkExtension() async { + func startTunnel() async { do { let tm = try await getTunnelManager() try tm.connection.startVPNTunnel() @@ -74,7 +83,7 @@ extension CoderVPNService { neState = .enabled } - func disableNetworkExtension() async { + func stopTunnel() async { do { let tm = try await getTunnelManager() tm.connection.stopVPNTunnel() @@ -88,7 +97,7 @@ extension CoderVPNService { } @discardableResult - func getTunnelManager() async throws(VPNServiceError) -> NETunnelProviderManager { + private func getTunnelManager() async throws(VPNServiceError) -> NETunnelProviderManager { var tunnels: [NETunnelProviderManager] = [] do { tunnels = try await NETunnelProviderManager.loadAllFromPreferences() diff --git a/Coder Desktop/Coder Desktop/SystemExtension.swift b/Coder Desktop/Coder Desktop/SystemExtension.swift index d085c85..934db09 100644 --- a/Coder Desktop/Coder Desktop/SystemExtension.swift +++ b/Coder Desktop/Coder Desktop/SystemExtension.swift @@ -29,16 +29,7 @@ protocol SystemExtensionAsyncRecorder: Sendable { extension CoderVPNService: SystemExtensionAsyncRecorder { func recordSystemExtensionState(_ state: SystemExtensionState) async { sysExtnState = state - if state == .uninstalled { - installSystemExtension() - } if state == .installed { - do { - try await getTunnelManager() - neState = .disabled - } catch { - neState = .unconfigured - } // system extension was successfully installed, so we don't need the delegate any more systemExtnDelegate = nil } @@ -73,21 +64,7 @@ extension CoderVPNService: SystemExtensionAsyncRecorder { return extensionBundle } - func attemptSystemExtensionInstall() { - logger.info("checking SystemExtension status") - guard let bundleID = extensionBundle.bundleIdentifier else { - logger.error("Bundle has no identifier") - return - } - let request = OSSystemExtensionRequest.propertiesRequest(forExtensionWithIdentifier: bundleID, queue: .main) - let delegate = SystemExtensionDelegate(asyncDelegate: self) - request.delegate = delegate - systemExtnDelegate = delegate - OSSystemExtensionManager.shared.submitRequest(request) - logger.info("submitted SystemExtension properties request with bundleID: \(bundleID)") - } - - private func installSystemExtension() { + func installSystemExtension() { logger.info("activating SystemExtension") guard let bundleID = extensionBundle.bundleIdentifier else { logger.error("Bundle has no identifier") @@ -97,9 +74,11 @@ extension CoderVPNService: SystemExtensionAsyncRecorder { forExtensionWithIdentifier: bundleID, queue: .main ) - request.delegate = systemExtnDelegate + let delegate = SystemExtensionDelegate(asyncDelegate: self) + systemExtnDelegate = delegate + request.delegate = delegate OSSystemExtensionManager.shared.submitRequest(request) - logger.info("submitted SystemExtension activate request with bundleID: \(bundleID)") + logger.info("submitted SystemExtension request with bundleID: \(bundleID)") } } @@ -109,8 +88,6 @@ class SystemExtensionDelegate: NSObject, OSSystemExtensionRequestDelegate { private var logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "vpn-installer") - // TODO: Refactor this to use a continuation, so the result of a request can be - // 'await'd for the determined state private var asyncDelegate: AsyncDelegate init(asyncDelegate: AsyncDelegate) { @@ -161,38 +138,4 @@ class SystemExtensionDelegate: logger.info("Replacing \(request.identifier) v\(existing.bundleShortVersion) with v\(`extension`.bundleShortVersion)") return .replace } - - public func request( - _: OSSystemExtensionRequest, - foundProperties properties: [OSSystemExtensionProperties] - ) { - // In debug builds we always replace the SE to test - // changes made without bumping the version - #if DEBUG - Task { [asyncDelegate] in - await asyncDelegate.recordSystemExtensionState(.uninstalled) - } - return - #else - let version = Bundle.main.object(forInfoDictionaryKey: "CFBundleVersion") as? String - let shortVersion = Bundle.main.object(forInfoDictionaryKey: "CFBundleShortVersionString") as? String - - let versionMatches = properties.contains { sysex in - sysex.isEnabled - && sysex.bundleVersion == version - && sysex.bundleShortVersion == shortVersion - } - if versionMatches { - Task { [asyncDelegate] in - await asyncDelegate.recordSystemExtensionState(.installed) - } - return - } - - // Either uninstalled or needs replacing - Task { [asyncDelegate] in - await asyncDelegate.recordSystemExtensionState(.uninstalled) - } - #endif - } } diff --git a/Coder Desktop/Coder Desktop/VPNService.swift b/Coder Desktop/Coder Desktop/VPNService.swift index 16b64bc..6b6657f 100644 --- a/Coder Desktop/Coder Desktop/VPNService.swift +++ b/Coder Desktop/Coder Desktop/VPNService.swift @@ -65,7 +65,15 @@ final class CoderVPNService: NSObject, VPNService { override init() { super.init() - attemptSystemExtensionInstall() + installSystemExtension() + Task { + neState = if await hasNetworkExtensionConfig() { + .disabled + } else { + .unconfigured + } + } + xpc.getPeerState() NotificationCenter.default.addObserver( self, selector: #selector(vpnDidUpdate(_:)), @@ -91,7 +99,7 @@ final class CoderVPNService: NSObject, VPNService { return } - await enableNetworkExtension() + await startTunnel() // this ping is somewhat load bearing since it causes xpc to init xpc.ping() logger.debug("network extension enabled") @@ -99,7 +107,7 @@ final class CoderVPNService: NSObject, VPNService { func stop() async { guard tunnelState == .connected else { return } - await disableNetworkExtension() + await stopTunnel() logger.info("network extension stopped") } @@ -134,11 +142,11 @@ final class CoderVPNService: NSObject, VPNService { } func onExtensionPeerState(_ data: Data?) { - logger.info("network extension peer state") guard let data else { - logger.error("could not retrieve peer state from network extension") + logger.error("could not retrieve peer state from network extension, it may not be running") return } + logger.info("received network extension peer state") do { let msg = try Vpn_PeerUpdate(serializedBytes: data) debugPrint(msg) @@ -218,11 +226,6 @@ extension CoderVPNService { case .connecting: tunnelState = .connecting case .connected: - // If we moved from disabled to connected, then the NE was already - // running, and we need to request the current peer state - if tunnelState == .disabled { - xpc.getPeerState() - } tunnelState = .connected case .reasserting: tunnelState = .connecting From c0a46aa01061cf16afe3c6f1851f1e4f7b88ed28 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 5 Feb 2025 19:04:14 +1100 Subject: [PATCH 6/7] reconnect xpc on each request --- Coder Desktop/Coder Desktop/VPNService.swift | 3 ++- Coder Desktop/Coder Desktop/XPCInterface.swift | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Coder Desktop/Coder Desktop/VPNService.swift b/Coder Desktop/Coder Desktop/VPNService.swift index 6b6657f..657d994 100644 --- a/Coder Desktop/Coder Desktop/VPNService.swift +++ b/Coder Desktop/Coder Desktop/VPNService.swift @@ -73,6 +73,7 @@ final class CoderVPNService: NSObject, VPNService { .unconfigured } } + xpc.connect() xpc.getPeerState() NotificationCenter.default.addObserver( self, @@ -100,7 +101,7 @@ final class CoderVPNService: NSObject, VPNService { } await startTunnel() - // this ping is somewhat load bearing since it causes xpc to init + xpc.connect() xpc.ping() logger.debug("network extension enabled") } diff --git a/Coder Desktop/Coder Desktop/XPCInterface.swift b/Coder Desktop/Coder Desktop/XPCInterface.swift index e8730a6..a3578d5 100644 --- a/Coder Desktop/Coder Desktop/XPCInterface.swift +++ b/Coder Desktop/Coder Desktop/XPCInterface.swift @@ -6,11 +6,14 @@ import VPNLib @objc final class VPNXPCInterface: NSObject, VPNXPCClientCallbackProtocol, @unchecked Sendable { private var svc: CoderVPNService private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "VPNXPCInterface") - private let xpc: VPNXPCProtocol + private var xpc: VPNXPCProtocol? = nil init(vpn: CoderVPNService) { svc = vpn + super.init() + } + func connect() { let networkExtDict = Bundle.main.object(forInfoDictionaryKey: "NetworkExtension") as? [String: Any] let machServiceName = networkExtDict?["NEMachServiceName"] as? String let xpcConn = NSXPCConnection(machServiceName: machServiceName!) @@ -21,24 +24,24 @@ import VPNLib } xpc = proxy - super.init() - xpcConn.exportedObject = self xpcConn.invalidationHandler = { [logger] in Task { @MainActor in logger.error("XPC connection invalidated.") + self.xpc = nil } } xpcConn.interruptionHandler = { [logger] in Task { @MainActor in logger.error("XPC connection interrupted.") + self.xpc = nil } } xpcConn.resume() } func ping() { - xpc.ping { + xpc?.ping { Task { @MainActor in self.logger.info("Connected to NE over XPC") } @@ -46,7 +49,7 @@ import VPNLib } func getPeerState() { - xpc.getPeerState { data in + xpc?.getPeerState { data in Task { @MainActor in self.svc.onExtensionPeerState(data) } From 95bf6b5aee35764980cc6686d7d71a454f7f27e0 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Thu, 6 Feb 2025 12:55:42 +1100 Subject: [PATCH 7/7] avoid double connecting --- Coder Desktop/Coder Desktop/XPCInterface.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Coder Desktop/Coder Desktop/XPCInterface.swift b/Coder Desktop/Coder Desktop/XPCInterface.swift index a3578d5..74baab5 100644 --- a/Coder Desktop/Coder Desktop/XPCInterface.swift +++ b/Coder Desktop/Coder Desktop/XPCInterface.swift @@ -6,7 +6,7 @@ import VPNLib @objc final class VPNXPCInterface: NSObject, VPNXPCClientCallbackProtocol, @unchecked Sendable { private var svc: CoderVPNService private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "VPNXPCInterface") - private var xpc: VPNXPCProtocol? = nil + private var xpc: VPNXPCProtocol? init(vpn: CoderVPNService) { svc = vpn @@ -14,6 +14,9 @@ import VPNLib } func connect() { + guard xpc == nil else { + return + } let networkExtDict = Bundle.main.object(forInfoDictionaryKey: "NetworkExtension") as? [String: Any] let machServiceName = networkExtDict?["NEMachServiceName"] as? String let xpcConn = NSXPCConnection(machServiceName: machServiceName!)