Skip to content

chore: run coder connect networking from launchdaemon #203

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 2 commits into from
Aug 6, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Jul 23, 2025

Continues to address #201.

This PR reworks all XPC connections, such that the networking code runs within the privileged helper, instead of the network extension.

The XPC interfaces are described in XPC.swift, and roughly follow this sequence diagram:
(One difference is that we don't posix spawn the tunnel in this PR)

sequenceDiagram
    note left of App: User requests to start VPN:
    App->>+NetExt: Start VPN
    NetExt->>+PrivHelper: Request start VPN with TUN FD
    note right of PrivHelper: Privileged helper downloads and verifies binary.
    PrivHelper->>Tunnel: posix_spawn child process with FDs
    PrivHelper->>+Tunnel: Send proto start request
    Tunnel-->>-PrivHelper: Send proto start response
    PrivHelper->>+NetExt: Request for network config change
    NetExt-->>-PrivHelper: Response for network config change
    PrivHelper-->>-NetExt: Start VPN respons
    NetExt-->>-App: VPN started
    App->>PrivHelper: Request peer state
    PrivHelper->>Tunnel: Request peer state
    Tunnel-->>PrivHelper: Peer state response
    PrivHelper-->>App: Peer state response

    note left of App: Tunnel updates (bypass NetExt):
    Tunnel->>PrivHelper: Tunnel update proto message
    PrivHelper->>App: Tunnel update proto message

    note left of App: User requests to stop VPN:
    App->>+NetExt: Stop VPN
    NetExt->>+PrivHelper: Request stop VPN
    PrivHelper->>+Tunnel: Request stop VPN
    Tunnel-->>-PrivHelper: Stop VPN response
    note right of Tunnel: Tunnel binary exits
    PrivHelper-->>-NetExt: Stop VPN response
    NetExt-->>-App: VPN stopped

Loading

Of note is that the network extension starts and stops the daemon running within the privileged helper.
This is to support starting and stopping the VPN from the toggle in System Settings, and to ensure the "Connecting" and "Disconnecting" phase of the system VPN is indicative of the time the VPN is actually setting itself up and tearing itself down.

To accomplish this, the privileged helper listens on two different service names. One is connected to by the app, the other the network extension. (Once an XPC listener is connected to, communication is bidirectional)

Copy link
Member Author

ethanndickson commented Jul 23, 2025

@ethanndickson ethanndickson self-assigned this Jul 23, 2025
@ethanndickson ethanndickson changed the base branch from main to graphite-base/203 July 24, 2025 07:06
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from 49d5c99 to 72071e5 Compare July 24, 2025 07:06
@ethanndickson ethanndickson changed the base branch from graphite-base/203 to ethan/mandatory-helper July 24, 2025 07:07
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from 72071e5 to c7dbde8 Compare July 24, 2025 09:07
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from c7dbde8 to ef8832a Compare July 28, 2025 07:50
@ethanndickson ethanndickson force-pushed the ethan/mandatory-helper branch from 1737580 to 16c716d Compare July 28, 2025 07:50
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from ef8832a to e32d7de Compare July 30, 2025 09:31
Comment on lines 66 to 77
guard let proxy = conn.remoteObjectProxyWithErrorHandler({ err in
self.logger.error("failed to connect to HelperXPC \(err.localizedDescription, privacy: .public)")
continuation.resume(throwing: err)
}) as? HelperAppXPCInterface else {
self.logger.error("failed to get proxy for HelperXPC")
continuation.resume(throwing: XPCError.wrongProxyType)
return
}
proxy.ping {
self.logger.info("Connected to Helper over XPC")
continuation.resume()
}
Copy link
Member Author

@ethanndickson ethanndickson Jul 30, 2025

Choose a reason for hiding this comment

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

Important to note that I've refactored all the XPC connections to use this pattern. With this, you're guaranteed that either the the XPC reply will be run (proxy.ping { reply } in this case) or the [...]WithErrorHandler callback.

Comment on lines +14 to +15
// /var/root/Downloads
private let dest = FileManager.default.urls(for: .downloadsDirectory, in: .userDomainMask)
Copy link
Member Author

@ethanndickson ethanndickson Jul 30, 2025

Choose a reason for hiding this comment

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

Temporary. I've put it in /var/root/Library/Application\ Support/com.coder.Coder-Desktop/ as part of the PR that downloads the slim binary.

@ethanndickson ethanndickson marked this pull request as ready for review July 30, 2025 13:09
Copy link
Member

@deansheather deansheather left a comment

Choose a reason for hiding this comment

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

The XPC code seems a lot nicer but the type names and directions of the XPC types are hard to understand

@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from eebf562 to 291e5a1 Compare July 31, 2025 07:26
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch 2 times, most recently from b0c196f to b81afc9 Compare August 4, 2025 03:00
@ethanndickson ethanndickson force-pushed the ethan/mandatory-helper branch from 7b9491c to 6a93fac Compare August 4, 2025 03:00
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from b81afc9 to e96075e Compare August 4, 2025 03:03
Copy link
Member Author

Choose a reason for hiding this comment

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

The class is called HelperXPCClient, but you can't have multiple Swift files with the same name. So, I've prepended NE, since this is the HelperXPCClient that runs within the network extension.

@ethanndickson ethanndickson force-pushed the ethan/mandatory-helper branch from 6eeb8aa to c8ecda2 Compare August 4, 2025 07:58
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch 2 times, most recently from a4b58e5 to bd905ae Compare August 4, 2025 08:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reworks the XPC architecture to move VPN networking functionality from the network extension to a privileged helper daemon. The helper now manages the VPN tunnel and communicates with both the GUI app and network extension via separate XPC interfaces, implementing a more secure and maintainable design.

Key changes:

  • Moved VPN networking code from network extension to privileged helper daemon
  • Established bidirectional XPC communication between helper, app, and network extension
  • Updated project configuration to support the new architecture

Reviewed Changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
project.yml Updated build configuration to support helper dependencies and framework loading
XPC.swift Defined new XPC interfaces for helper-app and helper-network extension communication
Download.swift Renamed SignatureValidator class to Validator
main.swift Simplified network extension entry point, removed XPC listener setup
PacketTunnelProvider.swift Refactored to delegate VPN operations to helper via XPC
NEHelperXPCClient.swift New XPC client for network extension to communicate with helper
Manager.swift Moved to helper, updated to work without direct PacketTunnelProvider dependency
HelperXPCListeners.swift New XPC server implementations for helper to handle app and network extension connections
AppHelperXPCClient.swift New XPC client for GUI app to communicate with helper

@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from d09250b to d286679 Compare August 6, 2025 03:38
@ethanndickson ethanndickson force-pushed the ethan/mandatory-helper branch from b535a7d to 493701d Compare August 6, 2025 03:38
Copy link
Member Author

ethanndickson commented Aug 6, 2025

Merge activity

  • Aug 6, 3:39 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 6, 3:43 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 6, 3:44 AM UTC: @ethanndickson merged this pull request with Graphite.

@ethanndickson ethanndickson changed the base branch from ethan/mandatory-helper to graphite-base/203 August 6, 2025 03:39
@ethanndickson ethanndickson changed the base branch from graphite-base/203 to main August 6, 2025 03:41
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from d286679 to d9c0210 Compare August 6, 2025 03:42
@ethanndickson ethanndickson merged commit 8533b31 into main Aug 6, 2025
4 checks passed
@ethanndickson ethanndickson deleted the ethan/networking-in-launchdaemon branch August 6, 2025 03:44
ethanndickson added a commit that referenced this pull request Aug 6, 2025
With the changes made in #203, it now takes a moment longer to receive the first progress update, when we either start the download (if not already downloaded), or validate the dylib. To address this, the progress indicator will immediately start making progress towards 25%. This prevents it from appearing stuck in what is an expected situation.


https://github.com/user-attachments/assets/da57270d-a50b-49ab-9e53-ae02368c71dc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants