Added support for using alternate desktops both and with and without alternate winstations in a single process

Right now it's not possible to have both configurations from a single parent
process. While I doubt Chromium needs this, it'd be useful for Firefox, where we
currently have one sandbox that uses an alternate desktop and an alternate
winstation, and we'd like to enable an alternate desktop for another sandbox but
it's not ready for an alternate winstation yet.

R=wfh@chromium.org

Bug: 
Cq-Include-Trybots: master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: I32adebf31a5627f52c0c98e95da146a4963568e3
Reviewed-on: https://chromium-review.googlesource.com/615145
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495376}

diff --git a/sandbox/win/src/policy_target_test.cc b/sandbox/win/src/policy_target_test.cc
index 7eaab94..4fadd0dd4 100644
--- a/sandbox/win/src/policy_target_test.cc
+++ b/sandbox/win/src/policy_target_test.cc

@@ -361,10 +361,31 @@
   temp_policy = nullptr;
 }
 
+// Creates multiple policies, with alternate desktops on both local and
+// alternate winstations.
+TEST(PolicyTargetTest, BothLocalAndAlternateWinstationDesktop) {
+  BrokerServices* broker = GetBroker();
+
+  scoped_refptr<TargetPolicy> policy1 = broker->CreatePolicy();
+  scoped_refptr<TargetPolicy> policy2 = broker->CreatePolicy();
+  scoped_refptr<TargetPolicy> policy3 = broker->CreatePolicy();
+
+  ResultCode result;
+  result = policy1->SetAlternateDesktop(false);
+  EXPECT_EQ(SBOX_ALL_OK, result);
+  result = policy2->SetAlternateDesktop(true);
+  EXPECT_EQ(SBOX_ALL_OK, result);
+  result = policy3->SetAlternateDesktop(false);
+  EXPECT_EQ(SBOX_ALL_OK, result);
+
+  policy1->DestroyAlternateDesktop();
+  policy2->DestroyAlternateDesktop();
+  policy3->DestroyAlternateDesktop();
+}
+
 // Launches the app in the sandbox and share a handle with it. The app should
 // be able to use the handle.
 TEST(PolicyTargetTest, ShareHandleTest) {
-
   BrokerServices* broker = GetBroker();
   ASSERT_TRUE(broker != NULL);
 

diff --git a/sandbox/win/src/sandbox_policy_base.cc b/sandbox/win/src/sandbox_policy_base.cc
index 25423e2..2d63df5 100644
--- a/sandbox/win/src/sandbox_policy_base.cc
+++ b/sandbox/win/src/sandbox_policy_base.cc

@@ -108,9 +108,12 @@
 SANDBOX_INTERCEPT IntegrityLevel g_shared_delayed_integrity_level;
 SANDBOX_INTERCEPT MitigationFlags g_shared_delayed_mitigations;
 
-// Initializes static members.
-HWINSTA PolicyBase::alternate_winstation_handle_ = NULL;
-HDESK PolicyBase::alternate_desktop_handle_ = NULL;
+// Initializes static members. alternate_desktop_handle_ is a desktop on
+// alternate_winstation_handle_, alternate_desktop_local_winstation_handle_ is a
+// desktop on the same winstation as the parent process.
+HWINSTA PolicyBase::alternate_winstation_handle_ = nullptr;
+HDESK PolicyBase::alternate_desktop_handle_ = nullptr;
+HDESK PolicyBase::alternate_desktop_local_winstation_handle_ = nullptr;
 IntegrityLevel PolicyBase::alternate_desktop_integrity_level_label_ =
     INTEGRITY_LEVEL_SYSTEM;
 
@@ -212,27 +215,26 @@
     return base::string16();
   }
 
-  // The desktop and winstation should have been created by now.
-  // If we hit this scenario, it means that the user ignored the failure
-  // during SetAlternateDesktop, so we ignore it here too.
-  if (use_alternate_desktop_ && !alternate_desktop_handle_) {
-    return base::string16();
+  if (use_alternate_winstation_) {
+    // The desktop and winstation should have been created by now.
+    // If we hit this scenario, it means that the user ignored the failure
+    // during SetAlternateDesktop, so we ignore it here too.
+    if (!alternate_desktop_handle_ || !alternate_winstation_handle_) {
+      return base::string16();
+    }
+    return GetFullDesktopName(alternate_winstation_handle_,
+                              alternate_desktop_handle_);
+  } else {
+    if (!alternate_desktop_local_winstation_handle_) {
+      return base::string16();
+    }
+    return GetFullDesktopName(nullptr,
+                              alternate_desktop_local_winstation_handle_);
   }
-  if (use_alternate_winstation_ && (!alternate_desktop_handle_ ||
-                                    !alternate_winstation_handle_)) {
-    return base::string16();
-  }
-
-  return GetFullDesktopName(alternate_winstation_handle_,
-                            alternate_desktop_handle_);
 }
 
 ResultCode PolicyBase::CreateAlternateDesktop(bool alternate_winstation) {
   if (alternate_winstation) {
-    // Previously called with alternate_winstation = false?
-    if (!alternate_winstation_handle_ && alternate_desktop_handle_)
-      return SBOX_ERROR_UNSUPPORTED;
-
     // Check if it's already created.
     if (alternate_winstation_handle_ && alternate_desktop_handle_)
       return SBOX_ALL_OK;
@@ -259,22 +261,19 @@
         GetWindowObjectName(alternate_desktop_handle_).empty())
       return SBOX_ERROR_CANNOT_CREATE_DESKTOP;
   } else {
-    // Previously called with alternate_winstation = true?
-    if (alternate_winstation_handle_)
-      return SBOX_ERROR_UNSUPPORTED;
-
     // Check if it already exists.
-    if (alternate_desktop_handle_)
+    if (alternate_desktop_local_winstation_handle_)
       return SBOX_ALL_OK;
 
     // Create the destkop.
-    ResultCode result = CreateAltDesktop(NULL, &alternate_desktop_handle_);
+    ResultCode result =
+        CreateAltDesktop(nullptr, &alternate_desktop_local_winstation_handle_);
     if (SBOX_ALL_OK != result)
       return result;
 
     // Verify that everything is fine.
-    if (!alternate_desktop_handle_ ||
-        GetWindowObjectName(alternate_desktop_handle_).empty())
+    if (!alternate_desktop_local_winstation_handle_ ||
+        GetWindowObjectName(alternate_desktop_local_winstation_handle_).empty())
       return SBOX_ERROR_CANNOT_CREATE_DESKTOP;
   }
 
@@ -282,14 +281,21 @@
 }
 
 void PolicyBase::DestroyAlternateDesktop() {
-  if (alternate_desktop_handle_) {
-    ::CloseDesktop(alternate_desktop_handle_);
-    alternate_desktop_handle_ = NULL;
-  }
+  if (use_alternate_winstation_) {
+    if (alternate_desktop_handle_) {
+      ::CloseDesktop(alternate_desktop_handle_);
+      alternate_desktop_handle_ = nullptr;
+    }
 
-  if (alternate_winstation_handle_) {
-    ::CloseWindowStation(alternate_winstation_handle_);
-    alternate_winstation_handle_ = NULL;
+    if (alternate_winstation_handle_) {
+      ::CloseWindowStation(alternate_winstation_handle_);
+      alternate_winstation_handle_ = nullptr;
+    }
+  } else {
+    if (alternate_desktop_local_winstation_handle_) {
+      ::CloseDesktop(alternate_desktop_local_winstation_handle_);
+      alternate_desktop_local_winstation_handle_ = nullptr;
+    }
   }
 }
 
@@ -441,20 +447,27 @@
   // integrity label on the object is no higher than the sandboxed process's
   // integrity level. So, we lower the label on the desktop process if it's
   // not already low enough for our process.
-  if (alternate_desktop_handle_ && use_alternate_desktop_ &&
-      integrity_level_ != INTEGRITY_LEVEL_LAST &&
+  if (use_alternate_desktop_ && integrity_level_ != INTEGRITY_LEVEL_LAST &&
       alternate_desktop_integrity_level_label_ < integrity_level_) {
     // Integrity label enum is reversed (higher level is a lower value).
     static_assert(INTEGRITY_LEVEL_SYSTEM < INTEGRITY_LEVEL_UNTRUSTED,
                   "Integrity level ordering reversed.");
-    result = SetObjectIntegrityLabel(alternate_desktop_handle_,
-                                     SE_WINDOW_OBJECT,
-                                     L"",
-                                     GetIntegrityLevelString(integrity_level_));
-    if (ERROR_SUCCESS != result)
-      return SBOX_ERROR_GENERIC;
+    HDESK desktop_handle = nullptr;
+    if (use_alternate_winstation_) {
+      desktop_handle = alternate_desktop_handle_;
+    } else {
+      desktop_handle = alternate_desktop_local_winstation_handle_;
+    }
+    // If the desktop_handle hasn't been created for any reason, skip this.
+    if (desktop_handle) {
+      result =
+          SetObjectIntegrityLabel(desktop_handle, SE_WINDOW_OBJECT, L"",
+                                  GetIntegrityLevelString(integrity_level_));
+      if (ERROR_SUCCESS != result)
+        return SBOX_ERROR_GENERIC;
 
-    alternate_desktop_integrity_level_label_ = integrity_level_;
+      alternate_desktop_integrity_level_label_ = integrity_level_;
+    }
   }
 
   if (lowbox_sid_) {

diff --git a/sandbox/win/src/sandbox_policy_base.h b/sandbox/win/src/sandbox_policy_base.h
index 1ec005b..5cf6f96 100644
--- a/sandbox/win/src/sandbox_policy_base.h
+++ b/sandbox/win/src/sandbox_policy_base.h

@@ -160,6 +160,7 @@
 
   static HDESK alternate_desktop_handle_;
   static HWINSTA alternate_winstation_handle_;
+  static HDESK alternate_desktop_local_winstation_handle_;
   static IntegrityLevel alternate_desktop_integrity_level_label_;
 
   // Contains the list of handles being shared with the target process.