Bug 1229829 - Part 1 - Apply chromium sandbox patches from upstream which improves alternate desktop support; r=bobowen

This is 0cb5dadc2b1f84fbbd9c6f75056e38d05a5b07d3 and
db4c64b63d6098294ed255e962700fd2d465575e in the chromium repository.

This allows a single process to create sandboxed children with alternate
desktops on both an alternate winstation and the local winstation.

MozReview-Commit-ID: 8sS7LjoveOk

--HG--
extra : rebase_source : 6915af73743f87ed74ddefe04210dbdd95bb56ed

security/sandbox/chromium/sandbox/win/src/policy_target_test.cc

@@ -352,10 +352,41 @@ TEST(PolicyTargetTest, WinstaPolicy) {
   temp_policy->Release();
 }
 
+// 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);
+
+  base::string16 policy1_desktop_name = policy1->GetAlternateDesktop();
+  base::string16 policy2_desktop_name = policy2->GetAlternateDesktop();
+
+  // Extract only the "desktop name" portion of
+  // "{winstation name}\\{desktop name}"
+  EXPECT_NE(policy1_desktop_name.substr(
+                policy1_desktop_name.find_first_of(L'\\') + 1),
+            policy2_desktop_name.substr(
+                policy2_desktop_name.find_first_of(L'\\') + 1));
+
+  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);
 

security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc

@@ -109,11 +109,17 @@ namespace sandbox {
 SANDBOX_INTERCEPT IntegrityLevel g_shared_delayed_integrity_level;
 SANDBOX_INTERCEPT MitigationFlags g_shared_delayed_mitigations;
 
-// Initializes static members.
+// Initializes static members. alternate_desktop_handle_ is a desktop on
-HWINSTA PolicyBase::alternate_winstation_handle_ = NULL;
+// alternate_winstation_handle_, alternate_desktop_local_winstation_handle_ is a
-HDESK PolicyBase::alternate_desktop_handle_ = NULL;
+// 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;
+IntegrityLevel
+    PolicyBase::alternate_desktop_local_winstation_integrity_level_label_ =
+        INTEGRITY_LEVEL_SYSTEM;
 
 PolicyBase::PolicyBase()
     : ref_count(1),
@@ -220,27 +226,26 @@ base::string16 PolicyBase::GetAlternateDesktop() const {
     return base::string16();
   }
 
-  // The desktop and winstation should have been created by now.
+  if (use_alternate_winstation_) {
-  // If we hit this scenario, it means that the user ignored the failure
+    // The desktop and winstation should have been created by now.
-  // during SetAlternateDesktop, so we ignore it here too.
+    // If we hit this scenario, it means that the user ignored the failure
-  if (use_alternate_desktop_ && !alternate_desktop_handle_) {
+    // during SetAlternateDesktop, so we ignore it here too.
-    return base::string16();
+    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;
@@ -267,22 +272,19 @@ ResultCode PolicyBase::CreateAlternateDesktop(bool alternate_winstation) {
         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_ ||
+    if (!alternate_desktop_local_winstation_handle_ ||
-        GetWindowObjectName(alternate_desktop_handle_).empty())
+        GetWindowObjectName(alternate_desktop_local_winstation_handle_).empty())
       return SBOX_ERROR_CANNOT_CREATE_DESKTOP;
   }
 
@@ -290,14 +292,21 @@ ResultCode PolicyBase::CreateAlternateDesktop(bool alternate_winstation) {
 }
 
 void PolicyBase::DestroyAlternateDesktop() {
-  if (alternate_desktop_handle_) {
+  if (use_alternate_winstation_) {
-    ::CloseDesktop(alternate_desktop_handle_);
+    if (alternate_desktop_handle_) {
-    alternate_desktop_handle_ = NULL;
+      ::CloseDesktop(alternate_desktop_handle_);
-  }
+      alternate_desktop_handle_ = nullptr;
+    }
 
-  if (alternate_winstation_handle_) {
+    if (alternate_winstation_handle_) {
-    ::CloseWindowStation(alternate_winstation_handle_);
+      ::CloseWindowStation(alternate_winstation_handle_);
-    alternate_winstation_handle_ = NULL;
+      alternate_winstation_handle_ = nullptr;
+    }
+  } else {
+    if (alternate_desktop_local_winstation_handle_) {
+      ::CloseDesktop(alternate_desktop_local_winstation_handle_);
+      alternate_desktop_local_winstation_handle_ = nullptr;
+    }
   }
 }
 
@@ -450,20 +459,35 @@ ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial,
   // 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_ &&
+  if (use_alternate_desktop_ && integrity_level_ != INTEGRITY_LEVEL_LAST) {
-      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_,
+    HDESK desktop_handle = nullptr;
-                                     SE_WINDOW_OBJECT,
+    IntegrityLevel desktop_integrity_level_label;
-                                     L"",
+    if (use_alternate_winstation_) {
-                                     GetIntegrityLevelString(integrity_level_));
+      desktop_handle = alternate_desktop_handle_;
-    if (ERROR_SUCCESS != result)
+      desktop_integrity_level_label = alternate_desktop_integrity_level_label_;
-      return SBOX_ERROR_GENERIC;
+    } else {
+      desktop_handle = alternate_desktop_local_winstation_handle_;
+      desktop_integrity_level_label =
+          alternate_desktop_local_winstation_integrity_level_label_;
+    }
+    // If the desktop_handle hasn't been created for any reason, skip this.
+    if (desktop_handle && desktop_integrity_level_label < integrity_level_) {
+      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_;
+      if (use_alternate_winstation_) {
+        alternate_desktop_integrity_level_label_ = integrity_level_;
+      } else {
+        alternate_desktop_local_winstation_integrity_level_label_ =
+            integrity_level_;
+      }
+    }
   }
 
   if (lowbox_sid_) {

security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.h

@@ -162,7 +162,10 @@ class PolicyBase final : public TargetPolicy {
 
   static HDESK alternate_desktop_handle_;
   static HWINSTA alternate_winstation_handle_;
+  static HDESK alternate_desktop_local_winstation_handle_;
   static IntegrityLevel alternate_desktop_integrity_level_label_;
+  static IntegrityLevel
+      alternate_desktop_local_winstation_integrity_level_label_;
 
   // Contains the list of handles being shared with the target process.
   // This list contains handles other than the stderr/stdout handles which are

security/sandbox/chromium/sandbox/win/src/window.cc

@@ -31,7 +31,7 @@ bool GetSecurityAttributes(HANDLE handle, SECURITY_ATTRIBUTES* attributes) {
   return false;
 }
 
-}
+}  // namespace
 
 namespace sandbox {
 
@@ -65,6 +65,10 @@ ResultCode CreateAltWindowStation(HWINSTA* winsta) {
 ResultCode CreateAltDesktop(HWINSTA winsta, HDESK* desktop) {
   base::string16 desktop_name = L"sbox_alternate_desktop_";
 
+  if (!winsta) {
+    desktop_name += L"local_winstation_";
+  }
+
   // Append the current PID to the desktop name.
   wchar_t buffer[16];
   _snwprintf_s(buffer, sizeof(buffer) / sizeof(wchar_t), L"0x%X",

security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt

@@ -8,3 +8,5 @@ https://hg.mozilla.org/mozilla-central/rev/d24db55deb85
 https://hg.mozilla.org/mozilla-central/rev/0e6bf137521e
 https://hg.mozilla.org/mozilla-central/rev/1755a454e2de
 https://bugzilla.mozilla.org/show_bug.cgi?id=1385928 bug1385928.patch
+https://chromium.googlesource.com/chromium/src/+/0cb5dadc2b1f84fbbd9c6f75056e38d05a5b07d3
+https://chromium.googlesource.com/chromium/src/+/db4c64b63d6098294ed255e962700fd2d465575e