commit 62001bb019dbf0956454f8fc550ccc1e81ee8d3c Author: Richard Pospesel <richard@torproject.org> Date: Sat Sep 15 04:01:17 2018 +0000 Bug 26381: about:tor page does not load on first start on Windows Child content processes require certain directories to be marked as readable or writeable when Sandboxing is enabled. The directories to be whitelisted are saved in static variables in sandboxBroker.cpp and are initialized in SandboxBroker::GeckoDependentInitialize(). Any child content process which is created before these directories are saved will be unable to read or write to them. The tor-launcher extension triggers the creation of a content process which hosts the tor network configuration settings window. This process is created before the whitelisted directories are saved. The network settings process doesn't need access to these directories to function, but subsequent content processes which are created once the settings window exits do need these directories to function. Sometimes, the creation of these subsequent processes is slow enough for the parent process to 'catch up' and create the whitelist resulting in the broken about:tor tab or broken white tab. A previous iteration of this patch moved the GeckoDependentInitialize() call directly above the call to DoStartup(). However, Mozilla dev Bob Owen objected to this since this places the call before various services are initialized which the SandboxBroker may depend on. Some experimentation would seem to confirm his objections: placing the whitelist init just prior to DoStartup() results in an empty value for the profile directory which prevents child processes reading the chrome and extensions directory. This patch inserts the GeckoDependentInitialize() call into DoStartup() just after the profile directory is known and queryable by the SandboxBroker, and before the 'profile-after-change' notification is fired. It also reverts the temp fix which reduced the sandbox level to 2 on windows. --- browser/app/profile/000-tor-browser.js | 5 ----- toolkit/xre/nsAppRunner.cpp | 6 ------ toolkit/xre/nsXREDirProvider.cpp | 19 +++++++++++++++++++ 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/browser/app/profile/000-tor-browser.js b/browser/app/profile/000-tor-browser.js index bc7c4b05e3a1..38f72579760a 100644 --- a/browser/app/profile/000-tor-browser.js +++ b/browser/app/profile/000-tor-browser.js @@ -329,11 +329,6 @@ pref("browser.onboarding.newtour", "welcome,privacy,tor-network,circuit-display, pref("browser.onboarding.updatetour", "welcome,privacy,tor-network,circuit-display,security,expect-differences,onion-services"); pref("browser.onboarding.skip-tour-button.hide", true); -#ifdef XP_WIN -// For now, reduce sandboxing level to 2 (see #26381). -pref("security.sandbox.content.level", 2); -#endif - #ifdef TOR_BROWSER_VERSION #expand pref("torbrowser.version", __TOR_BROWSER_VERSION__); #endif diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 1000014aedd0..7889919ca677 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -5249,12 +5249,6 @@ XREMain::XRE_mainRun() // We intentionally leak the string here since it is required by PR_SetEnv. PR_SetEnv(saved.release()); } - -#if defined(MOZ_SANDBOX) - // Call SandboxBroker to initialize things that depend on Gecko machinery like - // the directory provider. - SandboxBroker::GeckoDependentInitialize(); -#endif #endif SaveStateForAppInitiatedRestart(); diff --git a/toolkit/xre/nsXREDirProvider.cpp b/toolkit/xre/nsXREDirProvider.cpp index b54985292e81..72545d0eb422 100644 --- a/toolkit/xre/nsXREDirProvider.cpp +++ b/toolkit/xre/nsXREDirProvider.cpp @@ -66,6 +66,10 @@ #include "UIKitDirProvider.h" #endif +#if defined(MOZ_SANDBOX) && defined(XP_WIN) +#include "sandboxBroker.h" +#endif + #if defined(MOZ_CONTENT_SANDBOX) #include "mozilla/SandboxSettings.h" #include "nsIUUIDGenerator.h" @@ -1003,6 +1007,21 @@ nsXREDirProvider::DoStartup() policies->Observe(nullptr, "policies-startup", nullptr); } + #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 so that any child content processes spawned by + // extensions (such as tor-launcher launching the network configuration window) will have + // all the requisite directories white-listed for read/write access + + // It's inserted here (rather than in XREMain::XRE_mainRun) because we need + // NS_APP_USER_PROFILE_50_DIR to be known + + // See tor bug #26381 and mozilla bug #1485836 + SandboxBroker::GeckoDependentInitialize(); + #endif + // Init the Extension Manager nsCOMPtr<nsIObserver> em = do_GetService("@mozilla.org/addons/integration;1"); if (em) {