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 <sattard@anthropic.com>

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<audit_token_t> 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"])

shell/common/mac/codesign_util.cc

@@ -5,17 +5,22 @@
 
 #include "shell/common/mac/codesign_util.h"
 
+#include <CoreFoundation/CoreFoundation.h>
+#include <Security/Security.h>
+#include <mach/mach.h>
+#include <unistd.h>
+
 #include <optional>
 
 #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 <CoreFoundation/CoreFoundation.h>
-#include <Security/Security.h>
-
 namespace electron {
 
+namespace {
+
 std::optional<bool> IsUnsignedOrAdHocSigned(SecCodeRef code) {
   base::apple::ScopedCFTypeRef<SecStaticCodeRef> static_code;
   OSStatus status = SecCodeCopyStaticCode(code, kSecCSDefaultFlags,
@@ -59,13 +64,42 @@ std::optional<bool> IsUnsignedOrAdHocSigned(SecCodeRef code) {
   return false;
 }
 
-bool ProcessSignatureIsSameWithCurrentApp(pid_t pid) {
+}  // namespace
+
+std::optional<audit_token_t> 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<task_info_t>(&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<SecCodeRef> 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<bool> 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<CFNumberRef> 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<CFDataRef> audit_token_cf(
+      CFDataCreate(nullptr, reinterpret_cast<const UInt8*>(&audit_token),
+                   sizeof(audit_token_t)));
+  const void* attribute_keys[] = {kSecGuestAttributeAudit};
+  const void* attribute_values[] = {audit_token_cf.get()};
   base::apple::ScopedCFTypeRef<CFDictionaryRef> attributes(CFDictionaryCreate(
       nullptr, attribute_keys, attribute_values, std::size(attribute_keys),
       &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));

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 <unistd.h>
+#include <Security/Security.h>
+
+#include <optional>
 
 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<audit_token_t> 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