Win: Folds Lowbox concept into AppContainer

Folds the lowbox concept into AppContainer. This will unlock the ability
to add new security capabilities.

This change also renames AppContainerProfile to AppContainer since not all
app containers have profiles.

Validated the renderer process and GPU app containers, which exercise
these code paths, still worked appropriately.

Bug: 1184556
Change-Id: I39d276c6570d716041c463f2468527841572d3cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2734105
Reviewed-by: James Forshaw <forshaw@chromium.org>
Commit-Queue: Emily Andrews <emiled@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#872561}

sandbox/win/src/app_container.h

@@ -15,6 +15,8 @@
 
 namespace sandbox {
 
+enum AppContainerType { kNone, kDerived, kProfile, kLowbox };
+
 class AppContainer {
  public:
   // Increments the reference count of this object. The reference count must
@@ -67,6 +69,8 @@ class AppContainer {
   // Enable Low Privilege AC.
   virtual void SetEnableLowPrivilegeAppContainer(bool enable) = 0;
   virtual bool GetEnableLowPrivilegeAppContainer() = 0;
+
+  virtual AppContainerType GetAppContainerType() = 0;
 };
 
 }  // namespace sandbox

sandbox/win/src/app_container_base.cc

@@ -5,11 +5,13 @@
 #include <memory>
 
 #include <aclapi.h>
+#include <sddl.h>
 #include <userenv.h>
 
 #include "base/strings/stringprintf.h"
 #include "base/win/scoped_co_mem.h"
 #include "base/win/scoped_handle.h"
+#include "sandbox/win/src/acl.h"
 #include "sandbox/win/src/app_container_base.h"
 #include "sandbox/win/src/restricted_token_utils.h"
 #include "sandbox/win/src/win_utils.h"
@@ -97,7 +99,7 @@ AppContainerBase* AppContainerBase::CreateProfile(const wchar_t* package_name,
   if (FAILED(hr))
     return nullptr;
   std::unique_ptr<void, FreeSidDeleter> sid_deleter(package_sid);
-  return new AppContainerBase(Sid(package_sid));
+  return new AppContainerBase(Sid(package_sid), AppContainerType::kProfile);
 }
 
 // static
@@ -115,7 +117,17 @@ AppContainerBase* AppContainerBase::Open(const wchar_t* package_name) {
     return nullptr;
 
   std::unique_ptr<void, FreeSidDeleter> sid_deleter(package_sid);
-  return new AppContainerBase(Sid(package_sid));
+  return new AppContainerBase(Sid(package_sid), AppContainerType::kDerived);
+}
+
+// static
+AppContainerBase* AppContainerBase::CreateLowbox(const wchar_t* sid) {
+  PSID package_sid;
+  if (!ConvertStringSidToSid(sid, &package_sid))
+    return nullptr;
+
+  std::unique_ptr<void, LocalFreeDeleter> sid_deleter(package_sid);
+  return new AppContainerBase(Sid(package_sid), AppContainerType::kLowbox);
 }
 
 // static
@@ -129,10 +141,12 @@ bool AppContainerBase::Delete(const wchar_t* package_name) {
   return SUCCEEDED(delete_app_container_profile(package_name));
 }
 
-AppContainerBase::AppContainerBase(const Sid& package_sid)
+AppContainerBase::AppContainerBase(const Sid& package_sid,
+                                   AppContainerType type)
     : ref_count_(0),
       package_sid_(package_sid),
-      enable_low_privilege_app_container_(false) {}
+      enable_low_privilege_app_container_(false),
+      type_(type) {}
 
 AppContainerBase::~AppContainerBase() {}
 
@@ -159,7 +173,7 @@ bool AppContainerBase::GetRegistryLocation(REGSAM desired_access,
     return false;
 
   base::win::ScopedHandle token;
-  if (!BuildLowBoxToken(&token))
+  if (BuildLowBoxToken(&token) != SBOX_ALL_OK)
     return false;
 
   ScopedImpersonation impersonation(token);
@@ -245,7 +259,7 @@ bool AppContainerBase::AccessCheck(const wchar_t* object_name,
   DWORD priv_set_length = sizeof(PRIVILEGE_SET);
 
   base::win::ScopedHandle token;
-  if (!BuildLowBoxToken(&token))
+  if (BuildLowBoxToken(&token) != SBOX_ALL_OK)
     return false;
 
   return !!::AccessCheck(sd, token.Get(), desired_access, &generic_mapping,
@@ -309,15 +323,47 @@ bool AppContainerBase::GetEnableLowPrivilegeAppContainer() {
   return enable_low_privilege_app_container_;
 }
 
+AppContainerType AppContainerBase::GetAppContainerType() {
+  return type_;
+}
+
 std::unique_ptr<SecurityCapabilities>
 AppContainerBase::GetSecurityCapabilities() {
   return std::make_unique<SecurityCapabilities>(package_sid_, capabilities_);
 }
 
-bool AppContainerBase::BuildLowBoxToken(base::win::ScopedHandle* token) {
-  return CreateLowBoxToken(nullptr, IMPERSONATION,
-                           GetSecurityCapabilities().get(), nullptr, 0,
-                           token) == ERROR_SUCCESS;
+ResultCode AppContainerBase::BuildLowBoxToken(
+    base::win::ScopedHandle* token,
+    base::win::ScopedHandle* lockdown) {
+  if (type_ == AppContainerType::kLowbox) {
+    if (!lowbox_directory_.IsValid()) {
+      DWORD result = CreateLowBoxObjectDirectory(package_sid_.GetPSID(), true,
+                                                 &lowbox_directory_);
+      DCHECK(result == ERROR_SUCCESS);
+    }
+
+    // The order of handles isn't important in the CreateLowBoxToken call.
+    // The kernel will maintain a reference to the object directory handle.
+    HANDLE saved_handles[1] = {lowbox_directory_.Get()};
+    DWORD saved_handles_count = lowbox_directory_.IsValid() ? 1 : 0;
+
+    if (CreateLowBoxToken(lockdown->Get(), PRIMARY,
+                          GetSecurityCapabilities().get(), saved_handles,
+                          saved_handles_count, token) != ERROR_SUCCESS) {
+      return SBOX_ERROR_CANNOT_CREATE_LOWBOX_TOKEN;
+    }
+
+    if (!ReplacePackageSidInDacl(token->Get(), SE_KERNEL_OBJECT, package_sid_,
+                                 TOKEN_ALL_ACCESS)) {
+      return SBOX_ERROR_CANNOT_MODIFY_LOWBOX_TOKEN_DACL;
+    }
+  } else if (CreateLowBoxToken(nullptr, IMPERSONATION,
+                               GetSecurityCapabilities().get(), nullptr, 0,
+                               token) != ERROR_SUCCESS) {
+    return SBOX_ERROR_CANNOT_CREATE_LOWBOX_IMPERSONATION_TOKEN;
+  }
+
+  return SBOX_ALL_OK;
 }
 
 }  // namespace sandbox

sandbox/win/src/app_container_base.h

@@ -16,6 +16,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/win/scoped_handle.h"
 #include "sandbox/win/src/app_container.h"
+#include "sandbox/win/src/sandbox_types.h"
 #include "sandbox/win/src/security_capabilities.h"
 #include "sandbox/win/src/sid.h"
 
@@ -43,6 +44,7 @@ class AppContainerBase final : public AppContainer {
   bool AddImpersonationCapabilitySddl(const wchar_t* sddl_sid) override;
   void SetEnableLowPrivilegeAppContainer(bool enable) override;
   bool GetEnableLowPrivilegeAppContainer() override;
+  AppContainerType GetAppContainerType() override;
 
   // Get the package SID for this AC.
   Sid GetPackageSid() const;
@@ -64,19 +66,26 @@ class AppContainerBase final : public AppContainer {
                                          const wchar_t* display_name,
                                          const wchar_t* description);
 
-  // Opens an AppContainer object. No checks will be made on
+  // Opens a derived AppContainer object. No checks will be made on
   // whether the package exists or not.
   static AppContainerBase* Open(const wchar_t* package_name);
 
+  // Creates a new Lowbox object. Need to followup with a call to build lowbox
+  // token
+  static AppContainerBase* CreateLowbox(const wchar_t* sid);
+
   // Delete a profile based on name. Returns true if successful, or if the
   // package doesn't already exist.
   static bool Delete(const wchar_t* package_name);
 
+  // Build the token for the lowbox
+  ResultCode BuildLowBoxToken(base::win::ScopedHandle* token,
+                              base::win::ScopedHandle* lockdown = nullptr);
+
  private:
-  AppContainerBase(const Sid& package_sid);
+  AppContainerBase(const Sid& package_sid, AppContainerType type);
   ~AppContainerBase();
 
-  bool BuildLowBoxToken(base::win::ScopedHandle* token);
   bool AddCapability(const Sid& capability_sid, bool impersonation_only);
 
   // Standard object-lifetime reference counter.
@@ -85,6 +94,8 @@ class AppContainerBase final : public AppContainer {
   bool enable_low_privilege_app_container_;
   std::vector<Sid> capabilities_;
   std::vector<Sid> impersonation_capabilities_;
+  AppContainerType type_;
+  base::win::ScopedHandle lowbox_directory_;
 
   DISALLOW_COPY_AND_ASSIGN(AppContainerBase);
 };

sandbox/win/src/sandbox_policy_base.cc

@@ -108,7 +108,6 @@ PolicyBase::PolicyBase()
       is_csrss_connected_(true),
       policy_maker_(nullptr),
       policy_(nullptr),
-      lowbox_sid_(nullptr),
       lockdown_default_dacl_(false),
       add_restricting_random_sid_(false),
       effective_token_(nullptr) {
@@ -120,9 +119,6 @@ PolicyBase::~PolicyBase() {
   delete policy_maker_;
   delete policy_;
 
-  if (lowbox_sid_)
-    ::LocalFree(lowbox_sid_);
-
   ::DeleteCriticalSection(&lock_);
 }
 
@@ -295,10 +291,11 @@ ResultCode PolicyBase::SetLowBox(const wchar_t* sid) {
     return SBOX_ERROR_UNSUPPORTED;
 
   DCHECK(sid);
-  if (lowbox_sid_ || app_container_)
+  if (app_container_)
     return SBOX_ERROR_BAD_PARAMS;
 
-  if (!ConvertStringSidToSid(sid, &lowbox_sid_))
+  app_container_ = AppContainerBase::CreateLowbox(sid);
+  if (!app_container_)
     return SBOX_ERROR_INVALID_LOWBOX_SID;
 
   return SBOX_ALL_OK;
@@ -474,29 +471,12 @@ ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial,
     }
   }
 
-  if (lowbox_sid_) {
-    if (!lowbox_directory_.IsValid()) {
-      result =
-          CreateLowBoxObjectDirectory(lowbox_sid_, true, &lowbox_directory_);
-      DCHECK(result == ERROR_SUCCESS);
-    }
-
-    // The order of handles isn't important in the CreateLowBoxToken call.
-    // The kernel will maintain a reference to the object directory handle.
-    HANDLE saved_handles[1] = {lowbox_directory_.Get()};
-    DWORD saved_handles_count = lowbox_directory_.IsValid() ? 1 : 0;
-
-    Sid package_sid(lowbox_sid_);
-    SecurityCapabilities caps(package_sid);
-    if (CreateLowBoxToken(lockdown->Get(), PRIMARY, &caps, saved_handles,
-                          saved_handles_count, lowbox) != ERROR_SUCCESS) {
-      return SBOX_ERROR_CANNOT_CREATE_LOWBOX_TOKEN;
-    }
+  if (app_container_ &&
+      app_container_->GetAppContainerType() == AppContainerType::kLowbox) {
+    ResultCode result_code = app_container_->BuildLowBoxToken(lowbox, lockdown);
 
-    if (!ReplacePackageSidInDacl(lowbox->Get(), SE_KERNEL_OBJECT, package_sid,
-                                 TOKEN_ALL_ACCESS)) {
-      return SBOX_ERROR_CANNOT_MODIFY_LOWBOX_TOKEN_DACL;
-    }
+    if (result_code != SBOX_ALL_OK)
+      return result_code;
   }
 
   // Create the 'better' token. We use this token as the one that the main
@@ -511,10 +491,6 @@ ResultCode PolicyBase::MakeTokens(base::win::ScopedHandle* initial,
   return SBOX_ALL_OK;
 }
 
-PSID PolicyBase::GetLowBoxSid() const {
-  return lowbox_sid_;
-}
-
 ResultCode PolicyBase::AddTarget(std::unique_ptr<TargetProcess> target) {
   if (policy_) {
     if (!policy_maker_->Done())
@@ -641,8 +617,7 @@ ResultCode PolicyBase::AddAppContainerProfile(const wchar_t* package_name,
     return SBOX_ERROR_UNSUPPORTED;
 
   DCHECK(package_name);
-  if (lowbox_sid_ || app_container_ ||
-      integrity_level_ != INTEGRITY_LEVEL_LAST) {
+  if (app_container_ || integrity_level_ != INTEGRITY_LEVEL_LAST) {
     return SBOX_ERROR_BAD_PARAMS;
   }
 

sandbox/win/src/sandbox_policy_base.h

@@ -173,8 +173,6 @@ class PolicyBase final : public TargetPolicy {
   // target process. A null set means we need to close all handles of the
   // given type.
   HandleCloser handle_closer_;
-  PSID lowbox_sid_;
-  base::win::ScopedHandle lowbox_directory_;
   std::unique_ptr<Dispatcher> dispatcher_;
   bool lockdown_default_dacl_;
   bool add_restricting_random_sid_;

sandbox/win/src/sandbox_policy_diagnostic.cc

@@ -407,10 +407,9 @@ PolicyDiagnostic::PolicyDiagnostic(PolicyBase* policy) {
          policy->app_container_->GetImpersonationCapabilities()) {
       initial_capabilities_.push_back(sid);
     }
-  }
 
-  if (policy->lowbox_sid_)
-    lowbox_sid_ = std::make_unique<Sid>(policy->lowbox_sid_);
+    app_container_type_ = policy->app_container_->GetAppContainerType();
+  }
 
   if (policy->policy_) {
     size_t policy_mem_size = policy->policy_->data_size + sizeof(PolicyGlobal);
@@ -475,11 +474,11 @@ const char* PolicyDiagnostic::JsonString() {
       value.SetKey(kAppContainerInitialCapabilities,
                    base::Value(std::move(imp_caps)));
     }
-  }
 
-  if (lowbox_sid_) {
-    value.SetStringKey(
-        kLowboxSid, base::AsStringPiece16(GetSidAsString(lowbox_sid_.get())));
+    if (app_container_type_ == AppContainerType::kLowbox)
+      value.SetStringKey(
+          kLowboxSid,
+          base::AsStringPiece16(GetSidAsString(app_container_sid_.get())));
   }
 
   if (policy_rules_)

sandbox/win/src/sandbox_policy_diagnostic.h

@@ -13,6 +13,7 @@
 
 #include "base/macros.h"
 #include "base/values.h"
+#include "sandbox/win/src/app_container.h"
 #include "sandbox/win/src/handle_closer.h"
 #include "sandbox/win/src/policy_low_level.h"
 #include "sandbox/win/src/process_mitigations.h"
@@ -42,14 +43,12 @@ class PolicyDiagnostic final : public PolicyInfo {
   JobLevel job_level_ = JOB_NONE;
   IntegrityLevel desired_integrity_level_ = INTEGRITY_LEVEL_LAST;
   MitigationFlags desired_mitigations_ = 0;
-  // Cannot have both |lowbox_sid_| and |app_container_sid_|. May have neither.
   std::unique_ptr<Sid> app_container_sid_ = nullptr;
   // Only populated if |app_container_sid_| is present.
   std::vector<Sid> capabilities_;
   // Only populated if |app_container_sid_| is present.
   std::vector<Sid> initial_capabilities_;
-  // Cannot have both |lowbox_sid_| and |app_container_sid_|. May have neither.
-  std::unique_ptr<Sid> lowbox_sid_ = nullptr;
+  AppContainerType app_container_type_ = AppContainerType::kNone;
   std::unique_ptr<PolicyGlobal> policy_rules_ = nullptr;
   bool is_csrss_connected_ = false;
   HandleMap handles_to_close_;

sandbox/win/src/sandbox_types.h

@@ -141,6 +141,8 @@ enum ResultCode : int {
   SBOX_ERROR_CANNOT_INIT_BROKERSERVICES = 59,
   // Cannot update job active process limit.
   SBOX_ERROR_CANNOT_UPDATE_JOB_PROCESS_LIMIT = 60,
+  // Cannot create an impersonation lowbox token
+  SBOX_ERROR_CANNOT_CREATE_LOWBOX_IMPERSONATION_TOKEN = 61,
   // Placeholder for last item of the enum.
   SBOX_ERROR_LAST
 };

sandbox/win/src/startup_information_helper.cc

@@ -97,7 +97,8 @@ int StartupInformationHelper::CountAttributes() {
   if (!inherited_handle_list_.empty())
     ++attribute_count;
 
-  if (app_container_) {
+  if (app_container_ &&
+      app_container_->GetAppContainerType() != AppContainerType::kLowbox) {
     ++attribute_count;
     if (app_container_->GetEnableLowPrivilegeAppContainer())
       ++attribute_count;
@@ -171,7 +172,8 @@ bool StartupInformationHelper::BuildStartupInformation() {
     expected_attributes--;
   }
 
-  if (app_container_) {
+  if (app_container_ &&
+      app_container_->GetAppContainerType() != AppContainerType::kLowbox) {
     if (!startup_info_.UpdateProcThreadAttribute(
             PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES,
             security_capabilities_.get(), sizeof(SECURITY_CAPABILITIES))) {

tools/metrics/histograms/enums.xml

@@ -42830,6 +42830,7 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="58" label="SBOX_ERROR_INVALID_WRITE_VARIABLE_SIZE"/>
   <int value="59" label="SBOX_ERROR_CANNOT_INIT_BROKERSERVICES"/>
   <int value="60" label="SBOX_ERROR_CANNOT_UPDATE_JOB_PROCESS_LIMIT"/>
+  <int value="61" label="SBOX_ERROR_CANNOT_CREATE_LOWBOX_IMPERSONATION_TOKEN"/>
   <int value="1002" label="LAUNCH_RESULT_SUCCESS"/>
   <int value="1003" label="LAUNCH_RESULT_FAILURE"/>
 </enum>