Skip to content

fix: unquarantine dylib after download #38

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 3 commits into from
Feb 12, 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
6 changes: 0 additions & 6 deletions Coder Desktop/Coder Desktop/Coder_Desktop.entitlements
Original file line number Diff line number Diff line change
Expand Up @@ -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>
9 changes: 5 additions & 4 deletions Coder Desktop/Coder Desktop/NetworkExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
9 changes: 4 additions & 5 deletions Coder Desktop/Coder Desktop/VPNService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Coder Desktop/Coder Desktop/Views/AuthButton.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
29 changes: 20 additions & 9 deletions Coder Desktop/Coder Desktop/Views/VPNMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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())
Expand Down
21 changes: 15 additions & 6 deletions Coder Desktop/Coder Desktop/Views/VPNState.swift
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
35 changes: 35 additions & 0 deletions Coder Desktop/Coder Desktop/XPCInterface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
2 changes: 1 addition & 1 deletion Coder Desktop/Coder DesktopTests/VPNMenuTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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") }
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions Coder Desktop/Coder DesktopTests/VPNStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 37 additions & 1 deletion Coder Desktop/VPN/Manager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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)"
}
}
}
Expand All @@ -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
}
}
}
21 changes: 9 additions & 12 deletions Coder Desktop/VPN/PacketTunnelProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
}
}
}
Expand Down Expand Up @@ -152,9 +155,3 @@ class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable {
try await setTunnelNetworkSettings(currentSettings)
}
}

enum PTPError: Error {
case alreadyRunning
case missingConfiguration
case missingToken
}
14 changes: 11 additions & 3 deletions Coder Desktop/VPNLib/Util.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
Expand All @@ -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]
)
}
1 change: 1 addition & 0 deletions Coder Desktop/VPNLib/XPC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Loading
Loading