From 120be8544d877c13caef8702d47a4970c4e348e2 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Sat, 11 Apr 2026 19:05:43 -0700 Subject: [PATCH] fix: use audit token instead of PID for parent code-signature check (#50934) Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Sam Attard --- shell/app/node_main.cc | 7 +++- shell/common/mac/codesign_util.cc | 58 +++++++++++++++++++++++++------ shell/common/mac/codesign_util.h | 17 ++++++--- 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/shell/app/node_main.cc b/shell/app/node_main.cc index 7cd0a71eea..52e1fe5bd1 100644 --- a/shell/app/node_main.cc +++ b/shell/app/node_main.cc @@ -133,7 +133,12 @@ int NodeMain() { } #if BUILDFLAG(IS_MAC) - if (!ProcessSignatureIsSameWithCurrentApp(getppid())) { + // Capture the parent's audit token as early as possible. The audit token + // (unlike a raw PID) is bound to a single process instance. + std::optional parent_audit_token = + GetParentProcessAuditToken(); + if (!parent_audit_token || + !ProcessSignatureIsSameWithCurrentApp(*parent_audit_token)) { // On macOS, it is forbidden to run sandboxed app with custom arguments // from another app, i.e. args are discarded in following call: // exec("Sandboxed.app", ["--custom-args-will-be-discarded"]) diff --git a/shell/common/mac/codesign_util.cc b/shell/common/mac/codesign_util.cc index d40275d897..cb720692c2 100644 --- a/shell/common/mac/codesign_util.cc +++ b/shell/common/mac/codesign_util.cc @@ -5,17 +5,22 @@ #include "shell/common/mac/codesign_util.h" +#include +#include +#include +#include + #include #include "base/apple/foundation_util.h" +#include "base/apple/mach_logging.h" #include "base/apple/osstatus_logging.h" #include "base/apple/scoped_cftyperef.h" -#include -#include - namespace electron { +namespace { + std::optional IsUnsignedOrAdHocSigned(SecCodeRef code) { base::apple::ScopedCFTypeRef static_code; OSStatus status = SecCodeCopyStaticCode(code, kSecCSDefaultFlags, @@ -59,13 +64,42 @@ std::optional IsUnsignedOrAdHocSigned(SecCodeRef code) { return false; } -bool ProcessSignatureIsSameWithCurrentApp(pid_t pid) { +} // namespace + +std::optional GetParentProcessAuditToken() { + pid_t parent_pid = getppid(); + + // task_name_for_pid() yields a name port (not a control port), so it does + // not require any special entitlement. Same approach as Chromium's + // remoting/host/webauthn/remote_webauthn_caller_security_utils.cc. + task_name_t parent_task; + kern_return_t kr = + task_name_for_pid(mach_task_self(), parent_pid, &parent_task); + if (kr != KERN_SUCCESS) { + MACH_LOG(ERROR, kr) << "task_name_for_pid"; + return std::nullopt; + } + + audit_token_t token; + mach_msg_type_number_t size = TASK_AUDIT_TOKEN_COUNT; + kr = task_info(parent_task, TASK_AUDIT_TOKEN, + reinterpret_cast(&token), &size); + mach_port_deallocate(mach_task_self(), parent_task); + if (kr != KERN_SUCCESS) { + MACH_LOG(ERROR, kr) << "task_info(TASK_AUDIT_TOKEN)"; + return std::nullopt; + } + + return token; +} + +bool ProcessSignatureIsSameWithCurrentApp(audit_token_t audit_token) { // Get and check the code signature of current app. base::apple::ScopedCFTypeRef self_code; OSStatus status = SecCodeCopySelf(kSecCSDefaultFlags, self_code.InitializeInto()); if (status != errSecSuccess) { - OSSTATUS_LOG(ERROR, status) << "SecCodeCopyGuestWithAttributes"; + OSSTATUS_LOG(ERROR, status) << "SecCodeCopySelf"; return false; } std::optional not_signed = IsUnsignedOrAdHocSigned(self_code.get()); @@ -77,11 +111,15 @@ bool ProcessSignatureIsSameWithCurrentApp(pid_t pid) { // Current app is not signed. return true; } - // Get the code signature of process. - base::apple::ScopedCFTypeRef process_cf( - CFNumberCreate(nullptr, kCFNumberIntType, &pid)); - const void* attribute_keys[] = {kSecGuestAttributePid}; - const void* attribute_values[] = {process_cf.get()}; + // Get the code signature of process. kSecGuestAttributeAudit is preferred + // over kSecGuestAttributePid because the audit token is bound to a single + // process instance and cannot be spoofed by PID reuse or by the target + // exec()ing into a different image after the lookup. + base::apple::ScopedCFTypeRef audit_token_cf( + CFDataCreate(nullptr, reinterpret_cast(&audit_token), + sizeof(audit_token_t))); + const void* attribute_keys[] = {kSecGuestAttributeAudit}; + const void* attribute_values[] = {audit_token_cf.get()}; base::apple::ScopedCFTypeRef attributes(CFDictionaryCreate( nullptr, attribute_keys, attribute_values, std::size(attribute_keys), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks)); diff --git a/shell/common/mac/codesign_util.h b/shell/common/mac/codesign_util.h index 1fabc4fe42..5d94ca8f06 100644 --- a/shell/common/mac/codesign_util.h +++ b/shell/common/mac/codesign_util.h @@ -6,18 +6,27 @@ #ifndef SHELL_COMMON_MAC_CODESIGN_UTIL_H_ #define SHELL_COMMON_MAC_CODESIGN_UTIL_H_ -#include +#include + +#include namespace electron { -// Given a pid, return true if the process has the same code signature with -// with current app. +// Retrieves the audit token of the parent process via its task name port. +// Callers should invoke this as early in process startup as possible. +std::optional GetParentProcessAuditToken(); + +// Given a process audit token, return true if the process has the same code +// signature as the current app. An audit token is bound to a single process +// instance (it incorporates a per-exec version) so, unlike a PID, it cannot be +// reused by a process that exec()s into a different image after the token is +// captured. // This API returns true if current app is not signed or ad-hoc signed, because // checking code signature is meaningless in this case, and failing the // signature check would break some features with unsigned binary (for example, // process.send stops working in processes created by child_process.fork, due // to the NODE_CHANNEL_ID env getting removed). -bool ProcessSignatureIsSameWithCurrentApp(pid_t pid); +bool ProcessSignatureIsSameWithCurrentApp(audit_token_t audit_token); } // namespace electron