Bug 1799470 - Retighten GPU Process File Access r=nika,handyman

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

ipc/glue/GeckoChildProcessHost.cpp

@@ -253,7 +253,6 @@ class WindowsProcessLauncher : public BaseProcessLauncher {
   WindowsProcessLauncher(GeckoChildProcessHost* aHost,
                          std::vector<std::string>&& aExtraOpts)
       : BaseProcessLauncher(aHost, std::move(aExtraOpts)),
-        mProfileDir(aHost->mProfileDir),
         mCachedNtdllThunk(GetCachedNtDllThunk()),
         mWerDataPointer(&(aHost->mWerData)) {}
 
@@ -266,8 +265,6 @@ class WindowsProcessLauncher : public BaseProcessLauncher {
   mozilla::Maybe<CommandLine> mCmdLine;
   bool mUseSandbox = false;
 
-  nsCOMPtr<nsIFile> mProfileDir;
-
   const Buffer<IMAGE_THUNK_DATA>* mCachedNtdllThunk;
   CrashReporter::WindowsErrorReportingData const* mWerDataPointer;
 };
@@ -643,10 +640,6 @@ void GeckoChildProcessHost::PrepareLaunch() {
   mEnableSandboxLogging =
       mEnableSandboxLogging || !!PR_GetEnv("MOZ_SANDBOX_LOGGING");
 
-  if (ShouldHaveDirectoryService() && mProcessType == GeckoProcessType_GPU) {
-    mozilla::Unused << NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
-                                              getter_AddRefs(mProfileDir));
-  }
 #  endif
 #elif defined(XP_MACOSX)
 #  if defined(MOZ_SANDBOX)
@@ -1435,8 +1428,7 @@ bool WindowsProcessLauncher::DoSetup() {
         // 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.
-        mResults.mSandboxBroker->SetSecurityLevelForGPUProcess(mSandboxLevel,
-                                                               mProfileDir);
+        mResults.mSandboxBroker->SetSecurityLevelForGPUProcess(mSandboxLevel);
         mUseSandbox = true;
       }
       break;

security/sandbox/common/test/SandboxTestingChild.cpp

@@ -81,6 +81,10 @@ void SandboxTestingChild::Bind(Endpoint<PSandboxTestingChild>&& aEndpoint) {
     RunTestsSocket(this);
   }
 
+  if (XRE_IsGPUProcess()) {
+    RunTestsGPU(this);
+  }
+
   if (XRE_IsUtilityProcess()) {
     RefPtr<ipc::UtilityProcessChild> s = ipc::UtilityProcessChild::Get();
     MOZ_ASSERT(s, "Unable to grab a UtilityProcessChild");

security/sandbox/common/test/SandboxTestingChildTests.h

@@ -821,4 +821,24 @@ void RunTestsUtilityAudioDecoder(SandboxTestingChild* child,
 #endif    // XP_UNIX
 }
 
+void RunTestsGPU(SandboxTestingChild* child) {
+  MOZ_ASSERT(child, "No SandboxTestingChild*?");
+
+  RunGenericTests(child);
+
+#if defined(XP_WIN)
+
+  FileTest("R/W access to shader-cache dir"_ns, NS_APP_USER_PROFILE_50_DIR,
+           u"shader-cache\\"_ns, FILE_GENERIC_READ | FILE_GENERIC_WRITE, true,
+           child);
+
+  FileTest("R/W access to shader-cache files"_ns, NS_APP_USER_PROFILE_50_DIR,
+           u"shader-cache\\sandboxTest.txt"_ns,
+           FILE_GENERIC_READ | FILE_GENERIC_WRITE, true, child);
+
+#else   // defined(XP_WIN)
+  child->ReportNoTests();
+#endif  // defined(XP_WIN)
+}
+
 }  // namespace mozilla

security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp

@@ -142,8 +142,7 @@ void RemoteSandboxBroker::SetSecurityLevelForContentProcess(
       "RemoteSandboxBroker::SetSecurityLevelForContentProcess not Implemented");
 }
 
-void RemoteSandboxBroker::SetSecurityLevelForGPUProcess(
-    int32_t aSandboxLevel, const nsCOMPtr<nsIFile>& aProfileDir) {
+void RemoteSandboxBroker::SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) {
   MOZ_CRASH(
       "RemoteSandboxBroker::SetSecurityLevelForGPUProcess not Implemented");
 }

security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.h

@@ -35,8 +35,7 @@ class RemoteSandboxBroker : public AbstractSandboxBroker {
   // Security levels for different types of processes
   void SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
                                          bool aIsFileProcess) override;
-  void SetSecurityLevelForGPUProcess(
-      int32_t aSandboxLevel, const nsCOMPtr<nsIFile>& aProfileDir) override;
+  void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) override;
   bool SetSecurityLevelForRDDProcess() override;
   bool SetSecurityLevelForSocketProcess() override;
   bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel,

security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp

@@ -906,8 +906,7 @@ void SandboxBroker::SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
   }
 }
 
-void SandboxBroker::SetSecurityLevelForGPUProcess(
-    int32_t aSandboxLevel, const nsCOMPtr<nsIFile>& aProfileDir) {
+void SandboxBroker::SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) {
   MOZ_RELEASE_ASSERT(mPolicy, "mPolicy must be set before this call.");
 
   sandbox::JobLevel jobLevel;
@@ -1000,14 +999,14 @@ void SandboxBroker::SetSecurityLevelForGPUProcess(
       sandbox::SBOX_ALL_OK == result,
       "With these static arguments AddRule should never fail, what happened?");
 
-  // TEMPORARY WORKAROUND - We don't want to back out the GPU sandbox, so we're
-  // going to give access to the entire filesystem until we can figure out a
-  // reliable way to only give access to the Shader Cache
-  result = mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
-                            sandbox::TargetPolicy::FILES_ALLOW_ANY, L"*");
-  MOZ_RELEASE_ASSERT(
-      sandbox::SBOX_ALL_OK == result,
-      "With these static arguments AddRule should never fail, what happened?");
+  // The GPU process needs to write to a shader cache for performance reasons
+  if (sProfileDir) {
+    AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY,
+                     sProfileDir, u"\\shader-cache"_ns);
+
+    AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_ANY,
+                     sProfileDir, u"\\shader-cache\\*"_ns);
+  }
 
   // The process needs to be able to duplicate shared memory handles,
   // which are Section handles, to the broker process and other child processes.

security/sandbox/win/src/sandboxbroker/sandboxBroker.h

@@ -43,8 +43,7 @@ class AbstractSandboxBroker {
   virtual void SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
                                                  bool aIsFileProcess) = 0;
 
-  virtual void SetSecurityLevelForGPUProcess(
-      int32_t aSandboxLevel, const nsCOMPtr<nsIFile>& aProfileDir) = 0;
+  virtual void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) = 0;
   virtual bool SetSecurityLevelForRDDProcess() = 0;
   virtual bool SetSecurityLevelForSocketProcess() = 0;
   virtual bool SetSecurityLevelForUtilityProcess(
@@ -99,8 +98,7 @@ class SandboxBroker : public AbstractSandboxBroker {
   void SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
                                          bool aIsFileProcess) override;
 
-  void SetSecurityLevelForGPUProcess(
-      int32_t aSandboxLevel, const nsCOMPtr<nsIFile>& aProfileDir) override;
+  void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) override;
   bool SetSecurityLevelForRDDProcess() override;
   bool SetSecurityLevelForSocketProcess() override;
   bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel,

toolkit/xre/nsAppRunner.cpp

@@ -5345,6 +5345,16 @@ nsresult XREMain::XRE_mainRun() {
     // files can't override JS engine start-up prefs.
     mDirProvider.FinishInitializingUserPrefs();
 
+#if defined(MOZ_SANDBOX) && defined(XP_WIN)
+    // Now that we have preferences and the directory provider, we can
+    // finish initializing SandboxBroker. This must happen before the GFX
+    // platform is initialized (which will launch the GPU process), which
+    // occurs when the "app-startup" category is started up below
+    //
+    // After this completes, we are ready to launch sandboxed processes
+    mozilla::SandboxBroker::GeckoDependentInitialize();
+#endif
+
     nsCOMPtr<nsIFile> workingDir;
     rv = NS_GetSpecialDirectory(NS_OS_CURRENT_WORKING_DIR,
                                 getter_AddRefs(workingDir));

toolkit/xre/nsXREDirProvider.cpp

@@ -161,6 +161,8 @@ nsresult nsXREDirProvider::Initialize(nsIFile* aXULAppDir, nsIFile* aGREDir) {
 
 nsresult nsXREDirProvider::SetProfile(nsIFile* aDir, nsIFile* aLocalDir) {
   MOZ_ASSERT(aDir && aLocalDir, "We don't support no-profile apps!");
+  MOZ_ASSERT(!mProfileDir && !mProfileLocalDir,
+             "You may only set the profile directories once");
 
   nsresult rv = EnsureDirectoryExists(aDir);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -329,15 +331,10 @@ nsXREDirProvider::GetFile(const char* aProperty, bool* aPersistent,
   nsCOMPtr<nsIFile> file;
 
   if (!strcmp(aProperty, NS_APP_USER_PROFILE_LOCAL_50_DIR)) {
-    NS_ENSURE_TRUE(mProfileNotified, NS_ERROR_FAILURE);
-
     if (mProfileLocalDir) {
       rv = mProfileLocalDir->Clone(getter_AddRefs(file));
-    } else if (mProfileDir) {
-      rv = mProfileDir->Clone(getter_AddRefs(file));
     }
   } else if (!strcmp(aProperty, NS_APP_USER_PROFILE_50_DIR)) {
-    NS_ENSURE_TRUE(mProfileNotified, NS_ERROR_FAILURE);
     if (mProfileDir) {
       rv = mProfileDir->Clone(getter_AddRefs(file));
     }
@@ -401,8 +398,6 @@ nsXREDirProvider::GetFile(const char* aProperty, bool* aPersistent,
   } else if (!strcmp(aProperty, NS_APP_PROFILE_LOCAL_DIR_STARTUP)) {
     if (mProfileLocalDir) {
       rv = mProfileLocalDir->Clone(getter_AddRefs(file));
-    } else if (mProfileDir) {
-      rv = mProfileDir->Clone(getter_AddRefs(file));
     }
   }
 #if defined(XP_UNIX) || defined(XP_MACOSX)
@@ -714,31 +709,18 @@ nsXREDirProvider::GetFiles(const char* aProperty,
 NS_IMETHODIMP
 nsXREDirProvider::GetDirectory(nsIFile** aResult) {
   NS_ENSURE_TRUE(mProfileDir, NS_ERROR_NOT_INITIALIZED);
-
   return mProfileDir->Clone(aResult);
 }
 
 void nsXREDirProvider::InitializeUserPrefs() {
   if (!mPrefsInitialized) {
-    // Temporarily set mProfileNotified to true so that the preference service
-    // can access the profile directory during initialization. Afterwards, clear
-    // it so that no other code can inadvertently access it until we get to
-    // profile-do-change.
-    mozilla::AutoRestore<bool> ar(mProfileNotified);
-    mProfileNotified = true;
-
     mozilla::Preferences::InitializeUserPrefs();
   }
 }
 
 void nsXREDirProvider::FinishInitializingUserPrefs() {
   if (!mPrefsInitialized) {
-    // See InitializeUserPrefs above.
-    mozilla::AutoRestore<bool> ar(mProfileNotified);
-    mProfileNotified = true;
-
     mozilla::Preferences::FinishInitializingUserPrefs();
-
     mPrefsInitialized = true;
   }
 }
@@ -747,12 +729,12 @@ NS_IMETHODIMP
 nsXREDirProvider::DoStartup() {
   nsresult rv;
 
-  if (!mProfileNotified) {
+  if (!mAppStarted) {
     nsCOMPtr<nsIObserverService> obsSvc =
         mozilla::services::GetObserverService();
     if (!obsSvc) return NS_ERROR_FAILURE;
 
-    mProfileNotified = true;
+    mAppStarted = true;
 
     /*
        Make sure we've setup prefs before profile-do-change to be able to use
@@ -789,17 +771,6 @@ nsXREDirProvider::DoStartup() {
       }
     }
 
-#if defined(MOZ_SANDBOX) && defined(XP_WIN)
-    // Call SandboxBroker to initialize things that depend on Gecko machinery
-    // like the directory provider. We insert this initialization code here
-    // (rather than in XRE_mainRun) because we need NS_APP_USER_PROFILE_50_DIR
-    // to be known and so that any child content processes spawned by extensions
-    // from the notifications below will have all the requisite directories
-    // white-listed for read/write access. An example of this is the
-    // tor-launcher launching the network configuration window. See bug 1485836.
-    mozilla::SandboxBroker::GeckoDependentInitialize();
-#endif
-
 #ifdef MOZ_THUNDERBIRD
     bool bgtaskMode = false;
 #  ifdef MOZ_BACKGROUNDTASKS
@@ -882,7 +853,7 @@ nsXREDirProvider::DoStartup() {
 void nsXREDirProvider::DoShutdown() {
   AUTO_PROFILER_LABEL("nsXREDirProvider::DoShutdown", OTHER);
 
-  if (mProfileNotified) {
+  if (mAppStarted) {
     mozilla::AppShutdown::AdvanceShutdownPhase(
         mozilla::ShutdownPhase::AppShutdownNetTeardown, nullptr);
     mozilla::AppShutdown::AdvanceShutdownPhase(
@@ -901,7 +872,7 @@ void nsXREDirProvider::DoShutdown() {
         mozilla::ShutdownPhase::AppShutdownQM, nullptr);
     mozilla::AppShutdown::AdvanceShutdownPhase(
         mozilla::ShutdownPhase::AppShutdownTelemetry, nullptr);
-    mProfileNotified = false;
+    mAppStarted = false;
   }
 
   gDataDirProfileLocal = nullptr;
@@ -1186,12 +1157,13 @@ nsresult nsXREDirProvider::GetProfileStartupDir(nsIFile** aResult) {
 }
 
 nsresult nsXREDirProvider::GetProfileDir(nsIFile** aResult) {
-  if (mProfileDir) {
-    NS_ENSURE_TRUE(mProfileNotified, NS_ERROR_FAILURE);
-    return mProfileDir->Clone(aResult);
+  if (!mProfileDir) {
+    nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,
+                                         getter_AddRefs(mProfileDir));
+    NS_ENSURE_SUCCESS(rv, rv);
+    NS_ENSURE_TRUE(mProfileDir, NS_ERROR_FAILURE);
   }
-
-  return NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, aResult);
+  return mProfileDir->Clone(aResult);
 }
 
 NS_IMETHODIMP

toolkit/xre/nsXREDirProvider.h

@@ -147,7 +147,7 @@ class nsXREDirProvider final : public nsIDirectoryServiceProvider2,
   nsCOMPtr<nsIFile> mXULAppDir;
   nsCOMPtr<nsIFile> mProfileDir;
   nsCOMPtr<nsIFile> mProfileLocalDir;
-  bool mProfileNotified = false;
+  bool mAppStarted = false;
   bool mPrefsInitialized = false;
 #if defined(MOZ_SANDBOX)
   nsCOMPtr<nsIFile> mContentTempDir;