Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: ethan/slim-over-dylib
Choose a base branch
from
+53 −9
Open

chore: ensure downloaded slim binary version matches server #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion Coder-Desktop/Coder-DesktopHelper/Manager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ actor Manager {
}
pushProgress(stage: .validating)
do {
try Validator.validate(path: dest)
try Validator.validateSignature(binaryPath: dest)
try await Validator.validateVersion(binaryPath: dest, serverVersion: buildInfo.version)
} catch {
// Cleanup unvalid binary
try? FileManager.default.removeItem(at: dest)
Expand Down
59 changes: 51 additions & 8 deletions Coder-Desktop/VPNLib/Validate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import Subprocess

public enum ValidationError: Error {
case fileNotFound
Expand All @@ -7,7 +8,9 @@ public enum ValidationError: Error {
case unableToRetrieveSignature
case invalidIdentifier(identifier: String?)
case invalidTeamIdentifier(identifier: String?)
case invalidVersion(version: String?)
case unableToReadVersion(any Error)
case binaryVersionMismatch(binaryVersion: String, serverVersion: String)
case internalError(OSStatus)

public var description: String {
switch self {
Expand All @@ -21,10 +24,14 @@ public enum ValidationError: Error {
"Unable to retrieve signing information."
case let .invalidIdentifier(identifier):
"Invalid identifier: \(identifier ?? "unknown")."
case let .invalidVersion(version):
"Invalid runtime version: \(version ?? "unknown")."
case let .binaryVersionMismatch(binaryVersion, serverVersion):
"Binary version does not match server. Binary: \(binaryVersion), Server: \(serverVersion)."
case let .invalidTeamIdentifier(identifier):
"Invalid team identifier: \(identifier ?? "unknown")."
case let .unableToReadVersion(error):
"Unable to execute the binary to read version: \(error.localizedDescription)"
case let .internalError(status):
"Internal error with OSStatus code: \(status)."
}
}

Expand All @@ -37,22 +44,32 @@ public class Validator {
public static let minimumCoderVersion = "2.24.2"

private static let expectedIdentifier = "com.coder.cli"
// The Coder team identifier
private static let expectedTeamIdentifier = "4399GN35BJ"

// Apple-issued certificate chain
public static let anchorRequirement = "anchor apple generic"

private static let signInfoFlags: SecCSFlags = .init(rawValue: kSecCSSigningInformation)

public static func validate(path: URL) throws(ValidationError) {
guard FileManager.default.fileExists(atPath: path.path) else {
public static func validateSignature(binaryPath: URL) throws(ValidationError) {
guard FileManager.default.fileExists(atPath: binaryPath.path) else {
throw .fileNotFound
}

var staticCode: SecStaticCode?
let status = SecStaticCodeCreateWithPath(path as CFURL, SecCSFlags(), &staticCode)
let status = SecStaticCodeCreateWithPath(binaryPath as CFURL, SecCSFlags(), &staticCode)
guard status == errSecSuccess, let code = staticCode else {
throw .unableToCreateStaticCode
}

let validateStatus = SecStaticCodeCheckValidity(code, SecCSFlags(), nil)
var requirement: SecRequirement?
let reqStatus = SecRequirementCreateWithString(anchorRequirement as CFString, SecCSFlags(), &requirement)
guard reqStatus == errSecSuccess, let requirement else {
throw .internalError(OSStatus(reqStatus))
}

let validateStatus = SecStaticCodeCheckValidity(code, SecCSFlags(), requirement)
guard validateStatus == errSecSuccess else {
throw .invalidSignature
}
Expand All @@ -78,6 +95,32 @@ public class Validator {
}
}

public static let xpcPeerRequirement = "anchor apple generic" + // Apple-issued certificate chain
// This function executes the binary to read its version, and so it assumes
// the signature has already been validated.
public static func validateVersion(binaryPath: URL, serverVersion: String) async throws(ValidationError) {
guard FileManager.default.fileExists(atPath: binaryPath.path) else {
throw .fileNotFound
}

let version: String
do {
try chmodX(at: binaryPath)
let versionOutput = try await Subprocess.data(for: [binaryPath.path, "version", "--output=json"])
Copy link
Member

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...

Copy link
Member Author

@ethanndickson ethanndickson Aug 4, 2025 •
edited
Loading

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.

Copy link
Member

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

let parsed: VersionOutput = try JSONDecoder().decode(VersionOutput.self, from: versionOutput)
version = parsed.version
} catch {
throw .unableToReadVersion(error)
}

guard version == serverVersion else {
throw .binaryVersionMismatch(binaryVersion: version, serverVersion: serverVersion)
}
}

struct VersionOutput: Codable {
let version: String
}

public static let xpcPeerRequirement = anchorRequirement +
" and certificate leaf[subject.OU] = \"" + expectedTeamIdentifier + "\"" // Signed by the Coder team
}
Loading