Merge branch 'main' into colin/xpc

ethanndickson · ethanndickson · commit deb773affd69 · 2025-01-30T19:09:21.000+11:00
diff --git a/.gitignore b/.gitignore
@@ -290,4 +290,8 @@ xcuserdata
 /*.gcno
 **/xcshareddata/WorkspaceSettings.xcsettings
 
+### VSCode & Sweetpad ###
+.vscode/**
+buildServer.json
+
 # End of https://www.toptal.com/developers/gitignore/api/xcode,jetbrains,macos,direnv,swift,swiftpm,objective-c
diff --git a/Coder Desktop/.swiftformat b/Coder Desktop/.swiftformat
@@ -0,0 +1,3 @@
+--selfrequired log,info,error,debug,critical,fault
+--exclude **.pb.swift
+--condassignment always
diff --git a/Coder Desktop/Coder Desktop/State.swift b/Coder Desktop/Coder Desktop/State.swift
@@ -41,7 +41,9 @@ class SecureSession: ObservableObject, Session {
         if !hasSession { return nil }
         let proto = NETunnelProviderProtocol()
         proto.providerBundleIdentifier = "\(appId).VPN"
-        proto.passwordReference = keychain[attributes: Keys.sessionToken]?.persistentRef
+        // HACK: We can't write to the system keychain, and the user keychain
+        // isn't accessible, so we'll use providerConfiguration, which is over XPC.
+        proto.providerConfiguration = ["token": sessionToken!]
         proto.serverAddress = baseAccessURL!.absoluteString
         return proto
     }
diff --git a/Coder Desktop/VPN/Manager.swift b/Coder Desktop/VPN/Manager.swift
@@ -80,11 +80,11 @@ actor Manager {
                 case let .message(msg):
                     handleMessage(msg)
                 case let .RPC(rpc):
-                    handleRPC(rpc)
+                    await handleRPC(rpc)
                 }
             }
         } catch {
-            logger.error("tunnel read loop failed: \(error)")
+            logger.error("tunnel read loop failed: \(error.localizedDescription, privacy: .public)")
             try await tunnelHandle.close()
             if let conn = globalXPCListenerDelegate.getActiveConnection() {
                 conn.onError(error as NSError)
@@ -120,15 +120,28 @@ actor Manager {
         }
     }
 
-    func handleRPC(_ rpc: RPCRequest<Vpn_ManagerMessage, Vpn_TunnelMessage>) {
+    func handleRPC(_ rpc: RPCRequest<Vpn_ManagerMessage, Vpn_TunnelMessage>) async {
         guard let msgType = rpc.msg.msg else {
             logger.critical("received rpc with no type")
             return
         }
         switch msgType {
         case let .networkSettings(ns):
-            let neSettings = convertNetworkSettingsRequest(ns)
-            ptp.setTunnelNetworkSettings(neSettings)
+            do {
+                try await ptp.applyTunnelNetworkSettings(ns)
+                try? await rpc.sendReply(.with { resp in
+                    resp.networkSettings = .with { settings in
+                        settings.success = true
+                    }
+                })
+            } catch {
+                try? await rpc.sendReply(.with { resp in
+                    resp.networkSettings = .with { settings in
+                        settings.success = false
+                        settings.errorMessage = error.localizedDescription
+                    }
+                })
+            }
         case .log, .peerUpdate, .start, .stop:
             logger.critical("received unexpected rpc: `\(String(describing: msgType))`")
         }
@@ -262,5 +275,5 @@ func writeVpnLog(_ log: Vpn_Log) {
         category: log.loggerNames.joined(separator: ".")
     )
     let fields = log.fields.map { "\($0.name): \($0.value)" }.joined(separator: ", ")
-    logger.log(level: level, "\(log.message): \(fields)")
+    logger.log(level: level, "\(log.message, privacy: .public): \(fields, privacy: .public)")
 }
diff --git a/Coder Desktop/VPN/PacketTunnelProvider.swift b/Coder Desktop/VPN/PacketTunnelProvider.swift
@@ -9,6 +9,8 @@ let CTLIOCGINFO: UInt = 0xC064_4E03
 class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
     private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "provider")
     private var manager: Manager?
+    // a `tunnelRemoteAddress` is required, but not currently used.
+    private var currentSettings: NEPacketTunnelNetworkSettings = .init(tunnelRemoteAddress: "127.0.0.1")
 
     var tunnelFileDescriptor: Int32? {
         var ctlInfo = ctl_info()
@@ -48,25 +50,37 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
         logger.info("startTunnel called")
         guard manager == nil else {
             logger.error("startTunnel called with non-nil Manager")
-            completionHandler(nil)
+            completionHandler(PTPError.alreadyRunning)
             return
         }
+        guard let proto = protocolConfiguration as? NETunnelProviderProtocol,
+              let baseAccessURL = proto.serverAddress
+        else {
+            logger.error("startTunnel called with nil protocolConfiguration")
+            completionHandler(PTPError.missingConfiguration)
+            return
+        }
+        // HACK: We can't write to the system keychain, and the NE can't read the user keychain.
+        guard let token = proto.providerConfiguration?["token"] as? String else {
+            logger.error("startTunnel called with nil token")
+            completionHandler(PTPError.missingToken)
+            return
+        }
+        logger.debug("retrieved token & access URL")
         let completionHandler = CallbackWrapper(completionHandler)
         Task {
-            // TODO: Retrieve access URL & Token via Keychain
             do throws(ManagerError) {
-                logger.info("creating manager")
+                logger.debug("creating manager")
                 manager = try await Manager(
                     with: self,
                     cfg: .init(
-                        apiToken: "qGg1rDGWzL-a814TWDGcTDOs4AX7laDEI",
-                        serverUrl: .init(string: "https://dev.coder.com")!
+                        apiToken: token, serverUrl: .init(string: baseAccessURL)!
                     )
                 )
                 globalXPCListenerDelegate.vpnXPCInterface.setManager(manager)
-                logger.debug("calling manager.startVPN")
-                // try await manager!.startVPN()
-                logger.debug("vpn started")
+                logger.debug("starting vpn")
+                try await manager!.startVPN()
+                logger.info("vpn started")
                 if let conn = globalXPCListenerDelegate.getActiveConnection() {
                     conn.onStart()
                 } else {
@@ -75,6 +89,11 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
                 completionHandler(nil)
             } catch {
                 logger.error("error starting manager: \(error.description, privacy: .public)")
+                if let conn = globalXPCListenerDelegate.getActiveConnection() {
+                    conn.onError(error as NSError)
+                } else {
+                    logger.info("no active connection")
+                }
                 completionHandler(error as NSError)
             }
         }
@@ -84,30 +103,28 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
         with _: NEProviderStopReason, completionHandler: @escaping () -> Void
     ) {
         logger.debug("stopTunnel called")
-        guard manager != nil else {
+        guard let manager else {
             logger.error("stopTunnel called with nil Manager")
             completionHandler()
             return
         }
 
-        if let conn = globalXPCListenerDelegate.getActiveConnection() {
-            conn.onStop()
-        } else {
-            logger.info("no active connection")
-        }
-
-        let managerCopy = manager
-        Task {
+        let completionHandler = CompletionWrapper(completionHandler)
+        Task { [manager] in
             do throws(ManagerError) {
-                try await managerCopy?.stopVPN()
+                try await manager.stopVPN()
             } catch {
                 logger.error("error stopping manager: \(error.description, privacy: .public)")
             }
+            if let conn = globalXPCListenerDelegate.getActiveConnection() {
+                conn.onStop()
+            } else {
+                logger.info("no active connection")
+            }
+            globalXPCListenerDelegate.vpnXPCInterface.setManager(nil)
+            completionHandler()
         }
-
-        manager = nil
-        globalXPCListenerDelegate.vpnXPCInterface.setManager(nil)
-        completionHandler()
+        self.manager = nil
     }
 
     override func handleAppMessage(_ messageData: Data, completionHandler: ((Data?) -> Void)?) {
@@ -127,4 +144,33 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
         // Add code here to wake up.
         logger.debug("wake called")
     }
+
+    // Wrapper around `setTunnelNetworkSettings` that supports merging updates
+    func applyTunnelNetworkSettings(_ diff: Vpn_NetworkSettingsRequest) async throws {
+        logger.debug("applying settings diff: \(diff.debugDescription, privacy: .public)")
+
+        if diff.hasDnsSettings {
+            currentSettings.dnsSettings = convertDnsSettings(diff.dnsSettings)
+        }
+
+        if diff.mtu != 0 {
+            currentSettings.mtu = NSNumber(value: diff.mtu)
+        }
+
+        if diff.hasIpv4Settings {
+            currentSettings.ipv4Settings = convertIPv4Settings(diff.ipv4Settings)
+        }
+        if diff.hasIpv6Settings {
+            currentSettings.ipv6Settings = convertIPv6Settings(diff.ipv6Settings)
+        }
+
+        logger.info("applying settings: \(self.currentSettings.debugDescription, privacy: .public)")
+        try await setTunnelNetworkSettings(currentSettings)
+    }
+}
+
+enum PTPError: Error {
+    case alreadyRunning
+    case missingConfiguration
+    case missingToken
 }
diff --git a/Coder Desktop/VPNLib/Convert.swift b/Coder Desktop/VPNLib/Convert.swift
@@ -1,60 +1,61 @@
 import NetworkExtension
 import os
 
-// swiftlint:disable:next function_body_length
-public func convertNetworkSettingsRequest(_ req: Vpn_NetworkSettingsRequest) -> NEPacketTunnelNetworkSettings {
-    let networkSettings = NEPacketTunnelNetworkSettings(tunnelRemoteAddress: req.tunnelRemoteAddress)
-    networkSettings.tunnelOverheadBytes = NSNumber(value: req.tunnelOverheadBytes)
-    networkSettings.mtu = NSNumber(value: req.mtu)
+public func convertDnsSettings(_ req: Vpn_NetworkSettingsRequest.DNSSettings) -> NEDNSSettings {
+    let dnsSettings = NEDNSSettings(servers: req.servers)
+    dnsSettings.searchDomains = req.searchDomains
+    dnsSettings.domainName = req.domainName
+    dnsSettings.matchDomains = req.matchDomains
+    dnsSettings.matchDomainsNoSearch = req.matchDomainsNoSearch
+    return dnsSettings
+}
 
-    if req.hasDnsSettings {
-        let dnsSettings = NEDNSSettings(servers: req.dnsSettings.servers)
-        dnsSettings.searchDomains = req.dnsSettings.searchDomains
-        dnsSettings.domainName = req.dnsSettings.domainName
-        dnsSettings.matchDomains = req.dnsSettings.matchDomains
-        dnsSettings.matchDomainsNoSearch = req.dnsSettings.matchDomainsNoSearch
-        networkSettings.dnsSettings = dnsSettings
+public func convertIPv4Settings(_ req: Vpn_NetworkSettingsRequest.IPv4Settings) -> NEIPv4Settings {
+    let ipv4Settings = NEIPv4Settings(addresses: req.addrs, subnetMasks: req.subnetMasks)
+    if !req.router.isEmpty {
+        ipv4Settings.router = req.router
     }
-
-    if req.hasIpv4Settings {
-        let ipv4Settings = NEIPv4Settings(addresses: req.ipv4Settings.addrs, subnetMasks: req.ipv4Settings.subnetMasks)
-        ipv4Settings.router = req.ipv4Settings.router
-        ipv4Settings.includedRoutes = req.ipv4Settings.includedRoutes.map {
-            let route = NEIPv4Route(destinationAddress: $0.destination, subnetMask: $0.mask)
+    ipv4Settings.includedRoutes = req.includedRoutes.map {
+        let route = NEIPv4Route(destinationAddress: $0.destination, subnetMask: $0.mask)
+        if !$0.router.isEmpty {
             route.gatewayAddress = $0.router
-            return route
         }
-        ipv4Settings.excludedRoutes = req.ipv4Settings.excludedRoutes.map {
-            let route = NEIPv4Route(destinationAddress: $0.destination, subnetMask: $0.mask)
+        return route
+    }
+    ipv4Settings.excludedRoutes = req.excludedRoutes.map {
+        let route = NEIPv4Route(destinationAddress: $0.destination, subnetMask: $0.mask)
+        if !$0.router.isEmpty {
             route.gatewayAddress = $0.router
-            return route
         }
-        networkSettings.ipv4Settings = ipv4Settings
+        return route
     }
+    return ipv4Settings
+}
 
-    if req.hasIpv6Settings {
-        let ipv6Settings = NEIPv6Settings(
-            addresses: req.ipv6Settings.addrs,
-            networkPrefixLengths: req.ipv6Settings.prefixLengths.map { NSNumber(value: $0)
-            }
+public func convertIPv6Settings(_ req: Vpn_NetworkSettingsRequest.IPv6Settings) -> NEIPv6Settings {
+    let ipv6Settings = NEIPv6Settings(
+        addresses: req.addrs,
+        networkPrefixLengths: req.prefixLengths.map { NSNumber(value: $0) }
+    )
+    ipv6Settings.includedRoutes = req.includedRoutes.map {
+        let route = NEIPv6Route(
+            destinationAddress: $0.destination,
+            networkPrefixLength: NSNumber(value: $0.prefixLength)
         )
-        ipv6Settings.includedRoutes = req.ipv6Settings.includedRoutes.map {
-            let route = NEIPv6Route(
-                destinationAddress: $0.destination,
-                networkPrefixLength: NSNumber(value: $0.prefixLength)
-            )
+        if !$0.router.isEmpty {
             route.gatewayAddress = $0.router
-            return route
         }
-        ipv6Settings.excludedRoutes = req.ipv6Settings.excludedRoutes.map {
-            let route = NEIPv6Route(
-                destinationAddress: $0.destination,
-                networkPrefixLength: NSNumber(value: $0.prefixLength)
-            )
+        return route
+    }
+    ipv6Settings.excludedRoutes = req.excludedRoutes.map {
+        let route = NEIPv6Route(
+            destinationAddress: $0.destination,
+            networkPrefixLength: NSNumber(value: $0.prefixLength)
+        )
+        if !$0.router.isEmpty {
             route.gatewayAddress = $0.router
-            return route
         }
-        networkSettings.ipv6Settings = ipv6Settings
+        return route
     }
-    return networkSettings
+    return ipv6Settings
 }
diff --git a/Coder Desktop/VPNLib/Receiver.swift b/Coder Desktop/VPNLib/Receiver.swift
@@ -6,7 +6,6 @@ import SwiftProtobuf
 actor Receiver<RecvMsg: Message> {
     private let dispatch: DispatchIO
     private let queue: DispatchQueue
-    private var running = false
     private let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "proto")
 
     /// Creates an instance using the given `DispatchIO` channel and queue.
@@ -58,11 +57,7 @@ actor Receiver<RecvMsg: Message> {
     /// Starts reading protocol messages from the `DispatchIO` channel and returns them as an `AsyncStream` of messages.
     /// On read or decoding error, it logs and closes the stream.
     func messages() throws(ReceiveError) -> AsyncStream<RecvMsg> {
-        if running {
-            throw .alreadyRunning
-        }
-        running = true
-        return AsyncStream(
+        AsyncStream(
             unfolding: {
                 do {
                     let length = try await self.readLen()
@@ -83,7 +78,6 @@ actor Receiver<RecvMsg: Message> {
 enum ReceiveError: Error {
     case readError(String)
     case invalidLength
-    case alreadyRunning
 }
 
 func deserializeLen(_ data: Data) throws -> UInt32 {
diff --git a/Coder Desktop/VPNLib/Speaker.swift b/Coder Desktop/VPNLib/Speaker.swift
@@ -79,10 +79,10 @@ public actor Speaker<SendMsg: RPCMessage & Message, RecvMsg: RPCMessage & Messag
             }
         )
         receiver = Receiver(dispatch: dispatch, queue: queue)
-        if SendMsg.self == Vpn_TunnelMessage.self {
-            role = .tunnel
+        role = if SendMsg.self == Vpn_TunnelMessage.self {
+            .tunnel
         } else {
-            role = .manager
+            .manager
         }
     }
 
diff --git a/Coder Desktop/VPNLib/Util.swift b/Coder Desktop/VPNLib/Util.swift
@@ -1,12 +1,23 @@
-public final class CallbackWrapper<T, U>: @unchecked Sendable {
+public struct CallbackWrapper<T, U>: @unchecked Sendable {
     private let block: (T?) -> U
 
     public init(_ block: @escaping (T?) -> U) {
         self.block = block
     }
 
     public func callAsFunction(_ error: T?) -> U {
-        // Just forward to the original block
         block(error)
     }
 }
+
+public struct CompletionWrapper<T>: @unchecked Sendable {
+    private let block: () -> T
+
+    public init(_ block: @escaping () -> T) {
+        self.block = block
+    }
+
+    public func callAsFunction() -> T {
+        block()
+    }
+}
diff --git a/Coder Desktop/VPNLibTests/ConvertTests.swift b/Coder Desktop/VPNLibTests/ConvertTests.swift
diff --git a/Makefile b/Makefile

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+--selfrequired log,info,error,debug,critical,fault`
	`2`	`+--exclude **.pb.swift`
	`3`	`+--condassignment always`
Original file line number	Diff line number	Diff line change
`@@ -79,10 +79,10 @@ public actor Speaker<SendMsg: RPCMessage & Message, RecvMsg: RPCMessage & Messag`
`79`	`79`	`}`
`80`	`80`	`)`
`81`	`81`	`receiver = Receiver(dispatch: dispatch, queue: queue)`
`82`		`- if SendMsg.self == Vpn_TunnelMessage.self {`
`83`		`- role = .tunnel`
	`82`	`+ role = if SendMsg.self == Vpn_TunnelMessage.self {`
	`83`	`+ .tunnel`
`84`	`84`	`} else {`
`85`		`- role = .manager`
	`85`	`+ .manager`
`86`	`86`	`}`
`87`	`87`	`}`
`88`	`88`