Bug 1321724 - Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs. r=jimm, a=jcristau
authorBob Owen <bobowencode@gmail.com>
Tue, 07 Feb 2017 10:59:43 +0000
changeset 376091 7ce21ea51bc3b5be991b97254a15fd40a4aae13a
parent 376090 b14295dd3dc4c5ac02154280ae8bed1b40e264f4
child 376092 8ee7930272cb4e486f7f6b926ea9837a9d5ca0de
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, jcristau
bugs1321724
milestone53.0a2
Bug 1321724 - Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs. r=jimm, a=jcristau MozReview-Commit-ID: 9cx2R6kMUwa
security/sandbox/chromium/sandbox/win/src/restricted_token.cc
security/sandbox/chromium/sandbox/win/src/restricted_token.h
security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc
security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
--- a/security/sandbox/chromium/sandbox/win/src/restricted_token.cc
+++ b/security/sandbox/chromium/sandbox/win/src/restricted_token.cc
@@ -245,16 +245,49 @@ DWORD RestrictedToken::AddAllSidsForDeny
             reinterpret_cast<SID*>(token_groups->Groups[i].Sid));
       }
     }
   }
 
   return ERROR_SUCCESS;
 }
 
+DWORD RestrictedToken::AddDenyOnlySids(const std::vector<Sid>& deny_only_sids) {
+  DCHECK(init_);
+  if (!init_) {
+    return ERROR_NO_TOKEN;
+  }
+
+  DWORD error;
+  scoped_ptr<BYTE[]> buffer = GetTokenInfo(effective_token_, TokenGroups, &error);
+
+  if (!buffer) {
+    return error;
+  }
+
+  TOKEN_GROUPS* token_groups = reinterpret_cast<TOKEN_GROUPS*>(buffer.get());
+
+  // Build the list of the deny only group SIDs
+  for (unsigned int i = 0; i < token_groups->GroupCount ; ++i) {
+    if ((token_groups->Groups[i].Attributes & SE_GROUP_INTEGRITY) == 0 &&
+        (token_groups->Groups[i].Attributes & SE_GROUP_LOGON_ID) == 0) {
+      for (unsigned int j = 0; j < deny_only_sids.size(); ++j) {
+        if (::EqualSid(const_cast<SID*>(deny_only_sids[j].GetPSID()),
+                       token_groups->Groups[i].Sid)) {
+          sids_for_deny_only_.push_back(
+              reinterpret_cast<SID*>(token_groups->Groups[i].Sid));
+          break;
+        }
+      }
+    }
+  }
+
+  return ERROR_SUCCESS;
+}
+
 DWORD RestrictedToken::AddSidForDenyOnly(const Sid &sid) {
   DCHECK(init_);
   if (!init_)
     return ERROR_NO_TOKEN;
 
   sids_for_deny_only_.push_back(sid);
   return ERROR_SUCCESS;
 }
--- a/security/sandbox/chromium/sandbox/win/src/restricted_token.h
+++ b/security/sandbox/chromium/sandbox/win/src/restricted_token.h
@@ -83,16 +83,27 @@ class RestrictedToken {
   //    std::vector<Sid> sid_exceptions;
   //    sid_exceptions.push_back(ATL::Sids::Users().GetPSID());
   //    sid_exceptions.push_back(ATL::Sids::World().GetPSID());
   //    restricted_token.AddAllSidsForDenyOnly(&sid_exceptions);
   // Note: A Sid marked for Deny Only in a token cannot be used to grant
   // access to any resource. It can only be used to deny access.
   DWORD AddAllSidsForDenyOnly(std::vector<Sid> *exceptions);
 
+  // Lists all sids in the token and mark them as Deny Only if present in the
+  // deny_only_sids parameter.
+  //
+  // If the function succeeds, the return value is ERROR_SUCCESS. If the
+  // function fails, the return value is the win32 error code corresponding to
+  // the error.
+  //
+  // Note: A Sid marked for Deny Only in a token cannot be used to grant
+  // access to any resource. It can only be used to deny access.
+  DWORD AddDenyOnlySids(const std::vector<Sid>& deny_only_sids);
+
   // Adds a user or group SID for Deny Only in the restricted token.
   // Parameter: sid is the SID to add in the Deny Only list.
   // The return value is always ERROR_SUCCESS.
   //
   // Sample Usage:
   //    restricted_token.AddSidForDenyOnly(ATL::Sids::Admins().GetPSID());
   DWORD AddSidForDenyOnly(const Sid &sid);
 
--- a/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc
+++ b/security/sandbox/chromium/sandbox/win/src/restricted_token_utils.cc
@@ -22,16 +22,17 @@ DWORD CreateRestrictedToken(TokenLevel s
                             IntegrityLevel integrity_level,
                             TokenType token_type,
                             base::win::ScopedHandle* token) {
   RestrictedToken restricted_token;
   restricted_token.Init(NULL);  // Initialized with the current process token
 
   std::vector<base::string16> privilege_exceptions;
   std::vector<Sid> sid_exceptions;
+  std::vector<Sid> deny_only_sids;
 
   bool deny_sids = true;
   bool remove_privileges = true;
 
   switch (security_level) {
     case USER_UNPROTECTED: {
       deny_sids = false;
       remove_privileges = false;
@@ -43,20 +44,26 @@ DWORD CreateRestrictedToken(TokenLevel s
 
       unsigned err_code = restricted_token.AddRestrictingSidAllSids();
       if (ERROR_SUCCESS != err_code)
         return err_code;
 
       break;
     }
     case USER_NON_ADMIN: {
-      sid_exceptions.push_back(WinBuiltinUsersSid);
-      sid_exceptions.push_back(WinWorldSid);
-      sid_exceptions.push_back(WinInteractiveSid);
-      sid_exceptions.push_back(WinAuthenticatedUserSid);
+      deny_sids = false;
+      deny_only_sids.push_back(WinBuiltinAdministratorsSid);
+      deny_only_sids.push_back(WinAccountAdministratorSid);
+      deny_only_sids.push_back(WinAccountDomainAdminsSid);
+      deny_only_sids.push_back(WinAccountCertAdminsSid);
+      deny_only_sids.push_back(WinAccountSchemaAdminsSid);
+      deny_only_sids.push_back(WinAccountEnterpriseAdminsSid);
+      deny_only_sids.push_back(WinAccountPolicyAdminsSid);
+      deny_only_sids.push_back(WinBuiltinHyperVAdminsSid);
+      deny_only_sids.push_back(WinLocalAccountAndAdministratorSid);
       privilege_exceptions.push_back(SE_CHANGE_NOTIFY_NAME);
       break;
     }
     case USER_INTERACTIVE: {
       sid_exceptions.push_back(WinBuiltinUsersSid);
       sid_exceptions.push_back(WinWorldSid);
       sid_exceptions.push_back(WinInteractiveSid);
       sid_exceptions.push_back(WinAuthenticatedUserSid);
@@ -102,16 +109,21 @@ DWORD CreateRestrictedToken(TokenLevel s
     }
   }
 
   DWORD err_code = ERROR_SUCCESS;
   if (deny_sids) {
     err_code = restricted_token.AddAllSidsForDenyOnly(&sid_exceptions);
     if (ERROR_SUCCESS != err_code)
       return err_code;
+  } else if (!deny_only_sids.empty()) {
+    err_code = restricted_token.AddDenyOnlySids(deny_only_sids);
+    if (ERROR_SUCCESS != err_code) {
+      return err_code;
+    }
   }
 
   if (remove_privileges) {
     err_code = restricted_token.DeleteAllPrivileges(&privilege_exceptions);
     if (ERROR_SUCCESS != err_code)
       return err_code;
   }
 
--- a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
+++ b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
@@ -1,8 +1,9 @@
 Please add a link to the bugzilla bug and patch name that should be re-applied.
 Also, please update any existing links to their actual mozilla-central changeset.
 
-https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part5.patch
+https://hg.mozilla.org/mozilla-central/rev/a05726163a79
 https://hg.mozilla.org/mozilla-central/rev/7df8d6639971
-https://bugzilla.mozilla.org/show_bug.cgi?id=1287426 bug1287426part7.patch
-https://bugzilla.mozilla.org/show_bug.cgi?id=1273372 bug1273372part2.patch
-https://bugzilla.mozilla.org/show_bug.cgi?id=1273372 bug1273372part3.patch
+https://hg.mozilla.org/mozilla-central/rev/e834e810a3fa
+https://hg.mozilla.org/mozilla-central/rev/c70d06fa5302
+https://hg.mozilla.org/mozilla-central/rev/d24db55deb85
+https://bugzilla.mozilla.org/show_bug.cgi?id=1321724 bug1321724.patch