Skip to content

Commit 888fd0e

Browse files
ethanndicksonethanndickson
committed
fix: manually upgrade network extension
1 parent 314edbe commit 888fd0e

File tree

5 files changed

+110
-41
lines changed

5 files changed

+110
-41
lines changed

‎Coder-Desktop/Coder-Desktop/VPN/NetworkExtension.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ extension CoderVPNService {
5858
try await tm.saveToPreferences()
5959
neState = .disabled
6060
} catch {
61+
// This typically fails when the user declines the permission dialog
6162
logger.error("save tunnel failed: \(error)")
62-
neState = .failed(error.localizedDescription)
63+
neState = .failed("Failed to save tunnel: \(error.localizedDescription). Try logging in and out again.")
6364
}
6465
}
6566

‎Coder-Desktop/Coder-Desktop/VPN/VPNService.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,13 @@ final class CoderVPNService: NSObject, VPNService {
8181
// systemExtnDelegate holds a reference to the SystemExtensionDelegate so that it doesn't get
8282
// garbage collected while the OSSystemExtensionRequest is in flight, since the OS framework
8383
// only stores a weak reference to the delegate.
84-
var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>?
84+
var systemExtnDelegate: SystemExtensionDelegate<CoderVPNService>!
8585

8686
var serverAddress: String?
8787

8888
override init() {
8989
super.init()
90+
systemExtnDelegate = SystemExtensionDelegate(asyncDelegate: self)
9091
}
9192

9293
func start() async {

‎Coder-Desktop/Coder-Desktop/VPN/VPNSystemExtension.swift‎

Lines changed: 86 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,35 @@ enum SystemExtensionState: Equatable, Sendable {
2222
}
2323
}
2424

25+
let extensionBundle: Bundle = {
26+
let extensionsDirectoryURL = URL(
27+
fileURLWithPath: "Contents/Library/SystemExtensions",
28+
relativeTo: Bundle.main.bundleURL
29+
)
30+
let extensionURLs: [URL]
31+
do {
32+
extensionURLs = try FileManager.default.contentsOfDirectory(at: extensionsDirectoryURL,
33+
includingPropertiesForKeys: nil,
34+
options: .skipsHiddenFiles)
35+
} catch {
36+
fatalError("Failed to get the contents of " +
37+
"\(extensionsDirectoryURL.absoluteString): \(error.localizedDescription)")
38+
}
39+
40+
// here we're just going to assume that there is only ever going to be one SystemExtension
41+
// packaged up in the application bundle. If we ever need to ship multiple versions or have
42+
// multiple extensions, we'll need to revisit this assumption.
43+
guard let extensionURL = extensionURLs.first else {
44+
fatalError("Failed to find any system extensions")
45+
}
46+
47+
guard let extensionBundle = Bundle(url: extensionURL) else {
48+
fatalError("Failed to create a bundle with URL \(extensionURL.absoluteString)")
49+
}
50+
51+
return extensionBundle
52+
}()
53+
2554
protocol SystemExtensionAsyncRecorder: Sendable {
2655
func recordSystemExtensionState(_ state: SystemExtensionState) async
2756
}
@@ -36,35 +65,6 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
3665
}
3766
}
3867

39-
var extensionBundle: Bundle {
40-
let extensionsDirectoryURL = URL(
41-
fileURLWithPath: "Contents/Library/SystemExtensions",
42-
relativeTo: Bundle.main.bundleURL
43-
)
44-
let extensionURLs: [URL]
45-
do {
46-
extensionURLs = try FileManager.default.contentsOfDirectory(at: extensionsDirectoryURL,
47-
includingPropertiesForKeys: nil,
48-
options: .skipsHiddenFiles)
49-
} catch {
50-
fatalError("Failed to get the contents of " +
51-
"\(extensionsDirectoryURL.absoluteString): \(error.localizedDescription)")
52-
}
53-
54-
// here we're just going to assume that there is only ever going to be one SystemExtension
55-
// packaged up in the application bundle. If we ever need to ship multiple versions or have
56-
// multiple extensions, we'll need to revisit this assumption.
57-
guard let extensionURL = extensionURLs.first else {
58-
fatalError("Failed to find any system extensions")
59-
}
60-
61-
guard let extensionBundle = Bundle(url: extensionURL) else {
62-
fatalError("Failed to create a bundle with URL \(extensionURL.absoluteString)")
63-
}
64-
65-
return extensionBundle
66-
}
67-
6868
func installSystemExtension() {
6969
logger.info("activating SystemExtension")
7070
guard let bundleID = extensionBundle.bundleIdentifier else {
@@ -75,9 +75,7 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
7575
forExtensionWithIdentifier: bundleID,
7676
queue: .main
7777
)
78-
let delegate = SystemExtensionDelegate(asyncDelegate: self)
79-
systemExtnDelegate = delegate
80-
request.delegate = delegate
78+
request.delegate = systemExtnDelegate
8179
OSSystemExtensionManager.shared.submitRequest(request)
8280
logger.info("submitted SystemExtension request with bundleID: \(bundleID)")
8381
}
@@ -90,6 +88,10 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
9088
{
9189
private var logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "vpn-installer")
9290
private var asyncDelegate: AsyncDelegate
91+
// The `didFinishWithResult` function is called for both activation,
92+
// deactivation, and replacement requests. The API provides no way to
93+
// differentiate them. https://developer.apple.com/forums/thread/684021
94+
private var state: SystemExtensionDelegateState = .installing
9395

9496
init(asyncDelegate: AsyncDelegate) {
9597
self.asyncDelegate = asyncDelegate
@@ -109,9 +111,35 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
109111
}
110112
return
111113
}
112-
logger.info("SystemExtension activated")
113-
Task { [asyncDelegate] in
114-
await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
114+
switch state {
115+
case .installing:
116+
logger.info("SystemExtension installed")
117+
Task { [asyncDelegate] in
118+
await asyncDelegate.recordSystemExtensionState(SystemExtensionState.installed)
119+
}
120+
case .deleting:
121+
logger.info("SystemExtension deleted")
122+
Task { [asyncDelegate] in
123+
await asyncDelegate.recordSystemExtensionState(SystemExtensionState.uninstalled)
124+
}
125+
let request = OSSystemExtensionRequest.activationRequest(
126+
forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
127+
queue: .main
128+
)
129+
request.delegate = self
130+
state = .installing
131+
OSSystemExtensionManager.shared.submitRequest(request)
132+
case .replacing:
133+
logger.info("SystemExtension replaced")
134+
// The installed extension now has the same version strings as this
135+
// bundle, so sending the deactivationRequest will work.
136+
let request = OSSystemExtensionRequest.deactivationRequest(
137+
forExtensionWithIdentifier: extensionBundle.bundleIdentifier!,
138+
queue: .main
139+
)
140+
request.delegate = self
141+
state = .deleting
142+
OSSystemExtensionManager.shared.submitRequest(request)
115143
}
116144
}
117145

@@ -131,12 +159,32 @@ class SystemExtensionDelegate<AsyncDelegate: SystemExtensionAsyncRecorder>:
131159
}
132160

133161
func request(
134-
_ request: OSSystemExtensionRequest,
162+
_: OSSystemExtensionRequest,
135163
actionForReplacingExtension existing: OSSystemExtensionProperties,
136164
withExtension extension: OSSystemExtensionProperties
137165
) -> OSSystemExtensionRequest.ReplacementAction {
138-
// swiftlint:disable:next line_length
139-
logger.info("Replacing \(request.identifier) v\(existing.bundleShortVersion) with v\(`extension`.bundleShortVersion)")
166+
// This is counterintuitive, but this function is only called if the
167+
// versions are the same in a dev environment.
168+
// In a release build, this only gets called when the version string is
169+
// different. We don't want to manually reinstall the extension in a dev
170+
// environment, because the bug doesn't happen.
171+
if existing.bundleVersion == `extension`.bundleVersion {
172+
return .replace
173+
}
174+
// To work around the bug described in
175+
// https://github.com/coder/coder-desktop-macos/issues/121,
176+
// we're going to manually reinstall after the replacement is done.
177+
// If we returned `.cancel` here the deactivation request will fail as
178+
// it looks for an extension with the *current* version string.
179+
// There's no way to modify the deactivate request to use a different
180+
// version string (i.e. `existing.bundleVersion`).
181+
state = .replacing
140182
return .replace
141183
}
142184
}
185+
186+
enum SystemExtensionDelegateState {
187+
case installing
188+
case replacing
189+
case deleting
190+
}

‎Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,21 @@ struct VPNMenu<VPN: VPNService, FS: FileSyncDaemon>: View {
8181
}.buttonStyle(.plain)
8282
TrayDivider()
8383
}
84+
// This shows when
85+
// 1. The user is logged in
86+
// 2. The network extension is installed
87+
// 3. The VPN is unconfigured
88+
// It's accompanied by a message in the VPNState view
89+
// that the user needs to reconfigure.
90+
if state.hasSession, vpn.state == .failed(.networkExtensionError(.unconfigured)) {
91+
Button {
92+
state.reconfigure()
93+
} label: {
94+
ButtonRowView {
95+
Text("Reconfigure VPN")
96+
}
97+
}.buttonStyle(.plain)
98+
}
8499
if vpn.state == .failed(.systemExtensionError(.needsUserApproval)) {
85100
Button {
86101
openSystemExtensionSettings()

‎Coder-Desktop/Coder-Desktop/Views/VPN/VPNState.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ struct VPNState<VPN: VPNService>: View {
1717
Text("Sign in to use Coder Desktop")
1818
.font(.body)
1919
.foregroundColor(.secondary)
20+
case (.failed(.networkExtensionError(.unconfigured)), _):
21+
Text("The system VPN requires reconfiguration.")
22+
.font(.body)
23+
.foregroundStyle(.secondary)
2024
case (.disabled, _):
2125
Text("Enable Coder Connect to see workspaces")
2226
.font(.body)
@@ -38,7 +42,7 @@ struct VPNState<VPN: VPNService>: View {
3842
.padding(.horizontal, Theme.Size.trayInset)
3943
.padding(.vertical, Theme.Size.trayPadding)
4044
.frame(maxWidth: .infinity)
41-
default:
45+
case (.connected, true):
4246
EmptyView()
4347
}
4448
}

0 commit comments

Comments
 (0)