From 936a4f38a86d7123673679704b41a65be1abe34b Mon Sep 17 00:00:00 2001 From: Ares Date: Sun, 5 Jul 2026 09:02:42 +0000 Subject: [PATCH] Authenticate native broker socket peers --- .../Sources/NativeAppLib/BrokerCore.swift | 21 +++++++++++ .../NativeAppTests/BrokerCoreTests.swift | 17 +++++++++ rust/src/native_app.rs | 36 ++++++++++++++++--- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/native-app/Sources/NativeAppLib/BrokerCore.swift b/native-app/Sources/NativeAppLib/BrokerCore.swift index c7ca927..5a09509 100644 --- a/native-app/Sources/NativeAppLib/BrokerCore.swift +++ b/native-app/Sources/NativeAppLib/BrokerCore.swift @@ -420,6 +420,11 @@ final class BrokerServer { continue } + guard peerIsCurrentUser(client) else { + close(client) + continue + } + // Bound the lifetime of any single broker exchange. A peer that stops // sending or stops draining must not block the broker forever. See // issue #2 / `brokerRequestTimeoutMs`. @@ -453,6 +458,22 @@ final class BrokerServer { } } + func peerCredentials(for descriptor: Int32) -> (uid: uid_t, gid: gid_t)? { + var uid = uid_t() + var gid = gid_t() + guard getpeereid(descriptor, &uid, &gid) == 0 else { + return nil + } + return (uid, gid) + } + + func peerIsCurrentUser(_ descriptor: Int32, currentEuid: uid_t = geteuid()) -> Bool { + guard let credentials = peerCredentials(for: descriptor) else { + return false + } + return credentials.uid == currentEuid + } + private func handleRequest(from handle: FileHandle) throws -> ResponseEnvelope { guard let data = try handle.readToEnd(), !data.isEmpty else { throw BrokerError.message("Empty broker request.") diff --git a/native-app/Tests/NativeAppTests/BrokerCoreTests.swift b/native-app/Tests/NativeAppTests/BrokerCoreTests.swift index a1d0380..a2b5d02 100644 --- a/native-app/Tests/NativeAppTests/BrokerCoreTests.swift +++ b/native-app/Tests/NativeAppTests/BrokerCoreTests.swift @@ -187,6 +187,23 @@ final class BrokerCoreTests: XCTestCase { XCTAssertFalse(FileManager.default.fileExists(atPath: makePaths(root).socketPath.path)) } + func testPeerCredentialCheckRequiresCurrentEffectiveUser() throws { + let root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent(UUID().uuidString, isDirectory: true) + let server = makeServer(root: root) + var descriptors = [Int32](repeating: -1, count: 2) + XCTAssertEqual(socketpair(AF_UNIX, SOCK_STREAM, 0, &descriptors), 0) + defer { + close(descriptors[0]) + close(descriptors[1]) + } + + let currentEuid = geteuid() + let mismatchedEuid: uid_t = currentEuid == 0 ? 1 : 0 + XCTAssertTrue(server.peerIsCurrentUser(descriptors[0], currentEuid: currentEuid)) + XCTAssertFalse(server.peerIsCurrentUser(descriptors[0], currentEuid: mismatchedEuid)) + } + func testPromptForApprovalDecisionLogicUsesInjectedPrompter() throws { let root = URL(fileURLWithPath: NSTemporaryDirectory()) .appendingPathComponent(UUID().uuidString, isDirectory: true) diff --git a/rust/src/native_app.rs b/rust/src/native_app.rs index fac885c..4bdbb65 100644 --- a/rust/src/native_app.rs +++ b/rust/src/native_app.rs @@ -7,6 +7,7 @@ use std::env; use std::fs; use std::io::{Read, Write}; use std::os::unix::fs::FileTypeExt; +use std::os::unix::fs::MetadataExt; use std::os::unix::fs::PermissionsExt; use std::os::unix::net::UnixStream; use std::os::unix::process::CommandExt; @@ -620,11 +621,15 @@ fn socket_path_safe_to_connect(socket_path: &Path) -> bool { Err(_) => return false, }; - if !metadata.file_type().is_socket() { - return false; - } + // SAFETY: `geteuid` reads the effective uid for the current process and has + // no memory-safety preconditions. + socket_metadata_safe_to_connect(&metadata, unsafe { libc::geteuid() }) +} - metadata.permissions().mode() & 0o777 == NATIVE_APP_FILE_MODE +fn socket_metadata_safe_to_connect(metadata: &fs::Metadata, current_euid: libc::uid_t) -> bool { + metadata.file_type().is_socket() + && metadata.uid() == current_euid + && (metadata.permissions().mode() & 0o777) == NATIVE_APP_FILE_MODE } fn parse_response(payload: Value) -> Result { @@ -2099,6 +2104,29 @@ mod tests { }); } + #[test] + #[serial] + fn broker_socket_security_requires_socket_mode_and_current_owner() { + with_temp_home(|| { + ensure_runtime_dir().unwrap(); + let socket_path = native_app_socket_path(); + let listener = bind_restrictive_socket(&socket_path); + let metadata = fs::symlink_metadata(&socket_path).unwrap(); + let current_euid = unsafe { libc::geteuid() }; + let mismatched_euid = if current_euid == 0 { 1 } else { 0 }; + + assert!(socket_path_safe_to_connect(&socket_path)); + assert!(socket_metadata_safe_to_connect(&metadata, current_euid)); + assert!(!socket_metadata_safe_to_connect(&metadata, mismatched_euid)); + + fs::set_permissions(&socket_path, fs::Permissions::from_mode(0o666)).unwrap(); + let metadata = fs::symlink_metadata(&socket_path).unwrap(); + assert!(!socket_metadata_safe_to_connect(&metadata, current_euid)); + + drop(listener); + }); + } + #[test] #[serial] fn rotates_broker_log_when_it_exceeds_limit() {