Skip to content

fix: add code signing requirements to xpc connections #206

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 1 commit into from
Aug 6, 2025

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Jul 24, 2025

Continues to address #201.

I've manually tested that this change prevents binaries not signed by the Coder Apple development team from connecting to the Helper over XPC.

Most of the PR diff is me moving the validator out of Download.swift and into Validate.swift

Copy link
Member Author

ethanndickson commented Jul 24, 2025

@ethanndickson ethanndickson force-pushed the ethan/xpc-validation branch from 547fd97 to 6687411 Compare July 31, 2025 03:57
@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/xpc-validation branch from 6687411 to ef370db Compare July 31, 2025 07:26
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from 291e5a1 to b0c196f Compare August 4, 2025 02:59
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from b0c196f to b81afc9 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
@ethanndickson ethanndickson force-pushed the ethan/xpc-validation branch 2 times, most recently from be347a8 to e6a3578 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
@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from bd905ae to 33931d6 Compare August 4, 2025 08:07
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 enhances security by adding code signing requirements to XPC connections to prevent unauthorized binaries from connecting to the Helper service. The changes implement validation that ensures only binaries signed by the Coder Apple development team can establish XPC connections.

Key changes:

  • Refactored validation logic from Download.swift into a dedicated Validate.swift file
  • Added xpcPeerRequirement property to enforce code signing requirements on XPC connections
  • Applied code signing validation to all XPC connection points in the application

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Coder-Desktop/VPNLib/Validate.swift New file containing extracted validation logic with added XPC peer requirement string
Coder-Desktop/VPNLib/Download.swift Removed validation code that was moved to Validate.swift
Coder-Desktop/VPN/NEHelperXPCClient.swift Added code signing requirement to XPC client connection
Coder-Desktop/Coder-DesktopHelper/HelperXPCListeners.swift Added code signing requirements to both XPC server listeners
Coder-Desktop/Coder-Desktop/AppHelperXPCClient.swift Added code signing requirement to app helper XPC client

}

guard let plistName = infoPlist[infoNameKey] as? String, plistName == expectedName else {
throw .invalidIdentifier(identifier: infoPlist[infoNameKey] as? String)
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

The error type should be a name-specific validation error, not invalidIdentifier. This validation is checking the bundle name, not the identifier, so it should throw a different error type or the existing invalidIdentifier case should be renamed to be more generic.

Suggested change
throw .invalidIdentifier(identifier: infoPlist[infoNameKey] as? String)
throw .invalidName(name: infoPlist[infoNameKey] as? String)

Copilot uses AI. Check for mistakes.

@ethanndickson ethanndickson force-pushed the ethan/networking-in-launchdaemon branch from d09250b to d286679 Compare August 6, 2025 03:37
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:45 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 6, 3:47 AM UTC: @ethanndickson merged this pull request with Graphite.

@ethanndickson ethanndickson changed the base branch from ethan/networking-in-launchdaemon to graphite-base/206 August 6, 2025 03:42
@ethanndickson ethanndickson changed the base branch from graphite-base/206 to main August 6, 2025 03:44
@ethanndickson ethanndickson merged commit ff169e3 into main Aug 6, 2025
4 checks passed
@ethanndickson ethanndickson deleted the ethan/xpc-validation branch August 6, 2025 03:47
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