From 15c5b2278936070696250935b2af8703deffaf6f Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Mon, 23 Jan 2023 23:05:03 +0000 Subject: [PATCH] Bug 1799470 - Retighten GPU Process File Access r=nika,handyman Differential Revision: https://phabricator.services.mozilla.com/D165419 --- ipc/glue/GeckoChildProcessHost.cpp | 10 +--- .../common/test/SandboxTestingChild.cpp | 4 ++ .../common/test/SandboxTestingChildTests.h | 20 +++++++ .../remoteSandboxBroker.cpp | 3 +- .../remotesandboxbroker/remoteSandboxBroker.h | 3 +- .../win/src/sandboxbroker/sandboxBroker.cpp | 19 ++++--- .../win/src/sandboxbroker/sandboxBroker.h | 6 +-- toolkit/xre/nsAppRunner.cpp | 10 ++++ toolkit/xre/nsXREDirProvider.cpp | 52 +++++-------------- toolkit/xre/nsXREDirProvider.h | 2 +- 10 files changed, 61 insertions(+), 68 deletions(-) diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index 929764206600..19a51b6210d0 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -253,7 +253,6 @@ class WindowsProcessLauncher : public BaseProcessLauncher { WindowsProcessLauncher(GeckoChildProcessHost* aHost, std::vector&& aExtraOpts) : BaseProcessLauncher(aHost, std::move(aExtraOpts)), - mProfileDir(aHost->mProfileDir), mCachedNtdllThunk(GetCachedNtDllThunk()), mWerDataPointer(&(aHost->mWerData)) {} @@ -266,8 +265,6 @@ class WindowsProcessLauncher : public BaseProcessLauncher { mozilla::Maybe mCmdLine; bool mUseSandbox = false; - nsCOMPtr mProfileDir; - const Buffer* 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; diff --git a/security/sandbox/common/test/SandboxTestingChild.cpp b/security/sandbox/common/test/SandboxTestingChild.cpp index 64e347b958a3..e64a53d9f031 100644 --- a/security/sandbox/common/test/SandboxTestingChild.cpp +++ b/security/sandbox/common/test/SandboxTestingChild.cpp @@ -81,6 +81,10 @@ void SandboxTestingChild::Bind(Endpoint&& aEndpoint) { RunTestsSocket(this); } + if (XRE_IsGPUProcess()) { + RunTestsGPU(this); + } + if (XRE_IsUtilityProcess()) { RefPtr s = ipc::UtilityProcessChild::Get(); MOZ_ASSERT(s, "Unable to grab a UtilityProcessChild"); diff --git a/security/sandbox/common/test/SandboxTestingChildTests.h b/security/sandbox/common/test/SandboxTestingChildTests.h index f26bc68af0d0..a5cc0efecb92 100644 --- a/security/sandbox/common/test/SandboxTestingChildTests.h +++ b/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 diff --git a/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp b/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp index 6f9f4233c68e..7946e5b8382b 100644 --- a/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.cpp +++ b/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& aProfileDir) { +void RemoteSandboxBroker::SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) { MOZ_CRASH( "RemoteSandboxBroker::SetSecurityLevelForGPUProcess not Implemented"); } diff --git a/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.h b/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.h index aee84ee43de4..3a3d6dd294be 100644 --- a/security/sandbox/win/src/remotesandboxbroker/remoteSandboxBroker.h +++ b/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& aProfileDir) override; + void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) override; bool SetSecurityLevelForRDDProcess() override; bool SetSecurityLevelForSocketProcess() override; bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel, diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp index 9346ceb054db..fe4473e75cbe 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp +++ b/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& 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. diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h index 67260f9a2834..ec5159cafc53 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h +++ b/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& 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& aProfileDir) override; + void SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) override; bool SetSecurityLevelForRDDProcess() override; bool SetSecurityLevelForSocketProcess() override; bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel, diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 9a05cda9e23d..3f80ad2a9b19 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/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 workingDir; rv = NS_GetSpecialDirectory(NS_OS_CURRENT_WORKING_DIR, getter_AddRefs(workingDir)); diff --git a/toolkit/xre/nsXREDirProvider.cpp b/toolkit/xre/nsXREDirProvider.cpp index 677eb558742f..bf9b6ea4b821 100644 --- a/toolkit/xre/nsXREDirProvider.cpp +++ b/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 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 ar(mProfileNotified); - mProfileNotified = true; - mozilla::Preferences::InitializeUserPrefs(); } } void nsXREDirProvider::FinishInitializingUserPrefs() { if (!mPrefsInitialized) { - // See InitializeUserPrefs above. - mozilla::AutoRestore ar(mProfileNotified); - mProfileNotified = true; - mozilla::Preferences::FinishInitializingUserPrefs(); - mPrefsInitialized = true; } } @@ -747,12 +729,12 @@ NS_IMETHODIMP nsXREDirProvider::DoStartup() { nsresult rv; - if (!mProfileNotified) { + if (!mAppStarted) { nsCOMPtr 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 diff --git a/toolkit/xre/nsXREDirProvider.h b/toolkit/xre/nsXREDirProvider.h index 576e6391b5c7..b79c3b086319 100644 --- a/toolkit/xre/nsXREDirProvider.h +++ b/toolkit/xre/nsXREDirProvider.h @@ -147,7 +147,7 @@ class nsXREDirProvider final : public nsIDirectoryServiceProvider2, nsCOMPtr mXULAppDir; nsCOMPtr mProfileDir; nsCOMPtr mProfileLocalDir; - bool mProfileNotified = false; + bool mAppStarted = false; bool mPrefsInitialized = false; #if defined(MOZ_SANDBOX) nsCOMPtr mContentTempDir;