From 3e220ac0704015c441a3393afab120902e431d20 Mon Sep 17 00:00:00 2001 From: Steven Joruk Date: Sat, 3 Jul 2021 12:26:46 +0100 Subject: [PATCH] Support xpc_connection_set_peer_sig on macOS 12 Also support validating client code requirements using the audit token using the same APIs. --- examples/echo-server/Cargo.toml | 3 - examples/echo-server/src/main.rs | 59 +++------------ xpc-connection-sys/src/lib.rs | 2 +- xpc-connection/Cargo.toml | 10 ++- xpc-connection/src/dlsym.rs | 50 +++++++++++++ xpc-connection/src/lib.rs | 120 +++++++++++++++++++++++++++---- 6 files changed, 176 insertions(+), 68 deletions(-) create mode 100644 xpc-connection/src/dlsym.rs diff --git a/examples/echo-server/Cargo.toml b/examples/echo-server/Cargo.toml index 95dce79..15e261a 100644 --- a/examples/echo-server/Cargo.toml +++ b/examples/echo-server/Cargo.toml @@ -8,9 +8,6 @@ publish = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -core-foundation = "0.9" futures = { version = "0.3" } -# Support for SecCode was added in 2.3.1 -security-framework = "^2.3.1" tokio = { version = "1", features = ["macros", "rt-multi-thread"] } xpc-connection = { path = "../../xpc-connection", features = ["audit_token"] } diff --git a/examples/echo-server/src/main.rs b/examples/echo-server/src/main.rs index a99fe39..f222142 100644 --- a/examples/echo-server/src/main.rs +++ b/examples/echo-server/src/main.rs @@ -1,58 +1,10 @@ -use core_foundation::{base::TCFType, data::CFData}; use futures::stream::StreamExt; -use security_framework::os::macos::code_signing::{Flags, GuestAttributes, SecCode}; use std::{error::Error, ffi::CString}; use xpc_connection::{Message, MessageError, XpcClient, XpcListener}; -fn get_code_object_for_client(client: &XpcClient) -> SecCode { - let token_data = CFData::from_buffer(&client.audit_token()); - let mut attrs = GuestAttributes::new(); - attrs.set_audit_token(token_data.as_concrete_TypeRef()); - SecCode::copy_guest_with_attribues(None, &attrs, Flags::NONE).unwrap() -} - -#[allow(dead_code)] -/// This isn't used because we don't sign our builds, but it's a useful example. -fn validate_client_by_code_signing_requirement(client: &XpcClient) -> bool { - let requirement = "anchor apple".parse().unwrap(); - - if get_code_object_for_client(client) - .check_validity(Flags::NONE, &requirement) - .is_ok() - { - println!("The client's code signature matches"); - return true; - } - - println!("The client's code signature doesn't match"); - false -} - -fn validate_client_by_path(client: &XpcClient) -> bool { - if get_code_object_for_client(client) - .path(Flags::NONE) - .unwrap() - // It'd be better to use to_path - .get_string() - .to_string() - // This is insecure, it's just so the tests can be run from anywhere - .contains("message_round_trip") - { - println!("The client was validated using its path"); - return true; - } - - println!("The client's path doesn't contain 'message_round_trip'"); - false -} - async fn handle_client(mut client: XpcClient) { println!("New connection"); - if !validate_client_by_path(&client) { - return; - } - loop { match client.next().await { None => { @@ -80,7 +32,16 @@ async fn main() -> Result<(), Box> { mach_port_name.to_string_lossy() ); - let mut listener = XpcListener::listen(&mach_port_name); + let mut listener = XpcListener::listen( + &mach_port_name, + // An example requirement. Whe matching on the common name it's + // important to anchor to a trusted authority that you know doesn't + // allow for user-defined common names, otherwise it would be trivial + // to bypass. + // Some("anchor apple and certificate leaf[subject.CN] = \"Apple Development: Steven Joruk (Z84S59N9K4)\""), + None, + None, + ); while let Some(client) = listener.next().await { tokio::spawn(handle_client(client)); diff --git a/xpc-connection-sys/src/lib.rs b/xpc-connection-sys/src/lib.rs index a2f4e6d..75d93f8 100644 --- a/xpc-connection-sys/src/lib.rs +++ b/xpc-connection-sys/src/lib.rs @@ -1,6 +1,6 @@ #![allow( dead_code, - safe_packed_borrows, + unaligned_references, non_upper_case_globals, non_camel_case_types, non_snake_case, diff --git a/xpc-connection/Cargo.toml b/xpc-connection/Cargo.toml index 5b62108..e234256 100644 --- a/xpc-connection/Cargo.toml +++ b/xpc-connection/Cargo.toml @@ -11,14 +11,20 @@ keywords = ["xpc", "mac", "macOS"] categories = ["os", "api-bindings", "concurrency", "encoding"] [features] -audit_token = [] +audit_token = ["core-foundation", "security-framework"] default = [] [dependencies] block = "0.1.6" -core-foundation = { version = "0.9", optional = true } futures = "0.3.4" xpc-connection-sys = { path = "../xpc-connection-sys", version = "0.1.0" } +# Remove when weak linkage is stable +libc = "0.2.97" + +# For the audit_token feature +core-foundation = { version = "0.9", optional = true } +security-framework = { version = "^2.3.1", optional = true } + [dev-dependencies] tokio = { version = "1", features = ["macros", "rt-multi-thread"] } diff --git a/xpc-connection/src/dlsym.rs b/xpc-connection/src/dlsym.rs new file mode 100644 index 0000000..fef81ed --- /dev/null +++ b/xpc-connection/src/dlsym.rs @@ -0,0 +1,50 @@ +// This is copied from mio which is MIT licensed, it's to work around the +// linkage feature not yet being stable. +// +// https://github.com/carllerche/mio/blob/master/src/sys/unix/dlsym.rs + +use std::marker; +use std::mem; +use std::sync::atomic::{AtomicUsize, Ordering}; + +macro_rules! dlsym { + (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + #[allow(bad_style)] + static $name: crate::dlsym::DlSym $ret> = + crate::dlsym::DlSym { + name: concat!(stringify!($name), "\0"), + addr: AtomicUsize::new(0), + _marker: ::std::marker::PhantomData, + }; + ) +} + +pub struct DlSym { + pub name: &'static str, + pub addr: AtomicUsize, + pub _marker: marker::PhantomData, +} + +impl DlSym { + pub fn get(&self) -> Option<&F> { + assert_eq!(mem::size_of::(), mem::size_of::()); + unsafe { + if self.addr.load(Ordering::SeqCst) == 0 { + self.addr.store(fetch(self.name), Ordering::SeqCst); + } + if self.addr.load(Ordering::SeqCst) == 1 { + None + } else { + mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr) + } + } + } +} + +unsafe fn fetch(name: &str) -> usize { + assert_eq!(name.as_bytes()[name.len() - 1], 0); + match libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr() as *const _) as usize { + 0 => 1, + n => n, + } +} diff --git a/xpc-connection/src/lib.rs b/xpc-connection/src/lib.rs index fb60388..ddc114b 100644 --- a/xpc-connection/src/lib.rs +++ b/xpc-connection/src/lib.rs @@ -1,6 +1,6 @@ #[allow( dead_code, - safe_packed_borrows, + unaligned_references, non_upper_case_globals, non_camel_case_types, non_snake_case, @@ -11,20 +11,34 @@ extern crate xpc_connection_sys; mod message; pub use message::*; +#[macro_use] +mod dlsym; + use block::ConcreteBlock; use futures::{ channel::mpsc::{unbounded as unbounded_channel, UnboundedReceiver, UnboundedSender}, Stream, }; -use std::ffi::CStr; -use std::{ffi::c_void, ops::Deref}; -use std::{pin::Pin, task::Poll}; +use std::{ + ffi::{CStr, CString}, + ops::Deref, + os::raw::{c_char, c_int}, + pin::Pin, + sync::atomic::AtomicUsize, + task::Poll, +}; + use xpc_connection_sys::{ - xpc_connection_cancel, xpc_connection_create_mach_service, xpc_connection_resume, - xpc_connection_send_message, xpc_connection_set_event_handler, xpc_connection_t, xpc_object_t, - xpc_release, XPC_CONNECTION_MACH_SERVICE_LISTENER, XPC_CONNECTION_MACH_SERVICE_PRIVILEGED, + dispatch_queue_t, xpc_connection_cancel, xpc_connection_create_mach_service, + xpc_connection_resume, xpc_connection_send_message, xpc_connection_set_event_handler, + xpc_connection_t, xpc_object_t, xpc_release, XPC_CONNECTION_MACH_SERVICE_LISTENER, + XPC_CONNECTION_MACH_SERVICE_PRIVILEGED, }; +dlsym! { + fn xpc_connection_set_peer_code_sig(*const c_char) -> c_int +} + // A connection's event handler could still be waiting or running when we want // to drop a connection. We must cancel the handler and wait for the final // call to a handler to occur, which is always a message containing an @@ -87,12 +101,35 @@ impl Stream for XpcListener { impl XpcListener { /// The connection must be a listener. - fn from_raw(connection: xpc_connection_t) -> XpcListener { + fn from_raw(connection: xpc_connection_t, requirement: Option<&'static str>) -> XpcListener { let (sender, receiver) = unbounded_channel(); let sender_clone = sender.clone(); + let mut already_validated = false; + + if let Some(requirement) = requirement { + if let Some(f) = crate::xpc_connection_set_peer_code_sig.get() { + let requirement = CString::new(requirement).expect("Invalid requirement string"); + unsafe { + f(requirement.as_ptr()); + } + + already_validated = true; + } + } + let block = ConcreteBlock::new(move |event| match xpc_object_to_message(event) { - Message::Client(client) => sender_clone.unbounded_send(client).ok(), + Message::Client(mut client) => { + if already_validated + || Self::validate_client_using_audit_token(&client, &requirement) + { + sender_clone.unbounded_send(client).ok() + } else { + unsafe { xpc_connection_cancel(client.connection) }; + client.event_handler_is_running = false; + None + } + } _ => None, }); @@ -113,13 +150,68 @@ impl XpcListener { } } - pub fn listen(name: impl AsRef) -> Self { + /// If `requirement` is set then clients will have their code signature + /// validated before being available. See Apple's documentation on the + /// language [here](https://developer.apple.com/library/archive/documentation/Security/Conceptual/CodeSigningGuide/RequirementLang/RequirementLang.html). + /// + /// On macOS 12 this uses `xpc_connection_set_peer_code_sig`, and if the + /// `audit_token` feature is enabled then this will use a custom + /// implementation on older versions of macOS. + /// + /// # Panics + /// + /// * If `audit_token` feature is used and the `requirement` isn't parsable + /// as a `SecRequirement`. This will occur during client validation. + pub fn listen( + name: impl AsRef, + requirement: Option<&'static str>, + queue: Option, + ) -> XpcListener { let name = name.as_ref(); let flags = XPC_CONNECTION_MACH_SERVICE_LISTENER as u64; - let connection = unsafe { - xpc_connection_create_mach_service(name.as_ref().as_ptr(), std::ptr::null_mut(), flags) + let queue = queue.unwrap_or(std::ptr::null_mut()); + + let connection = + unsafe { xpc_connection_create_mach_service(name.as_ref().as_ptr(), queue, flags) }; + + Self::from_raw(connection, requirement) + } + + #[inline] + #[cfg(feature = "audit_token")] + fn validate_client_using_audit_token(client: &XpcClient, requirement: &Option<&str>) -> bool { + use core_foundation::{base::TCFType, data::CFData}; + use security_framework::os::macos::code_signing::{Flags, GuestAttributes, SecCode}; + + let requirement = match requirement { + Some(r) => r, + None => return true, }; - Self::from_raw(connection) + + let requirement = requirement + .parse() + .expect("Unable to parse the requirement"); + + let token_data = CFData::from_buffer(&client.audit_token()); + let mut attrs = GuestAttributes::new(); + attrs.set_audit_token(token_data.as_concrete_TypeRef()); + + if let Ok(code_object) = SecCode::copy_guest_with_attribues(None, &attrs, Flags::NONE) { + return code_object + .check_validity(Flags::NONE, &requirement) + .is_ok(); + } + + false + } + + #[inline] + #[cfg(not(feature = "audit_token"))] + fn validate_client_using_audit_token(_client: &XpcClient, _requirement: &Option<&str>) -> bool { + // TODO: log an error: + // Attempted to use code signature requirements on an unsupported + // version of macOS without the `audit_token` feature enabled + false } } @@ -223,6 +315,8 @@ impl XpcClient { #[cfg(feature = "audit_token")] pub fn audit_token(&self) -> [u8; 32] { + use libc::c_void; + // This is a private API, but it's also required in order to // authenticate XPC clients without requiring a handshake. // See https://developer.apple.com/forums/thread/72881 for more info.