diff --git a/Coder Desktop/Coder Desktop/Coder_Desktop.entitlements b/Coder Desktop/Coder Desktop/Coder_Desktop.entitlements index 7d90a16..0d80c22 100644 --- a/Coder Desktop/Coder Desktop/Coder_Desktop.entitlements +++ b/Coder Desktop/Coder Desktop/Coder_Desktop.entitlements @@ -8,15 +8,9 @@ com.apple.developer.system-extension.install - com.apple.security.app-sandbox - com.apple.security.application-groups $(TeamIdentifierPrefix)com.coder.Coder-Desktop - com.apple.security.files.user-selected.read-only - - com.apple.security.network.client - diff --git a/Coder Desktop/Coder Desktop/NetworkExtension.swift b/Coder Desktop/Coder Desktop/NetworkExtension.swift index 16d18bb..effd194 100644 --- a/Coder Desktop/Coder Desktop/NetworkExtension.swift +++ b/Coder Desktop/Coder Desktop/NetworkExtension.swift @@ -24,12 +24,13 @@ enum NetworkExtensionState: Equatable { /// An actor that handles configuring, enabling, and disabling the VPN tunnel via the /// NetworkExtension APIs. extension CoderVPNService { - func hasNetworkExtensionConfig() async -> Bool { + func loadNetworkExtensionConfig() async { do { - _ = try await getTunnelManager() - return true + let tm = try await getTunnelManager() + neState = .disabled + serverAddress = tm.protocolConfiguration?.serverAddress } catch { - return false + neState = .unconfigured } } diff --git a/Coder Desktop/Coder Desktop/VPNService.swift b/Coder Desktop/Coder Desktop/VPNService.swift index 657d994..9d8abb8 100644 --- a/Coder Desktop/Coder Desktop/VPNService.swift +++ b/Coder Desktop/Coder Desktop/VPNService.swift @@ -63,15 +63,13 @@ final class CoderVPNService: NSObject, VPNService { // only stores a weak reference to the delegate. var systemExtnDelegate: SystemExtensionDelegate? + var serverAddress: String? + override init() { super.init() installSystemExtension() Task { - neState = if await hasNetworkExtensionConfig() { - .disabled - } else { - .unconfigured - } + await loadNetworkExtensionConfig() } xpc.connect() xpc.getPeerState() @@ -115,6 +113,7 @@ final class CoderVPNService: NSObject, VPNService { func configureTunnelProviderProtocol(proto: NETunnelProviderProtocol?) { Task { if let proto { + serverAddress = proto.serverAddress await configureNetworkExtension(proto: proto) // this just configures the VPN, it doesn't enable it tunnelState = .disabled diff --git a/Coder Desktop/Coder Desktop/Views/AuthButton.swift b/Coder Desktop/Coder Desktop/Views/AuthButton.swift index cfab088..de10208 100644 --- a/Coder Desktop/Coder Desktop/Views/AuthButton.swift +++ b/Coder Desktop/Coder Desktop/Views/AuthButton.swift @@ -17,7 +17,7 @@ struct AuthButton: View { } } label: { ButtonRowView { - Text(session.hasSession ? "Sign Out" : "Sign In") + Text(session.hasSession ? "Sign out" : "Sign in") } }.buttonStyle(.plain) } diff --git a/Coder Desktop/Coder Desktop/Views/VPNMenu.swift b/Coder Desktop/Coder Desktop/Views/VPNMenu.swift index 26266c8..3f253e1 100644 --- a/Coder Desktop/Coder Desktop/Views/VPNMenu.swift +++ b/Coder Desktop/Coder Desktop/Views/VPNMenu.swift @@ -31,13 +31,7 @@ struct VPNMenu: View { Text("Workspace Agents") .font(.headline) .foregroundColor(.gray) - if session.hasSession { - VPNState() - } else { - Text("Sign in to use CoderVPN") - .font(.body) - .foregroundColor(.gray) - } + VPNState() }.padding([.horizontal, .top], Theme.Size.trayInset) Agents() // Trailing stack @@ -52,7 +46,15 @@ struct VPNMenu: View { }.buttonStyle(.plain) TrayDivider() } - AuthButton() + if vpn.state == .failed(.systemExtensionError(.needsUserApproval)) { + Button { + openSystemExtensionSettings() + } label: { + ButtonRowView { Text("Approve in System Settings") } + }.buttonStyle(.plain) + } else { + AuthButton() + } Button { openSettings() appActivate() @@ -84,10 +86,19 @@ struct VPNMenu: View { private var vpnDisabled: Bool { !session.hasSession || vpn.state == .connecting || - vpn.state == .disconnecting + vpn.state == .disconnecting || + vpn.state == .failed(.systemExtensionError(.needsUserApproval)) } } +func openSystemExtensionSettings() { + // Sourced from: + // https://gist.github.com/rmcdongit/f66ff91e0dad78d4d6346a75ded4b751?permalink_comment_id=5261757 + // We'll need to ensure this continues to work in future macOS versions + // swiftlint:disable:next line_length + NSWorkspace.shared.open(URL(string: "x-apple.systempreferences:com.apple.ExtensionsPreferences?extensionPointIdentifier=com.apple.system_extension.network_extension.extension-point")!) +} + #Preview { VPNMenu().frame(width: 256) .environmentObject(PreviewVPN()) diff --git a/Coder Desktop/Coder Desktop/Views/VPNState.swift b/Coder Desktop/Coder Desktop/Views/VPNState.swift index 1710203..4afc6c2 100644 --- a/Coder Desktop/Coder Desktop/Views/VPNState.swift +++ b/Coder Desktop/Coder Desktop/Views/VPNState.swift @@ -1,18 +1,27 @@ import SwiftUI -struct VPNState: View { +struct VPNState: View { @EnvironmentObject var vpn: VPN + @EnvironmentObject var session: S let inspection = Inspection() var body: some View { Group { - switch vpn.state { - case .disabled: - Text("Enable CoderVPN to see agents") + switch (vpn.state, session.hasSession) { + case (.failed(.systemExtensionError(.needsUserApproval)), _): + Text("Awaiting System Extension approval") + .font(.body) + .foregroundStyle(.gray) + case (_, false): + Text("Sign in to use CoderVPN") .font(.body) .foregroundColor(.gray) - case .connecting, .disconnecting: + case (.disabled, _): + Text("Enable CoderVPN to see agents") + .font(.body) + .foregroundStyle(.gray) + case (.connecting, _), (.disconnecting, _): HStack { Spacer() ProgressView( @@ -20,7 +29,7 @@ struct VPNState: View { ).padding() Spacer() } - case let .failed(vpnErr): + case let (.failed(vpnErr), _): Text("\(vpnErr.description)") .font(.headline) .foregroundColor(.red) diff --git a/Coder Desktop/Coder Desktop/XPCInterface.swift b/Coder Desktop/Coder Desktop/XPCInterface.swift index 74baab5..73586ca 100644 --- a/Coder Desktop/Coder Desktop/XPCInterface.swift +++ b/Coder Desktop/Coder Desktop/XPCInterface.swift @@ -64,4 +64,39 @@ import VPNLib svc.onExtensionPeerUpdate(data) } } + + // The NE has verified the dylib and knows better than Gatekeeper + func removeQuarantine(path: String, reply: @escaping (Bool) -> Void) { + let reply = CallbackWrapper(reply) + Task { @MainActor in + let prompt = """ + Coder Desktop wants to execute code downloaded from \ + \(svc.serverAddress ?? "the Coder deployment"). The code has been \ + verified to be signed by Coder. + """ + let source = """ + do shell script "xattr -d com.apple.quarantine \(path)" \ + with prompt "\(prompt)" \ + with administrator privileges + """ + let success = await withCheckedContinuation { continuation in + guard let script = NSAppleScript(source: source) else { + continuation.resume(returning: false) + return + } + // Run on a background thread + Task.detached { + var error: NSDictionary? + script.executeAndReturnError(&error) + if let error { + self.logger.error("AppleScript error: \(error)") + continuation.resume(returning: false) + } else { + continuation.resume(returning: true) + } + } + } + reply(success) + } + } } diff --git a/Coder Desktop/Coder DesktopTests/VPNMenuTests.swift b/Coder Desktop/Coder DesktopTests/VPNMenuTests.swift index 4b446ac..b0484a9 100644 --- a/Coder Desktop/Coder DesktopTests/VPNMenuTests.swift +++ b/Coder Desktop/Coder DesktopTests/VPNMenuTests.swift @@ -27,7 +27,7 @@ struct VPNMenuTests { let toggle = try view.find(ViewType.Toggle.self) #expect(toggle.isDisabled()) #expect(throws: Never.self) { try view.find(text: "Sign in to use CoderVPN") } - #expect(throws: Never.self) { try view.find(button: "Sign In") } + #expect(throws: Never.self) { try view.find(button: "Sign in") } } } } diff --git a/Coder Desktop/Coder DesktopTests/VPNStateTests.swift b/Coder Desktop/Coder DesktopTests/VPNStateTests.swift index 4d630cd..298bacd 100644 --- a/Coder Desktop/Coder DesktopTests/VPNStateTests.swift +++ b/Coder Desktop/Coder DesktopTests/VPNStateTests.swift @@ -7,13 +7,16 @@ import ViewInspector @Suite(.timeLimit(.minutes(1))) struct VPNStateTests { let vpn: MockVPNService - let sut: VPNState + let session: MockSession + let sut: VPNState let view: any View init() { vpn = MockVPNService() - sut = VPNState() - view = sut.environmentObject(vpn) + sut = VPNState() + session = MockSession() + session.hasSession = true + view = sut.environmentObject(vpn).environmentObject(session) } @Test diff --git a/Coder Desktop/VPN/Manager.swift b/Coder Desktop/VPN/Manager.swift index c938818..58a65b5 100644 --- a/Coder Desktop/VPN/Manager.swift +++ b/Coder Desktop/VPN/Manager.swift @@ -46,6 +46,11 @@ actor Manager { } catch { throw .validation(error) } + + // HACK: The downloaded dylib may be quarantined, but we've validated it's signature + // so it's safe to execute. However, this SE must be sandboxed, so we defer to the app. + try await removeQuarantine(dest) + do { try tunnelHandle = TunnelHandle(dylibPath: dest) } catch { @@ -85,7 +90,9 @@ actor Manager { } catch { logger.error("tunnel read loop failed: \(error.localizedDescription, privacy: .public)") try await tunnelHandle.close() - ptp.cancelTunnelWithError(error) + ptp.cancelTunnelWithError( + makeNSError(suffix: "Manager", desc: "Tunnel read loop failed: \(error.localizedDescription)") + ) return } logger.info("tunnel read loop exited") @@ -227,6 +234,9 @@ enum ManagerError: Error { case serverInfo(String) case errorResponse(msg: String) case noTunnelFileDescriptor + case noApp + case permissionDenied + case tunnelFail(any Error) var description: String { switch self { @@ -248,6 +258,12 @@ enum ManagerError: Error { msg case .noTunnelFileDescriptor: "Could not find a tunnel file descriptor" + case .noApp: + "The VPN must be started with the app open during first-time setup." + case .permissionDenied: + "Permission was not granted to execute the CoderVPN dylib" + case let .tunnelFail(err): + "Failed to communicate with dylib over tunnel: \(err)" } } } @@ -272,3 +288,23 @@ func writeVpnLog(_ log: Vpn_Log) { let fields = log.fields.map { "\($0.name): \($0.value)" }.joined(separator: ", ") logger.log(level: level, "\(log.message, privacy: .public): \(fields, privacy: .public)") } + +private func removeQuarantine(_ dest: URL) async throws(ManagerError) { + var flag: AnyObject? + let file = NSURL(fileURLWithPath: dest.path) + try? file.getResourceValue(&flag, forKey: kCFURLQuarantinePropertiesKey as URLResourceKey) + if flag != nil { + guard let conn = globalXPCListenerDelegate.conn else { + throw .noApp + } + // Wait for unsandboxed app to accept our file + let success = await withCheckedContinuation { [dest] continuation in + conn.removeQuarantine(path: dest.path) { success in + continuation.resume(returning: success) + } + } + if !success { + throw .permissionDenied + } + } +} diff --git a/Coder Desktop/VPN/PacketTunnelProvider.swift b/Coder Desktop/VPN/PacketTunnelProvider.swift index 3cad498..0102295 100644 --- a/Coder Desktop/VPN/PacketTunnelProvider.swift +++ b/Coder Desktop/VPN/PacketTunnelProvider.swift @@ -49,20 +49,20 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { logger.info("startTunnel called") guard manager == nil else { logger.error("startTunnel called with non-nil Manager") - completionHandler(PTPError.alreadyRunning) + completionHandler(makeNSError(suffix: "PTP", desc: "Already running")) return } guard let proto = protocolConfiguration as? NETunnelProviderProtocol, let baseAccessURL = proto.serverAddress else { logger.error("startTunnel called with nil protocolConfiguration") - completionHandler(PTPError.missingConfiguration) + completionHandler(makeNSError(suffix: "PTP", desc: "Missing Configuration")) 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) + completionHandler(makeNSError(suffix: "PTP", desc: "Missing Token")) return } logger.debug("retrieved token & access URL") @@ -70,7 +70,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { Task { do throws(ManagerError) { logger.debug("creating manager") - manager = try await Manager( + let manager = try await Manager( with: self, cfg: .init( apiToken: token, serverUrl: .init(string: baseAccessURL)! @@ -78,12 +78,15 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { ) globalXPCListenerDelegate.vpnXPCInterface.manager = manager logger.debug("starting vpn") - try await manager!.startVPN() + try await manager.startVPN() logger.info("vpn started") + self.manager = manager completionHandler(nil) } catch { logger.error("error starting manager: \(error.description, privacy: .public)") - completionHandler(error as NSError) + completionHandler( + makeNSError(suffix: "Manager", desc: error.description) + ) } } } @@ -152,9 +155,3 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { try await setTunnelNetworkSettings(currentSettings) } } - -enum PTPError: Error { - case alreadyRunning - case missingConfiguration - case missingToken -} diff --git a/Coder Desktop/VPNLib/Util.swift b/Coder Desktop/VPNLib/Util.swift index ff31e4f..fd9bbc3 100644 --- a/Coder Desktop/VPNLib/Util.swift +++ b/Coder Desktop/VPNLib/Util.swift @@ -1,11 +1,11 @@ public struct CallbackWrapper: @unchecked Sendable { - private let block: (T?) -> U + private let block: (T) -> U - public init(_ block: @escaping (T?) -> U) { + public init(_ block: @escaping (T) -> U) { self.block = block } - public func callAsFunction(_ error: T?) -> U { + public func callAsFunction(_ error: T) -> U { block(error) } } @@ -21,3 +21,11 @@ public struct CompletionWrapper: @unchecked Sendable { block() } } + +public func makeNSError(suffix: String, code: Int = -1, desc: String) -> NSError { + NSError( + domain: "\(Bundle.main.bundleIdentifier!).\(suffix)", + code: code, + userInfo: [NSLocalizedDescriptionKey: desc] + ) +} diff --git a/Coder Desktop/VPNLib/XPC.swift b/Coder Desktop/VPNLib/XPC.swift index eda8ab0..dc79651 100644 --- a/Coder Desktop/VPNLib/XPC.swift +++ b/Coder Desktop/VPNLib/XPC.swift @@ -10,4 +10,5 @@ import Foundation @objc public protocol VPNXPCClientCallbackProtocol { // data is a serialized `Vpn_PeerUpdate` func onPeerUpdate(_ data: Data) + func removeQuarantine(path: String, reply: @escaping (Bool) -> Void) } diff --git a/Coder Desktop/project.yml b/Coder Desktop/project.yml index 255bc53..54ce06a 100644 --- a/Coder Desktop/project.yml +++ b/Coder Desktop/project.yml @@ -116,9 +116,6 @@ targets: com.apple.developer.networking.networkextension: - packet-tunnel-provider com.apple.developer.system-extension.install: true - com.apple.security.app-sandbox: true - com.apple.security.files.user-selected.read-only: true - com.apple.security.network.client: true com.apple.security.application-groups: - $(TeamIdentifierPrefix)com.coder.Coder-Desktop settings: