- Notifications
You must be signed in to change notification settings - Fork 5
chore: ensure downloaded slim binary version matches server#211
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
Conversation
ethanndickson commented Jul 31, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
ethanndickson commented Jul 31, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
matifali commented Jul 31, 2025
Not directly related but @jdomeracki-coder should we also implement binary verification in Coderd Desktop? cc: @deansheather |
ethanndickson commented Jul 31, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
929c831 to 262c4d1Compare| letversion:String | ||
| do{ | ||
| letversionOutput=tryawaitSubprocess.data(for:[binaryPath.path,"version","--output=json"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any way to drop privileges for this subprocess? We are root after all...
ethanndicksonAug 4, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is possible with a combination of launchctl asuser <uid> and sudo -u [<uid>|<username>]:
sudo -u '#501' launchctl asuser 501 /usr/bin/whoami One switches the UID, the other the (macOS specific) execution context.
Unfortunately,
$ sudo -u nobody launchctl asuser -2 /usr/bin/whoami Could not switch to audit session 0x187c3: 1: Operation not permitted does not work, and so we'd need to determine the currently logged in user in some other way.501 is the first created user on the machine, and on corporate devices this will likely be some admin user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine to execute it as root if it's too difficult, the signature is still ours so the risk is small
e9e15db to c7602c2Compare262c4d1 to a7b16eaComparec7602c2 to f008801Comparea7b16ea to 9695afeComparef008801 to 847006fComparec229789 to a723a9aCompare847006f to d9255bcComparea723a9a to ffe7d2eCompared9255bc to 4f29ff3Compareffe7d2e to ab9ca27Compare4f29ff3 to 5840bceCompareab9ca27 to 50e4a63Compare5840bce to 3f7f6b6Compare50e4a63 to 8fb9382Compare3f7f6b6 to 42b8f0bCompare8fb9382 to 431149fCompare42b8f0b to b716554Compareb716554 to 02b3369Compare431149f to 8fceeabCompareThere was a problem hiding this 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 the security validation of downloaded Coder CLI binaries by adding version validation and strengthening certificate chain verification. The changes prevent downgrade attacks by ensuring the downloaded binary version matches the server version, and add explicit Apple certificate chain validation.
Key changes:
- Split validation into separate signature and version validation functions
- Add binary version validation by executing
coder version --output=json - Strengthen certificate validation to require Apple-issued certificate chain
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Coder-Desktop/VPNLib/Validate.swift | Refactors validation logic, adds version validation function, and enhances certificate chain requirements |
| Coder-Desktop/Coder-DesktopHelper/Manager.swift | Updates to use new two-step validation process (signature then version) |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
02b3369 to a4bbf6bCompare8fceeab to 62c3d0aCompareethanndickson commented Aug 6, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Merge activity
|
62c3d0a to 7c6b9a7Compareb74def3 into mainUh oh!
There was an error while loading. Please reload this page.

Relates to #201.
After we've validated the binary signature, we exec
coder version --output=jsonto validate the version of the downloaded binary matches the server. This is done to prevent against downgrade attacks, and to match the checking we had on the dylib before.Additionally, this PR also ensures the certificate used to sign the binary is part of an Apple-issued certificate chain.
I assumed we were checking this before (by default) but we weren't.
Though we weren't previously checking it, we were only ever downloading and executing a dylib.
My understanding is that macOS won't execute a dylib unless the executing process and the dylib were signed by the same Apple developer team (at least in a sandboxed process, as is the Network Extension).
Only now, when
posix_spawning the slim binary from an unsandboxed LaunchDaemon, is this check absolutely necessary.