Bug 1127230: Change the NPAPI sandbox prefs to integers to indicate the level of sandboxing. r=bsmedberg
authorBob Owen <bobowencode@gmail.com>
Fri, 30 Jan 2015 17:48:15 +0000
changeset 244188 9c95e28087ca0f3c355e0ebab92d11b1be9fba10
parent 244187 67825403fac80bc2abe4535b4b8ce624a637e01a
child 244202 393838728c27c97afe879c074ddd44a70be47d2d
child 244218 17ad80d798243aceababa9976efea3b7ed0d1262
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg
bugs1127230
milestone38.0a1
Bug 1127230: Change the NPAPI sandbox prefs to integers to indicate the level of sandboxing. r=bsmedberg
browser/app/profile/firefox.js
dom/plugins/ipc/PluginModuleParent.cpp
dom/plugins/ipc/PluginProcessParent.cpp
dom/plugins/ipc/PluginProcessParent.h
ipc/glue/GeckoChildProcessHost.cpp
ipc/glue/GeckoChildProcessHost.h
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.h
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1182,25 +1182,25 @@ pref("browser.tabs.remote.desktopbehavio
 
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
 // When this pref is true the Windows process sandbox will set up dummy
 // interceptions and log to the browser console when calls fail in the sandboxed
 // process and also if they are subsequently allowed by the broker process.
 // This will require a restart.
 pref("security.sandbox.windows.log", false);
 
-// Controls whether the Windows NPAPI plugin process is sandboxed by default.
+// Controls whether and how the Windows NPAPI plugin process is sandboxed.
 // To get a different setting for a particular plugin replace "default", with
 // the plugin's nice file name, see: nsPluginTag::GetNiceFileName.
-pref("dom.ipc.plugins.sandbox.default", false);
-pref("dom.ipc.plugins.sandbox.flash", true);
-
-// This controls whether the Windows NPAPI process sandbox is using a more
-// strict sandboxing policy.  This will require a restart.
-pref("dom.ipc.plugins.moreStrictSandbox", false);
+// On windows these levels are:
+// 0 - no sandbox
+// 1 - sandbox with USER_NON_ADMIN access token level
+// 2 - a more strict sandbox, which might cause functionality issues
+pref("dom.ipc.plugins.sandbox-level.default", 0);
+pref("dom.ipc.plugins.sandbox-level.flash", 1);
 
 #if defined(MOZ_CONTENT_SANDBOX)
 // This controls whether the Windows content process sandbox is using a more
 // strict sandboxing policy.  This will require a restart.
 pref("security.sandbox.windows.content.moreStrict", false);
 
 #if defined(MOZ_STACKWALKING)
 // This controls the depth of stack trace that is logged when Windows sandbox
--- a/dom/plugins/ipc/PluginModuleParent.cpp
+++ b/dom/plugins/ipc/PluginModuleParent.cpp
@@ -389,31 +389,31 @@ PluginModuleChromeParent::SendAssociateP
 
 // static
 PluginLibrary*
 PluginModuleChromeParent::LoadModule(const char* aFilePath, uint32_t aPluginId,
                                      nsPluginTag* aPluginTag)
 {
     PLUGIN_LOG_DEBUG_FUNCTION;
 
-    bool enableSandbox = false;
+    int32_t sandboxLevel = 0;
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
-    nsAutoCString sandboxPref("dom.ipc.plugins.sandbox.");
+    nsAutoCString sandboxPref("dom.ipc.plugins.sandbox-level.");
     sandboxPref.Append(aPluginTag->GetNiceFileName());
-    if (NS_FAILED(Preferences::GetBool(sandboxPref.get(), &enableSandbox))) {
-      enableSandbox = Preferences::GetBool("dom.ipc.plugins.sandbox.default");
+    if (NS_FAILED(Preferences::GetInt(sandboxPref.get(), &sandboxLevel))) {
+      sandboxLevel = Preferences::GetInt("dom.ipc.plugins.sandbox-level.default");
     }
 #endif
 
     nsAutoPtr<PluginModuleChromeParent> parent(new PluginModuleChromeParent(aFilePath, aPluginId));
     UniquePtr<LaunchCompleteTask> onLaunchedRunnable(new LaunchedTask(parent));
     parent->mSubprocess->SetCallRunnableImmediately(!parent->mIsStartingAsync);
     TimeStamp launchStart = TimeStamp::Now();
     bool launched = parent->mSubprocess->Launch(Move(onLaunchedRunnable),
-                                                enableSandbox);
+                                                sandboxLevel);
     if (!launched) {
         // We never reached open
         parent->mShutdown = true;
         return nullptr;
     }
     parent->mIsFlashPlugin = aPluginTag->mIsFlashPlugin;
     parent->mIsBlocklisted = aPluginTag->GetBlocklistState() != 0;
     if (!parent->mIsStartingAsync) {
--- a/dom/plugins/ipc/PluginProcessParent.cpp
+++ b/dom/plugins/ipc/PluginProcessParent.cpp
@@ -9,20 +9,16 @@
 #include "base/string_util.h"
 #include "base/process_util.h"
 
 #include "mozilla/ipc/BrowserProcessSubThread.h"
 #include "mozilla/plugins/PluginMessageUtils.h"
 #include "mozilla/Telemetry.h"
 #include "nsThreadUtils.h"
 
-#if defined(XP_WIN) && defined(MOZ_SANDBOX)
-#include "mozilla/Preferences.h"
-#endif
-
 using std::vector;
 using std::string;
 
 using mozilla::ipc::BrowserProcessSubThread;
 using mozilla::ipc::GeckoChildProcessHost;
 using mozilla::plugins::LaunchCompleteTask;
 using mozilla::plugins::PluginProcessParent;
 using base::ProcessArchitecture;
@@ -43,24 +39,22 @@ PluginProcessParent::PluginProcessParent
 }
 
 PluginProcessParent::~PluginProcessParent()
 {
 }
 
 bool
 PluginProcessParent::Launch(mozilla::UniquePtr<LaunchCompleteTask> aLaunchCompleteTask,
-                            bool aEnableSandbox)
+                            int32_t aSandboxLevel)
 {
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
-    mEnableNPAPISandbox = aEnableSandbox;
-    mMoreStrictSandbox =
-      Preferences::GetBool("dom.ipc.plugins.moreStrictSandbox");
+    mSandboxLevel = aSandboxLevel;
 #else
-    if (aEnableSandbox) {
+    if (aSandboxLevel != 0) {
         MOZ_ASSERT(false,
                    "Can't enable an NPAPI process sandbox for platform/build.");
     }
 #endif
 
     ProcessArchitecture currentArchitecture = base::GetCurrentProcessArchitecture();
     uint32_t containerArchitectures = GetSupportedArchitecturesForProcessType(GeckoProcessType_Plugin);
 
--- a/dom/plugins/ipc/PluginProcessParent.h
+++ b/dom/plugins/ipc/PluginProcessParent.h
@@ -45,21 +45,21 @@ public:
     ~PluginProcessParent();
 
     /**
      * Launch the plugin process. If the process fails to launch,
      * this method will return false.
      *
      * @param aLaunchCompleteTask Task that is executed on the main
      * thread once the asynchonous launch has completed.
-     * @param aEnableSandbox Enables a process sandbox if one is available for
-     * this platform/build. Will assert if true passed and one is not available.
+     * @param aSandboxLevel Determines the strength of the sandbox.
+     * <= 0 means no sandbox.
      */
     bool Launch(UniquePtr<LaunchCompleteTask> aLaunchCompleteTask = UniquePtr<LaunchCompleteTask>(),
-                bool aEnableSandbox = false);
+                int32_t aSandboxLevel = 0);
 
     void Delete();
 
     virtual bool CanShutdown() MOZ_OVERRIDE
     {
         return true;
     }
 
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -92,17 +92,17 @@ GeckoChildProcessHost::GeckoChildProcess
   : ChildProcessHost(RENDER_PROCESS), // FIXME/cjones: we should own this enum
     mProcessType(aProcessType),
     mPrivileges(aPrivileges),
     mMonitor("mozilla.ipc.GeckChildProcessHost.mMonitor"),
     mProcessState(CREATING_CHANNEL),
     mDelegate(nullptr),
 #if defined(MOZ_SANDBOX) && defined(XP_WIN)
     mEnableSandboxLogging(false),
-    mEnableNPAPISandbox(false),
+    mSandboxLevel(0),
     mMoreStrictSandbox(false),
 #endif
     mChildProcessHandle(0)
 #if defined(MOZ_WIDGET_COCOA)
   , mChildTask(MACH_PORT_NULL)
 #endif
 {
     MOZ_COUNT_CTOR(GeckoChildProcessHost);
@@ -796,30 +796,34 @@ GeckoChildProcessHost::PerformAsyncLaunc
     if (file && NS_SUCCEEDED(file->GetPath(path))) {
       cmdLine.AppendLooseValue(UTF8ToWide("-appomni"));
       cmdLine.AppendLooseValue(path.get());
     }
   }
 
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
   bool shouldSandboxCurrentProcess = false;
+
+  // XXX: Bug 1124167: We should get rid of the process specific logic for
+  // sandboxing in this class at some point. Unfortunately it will take a bit
+  // of reorganizing so I don't think this patch is the right time.
   switch (mProcessType) {
     case GeckoProcessType_Content:
 #if defined(MOZ_CONTENT_SANDBOX)
       if (!PR_GetEnv("MOZ_DISABLE_CONTENT_SANDBOX")) {
         mSandboxBroker.SetSecurityLevelForContentProcess(mMoreStrictSandbox);
         cmdLine.AppendLooseValue(UTF8ToWide("-sandbox"));
         shouldSandboxCurrentProcess = true;
       }
 #endif // MOZ_CONTENT_SANDBOX
       break;
     case GeckoProcessType_Plugin:
-      if (mEnableNPAPISandbox &&
+      if (mSandboxLevel > 0 &&
           !PR_GetEnv("MOZ_DISABLE_NPAPI_SANDBOX")) {
-        mSandboxBroker.SetSecurityLevelForPluginProcess(mMoreStrictSandbox);
+        mSandboxBroker.SetSecurityLevelForPluginProcess(mSandboxLevel);
         cmdLine.AppendLooseValue(UTF8ToWide("-sandbox"));
         shouldSandboxCurrentProcess = true;
       }
       break;
     case GeckoProcessType_IPDLUnitTest:
       // XXX: We don't sandbox this process type yet
       // mSandboxBroker.SetSecurityLevelForIPDLUnitTestProcess();
       // cmdLine.AppendLooseValue(UTF8ToWide("-sandbox"));
--- a/ipc/glue/GeckoChildProcessHost.h
+++ b/ipc/glue/GeckoChildProcessHost.h
@@ -167,21 +167,17 @@ protected:
 #ifdef XP_WIN
   void InitWindowsGroupID();
   nsString mGroupId;
 
 #ifdef MOZ_SANDBOX
   SandboxBroker mSandboxBroker;
   std::vector<std::wstring> mAllowedFilesRead;
   bool mEnableSandboxLogging;
-
-  // XXX: Bug 1124167: We should get rid of the process specific logic for
-  // sandboxing in this class at some point. Unfortunately it will take a bit
-  // of reorganizing so I don't think this patch is the right time.
-  bool mEnableNPAPISandbox;
+  int32_t mSandboxLevel;
   bool mMoreStrictSandbox;
 #endif
 #endif // XP_WIN
 
 #if defined(OS_POSIX)
   base::file_handle_mapping_vector mFileMap;
 #endif
 
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -112,25 +112,25 @@ SandboxBroker::SetSecurityLevelForConten
                             L"\\??\\pipe\\chrome.*");
   ret = ret && (sandbox::SBOX_ALL_OK == result);
 
   return ret;
 }
 #endif
 
 bool
-SandboxBroker::SetSecurityLevelForPluginProcess(bool aMoreStrict)
+SandboxBroker::SetSecurityLevelForPluginProcess(int32_t aSandboxLevel)
 {
   if (!mPolicy) {
     return false;
   }
 
   sandbox::ResultCode result;
   bool ret;
-  if (aMoreStrict) {
+  if (aSandboxLevel >= 2) {
     result = mPolicy->SetJobLevel(sandbox::JOB_UNPROTECTED,
                                      0 /* ui_exceptions */);
     ret = (sandbox::SBOX_ALL_OK == result);
 
     result = mPolicy->SetTokenLevel(sandbox::USER_RESTRICTED_SAME_ACCESS,
                                     sandbox::USER_INTERACTIVE);
     ret = ret && (sandbox::SBOX_ALL_OK == result);
 
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
@@ -8,16 +8,18 @@
 #define __SECURITY_SANDBOX_SANDBOXBROKER_H__
 
 #ifdef SANDBOX_EXPORTS
 #define SANDBOX_EXPORT __declspec(dllexport)
 #else
 #define SANDBOX_EXPORT __declspec(dllimport)
 #endif
 
+#include <stdint.h>
+
 namespace sandbox {
   class BrokerServices;
   class TargetPolicy;
 }
 
 namespace mozilla {
 
 class SANDBOX_EXPORT SandboxBroker
@@ -29,17 +31,17 @@ public:
                  const bool aEnableLogging,
                  void **aProcessHandle);
   virtual ~SandboxBroker();
 
   // Security levels for different types of processes
 #if defined(MOZ_CONTENT_SANDBOX)
   bool SetSecurityLevelForContentProcess(bool aMoreStrict);
 #endif
-  bool SetSecurityLevelForPluginProcess(bool aMoreStrict);
+  bool SetSecurityLevelForPluginProcess(int32_t aSandboxLevel);
   bool SetSecurityLevelForIPDLUnitTestProcess();
   bool SetSecurityLevelForGMPlugin();
 
   // File system permissions
   bool AllowReadFile(wchar_t const *file);
   bool AllowReadWriteFile(wchar_t const *file);
   bool AllowDirectory(wchar_t const *dir);