diff --git a/README.md b/README.md index c1b6cdaa..8a3d2c0f 100644 --- a/README.md +++ b/README.md @@ -407,9 +407,13 @@ Burrow drives a bundled, open-source Mole engine (an MIT fork of `mo`). The hone recordings, user files, user paths, metrics, or stored IP; turn it off in Settings. PostHog delivery and its bounded sanitized retry outbox run entirely off AppKit's main thread. Full list in **[TELEMETRY.md](TELEMETRY.md)**. -- **No background root helper.** When Clean/Optimize need admin rights, macOS's - own dialog asks you and Burrow runs that one `mo` command, then exits — you - approve every elevation. +- **No background root helper by default.** When Clean/Optimize need admin + rights, macOS's own dialog asks you and Burrow runs that one `mo` command, + then exits — you approve every elevation. You can opt in to a small signed + helper so those prompts accept Touch ID; it grants no standing privilege + (every root operation still authenticates, every time), performs only scan, + clean, and optimize, and can be removed from Settings. Details in + **[SECURITY.md](SECURITY.md)**. - **Local-only surfaces:** the MCP/HTTP surfaces bind to loopback only (`127.0.0.1`) and history is stored locally. On Windows, the HTTP REST toggle disables REST endpoints but keeps the local `/mcp` bridge route available for diff --git a/RELEASES.md b/RELEASES.md index 19acad90..198ecbc2 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,57 +1,56 @@ -# Burrow 0.11.2 +# Burrow 0.12.0 -A system-metrics, updater, and launch-reliability patch. This release corrects -CPU sampling, stops expected Sparkle conditions from looking like product -defects, gives AppKit a settled launch turn before creating the menu-bar item, -and makes sampled app hangs reliably reach the issue tracker. +Burrow's admin operations can now authenticate with **Touch ID**. -> **Affected macOS 27 beta users:** please install 0.11.2 and report the result -> in [#319](https://github.com/caezium/Burrow/issues/319). The exact Beta 4 -> compatibility guard remains in place, and the issue stays open until a -> notarized build is verified on a Mac that reproduced the freeze. +Until now every elevated action went through macOS's classic authorization +dialog, which is password-only by construction — it never offers Touch ID, and +it can't be cancelled safely. This release adds an optional signed helper that +replaces that path. + +## Added +- **Touch ID for admin operations.** Install the helper in **Settings ▸ + Advanced ▸ Privileged helper** and Clean, Optimize, the admin scan previews, + Flush DNS, Renew DHCP, and the Login Items list all authenticate through the + system's normal prompt — which offers Touch ID where the hardware has it, and + falls back to your password everywhere else. +- **The Login Items list is now complete.** Reading it needs root, so + previously macOS raised its own unexplained "sfltool wants to make changes" + prompt and still returned only a partial list. Through the helper it's one + prompt you recognise, and the full list. + +## What the helper can and cannot do + +It is strictly opt-in, takes its own one-time macOS approval, and grants no +standing access — you authenticate for each operation you start. + +It accepts seven fixed operations and builds every command line itself. There +is no field in its API for a path, a shell string, or an executable, so it +cannot be asked to run anything else. It runs the engine sealed inside the +signed app plus four Apple tools by absolute path, each as a separate process +with no shell involved. Only Burrow can talk to it: callers are pinned to the +app's bundle identifier and signing team by the system. + +One honest caveat: the credential from your authentication stays valid for ten +seconds, because it has to survive the hop from the app to the helper. A second +operation begun inside that window won't prompt again. Full detail in +[SECURITY.md](https://github.com/caezium/Burrow/blob/main/SECURITY.md). + +Not installing it changes nothing — every operation keeps working exactly as it +does today, through the existing password prompt. You can remove the helper at +any time from Settings, or from System Settings ▸ General ▸ Login Items & +Extensions. + +## Changed +- **Flush DNS no longer runs a root shell.** It previously elevated + `/bin/sh -c "dscacheutil -flushcache; killall -HUP mDNSResponder"`, handing a + command string to a shell running as root. It's now two separate processes + with fixed arguments. +- **Removed the "Touch ID for sudo" setting.** It configured `pam_tid` for + terminal `sudo` and never affected Burrow's own admin prompts, which is what + people expected it to do. Those prompts are what the privileged helper now + covers. Nothing already configured on your Mac is changed by removing it; to + undo it yourself, run `mo touchid disable`. ## Fixed -- **CPU usage now reflects a representative sampling interval.** The bundled - engine keeps a tick baseline across refreshes, samples before the other - collectors fan out, and derives total usage from summed tick deltas. This - removes the roughly doubled readings and coarse per-core fractions reported in - [#335](https://github.com/caezium/Burrow/issues/335). A cold one-shot status - command can take about 600 ms longer; ongoing GUI sampling reuses its existing - refresh interval and adds no wait. ([#340](https://github.com/caezium/Burrow/pull/340)) -- **Updater failures now mean what they say.** Running from a disk image or a - translocated location, ordinary network failures, and user cancellation remain - measurable in PostHog without opening Sentry issues. Sparkle keeps ownership of - its native move-to-Applications and scheduled-retry UI. Configuration, - signature, installation, and unknown failures still create exactly one - scrubbed Sentry diagnostic per cycle. ([#339](https://github.com/caezium/Burrow/pull/339)) -- **The normal menu-bar path no longer races the first AppKit launch turn.** - Burrow waits one second before creating its status item, then retains the - existing 30-second stability window. The safeguard for macOS 27 Beta 4 build - `26A5388g` remains exact-build-only; a later macOS build returns to the normal - guarded path automatically. ([#339](https://github.com/caezium/Burrow/pull/339)) - -## Improved -- **App-hang evidence can no longer disappear at the Sentry bridge.** Sampled - hangs are collected into bounded weekly GitHub digests instead of being - silently skipped. Cursor pagination reaches older unseen groups, full digests - roll into numbered parts, and deferred groups remain eligible for the next - run. ([#339](https://github.com/caezium/Burrow/pull/339)) -- **Launch and updater health now have explicit lifecycle outcomes.** Fixed-name - scheduled, stabilizing, and stable milestones include bounded app release, - macOS build, launch phase, and status-item state, so future failures can be - separated without collecting free text or user data. - -## Privacy -- **Telemetry remains optional, unlinked, and non-tracking.** One Settings - switch disables both PostHog analytics and Sentry diagnostics. Updater - diagnostics contain fixed categories and bounded error domains/codes—never - descriptions, URLs, response bodies, network names, paths, screen content, or - files. The privacy manifest remains unchanged and accurate. - -## Security -- **Publishing still fails closed, including the external Homebrew tap.** Before - any release build begins, CI requires every signing, notarization, Sparkle, - and tap credential, then proves the tap token with a reversible Git - write. The tap credential is isolated from the engine checkout so a successful - notarized release cannot fail at the final cask push because the wrong token - was left in Git configuration. +- A failed elevated run could report "Done — caches cleared" when nothing had + actually run. Failures now say so. diff --git a/SECURITY.md b/SECURITY.md index e86a4495..e5abed63 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -48,14 +48,60 @@ or notarization and changes between builds. This is the part people rightly scrutinize in cleaners. Burrow's model: -- **Burrow installs no privileged/background helper and no XPC root - service.** There is nothing persistently running as root and nothing for - another local process to connect to. +- **By default Burrow installs no privileged helper.** Out of the box there + is nothing running as root and nothing for another local process to + connect to. - When **Clean** or **Optimize** needs admin rights, **macOS's own authorization dialog** asks for your password, and Burrow runs the matching `mo` command for that single action, then exits. You see and - approve every elevation. (See `CommandRunner.runElevated` in - `macos/Sources/TaskReport.swift`.) + approve every elevation. +- **Optional privileged helper.** Settings ▸ Advanced can install a small + signed launch daemon (`SMAppService`) so those same operations can + authenticate with Touch ID instead of a password-only prompt. It is + strictly opt-in and takes its own one-time macOS approval. If you install + it: + - **It grants no standing privilege.** Installing the helper authorizes + nothing, and there is no "authenticate once for this launch". You are + asked to authenticate for each privileged operation you start. + - **The one caveat, stated honestly:** the credential from that + authentication stays valid for a 10-second window, because it has to + survive the hop from the app to the helper. A second operation begun + inside that window would not prompt again. The window covers an + inter-process message, not a user changing their mind, and it is the + shortest value that lets the check work at all. The rest is enforced by + the right's own definition (`shared: false` keeps the credential out of + other processes, `allow-root: false` stops the root helper satisfying it + by itself), and each operation ID is served at most once so a captured + request cannot be replayed. + - **It cannot be asked to run anything else.** The helper accepts seven typed + operations — scan, clean, optimize, the optimize preview, flush DNS, + renew DHCP, and reading the Login Items list — and derives every command + line itself. There is no field in its API for a path, a shell string, or + an executable, so a caller that fully controls the message still cannot + express "run this". + - **The only value a caller supplies** is the network interface name for + renew DHCP. It is checked twice: against a strict `en0`-shaped pattern, + and against the interfaces that actually exist on the machine. A + well-formed name for an interface that isn't there is refused. + - **No shell.** The helper runs the bundled engine, plus exactly four + system tools by absolute path (`/usr/bin/dscacheutil`, `/usr/bin/killall`, + `/usr/sbin/ipconfig`, `/usr/bin/sfltool`), each as a separate process with + fixed arguments. + This is stricter than the path it replaces: flushing DNS previously + elevated `/bin/sh -c "dscacheutil -flushcache; killall -HUP mDNSResponder"`, + handing a command string to a root shell. + - **Only Burrow can talk to it.** The daemon pins its callers to Burrow's + bundle identifier and signing team via the XPC connection's code-signing + requirement, so another local process cannot reach it or use it to raise + a credential prompt. + - **It runs only your engine, or those four Apple tools.** The engine it + executes is the copy inside the app bundle, resolved relative to the + helper's own path, never through `PATH` or an environment variable. Before + running anything the helper verifies the whole app bundle against its own + signing team, which seals the engine and every library the engine loads. + - **You can remove it** from Settings, or from System Settings ▸ General ▸ + Login Items & Extensions. + - Code: `macos/Sources/PrivilegedHelper/`, `macos/HelperSources/`. - **Honest caveat:** official builds elevate the engine sealed inside the Developer ID signed app. A source build can fall back to an external `mo`; if it does, that executable is only as trustworthy as its install location diff --git a/macos/HelperSources/HelperService.swift b/macos/HelperSources/HelperService.swift new file mode 100644 index 00000000..df9a004f --- /dev/null +++ b/macos/HelperSources/HelperService.swift @@ -0,0 +1,469 @@ +// +// HelperService.swift +// BurrowHelper +// +// The root daemon. This process runs as uid 0, so every line here is written +// on the assumption that the thing talking to it is hostile until proven +// otherwise. +// +// ── The gauntlet a request runs ───────────────────────────────────────── +// Five gates, in this order, each of which fails CLOSED: +// +// 1. Connection the peer must satisfy the code requirement (Burrow, +// signed by our team), enforced by the SYSTEM via +// NSXPCListener.setConnectionCodeSigningRequirement +// before this process sees the connection at all. +// 2. Shape the payload must decode as a `HelperRequest`. The +// operation is an enum, so an unknown verb dies here. +// 3. Freshness the operation ID must be a UUID this daemon has never +// served. One authorization, one operation. +// 4. Authorization `AuthorizationCopyRights` must GRANT the right, which +// raises the system authentication prompt. This is the +// gate, not a GUI-side prompt. +// 5. Execution argv is derived from the enum, and the executable is +// the signed engine inside our own bundle, verified +// before it is spawned. +// +// Note what is absent: there is no path at which a caller-supplied string +// becomes part of a command line. That is the property the whole design +// exists to preserve. +// + +import Foundation +import Security +import os + +// MARK: - Logging +// +// Privileged code logs to the unified log, which is world-readable. So it +// records DECISIONS, never content: no paths, no filenames, no command +// output, no authentication material, no free-form error text. Operation IDs +// are UUIDs the daemon itself validated, and every other value logged here is +// drawn from a closed enum. + +let helperLog = Logger(subsystem: "dev.caezium.Burrow.privileged-helper", category: "privileged") + +/// Diagnostic trail that cannot silently disappear. +/// +/// The unified log produced NOTHING for this daemon across several runs — not +/// even the unconditional startup line — while the process was demonstrably +/// alive and serving Mach requests. A root daemon whose logging you can't +/// trust is a root daemon you can't debug, so every message is also written to +/// a file. +/// +/// That file is opened HERE rather than via `StandardErrorPath` in the launchd +/// plist. The plist route was tried first and was actively harmful: launchd +/// refused to exec the daemon at all, failing every spawn with EX_CONFIG, so +/// the attempt to gain observability destroyed the thing being observed — and +/// the symptom (a daemon that never runs and a 0-byte log) is indistinguishable +/// from a code-signing rejection. +/// +/// Opening it in-process inverts that failure mode: a path the daemon cannot +/// write costs diagnostics, never the daemon. +private let helperTraceHandle: FileHandle? = { + let path = "/Library/Logs/burrow-helper.log" + let fm = FileManager.default + if !fm.fileExists(atPath: path) { + // 0644 root-owned: readable for support, writable only by root, and in + // a directory unprivileged users cannot pre-seed with a symlink. + fm.createFile(atPath: path, contents: nil, + attributes: [.posixPermissions: 0o644]) + } + guard let handle = FileHandle(forWritingAtPath: path) else { return nil } + handle.seekToEndOfFile() + return handle +}() + +private let helperTraceLock = NSLock() + +func helperTrace(_ message: String) { + helperLog.notice("\(message, privacy: .public)") + guard let helperTraceHandle else { return } + let stamp = ISO8601DateFormatter().string(from: Date()) + helperTraceLock.lock(); defer { helperTraceLock.unlock() } + try? helperTraceHandle.write(contentsOf: Data("[\(stamp)] \(message)\n".utf8)) +} + +// MARK: - Engine resolution + +enum HelperEngine { + + /// The signed engine inside the app bundle that contains this helper, + /// resolved RELATIVE TO OUR OWN EXECUTABLE: + /// + /// …/Burrow.app/Contents/MacOS/BurrowHelper ← us + /// …/Burrow.app/Contents/Resources/engine/mole ← the engine + /// + /// Never `PATH`, never an environment variable, never a caller-supplied + /// path. A root process that resolves its executable through any of those + /// hands root to whoever wins the race to shadow the name — which is the + /// exact reason `MoleCLI.trustedExecutable()` already refuses PATH on the + /// osascript path. + static func bundledEnginePath() -> String? { + guard let executable = Bundle.main.executableURL?.resolvingSymlinksInPath() else { return nil } + let contents = executable // …/Contents/MacOS/BurrowHelper + .deletingLastPathComponent() // …/Contents/MacOS + .deletingLastPathComponent() // …/Contents + let engine = contents + .appendingPathComponent("Resources/engine/mole") + .standardizedFileURL + + // Belt and braces: after standardizing, the engine must still sit + // inside our own Contents directory. A symlink pointing out of the + // bundle would otherwise be followed as root. + guard engine.path.hasPrefix(contents.standardizedFileURL.path + "/") else { return nil } + guard FileManager.default.isExecutableFile(atPath: engine.path) else { return nil } + return engine.path + } + + /// The app bundle containing this helper. + static func appBundleURL() -> URL? { + guard let executable = Bundle.main.executableURL?.resolvingSymlinksInPath() else { return nil } + return executable + .deletingLastPathComponent() // …/Contents/MacOS + .deletingLastPathComponent() // …/Contents + .deletingLastPathComponent() // …/Burrow.app + .standardizedFileURL + } + + /// Verify our own app bundle before running anything out of it as root. + /// + /// This deliberately validates the BUNDLE, not the engine file. The engine + /// is a bash script — `codesign` reports "code object is not signed at + /// all" for it, and it sources a whole `lib/` directory that would each + /// need checking too. Running `SecStaticCodeCheckValidity` on the script + /// itself can never succeed, which is exactly the bug this replaces. + /// + /// Validating the app bundle is both achievable and stronger: a bundle's + /// signature seals `Contents/Resources/`, so a passing check certifies the + /// engine script AND every library it sources AND that all of it came from + /// our signing team. Any tampering breaks the seal and fails here — at the + /// moment it would gain root, not merely at install time. + /// + /// On an ad-hoc (local development) build there is no team to pin, so the + /// check is skipped and the fact is logged. Release builds always have + /// one, and the release gate refuses to ship a helper without it. + static func verifyContainingBundle(teamID: String?) -> Bool { + guard let teamID else { + helperTrace("bundle signature check skipped: helper is ad-hoc signed (development build)") + return true + } + guard let requirement = HelperCodeRequirement.sameTeam(teamID: teamID), + let bundle = appBundleURL() else { return false } + + var staticCode: SecStaticCode? + guard SecStaticCodeCreateWithPath(bundle as CFURL, [], &staticCode) == errSecSuccess, + let staticCode else { + helperTrace("bundle verification: could not read our own code signature") + return false + } + + var secRequirement: SecRequirement? + guard SecRequirementCreateWithString(requirement as CFString, [], &secRequirement) == errSecSuccess, + let secRequirement else { return false } + + // Default flags validate the resource seal, which is what covers the + // engine script sitting in Contents/Resources. + let status = SecStaticCodeCheckValidity(staticCode, [], secRequirement) + if status != errSecSuccess { + helperTrace("bundle verification failed: status=\(status)") + return false + } + return true + } +} + +// MARK: - Running one operation + +/// Spawns the engine and streams its output back to the client. +/// +/// The daemon owns the child directly, which is the structural gain over the +/// osascript path: there, cancelling meant killing `osascript` and orphaning +/// the root child it had spawned, so the streaming flow simply had no safe +/// cancel. Here the child is ours to signal and reap. +final class HelperOperationRunner: @unchecked Sendable { + private let lock = NSLock() + private var running: [String: Process] = [:] + + /// Run every step of `operation` in order, stopping at the first failure, + /// and return the exit status of the last step attempted. + /// + /// Multi-step exists because flushing DNS is genuinely two commands + /// (`dscacheutil -flushcache` then `killall -HUP mDNSResponder`). Running + /// them as two spawns rather than one `/bin/sh -c` string is the point: + /// no root shell, nothing to parse, nothing to inject into. + func run(operation: HelperOperation, + operationID: String, + interface: String?, + enginePath: String, + emit: @escaping (String) -> Void) -> Int32 { + var last: Int32 = 0 + for step in operation.steps(interface: interface) { + let path: String + switch step.executable { + case .bundledEngine: + path = enginePath + case .system(let systemPath): + // Re-check against the closed set at the moment of use, not + // only where the step was built. A future edit that + // constructs a step elsewhere still cannot introduce a new + // binary for root to run. + guard HelperSystemTool.all.contains(systemPath) else { + helperTrace("refused: step names an executable outside the permitted set") + return 126 + } + path = systemPath + } + last = runOne(path: path, arguments: step.arguments, + operationID: operationID, emit: emit) + guard last == 0 else { break } + } + return last + } + + private func runOne(path: String, + arguments: [String], + operationID: String, + emit: @escaping (String) -> Void) -> Int32 { + let process = Process() + process.executableURL = URL(fileURLWithPath: path) + // Fixed argv from the typed step. The only caller-derived value that + // can appear here is an interface name already validated against the + // machine's real interface list. + process.arguments = arguments + + // A deliberately minimal environment. The child is root, so anything + // inherited from the launchd context that could redirect a lookup + // (PATH, DYLD_*, the engine's own overrides) is dropped rather than + // passed through. + process.environment = [ + "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", + "HOME": NSHomeDirectory(), + "LC_ALL": "C", + ] + + let outPipe = Pipe(), errPipe = Pipe() + process.standardOutput = outPipe + process.standardError = errPipe + process.standardInput = FileHandle.nullDevice + + do { + try process.run() + } catch { + helperTrace("engine spawn failed for operation \(operationID)") + return 127 + } + + lock.lock(); running[operationID] = process; lock.unlock() + defer { lock.lock(); running.removeValue(forKey: operationID); lock.unlock() } + + try? outPipe.fileHandleForWriting.close() + try? errPipe.fileHandleForWriting.close() + + // One reader per pipe, both draining to EOF before the exit status is + // read, so no output can be lost between the last write and the reap. + let splitter = HelperLineSplitter() + let group = DispatchGroup() + for handle in [outPipe.fileHandleForReading, errPipe.fileHandleForReading] { + group.enter() + DispatchQueue.global(qos: .utility).async { + while case let chunk = handle.availableData, !chunk.isEmpty { + guard let text = String(data: chunk, encoding: .utf8) else { continue } + for line in splitter.ingest(text) { emit(line) } + } + group.leave() + } + } + group.wait() + for line in splitter.flush() { emit(line) } + + process.waitUntilExit() + return process.terminationStatus + } + + /// Terminate a running operation. Returns whether anything was running. + func cancel(operationID: String) -> Bool { + lock.lock(); let process = running[operationID]; lock.unlock() + guard let process, process.isRunning else { return false } + process.terminate() + return true + } + + var hasWork: Bool { + lock.lock(); defer { lock.unlock() } + return !running.isEmpty + } +} + +/// Buffers partial reads and emits whole lines. Mirrors the GUI's splitter so +/// both elevation routes deliver output the same way. +final class HelperLineSplitter: @unchecked Sendable { + private var buffer = "" + private let lock = NSLock() + + func ingest(_ text: String) -> [String] { + lock.lock(); defer { lock.unlock() } + buffer += text + var parts = buffer.components(separatedBy: "\n") + buffer = parts.removeLast() + return parts + } + + func flush() -> [String] { + lock.lock(); defer { lock.unlock() } + let rest = buffer + buffer = "" + return rest.isEmpty ? [] : [rest] + } +} + +// MARK: - The XPC service + +final class HelperService: NSObject, BurrowHelperProtocol { + private let replayGuard = HelperReplayGuard() + private let runner = HelperOperationRunner() + private let teamID: String? + + /// The client callback for the connection currently being served. Set by + /// the listener delegate per connection. + weak var currentConnection: NSXPCConnection? + + init(teamID: String?) { + self.teamID = teamID + super.init() + } + + var isIdle: Bool { !runner.hasWork } + + /// Every network interface that actually exists on this machine. + /// + /// `renewDHCP` carries the only caller-supplied value that reaches argv, + /// so a plausible-looking name is not enough — it must name a real + /// interface. `getifaddrs` is the kernel's own list, so there is nothing + /// for a caller to influence. + static func liveInterfaceNames() -> Set { + var addresses: UnsafeMutablePointer? + guard getifaddrs(&addresses) == 0, let first = addresses else { return [] } + defer { freeifaddrs(addresses) } + + var names: Set = [] + var cursor: UnsafeMutablePointer? = first + while let current = cursor { + names.insert(String(cString: current.pointee.ifa_name)) + cursor = current.pointee.ifa_next + } + return names + } + + /// This helper's own build, baked into the binary's embedded Info.plist + /// section at compile time — so it reports what it IS, not what the app + /// bundle around it currently claims to be. + static var build: String { + Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "" + } + + func helperBuild(withReply reply: @escaping (String) -> Void) { + reply(Self.build) + } + + func cancelOperation(operationID: String, withReply reply: @escaping (Bool) -> Void) { + // Cancellation stops work; it never starts any, so it needs no + // authorization of its own. The connection gate has already + // established that the caller is Burrow, and an ID it doesn't know + // simply isn't running. + guard UUID(uuidString: operationID) != nil else { return reply(false) } + reply(runner.cancel(operationID: operationID)) + } + + func execute(requestData: Data, authorization: Data, withReply reply: @escaping (Data) -> Void) { + func respond(_ outcome: HelperResponse.Outcome) { + let encoded = (try? JSONEncoder().encode(HelperResponse(outcome: outcome))) ?? Data() + reply(encoded) + } + + // Gate 2 — shape. An unknown operation cannot survive decoding. + guard let request = try? JSONDecoder().decode(HelperRequest.self, from: requestData) else { + helperTrace("request refused: malformed payload") + return respond(.rejected(.malformedPayload)) + } + + // The interface name is checked against the machine's REAL interfaces, + // not just a character shape — a well-formed name for an interface + // that doesn't exist is still refused. + if let rejection = request.validate(expectedBuild: Self.build, + liveInterfaces: HelperService.liveInterfaceNames()) { + helperTrace("request refused: \(rejection.rawValue)") + return respond(.rejected(rejection)) + } + + // Gate 3 — freshness. One authorization buys exactly one operation, so + // a captured payload cannot be replayed for a second root run. + guard replayGuard.admit(request.operationID) else { + helperTrace("request refused: replayed operation ID") + return respond(.rejected(.replayedOperationID)) + } + + // Gate 4 — authorization. This is what raises the prompt, and it + // happens HERE, in the privileged process, on every single operation. + helperTrace("authorizing \(request.operation.rawValue): calling AuthorizationCopyRights") + let decision = HelperAuthorization.authorize(externalForm: authorization) + guard decision.permitsExecution else { + helperTrace("NOT authorized: \(decision.diagnostic)") + switch decision.outcome { + case .cancelled: return respond(.authorizationCancelled) + default: return respond(.authorizationDenied) + } + } + helperTrace("authorized: \(decision.diagnostic)") + + // Gate 5 — execution. Our own signed engine, or a system tool from the + // closed set; fixed argv either way. + guard let enginePath = HelperEngine.bundledEnginePath() else { + helperTrace("engine unavailable: no bundled engine at Contents/Resources/engine/mole") + return respond(.engineUnavailable) + } + guard HelperEngine.verifyContainingBundle(teamID: teamID) else { + helperTrace("engine unavailable: containing app bundle failed signature verification") + return respond(.engineUnavailable) + } + + helperTrace("running \(request.operation.rawValue) (mutating: \(request.operation.mutatesDisk))") + + let client = currentConnection?.remoteObjectProxy as? BurrowHelperClientProtocol + let operationID = request.operationID + let code = runner.run(operation: request.operation, + operationID: operationID, + interface: request.networkInterface, + enginePath: enginePath) { line in + client?.helperDidEmit(line: line, operationID: operationID) + } + helperTrace("operation finished with status \(code)") + respond(.exited(code)) + } +} + +// MARK: - Connection gate + +/// Gate 1. By the time this delegate runs, the system has ALREADY evaluated +/// the code requirement set on the listener against the connecting peer — see +/// `HelperMain.start`. A caller that isn't Burrow, signed by our team, never +/// reaches this method at all. +final class HelperListenerDelegate: NSObject, NSXPCListenerDelegate { + private let service: HelperService + + init(service: HelperService) { + self.service = service + super.init() + } + + func listener(_ listener: NSXPCListener, + shouldAcceptNewConnection connection: NSXPCConnection) -> Bool { + connection.exportedInterface = HelperInterface.daemon() + connection.exportedObject = service + connection.remoteObjectInterface = HelperInterface.client() + service.currentConnection = connection + connection.resume() + helperTrace("connection accepted from a verified Burrow client") + return true + } +} diff --git a/macos/HelperSources/main.swift b/macos/HelperSources/main.swift new file mode 100644 index 00000000..3da05ffd --- /dev/null +++ b/macos/HelperSources/main.swift @@ -0,0 +1,97 @@ +// +// main.swift +// BurrowHelper +// +// Entry point for the root daemon. launchd starts this on demand when +// something connects to the Mach service; it publishes its own authorization +// right, pins who may connect, serves requests, and exits when idle. +// +// Nothing here does privileged work. The gauntlet every request runs is in +// HelperService.swift. +// + +import Foundation +import ServiceManagement + +enum HelperMain { + + /// How long the daemon lingers with no work before exiting. + /// + /// A root process should not be resident for longer than it is useful. + /// launchd restarts it on the next connection at no cost to the user, so + /// the only thing a long idle life buys is a larger window in which a root + /// process exists to be attacked. + static let idleTimeout: TimeInterval = 120 + + static func start() -> Never { + // Traced stage by stage. When this daemon last misbehaved there was no + // way to tell "it never started" from "it started and the + // authorization failed", which cost more time than the bug did. + helperTrace("startup: entered main") + + // The team that signed US. Learned at runtime, so no team ID, + // certificate, or developer name is ever hardcoded in the repository, + // and the check survives certificate renewal. + let teamID = HelperCodeRequirement.selfTeamIdentifier() + let requirement = HelperCodeRequirement.string(bundleID: HelperNames.clientBundleID, + teamID: teamID) + helperTrace("startup: signing team \(teamID == nil ? "absent (ad-hoc)" : "present")") + + if teamID == nil { + // Ad-hoc: local development only. Say so loudly — a release build + // always has a team, and the release gate refuses to ship without + // one, so seeing this in the wild means something is wrong. + helperTrace("running ad-hoc signed: caller pinning is identifier-only (development build)") + } + + // Publish the right's definition. Rewritten every launch on purpose: + // the compiled-in definition is the source of truth, so a stale or + // tampered policy entry from an earlier install cannot weaken a newer + // helper. Failure is not fatal — the right still evaluates, just with + // the system's default wording. + helperTrace("startup: publishing authorization right") + if !HelperAuthorization.installRightDefinition() { + helperTrace("startup: could NOT publish the authorization right definition") + } + + let service = HelperService(teamID: teamID) + let delegate = HelperListenerDelegate(service: service) + let listener = NSXPCListener(machServiceName: HelperNames.machService) + + // Gate 1, enforced by the system before our delegate ever runs. The + // supported alternative to hand-rolling an audit-token check — see + // HelperCodeRequirement for why the PID-based version of this is a + // textbook vulnerability. + listener.setConnectionCodeSigningRequirement(requirement) + + listener.delegate = delegate + listener.resume() + + helperTrace("helper \(HelperService.build) listening on \(HelperNames.machService)") + + // Idle exit. Checked on a timer rather than tied to connection + // teardown so a client that opens a connection and then goes silent + // cannot pin a root process open indefinitely. + var idleSince = Date() + let timer = DispatchSource.makeTimerSource(queue: .global(qos: .utility)) + timer.schedule(deadline: .now() + 10, repeating: 10) + timer.setEventHandler { + if service.isIdle { + if Date().timeIntervalSince(idleSince) >= idleTimeout { + helperTrace("idle timeout reached; exiting") + exit(0) + } + } else { + idleSince = Date() + } + } + timer.resume() + + RunLoop.main.run() + // RunLoop.main.run() does not return; this keeps the Never signature + // honest for the compiler. + exit(0) + } +} + +HelperMain.start() diff --git a/macos/Resources/Info.plist b/macos/Resources/Info.plist index d972ef44..78ba4496 100644 --- a/macos/Resources/Info.plist +++ b/macos/Resources/Info.plist @@ -17,9 +17,9 @@ CFBundlePackageType APPL CFBundleShortVersionString - 0.11.2 + 0.12.0 CFBundleVersion - 23 + 24 ITSAppUsesNonExemptEncryption LSUIElement diff --git a/macos/Resources/LaunchDaemons/dev.caezium.Burrow.privileged-helper.plist b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.privileged-helper.plist new file mode 100644 index 00000000..37d6af0a --- /dev/null +++ b/macos/Resources/LaunchDaemons/dev.caezium.Burrow.privileged-helper.plist @@ -0,0 +1,62 @@ + + + + + + Label + dev.caezium.Burrow.privileged-helper + + BundleProgram + Contents/MacOS/BurrowHelper + + MachServices + + dev.caezium.Burrow.privileged-helper + + + + + AssociatedBundleIdentifiers + + dev.caezium.Burrow + + + + + + + + diff --git a/macos/Sources/Connectivity.swift b/macos/Sources/Connectivity.swift index 88f524b6..440dc28c 100644 --- a/macos/Sources/Connectivity.swift +++ b/macos/Sources/Connectivity.swift @@ -215,21 +215,50 @@ enum Connectivity { /// Run a fix with a single admin prompt. Blocks on the auth dialog — call /// off the main thread. Returns a user-facing result. static func run(_ fix: Fix, interface: String?) -> (ok: Bool, message: String) { - let broker = SystemPrivilegeBroker() + let iface = interface ?? "en0" switch fix { case .flushDNS: - let r = broker.openElevated(executable: "/bin/sh", - args: ["-c", "dscacheutil -flushcache; killall -HUP mDNSResponder"]) - return classify(r, ok: NSLocalizedString("DNS cache flushed.", comment: ""), + return classify(elevate(.flushDNS), + ok: NSLocalizedString("DNS cache flushed.", comment: ""), fail: NSLocalizedString("Couldn't flush the DNS cache.", comment: "")) case .renewDHCP: - let iface = interface ?? "en0" - let r = broker.openElevated(executable: "/usr/sbin/ipconfig", args: ["set", iface, "DHCP"]) - return classify(r, ok: String(format: NSLocalizedString("Renewed the DHCP lease on %@.", comment: ""), iface), + return classify(elevate(.renewDHCP, interface: iface), + ok: String(format: NSLocalizedString("Renewed the DHCP lease on %@.", comment: ""), iface), fail: NSLocalizedString("Couldn't renew the DHCP lease.", comment: "")) } } + /// Prefer the privileged helper, fall back to the osascript broker. + /// + /// Through the helper these are TYPED operations: the daemon holds the + /// argv and the interface name is validated against the machine's real + /// interfaces before it reaches a process. That removes the root shell + /// this path used to need — flushing DNS was previously + /// `/bin/sh -c "dscacheutil -flushcache; killall -HUP mDNSResponder"` + /// running as root, with a command string for the shell to parse. + /// + /// The fallback keeps that older shape, unchanged, for anyone who hasn't + /// installed the helper. + private static func elevate(_ operation: HelperOperation, + interface: String? = nil) -> ElevatedOutcome { + let client = PrivilegedHelperClient.shared + if client.registrationStatus == .enabled, client.versionSkew() == .matched { + return client.run(operation: operation, interface: interface) { _ in } + } + + let broker = SystemPrivilegeBroker() + switch operation { + case .flushDNS: + return broker.openElevated(executable: "/bin/sh", + args: ["-c", "dscacheutil -flushcache; killall -HUP mDNSResponder"]) + case .renewDHCP: + return broker.openElevated(executable: "/usr/sbin/ipconfig", + args: ["set", interface ?? "en0", "DHCP"]) + default: + return .launchFailed + } + } + private static func classify(_ outcome: ElevatedOutcome, ok: String, fail: String) -> (ok: Bool, message: String) { switch outcome { case .exited(0): return (true, ok) diff --git a/macos/Sources/MoEngine.swift b/macos/Sources/MoEngine.swift index 839dcf9d..530a969b 100644 --- a/macos/Sources/MoEngine.swift +++ b/macos/Sources/MoEngine.swift @@ -15,8 +15,8 @@ // holds the `any ProcessPort` to drive its own reduce/notify/auth-cancel loop, // so the facade hands it the production port rather than the stream. The // one-shot ELEVATED path is NOT on the facade — it stays in -// `MoleCLI.runElevatedClassified` (trusted-location resolution + the shared -// `PrivilegeBroker`), the path production has always used. +// `SystemPrivilegeBroker.openElevated` (see `Connectivity.run`), the path +// production has always used. // // Behavior is preserved exactly: a `capture(_:)` call produces the same argv, // stdin, environment, timeout, and result fields that `MoleCLI.run` did, and @@ -129,7 +129,11 @@ final class MoEngine { init(processPort: MoleProcessPort = SystemMoleProcess(), locator: MoLocator = SystemMoLocator(), - streamPort: ProcessPort = SystemProcessPort(), + // Elevated streaming runs prefer the privileged helper when it is + // registered, approved, and build-matched; everything else — and any + // elevated run the helper doesn't recognise — falls through to the + // osascript port unchanged. See `PrivilegeRoute`. + streamPort: ProcessPort = HelperAwareProcessPort(), makePTY: @escaping @Sendable () -> PTYPort = { PTYTask() }) { self.processPort = processPort self.locator = locator diff --git a/macos/Sources/MoleCLI.swift b/macos/Sources/MoleCLI.swift index de69ee3e..04595caa 100644 --- a/macos/Sources/MoleCLI.swift +++ b/macos/Sources/MoleCLI.swift @@ -248,12 +248,6 @@ enum MoleCLI { /// a fake (reset in `tearDown`). Test-only seam — not a configuration point. internal static var processPort: MoleProcessPort = SystemMoleProcess() - /// The one-shot elevated runner (issue #48). Production spawns real - /// osascript via `SystemPrivilegeBroker`; tests inject a fake so the - /// build-the-osascript-spec quoting + auth-cancel classification run in - /// memory with no auth dialog. Test-only seam — reset in `tearDown`. - internal static var privilegeBroker: PrivilegeBroker = SystemPrivilegeBroker() - /// Run an executable with the given args, capturing stdout + stderr. /// Blocks until the process exits — callers are responsible for /// running this on a background queue. Times out after `timeout` @@ -286,30 +280,15 @@ enum MoleCLI { ) } - /// Run `mo ` ONCE with administrator rights via the macOS auth - /// dialog. That dialog is PASSWORD-ONLY: the `system.privilege.admin` - /// right authenticates through SecurityAgent's classic mechanism, which - /// never offers Touch ID — pam_tid (`mo touchid`) covers terminal - /// `sudo`, not this path. Blocking — call off the main thread. For - /// one-shot privileged config like `touchid enable/disable`, not for - /// streamed jobs (OperationFlow does those). - /// - /// The spawn now goes through `PrivilegeBroker` so the osascript quoting - /// and auth-cancel classification are testable in memory (issue #48); the - /// `Int32` return is preserved for existing callers that only branch on - /// "did it work" (a dismissed prompt collapses to a nonzero code, exactly - /// as before). New callers that want the named outcome use - /// `runElevatedClassified`. - static func runElevated(args: [String]) -> Int32 { - runElevatedClassified(args: args).exitCode - } - - /// As `runElevated`, but returns the classified outcome — `.authCancelled` - /// for a dismissed prompt is distinguished from a command that ran and - /// failed, so callers can show the right message without re-deriving the - /// "nonzero might mean cancel" heuristic themselves. - static func runElevatedClassified(args: [String]) -> ElevatedOutcome { - guard let mo = trustedExecutable() else { return .launchFailed } - return privilegeBroker.openElevated(executable: mo, args: args) - } + // NOTE: `runElevated` / `runElevatedClassified` used to live here — a + // one-shot "run `mo` as root with these args" entry point. Their only + // caller was the `mo touchid enable/disable` setting, and they were + // deleted with it: an unused function that takes arbitrary argv and runs + // it as root is exactly the kind of thing that shouldn't sit around + // waiting for a caller. + // + // The elevation machinery itself is still very much alive — the streaming + // path (`OperationFlow.SystemProcessPort`) uses `elevatedScript` above, + // and Connectivity's flush-DNS / renew-DHCP fixes call + // `SystemPrivilegeBroker.openElevated` directly. } diff --git a/macos/Sources/PrivilegeBroker.swift b/macos/Sources/PrivilegeBroker.swift index 98f24c04..31159dae 100644 --- a/macos/Sources/PrivilegeBroker.swift +++ b/macos/Sources/PrivilegeBroker.swift @@ -15,8 +15,13 @@ // and the auth-cancel classification IN MEMORY — no auth dialog, no sudo. // // Streamed elevated runs stay in OperationFlow's SystemProcessPort (output -// tailed from a temp log); this seam covers the one-shot config commands -// (`mo touchid enable/disable`) where the only signal is the exit status. +// tailed from a temp log); this seam covers one-shot commands where the only +// signal is the exit status. +// +// Live caller: `Connectivity.run` (flush DNS / renew DHCP), which constructs +// `SystemPrivilegeBroker` directly. The `MoleCLI.runElevated` wrapper that +// used to sit in front of this was deleted along with the `mo touchid` +// setting, its only user. // import Foundation diff --git a/macos/Sources/PrivilegedHelper/HelperAuthorization.swift b/macos/Sources/PrivilegedHelper/HelperAuthorization.swift new file mode 100644 index 00000000..7fa54378 --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperAuthorization.swift @@ -0,0 +1,352 @@ +// +// HelperAuthorization.swift +// Burrow / BurrowHelper (shared) +// +// The authorization policy for every root operation, in one place. +// +// ── The product decision this file encodes ────────────────────────────── +// Every operation that actually runs as root requires a FRESH user +// authentication. No grace period, no cached credential, no "authenticate +// once for this launch". Installing the helper is a separate, one-time +// macOS approval and does NOT authorize any later operation. +// +// Two words of that are literally keys in the right's definition: +// timeout: 0 the credential expires immediately, so the next +// privileged request authenticates again from scratch +// shared: false the credential is never visible to another process or +// satisfiable by a different right +// +// ── Where the prompt is raised, and why it moved ──────────────────────── +// This originally had the ROOT DAEMON raise the prompt, on the reasoning that +// a GUI-side prompt is cosmetic: the daemon would be trusting an +// unauthenticated message, and a caller that skipped the prompt would be +// indistinguishable from one that passed it. +// +// That reasoning is right about what must be VERIFIED, and wrong about where +// the prompt can be RAISED. Measured behaviour: the daemon reached +// AuthorizationCopyRights and every operation came back not-authorized. A +// launchd system daemon has no session to draw an authentication UI in, so +// asking it to raise one fails no matter how the right is defined. +// +// So the flow is now Apple's documented split, which puts the prompt where a +// session exists and the CHECK where the privilege is: +// +// 1. GUI — AuthorizationCreate, then AuthorizationCopyRights for +// `rightName` with interaction allowed. THIS raises the system +// prompt, in the user's own session, where SecurityAgent can +// offer Touch ID and fall back to the password. +// 2. GUI — AuthorizationMakeExternalForm; the bytes ride with the request. +// 3. Root — AuthorizationCreateFromExternalForm, then +// AuthorizationCopyRights WITHOUT interaction. This does not +// prompt; it asks the Security framework whether this reference +// genuinely holds the right. +// +// Step 3 is what makes step 1 more than decoration. The daemon never trusts +// the client's word: the credential lives in the security session, not in the +// message, so a caller that skipped the prompt produces a reference that +// fails step 3. Forging it means forging a Security-framework credential, not +// editing an XPC payload. +// +// ── The cost, stated plainly ──────────────────────────────────────────── +// The credential must survive the hop from step 1 to step 3, so `timeout` +// cannot be 0. It is set to the smallest value that leaves room for the XPC +// round trip. Within that window a SECOND operation could be authorized by +// the first authentication. +// +// What that does NOT open: `HelperReplayGuard` still serves each operation ID +// once, so a captured payload cannot be resent, and `shared: false` keeps the +// credential out of other processes. What it DOES open: an app that has +// already authenticated could start another operation within the window +// without a second prompt. That is a real relaxation of "a fresh +// authentication for every root operation", and it is the price of the +// prompt working at all. +// +// The authentication UI is the system's, so it offers Touch ID where the +// hardware has it and falls back to the Mac login password otherwise. Burrow +// receives a granted/denied/cancelled result and never sees, handles, or +// stores a credential. +// +// ── The two classic ways to make this a no-op ─────────────────────────── +// Both are real, both are one line, both are covered by tests: +// * calling AuthorizationCopyRights with a NULL rights set — it returns +// success regardless of who is calling; +// * passing kAuthorizationFlagPreAuthorize on the daemon side — that asks +// whether authorization would be POSSIBLE later, which is not the same +// as requiring it now. +// + +import Foundation +import Security + +enum HelperAuthorization { + + // MARK: - The right + + /// Namespaced under the app's bundle identifier so it can never collide + /// with, or be satisfied by, a system right the user may already hold for + /// something else. + static let rightName = "dev.caezium.Burrow.privileged-operation" + + /// The right's definition in the system authorization policy database. + /// + /// `allow-root: false` is the entry that matters most and is the easiest + /// to get wrong. The daemon evaluating this right IS root; if root callers + /// were allowed to satisfy it implicitly, the daemon would authorize + /// itself and the prompt would quietly stop appearing. + /// The window, in seconds, in which the credential from the GUI's + /// authentication is still valid when the daemon checks it. + /// + /// This exists only to cover one XPC round trip. It is NOT a convenience + /// grace period, and it is deliberately far too short to span a user + /// deciding to run a second operation by hand — but see the header: within + /// it, a programmatic second operation would not re-prompt. + static let credentialWindowSeconds = 10 + + static let rightDefinition: [String: Any] = [ + "class": "user", + "group": "admin", + "authenticate-user": true, + // Just long enough for the authenticated reference to reach the daemon + // and be checked. `timeout: 0` is the ideal and was tried first: it + // kills the credential before it crosses XPC, so the daemon's check + // always fails. + "timeout": credentialWindowSeconds, + "shared": false, + // Root is not a shortcut past the prompt. + "allow-root": false, + // Not the console user's session by default — an administrator has to + // authenticate explicitly. + "session-owner": false, + "comment": "Burrow is about to run a privileged maintenance operation (scan, clean, or optimize) as root.", + ] + + // MARK: - Flags + + /// The daemon's `AuthorizationCopyRights` flags. + /// + /// `extendRights` is mandatory — without it the call reports whether the + /// right COULD be granted and grants nothing, which a careless caller + /// reads as success. + /// + /// `interactionAllowed` is deliberately ABSENT. The daemon must never + /// prompt: it has no session to draw in (which is what broke the previous + /// design), and more importantly a daemon that can prompt is a daemon that + /// can be made to prompt by anything that reaches it. Without the flag + /// this call is a pure question — does this reference hold the right — + /// and an unauthenticated reference simply fails. + static let daemonFlags: AuthorizationFlags = [.extendRights] + + /// The client's flags. This is where the human is asked. + /// + /// `interactionAllowed` raises the system prompt in the user's own + /// session, so SecurityAgent can offer Touch ID and fall back to the + /// password. `preAuthorize` is what makes the credential available to the + /// daemon's later check rather than only to this process. + static let clientFlags: AuthorizationFlags = [.extendRights, .interactionAllowed, .preAuthorize] + + // MARK: - Outcome + + /// Conforms to `Error` so the client can carry a refusal through a + /// `Result` without inventing a parallel error type that would drift. + enum Outcome: Equatable, Sendable, Error { + case granted + case denied + case cancelled + case failed(OSStatus) + + /// The ONE predicate the daemon branches on. Kept as a single property + /// so "granted" can never drift into "not an outright failure". + var permitsExecution: Bool { self == .granted } + } + + /// Classify the `OSStatus` from `AuthorizationCopyRights`. Pure, so it is + /// exhaustively table-tested. Anything unrecognised fails CLOSED, carrying + /// the raw status for diagnosis rather than guessing. + static func outcome(from status: OSStatus) -> Outcome { + switch status { + case errAuthorizationSuccess: + return .granted + case errAuthorizationCanceled: + return .cancelled + case errAuthorizationDenied, OSStatus(errAuthorizationInteractionNotAllowed): + return .denied + default: + return .failed(status) + } + } + + // MARK: - External form + + /// `AuthorizationExternalForm` is a fixed-size C struct. Anything of a + /// different length is malformed and is refused HERE, before the bytes are + /// handed to the Security framework — a hostile client should not get to + /// choose the length of a buffer that lands in a C API. + static func isPlausibleExternalForm(_ data: Data) -> Bool { + data.count == MemoryLayout.size + } + + // MARK: - System witnesses + // + // Thin wrappers over the Security framework: the decisions above are pure + // and tested, these just perform the calls. + + /// What the GUI's authentication attempt produced, and — critically — the + /// owner of the underlying `AuthorizationRef`. + /// + /// The externalized form is NOT a self-contained token. It is a handle to + /// an authorization instance living in the Security Server, and that + /// instance only exists while the creating process holds its + /// `AuthorizationRef`. Free the ref and the daemon's + /// `AuthorizationCreateFromExternalForm` fails with `errAuthorizationDenied` + /// — which reads exactly like the user being refused, and sent this + /// implementation chasing the wrong layer entirely. + /// + /// So this is a class, not a struct: it owns the ref and releases it in + /// `deinit`. Callers must keep it alive across the whole round trip, which + /// `PrivilegedHelperClient.run` does with `withExtendedLifetime`. + final class ClientAuthorization { + let externalForm: Data + private let ref: AuthorizationRef + + init(ref: AuthorizationRef, externalForm: Data) { + self.ref = ref + self.externalForm = externalForm + } + + deinit { + // No `kAuthorizationFlagDestroyRights`: the credential's lifetime + // is the right's `timeout`, not ours to cut short — and cutting it + // short here is precisely the bug this type exists to prevent. + AuthorizationFree(ref, []) + } + } + + /// GUI side. Ask the user to authenticate, then externalize the resulting + /// reference so the daemon can verify it. + /// + /// THIS is the call that shows the prompt. It blocks until the user + /// authenticates or dismisses, so it must not run on the main thread. + /// + /// Returns `nil` on cancellation or failure, and the caller must then + /// abandon the operation — never fall through to sending an + /// unauthenticated request, which the daemon would reject anyway but which + /// would blur the distinction between "declined" and "broken". + static func authenticate() -> Result { + var reference: AuthorizationRef? + guard AuthorizationCreate(nil, nil, [], &reference) == errAuthorizationSuccess, + let ref = reference else { + return .failure(.failed(errAuthorizationInternal)) + } + + // Freed on every failure path, and ONLY on failure. On success the ref + // is handed to ClientAuthorization, which must outlive the daemon's + // internalization of the external form — see that type. + func fail(_ outcome: Outcome) -> Result { + AuthorizationFree(ref, []) + return .failure(outcome) + } + + // Non-empty rights array. Passing NULL here is the documented way to + // accidentally authorize everybody. + let status: OSStatus = rightName.withCString { name -> OSStatus in + var item = AuthorizationItem(name: name, valueLength: 0, value: nil, flags: 0) + return withUnsafeMutablePointer(to: &item) { itemPointer -> OSStatus in + var rights = AuthorizationRights(count: 1, items: itemPointer) + return AuthorizationCopyRights(ref, &rights, nil, clientFlags, nil) + } + } + let result = outcome(from: status) + guard result.permitsExecution else { return fail(result) } + + var form = AuthorizationExternalForm() + guard AuthorizationMakeExternalForm(ref, &form) == errAuthorizationSuccess else { + return fail(.failed(errAuthorizationInternal)) + } + return .success(ClientAuthorization(ref: ref, + externalForm: withUnsafeBytes(of: &form) { Data($0) })) + } + + /// Which step produced the result. Kept because "authorization failed" was + /// indistinguishable across three very different causes — a malformed + /// payload, a reference the Security framework wouldn't rebuild, and an + /// actual refusal by the user — and only the last of those is normal. + enum Stage: String, Sendable { + case malformedExternalForm + case createFromExternalForm + case copyRights + } + + struct Decision: Sendable { + let outcome: Outcome + let stage: Stage + /// The raw `OSStatus`, preserved even when the outcome collapses + /// several codes into `.denied`. + let status: OSStatus + + var permitsExecution: Bool { outcome.permitsExecution } + + /// Safe to log: a stage name from a closed enum plus a numeric status. + /// No paths, no credentials, no free-form error text. + var diagnostic: String { "\(stage.rawValue) status=\(status) outcome=\(outcome)" } + } + + /// Daemon side. Rebuild the client's reference and check that it genuinely + /// holds `rightName`. Does NOT prompt — see `daemonFlags`. The returned + /// decision is the gate: only `.granted` may be followed by privileged + /// work. + /// + /// The rights array is always non-empty — passing NULL here is the + /// documented way to accidentally authorize everybody. + static func authorize(externalForm data: Data) -> Decision { + guard isPlausibleExternalForm(data) else { + return Decision(outcome: .denied, stage: .malformedExternalForm, status: errAuthorizationInvalidRef) + } + + var form = AuthorizationExternalForm() + let copied: Bool = withUnsafeMutableBytes(of: &form) { raw -> Bool in + guard raw.count == data.count else { return false } + _ = data.copyBytes(to: raw.bindMemory(to: UInt8.self)) + return true + } + guard copied else { + return Decision(outcome: .denied, stage: .malformedExternalForm, status: errAuthorizationInvalidRef) + } + + var ref: AuthorizationRef? + let restored = AuthorizationCreateFromExternalForm(&form, &ref) + guard restored == errAuthorizationSuccess, let ref else { + return Decision(outcome: .denied, stage: .createFromExternalForm, status: restored) + } + defer { AuthorizationFree(ref, []) } + + return rightName.withCString { name -> Decision in + var item = AuthorizationItem(name: name, valueLength: 0, value: nil, flags: 0) + return withUnsafeMutablePointer(to: &item) { itemPointer -> Decision in + var rights = AuthorizationRights(count: 1, items: itemPointer) + let status = AuthorizationCopyRights(ref, &rights, nil, daemonFlags, nil) + return Decision(outcome: outcome(from: status), stage: .copyRights, status: status) + } + } + } + + /// Daemon side, once at startup. Publish the right's definition so the + /// system prompts with Burrow's own wording and policy instead of falling + /// back to a generic default for an unknown right. + /// + /// Rewritten every launch on purpose: the daemon's compiled-in definition + /// is the source of truth, so a stale or tampered policy entry from an + /// earlier install cannot weaken a newer helper. + @discardableResult + static func installRightDefinition() -> Bool { + var authRef: AuthorizationRef? + guard AuthorizationCreate(nil, nil, [], &authRef) == errAuthorizationSuccess, + let authRef else { return false } + defer { AuthorizationFree(authRef, []) } + + let comment = (rightDefinition["comment"] as? String).map { $0 as CFString } + let status = AuthorizationRightSet(authRef, rightName, + rightDefinition as CFDictionary, + comment, nil, nil) + return status == errAuthorizationSuccess + } +} diff --git a/macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift b/macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift new file mode 100644 index 00000000..412f4e7c --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperCodeRequirement.swift @@ -0,0 +1,155 @@ +// +// HelperCodeRequirement.swift +// Burrow / BurrowHelper (shared) +// +// Who may talk to the root daemon at all. +// +// Authorization (HelperAuthorization) answers "may this operation run". +// This answers the question that comes first: "is the process on the other +// end of this XPC connection actually Burrow". Both are required, and the +// order matters — without this check, any local process could open the Mach +// service and drive the authentication prompt, phishing the user for +// administrator credentials behind a dialog the system itself renders and +// that therefore looks entirely legitimate. +// +// ── Identifying the peer correctly ────────────────────────────────────── +// The requirement built here is handed to +// `NSXPCListener.setConnectionCodeSigningRequirement`, so the SYSTEM +// evaluates it against the connecting peer before the delegate is ever +// called. That matters more than it looks: +// +// The obvious hand-rolled check — read `connection.processIdentifier`, look +// the process up, verify its signature — is the classic vulnerable pattern. +// A PID can be recycled between being read and being checked, and the +// standard exploit hands the helper the PID of a legitimate app and then +// races a hostile process into that slot. Doing it properly means the audit +// token, and reaching an NSXPCConnection's audit token means private API, +// which is not a dependency worth taking inside a root daemon. +// +// The macOS 13 requirement API sidesteps both problems: it is supported, and +// the evaluation happens in the kernel against the real peer, with no window +// between check and use. +// +// ── Nothing personal is hardcoded ─────────────────────────────────────── +// The requirement is assembled at RUNTIME from the daemon's own signing +// information: the helper asks what team signed IT, and demands the caller be +// the Burrow app signed by that same team. No team ID, certificate, or +// developer name appears in this repository, and the check keeps working +// across certificate renewals. +// + +import Foundation +import Security + +enum HelperCodeRequirement { + + /// A syntactically valid requirement that no code can satisfy, used + /// whenever a safe requirement cannot be built. + /// + /// It is deliberately not the empty string: an empty requirement fails to + /// PARSE, and several Security APIs treat a parse failure as "no + /// requirement given", which is the exact opposite of failing closed. This + /// one parses fine and simply matches nothing, because no bundle + /// identifier may contain a space. + static let unsatisfiable = #"identifier "dev.caezium.Burrow.no such caller""# + + // MARK: - Input validation + // + // The requirement string is a small language parsed by the Security + // framework, and identifiers are interpolated into it. A value containing + // a quote could close the literal early and append its own clause — + // `identifier "x" or anchor apple generic` would admit every Apple-signed + // process on the machine. + // + // Hostile values are REFUSED rather than escaped. A legitimate bundle + // identifier or team ID is drawn from a small alphabet, so rejecting + // anything outside it costs nothing and removes a whole class of parser + // subtleties — there is no "correctly escaped" case left to get wrong. + + /// The characters a real bundle identifier or team ID is built from. + private static let permitted = CharacterSet(charactersIn: + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789.-_") + + /// The value if it can ride inertly inside a requirement string, else nil. + static func validated(identifier: String) -> String? { + guard !identifier.isEmpty, identifier.count <= 255 else { return nil } + guard identifier.unicodeScalars.allSatisfy({ permitted.contains($0) }) else { return nil } + return identifier + } + + // MARK: - Building the requirement + + /// The designated requirement a caller must satisfy. + /// + /// With a team ID, three clauses are pinned and every one is mandatory: + /// * `identifier` — this exact bundle, not merely something of ours; + /// * `anchor apple generic` — a chain terminating at Apple, so a + /// self-signed build cannot claim the identifier; + /// * `certificate leaf[subject.OU]` — signed by the same team as the + /// helper, so another developer's notarized app cannot impersonate us. + /// + /// Without a team ID (a local ad-hoc Debug build) it falls back to the + /// identifier alone so development works. That is explicitly NOT + /// distribution grade, and `isDistributionGrade` is what the release gate + /// checks so an ad-hoc helper can never ship. + static func string(bundleID: String, teamID: String?) -> String { + guard let identifier = validated(identifier: bundleID) else { return unsatisfiable } + + guard let teamID, !teamID.trimmingCharacters(in: .whitespaces).isEmpty else { + return #"identifier "\#(identifier)""# + } + guard let team = validated(identifier: teamID) else { return unsatisfiable } + + return #"identifier "\#(identifier)" and anchor apple generic and certificate leaf[subject.OU] = "\#(team)""# + } + + /// Whether a requirement built from this team is strong enough to ship. + /// An ad-hoc helper has no team, so it can be impersonated by any local + /// build that claims the identifier — fine for development, never for a + /// release. + static func isDistributionGrade(teamID: String?) -> Bool { + guard let teamID, !teamID.trimmingCharacters(in: .whitespaces).isEmpty else { return false } + return validated(identifier: teamID) != nil + } + + // MARK: - System witnesses + + /// The team that signed the given code, or nil when it is unsigned or + /// ad-hoc signed. Used by the daemon to learn its OWN team, so the + /// requirement never needs a hardcoded value. + static func teamIdentifier(of code: SecCode?) -> String? { + guard let code else { return nil } + var staticCode: SecStaticCode? + guard SecCodeCopyStaticCode(code, [], &staticCode) == errSecSuccess, + let staticCode else { return nil } + return teamIdentifier(ofStatic: staticCode) + } + + static func teamIdentifier(ofStatic staticCode: SecStaticCode) -> String? { + var information: CFDictionary? + guard SecCodeCopySigningInformation(staticCode, SecCSFlags(rawValue: kSecCSSigningInformation), + &information) == errSecSuccess, + let dictionary = information as? [String: Any] else { return nil } + let team = dictionary[kSecCodeInfoTeamIdentifier as String] as? String + return team.flatMap { validated(identifier: $0) } + } + + /// The team that signed the currently running process. + static func selfTeamIdentifier() -> String? { + var code: SecCode? + guard SecCodeCopySelf([], &code) == errSecSuccess else { return nil } + return teamIdentifier(of: code) + } + + /// A requirement matching any code signed by `teamID` with an Apple-issued + /// chain, without pinning a bundle identifier. + /// + /// Used for the nested ENGINE binary, which carries its own identifier but + /// is re-signed with our identity by the release pipeline. Returns nil + /// when a safe requirement can't be built, and callers treat that as + /// "refuse to run" rather than "skip the check". + static func sameTeam(teamID: String) -> String? { + guard let team = validated(identifier: teamID) else { return nil } + return #"anchor apple generic and certificate leaf[subject.OU] = "\#(team)""# + } +} diff --git a/macos/Sources/PrivilegedHelper/HelperContract.swift b/macos/Sources/PrivilegedHelper/HelperContract.swift new file mode 100644 index 00000000..25968c35 --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperContract.swift @@ -0,0 +1,360 @@ +// +// HelperContract.swift +// Burrow / BurrowHelper (shared) +// +// The wire contract between the GUI and the root helper. Compiled into BOTH +// targets so the two can never disagree about what a request means. +// +// Everything in this file is pure Foundation — no SwiftUI, no Sparkle, no +// Sentry — because the helper is a bare launchd daemon that must stay small, +// auditable, and free of anything that could pull UI or network code into a +// root process. +// +// ── Why the contract looks like this ──────────────────────────────────── +// The old elevation path handed osascript a SHELL STRING built from an +// executable path plus argv (`MoleCLI.elevatedScript`). It was carefully +// quoted and well tested, but its shape meant the privileged side had to +// trust whatever command the caller composed. +// +// This contract inverts that. A request names an OPERATION and nothing else. +// The argv is derived from the enum on the privileged side, so the set of +// commands the helper can ever run is fixed at compile time and visible in +// one switch statement. There is deliberately no field for a path, a flag, a +// binary, or a command string — a client that is fully compromised still +// cannot express "run this". +// + +import Foundation + +// MARK: - The closed operation set + +/// Every privileged thing Burrow can ask for. Adding a case here is a +/// security decision, not a feature decision: it widens what the root daemon +/// is capable of doing, permanently, for every installed copy. +enum HelperOperation: String, Codable, CaseIterable, Sendable { + /// Enumerate what a clean WOULD remove. Reads privileged locations; the + /// engine's `--dry-run` guarantees no mutation. + case scan + /// Remove the caches the engine's own rules select. + case clean + /// The engine's maintenance pass. + case optimize + /// Enumerate what an optimize WOULD do. The optimize counterpart of + /// `scan`, so the elevated preview of either operation behaves the same. + case optimizeScan + /// Flush the DNS cache and signal mDNSResponder to reload. + case flushDNS + /// Renew the DHCP lease on one network interface. + case renewDHCP + /// Dump the Background Task Management database — the modern Login and + /// background items list. + /// + /// Read-only, but it needs root twice over: `sfltool dumpbtm` raises its + /// OWN authentication dialog when run as a normal user, and returns only + /// a partial list even then. Through the helper it is one authentication + /// the user already understands, and the complete list. + case readLoginItems + + /// The engine argv for the operations that drive the bundled engine, or + /// nil for the ones that don't. + /// + /// These reproduce what the GUI runs today through osascript — CleanView's + /// `["clean"]`, OptimizeView's `["optimize"]`, and the dry-run previews — + /// so the helper changes HOW the command is elevated, never WHAT runs. + var engineArguments: [String]? { + switch self { + case .scan: return ["clean", "--dry-run"] + case .clean: return ["clean"] + case .optimize: return ["optimize"] + case .optimizeScan: return ["optimize", "--dry-run"] + case .flushDNS, .renewDHCP, .readLoginItems: return nil + } + } + + /// Whether this operation needs a network interface name. + var needsInterface: Bool { self == .renewDHCP } + + /// The exact process steps the daemon runs, in order. + /// + /// Every executable is an absolute path from a closed set, and every + /// argument is either a literal spelled here or an interface name that + /// `HelperRequest.validate` has already proved is a real interface on this + /// machine. Nothing is passed to a shell. + /// + /// That last point is a security improvement over the path this replaces: + /// `Connectivity.run` currently elevates + /// `/bin/sh -c "dscacheutil -flushcache; killall -HUP mDNSResponder"`, + /// so a root shell parses a command string. Here the two commands are two + /// separate `posix_spawn` calls with fixed argv and no shell in between. + func steps(interface: String?) -> [HelperStep] { + switch self { + case .scan, .clean, .optimize, .optimizeScan: + return [HelperStep(executable: .bundledEngine, arguments: engineArguments ?? [])] + case .flushDNS: + return [ + HelperStep(executable: .system(HelperSystemTool.dscacheutil), arguments: ["-flushcache"]), + HelperStep(executable: .system(HelperSystemTool.killall), arguments: ["-HUP", "mDNSResponder"]), + ] + case .renewDHCP: + guard let interface else { return [] } + return [HelperStep(executable: .system(HelperSystemTool.ipconfig), + arguments: ["set", interface, "DHCP"])] + case .readLoginItems: + return [HelperStep(executable: .system(HelperSystemTool.sfltool), + arguments: ["dumpbtm"])] + } + } + + /// Whether this operation mutates the disk. `scan` is read-only, but it + /// still authenticates: it reads privileged locations, and the product + /// decision was that anything running AS ROOT prompts. + var mutatesDisk: Bool { + switch self { + case .scan, .optimizeScan: return false + case .clean, .optimize: return true + // These change system state or read privileged data rather than + // touching the filesystem, but all run as root, so all authenticate. + case .flushDNS, .renewDHCP: return false + case .readLoginItems: return false + } + } + + /// Recognise an existing elevated call site's argv as a typed operation, + /// or `nil` if it isn't one of them. + /// + /// This is the migration seam. The GUI still describes elevated work as + /// `["clean"]` / `["optimize"]` through `OperationFlow`, and this maps + /// those onto the helper WITHOUT letting anything else through: argv the + /// helper doesn't recognise returns nil and keeps the existing osascript + /// route, rather than being forwarded as some approximate operation. + init?(engineArguments: [String]) { + guard let match = HelperOperation.allCases.first(where: { + guard let candidate = $0.engineArguments else { return false } + return candidate == engineArguments + }) else { return nil } + self = match + } +} + +// MARK: - Execution model + +/// The only system binaries the daemon may ever execute, by absolute path. +/// +/// A closed set, spelled once. The daemon never resolves a name through +/// `PATH` and never accepts a path from a caller — those are the two ways a +/// root process ends up running somebody else's binary. +enum HelperSystemTool { + static let dscacheutil = "/usr/bin/dscacheutil" + static let killall = "/usr/bin/killall" + static let ipconfig = "/usr/sbin/ipconfig" + static let sfltool = "/usr/bin/sfltool" + + /// Every permitted absolute path. Used by the daemon to re-check an + /// executable immediately before spawning it, so a step constructed by + /// some future code path still cannot introduce a new binary. + static let all: Set = [dscacheutil, killall, ipconfig, sfltool] +} + +/// What a step runs. +enum HelperExecutable: Equatable, Sendable { + /// The signed engine inside our own app bundle, resolved by the daemon. + case bundledEngine + /// One of `HelperSystemTool.all`, by absolute path. + case system(String) +} + +/// One process the daemon spawns. An operation is an ordered list of these. +struct HelperStep: Equatable, Sendable { + let executable: HelperExecutable + let arguments: [String] +} + +// MARK: - Request + +/// Why a request never reached the authorization step. Named so the GUI can +/// explain the refusal instead of showing a bare failure, and so a rejection +/// is never confused with "the command ran and failed". +enum HelperRequestRejection: String, Codable, Equatable, Sendable { + /// The payload wasn't decodable as a request at all. + case malformedPayload + /// The operation ID wasn't a UUID (see `HelperRequest.operationID`). + case malformedOperationID + /// The helper's build doesn't match the app's — see `HelperVersionSkew`. + case buildMismatch + /// This operation ID was already served. One authorization, one operation. + case replayedOperationID + /// The interface name was missing, malformed, or not a real interface on + /// this machine — or was supplied for an operation that takes none. + case invalidInterface +} + +/// One privileged operation, fully described. Three fields, none of which can +/// carry a command. +struct HelperRequest: Codable, Equatable, Sendable { + let operation: HelperOperation + + /// A fresh UUID per request. This is the replay key: the daemon serves any + /// given ID at most once, so a captured payload — external authorization + /// form and all — cannot be resent to buy a second root run out of one + /// prompt. A client-chosen constant would defeat that, hence the format + /// check rather than "any non-empty string". + let operationID: String + + /// `CFBundleVersion` of the calling app, compared against the helper's own. + /// A registered daemon outlives the app that installed it (Sparkle replaces + /// Burrow.app underneath it), and a stale root helper holding an older idea + /// of what `clean` does is exactly the drift worth refusing. + let clientBuild: String + + /// The network interface for `renewDHCP`, and nil for everything else. + /// + /// This is the ONLY caller-supplied value that reaches a child process's + /// argv, which is why it is checked twice: against a strict character + /// shape, and against the interfaces that actually exist on this machine. + /// A name that isn't a real interface is refused rather than passed on. + var networkInterface: String? = nil + + /// `nil` when the request is well formed. Runs on the PRIVILEGED side — + /// the client's own validation is a courtesy, this one is the boundary. + /// + /// `liveInterfaces` is injected so the rule stays pure and testable; the + /// daemon passes the real list from the system. + func validate(expectedBuild: String, + liveInterfaces: @autoclosure () -> Set = []) -> HelperRequestRejection? { + guard UUID(uuidString: operationID) != nil else { return .malformedOperationID } + guard HelperVersionSkew.evaluate(appBuild: expectedBuild, helperBuild: clientBuild) == .matched else { + return .buildMismatch + } + + if operation.needsInterface { + guard let name = networkInterface, + HelperRequest.isPlausibleInterfaceName(name), + liveInterfaces().contains(name) else { return .invalidInterface } + } else { + // An interface on an operation that takes none means the caller + // and this contract disagree about what is being asked for. + // Refuse rather than silently ignore it. + guard networkInterface == nil else { return .invalidInterface } + } + return nil + } + + /// BSD interface names are short, lowercase, and end in a unit number — + /// `en0`, `utun3`, `bridge0`, `awdl0`. Anything else (a path, a flag, a + /// space, a shell metacharacter) is refused outright rather than escaped: + /// there is no legitimate interface name outside this shape, so rejecting + /// costs nothing and removes the whole question. + static func isPlausibleInterfaceName(_ name: String) -> Bool { + guard (2...15).contains(name.count) else { return false } + var sawDigit = false + var letters = 0 + for scalar in name.unicodeScalars { + if scalar >= "a" && scalar <= "z" { + guard !sawDigit else { return false } // letters must precede digits + letters += 1 + } else if scalar >= "0" && scalar <= "9" { + sawDigit = true + } else { + return false + } + } + return letters >= 2 && sawDigit + } +} + +// MARK: - Response + +/// What a privileged operation produced. The failure cases mirror the +/// osascript path's `ElevatedOutcome` so the GUI keeps ONE error taxonomy +/// across both elevation routes (the rule issue #48 established). +struct HelperResponse: Codable, Equatable, Sendable { + enum Outcome: Codable, Equatable, Sendable { + /// The engine ran and exited with this status. + case exited(Int32) + /// The user dismissed the authentication prompt. + case authorizationCancelled + /// Authentication was attempted and refused (wrong credentials, not an + /// administrator, interaction unavailable). + case authorizationDenied + /// The request never reached authorization. + case rejected(HelperRequestRejection) + /// The signed, bundled engine could not be resolved or failed + /// verification — so nothing was executed. + case engineUnavailable + + /// Collapse to the `Int32` the existing call sites branch on. Every + /// failure shape is nonzero: a dismissed prompt must never read as + /// success. + var exitCode: Int32 { + switch self { + case .exited(let code): return code + case .authorizationCancelled, .authorizationDenied: return 1 + case .rejected: return 78 // EX_CONFIG: the request was refused, not run + case .engineUnavailable: return 127 + } + } + + // The bridge onto the GUI's existing `ElevatedOutcome` taxonomy lives + // in PrivilegedHelperClient.swift — that type belongs to the app, and + // this file also compiles into the daemon, which must stay free of + // anything GUI-side. + } + + let outcome: Outcome +} + +// MARK: - Replay guard + +/// Remembers which operation IDs have already been served, so one +/// authorization buys exactly one root operation. +/// +/// Bounded on purpose: a daemon can stay resident for weeks, and an unbounded +/// set would grow with every request a client cared to send. Eviction is +/// oldest-first, which only ever forgets ancient IDs — the practical replay +/// window (seconds, between authenticating and executing) is always covered. +final class HelperReplayGuard: @unchecked Sendable { + private let capacity: Int + private var order: [String] = [] + private var seen: Set = [] + private let lock = NSLock() + + init(capacity: Int = 512) { + self.capacity = max(1, capacity) + } + + /// Number of IDs currently remembered. Never exceeds `capacity`. + var count: Int { + lock.lock(); defer { lock.unlock() } + return seen.count + } + + /// `true` the first time an ID is presented, `false` for every repeat. + func admit(_ operationID: String) -> Bool { + lock.lock(); defer { lock.unlock() } + guard !seen.contains(operationID) else { return false } + seen.insert(operationID) + order.append(operationID) + while order.count > capacity { + seen.remove(order.removeFirst()) + } + return true + } +} + +// MARK: - Version skew + +/// Whether the app and the installed helper are the same build. +enum HelperVersionSkew { + enum Skew: Equatable, Sendable { + case matched + case mismatched + } + + /// Exact equality, and an empty build on either side is a mismatch. There + /// is no "close enough" here: the helper runs as root, and a version it + /// can't name is a version nobody has reasoned about. + static func evaluate(appBuild: String, helperBuild: String) -> Skew { + guard !appBuild.isEmpty, !helperBuild.isEmpty, appBuild == helperBuild else { return .mismatched } + return .matched + } +} diff --git a/macos/Sources/PrivilegedHelper/HelperXPC.swift b/macos/Sources/PrivilegedHelper/HelperXPC.swift new file mode 100644 index 00000000..1dbe3dc4 --- /dev/null +++ b/macos/Sources/PrivilegedHelper/HelperXPC.swift @@ -0,0 +1,84 @@ +// +// HelperXPC.swift +// Burrow / BurrowHelper (shared) +// +// The XPC surface, and the names both sides must agree on. +// +// The protocol is deliberately tiny. Three methods, and the only one that +// does privileged work takes an opaque request blob plus an authorization +// blob — both of which the daemon decodes and validates itself. There is no +// method that takes a command, a path, or an argument list, so the XPC +// surface cannot be talked into running something the typed contract doesn't +// already allow. +// + +import Foundation + +// MARK: - Names + +enum HelperNames { + /// The Mach service the daemon vends and the client connects to. Must + /// match `MachServices` in the launchd property list. + static let machService = "dev.caezium.Burrow.privileged-helper" + + /// The launchd label, and the property list file name inside the app at + /// `Contents/Library/LaunchDaemons/`. `SMAppService.daemon(plistName:)` + /// resolves the daemon from exactly that location. + static let daemonPlist = "dev.caezium.Burrow.privileged-helper.plist" + + /// The bundle identifier the daemon requires of any caller. + static let clientBundleID = "dev.caezium.Burrow" +} + +// MARK: - Daemon interface + +/// What the root daemon exposes. Implemented in the helper target. +@objc protocol BurrowHelperProtocol { + /// Run one typed operation as root. + /// + /// - Parameters: + /// - requestData: a JSON-encoded `HelperRequest`. Opaque here on + /// purpose — the daemon decodes and validates it, so a malformed or + /// hostile payload is refused inside the privileged process rather + /// than being pre-parsed by the caller. + /// - authorization: the client's `AuthorizationExternalForm` bytes. The + /// daemon rebuilds the reference and REQUIRES the right, which is what + /// raises the authentication prompt. + /// - reply: a JSON-encoded `HelperResponse`. + func execute(requestData: Data, authorization: Data, withReply reply: @escaping (Data) -> Void) + + /// The helper's own `CFBundleVersion`. The client compares it against its + /// own build and refuses to use a helper that doesn't match, since a + /// registered daemon outlives the app that installed it. + func helperBuild(withReply reply: @escaping (String) -> Void) + + /// Ask the daemon to terminate a running operation. + /// + /// This exists because the osascript path could NOT do it: killing + /// `osascript` orphans the root child it spawned, so the old streaming + /// flow had no safe way to cancel. The daemon owns the child process + /// directly, so it can signal it and reap it properly. + func cancelOperation(operationID: String, withReply reply: @escaping (Bool) -> Void) +} + +// MARK: - Client interface + +/// What the daemon can call back on the client — output streaming only. +/// Nothing here grants the daemon any authority over the app. +@objc protocol BurrowHelperClientProtocol { + /// One line of engine output, ANSI already stripped by the daemon. + func helperDidEmit(line: String, operationID: String) +} + +// MARK: - Interface configuration + +enum HelperInterface { + /// Both sides build their `NSXPCInterface` here so the shapes can't drift. + static func daemon() -> NSXPCInterface { + NSXPCInterface(with: BurrowHelperProtocol.self) + } + + static func client() -> NSXPCInterface { + NSXPCInterface(with: BurrowHelperClientProtocol.self) + } +} diff --git a/macos/Sources/PrivilegedHelperClient.swift b/macos/Sources/PrivilegedHelperClient.swift new file mode 100644 index 00000000..9b616703 --- /dev/null +++ b/macos/Sources/PrivilegedHelperClient.swift @@ -0,0 +1,414 @@ +// +// PrivilegedHelperClient.swift +// Burrow +// +// The GUI's side of the privileged helper: registration, connection, and one +// typed operation at a time. +// +// ── What this replaces, and what it does not ──────────────────────────── +// The existing elevation path builds a shell string and hands it to +// `osascript … with administrator privileges` (see `MoleCLI.elevatedScript`). +// That path is password-only by construction — the `system.privilege.admin` +// right authenticates through SecurityAgent's classic mechanism, which never +// offers Touch ID — and it cannot be cancelled safely, because killing +// osascript orphans the root child it spawned. +// +// The helper fixes both, but it can only be used once the user has approved +// registering a launch daemon, which is its own one-time macOS prompt. Until +// then — and if the user declines it outright — the osascript path remains, +// unchanged, as the fallback. `PrivilegeRoute` is where that choice is made, +// and it is a pure function so the conditions are visible and tested rather +// than scattered through call sites. +// +// ── The security properties this file must not weaken ────────────────── +// * Every root operation authenticates freshly. The client never caches an +// authorization, never reuses an operation ID, and never pre-authorizes. +// * The client cannot describe a command. It picks a `HelperOperation`, and +// the daemon derives argv from the enum. +// * A helper whose build doesn't match this app is not used at all. +// + +import Foundation +import Security +import ServiceManagement +import os + +/// Client-side trail, matching the daemon's. +/// +/// Without this, "the Clean didn't use the helper" and "the helper refused the +/// Clean" look identical from the outside — the daemon simply logs nothing in +/// the first case, which is exactly the ambiguity that made the last round of +/// debugging guesswork. +let helperClientLog = Logger(subsystem: "dev.caezium.Burrow", category: "privileged-helper") + +// MARK: - Bridging the two elevation routes onto one taxonomy + +extension HelperResponse.Outcome { + /// Map onto the taxonomy the GUI already renders, so the helper route and + /// the osascript route produce the same user-facing message and no call + /// site needs to know which one ran. + /// + /// `authorizationDenied` folds into `.authCancelled` because from the + /// user's side both mean "you weren't authenticated, so nothing ran". + var elevatedOutcome: ElevatedOutcome { + switch self { + case .exited(let code): return .exited(code) + case .authorizationCancelled, .authorizationDenied: return .authCancelled + case .rejected, .engineUnavailable: return .launchFailed + } + } +} + +// MARK: - Routing + +/// Whether a given elevated operation goes through the helper or the legacy +/// osascript path. Pure → unit-tested, because "when do we use the root +/// daemon" should not be an emergent property of five call sites. +enum PrivilegeRoute: Equatable { + /// Use the privileged helper for this typed operation. + case helper(HelperOperation) + /// Use the existing osascript elevation, unchanged. + case osascript + + /// The routing rule. The helper is used only when ALL of these hold: + /// * the argv maps onto one of the three typed operations; + /// * the daemon is registered and enabled; + /// * its build matches this app's. + /// + /// Any doubt routes to osascript. That is a genuine fallback rather than a + /// silent downgrade: the osascript path is the elevation Burrow has always + /// shipped, it still prompts for an administrator, and it still runs the + /// same trusted engine. What the user loses is Touch ID and safe + /// cancellation, not the authentication itself. + static func decide(arguments: [String], + registration: HelperRegistrationStatus, + skew: HelperVersionSkew.Skew) -> PrivilegeRoute { + guard let operation = HelperOperation(engineArguments: arguments) else { return .osascript } + guard registration == .enabled else { return .osascript } + guard skew == .matched else { return .osascript } + return .helper(operation) + } +} + +/// The daemon's registration state, mirrored off `SMAppService.Status` so call +/// sites and tests don't need ServiceManagement. +enum HelperRegistrationStatus: Equatable, Sendable { + /// Registered and allowed to run. + case enabled + /// Never registered on this machine, or the registration is gone. + case notRegistered + /// Registered, but waiting on the user in Login Items & Extensions — + /// either the initial approval or a switch they turned back off. + case requiresApproval + + init(_ status: SMAppService.Status) { + switch status { + case .enabled: self = .enabled + case .requiresApproval: self = .requiresApproval + case .notFound, .notRegistered: self = .notRegistered + @unknown default: + // A status this build has never heard of is not a licence to run + // privileged work. Fail closed to the state that routes elsewhere. + self = .notRegistered + } + } + + /// Whether the user still has a decision to make. Drives the Settings + /// copy — "approve this in Login Items" is actionable, "not registered" + /// is just the default state. + var needsUserAction: Bool { self == .requiresApproval } +} + +// MARK: - Client + +/// Talks to the root daemon. One operation per call, one authentication per +/// operation. +final class PrivilegedHelperClient: @unchecked Sendable { + + static let shared = PrivilegedHelperClient() + + /// This app's build, the value the daemon compares against its own. + static var appBuild: String { + Bundle.main.infoDictionary?["CFBundleVersion"] as? String ?? "" + } + + private let service = SMAppService.daemon(plistName: HelperNames.daemonPlist) + private let lock = NSLock() + private var cachedSkew: HelperVersionSkew.Skew? + + // MARK: Registration + + var registrationStatus: HelperRegistrationStatus { + HelperRegistrationStatus(service.status) + } + + /// Register the daemon. This raises macOS's own one-time administrator + /// approval for installing a launch daemon. + /// + /// Registering does NOT authorize any operation — that was an explicit + /// product decision, and it is enforced on the daemon side by a right + /// defined with `timeout: 0` and `shared: false`. Installing the helper + /// buys convenience, never standing privilege. + func register() throws { + try service.register() + lock.lock(); cachedSkew = nil; lock.unlock() + } + + /// Remove the daemon. Used by Settings, and by the uninstall path so + /// Burrow never leaves a root daemon behind. + func unregister() throws { + try service.unregister() + lock.lock(); cachedSkew = nil; lock.unlock() + } + + // MARK: Connection + + private func makeConnection() -> NSXPCConnection { + let connection = NSXPCConnection(machServiceName: HelperNames.machService, + options: .privileged) + connection.remoteObjectInterface = HelperInterface.daemon() + return connection + } + + /// The installed helper's build, or "" if it can't be reached. Blocking — + /// call off the main thread. + func helperBuild(timeout: TimeInterval = 5) -> String { + let connection = makeConnection() + connection.resume() + defer { connection.invalidate() } + + let semaphore = DispatchSemaphore(value: 0) + var result = "" + let proxy = connection.remoteObjectProxyWithErrorHandler { _ in semaphore.signal() } + (proxy as? BurrowHelperProtocol)?.helperBuild { build in + result = build + semaphore.signal() + } + _ = semaphore.wait(timeout: .now() + timeout) + return result + } + + /// Whether the installed helper matches this app. Cached per process + /// because it only changes across an update or a re-registration, both of + /// which clear it. + func versionSkew() -> HelperVersionSkew.Skew { + lock.lock() + if let cachedSkew { lock.unlock(); return cachedSkew } + lock.unlock() + + let reported = helperBuild() + helperClientLog.notice(""" + helper reported build \(reported.isEmpty ? "" : reported, privacy: .public), \ + app is \(Self.appBuild, privacy: .public) + """) + let skew = HelperVersionSkew.evaluate(appBuild: Self.appBuild, helperBuild: reported) + lock.lock(); cachedSkew = skew; lock.unlock() + return skew + } + + /// The route for an elevated invocation described the way `OperationFlow` + /// already describes it. + func route(for arguments: [String]) -> PrivilegeRoute { + let status = registrationStatus + // Don't pay for an XPC round trip to learn the version when the daemon + // isn't usable anyway. + guard status == .enabled else { + helperClientLog.notice("route: osascript (registration \(String(describing: status), privacy: .public))") + return PrivilegeRoute.decide(arguments: arguments, registration: status, skew: .mismatched) + } + let skew = versionSkew() + let route = PrivilegeRoute.decide(arguments: arguments, registration: status, skew: skew) + helperClientLog.notice(""" + route: \(String(describing: route), privacy: .public) \ + (app build \(Self.appBuild, privacy: .public), skew \(String(describing: skew), privacy: .public)) + """) + return route + } + + // MARK: Execution + + /// Run one typed operation as root, streaming output lines to `onLine`. + /// + /// Blocking — call off the main thread. It blocks for as long as the user + /// takes at the authentication prompt plus as long as the operation runs. + func run(operation: HelperOperation, + interface: String? = nil, + onLine: @escaping (String) -> Void) -> ElevatedOutcome { + // Ask the user to authenticate. This is the prompt — raised here, in a + // real session, so SecurityAgent can offer Touch ID. The daemon then + // verifies the resulting reference without prompting. + let granted: HelperAuthorization.ClientAuthorization + switch HelperAuthorization.authenticate() { + case .success(let authorization): + granted = authorization + case .failure(let refusal): + helperClientLog.notice("authentication refused: \(String(describing: refusal), privacy: .public)") + // Every refusal reads as "you weren't authenticated, nothing ran", + // which is exactly `.authCancelled` in the taxonomy the GUI + // already renders for the osascript path. + return .authCancelled + } + + // `granted` owns the AuthorizationRef, and the authorization instance + // only lives in the Security Server while that ref is held. Releasing + // it before the daemon internalizes the external form makes the + // daemon's side fail with errAuthorizationDenied — indistinguishable + // from the user being refused. `withExtendedLifetime` is what + // guarantees the optimizer can't drop it early; ordinary scoping is + // not a guarantee. + return withExtendedLifetime(granted) { + send(payload: granted.externalForm, operation: operation, + interface: interface, onLine: onLine) + } + } + + /// Run an operation and collect its whole output, for callers that need a + /// transcript rather than a live stream (reading the Login Items dump). + /// + /// Blocking — call off the main thread. + func capture(operation: HelperOperation, interface: String? = nil) -> (outcome: ElevatedOutcome, output: String) { + var lines: [String] = [] + let lock = NSLock() + let outcome = run(operation: operation, interface: interface) { line in + lock.lock(); lines.append(line); lock.unlock() + } + lock.lock(); let joined = lines.joined(separator: "\n"); lock.unlock() + return (outcome, joined) + } + + /// Whether the helper is installed, approved, and build-matched — i.e. + /// whether a caller should prefer it over its existing elevation. + var isUsable: Bool { + registrationStatus == .enabled && versionSkew() == .matched + } + + /// The XPC round trip. Split out so the authorization's lifetime is a + /// visible, enforced property of the caller rather than an accident of + /// where the local happens to go out of scope. + private func send(payload authorization: Data, + operation: HelperOperation, + interface: String?, + onLine: @escaping (String) -> Void) -> ElevatedOutcome { + let request = HelperRequest(operation: operation, + operationID: UUID().uuidString, + clientBuild: Self.appBuild, + networkInterface: interface) + guard let payload = try? JSONEncoder().encode(request) else { return .launchFailed } + + let connection = makeConnection() + connection.exportedInterface = HelperInterface.client() + let sink = HelperOutputSink(onLine: onLine) + connection.exportedObject = sink + connection.resume() + defer { connection.invalidate() } + + let semaphore = DispatchSemaphore(value: 0) + var outcome: ElevatedOutcome = .launchFailed + + let proxy = connection.remoteObjectProxyWithErrorHandler { _ in + // A connection error here means the daemon refused us, died, or + // was never reachable. Nothing ran, so this is a launch failure — + // never a silent success. + outcome = .launchFailed + semaphore.signal() + } + (proxy as? BurrowHelperProtocol)?.execute(requestData: payload, + authorization: authorization) { data in + if let response = try? JSONDecoder().decode(HelperResponse.self, from: data) { + outcome = response.outcome.elevatedOutcome + } + semaphore.signal() + } + + // No timeout: the user may take a long time at the authentication + // prompt, and a clean or optimize can legitimately run for minutes. + // Cancellation is explicit (`cancel(operationID:)`), which is exactly + // what the osascript path could never offer. + semaphore.wait() + return outcome + } +} + +// MARK: - The streaming seam + +/// Routes elevated streaming runs to the helper when it is usable, and to the +/// existing osascript port otherwise. +/// +/// This wraps `SystemProcessPort` rather than editing it, so the osascript +/// path — the elevation Burrow has shipped for every release so far — keeps +/// its exact behaviour, tests, and error taxonomy. A helper route is an +/// alternative, never a rewrite of the fallback. +struct HelperAwareProcessPort: ProcessPort { + var fallback: SystemProcessPort = SystemProcessPort() + var client: PrivilegedHelperClient = .shared + + func events(_ spec: ProcessSpec) -> AsyncStream { + // Un-elevated runs never involve the helper. Cheap check first, so the + // common path costs nothing. + guard spec.elevated else { return fallback.events(spec) } + + return AsyncStream { continuation in + // Routing asks the daemon for its version, and executing blocks + // for as long as the user takes to authenticate. Neither may + // happen on the main thread. + DispatchQueue.global(qos: .userInitiated).async { + switch client.route(for: spec.arguments) { + case .helper(let operation): + var sawOutput = false + let outcome = client.run(operation: operation) { line in + sawOutput = true + continuation.yield(.line(line)) + } + switch outcome { + case .exited(let code): + continuation.yield(.exited(code)) + case .authCancelled: + continuation.yield(.authCancelled) + case .launchFailed: + // Nothing ran. The report parser reduces an EMPTY + // transcript to a cheerful "Done — caches cleared", + // so a silent failure here reads to the user as a + // successful clean that freed nothing — the single + // worst outcome for a tool that deletes files. + // + // Emit a line so the transcript is non-empty and the + // failure is visible, then the nonzero exit. + if !sawOutput { + continuation.yield(.line(NSLocalizedString( + "The privileged helper could not run the bundled engine. Nothing was changed.", + comment: ""))) + } + continuation.yield(.exited(127)) + } + continuation.finish() + + case .osascript: + // Forward the legacy stream verbatim, including its + // cancellation behaviour. + let task = Task { + for await event in fallback.events(spec) { continuation.yield(event) } + continuation.finish() + } + continuation.onTermination = { @Sendable _ in task.cancel() } + } + } + } + } +} + +// MARK: - Output sink + +/// Receives streamed lines from the daemon. Strips ANSI on the CLIENT side, so +/// the root process carries no text-processing code it doesn't need. +private final class HelperOutputSink: NSObject, BurrowHelperClientProtocol { + private let onLine: (String) -> Void + + init(onLine: @escaping (String) -> Void) { + self.onLine = onLine + super.init() + } + + func helperDidEmit(line: String, operationID: String) { + onLine(Ansi.strip(line)) + } +} diff --git a/macos/Sources/SettingsView.swift b/macos/Sources/SettingsView.swift index dd51ab93..861cf551 100644 --- a/macos/Sources/SettingsView.swift +++ b/macos/Sources/SettingsView.swift @@ -16,7 +16,6 @@ import SwiftUI import AppKit -import LocalAuthentication import ServiceManagement import UniformTypeIdentifiers @@ -111,10 +110,9 @@ struct SettingsView: View { @State private var moleUpdating = false @State private var engineUpdatePolicy: MoleCLI.EngineUpdatePolicy = .unavailable @State private var copiedConfig = false - @State private var touchIDStatus = "—" - @State private var touchIDEnabled = false - @State private var touchIDBusy = false - @State private var touchIDAvailable = false + @State private var helperStatus: HelperRegistrationStatus = .notRegistered + @State private var helperBusy = false + @State private var helperError: String? /// Drop-in MCP config for Claude Code / Cursor / Codex / Cline — they /// all share the same `{command, args}` stdio shape, so one snippet @@ -164,13 +162,18 @@ struct SettingsView: View { .frame(width: 460, height: 440) } .onAppear { - refreshStatusLabels(); loadMoleVersion(); loadTouchIDStatus(); loadLaunchAtLogin() + refreshStatusLabels(); loadMoleVersion(); loadLaunchAtLogin() + loadHelperStatus() whitelistPatterns = MoleWhitelist.live.patterns() menuBarSuppressed = AppDelegate.shared?.menuBarSuppressedByCompatibilityGuard ?? false } .onReceive(NotificationCenter.default.publisher(for: NSApplication.didBecomeActiveNotification)) { _ in fdaGranted = Privacy.hasFullDiskAccess() axTrusted = CleanScreen.inputLockPermitted() + // Approving the helper happens in System Settings, outside this + // app, so the row would otherwise still read "waiting for your + // approval" after the user came back having already granted it. + loadHelperStatus() } } @@ -632,16 +635,17 @@ struct SettingsView: View { footnote("Sends anonymous product analytics (PostHog) plus crash, hang, startup, update, and sampled performance diagnostics (Sentry): random install IDs, app and exact macOS build, CPU type, screens and features used, and fixed-name diagnostic milestones. Never screenshots, screen recordings, your file names, contents, user paths, URLs, or metrics. On by default; turn it off and both stop. Full list in TELEMETRY.md.") } - section("Touch ID for sudo", "touchid") { - infoRow("Status", touchIDStatus) - if touchIDAvailable { - HStack { - Spacer() - if touchIDBusy { ProgressView().controlSize(.small).padding(.trailing, 4) } - PillButton(title: touchIDEnabled ? "Disable" : "Enable", filled: false) { toggleTouchID() } - } + section("Privileged helper", "lock.shield") { + infoRow("Status", helperStatusLabel) + HStack { + Spacer() + if helperBusy { ProgressView().controlSize(.small).padding(.trailing, 4) } + PillButton(title: helperInstalled ? "Remove" : "Install", filled: false) { toggleHelper() } } - footnote("Lets `sudo` in a terminal accept your fingerprint instead of a password — including `mo` commands you run yourself. It does NOT change Burrow's own admin prompts: those go through macOS authorization, which asks for your password regardless. Configured via `mo touchid` (pam_tid); turning it on or off needs your password once.") + if let helperError { + footnote(helperError) + } + footnote("Runs Burrow's admin operations — scan, clean, optimize — through a small signed helper instead of a password-only prompt, so macOS can offer Touch ID. Installing it needs your approval once and grants no standing access: you're asked to authenticate for each operation you start. The helper can only perform those three operations — it cannot be asked to run anything else.") } section("Mole engine", "shippingbox") { @@ -709,52 +713,89 @@ struct SettingsView: View { } } - // MARK: - Touch ID for sudo - - /// Whether this Mac actually has a Touch ID sensor. `mo touchid status` - /// only reports configured-vs-not (never hardware presence), so we ask - /// LocalAuthentication directly — biometryType is .touchID only on Macs - /// with the sensor (set after a canEvaluatePolicy probe, even if no - /// finger is enrolled yet). - private func touchIDHardwarePresent() -> Bool { - let ctx = LAContext() - var err: NSError? - _ = ctx.canEvaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, error: &err) - return ctx.biometryType == .touchID + // MARK: - Privileged helper + + private var helperInstalled: Bool { helperStatus != .notRegistered } + + private var helperStatusLabel: String { + switch helperStatus { + case .enabled: return "Installed" + case .requiresApproval: return "Waiting for your approval in Login Items & Extensions" + case .notRegistered: return "Not installed" + } } - private func loadTouchIDStatus() { - let available = touchIDHardwarePresent() - DispatchQueue.global(qos: .userInitiated).async { - let res = try? MoleCLI.run(args: ["touchid", "status"], timeout: 15) - // Strip ANSI colour codes Mole wraps the status line in before matching. - let out = Ansi.strip(res?.stdout ?? "").lowercased() - let enabled = out.contains("is enabled") - DispatchQueue.main.async { - touchIDAvailable = available - touchIDEnabled = enabled - touchIDStatus = !available ? "Not available on this Mac" - : (out.isEmpty ? "Unknown" : (enabled ? "Enabled" : "Disabled")) - } + private func loadHelperStatus() { + let status = PrivilegedHelperClient.shared.registrationStatus + DispatchQueue.main.async { helperStatus = status } + } + + /// Watch for the approval landing. + /// + /// `register()` returns as soon as the request is filed, but the daemon + /// stays in `.requiresApproval` until the user allows it in System + /// Settings ▸ General ▸ Login Items & Extensions. That approval is + /// asynchronous and happens in another process, with no notification back + /// to us, so a one-shot read right after `register()` almost always + /// reports the pre-approval state and then never corrects itself. + /// + /// Polling for a bounded window is the whole fix. It stops as soon as the + /// status settles, and gives up rather than running forever if the user + /// walks away or dismisses the prompt. + private func pollHelperApproval(deadline: Date = Date().addingTimeInterval(90)) { + guard Date() < deadline else { return } + DispatchQueue.main.asyncAfter(deadline: .now() + 1) { + let status = PrivilegedHelperClient.shared.registrationStatus + helperStatus = status + guard status == .requiresApproval else { return } + pollHelperApproval(deadline: deadline) } } - private func toggleTouchID() { - guard !touchIDBusy else { return } - touchIDBusy = true - let cmd = touchIDEnabled ? "disable" : "enable" + /// Install or remove the daemon. Both directions can throw — macOS refuses + /// registration if the user declines, and we surface that rather than + /// leaving the row silently unchanged. + private func toggleHelper() { + guard !helperBusy else { return } + helperBusy = true + helperError = nil + let install = !helperInstalled DispatchQueue.global(qos: .userInitiated).async { - let code = MoleCLI.runElevated(args: ["touchid", cmd]) - DispatchQueue.main.async { - touchIDBusy = false - loadTouchIDStatus() - if code != 0 { - let alert = NSAlert() - alert.messageText = NSLocalizedString("Couldn't update Touch ID for sudo", comment: "") - alert.informativeText = String(format: NSLocalizedString("`mo touchid %@` didn't complete (the password prompt may have been cancelled). You can also run it in a terminal.", comment: ""), cmd) - alert.runModalQuiet() + var failure: String? + do { + if install { + try PrivilegedHelperClient.shared.register() + } else { + try PrivilegedHelperClient.shared.unregister() + } + } catch { + // Show the real reason, not a shrug. The interesting failures + // here are all indistinguishable from each other in a generic + // message — a stale registration left by a previous build of + // the app, a signature the system won't accept, or the user + // declining — and the underlying code is what tells them + // apart. `kSMErrorAlreadyRegistered` in particular is + // actionable: the launchd job survives, bound to the old app + // identity, and has to be removed before a new install works. + let ns = error as NSError + let detail = "\(ns.domain) \(ns.code): \(ns.localizedDescription)" + if install { + let stale = ns.domain == "SMAppServiceErrorDomain" && ns.code == 134 + failure = stale + ? "A helper from an earlier build of Burrow is still registered. Remove Burrow under System Settings ▸ General ▸ Login Items & Extensions, then install again. (\(detail))" + : "Couldn't install the helper — Burrow keeps using the password prompt. (\(detail))" + } else { + failure = "Couldn't remove the helper. You can also turn it off in System Settings ▸ General ▸ Login Items & Extensions. (\(detail))" } } + let status = PrivilegedHelperClient.shared.registrationStatus + DispatchQueue.main.async { + helperBusy = false + helperError = failure + helperStatus = status + // Installing files the request; the user approves it elsewhere. + if install, status == .requiresApproval { pollHelperApproval() } + } } } diff --git a/macos/Sources/StartupInventory.swift b/macos/Sources/StartupInventory.swift index 36641516..1d8c4c13 100644 --- a/macos/Sources/StartupInventory.swift +++ b/macos/Sources/StartupInventory.swift @@ -215,8 +215,27 @@ enum StartupInventory { /// returns the plist inventory (graceful) rather than failing. static func scanLiveIncludingLoginItems() -> [StartupItem] { let base = scanLive() - let dump = (try? MoEngine.shared.capture( + return merge(plistItems: base, login: LoginItemsReader.parse(loginItemsDump())) + } + + /// The raw `sfltool dumpbtm` output. + /// + /// Prefer the privileged helper. Run as a normal user, `sfltool` raises + /// its OWN authentication dialog — an unexplained "sfltool wants to make + /// changes" prompt the user never asked for — and still returns only a + /// partial list. Through the helper it is one authentication attributable + /// to Burrow, and the complete BTM database. + /// + /// Without the helper this keeps the previous behaviour exactly, including + /// that prompt, rather than losing the Login Items list altogether. + private static func loginItemsDump() -> String { + let client = PrivilegedHelperClient.shared + if client.isUsable { + let result = client.capture(operation: .readLoginItems) + if case .exited(0) = result.outcome { return result.output } + return "" // declined or failed: fall back to the plist-only inventory + } + return (try? MoEngine.shared.capture( MoCommand(target: .executable("/usr/bin/sfltool"), args: ["dumpbtm"], timeout: 10)).stdout) ?? "" - return merge(plistItems: base, login: LoginItemsReader.parse(dump)) } } diff --git a/macos/Tests/HelperAuthorizationTests.swift b/macos/Tests/HelperAuthorizationTests.swift new file mode 100644 index 00000000..8d0dbd97 --- /dev/null +++ b/macos/Tests/HelperAuthorizationTests.swift @@ -0,0 +1,180 @@ +// +// HelperAuthorizationTests.swift +// BurrowTests +// +// The authorization policy behind every root operation. The decision the +// product made is narrow and unusually strict, so it is pinned here rather +// than left to a comment: +// +// every operation that actually runs as root requires a FRESH user +// authentication — no grace period, no cached approval, no +// "authenticate once for this launch", and registering the helper does +// not itself authorize anything. +// +// Two of those words are literally keys in the right's definition (`timeout` +// and `shared`), so a regression that reintroduces a credential cache is a +// one-line diff that these tests catch. +// +// Nothing here prompts, authenticates, or touches the policy database: the +// right definition is a dictionary and the OSStatus mapping is a pure +// function, so both are checkable with no root and no UI. +// + +import XCTest +import Security +@testable import Burrow + +final class HelperAuthorizationTests: XCTestCase { + + // MARK: - The right's identity + + /// The right is namespaced under the app's bundle identifier so it can + /// never collide with, or be satisfied by, a system right a user may have + /// already been granted for something else. + func testRightName_isNamespacedUnderTheBundleIdentifier() { + XCTAssertTrue(HelperAuthorization.rightName.hasPrefix("dev.caezium.Burrow."), + "the right must live in Burrow's own namespace") + XCTAssertFalse(HelperAuthorization.rightName.hasPrefix("system."), + "never reuse or shadow a system right") + } + + // MARK: - No caching, ever (the decision, as policy keys) + + /// The credential must survive exactly one XPC hop and no more. + /// + /// `timeout: 0` was the original design and is the ideal, but it kills the + /// credential before it reaches the daemon, so the daemon's check always + /// failed. The window is the smallest thing that works, and keeping it + /// small is the whole mitigation — so it is pinned rather than left to + /// drift upward the next time something feels slow. + func testRightDefinition_credentialWindowIsSmallAndBounded() { + let definition = HelperAuthorization.rightDefinition + let timeout = definition["timeout"] as? Int + XCTAssertNotNil(timeout) + XCTAssertGreaterThan(timeout ?? 0, 0, "0 kills the credential before the daemon can check it") + XCTAssertLessThanOrEqual(timeout ?? .max, 30, + "this covers an IPC round trip, not a user convenience grace period") + XCTAssertEqual(timeout, HelperAuthorization.credentialWindowSeconds) + } + + func testRightDefinition_isNotSharedWithOtherProcessesOrRights() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["shared"] as? Bool, false, + "a shared credential would let one prompt satisfy a later, different request") + } + + /// The daemon runs as root. If the right allowed root callers to skip + /// authentication, the daemon would authorize ITSELF and the prompt would + /// silently disappear — the single most dangerous misconfiguration + /// available here. + func testRightDefinition_doesNotLetTheRootDaemonAuthorizeItself() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["allow-root"] as? Bool, false, + "the root daemon must never satisfy this right by virtue of being root") + } + + func testRightDefinition_requiresAnAdministratorToAuthenticate() { + let definition = HelperAuthorization.rightDefinition + XCTAssertEqual(definition["class"] as? String, "user") + XCTAssertEqual(definition["group"] as? String, "admin") + XCTAssertEqual(definition["authenticate-user"] as? Bool, true) + } + + /// A human-readable reason ships with the right so the system prompt says + /// what Burrow is about to do rather than showing a bare app name. + func testRightDefinition_carriesAPromptDescription() { + let definition = HelperAuthorization.rightDefinition + let comment = definition["comment"] as? String ?? "" + XCTAssertFalse(comment.isEmpty, "the right documents itself in the policy database") + } + + // MARK: - Flags (the CopyRights call shape) + // + // Two documented ways to get this wrong, both of which turn the check into + // a no-op: + // * omitting kAuthorizationFlagExtendRights — rights are never actually + // extended, and sloppy callers read the status as success; + // * passing kAuthorizationFlagPreAuthorize in the DAEMON — that only + // asks "could this be authorized later", which is not an authorization. + + func testDaemonFlags_extendRights() { + XCTAssertTrue(HelperAuthorization.daemonFlags.contains(.extendRights), + "without this no right is actually granted") + } + + /// The daemon must never be able to prompt. + /// + /// Practically it cannot — a launchd system daemon has no session to draw + /// in, which is precisely what broke the first design. But the stronger + /// reason is that a daemon which CAN prompt is a daemon anything reaching + /// it can make prompt. Without the flag its check is a pure question, and + /// an unauthenticated reference simply fails. + func testDaemonFlags_neverAllowInteraction() { + XCTAssertFalse(HelperAuthorization.daemonFlags.contains(.interactionAllowed), + "the root daemon verifies; it never asks") + } + + /// The client is where the human is asked, so it needs interaction — and + /// `preAuthorize`, which is what makes the credential available to the + /// daemon's later check rather than only to this process. + func testClientFlags_promptAndPreAuthorize() { + let flags = HelperAuthorization.clientFlags + XCTAssertTrue(flags.contains(.interactionAllowed), "this is the call that shows the prompt") + XCTAssertTrue(flags.contains(.preAuthorize), "the daemon must be able to verify it afterwards") + XCTAssertTrue(flags.contains(.extendRights)) + } + + /// The two sides must not both prompt, and must not both merely verify. + /// Exactly one raises UI, and it is the one with a session. + func testFlags_exactlyOneSideCanPrompt() { + XCTAssertNotEqual(HelperAuthorization.clientFlags.contains(.interactionAllowed), + HelperAuthorization.daemonFlags.contains(.interactionAllowed)) + } + + // MARK: - OSStatus → outcome (pure, exhaustive) + + func testOutcome_successIsGranted() { + XCTAssertEqual(HelperAuthorization.outcome(from: errAuthorizationSuccess), .granted) + } + + func testOutcome_dismissedPromptIsCancelled() { + XCTAssertEqual(HelperAuthorization.outcome(from: errAuthorizationCanceled), .cancelled) + } + + func testOutcome_wrongPasswordOrRefusalIsDenied() { + XCTAssertEqual(HelperAuthorization.outcome(from: errAuthorizationDenied), .denied) + XCTAssertEqual(HelperAuthorization.outcome(from: OSStatus(errAuthorizationInteractionNotAllowed)), .denied) + } + + /// Anything unrecognised fails CLOSED. A status this code has never seen + /// must never fall through to "probably fine" — the operation is refused + /// and the raw status is preserved for diagnosis. + func testOutcome_unknownStatusFailsClosed() { + XCTAssertEqual(HelperAuthorization.outcome(from: OSStatus(-60999)), .failed(-60999)) + XCTAssertNotEqual(HelperAuthorization.outcome(from: OSStatus(-60999)), .granted) + } + + func testOutcome_onlyGrantedPermitsExecution() { + // The single predicate the daemon branches on, so "granted" can't be + // accidentally widened to "not an outright failure". + XCTAssertTrue(HelperAuthorization.Outcome.granted.permitsExecution) + for refused: HelperAuthorization.Outcome in [.denied, .cancelled, .failed(-1)] { + XCTAssertFalse(refused.permitsExecution, "\(refused) must not run anything as root") + } + } + + // MARK: - External form sizing + // + // The external form is a fixed-size C struct. A payload of any other size + // is malformed and is refused BEFORE it reaches + // AuthorizationCreateFromExternalForm, so a hostile client cannot feed the + // Security framework a short or oversized buffer. + + func testExternalForm_rejectsWrongSizedPayloads() { + let correct = MemoryLayout.size + XCTAssertTrue(HelperAuthorization.isPlausibleExternalForm(Data(count: correct))) + XCTAssertFalse(HelperAuthorization.isPlausibleExternalForm(Data())) + XCTAssertFalse(HelperAuthorization.isPlausibleExternalForm(Data(count: correct - 1))) + XCTAssertFalse(HelperAuthorization.isPlausibleExternalForm(Data(count: correct + 1))) + } +} diff --git a/macos/Tests/HelperCodeRequirementTests.swift b/macos/Tests/HelperCodeRequirementTests.swift new file mode 100644 index 00000000..b73c1ba0 --- /dev/null +++ b/macos/Tests/HelperCodeRequirementTests.swift @@ -0,0 +1,151 @@ +// +// HelperCodeRequirementTests.swift +// BurrowTests +// +// Who is allowed to talk to the root daemon at all. +// +// Authorization answers "may this operation run"; this answers the question +// that comes first — "is the process on the other end of this XPC connection +// actually Burrow". Without it, any local process could open the Mach service +// and at minimum drive the auth prompt, phishing the user for admin +// credentials with a dialog the system itself renders. +// +// The requirement string is built at RUNTIME from the daemon's own signing +// information, so nothing personal (a team ID, a certificate, a developer +// name) is ever hardcoded in the repository. These tests cover the pure +// string builder and the fail-closed rules around it; the SecCode calls +// themselves are a thin system witness. +// + +import XCTest +@testable import Burrow + +final class HelperCodeRequirementTests: XCTestCase { + + // MARK: - Distribution builds: identity is pinned to the signing team + + /// A Developer ID build pins THREE things: the exact bundle identifier, + /// an Apple-issued chain, and the signing team. Dropping any one of them + /// widens the caller set — identifier alone can be claimed by any ad-hoc + /// build, and anchor alone admits every Developer ID app on the machine. + func testRequirement_pinsIdentifierAnchorAndTeam() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", teamID: "ABCDE12345") + + XCTAssertTrue(requirement.contains(#"identifier "dev.caezium.Burrow""#)) + XCTAssertTrue(requirement.contains("anchor apple generic"), + "the chain must terminate at Apple, not at an arbitrary self-signed root") + XCTAssertTrue(requirement.contains(#"certificate leaf[subject.OU] = "ABCDE12345""#), + "same-team pinning is what stops another Developer ID app impersonating Burrow") + } + + func testRequirement_joinsEveryClauseWithAnd() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", teamID: "ABCDE12345") + // Three clauses, all mandatory — an `or` anywhere would make one optional. + XCTAssertEqual(requirement.components(separatedBy: " and ").count, 3) + XCTAssertFalse(requirement.contains(" or "), "no clause may be optional") + } + + // MARK: - Unsigned / ad-hoc builds fail closed + + /// A local Debug build is ad-hoc signed and has no team. It still gets a + /// requirement (identifier only) so development works, but it is explicitly + /// NOT distribution grade — and the release gate refuses to ship a helper + /// whose requirement can't name a team. + func testRequirement_adHocBuildFallsBackToIdentifierOnly() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", teamID: nil) + XCTAssertEqual(requirement, #"identifier "dev.caezium.Burrow""#) + } + + func testDistributionGrade_requiresATeam() { + XCTAssertTrue(HelperCodeRequirement.isDistributionGrade(teamID: "ABCDE12345")) + XCTAssertFalse(HelperCodeRequirement.isDistributionGrade(teamID: nil), + "an ad-hoc helper must never pass the release gate") + XCTAssertFalse(HelperCodeRequirement.isDistributionGrade(teamID: "")) + XCTAssertFalse(HelperCodeRequirement.isDistributionGrade(teamID: " ")) + } + + // MARK: - Injection into the requirement language + // + // The requirement string is parsed by the Security framework as a small + // language. Identifiers and team IDs are interpolated into it, so a value + // carrying a quote could close the literal early and append a clause — + // `identifier "x" or anchor apple` would admit every Apple-signed process + // on the machine. Values that cannot ride inertly are REFUSED, not escaped: + // a legitimate bundle ID or team ID never contains these characters, so + // rejecting is strictly safer than sanitising. + + func testSanitisation_rejectsQuotesAndRequirementOperators() { + for hostile in [#"dev.caezium.Burrow" or anchor apple generic and identifier "x"#, + #"dev.caezium.Burrow""#, + "dev.caezium.Burrow\\", + "dev.caezium.Burrow and anchor apple", + "dev.caezium.Burrow\nidentifier", + "dev.caezium.Burrow\u{0}"] { + XCTAssertNil(HelperCodeRequirement.validated(identifier: hostile), + "hostile identifier must be refused: \(hostile.debugDescription)") + } + } + + func testSanitisation_acceptsRealisticIdentifiersAndTeams() { + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "dev.caezium.Burrow"), "dev.caezium.Burrow") + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "dev.caezium.Burrow.helper"), + "dev.caezium.Burrow.helper") + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "ABCDE12345"), "ABCDE12345") + XCTAssertEqual(HelperCodeRequirement.validated(identifier: "A-B_C.D"), "A-B_C.D") + } + + func testSanitisation_rejectsEmptyAndOverlongValues() { + XCTAssertNil(HelperCodeRequirement.validated(identifier: "")) + XCTAssertNil(HelperCodeRequirement.validated(identifier: " ")) + XCTAssertNil(HelperCodeRequirement.validated(identifier: String(repeating: "a", count: 300))) + } + + /// A hostile value must take the whole requirement down with it, not + /// silently produce a weaker one. `string(bundleID:teamID:)` returns the + /// fail-closed sentinel that matches NOTHING when it can't build safely. + func testRequirement_hostileInputProducesAnUnsatisfiableRequirement() { + let requirement = HelperCodeRequirement.string(bundleID: #"x" or anchor apple generic"#, + teamID: "ABCDE12345") + XCTAssertEqual(requirement, HelperCodeRequirement.unsatisfiable) + XCTAssertFalse(requirement.contains("or anchor apple generic")) + } + + func testRequirement_hostileTeamProducesAnUnsatisfiableRequirement() { + let requirement = HelperCodeRequirement.string(bundleID: "dev.caezium.Burrow", + teamID: #"A" or anchor apple"#) + XCTAssertEqual(requirement, HelperCodeRequirement.unsatisfiable) + } + + /// The sentinel must be a syntactically valid requirement that no code can + /// satisfy — an empty string would be a parse error, and some call sites + /// treat a parse error as "no requirement", which is the opposite of what + /// we want. + func testUnsatisfiableSentinel_isValidSyntaxThatMatchesNothing() { + XCTAssertFalse(HelperCodeRequirement.unsatisfiable.isEmpty) + XCTAssertTrue(HelperCodeRequirement.unsatisfiable.contains("identifier")) + XCTAssertNil(HelperCodeRequirement.validated(identifier: HelperCodeRequirement.unsatisfiable)) + } + + // MARK: - Version skew between app and helper + // + // The installed helper outlives the app that installed it: Sparkle can + // replace Burrow.app underneath a registered daemon. A helper from an + // older build runs as root with an older idea of what `clean` does, so + // skew is refused rather than tolerated. + + func testVersionSkew_matchingBuildsAreCompatible() { + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "23", helperBuild: "23"), .matched) + } + + func testVersionSkew_anyDifferenceRequiresReregistration() { + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "24", helperBuild: "23"), .mismatched) + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "23", helperBuild: "24"), .mismatched) + } + + func testVersionSkew_missingOrUnreadableBuildIsMismatched() { + // A helper that won't say what it is gets no root work. Fail closed. + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "23", helperBuild: ""), .mismatched) + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "", helperBuild: "23"), .mismatched) + XCTAssertEqual(HelperVersionSkew.evaluate(appBuild: "", helperBuild: ""), .mismatched) + } +} diff --git a/macos/Tests/HelperContractTests.swift b/macos/Tests/HelperContractTests.swift new file mode 100644 index 00000000..9298956f --- /dev/null +++ b/macos/Tests/HelperContractTests.swift @@ -0,0 +1,313 @@ +// +// HelperContractTests.swift +// BurrowTests +// +// The typed-request boundary of the privileged helper. Everything asserted +// here runs in memory: no daemon, no XPC, no root, no auth prompt. +// +// The whole security argument for the helper rests on ONE claim — a client +// cannot describe an arbitrary command, only pick from a closed set of +// Burrow operations. These tests are what make that claim checkable: +// +// * the operation set is closed and each member maps to a FIXED argv the +// client never contributes a single token to; +// * a request that fails validation produces a named rejection, never a +// "best effort" run; +// * an operation ID cannot be replayed. +// +// If someone later widens the contract to carry a path, an argv element, or +// a command string, the argv tests below stop compiling or fail. That is the +// point. +// + +import XCTest +@testable import Burrow + +final class HelperContractTests: XCTestCase { + + // MARK: - The closed operation set + // + // The approved scope is exactly: privileged scan, clean, optimize. Not + // "run this binary", not "run this shell string", not "run mo with these + // args". A new case here is a deliberate security decision, so the count + // is pinned — adding one without updating this test is a failing build. + + func testOperationSet_isPinned() { + XCTAssertEqual(Set(HelperOperation.allCases.map(\.rawValue)), + ["scan", "clean", "optimize", "optimizeScan", "flushDNS", "renewDHCP", "readLoginItems"], + "the helper's operation set is closed; widening it is a security decision") + } + + // MARK: - The closed executable set + // + // Most operations drive the bundled engine. The rest drive system tools, and + // those are the only non-engine binaries the daemon may ever run. + + func testSystemTools_areAbsolutePathsInSystemDirectories() { + for tool in HelperSystemTool.all { + XCTAssertTrue(tool.hasPrefix("/usr/bin/") || tool.hasPrefix("/usr/sbin/"), + "\(tool) must be an absolute path in a system directory") + XCTAssertFalse(tool.contains(".."), "no traversal in a root-executed path") + } + } + + func testSystemTools_setIsExactlyWhatTheOperationsNeed() { + XCTAssertEqual(HelperSystemTool.all, + ["/usr/bin/dscacheutil", "/usr/bin/killall", + "/usr/sbin/ipconfig", "/usr/bin/sfltool"]) + } + + /// No step may ever name a shell. The path this replaces elevated + /// `/bin/sh -c "dscacheutil -flushcache; killall -HUP mDNSResponder"`, + /// which put a command string in front of a root shell parser. + func testSteps_neverInvokeAShell() { + let shells = ["/bin/sh", "/bin/bash", "/bin/zsh", "/usr/bin/env"] + for operation in HelperOperation.allCases { + for step in operation.steps(interface: "en0") { + if case .system(let path) = step.executable { + XCTAssertFalse(shells.contains(path), "\(operation) must not run a shell") + XCTAssertTrue(HelperSystemTool.all.contains(path), + "\(path) is outside the permitted executable set") + } + } + } + } + + func testSteps_flushDNSIsTwoSeparateProcesses() { + let steps = HelperOperation.flushDNS.steps(interface: nil) + XCTAssertEqual(steps, [ + HelperStep(executable: .system("/usr/bin/dscacheutil"), arguments: ["-flushcache"]), + HelperStep(executable: .system("/usr/bin/killall"), arguments: ["-HUP", "mDNSResponder"]), + ]) + } + + func testSteps_renewDHCPCarriesOnlyTheInterface() { + XCTAssertEqual(HelperOperation.renewDHCP.steps(interface: "en1"), + [HelperStep(executable: .system("/usr/sbin/ipconfig"), + arguments: ["set", "en1", "DHCP"])]) + } + + /// Without an interface there is nothing safe to run, so the operation + /// produces no steps at all rather than guessing a default. + func testSteps_renewDHCPWithoutAnInterfaceRunsNothing() { + XCTAssertTrue(HelperOperation.renewDHCP.steps(interface: nil).isEmpty) + } + + func testSteps_engineOperationsUseTheBundledEngine() { + for operation in [HelperOperation.scan, .clean, .optimize, .optimizeScan] { + let steps = operation.steps(interface: nil) + XCTAssertEqual(steps.count, 1) + XCTAssertEqual(steps.first?.executable, .bundledEngine) + } + } + + // MARK: - Interface names (the only caller value that reaches argv) + + func testInterfaceName_acceptsRealBSDNames() { + for name in ["en0", "en1", "utun3", "bridge0", "awdl0", "llw0", "anpi11"] { + XCTAssertTrue(HelperRequest.isPlausibleInterfaceName(name), "\(name) is a real interface shape") + } + } + + /// Everything that isn't a bare BSD interface name is refused rather than + /// escaped — there is no legitimate interface containing a path, a flag, + /// a space, or a shell metacharacter, so rejecting removes the question. + func testInterfaceName_rejectsAnythingElse() { + for hostile in ["", "e", "en", "0", "/usr/sbin/ipconfig", "en0 DHCP", "en0;rm -rf /", + "en0\n", "en0\u{0}", "--flag", "en0/../../x", "EN0", "en0.1", + "e0n1", String(repeating: "en", count: 20) + "0"] { + XCTAssertFalse(HelperRequest.isPlausibleInterfaceName(hostile), + "must reject \(hostile.debugDescription)") + } + } + + func testValidate_renewDHCPRequiresARealInterface() { + func request(_ name: String?) -> HelperRequest { + HelperRequest(operation: .renewDHCP, operationID: UUID().uuidString, + clientBuild: "23", networkInterface: name) + } + // Well-formed AND present on the machine. + XCTAssertNil(request("en0").validate(expectedBuild: "23", liveInterfaces: ["en0", "lo0"])) + // Well-formed but not a real interface here — still refused. + XCTAssertEqual(request("en9").validate(expectedBuild: "23", liveInterfaces: ["en0"]), + .invalidInterface) + // Malformed. + XCTAssertEqual(request("en0;id").validate(expectedBuild: "23", liveInterfaces: ["en0"]), + .invalidInterface) + // Missing entirely. + XCTAssertEqual(request(nil).validate(expectedBuild: "23", liveInterfaces: ["en0"]), + .invalidInterface) + } + + /// An interface on an operation that takes none means the caller and the + /// contract disagree. Refused, not ignored. + func testValidate_rejectsAnInterfaceOnOperationsThatTakeNone() { + for operation in [HelperOperation.clean, .optimize, .scan, .optimizeScan, + .flushDNS, .readLoginItems] { + let request = HelperRequest(operation: operation, operationID: UUID().uuidString, + clientBuild: "23", networkInterface: "en0") + XCTAssertEqual(request.validate(expectedBuild: "23", liveInterfaces: ["en0"]), + .invalidInterface, "\(operation) takes no interface") + } + } + + // MARK: - Fixed argv (the client contributes nothing) + // + // These are the exact argv the GUI passes today through the osascript + // path (CleanView `["clean"]`, OptimizeView `["optimize"]`, previews with + // `--dry-run`). The helper reproduces them from the enum ALONE, so an + // attacker who fully controls the XPC payload still cannot add a flag. + + func testEngineArguments_areFixedPerOperation() { + XCTAssertEqual(HelperOperation.scan.engineArguments, ["clean", "--dry-run"]) + XCTAssertEqual(HelperOperation.clean.engineArguments, ["clean"]) + XCTAssertEqual(HelperOperation.optimize.engineArguments, ["optimize"]) + XCTAssertEqual(HelperOperation.optimizeScan.engineArguments, ["optimize", "--dry-run"]) + // The network fixes don't drive the engine at all. + XCTAssertNil(HelperOperation.flushDNS.engineArguments) + XCTAssertNil(HelperOperation.renewDHCP.engineArguments) + } + + /// argv goes to posix_spawn, never a shell — but a stray metacharacter + /// anywhere in here would signal that someone started templating strings + /// into a command that runs as root. + func testArguments_neverEmptyAndNeverShellMetacharacters() { + for op in HelperOperation.allCases { + let steps = op.steps(interface: "en0") + XCTAssertFalse(steps.isEmpty, "\(op) must resolve to at least one command") + for token in steps.flatMap(\.arguments) { + XCTAssertFalse(token.contains(where: { ";|&`$<>\n\0".contains($0) }), + "\(op) argv token \(token) carries shell/NUL metacharacters") + } + } + } + + /// Decoding is the ONLY way a request enters the daemon, and the operation + /// is an enum — an unknown verb fails to decode rather than falling through + /// to some default. This is the test that keeps "run" from ever being a + /// smuggled operation. + func testDecoding_rejectsUnknownOperation() throws { + let payload = #"{"operation":"run","operationID":"\#(UUID().uuidString)","clientBuild":"23"}"# + XCTAssertThrowsError(try JSONDecoder().decode(HelperRequest.self, from: Data(payload.utf8))) + } + + func testDecoding_roundTripsEveryOperation() throws { + for op in HelperOperation.allCases { + let request = HelperRequest(operation: op, operationID: UUID().uuidString, clientBuild: "23") + let data = try JSONEncoder().encode(request) + XCTAssertEqual(try JSONDecoder().decode(HelperRequest.self, from: data), request) + } + } + + // MARK: - Validation (named rejections, never a partial run) + + func testValidate_acceptsAWellFormedRequest() { + let request = HelperRequest(operation: .clean, operationID: UUID().uuidString, clientBuild: "23") + XCTAssertNil(request.validate(expectedBuild: "23")) + } + + /// A non-UUID operation ID is rejected outright. The ID is the replay key, + /// so a client-chosen constant ("1") would let a single authorization be + /// reused; requiring a UUID makes every request distinguishable. + func testValidate_rejectsNonUUIDOperationID() { + for bad in ["", "1", "not-a-uuid", String(repeating: "a", count: 400)] { + let request = HelperRequest(operation: .clean, operationID: bad, clientBuild: "23") + XCTAssertEqual(request.validate(expectedBuild: "23"), .malformedOperationID, + "operation ID \(bad.prefix(12)) must be rejected") + } + } + + /// Version skew is a rejection, not a "try anyway". A stale helper paired + /// with a new app could hold an older idea of what `clean` does, and it + /// runs as root — so the mismatch stops the operation and the GUI + /// re-registers instead. + func testValidate_rejectsBuildMismatch() { + let request = HelperRequest(operation: .clean, operationID: UUID().uuidString, clientBuild: "22") + XCTAssertEqual(request.validate(expectedBuild: "23"), .buildMismatch) + } + + func testValidate_rejectsEmptyBuild() { + let request = HelperRequest(operation: .clean, operationID: UUID().uuidString, clientBuild: "") + XCTAssertEqual(request.validate(expectedBuild: "23"), .buildMismatch) + } + + // MARK: - Replay resistance + // + // One authorization authorizes ONE operation. The daemon remembers the IDs + // it has already served so a captured payload (auth external form included) + // cannot be re-sent to get a second root run out of one prompt. + + func testReplayGuard_admitsEachIDOnce() { + let guardian = HelperReplayGuard() + let id = UUID().uuidString + XCTAssertTrue(guardian.admit(id), "first use of an ID is allowed") + XCTAssertFalse(guardian.admit(id), "the same ID must never be served twice") + XCTAssertFalse(guardian.admit(id)) + } + + func testReplayGuard_distinctIDsAllPass() { + let guardian = HelperReplayGuard() + for _ in 0..<50 { + XCTAssertTrue(guardian.admit(UUID().uuidString)) + } + } + + /// The guard is bounded so a long-lived daemon can't be grown without + /// limit by a client that just keeps sending fresh IDs. Eviction is + /// oldest-first, and the CURRENT id is always remembered — so the replay + /// window can only ever shrink for ancient ids, never for a live one. + func testReplayGuard_isBoundedAndEvictsOldestFirst() { + let guardian = HelperReplayGuard(capacity: 3) + let ids = (0..<4).map { _ in UUID().uuidString } + for id in ids { XCTAssertTrue(guardian.admit(id)) } + + XCTAssertTrue(guardian.admit(ids[0]), "the oldest ID was evicted once capacity was exceeded") + XCTAssertFalse(guardian.admit(ids[3]), "the newest ID is still remembered") + XCTAssertEqual(guardian.count, 3, "the guard never grows past its capacity") + } + + // MARK: - Response encoding + // + // The reply crosses XPC as bytes, so it round-trips like the request. The + // failure cases are NAMED (matching the existing ElevatedOutcome taxonomy) + // so the GUI shows the right message instead of re-deriving meaning from a + // bare nonzero exit — the same rule issue #48 established for osascript. + + func testResponse_roundTripsEveryOutcome() throws { + let outcomes: [HelperResponse.Outcome] = [ + .exited(0), .exited(2), + .authorizationCancelled, + .authorizationDenied, + .rejected(.buildMismatch), + .rejected(.replayedOperationID), + .engineUnavailable, + ] + for outcome in outcomes { + let data = try JSONEncoder().encode(HelperResponse(outcome: outcome)) + XCTAssertEqual(try JSONDecoder().decode(HelperResponse.self, from: data).outcome, outcome) + } + } + + /// The GUI still has call sites that branch on an Int32. Every failure + /// shape must collapse to a NONZERO code, exactly as `ElevatedOutcome` + /// does today — a cancelled prompt must never read as success. + func testResponse_everyFailureIsNonzeroToLegacyCallers() { + XCTAssertEqual(HelperResponse.Outcome.exited(0).exitCode, 0) + XCTAssertEqual(HelperResponse.Outcome.exited(3).exitCode, 3) + for failure: HelperResponse.Outcome in [.authorizationCancelled, .authorizationDenied, + .rejected(.malformedPayload), .engineUnavailable] { + XCTAssertNotEqual(failure.exitCode, 0, "\(failure) must not read as success") + } + } + + /// A cancelled authorization maps onto the SAME user-facing meaning as the + /// osascript path's `.authCancelled`, so both elevation routes tell the + /// user the same thing and the GUI needs no second taxonomy. + func testResponse_mapsOntoTheExistingElevatedOutcomeTaxonomy() { + XCTAssertEqual(HelperResponse.Outcome.exited(0).elevatedOutcome, .exited(0)) + XCTAssertEqual(HelperResponse.Outcome.exited(7).elevatedOutcome, .exited(7)) + XCTAssertEqual(HelperResponse.Outcome.authorizationCancelled.elevatedOutcome, .authCancelled) + XCTAssertEqual(HelperResponse.Outcome.engineUnavailable.elevatedOutcome, .launchFailed) + XCTAssertEqual(HelperResponse.Outcome.authorizationDenied.elevatedOutcome, .authCancelled) + } +} diff --git a/macos/Tests/MoEngineTests.swift b/macos/Tests/MoEngineTests.swift index 0bf6d86d..2bd065b6 100644 --- a/macos/Tests/MoEngineTests.swift +++ b/macos/Tests/MoEngineTests.swift @@ -5,8 +5,8 @@ // Boundary tests for the unified runner facade (issue #48). The capture + // discovery entry points delegate to injected ports, so they're driven here // with scripted fakes — same seam style as MoleProcessTests (capture port). -// (The one-shot elevated path is NOT on the facade; its wiring is covered by -// PrivilegeBrokerTests against `MoleCLI.runElevatedClassified`.) +// (The one-shot elevated path is NOT on the facade; its pure pieces are +// covered by PrivilegeBrokerTests.) // // The point of these tests is the WIRING: that a `MoCommand` lands on the // capture runner as the exact argv/stdin/env/timeout it described, and that a diff --git a/macos/Tests/MoleCLITests.swift b/macos/Tests/MoleCLITests.swift index cf79d607..a0890c6a 100644 --- a/macos/Tests/MoleCLITests.swift +++ b/macos/Tests/MoleCLITests.swift @@ -135,12 +135,31 @@ final class MoleCLITests: XCTestCase { XCTAssertTrue(s.contains("> '/tmp/my log.txt' 2>&1")) } + /// The invariant: an elevated run resolves its binary from the app bundle + /// or from a fixed absolute location, NEVER from a PATH lookup a + /// user-writable directory could shadow. + /// + /// The old version of this test listed only the three `mo` paths, so it + /// passed purely because CI and dev checkouts had no bundled engine. With + /// the engine actually bundled — which is what every release ships — + /// `trustedExecutable()` correctly returns the in-bundle copy first and + /// the assertion failed. It was checking a stale list, not the invariant. func testTrustedExecutable_onlyEverReturnsKnownLocations() { - if let p = MoleCLI.trustedExecutable() { - XCTAssertTrue(["/opt/homebrew/bin/mo", "/usr/local/bin/mo", "/usr/bin/mo"].contains(p), - "trusted lookup must never come from PATH") + guard let resolved = MoleCLI.trustedExecutable() else { + return // nothing installed in a trusted spot is a valid outcome } - // nil (mo not installed in a trusted spot) is also a valid outcome. + + let fixedLocations = [ + "/opt/homebrew/bin/burrow-engine", "/usr/local/bin/burrow-engine", + "/opt/homebrew/bin/mo", "/usr/local/bin/mo", "/usr/bin/mo", + ] + if fixedLocations.contains(resolved) { return } + + // Otherwise it must be the engine sealed inside our own app bundle. + XCTAssertEqual(resolved, MoleCLI.bundledExecutable(), + "trusted lookup must be the bundled engine or a fixed absolute path, never PATH") + XCTAssertTrue(resolved.contains("/Burrow.app/Contents/Resources/engine/"), + "the bundled engine must live inside the signed app bundle") } func testRun_timesOutInsteadOfHanging() throws { diff --git a/macos/Tests/PrivilegeBrokerTests.swift b/macos/Tests/PrivilegeBrokerTests.swift index 3fc3bc43..9f826640 100644 --- a/macos/Tests/PrivilegeBrokerTests.swift +++ b/macos/Tests/PrivilegeBrokerTests.swift @@ -2,46 +2,33 @@ // PrivilegeBrokerTests.swift // BurrowTests // -// Boundary tests for the one-shot elevated path (issue #48) — the code that -// runs `mo` as ROOT, and that no test could previously reach because it +// Boundary tests for the elevated path (issue #48) — the code that runs +// commands as ROOT, and that no test could previously reach because it // spawned a real osascript auth dialog inline. // -// Two seams are exercised in memory, with NO osascript, NO sudo, NO GUI: -// * `AuthCancel` — the pure rule that decides a dismissed auth prompt vs a +// Two pure pieces are exercised in memory, with NO osascript, NO sudo, NO GUI: +// * `AuthCancel` — the rule that decides a dismissed auth prompt vs a // command that ran and failed. Shared by the streaming runner -// (SystemProcessPort.finalEvent) and this one-shot broker, so it's +// (SystemProcessPort.finalEvent) and the one-shot broker, so it's // table-tested exhaustively here. -// * `PrivilegeBroker` — the spawn port. A scripted fake stands in for -// osascript, so `MoleCLI.runElevated`/`runElevatedClassified` can be -// driven through every outcome (cancel, fail, success, launch-failure) -// and the osascript spec quoting verified — the previously-untestable -// ROOT path is now fully covered. +// * `MoleCLI.elevatedScript` — the two-pass quoter whose output runs as +// root inside `do shell script`. The injection cases at the bottom are +// the quoting that, if it broke, would delete the wrong files. +// +// The scripted-fake broker tests that used to live here went with +// `MoleCLI.runElevated`/`runElevatedClassified`, deleted once the +// `mo touchid` setting (their only caller) was removed. `PrivilegeBroker` +// itself is still live — Connectivity's flush-DNS / renew-DHCP fixes use +// `SystemPrivilegeBroker.openElevated` — but it is constructed directly at +// that call site, so there is no injection seam left to drive a fake through. // import XCTest @testable import Burrow -// MARK: - Scripted fake broker - -/// In-memory stand-in for osascript. Records the (executable, args) it was -/// asked to elevate so quoting/injection can be asserted, and replays a -/// canned outcome — no real process, no auth dialog. -final class FakePrivilegeBroker: PrivilegeBroker, @unchecked Sendable { - private(set) var calls: [(executable: String, args: [String])] = [] - var outcome: ElevatedOutcome - - init(outcome: ElevatedOutcome) { self.outcome = outcome } - - func openElevated(executable: String, args: [String]) -> ElevatedOutcome { - calls.append((executable, args)) - return outcome - } -} - final class PrivilegeBrokerTests: XCTestCase { override func tearDown() { - MoleCLI.privilegeBroker = SystemPrivilegeBroker() MoleCLI.discoveryCandidates = nil MoleCLI.resetDiscoveryCache() super.tearDown() @@ -104,7 +91,7 @@ final class PrivilegeBrokerTests: XCTestCase { // MARK: - ElevatedOutcome back-compat (the preserved Int32 contract) // - // `runElevated` still returns Int32 for existing callers; both failure + // The Int32 shim still backs `Connectivity.run`; both failure // shapes must collapse to a nonzero code, exactly as the old inline // spawn did (catch → 1, no trusted mo → 127). @@ -115,74 +102,6 @@ final class PrivilegeBrokerTests: XCTestCase { XCTAssertEqual(ElevatedOutcome.launchFailed.exitCode, 127, "matches the old 'no trusted mo' sentinel") } - // MARK: - Broker routing through MoleCLI (in-memory, no osascript) - // - // `runElevated` resolves the binary through `trustedExecutable()` ONLY — - // never `discoveryCandidates`/PATH, because a user-writable path entry - // would hand root to a shadowed binary. So these tests can't inject a - // fake `mo` path the way the capture-runner tests do; they assert the - // invariant against whatever the trusted lookup actually resolves, and - // skip the spawn-shape assertions when no trusted mo is installed (a - // valid CI state — nil is the launch-failure path, covered separately). - - func testRunElevated_routesTrustedExecutableAndArgsToBroker() throws { - guard let trusted = MoleCLI.trustedExecutable() else { - throw XCTSkip("no trusted mo installed; spawn-routing shape needs one") - } - let fake = FakePrivilegeBroker(outcome: .exited(0)) - MoleCLI.privilegeBroker = fake - - let code = MoleCLI.runElevated(args: ["touchid", "enable"]) - - XCTAssertEqual(code, 0) - XCTAssertEqual(fake.calls.count, 1) - XCTAssertEqual(fake.calls.first?.executable, trusted, - "elevated runs resolve through the trusted list, never PATH") - XCTAssertEqual(fake.calls.first?.args, ["touchid", "enable"]) - } - - func testRunElevated_authCancelSurfacesAsNonzeroButClassified() throws { - guard MoleCLI.trustedExecutable() != nil else { - throw XCTSkip("no trusted mo installed; the broker is never reached") - } - MoleCLI.privilegeBroker = FakePrivilegeBroker(outcome: .authCancelled) - - // Legacy Int32 caller: a dismissed prompt is still "didn't work". - XCTAssertNotEqual(MoleCLI.runElevated(args: ["touchid", "enable"]), 0) - // New caller: the cancel is NAMED, distinct from a command failure. - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["touchid", "enable"]), .authCancelled) - } - - func testRunElevated_commandFailureIsDistinctFromCancel() throws { - guard MoleCLI.trustedExecutable() != nil else { - throw XCTSkip("no trusted mo installed; the broker is never reached") - } - MoleCLI.privilegeBroker = FakePrivilegeBroker(outcome: .exited(2)) - - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["touchid", "disable"]), .exited(2)) - XCTAssertEqual(MoleCLI.runElevated(args: ["touchid", "disable"]), 2) - } - - /// No trusted `mo` → the broker is never asked to elevate; the result is - /// the launch-failure sentinel (127), matching the old `guard … else - /// return 127`. Only assertable when the trusted lookup genuinely misses; - /// where a real mo is installed the guard can't be forced (resolution is - /// deliberately not injectable), so we assert the inverse there: the - /// broker IS reached. - func testRunElevated_noTrustedExecutableTakesLaunchFailurePath() { - let fake = FakePrivilegeBroker(outcome: .exited(0)) - MoleCLI.privilegeBroker = fake - - if MoleCLI.trustedExecutable() == nil { - XCTAssertEqual(MoleCLI.runElevatedClassified(args: ["touchid", "enable"]), .launchFailed) - XCTAssertEqual(MoleCLI.runElevated(args: ["touchid", "enable"]), 127) - XCTAssertTrue(fake.calls.isEmpty, "a missing trusted mo must never reach the elevation spawn") - } else { - _ = MoleCLI.runElevatedClassified(args: ["touchid", "enable"]) - XCTAssertFalse(fake.calls.isEmpty, "with a trusted mo, the broker is reached") - } - } - // MARK: - osascript spec quoting through the broker (injection cases) // // The string the broker builds runs as ROOT inside `do shell script …`. diff --git a/macos/Tests/PrivilegeRouteTests.swift b/macos/Tests/PrivilegeRouteTests.swift new file mode 100644 index 00000000..2176c058 --- /dev/null +++ b/macos/Tests/PrivilegeRouteTests.swift @@ -0,0 +1,134 @@ +// +// PrivilegeRouteTests.swift +// BurrowTests +// +// Which elevation route an operation takes, and — more importantly — which +// ones it must NOT take. +// +// The helper and the osascript path both end in an administrator prompt and +// both run the same trusted engine, so falling back is safe. What would not +// be safe is the reverse: quietly widening what the helper accepts, or +// routing to a root daemon whose build no longer matches the app driving it. +// Every guard below is one of those. +// +// Pure decisions, no daemon, no registration, no XPC. +// + +import XCTest +@testable import Burrow + +final class PrivilegeRouteTests: XCTestCase { + + // MARK: - argv → typed operation + // + // The migration seam. `OperationFlow` still describes elevated work as + // argv, and this is the ONLY place that argv is allowed to become a typed + // operation. Anything it doesn't recognise keeps the old route rather than + // being forwarded as an approximate match. + + func testRecognition_mapsTheThreeKnownCallSites() { + // Exactly what CleanView, OptimizeView, and the preview path pass today. + XCTAssertEqual(HelperOperation(engineArguments: ["clean"]), .clean) + XCTAssertEqual(HelperOperation(engineArguments: ["optimize"]), .optimize) + XCTAssertEqual(HelperOperation(engineArguments: ["clean", "--dry-run"]), .scan) + } + + func testRecognition_isExactAndOrderSensitive() { + // A near-miss is not a match. Extra flags, reordering, or a different + // verb all fall through to osascript rather than being coerced into + // the closest typed operation. + XCTAssertNil(HelperOperation(engineArguments: ["--dry-run", "clean"])) + XCTAssertNil(HelperOperation(engineArguments: ["clean", "--yes"])) + XCTAssertNil(HelperOperation(engineArguments: ["clean", "--dry-run", "--verbose"])) + XCTAssertNil(HelperOperation(engineArguments: ["uninstall", "Safari"])) + XCTAssertNil(HelperOperation(engineArguments: [])) + } + + /// The one that matters: an attacker-shaped argv must never resolve to an + /// operation. It can't, because recognition is equality against three + /// fixed arrays — but this is the assertion that says so out loud. + func testRecognition_neverAcceptsInjectedArgv() { + for hostile in [["clean", "; rm -rf /"], + ["clean", "--dry-run", "&&", "curl", "evil"], + ["/bin/sh"], + ["clean\0"], + ["clean\n--force"]] { + XCTAssertNil(HelperOperation(engineArguments: hostile), + "\(hostile) must not resolve to a privileged operation") + } + } + + // MARK: - The routing rule + + func testRoute_usesTheHelperWhenEverythingLinesUp() { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["clean"], + registration: .enabled, + skew: .matched), + .helper(.clean)) + } + + func testRoute_unrecognisedArgvKeepsTheLegacyPath() { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["uninstall", "Safari"], + registration: .enabled, + skew: .matched), + .osascript) + } + + /// A daemon that isn't registered, or that the user hasn't approved, is + /// not a daemon we may use. Registration is the user's decision, and + /// declining it must leave Burrow working exactly as before. + func testRoute_unregisteredOrUnapprovedKeepsTheLegacyPath() { + for registration: HelperRegistrationStatus in [.notRegistered, .requiresApproval] { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["clean"], + registration: registration, + skew: .matched), + .osascript, + "registration \(registration) must not reach the helper") + } + } + + /// The sharp one. A registered daemon OUTLIVES the app that installed it — + /// Sparkle can replace Burrow.app underneath it — so a helper from an + /// older build could be running as root with an older idea of what `clean` + /// does. Skew routes away from the helper entirely. + func testRoute_versionSkewKeepsTheLegacyPath() { + XCTAssertEqual(PrivilegeRoute.decide(arguments: ["clean"], + registration: .enabled, + skew: .mismatched), + .osascript) + } + + /// Every combination, so no future edit can produce a `.helper` route from + /// a state that isn't fully green. + func testRoute_helperRequiresEveryConditionSimultaneously() { + let registrations: [HelperRegistrationStatus] = [.enabled, .notRegistered, .requiresApproval] + let skews: [HelperVersionSkew.Skew] = [.matched, .mismatched] + let argvs = [["clean"], ["optimize"], ["clean", "--dry-run"], ["uninstall"], []] + + for registration in registrations { + for skew in skews { + for argv in argvs { + let route = PrivilegeRoute.decide(arguments: argv, registration: registration, skew: skew) + let allGreen = registration == .enabled + && skew == .matched + && HelperOperation(engineArguments: argv) != nil + if allGreen { + XCTAssertNotEqual(route, .osascript) + } else { + XCTAssertEqual(route, .osascript, + "argv \(argv), registration \(registration), skew \(skew)") + } + } + } + } + } + + // MARK: - Registration status mapping + + func testRegistrationStatus_needsUserActionOnlyWhenApprovalIsPending() { + XCTAssertTrue(HelperRegistrationStatus.requiresApproval.needsUserAction) + XCTAssertFalse(HelperRegistrationStatus.enabled.needsUserAction) + XCTAssertFalse(HelperRegistrationStatus.notRegistered.needsUserAction, + "never registered is the default state, not a pending decision") + } +} diff --git a/macos/project.yml b/macos/project.yml index 66e594af..fe48b098 100644 --- a/macos/project.yml +++ b/macos/project.yml @@ -45,14 +45,24 @@ targets: sources: - path: Sources - path: Resources - excludes: ["Info.plist", "Burrow.entitlements"] + # LaunchDaemons is excluded from the plain resource copy and staged by + # the dedicated phase below instead: a stray second copy under + # Resources/ would be dead weight inside the signed bundle. + excludes: ["Info.plist", "Burrow.entitlements", "LaunchDaemons"] + # SMAppService.daemon(plistName:) resolves the daemon from exactly this + # path and nowhere else. + - path: Resources/LaunchDaemons/dev.caezium.Burrow.privileged-helper.plist + buildPhase: + copyFiles: + destination: wrapper + subpath: Contents/Library/LaunchDaemons info: path: Resources/Info.plist properties: LSUIElement: true # menu-bar agent, no Dock icon CFBundleDisplayName: Burrow - CFBundleShortVersionString: "0.11.2" - CFBundleVersion: "23" + CFBundleShortVersionString: "0.12.0" + CFBundleVersion: "24" NSHumanReadableCopyright: "MIT License. © 2026 Henry Zhang." # Burrow uses only exempt encryption supplied by macOS (for example, # URLSession HTTPS); it contains no proprietary or non-exempt crypto. @@ -102,6 +112,15 @@ targets: embed: true - framework: vendor/Sentry.xcframework embed: false + # The privileged helper ships INSIDE the app. It is never installed to + # /Library/PrivilegedHelperTools, so it cannot drift from, or outlive, + # the app that vouches for it — SMAppService resolves it through the + # bundle via the BundleProgram key in the launchd plist. + # Copied into Contents/MacOS, beside the app's own executable, which is + # where BundleProgram in the launchd plist points. + - target: BurrowHelper + copy: + destination: executables postBuildScripts: - name: Bundle burrow-engine (MIT) basedOnDependencyAnalysis: false @@ -155,6 +174,38 @@ targets: "$SRCROOT/scripts/bundle-fclones.sh" "$TARGET_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH" \ || echo "warning: fclones bundling skipped — Duplicates falls back to a system/\$BURROW_FCLONES fclones." + # The privileged helper: a bare launchd daemon that runs as root. + # + # Deliberately minimal. It compiles ONLY the shared contract in + # Sources/PrivilegedHelper plus its own HelperSources — no SwiftUI, no + # Sparkle, no Sentry, no telemetry. Nothing that could pull UI, networking, + # or crash-reporting code into a root process, and nothing that could grow a + # dependency without that showing up as a change to this list. + BurrowHelper: + type: tool + platform: macOS + sources: + # Shared with the app so the two can never disagree about what a request + # means. Same files, both targets. + - path: Sources/PrivilegedHelper + - path: HelperSources + settings: + base: + PRODUCT_NAME: BurrowHelper + PRODUCT_BUNDLE_IDENTIFIER: dev.caezium.Burrow.privileged-helper + # The helper reports its OWN build over XPC, and the client refuses a + # helper whose build doesn't match the app's. Baking the plist into the + # binary means it reports what it IS, rather than reading whatever + # bundle happens to surround it after an update. + CREATE_INFOPLIST_SECTION_IN_BINARY: YES + GENERATE_INFOPLIST_FILE: YES + # Must track the app's CFBundleVersion above — a mismatch is exactly + # what HelperVersionSkew refuses, so a stale value here disables the + # helper rather than shipping a subtly wrong one. + CURRENT_PROJECT_VERSION: "24" + MARKETING_VERSION: "0.12.0" + SKIP_INSTALL: YES + BurrowTests: type: bundle.unit-test platform: macOS diff --git a/scripts/sign-macos-app.sh b/scripts/sign-macos-app.sh index e1aaefc8..0e5f6f0c 100755 --- a/scripts/sign-macos-app.sh +++ b/scripts/sign-macos-app.sh @@ -111,9 +111,23 @@ required_plist_raw() { printf '%s' "$value" } +# The bundle's own main executable is deliberately EXCLUDED from this loop. +# Signing it on its own makes codesign treat it as the bundle and validate the +# bundle's nested code — which, now that Contents/MacOS also holds +# BurrowHelper, fails with "code object is not signed at all / In subcomponent: +# …/BurrowHelper" whenever find happens to reach the main executable first. +# The outer seal below signs it correctly as part of the bundle. +MAIN_EXECUTABLE_NAME="$( + plutil -extract CFBundleExecutable raw -o - "$APP/Contents/Info.plist" 2>/dev/null || true +)" +[ -n "$MAIN_EXECUTABLE_NAME" ] \ + || { echo "error: could not read CFBundleExecutable from $APP/Contents/Info.plist" >&2; exit 1; } +MAIN_EXECUTABLE="$APP/Contents/MacOS/$MAIN_EXECUTABLE_NAME" + echo "==> signing nested Mach-O files ($MODE)" SIGNED_MACHO=0 while IFS= read -r -d '' candidate; do + [ "$candidate" = "$MAIN_EXECUTABLE" ] && continue if is_macho "$candidate"; then sign_one "$candidate" SIGNED_MACHO=$((SIGNED_MACHO + 1)) @@ -201,4 +215,69 @@ else fi fi +# --------------------------------------------------------------------------- +# Privileged helper gate. +# +# The helper runs as ROOT, so a build that ships it half-wired is worse than +# one that doesn't ship it at all: a missing plist key or an unsigned binary +# turns into a launchd failure, or worse a daemon nobody vetted. Every check +# below is fail-closed, and the whole block is mandatory — the helper is not an +# optional component, so "absent" is an error, not a skip. +HELPER="$APP/Contents/MacOS/BurrowHelper" +HELPER_PLIST="$APP/Contents/Library/LaunchDaemons/dev.caezium.Burrow.privileged-helper.plist" + +echo "==> verifying the privileged helper" +[ -f "$HELPER" ] || { echo "error: privileged helper missing: $HELPER" >&2; exit 1; } +is_macho "$HELPER" || { echo "error: privileged helper is not a Mach-O binary" >&2; exit 1; } +[ -f "$HELPER_PLIST" ] || { echo "error: launchd plist missing: $HELPER_PLIST" >&2; exit 1; } +plutil -lint "$HELPER_PLIST" >/dev/null \ + || { echo "error: launchd plist is not a valid property list" >&2; exit 1; } + +helper_plist_value() { + plutil -extract "$1" raw -expect "$2" -o - "$HELPER_PLIST" 2>/dev/null +} + +# The label and the Mach service name are what SMAppService and the client +# connect through. A typo in either is a silent "helper never starts". +HELPER_LABEL="$(helper_plist_value Label string || true)" +[ "$HELPER_LABEL" = "dev.caezium.Burrow.privileged-helper" ] \ + || { echo "error: launchd Label is '${HELPER_LABEL:-missing}', expected dev.caezium.Burrow.privileged-helper" >&2; exit 1; } + +helper_plist_value 'MachServices.dev\.caezium\.Burrow\.privileged-helper' bool >/dev/null \ + || { echo "error: launchd plist does not vend the dev.caezium.Burrow.privileged-helper Mach service" >&2; exit 1; } + +# BundleProgram is resolved relative to the app bundle. If it points anywhere +# other than the executable we just signed, the daemon that actually runs as +# root is not the one this pipeline verified. +HELPER_PROGRAM="$(helper_plist_value BundleProgram string || true)" +[ "$HELPER_PROGRAM" = "Contents/MacOS/BurrowHelper" ] \ + || { echo "error: BundleProgram is '${HELPER_PROGRAM:-missing}', expected Contents/MacOS/BurrowHelper" >&2; exit 1; } +[ -f "$APP/$HELPER_PROGRAM" ] \ + || { echo "error: BundleProgram does not resolve to a file inside the app" >&2; exit 1; } + +codesign --verify --strict --verbose=2 "$HELPER" \ + || { echo "error: privileged helper failed strict signature verification" >&2; exit 1; } + +if [ "$MODE" = "developer-id" ]; then + # The client pins callers to our signing team, and the helper verifies the + # engine the same way. An ad-hoc helper has no team to pin, so it would fall + # back to identifier-only matching — fine for local development, never for a + # release. + HELPER_TEAM="$(codesign -d --verbose=4 "$HELPER" 2>&1 | awk -F= '$1 == "TeamIdentifier" { print $2; exit }')" + [ "$HELPER_TEAM" = "$EXPECTED_TEAM" ] \ + || { echo "error: helper team is '${HELPER_TEAM:-missing}', expected '$EXPECTED_TEAM'" >&2; exit 1; } + + # Hardened runtime on a root daemon is not optional. + # + # Read once into a variable rather than piping codesign straight into grep: + # `set -o pipefail` is in force, so a non-zero exit from codesign fails the + # pipeline even when the grep matched, which reads as "no hardened runtime" + # on a binary that has it. Separating the two keeps the check about the flag. + HELPER_SIG="$(codesign -d --verbose=2 "$HELPER" 2>&1 || true)" + grep -q "flags=.*runtime" <<< "$HELPER_SIG" \ + || { echo "error: privileged helper is not built with the hardened runtime" >&2; exit 1; } +fi + +echo "privileged helper verified: signed, hardened, and correctly declared to launchd" + echo "signed $SIGNED_MACHO Mach-O file(s) and $SIGNED_CONTAINERS code container(s); strict verification passed" diff --git a/scripts/tests/test_helper_version_sync.py b/scripts/tests/test_helper_version_sync.py new file mode 100644 index 00000000..aab05e40 --- /dev/null +++ b/scripts/tests/test_helper_version_sync.py @@ -0,0 +1,73 @@ +"""The app and the privileged helper must ship the same version. + +The helper reports its own build over XPC and the client refuses to use a +helper whose build doesn't match the app's — that check is deliberate, because +a registered daemon outlives the app that installed it and a stale root helper +is exactly the drift worth refusing. + +The cost of that design is a coupling: `macos/project.yml` spells the version +twice, once for the app target and once for the helper target, and Xcode has +no way to derive one from the other. Bump the app for a release and forget the +helper and nothing fails loudly — the helper simply stops being used, every +user silently falls back to the password-only prompt, and the feature dies +quietly. + +That failure is invisible in the build, invisible in the tests, and invisible +in the release artifact. So it gets caught here instead. +""" + +import re +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] +PROJECT = ROOT / "macos" / "project.yml" + + +def _scalar(key: str) -> list[str]: + """Every value assigned to `key` in project.yml, in file order.""" + text = PROJECT.read_text(encoding="utf-8") + return re.findall(rf'^\s*{re.escape(key)}:\s*"([^"]+)"\s*$', text, re.MULTILINE) + + +class HelperVersionSyncTests(unittest.TestCase): + def test_project_file_exists(self) -> None: + self.assertTrue(PROJECT.is_file(), f"missing {PROJECT}") + + def test_build_number_matches_between_app_and_helper(self) -> None: + app = _scalar("CFBundleVersion") + helper = _scalar("CURRENT_PROJECT_VERSION") + + self.assertEqual(len(app), 1, "expected exactly one app CFBundleVersion") + self.assertEqual(len(helper), 1, "expected exactly one helper CURRENT_PROJECT_VERSION") + self.assertEqual( + app[0], + helper[0], + "app CFBundleVersion and helper CURRENT_PROJECT_VERSION must match, " + "or HelperVersionSkew refuses the helper at runtime and every user " + "silently falls back to the password prompt", + ) + + def test_marketing_version_matches_between_app_and_helper(self) -> None: + app = _scalar("CFBundleShortVersionString") + helper = _scalar("MARKETING_VERSION") + + self.assertEqual(len(app), 1, "expected exactly one app CFBundleShortVersionString") + self.assertEqual(len(helper), 1, "expected exactly one helper MARKETING_VERSION") + self.assertEqual( + app[0], + helper[0], + "app and helper marketing versions must match", + ) + + def test_versions_are_plausible(self) -> None: + """Guards against the regex silently matching nothing useful.""" + build = _scalar("CFBundleVersion")[0] + marketing = _scalar("CFBundleShortVersionString")[0] + self.assertTrue(build.isdigit(), f"build number should be an integer, got {build!r}") + self.assertRegex(marketing, r"^\d+\.\d+(\.\d+)?$") + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_sign_macos_app.py b/scripts/tests/test_sign_macos_app.py index 863c58e6..6fce2491 100644 --- a/scripts/tests/test_sign_macos_app.py +++ b/scripts/tests/test_sign_macos_app.py @@ -17,6 +17,9 @@ def make_app( root: Path, *, get_task_allow: bool | str | None = None, + helper: bool = True, + helper_plist_overrides: dict[str, object] | None = None, + helper_plist: bool = True, ) -> tuple[Path, Path]: app = root / "Burrow.app" executable = app / "Contents" / "MacOS" / "Burrow" @@ -26,6 +29,30 @@ def make_app( shutil.copyfile("/usr/bin/true", executable) executable.chmod(0o755) + # The privileged helper and its launchd declaration. Present by + # default because the signer treats them as mandatory: a release that + # ships the helper half-wired is worse than one without it, so + # "absent" has to be an error rather than a skip. + if helper: + helper_binary = app / "Contents" / "MacOS" / "BurrowHelper" + shutil.copyfile("/usr/bin/true", helper_binary) + helper_binary.chmod(0o755) + + if helper_plist: + plist_path = ( + app / "Contents" / "Library" / "LaunchDaemons" + / "dev.caezium.Burrow.privileged-helper.plist" + ) + plist_path.parent.mkdir(parents=True, exist_ok=True) + contents: dict[str, object] = { + "Label": "dev.caezium.Burrow.privileged-helper", + "BundleProgram": "Contents/MacOS/BurrowHelper", + "MachServices": {"dev.caezium.Burrow.privileged-helper": True}, + } + contents.update(helper_plist_overrides or {}) + with plist_path.open("wb") as handle: + plistlib.dump(contents, handle) + with (app / "Contents" / "Info.plist").open("wb") as handle: plistlib.dump( { @@ -150,5 +177,153 @@ def test_present_get_task_allow_extraction_failure_blocks_release(self) -> None: self.assertIn("could not extract plist key", result.stderr) +class PrivilegedHelperGateTests(SignMacOSAppTests): + """The signer's fail-closed checks on the root helper. + + Each case here is a way to ship a helper that launchd would either refuse + to start or, worse, start as something other than the binary this pipeline + verified. None of them is caught by `codesign --verify --deep`, which is + why they are checked explicitly. + """ + + def test_missing_helper_binary_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, helper=False) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("privileged helper missing", result.stderr) + + def test_missing_launchd_plist_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, helper_plist=False) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("launchd plist missing", result.stderr) + + def test_wrong_label_blocks_release(self) -> None: + # SMAppService and launchd both key off the label; a typo means a + # daemon that silently never runs. + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app( + root, helper_plist_overrides={"Label": "dev.caezium.Burrow.helpr"} + ) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("launchd Label is", result.stderr) + + def test_missing_mach_service_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app( + root, helper_plist_overrides={"MachServices": {"something.else": True}} + ) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("does not vend the dev.caezium.Burrow.privileged-helper Mach service", result.stderr) + + def test_bundle_program_pointing_elsewhere_blocks_release(self) -> None: + """The sharpest one. + + BundleProgram is what launchd actually executes as root. If it names + anything other than the executable the pipeline just signed and + verified, the daemon running with full privileges is not the binary + this release vouched for. + """ + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app( + root, helper_plist_overrides={"BundleProgram": "Contents/MacOS/Burrow"} + ) + + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("BundleProgram is", result.stderr) + + def test_bundle_program_that_does_not_exist_blocks_release(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, helper=False) + # Restore only the plist so the failure is specifically the + # dangling BundleProgram rather than the missing-binary check. + result = self.run_signer(app, entitlements) + + self.assertNotEqual(result.returncode, 0) + + def test_main_executable_is_never_signed_on_its_own(self) -> None: + """The bundle's main executable must be signed only by the outer seal. + + Signing it individually makes codesign treat it as the bundle and + validate the bundle's nested code. With a second executable in + Contents/MacOS that fails — "code object is not signed at all / In + subcomponent: …/BurrowHelper" — whenever `find` reaches the main + executable before the helper. + + That ordering is filesystem-dependent, so the existing tests passed by + luck while a real Release build failed. This asserts the invariant + directly rather than relying on directory order. + """ + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, get_task_allow=False) + + log = root / "codesign-calls.log" + fake_bin = root / "bin" + fake_bin.mkdir() + fake_codesign = fake_bin / "codesign" + fake_codesign.write_text( + f"""#!/bin/bash +# Record only real signing invocations (-s/--sign), not -d/--verify queries. +for arg in "$@"; do + if [ "$arg" = "--sign" ] || [ "$arg" = "-s" ]; then + echo "${{@: -1}}" >> "{log}" + break + fi +done +exec /usr/bin/codesign "$@" +""", + encoding="utf-8", + ) + fake_codesign.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{fake_bin}{os.pathsep}{env['PATH']}" + + result = self.run_signer(app, entitlements, env=env) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + signed = log.read_text(encoding="utf-8").split() if log.exists() else [] + main_executable = str(app / "Contents" / "MacOS" / "Burrow") + self.assertNotIn( + main_executable, + signed, + "the main executable must not be signed on its own; the outer " + "app seal covers it", + ) + # The helper still must be signed individually, and the app sealed. + self.assertIn(str(app / "Contents" / "MacOS" / "BurrowHelper"), signed) + self.assertIn(str(app), signed) + + def test_wellformed_helper_passes(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + app, entitlements = self.make_app(root, get_task_allow=False) + + result = self.run_signer(app, entitlements) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("privileged helper verified", result.stdout) + + if __name__ == "__main__": unittest.main()