Bug 1530245 - Make GeckoChildProcessHost::mSandboxBroker an abstract pointer. r=bobowen
authorChris Pearce <cpearce@mozilla.com>
Wed, 13 Mar 2019 09:24:37 +0000
changeset 521672 00fe102f6c0676cafce9859fa75a4e558b6e3661
parent 521671 bad1df2f11e3962bea1ec567845e50399cf87357
child 521673 64abd0254f73af2d4580b5ec7188f513a3fc785a
push id10867
push userdvarga@mozilla.com
push dateThu, 14 Mar 2019 15:20:45 +0000
treeherdermozilla-beta@abad13547875 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1530245
milestone67.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 1530245 - Make GeckoChildProcessHost::mSandboxBroker an abstract pointer. r=bobowen Differential Revision: https://phabricator.services.mozilla.com/D22051
ipc/glue/GeckoChildProcessHost.cpp
ipc/glue/GeckoChildProcessHost.h
security/sandbox/win/src/sandboxbroker/sandboxBroker.h
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -98,16 +98,17 @@ GeckoChildProcessHost::GeckoChildProcess
       mIsFileContent(aIsFileContent),
       mMonitor("mozilla.ipc.GeckChildProcessHost.mMonitor"),
       mLaunchOptions(MakeUnique<base::LaunchOptions>()),
       mProcessState(CREATING_CHANNEL),
 #ifdef XP_WIN
       mGroupId(u"-"),
 #endif
 #if defined(MOZ_SANDBOX) && defined(XP_WIN)
+      mSandboxBroker(new SandboxBroker()),
       mEnableSandboxLogging(false),
       mSandboxLevel(0),
 #endif
       mChildProcessHandle(0),
 #if defined(MOZ_WIDGET_COCOA)
       mChildTask(MACH_PORT_NULL),
 #endif
       mDestroying(false) {
@@ -1071,26 +1072,26 @@ bool GeckoChildProcessHost::PerformAsync
   switch (mProcessType) {
     case GeckoProcessType_Content:
 #    if defined(MOZ_CONTENT_SANDBOX)
       if (mSandboxLevel > 0) {
         // For now we treat every failure as fatal in
         // SetSecurityLevelForContentProcess and just crash there right away.
         // Should this change in the future then we should also handle the error
         // here.
-        mSandboxBroker.SetSecurityLevelForContentProcess(mSandboxLevel,
-                                                         mIsFileContent);
+        mSandboxBroker->SetSecurityLevelForContentProcess(mSandboxLevel,
+                                                          mIsFileContent);
         shouldSandboxCurrentProcess = true;
       }
 #    endif  // defined(MOZ_CONTENT_SANDBOX)
       break;
     case GeckoProcessType_Plugin:
       if (mSandboxLevel > 0 && !PR_GetEnv("MOZ_DISABLE_NPAPI_SANDBOX")) {
         bool ok =
-            mSandboxBroker.SetSecurityLevelForPluginProcess(mSandboxLevel);
+            mSandboxBroker->SetSecurityLevelForPluginProcess(mSandboxLevel);
         if (!ok) {
           return false;
         }
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_IPDLUnitTest:
       // XXX: We don't sandbox this process type yet
@@ -1098,40 +1099,40 @@ bool GeckoChildProcessHost::PerformAsync
     case GeckoProcessType_GMPlugin:
       if (!PR_GetEnv("MOZ_DISABLE_GMP_SANDBOX")) {
         // The Widevine CDM on Windows can only load at USER_RESTRICTED,
         // not at USER_LOCKDOWN. So look in the command line arguments
         // to see if we're loading the path to the Widevine CDM, and if
         // so use sandbox level USER_RESTRICTED instead of USER_LOCKDOWN.
         auto level =
             isWidevine ? SandboxBroker::Restricted : SandboxBroker::LockDown;
-        bool ok = mSandboxBroker.SetSecurityLevelForGMPlugin(level);
+        bool ok = mSandboxBroker->SetSecurityLevelForGMPlugin(level);
         if (!ok) {
           return false;
         }
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_GPU:
       if (mSandboxLevel > 0 && !PR_GetEnv("MOZ_DISABLE_GPU_SANDBOX")) {
         // For now we treat every failure as fatal in
         // SetSecurityLevelForGPUProcess and just crash there right away. Should
         // this change in the future then we should also handle the error here.
-        mSandboxBroker.SetSecurityLevelForGPUProcess(mSandboxLevel);
+        mSandboxBroker->SetSecurityLevelForGPUProcess(mSandboxLevel);
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_VR:
       if (mSandboxLevel > 0 && !PR_GetEnv("MOZ_DISABLE_VR_SANDBOX")) {
         // TODO: Implement sandbox for VR process, Bug 1430043.
       }
       break;
     case GeckoProcessType_RDD:
       if (!PR_GetEnv("MOZ_DISABLE_RDD_SANDBOX")) {
-        if (!mSandboxBroker.SetSecurityLevelForRDDProcess()) {
+        if (!mSandboxBroker->SetSecurityLevelForRDDProcess()) {
           return false;
         }
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_Socket:
       // TODO - setup sandboxing for the socket process.
       break;
@@ -1139,17 +1140,17 @@ bool GeckoChildProcessHost::PerformAsync
     default:
       MOZ_CRASH("Bad process type in GeckoChildProcessHost");
       break;
   };
 
   if (shouldSandboxCurrentProcess) {
     for (auto it = mAllowedFilesRead.begin(); it != mAllowedFilesRead.end();
          ++it) {
-      mSandboxBroker.AllowReadFile(it->c_str());
+      mSandboxBroker->AllowReadFile(it->c_str());
     }
   }
 #  endif    // defined(MOZ_SANDBOX)
 
   // Add the application directory path (-appdir path)
   AddAppDirToCommandLine(cmdLine);
 
   // XXX Command line params past this point are expected to be at
@@ -1174,23 +1175,23 @@ bool GeckoChildProcessHost::PerformAsync
 
   // Process type
   cmdLine.AppendLooseValue(UTF8ToWide(childProcessType));
 
 #  if defined(MOZ_SANDBOX)
   if (shouldSandboxCurrentProcess) {
     // Mark the handles to inherit as inheritable.
     for (HANDLE h : mLaunchOptions->handles_to_inherit) {
-      mSandboxBroker.AddHandleToShare(h);
+      mSandboxBroker->AddHandleToShare(h);
     }
 
-    if (mSandboxBroker.LaunchApp(cmdLine.program().c_str(),
-                                 cmdLine.command_line_string().c_str(),
-                                 mLaunchOptions->env_map, mProcessType,
-                                 mEnableSandboxLogging, &process)) {
+    if (mSandboxBroker->LaunchApp(cmdLine.program().c_str(),
+                                  cmdLine.command_line_string().c_str(),
+                                  mLaunchOptions->env_map, mProcessType,
+                                  mEnableSandboxLogging, &process)) {
       EnvironmentLog("MOZ_PROCESS_LOG")
           .print("==> process %d launched child process %d (%S)\n",
                  base::GetCurrentProcId(), base::GetProcId(process),
                  cmdLine.command_line_string().c_str());
     }
   } else
 #  endif  // defined(MOZ_SANDBOX)
   {
@@ -1202,17 +1203,17 @@ bool GeckoChildProcessHost::PerformAsync
     switch (mProcessType) {
       case GeckoProcessType_Default:
         MOZ_CRASH("shouldn't be launching a parent process");
       case GeckoProcessType_Plugin:
       case GeckoProcessType_IPDLUnitTest:
         // No handle duplication necessary.
         break;
       default:
-        if (!mSandboxBroker.AddTargetPeer(process)) {
+        if (!mSandboxBroker->AddTargetPeer(process)) {
           NS_WARNING("Failed to add child process as target peer.");
         }
         break;
     }
 #  endif  // MOZ_SANDBOX
   }
 
 #else  // goes with defined(OS_POSIX)
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -167,17 +167,17 @@ class GeckoChildProcessHost : public Chi
 
   void PrepareLaunch();
 
 #ifdef XP_WIN
   void InitWindowsGroupID();
   nsString mGroupId;
 
 #  ifdef MOZ_SANDBOX
-  SandboxBroker mSandboxBroker;
+  RefPtr<AbstractSandboxBroker> mSandboxBroker;
   std::vector<std::wstring> mAllowedFilesRead;
   bool mEnableSandboxLogging;
   int32_t mSandboxLevel;
 #  endif
 #endif  // XP_WIN
 
   ProcessHandle mChildProcessHandle;
 #if defined(OS_MACOSX)
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
@@ -8,71 +8,114 @@
 #define __SECURITY_SANDBOX_SANDBOXBROKER_H__
 
 #include <stdint.h>
 #include <windows.h>
 
 #include "build/build_config.h"
 #include "mozilla/ipc/EnvironmentMap.h"
 #include "nsXULAppAPI.h"
+#include "nsISupportsImpl.h"
 
 namespace sandbox {
 class BrokerServices;
 class TargetPolicy;
 }  // namespace sandbox
 
 namespace mozilla {
 
-class SandboxBroker {
+class AbstractSandboxBroker {
+ public:
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AbstractSandboxBroker)
+
+  virtual bool LaunchApp(const wchar_t *aPath, const wchar_t *aArguments,
+                         base::EnvironmentMap &aEnvironment,
+                         GeckoProcessType aProcessType,
+                         const bool aEnableLogging, void **aProcessHandle) = 0;
+
+  // Security levels for different types of processes
+#if defined(MOZ_CONTENT_SANDBOX)
+  virtual void SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
+                                                 bool aIsFileProcess) = 0;
+#endif
+
+  virtual void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) = 0;
+  virtual bool SetSecurityLevelForRDDProcess() = 0;
+
+  virtual bool SetSecurityLevelForPluginProcess(int32_t aSandboxLevel) = 0;
+  enum SandboxLevel { LockDown, Restricted };
+  virtual bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel) = 0;
+
+  // File system permissions
+  virtual bool AllowReadFile(wchar_t const *file) = 0;
+
+  /**
+   * Exposes AddTargetPeer from broker services, so that non-sandboxed
+   * processes can be added as handle duplication targets.
+   */
+  virtual bool AddTargetPeer(HANDLE aPeerProcess) = 0;
+
+  /**
+   * Share a HANDLE with the child process. The HANDLE will be made available
+   * in the child process at the memory address
+   * |reinterpret_cast<uintptr_t>(aHandle)|. It is the caller's responsibility
+   * to communicate this address to the child.
+   */
+  virtual void AddHandleToShare(HANDLE aHandle) = 0;
+
+ protected:
+  virtual ~AbstractSandboxBroker() {}
+};
+
+class SandboxBroker : public AbstractSandboxBroker {
  public:
   SandboxBroker();
 
   static void Initialize(sandbox::BrokerServices *aBrokerServices);
 
   /**
    * Do initialization that depends on parts of the Gecko machinery having been
    * created first.
    */
   static void GeckoDependentInitialize();
 
   bool LaunchApp(const wchar_t *aPath, const wchar_t *aArguments,
                  base::EnvironmentMap &aEnvironment,
                  GeckoProcessType aProcessType, const bool aEnableLogging,
-                 void **aProcessHandle);
+                 void **aProcessHandle) override;
   virtual ~SandboxBroker();
 
   // Security levels for different types of processes
 #if defined(MOZ_CONTENT_SANDBOX)
   void SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
-                                         bool aIsFileProcess);
+                                         bool aIsFileProcess) override;
 #endif
 
-  void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel);
-  bool SetSecurityLevelForRDDProcess();
+  void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) override;
+  bool SetSecurityLevelForRDDProcess() override;
 
-  bool SetSecurityLevelForPluginProcess(int32_t aSandboxLevel);
-  enum SandboxLevel { LockDown, Restricted };
-  bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel);
+  bool SetSecurityLevelForPluginProcess(int32_t aSandboxLevel) override;
+  bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel) override;
 
   // File system permissions
-  bool AllowReadFile(wchar_t const *file);
+  bool AllowReadFile(wchar_t const *file) override;
 
   /**
    * Exposes AddTargetPeer from broker services, so that non-sandboxed
    * processes can be added as handle duplication targets.
    */
-  bool AddTargetPeer(HANDLE aPeerProcess);
+  bool AddTargetPeer(HANDLE aPeerProcess) override;
 
   /**
    * Share a HANDLE with the child process. The HANDLE will be made available
    * in the child process at the memory address
    * |reinterpret_cast<uintptr_t>(aHandle)|. It is the caller's responsibility
    * to communicate this address to the child.
    */
-  void AddHandleToShare(HANDLE aHandle);
+  void AddHandleToShare(HANDLE aHandle) override;
 
   // Set up dummy interceptions via the broker, so we can log calls.
   void ApplyLoggingPolicy();
 
  private:
   static sandbox::BrokerServices *sBrokerService;
   static bool sRunningFromNetworkDrive;
   sandbox::TargetPolicy *mPolicy;