From 7455af030bddab9f115bdbca7236fdc361f8c8ae Mon Sep 17 00:00:00 2001 From: Josh Kaplan Date: Thu, 6 Jan 2022 21:36:31 +1300 Subject: [PATCH 1/2] Addresses issue #36 Factors out the security logic into a SecureMessageAcceptor struct. --- .../Server/SecureMessageAcceptor.swift | 70 +++++++++++++++++++ .../SecureXPC/Server/XPCAnonymousServer.swift | 12 ++-- Sources/SecureXPC/Server/XPCMachServer.swift | 66 ++--------------- Sources/SecureXPC/Server/XPCServer.swift | 6 +- .../Round-trip Integration Test.swift | 10 ++- 5 files changed, 96 insertions(+), 68 deletions(-) create mode 100644 Sources/SecureXPC/Server/SecureMessageAcceptor.swift diff --git a/Sources/SecureXPC/Server/SecureMessageAcceptor.swift b/Sources/SecureXPC/Server/SecureMessageAcceptor.swift new file mode 100644 index 0000000..d17533d --- /dev/null +++ b/Sources/SecureXPC/Server/SecureMessageAcceptor.swift @@ -0,0 +1,70 @@ +// +// SecureMessageAcceptor.swift +// SecureXPC +// +// Created by Josh Kaplan on 2022-01-06 +// + +import Foundation + +/// Accepts messages which meet the provided code signing requirements. +/// +/// Uses undocumented functionality prior to macOS 11. +internal struct SecureMessageAcceptor { + /// At least one of these code signing requirements must be met in order for the message to be accepted + internal let requirements: [SecRequirement] + + func acceptMessage(connection: xpc_connection_t, message: xpc_object_t) -> Bool { + // Get the code representing the client + var code: SecCode? + if #available(macOS 11, *) { // publicly documented, but only available since macOS 11 + SecCodeCreateWithXPCMessage(message, SecCSFlags(), &code) + } else { // private undocumented function: xpc_connection_get_audit_token, available on prior versions of macOS + if var auditToken = xpc_connection_get_audit_token(connection) { + let tokenData = NSData(bytes: &auditToken, length: MemoryLayout.size(ofValue: auditToken)) + let attributes = [kSecGuestAttributeAudit : tokenData] as NSDictionary + SecCodeCopyGuestWithAttributes(nil, attributes, SecCSFlags(), &code) + } + } + + // Accept message if code is valid and meets any of the client requirements + var accept = false + if let code = code { + for requirement in self.requirements { + if SecCodeCheckValidity(code, SecCSFlags(), requirement) == errSecSuccess { + accept = true + } + } + } + + return accept + } + + /// Wrapper around the private undocumented function `void xpc_connection_get_audit_token(xpc_connection_t, audit_token_t *)`. + /// + /// The private undocumented function will attempt to be dynamically loaded and then invoked. If no function exists with this name `nil` will be returned. If + /// the function does exist, but does not match the expected signature, the process calling this function is expected to crash. However, because this is only + /// called on older versions of macOS which are expected to have a stable non-changing API this is very unlikely to occur. + /// + /// - Parameters: + /// - _: The connection for which the audit token will be retrieved for. + /// - Returns: The audit token or `nil` if the function could not be called. + private func xpc_connection_get_audit_token(_ connection: xpc_connection_t) -> audit_token_t? { + // Attempt to dynamically load the function + guard let handle = dlopen(nil, RTLD_LAZY) else { + return nil + } + defer { dlclose(handle) } + guard let sym = dlsym(handle, "xpc_connection_get_audit_token") else { + return nil + } + typealias functionSignature = @convention(c) (xpc_connection_t, UnsafeMutablePointer) -> Void + let function = unsafeBitCast(sym, to: functionSignature.self) + + // Call the function + var token = audit_token_t() + function(connection, &token) + + return token + } +} diff --git a/Sources/SecureXPC/Server/XPCAnonymousServer.swift b/Sources/SecureXPC/Server/XPCAnonymousServer.swift index 77a24af..31e4616 100644 --- a/Sources/SecureXPC/Server/XPCAnonymousServer.swift +++ b/Sources/SecureXPC/Server/XPCAnonymousServer.swift @@ -1,6 +1,6 @@ // // XPCAnonymousServer.swift -// +// SecureXPC // // Created by Alexander Momchilov on 2021-11-28. // @@ -9,9 +9,13 @@ import Foundation internal class XPCAnonymousServer: XPCServer { private let anonymousListenerConnection: xpc_connection_t + + /// Determines if an incoming request can be handled based on the provided client requirements + private let messageAcceptor: SecureMessageAcceptor - internal override init() { + internal init(clientRequirements: [SecRequirement]) { self.anonymousListenerConnection = xpc_connection_create(nil, nil) + self.messageAcceptor = SecureMessageAcceptor(requirements: clientRequirements) super.init() // Start listener for the new anonymous connection, all received events should be for incoming client connections @@ -25,9 +29,7 @@ internal class XPCAnonymousServer: XPCServer { } internal override func acceptMessage(connection: xpc_connection_t, message: xpc_object_t) -> Bool { - // Anonymous service connections should only ever passed among trusted parties. - // TODO: add support for client security requirements https://github.com/trilemma-dev/SecureXPC/issues/36 - true + self.messageAcceptor.acceptMessage(connection: connection, message: message) } /// Begins processing requests received by this XPC server and never returns. diff --git a/Sources/SecureXPC/Server/XPCMachServer.swift b/Sources/SecureXPC/Server/XPCMachServer.swift index 697d160..7472b1c 100644 --- a/Sources/SecureXPC/Server/XPCMachServer.swift +++ b/Sources/SecureXPC/Server/XPCMachServer.swift @@ -16,13 +16,13 @@ internal class XPCMachServer: XPCServer, NonBlockingStartable { private let machServiceName: String /// Receives new incoming connections, created once the server is started. private var listenerConnection: xpc_connection_t? - /// The code signing requirements a client must match in order for an incoming request to be handled. - private let clientRequirements: [SecRequirement] + /// Determines if an incoming request will be accepted based on the provided client requirements + private let messageAcceptor: SecureMessageAcceptor /// This should only ever be called from `getXPCMachServer(...)` so that client requirement invariants are upheld. private init(machServiceName: String, clientRequirements: [SecRequirement]) { self.machServiceName = machServiceName - self.clientRequirements = clientRequirements + self.messageAcceptor = SecureMessageAcceptor(requirements: clientRequirements) } /// Cache of servers with the machServiceName as the key. @@ -63,7 +63,8 @@ internal class XPCMachServer: XPCServer, NonBlockingStartable { // Turn into sets so they can be compared without taking into account the order of requirements let requirementsData = Set(try clientRequirements.map(requirementTransform)) - let cachedRequirementsData = Set(try cachedServer.clientRequirements.map(requirementTransform)) + let cachedRequirementsData = Set(try cachedServer.messageAcceptor.requirements + .map(requirementTransform)) guard requirementsData == cachedRequirementsData else { throw XPCError.conflictingClientRequirements } @@ -177,29 +178,7 @@ internal class XPCMachServer: XPCServer, NonBlockingStartable { } internal override func acceptMessage(connection: xpc_connection_t, message: xpc_object_t) -> Bool { - // Get the code representing the client - var code: SecCode? - if #available(macOS 11, *) { // publicly documented, but only available since macOS 11 - SecCodeCreateWithXPCMessage(message, SecCSFlags(), &code) - } else { // private undocumented function: xpc_connection_get_audit_token, available on prior versions of macOS - if var auditToken = xpc_connection_get_audit_token(connection) { - let tokenData = NSData(bytes: &auditToken, length: MemoryLayout.size(ofValue: auditToken)) - let attributes = [kSecGuestAttributeAudit : tokenData] as NSDictionary - SecCodeCopyGuestWithAttributes(nil, attributes, SecCSFlags(), &code) - } - } - - // Accept message if code is valid and meets any of the client requirements - var accept = false - if let code = code { - for requirement in self.clientRequirements { - if SecCodeCheckValidity(code, SecCSFlags(), requirement) == errSecSuccess { - accept = true - } - } - } - - return accept + self.acceptMessage(connection: connection, message: message) } public override var endpoint: XPCServerEndpoint { @@ -219,39 +198,6 @@ internal class XPCMachServer: XPCServer, NonBlockingStartable { endpoint: endpoint ) } - - /// Wrapper around the private undocumented function `void xpc_connection_get_audit_token(xpc_connection_t, audit_token_t *)`. - /// - /// The private undocumented function will attempt to be dynamically loaded and then invoked. If no function exists with this name `nil` will be returned. If - /// the function does exist, but does not match the expected signature, the process calling this function is expected to crash. However, because this is only - /// called on older versions of macOS which are expected to have a stable non-changing API this is very unlikely to occur. - /// - /// - Parameters: - /// - _: The connection for which the audit token will be retrieved for. - /// - Returns: The audit token or `nil` if the function could not be called. - private func xpc_connection_get_audit_token(_ connection: xpc_connection_t) -> audit_token_t? { - typealias functionSignature = @convention(c) (xpc_connection_t, UnsafeMutablePointer) -> Void - let auditToken: audit_token_t? - - // Attempt to dynamically load the function - if let handle = dlopen(nil, RTLD_LAZY) { - defer { dlclose(handle) } - if let sym = dlsym(handle, "xpc_connection_get_audit_token") { - let function = unsafeBitCast(sym, to: functionSignature.self) - - // Call the function - var token = audit_token_t() - function(connection, &token) - auditToken = token - } else { - auditToken = nil - } - } else { - auditToken = nil - } - - return auditToken - } } extension XPCMachServer: CustomDebugStringConvertible { diff --git a/Sources/SecureXPC/Server/XPCServer.swift b/Sources/SecureXPC/Server/XPCServer.swift index 968fc59..01642a0 100644 --- a/Sources/SecureXPC/Server/XPCServer.swift +++ b/Sources/SecureXPC/Server/XPCServer.swift @@ -112,8 +112,10 @@ public class XPCServer { try XPCServiceServer._forThisXPCService() } - internal static func makeAnonymousService() -> XPCServer & NonBlockingStartable { - XPCAnonymousServer() + internal static func makeAnonymousService( + clientRequirements: [SecRequirement] + ) -> XPCServer & NonBlockingStartable { + XPCAnonymousServer(clientRequirements: clientRequirements) } /// Provides a server for this helper tool if it was installed with diff --git a/Tests/SecureXPCTests/Client & Server/Round-trip Integration Test.swift b/Tests/SecureXPCTests/Client & Server/Round-trip Integration Test.swift index 0bb6d44..d39b7fb 100644 --- a/Tests/SecureXPCTests/Client & Server/Round-trip Integration Test.swift +++ b/Tests/SecureXPCTests/Client & Server/Round-trip Integration Test.swift @@ -11,7 +11,15 @@ import XCTest class RoundTripIntegrationTest: XCTestCase { var xpcClient: XPCClient! = nil - let anonymousServer = XPCServer.makeAnonymousService() + static let dummyRequirements: [SecRequirement] = { + var requirement: SecRequirement? + // This is a worthless security requirement which should always result in the connection being accepted + SecRequirementCreateWithString("info [CFBundleVersion] exists" as CFString, SecCSFlags(), &requirement) + + return [requirement!] + }() + + let anonymousServer = XPCServer.makeAnonymousService(clientRequirements: RoundTripIntegrationTest.dummyRequirements) override func setUp() { let endpoint = anonymousServer.endpoint From 27c2ea5b5acee8dd95ed0bd9e506b302dc551f99 Mon Sep 17 00:00:00 2001 From: Josh Kaplan Date: Fri, 7 Jan 2022 17:40:15 +1300 Subject: [PATCH 2/2] Incorporates feedback for `acceptMessage(...)` function --- .../Server/SecureMessageAcceptor.swift | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/Sources/SecureXPC/Server/SecureMessageAcceptor.swift b/Sources/SecureXPC/Server/SecureMessageAcceptor.swift index d17533d..a2b77d5 100644 --- a/Sources/SecureXPC/Server/SecureMessageAcceptor.swift +++ b/Sources/SecureXPC/Server/SecureMessageAcceptor.swift @@ -20,24 +20,21 @@ internal struct SecureMessageAcceptor { if #available(macOS 11, *) { // publicly documented, but only available since macOS 11 SecCodeCreateWithXPCMessage(message, SecCSFlags(), &code) } else { // private undocumented function: xpc_connection_get_audit_token, available on prior versions of macOS - if var auditToken = xpc_connection_get_audit_token(connection) { - let tokenData = NSData(bytes: &auditToken, length: MemoryLayout.size(ofValue: auditToken)) - let attributes = [kSecGuestAttributeAudit : tokenData] as NSDictionary - SecCodeCopyGuestWithAttributes(nil, attributes, SecCSFlags(), &code) + guard var auditToken = xpc_connection_get_audit_token(connection) else { + return false } + + let tokenData = NSData(bytes: &auditToken, length: MemoryLayout.size(ofValue: auditToken)) + let attributes = [kSecGuestAttributeAudit : tokenData] as NSDictionary + SecCodeCopyGuestWithAttributes(nil, attributes, SecCSFlags(), &code) } // Accept message if code is valid and meets any of the client requirements - var accept = false - if let code = code { - for requirement in self.requirements { - if SecCodeCheckValidity(code, SecCSFlags(), requirement) == errSecSuccess { - accept = true - } - } + guard let code = code else { + return false } - return accept + return self.requirements.contains { SecCodeCheckValidity(code, SecCSFlags(), $0) == errSecSuccess } } /// Wrapper around the private undocumented function `void xpc_connection_get_audit_token(xpc_connection_t, audit_token_t *)`.