Bug 1505445 - [Mac] With sandbox early startup, start the sandbox after the port exchange r=Alex_Gaynor
authorHaik Aftandilian <haftandilian@mozilla.com>
Thu, 08 Nov 2018 21:04:19 +0000
changeset 445208 b246415f6864c04591ea05ca9e06e1cd7e6f9ca2
parent 445207 853e4a06fb03aee6c1a732a8d594a3bfd14d97a2
child 445209 d6644d7251e9834a0ef21ae869a75ce0260a5f9e
push id35012
push userbtara@mozilla.com
push dateFri, 09 Nov 2018 05:26:19 +0000
treeherdermozilla-central@63eb34f9b171 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersAlex_Gaynor
bugs1505445
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
browser/app/nsBrowserApp.cpp
ipc/app/MozillaRuntimeMain.cpp
security/sandbox/mac/Sandbox.h
security/sandbox/mac/Sandbox.mm
security/sandbox/mac/SandboxPolicies.h
toolkit/xre/nsEmbedFunctions.cpp
--- a/browser/app/nsBrowserApp.cpp
+++ b/browser/app/nsBrowserApp.cpp
@@ -38,20 +38,16 @@
 #include "mozilla/Sprintf.h"
 #include "mozilla/StartupTimeline.h"
 #include "mozilla/WindowsDllBlocklist.h"
 
 #ifdef LIBFUZZER
 #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"
 
 static bool
 IsSSE2Available()
 {
   // The rest of the app has been compiled to assume that SSE2 is present
@@ -262,26 +258,16 @@ InitXPCOMGlue()
 // NB: This must be extern, as this value is checked elsewhere
 uint32_t gBlocklistInitFlags = eDllBlocklistInitFlagDefault;
 #endif
 
 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
   if (argc > 1 && IsArg(argv[1], "contentproc")) {
 #ifdef HAS_DLL_BLOCKLIST
     DllBlocklist_Initialize(eDllBlocklistInitFlagIsChildProcess);
 #endif
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
--- a/ipc/app/MozillaRuntimeMain.cpp
+++ b/ipc/app/MozillaRuntimeMain.cpp
@@ -4,33 +4,21 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "../contentproc/plugin-container.cpp"
 
 #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
 
   Bootstrap::UniquePtr bootstrap = GetBootstrap();
   if (!bootstrap) {
     return 2;
   }
--- a/security/sandbox/mac/Sandbox.h
+++ b/security/sandbox/mac/Sandbox.h
@@ -64,17 +64,16 @@ typedef struct _MacSandboxInfo {
   std::string profileDir;
   std::string debugWriteDir;
 
   std::string testingReadPath1;
   std::string testingReadPath2;
   std::string testingReadPath3;
   std::string testingReadPath4;
 
-  std::string parentPort;
   std::string crashServerPort;
 
   bool shouldLog;
 } MacSandboxInfo;
 
 namespace mozilla {
 
 bool StartMacSandbox(MacSandboxInfo const &aInfo, std::string &aErrorMessage);
--- a/security/sandbox/mac/Sandbox.mm
+++ b/security/sandbox/mac/Sandbox.mm
@@ -229,20 +229,16 @@ bool StartMacSandbox(MacSandboxInfo cons
       params.push_back("PROFILE_DIR");
       params.push_back(aInfo.profileDir.c_str());
       params.push_back("HOME_PATH");
       params.push_back(getenv("HOME"));
       params.push_back("HAS_SANDBOXED_PROFILE");
       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());
       }
       if (!aInfo.testingReadPath1.empty()) {
         params.push_back("TESTING_READ_PATH1");
         params.push_back(aInfo.testingReadPath1.c_str());
       }
@@ -337,17 +333,16 @@ bool StartMacSandbox(MacSandboxInfo cons
  */
 bool
 GetContentSandboxParamsFromArgs(int aArgc, char** aArgv, MacSandboxInfo& aInfo)
 {
   // Ensure we find these paramaters in the command
   // 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
   int nTestingReadPaths = 0;
   std::string testingReadPaths[MAX_TESTING_READ_PATHS] = {};
 
   // Collect sandbox params from CLI arguments
   for (int i = 0; i < aArgc; i++) {
@@ -411,23 +406,17 @@ GetContentSandboxParamsFromArgs(int aArg
 #ifdef DEBUG
     if ((strcmp(aArgv[i], "-sbDebugWriteDir") == 0) && (i + 1 < aArgc)) {
       aInfo.debugWriteDir.assign(aArgv[i+1]);
       i++;
       continue;
     }
 #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;
     }
   }
 
   if (!foundSandboxLevel) {
     fprintf(stderr, "Content sandbox disabled due to "
@@ -436,22 +425,16 @@ GetContentSandboxParamsFromArgs(int aArg
   }
 
   if (!foundValidSandboxLevel) {
     fprintf(stderr, "Content sandbox disabled due to invalid"
                     "sandbox level (%d)\n", aInfo.level);
     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");
     return false;
   }
 
   aInfo.testingReadPath1 = testingReadPaths[0];
   aInfo.testingReadPath2 = testingReadPaths[1];
--- a/security/sandbox/mac/SandboxPolicies.h
+++ b/security/sandbox/mac/SandboxPolicies.h
@@ -56,17 +56,16 @@ static const char contentSandboxRules[] 
   (define profileDir (param "PROFILE_DIR"))
   (define hasWindowServer (param "HAS_WINDOW_SERVER"))
   (define home-path (param "HOME_PATH"))
   (define debugWriteDir (param "DEBUG_WRITE_DIR"))
   (define testingReadPath1 (param "TESTING_READ_PATH1"))
   (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")
     (deny default)
     (deny default (with no-log)))
   (debug deny)
   ; These are not included in (deny default)
   (deny process-info*)
@@ -183,18 +182,16 @@ static const char contentSandboxRules[] 
   (define (allow-shared-list domain)
     (allow file-read*
            (home-regex (string-append "/Library/Preferences/" (regex-quote domain)))))
 
   (allow ipc-posix-shm-read-data ipc-posix-shm-write-data
     (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")
     (allow mach-lookup (global-name "com.apple.windowserver.active")))
   (allow mach-lookup (global-name "com.apple.coreservices.launchservicesd"))
   (allow mach-lookup (global-name "com.apple.lsd.mapdb"))
 
   (if (>= macosMinorVersion 13)
--- a/toolkit/xre/nsEmbedFunctions.cpp
+++ b/toolkit/xre/nsEmbedFunctions.cpp
@@ -437,16 +437,23 @@ XRE_InitChildProcess(int aArgc,
   // work properly.
   AbstractThread::InitTLS();
 
   // Complete 'task_t' exchange for Mac OS X. This structure has the same size
   // regardless of architecture so we don't have any cross-arch issues here.
 #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;
   pt.emplace();
 
   const int kTimeoutMs = 1000;
 
   MachSendMessage child_message(0);
@@ -508,18 +515,26 @@ XRE_InitChildProcess(int aArgc,
   }
   MachPortSender* ports_in_sender = new MachPortSender(parent_message.GetTranslatedPort(2));
 
   if (err != KERN_SUCCESS) {
     NS_WARNING("child task_set_bootstrap_port() failed");
     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]);
 
   if (!CrashReporter::IsDummy()) {
 #if defined(XP_WIN)
     if (aArgc < 1) {
       return NS_ERROR_FAILURE;
     }