[ros-diffs] [reactos] 20/21: [NTOS:SE] Enable support for principal and restricted SIDs

[George Bișoc]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f48191b4b5b2f5d5498ff…

commit f48191b4b5b2f5d5498ff77a651d062a59dc546b
Author:     George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Feb 5 22:21:14 2022 +0100
Commit:     George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri May 6 10:09:53 2022 +0200

    [NTOS:SE] Enable support for principal and restricted SIDs
    
    SepSidInTokenEx function already provides the necessary mechanism to handle scenario
where a token has restricted SIDs or a principal SID is given to the call. There's no
reason to have these redundant ASSERTs anymore.
    
    In addition to that make sure if the SID is not a restricted and if that SID is the
first element on the array and it's enabled, this is the primary user.
---
 ntoskrnl/se/access.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/ntoskrnl/se/access.c b/ntoskrnl/se/access.c
index 7e6eb23d136..98b00a70d1e 100644
--- a/ntoskrnl/se/access.c
+++ b/ntoskrnl/se/access.c
@@ -37,7 +37,7 @@ ERESOURCE SepSubjectContextLock;
  *
  * @param[in] Restricted
  * If set to TRUE, the caller expects that a SID in a token is
- * restricted.
+ * restricted (by the general definition, a token is restricted).
  *
  * @return
  * Returns TRUE if the specified SID in the call is present in the token,
@@ -52,7 +52,7 @@ SepSidInTokenEx(
     _In_ BOOLEAN Deny,
     _In_ BOOLEAN Restricted)
 {
-    ULONG i;
+    ULONG SidIndex;
     PTOKEN Token = (PTOKEN)_Token;
     PISID TokenSid, Sid = (PISID)_Sid;
     PSID_AND_ATTRIBUTES SidAndAttributes;
@@ -60,10 +60,6 @@ SepSidInTokenEx(
     USHORT SidMetadata;
     PAGED_CODE();
 
-    /* Not yet supported */
-    ASSERT(PrincipalSelfSid == NULL);
-    ASSERT(Restricted == FALSE);
-
     /* Check if a principal SID was given, and this is our current SID already */
     if ((PrincipalSelfSid) && (RtlEqualSid(SePrincipalSelfSid, Sid)))
     {
@@ -91,7 +87,7 @@ SepSidInTokenEx(
     SidMetadata = *(PUSHORT)&Sid->Revision;
 
     /* Loop every SID */
-    for (i = 0; i < SidCount; i++)
+    for (SidIndex = 0; SidIndex < SidCount; SidIndex++)
     {
         TokenSid = (PISID)SidAndAttributes->Sid;
 #if SE_SID_DEBUG
@@ -106,8 +102,15 @@ SepSidInTokenEx(
             /* Check if the SID data matches */
             if (RtlEqualMemory(Sid, TokenSid, SidLength))
             {
-                /* Check if the group is enabled, or used for deny only */
-                if ((!(i) && !(SidAndAttributes->Attributes &
SE_GROUP_USE_FOR_DENY_ONLY)) ||
+                /*
+                 * Check if the group is enabled, or used for deny only.
+                 * Otherwise we have to check if this is the first user.
+                 * We understand that by looking if this SID is not
+                 * restricted, this is the first element we are iterating
+                 * and that it doesn't have SE_GROUP_USE_FOR_DENY_ONLY
+                 * attribute.
+                 */
+                if ((!Restricted && (SidIndex == 0) &&
!(SidAndAttributes->Attributes & SE_GROUP_USE_FOR_DENY_ONLY)) ||
                     (SidAndAttributes->Attributes & SE_GROUP_ENABLED) ||
                     ((Deny) && (SidAndAttributes->Attributes &
SE_GROUP_USE_FOR_DENY_ONLY)))
                 {