Bug 1505445 - [Mac] With sandbox early startup, start the sandbox after the port exchange r=Alex_Gaynor

Don't start the sandbox until after the port exchange so the parent process does not have to wait longer in ContentParent::LaunchSubprocess() for the (expensive) sandbox_init_with_parameters call to complete in the child. Remove the policy rule allowing access to the parent port now that it is already open when the sandbox is initialized and therefore not needed.

Differential Revision: https://phabricator.services.mozilla.com/D11186

--HG--
extra : moz-landing-system : lando

Author: hafta

browser/app/nsBrowserApp.cpp

@@ -43,10 +43,6 @@
 #include "FuzzerDefs.h"
 #endif
 
-#if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
-#include "mozilla/Sandbox.h"
-#endif
-
 #ifdef MOZ_LINUX_32_SSE2_STARTUP_ERROR
 #include <cpuid.h>
 #include "mozilla/Unused.h"
@@ -267,16 +263,6 @@ int main(int argc, char* argv[], char* envp[])
 {
   mozilla::TimeStamp start = mozilla::TimeStamp::Now();
 
-#if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
-  if (argc > 1 && IsArg(argv[1], "contentproc")) {
-    std::string err;
-    if (!mozilla::EarlyStartMacSandboxIfEnabled(argc, argv, err)) {
-      Output("Sandbox error: %s\n", err.c_str());
-      MOZ_CRASH("Sandbox initialization failed");
-    }
-  }
-#endif
-
 #ifdef MOZ_BROWSER_CAN_BE_CONTENTPROC
   // We are launching as a content process, delegate to the appropriate
   // main

ipc/app/MozillaRuntimeMain.cpp

@@ -9,23 +9,11 @@
 #include "mozilla/Bootstrap.h"
 #include "mozilla/WindowsDllBlocklist.h"
 
-#if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
-#include "mozilla/Sandbox.h"
-#endif
-
 using namespace mozilla;
 
 int
 main(int argc, char *argv[])
 {
-#if defined(XP_MACOSX) && defined(MOZ_CONTENT_SANDBOX)
-  std::string err;
-  if (!mozilla::EarlyStartMacSandboxIfEnabled(argc, argv, err)) {
-    fprintf(stderr, "Sandbox error: %s\n", err.c_str());
-    MOZ_CRASH("Sandbox initialization failed");
-  }
-#endif
-
 #ifdef HAS_DLL_BLOCKLIST
   DllBlocklist_Initialize(eDllBlocklistInitFlagIsChildProcess);
 #endif

security/sandbox/mac/Sandbox.h

@@ -69,7 +69,6 @@ typedef struct _MacSandboxInfo {
   std::string testingReadPath3;
   std::string testingReadPath4;
 
-  std::string parentPort;
   std::string crashServerPort;
 
   bool shouldLog;

security/sandbox/mac/Sandbox.mm

@@ -234,10 +234,6 @@ bool StartMacSandbox(MacSandboxInfo const &aInfo, std::string &aErrorMessage)
       params.push_back(aInfo.hasSandboxedProfile ? "TRUE" : "FALSE");
       params.push_back("HAS_WINDOW_SERVER");
       params.push_back(aInfo.hasWindowServer ? "TRUE" : "FALSE");
-      if (!aInfo.parentPort.empty()) {
-        params.push_back("PARENT_PORT");
-        params.push_back(aInfo.parentPort.c_str());
-      }
       if (!aInfo.crashServerPort.empty()) {
         params.push_back("CRASH_PORT");
         params.push_back(aInfo.crashServerPort.c_str());
@@ -342,7 +338,6 @@ bool StartMacSandbox(MacSandboxInfo const &aInfo, std::string &aErrorMessage)
   // line arguments. Return false if any are missing.
   bool foundSandboxLevel = false;
   bool foundValidSandboxLevel = false;
-  bool foundParentPort = false;
   bool foundAppPath = false;
 
   // Read access directories used in testing
@@ -416,13 +411,7 @@ bool StartMacSandbox(MacSandboxInfo const &aInfo, std::string &aErrorMessage)
     }
 #endif // DEBUG
 
-    // Handle positional arguments
-    if (strstr(aArgv[i], "org.mozilla.machname") != NULL) {
-      foundParentPort = true;
-      aInfo.parentPort.assign(aArgv[i]);
-      continue;
-    }
-
+    // Handle crash server positional argument
     if (strstr(aArgv[i], "gecko-crash-server-pipe") != NULL) {
       aInfo.crashServerPort.assign(aArgv[i]);
       continue;
@@ -441,12 +430,6 @@ bool StartMacSandbox(MacSandboxInfo const &aInfo, std::string &aErrorMessage)
     return false;
   }
 
-  if (!foundParentPort) {
-    fprintf(stderr, "Content sandbox disabled due to "
-                    "missing sandbox CLI parent port parameter.\n");
-    return false;
-  }
-
   if (!foundAppPath) {
     fprintf(stderr, "Content sandbox disabled due to "
                     "missing sandbox CLI app path parameter.\n");

security/sandbox/mac/SandboxPolicies.h

@@ -61,7 +61,6 @@ static const char contentSandboxRules[] = R"SANDBOX_LITERAL(
   (define testingReadPath2 (param "TESTING_READ_PATH2"))
   (define testingReadPath3 (param "TESTING_READ_PATH3"))
   (define testingReadPath4 (param "TESTING_READ_PATH4"))
-  (define parentPort (param "PARENT_PORT"))
   (define crashPort (param "CRASH_PORT"))
 
   (if (string=? should-log "TRUE")
@@ -188,8 +187,6 @@ static const char contentSandboxRules[] = R"SANDBOX_LITERAL(
     (ipc-posix-name-regex #"^CFPBS:"))
 
   (allow signal (target self))
-  (if (string? parentPort)
-    (allow mach-lookup (global-name parentPort)))
   (if (string? crashPort)
     (allow mach-lookup (global-name crashPort)))
   (if (string=? hasWindowServer "TRUE")

toolkit/xre/nsEmbedFunctions.cpp

@@ -442,6 +442,13 @@ XRE_InitChildProcess(int aArgc,
 #ifdef XP_MACOSX
   if (aArgc < 1)
     return NS_ERROR_FAILURE;
+
+#if defined(MOZ_CONTENT_SANDBOX)
+  // Save the original number of arguments to pass to the sandbox
+  // setup routine which also uses the crash server argument.
+  int allArgc = aArgc;
+#endif /* MOZ_CONTENT_SANDBOX */
+
   const char* const mach_port_name = aArgv[--aArgc];
 
   Maybe<recordreplay::AutoPassThroughThreadEvents> pt;
@@ -513,8 +520,16 @@ XRE_InitChildProcess(int aArgc,
     return NS_ERROR_FAILURE;
   }
 
+#if defined(MOZ_CONTENT_SANDBOX)
+  std::string sandboxError;
+  if (!EarlyStartMacSandboxIfEnabled(allArgc, aArgv, sandboxError)) {
+    printf_stderr("Sandbox error: %s\n", sandboxError.c_str());
+    MOZ_CRASH("Sandbox initialization failed");
+  }
+#endif /* MOZ_CONTENT_SANDBOX */
+
   pt.reset();
-#endif
+#endif /* XP_MACOSX */
 
   SetupErrorHandling(aArgv[0]);