Skip to content

chore: pass session token to network extension #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions Coder Desktop/.swiftformat
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
--selfrequired log,info,error,debug,critical,fault
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the docs:

In the rare case of functions with @autoclosure arguments, self may be required at the call site, but SwiftFormat is unable to detect this automatically. You can use the --selfrequired command-line option to specify a list of such methods, and the redundantSelf rule will then ignore them.

--exclude **.pb.swift
--condassignment always
8 changes: 4 additions & 4 deletions Coder Desktop/Coder Desktop/Coder_DesktopApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ struct DesktopApp: App {
.environmentObject(appDelegate.settings)
}
.windowResizability(.contentSize)
SwiftUI.Settings { SettingsView<CoderVPNService>()
.environmentObject(appDelegate.vpn)
.environmentObject(appDelegate.settings)
SwiftUI.Settings {
SettingsView<CoderVPNService>()
.environmentObject(appDelegate.vpn)
.environmentObject(appDelegate.settings)
}
.windowResizability(.contentSize)
}
Expand All @@ -32,7 +33,6 @@ class AppDelegate: NSObject, NSApplicationDelegate {
let settings: Settings

override init() {
// TODO: Replace with real implementation
vpn = CoderVPNService()
settings = Settings()
session = SecureSession(onChange: vpn.configureTunnelProviderProtocol)
Expand Down
4 changes: 3 additions & 1 deletion Coder Desktop/Coder Desktop/State.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
27 changes: 19 additions & 8 deletions Coder Desktop/VPN/Manager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,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()
// TODO: Notify app over XPC
return
Expand All @@ -108,21 +108,33 @@ 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))`")
}
}

// TODO: Call via XPC
func startVPN() async throws(ManagerError) {
logger.info("sending start rpc")
guard let tunFd = ptp.tunnelFileDescriptor else {
Expand All @@ -149,7 +161,6 @@ actor Manager {
// TODO: notify app over XPC
}

// TODO: Call via XPC
func stopVPN() async throws(ManagerError) {
logger.info("sending stop rpc")
let resp: Vpn_TunnelMessage
Expand Down Expand Up @@ -246,5 +257,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)")
}
79 changes: 71 additions & 8 deletions Coder Desktop/VPN/PacketTunnelProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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()
Expand Down Expand Up @@ -41,21 +43,42 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
return nil
}

override func startTunnel(options _: [String: NSObject]?, completionHandler: @escaping (Error?) -> Void) {
override func startTunnel(
options _: [String: NSObject]?, completionHandler: @escaping (Error?) -> Void
) {
logger.debug("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.debug("creating manager")
manager = try await Manager(
with: self,
cfg: .init(apiToken: "fake-token", serverUrl: .init(string: "https://dev.coder.com")!)
cfg: .init(
apiToken: token, serverUrl: .init(string: baseAccessURL)!
)
)
logger.debug("starting vpn")
try await manager!.startVPN()
logger.info("vpn started")
completionHandler(nil)
} catch {
completionHandler(error)
Expand All @@ -64,15 +87,26 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
}
}

override func stopTunnel(with _: NEProviderStopReason, completionHandler: @escaping () -> Void) {
override func stopTunnel(
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
}
manager = nil
completionHandler()

let completionHandler = CompletionWrapper(completionHandler)
Task { [manager] in
do throws(ManagerError) {
try await manager.stopVPN()
} catch {
logger.error("error stopping manager: \(error.description, privacy: .public)")
}
completionHandler()
}
self.manager = nil
}

override func handleAppMessage(_ messageData: Data, completionHandler: ((Data?) -> Void)?) {
Expand All @@ -92,4 +126,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
}
85 changes: 43 additions & 42 deletions Coder Desktop/VPNLib/Convert.swift
Original file line number Diff line number Diff line change
@@ -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
}
8 changes: 1 addition & 7 deletions Coder Desktop/VPNLib/Receiver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Copy link
Member Author

@ethanndickson ethanndickson Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

messages() is called each time next() is called on the Speaker, but it's not possible to mutate the running variable from within the stream in Swift 6.

Even if it were possible, I don't think this check is of any value. Isn't it usually the case (in general) that iterating over the same (mutating) stream multiple times is just an incorrect use of an API? I think actor re-entrancy means it's possible to do so here.

If we do want to keep the check, I'm open to suggestions on how best to do that.

AsyncStream(
unfolding: {
do {
let length = try await self.readLen()
Expand All @@ -83,7 +78,6 @@ actor Receiver<RecvMsg: Message> {
enum ReceiveError: Error {
case readError(String)
case invalidLength
case alreadyRunning
}

func deserializeLen(_ data: Data) throws -> UInt32 {
Expand Down
6 changes: 3 additions & 3 deletions Coder Desktop/VPNLib/Speaker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
Loading
Loading