Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions native-app/Sources/NativeAppLib/BrokerCore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.")
Expand Down
17 changes: 17 additions & 0 deletions native-app/Tests/NativeAppTests/BrokerCoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 32 additions & 4 deletions rust/src/native_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Value> {
Expand Down Expand Up @@ -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() {
Expand Down
Loading