Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.
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
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,13 @@ final class NetworkProtectionTunnelController: NetworkProtection.TunnelControlle
// We'll only update to completed if we were showing the onboarding step to
// allow the system extension. Otherwise we may override the allow-VPN
// onboarding step.
//
// Additionally if the onboarding step was allowing the system extension, we won't
// start the tunnel at once, and instead require that the user enables the toggle.
//
if onboardingStatusRawValue == OnboardingStatus.isOnboarding(step: .userNeedsToAllowExtension).rawValue {
onboardingStatusRawValue = OnboardingStatus.completed.rawValue
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This modifies the flow - if we're enabling the system extension, we'll have the user start the tunnel manually again.

As requested here: https://app.asana.com/0/0/1205275221447701/1205278158970389/f

}
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,76 @@ struct SystemExtensionManager {
case willActivateAfterReboot
}

let bundleID: String
let manager: OSSystemExtensionManager
private static let systemSettingsSecurityURL = "x-apple.systempreferences:com.apple.preference.security?Security"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, I hadn't seen this URL format before


private let bundleID: String
private let manager: OSSystemExtensionManager
private let workspace: NSWorkspace

init(bundleID: String = NetworkProtectionBundle.extensionBundle().bundleIdentifier!,
manager: OSSystemExtensionManager = .shared) {
manager: OSSystemExtensionManager = .shared,
workspace: NSWorkspace = .shared) {

self.bundleID = bundleID
self.manager = manager
self.workspace = workspace
}

func activate() -> AsyncThrowingStream<ActivationRequestEvent, Error> {
/// Documenting a workaround for the issue discussed in https://app.asana.com/0/0/1205275221447702/f
/// Background: For a lot of users, the system won't show the system-extension-blocked alert if there's a previous request
/// to activate the extension. You can see active requests in your console using command `systemextensionsctl list`.
///
/// Proposed workaround: Just open system settings into the right section when we detect a previous activation request already exists.
///
/// Tradeoffs: Unfortunately we don't know if the previous request was sent out by the currently runing-instance of this App
/// or if an activation request was made, and then the App was reopened.
/// This means we don't know if we'll be notified when the previous activation request completes or fails. Because we
/// need to update our UI once the extension is allowed, we can't avoid sending a new activation request every time.
/// For the users that don't see the alert come up more than once this should be invisible. For users (like myself) that
/// see the alert every single time, they'll see both the alert and system settings being opened automatically.
///
if hasPendingActivationRequests() {
openSystemSettingsSecurity()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there are pending activation requests we'll open system settings directly.

}

return SystemExtensionRequest.activationRequest(forExtensionWithIdentifier: bundleID, manager: manager).submit()
}

func deactivate() async throws {
for try await _ in SystemExtensionRequest.deactivationRequest(forExtensionWithIdentifier: bundleID, manager: manager).submit() {}
}

// MARK: - Activation: Checking if there are pending requests

/// Checks if there are pending activation requests for the system extension.
///
/// This implementation should work well for all macOS 11+ releases. A better implementation for macOS 12+
/// would be to use a properties request, but that option requires bigger changes and some rethinking of these
/// classes which I'd rather avoid right now. In short this solution was picked as a quick solution with the best
/// ROI to avoid getting blocked.
///
private func hasPendingActivationRequests() -> Bool {
let task = Process()
let pipe = Pipe()

task.standardOutput = pipe
task.launchPath = "/bin/bash" // Specify the shell to use
task.arguments = ["-c", "$(which systemextensionsctl) list | $(which egrep) -c '(?:\(bundleID)).+(?:activated waiting for user)+'"]

task.launch()
task.waitUntilExit()

let data = pipe.fileHandleForReading.readDataToEndOfFile()
let output = String(data: data, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines)

return (Int(output ?? "0") ?? 0) > 0
Comment on lines +82 to +95
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While we can request the system extension properties using a specific type of request, doing so requires more complicated modifications to the code.

This gives us quickly something that works immediately for macOS 11+.

Sending a properties request is much more complicated because it requires me to modify SystemExtensionManager to support different delegates and running multiple requests.

This decision was a tradeoff.

}

private func openSystemSettingsSecurity() {
let url = URL(string: Self.systemSettingsSecurityURL)!
workspace.open(url)
}
}

final class SystemExtensionRequest: NSObject {
Expand Down