fix: unquarantine dylib after download (#38)

Author: ethanndickson

Coder Desktop/Coder Desktop/Coder_Desktop.entitlements

@@ -8,15 +8,9 @@
 	</array>
 	<key>com.apple.developer.system-extension.install</key>
 	<true/>
-	<key>com.apple.security.app-sandbox</key>
-	<true/>
 	<key>com.apple.security.application-groups</key>
 	<array>
 		<string>$(TeamIdentifierPrefix)com.coder.Coder-Desktop</string>
 	</array>
-	<key>com.apple.security.files.user-selected.read-only</key>
-	<true/>
-	<key>com.apple.security.network.client</key>
-	<true/>
 </dict>
 </plist>

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
         }
     }
 

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<CoderVPNService>?
 
+    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

Coder Desktop/Coder Desktop/Views/AuthButton.swift

@@ -17,7 +17,7 @@ struct AuthButton<VPN: VPNService, S: Session>: View {
             }
         } label: {
             ButtonRowView {
-                Text(session.hasSession ? "Sign Out" : "Sign In")
+                Text(session.hasSession ? "Sign out" : "Sign in")
             }
         }.buttonStyle(.plain)
     }

Coder Desktop/Coder Desktop/Views/VPNMenu.swift

@@ -31,13 +31,7 @@ struct VPNMenu<VPN: VPNService, S: Session>: View {
                 Text("Workspace Agents")
                     .font(.headline)
                     .foregroundColor(.gray)
-                if session.hasSession {
-                    VPNState<VPN>()
-                } else {
-                    Text("Sign in to use CoderVPN")
-                        .font(.body)
-                        .foregroundColor(.gray)
-                }
+                VPNState<VPN, S>()
             }.padding([.horizontal, .top], Theme.Size.trayInset)
             Agents<VPN, S>()
             // Trailing stack
@@ -52,7 +46,15 @@ struct VPNMenu<VPN: VPNService, S: Session>: View {
                     }.buttonStyle(.plain)
                     TrayDivider()
                 }
-                AuthButton<VPN, S>()
+                if vpn.state == .failed(.systemExtensionError(.needsUserApproval)) {
+                    Button {
+                        openSystemExtensionSettings()
+                    } label: {
+                        ButtonRowView { Text("Approve in System Settings") }
+                    }.buttonStyle(.plain)
+                } else {
+                    AuthButton<VPN, S>()
+                }
                 Button {
                     openSettings()
                     appActivate()
@@ -84,10 +86,19 @@ struct VPNMenu<VPN: VPNService, S: Session>: 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<PreviewVPN, PreviewSession>().frame(width: 256)
         .environmentObject(PreviewVPN())

Coder Desktop/Coder Desktop/Views/VPNState.swift

@@ -1,26 +1,35 @@
 import SwiftUI
 
-struct VPNState<VPN: VPNService>: View {
+struct VPNState<VPN: VPNService, S: Session>: View {
     @EnvironmentObject var vpn: VPN
+    @EnvironmentObject var session: S
 
     let inspection = Inspection<Self>()
 
     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(
                         vpn.state == .connecting ? "Starting CoderVPN..." : "Stopping CoderVPN..."
                     ).padding()
                     Spacer()
                 }
-            case let .failed(vpnErr):
+            case let (.failed(vpnErr), _):
                 Text("\(vpnErr.description)")
                     .font(.headline)
                     .foregroundColor(.red)

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)
+        }
+    }
 }

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") }
             }
         }
     }

Coder Desktop/Coder DesktopTests/VPNStateTests.swift

@@ -7,13 +7,16 @@ import ViewInspector
 @Suite(.timeLimit(.minutes(1)))
 struct VPNStateTests {
     let vpn: MockVPNService
-    let sut: VPNState<MockVPNService>
+    let session: MockSession
+    let sut: VPNState<MockVPNService, MockSession>
     let view: any View
 
     init() {
         vpn = MockVPNService()
-        sut = VPNState<MockVPNService>()
-        view = sut.environmentObject(vpn)
+        sut = VPNState<MockVPNService, MockSession>()
+        session = MockSession()
+        session.hasSession = true
+        view = sut.environmentObject(vpn).environmentObject(session)
     }
 
     @Test

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
+        }
+    }
+}

Coder Desktop/VPN/PacketTunnelProvider.swift

@@ -49,41 +49,44 @@ 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")
         let completionHandler = CallbackWrapper(completionHandler)
         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)!
                     )
                 )
                 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
-}

Coder Desktop/VPNLib/Util.swift

@@ -1,11 +1,11 @@
 public struct CallbackWrapper<T, U>: @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<T>: @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]
+    )
+}

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)
 }