Bug 1321724: Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs. r=jimm
authorBob Owen <bobowencode@gmail.com>
Tue, 07 Feb 2017 10:59:43 +0000
changeset 341072 0e6bf137521eb503b9a35b967cffcd1461a91997
parent 341071 c7e9ab08a73af3047e370002f41f10119fdaefc1
child 341073 9fc1fccf53d3ba003829a46be3e3396022d81d92
push id86631
push userbobowencode@gmail.com
push dateTue, 07 Feb 2017 11:00:00 +0000
treeherdermozilla-inbound@0e6bf137521e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1321724
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1321724: Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs. r=jimm 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