From b2f5f3ff2f8c6b47a34f13af91cb74e38e8071ce Mon Sep 17 00:00:00 2001 From: J Logan Date: Wed, 19 Nov 2025 00:41:56 -0300 Subject: [PATCH] Adds client uid validation to XPC server. (#896) - When a user performs an `su` the effective UID changes but the bootstrap mach port does not, so that if container is running as `alice` from a GUI login session, it's possible to `su bob` and continue running container. While this doesn't pose a significant security risk as it's necessary for Alice to know Bob's password and manually enter it with `su`, this change closes the loophole by validating that client UID from the caller's audit token matches that of the API server. --- Package.swift | 9 ++++ Sources/CAuditToken/include/AuditToken.h | 20 +++++++ Sources/ContainerXPC/XPCServer.swift | 67 ++++++++++++++++++++---- 3 files changed, 87 insertions(+), 9 deletions(-) create mode 100644 Sources/CAuditToken/include/AuditToken.h diff --git a/Package.swift b/Package.swift index 69aa5b0c..2b36d3b1 100644 --- a/Package.swift +++ b/Package.swift @@ -303,6 +303,7 @@ let package = Package( dependencies: [ .product(name: "ContainerizationExtras", package: "containerization"), .product(name: "Logging", package: "swift-log"), + "CAuditToken", ] ), .target( @@ -373,7 +374,15 @@ let package = Package( .define("GIT_COMMIT", to: "\"\(gitCommit)\""), .define("RELEASE_VERSION", to: "\"\(releaseVersion)\""), .define("BUILDER_SHIM_VERSION", to: "\"\(builderShimVersion)\""), + ], + linkerSettings: [ + .linkedLibrary("bsm") ] ), + .target( + name: "CAuditToken", + dependencies: [], + publicHeadersPath: "include" + ), ] ) diff --git a/Sources/CAuditToken/include/AuditToken.h b/Sources/CAuditToken/include/AuditToken.h new file mode 100644 index 00000000..802b828d --- /dev/null +++ b/Sources/CAuditToken/include/AuditToken.h @@ -0,0 +1,20 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +#include +#include + +void xpc_dictionary_get_audit_token(xpc_object_t xdict, audit_token_t *token); diff --git a/Sources/ContainerXPC/XPCServer.swift b/Sources/ContainerXPC/XPCServer.swift index a5b124ee..7eb81477 100644 --- a/Sources/ContainerXPC/XPCServer.swift +++ b/Sources/ContainerXPC/XPCServer.swift @@ -15,6 +15,7 @@ //===----------------------------------------------------------------------===// #if os(macOS) +import CAuditToken import ContainerizationError import Foundation import Logging @@ -25,7 +26,7 @@ public struct XPCServer: Sendable { public typealias RouteHandler = @Sendable (XPCMessage) async throws -> XPCMessage private let routes: [String: RouteHandler] - // Access to `connection` is protected by a lock + // Access to `connection` is protected by a lock. private nonisolated(unsafe) let connection: xpc_connection_t private let lock = NSLock() @@ -112,8 +113,10 @@ public struct XPCServer: Sendable { cont.finish() } if !(replySent.withLock({ $0 }) && object.connectionClosed) { - // When a xpc connection is closed, the framework sends a final XPC_ERROR_CONNECTION_INVALID message. - // We can ignore this if we know we have already handled the request. + // When a xpc connection is closed, the framework sends + // a final XPC_ERROR_CONNECTION_INVALID message. + // We can ignore this if we know we have already handled + // the request. self.log.error("xpc client handler connection error \(object.errorDescription ?? "no description")") } default: @@ -145,24 +148,63 @@ public struct XPCServer: Sendable { } func handleMessage(connection: xpc_connection_t, object: xpc_object_t) async throws { + // All requests are dictionary-valued. + guard xpc_get_type(object) == XPC_TYPE_DICTIONARY else { + log.error("invalid request - not a dictionary") + Self.replyWithError( + connection: connection, + object: object, + err: ContainerizationError(.invalidArgument, message: "invalid request") + ) + return + } + + // Ensure that the client has our EUID + var token = audit_token_t() + xpc_dictionary_get_audit_token(object, &token) + let serverEuid = geteuid() + let clientEuid = audit_token_to_euid(token) + guard clientEuid == serverEuid else { + log.error( + "unauthorized request - uid mismatch", + metadata: [ + "server_euid": "\(serverEuid)", + "client_euid": "\(clientEuid)", + ]) + Self.replyWithError( + connection: connection, + object: object, + err: ContainerizationError(.invalidState, message: "unauthorized request") + ) + return + } + guard let route = object.route else { - log.error("empty route") + log.error("invalid request - empty route") + Self.replyWithError( + connection: connection, + object: object, + err: ContainerizationError(.invalidArgument, message: "invalid request") + ) return } if let handler = routes[route] { - let message = XPCMessage(object: object) do { + let message = XPCMessage(object: object) let response = try await handler(message) xpc_connection_send_message(connection, response.underlying) } catch let error as ContainerizationError { - let reply = message.reply() log.error("handler for \(route) threw error \(error)") - reply.set(error: error) - xpc_connection_send_message(connection, reply.underlying) + Self.replyWithError( + connection: connection, + object: object, + err: error + ) } catch { - let reply = message.reply() log.error("handler for \(route) threw error \(error)") + let message = XPCMessage(object: object) + let reply = message.reply() // Check if this is a VolumeError by looking at the error description let errorMessage = error.localizedDescription @@ -178,6 +220,13 @@ public struct XPCServer: Sendable { } } } + + private static func replyWithError(connection: xpc_connection_t, object: xpc_object_t, err: ContainerizationError) { + let message = XPCMessage(object: object) + let reply = message.reply() + reply.set(error: err) + xpc_connection_send_message(connection, reply.underlying) + } } extension xpc_object_t {