Bug 1392570 - On Windows 7 don't attempt to use a job object for the sandbox when it will fail. r=jimm, data-r=rweiss, a=gchang
authorBob Owen <bobowencode@gmail.com>
Fri, 01 Sep 2017 14:05:49 +0100
changeset 423942 984bda56d03cebb368dd9340fa892bdd41b0b5d0
parent 423941 d1940f3854236047ea6c6be07280caa312ecb41b
child 423943 edd07c36dc0dc87e587fbfc61aec8916d4226661
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, gchang
bugs1392570
milestone56.0
Bug 1392570 - On Windows 7 don't attempt to use a job object for the sandbox when it will fail. r=jimm, data-r=rweiss, a=gchang This patch also adds telemetry for when this occurs, breaking it down for local and remote sessions.
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
toolkit/components/telemetry/Scalars.yaml
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -10,16 +10,17 @@
 
 #include "base/win/windows_version.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Logging.h"
 #include "mozilla/NSPRLogModulesParser.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Telemetry.h"
+#include "mozilla/WindowsVersion.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "nsCOMPtr.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsIFile.h"
 #include "nsIProperties.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
 #include "sandbox/win/src/sandbox.h"
@@ -248,16 +249,77 @@ AddCachedDirRule(sandbox::TargetPolicy* 
                      rulePath.get());
   if (sandbox::SBOX_ALL_OK != result) {
     NS_ERROR("Failed to add file policy rule.");
     LOG_E("Failed (ResultCode %d) to add %d access to: %S",
           result, aAccess, rulePath.get());
   }
 }
 
+// Checks whether we can use a job object as part of the sandbox.
+static bool
+CanUseJob()
+{
+  // Windows 8 and later allows nested jobs, no need for further checks.
+  if (IsWin8OrLater()) {
+    return true;
+  }
+
+  BOOL inJob = true;
+  // If we can't determine if we are in a job then assume we can use one.
+  if (!::IsProcessInJob(::GetCurrentProcess(), nullptr, &inJob)) {
+    return true;
+  }
+
+  // If there is no job then we are fine to use one.
+  if (!inJob) {
+    return true;
+  }
+
+  JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = {};
+  // If we can't get the job object flags then again assume we can use a job.
+  if (!::QueryInformationJobObject(nullptr, JobObjectExtendedLimitInformation,
+                                   &job_info, sizeof(job_info), nullptr)) {
+    return true;
+  }
+
+  // If we can break away from the current job then we are free to set our own.
+  if (job_info.BasicLimitInformation.LimitFlags & JOB_OBJECT_LIMIT_BREAKAWAY_OK) {
+    return true;
+  }
+
+  // Chromium added a command line flag to allow no job to be used, which was
+  // originally supposed to only be used for remote sessions. If you use runas
+  // to start Firefox then this also uses a separate job and we would fail to
+  // start on Windows 7. An unknown number of people use (or used to use) runas
+  // with Firefox for some security benefits (see bug 1228880). This is now a
+  // counterproductive technique, but allowing both the remote and local case
+  // for now and adding telemetry to see if we can restrict this to just remote.
+  nsAutoString localRemote(::GetSystemMetrics(SM_REMOTESESSION)
+                           ? u"remote" : u"local");
+  Telemetry::ScalarSet(Telemetry::ScalarID::SANDBOX_NO_JOB, localRemote, true);
+
+  // Allow running without the job object in this case. This slightly reduces the
+  // ability of the sandbox to protect its children from spawning new processes
+  // or preventing them from shutting down Windows or accessing the clipboard.
+  return false;
+}
+
+static sandbox::ResultCode
+SetJobLevel(sandbox::TargetPolicy* aPolicy, sandbox::JobLevel aJobLevel,
+            uint32_t aUiExceptions)
+{
+  static bool sCanUseJob = CanUseJob();
+  if (sCanUseJob) {
+    return aPolicy->SetJobLevel(aJobLevel, aUiExceptions);
+  }
+
+  return aPolicy->SetJobLevel(sandbox::JOB_NONE, 0);
+}
+
 #if defined(MOZ_CONTENT_SANDBOX)
 
 void
 SandboxBroker::SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
                                                  base::ChildPrivileges aPrivs)
 {
   MOZ_RELEASE_ASSERT(mPolicy, "mPolicy must be set before this call.");
 
@@ -309,17 +371,17 @@ SandboxBroker::SetSecurityLevelForConten
 
 #if defined(DEBUG)
   // This is required for a MOZ_ASSERT check in WindowsMessageLoop.cpp
   // WinEventHook, see bug 1366694 for details.
   DWORD uiExceptions = JOB_OBJECT_UILIMIT_HANDLES;
 #else
   DWORD uiExceptions = 0;
 #endif
-  sandbox::ResultCode result = mPolicy->SetJobLevel(jobLevel, uiExceptions);
+  sandbox::ResultCode result = SetJobLevel(mPolicy, jobLevel, uiExceptions);
   MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
                      "Setting job level failed, have you set memory limit when jobLevel == JOB_NONE?");
 
   // If the delayed access token is not restricted we don't want the initial one
   // to be either, because it can interfere with running from a network drive.
   sandbox::TokenLevel initialAccessTokenLevel =
     (accessTokenLevel == sandbox::USER_UNPROTECTED ||
      accessTokenLevel == sandbox::USER_NON_ADMIN)
@@ -465,18 +527,18 @@ SandboxBroker::SetSecurityLevelForGPUPro
     delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW;
   } else if (aSandboxLevel == 1) {
     jobLevel = sandbox::JOB_NONE;
     accessTokenLevel = sandbox::USER_NON_ADMIN;
     initialIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW;
     delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW;
   }
 
-  sandbox::ResultCode result = mPolicy->SetJobLevel(jobLevel,
-                                                    0 /* ui_exceptions */);
+  sandbox::ResultCode result = SetJobLevel(mPolicy, jobLevel,
+                                           0 /* ui_exceptions */);
   MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
                      "Setting job level failed, have you set memory limit when jobLevel == JOB_NONE?");
 
   // If the delayed access token is not restricted we don't want the initial one
   // to be either, because it can interfere with running from a network drive.
   sandbox::TokenLevel initialAccessTokenLevel =
     (accessTokenLevel == sandbox::USER_UNPROTECTED ||
      accessTokenLevel == sandbox::USER_NON_ADMIN)
@@ -575,18 +637,18 @@ SandboxBroker::SetSecurityLevelForPlugin
     jobLevel = sandbox::JOB_NONE;
     accessTokenLevel = sandbox::USER_NON_ADMIN;
     initialIntegrityLevel = sandbox::INTEGRITY_LEVEL_MEDIUM;
     delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_MEDIUM;
   }
 
   mPolicy->SetDoNotUseRestrictingSIDs();
 
-  sandbox::ResultCode result = mPolicy->SetJobLevel(jobLevel,
-                                                    0 /* ui_exceptions */);
+  sandbox::ResultCode result = SetJobLevel(mPolicy, jobLevel,
+                                           0 /* ui_exceptions */);
   SANDBOX_ENSURE_SUCCESS(result,
                          "Setting job level failed, have you set memory limit when jobLevel == JOB_NONE?");
 
   result = mPolicy->SetTokenLevel(sandbox::USER_RESTRICTED_SAME_ACCESS,
                                   accessTokenLevel);
   SANDBOX_ENSURE_SUCCESS(result,
                          "Lockdown level cannot be USER_UNPROTECTED or USER_LAST if initial level was USER_RESTRICTED_SAME_ACCESS");
 
@@ -694,17 +756,18 @@ SandboxBroker::SetSecurityLevelForPlugin
 
 bool
 SandboxBroker::SetSecurityLevelForGMPlugin(SandboxLevel aLevel)
 {
   if (!mPolicy) {
     return false;
   }
 
-  auto result = mPolicy->SetJobLevel(sandbox::JOB_LOCKDOWN, 0);
+  auto result = SetJobLevel(mPolicy, sandbox::JOB_LOCKDOWN,
+                            0 /* ui_exceptions */);
   SANDBOX_ENSURE_SUCCESS(result,
                          "SetJobLevel should never fail with these arguments, what happened?");
   auto level = (aLevel == Restricted) ?
     sandbox::USER_RESTRICTED : sandbox::USER_LOCKDOWN;
   result = mPolicy->SetTokenLevel(sandbox::USER_RESTRICTED_SAME_ACCESS, level);
   SANDBOX_ENSURE_SUCCESS(result,
                          "SetTokenLevel should never fail with these arguments, what happened?");
 
--- a/toolkit/components/telemetry/Scalars.yaml
+++ b/toolkit/components/telemetry/Scalars.yaml
@@ -439,16 +439,33 @@ security:
     kind: boolean
     keyed: true
     notification_emails:
       - seceng-telemetry@mozilla.com
     release_channel_collection: opt-out
     record_in_processes:
       - main
 
+sandbox:
+  no_job:
+    bug_numbers:
+      - 1392570
+    description: >
+      Indicates that we can't use a job object for sandboxed child processes.
+      Keyed by whether we are in a remote session or not from ::GetSystemMetrics(SM_REMOTESESSION).
+    expires: "62"
+    kind: boolean
+    keyed: true
+    notification_emails:
+      - bowen@mozilla.com
+    release_channel_collection: opt-out
+    record_in_processes:
+      - main
+    cpp_guard: 'XP_WIN'
+
 preferences:
   created_new_user_prefs_file:
     bug_numbers:
       - 1367813
     description: >-
       A boolean indicating that profile/prefs.js was not found and it is being
       created for the first time in this session.
     expires: "62"