Never add win32k or signed policy rules on unsupported OS.

This CL also ensures that mitigation policy is applied before
adding any rules that depend on the policy.

It also restores the sandbox convention that mitigation
policies and rules can always be requested by a sandbox
user and the sandbox will try and enforce the maximum set
of rules supported by the running OS. i.e. the user should
not be aware of OS capabilities, merely sandbox capabilities.

BUG=996834

Change-Id: I5a8432d94dc1bcfbe327589f2b21c4c76fb9fedb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810937Reviewed-by: James Forshaw <forshaw@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697622}

sandbox/win/src/sandbox_policy_base.cc

@@ -759,17 +759,30 @@ ResultCode PolicyBase::AddRuleInternal(SubSystem subsystem,
       break;
     }
     case SUBSYS_WIN32K_LOCKDOWN: {
-      if (!ProcessMitigationsWin32KLockdownPolicy::GenerateRules(
-              pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
+      // Win32k intercept rules only supported on Windows 8 and above. This must
+      // match the version checks in process_mitigations.cc for consistency.
+      if (base::win::GetVersion() >= base::win::Version::WIN8) {
+        DCHECK_EQ(MITIGATION_WIN32K_DISABLE,
+                  mitigations_ & MITIGATION_WIN32K_DISABLE)
+            << "Enable MITIGATION_WIN32K_DISABLE before adding win32k policy "
+               "rules.";
+        if (!ProcessMitigationsWin32KLockdownPolicy::GenerateRules(
+                pattern, semantics, policy_maker_)) {
+          NOTREACHED();
+          return SBOX_ERROR_BAD_PARAMS;
+        }
       }
       break;
     }
     case SUBSYS_SIGNED_BINARY: {
-      // These rules only need to be added if the
-      // MITIGATION_FORCE_MS_SIGNED_BINS pre-startup mitigation is set.
-      if (mitigations_ & MITIGATION_FORCE_MS_SIGNED_BINS) {
+      // Signed intercept rules only supported on Windows 10 TH2 and above. This
+      // must match the version checks in process_mitigations.cc for
+      // consistency.
+      if (base::win::GetVersion() >= base::win::Version::WIN10_TH2) {
+        DCHECK_EQ(MITIGATION_FORCE_MS_SIGNED_BINS,
+                  mitigations_ & MITIGATION_FORCE_MS_SIGNED_BINS)
+            << "Enable MITIGATION_FORCE_MS_SIGNED_BINS before adding signed "
+               "policy rules.";
         if (!SignedPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
           NOTREACHED();
           return SBOX_ERROR_BAD_PARAMS;

services/service_manager/sandbox/win/sandbox_win.cc

@@ -776,13 +776,16 @@ sandbox::ResultCode SandboxWin::AddWin32kLockdownPolicy(
   if (!service_manager::IsWin32kLockdownEnabled())
     return sandbox::SBOX_ALL_OK;
 
-  // Enable win32k lockdown if not already.
   sandbox::MitigationFlags flags = policy->GetProcessMitigations();
-  if ((flags & sandbox::MITIGATION_WIN32K_DISABLE) ==
-      sandbox::MITIGATION_WIN32K_DISABLE)
-    return sandbox::SBOX_ALL_OK;
+  // Check not enabling twice. Should not happen.
+  DCHECK_EQ(0U, flags & sandbox::MITIGATION_WIN32K_DISABLE);
 
-  sandbox::ResultCode result =
+  flags |= sandbox::MITIGATION_WIN32K_DISABLE;
+  sandbox::ResultCode result = policy->SetProcessMitigations(flags);
+  if (result != sandbox::SBOX_ALL_OK)
+    return result;
+
+  result =
       policy->AddRule(sandbox::TargetPolicy::SUBSYS_WIN32K_LOCKDOWN,
                       enable_opm ? sandbox::TargetPolicy::IMPLEMENT_OPM_APIS
                                  : sandbox::TargetPolicy::FAKE_USER_GDI_INIT,
@@ -792,8 +795,7 @@ sandbox::ResultCode SandboxWin::AddWin32kLockdownPolicy(
   if (enable_opm)
     policy->SetEnableOPMRedirection();
 
-  flags |= sandbox::MITIGATION_WIN32K_DISABLE;
-  return policy->SetProcessMitigations(flags);
+  return result;
 #else
   return sandbox::SBOX_ALL_OK;
 #endif