[ros-diffs] [tkreuzer] 62058: [NTOSKRNL] - Lock the token in SepPrivilegeCheck - Make use of RtlEqualLuid instead of manual comparison of fields - Implement SepSinglePrivilegeCheck and SePrivilegePolicyCheck a...

Author: tkreuzer
Date: Sat Feb  8 20:01:09 2014
New Revision: 62058

URL: http://svn.reactos.org/svn/reactos?rev=62058&view=rev
Log:
[NTOSKRNL]
- Lock the token in SepPrivilegeCheck
- Make use of RtlEqualLuid instead of manual comparison of fields
- Implement SepSinglePrivilegeCheck and SePrivilegePolicyCheck and use the latter in
SepAccessCheck

Modified:
    trunk/reactos/ntoskrnl/include/internal/se.h
    trunk/reactos/ntoskrnl/se/accesschk.c
    trunk/reactos/ntoskrnl/se/priv.c

Modified: trunk/reactos/ntoskrnl/include/internal/se.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/se.h	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/se.h	[iso-8859-1] Sat Feb  8 20:01:09 2014
@@ -380,6 +380,16 @@
     KPROCESSOR_MODE PreviousMode
 );
 
+NTSTATUS
+NTAPI
+SePrivilegePolicyCheck(
+    _Inout_ PACCESS_MASK DesiredAccess,
+    _Inout_ PACCESS_MASK GrantedAccess,
+    _In_ PSECURITY_SUBJECT_CONTEXT SubjectContext,
+    _In_ PTOKEN Token,
+    _Out_opt_ PPRIVILEGE_SET *OutPrivilegeSet,
+    _In_ KPROCESSOR_MODE PreviousMode);
+
 BOOLEAN
 NTAPI
 SeCheckPrivilegedObject(

Modified: trunk/reactos/ntoskrnl/se/accesschk.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/accesschk.c?re…
==============================================================================
--- trunk/reactos/ntoskrnl/se/accesschk.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/se/accesschk.c	[iso-8859-1] Sat Feb  8 20:01:09 2014
@@ -31,7 +31,6 @@
                OUT PACCESS_MASK GrantedAccess,
                OUT PNTSTATUS AccessStatus)
 {
-    LUID_AND_ATTRIBUTES Privilege;
 #ifdef OLD_ACCESS_CHECK
     ACCESS_MASK CurrentAccess, AccessMask;
 #endif
@@ -72,43 +71,31 @@
     if (PreviouslyGrantedAccess)
         RtlMapGenericMask(&PreviouslyGrantedAccess, GenericMapping);
 
-#ifdef OLD_ACCESS_CHECK
-    CurrentAccess = PreviouslyGrantedAccess;
-#endif
     /* Initialize remaining access rights */
     RemainingAccess = DesiredAccess;
 
     Token = SubjectSecurityContext->ClientToken ?
-    SubjectSecurityContext->ClientToken : SubjectSecurityContext->PrimaryToken;
-
-    /* Check for system security access */
-    if (RemainingAccess & ACCESS_SYSTEM_SECURITY)
-    {
-        Privilege.Luid = SeSecurityPrivilege;
-        Privilege.Attributes = SE_PRIVILEGE_ENABLED;
-
-        /* Fail if we do not the SeSecurityPrivilege */
-        if (!SepPrivilegeCheck(Token,
-                               &Privilege,
-                               1,
-                               PRIVILEGE_SET_ALL_NECESSARY,
-                               AccessMode))
-        {
-            *AccessStatus = STATUS_PRIVILEGE_NOT_HELD;
-            return FALSE;
-        }
-
-        /* Adjust access rights */
-        RemainingAccess &= ~ACCESS_SYSTEM_SECURITY;
-        PreviouslyGrantedAccess |= ACCESS_SYSTEM_SECURITY;
-
-        /* Succeed if there are no more rights to grant */
-        if (RemainingAccess == 0)
-        {
-            *GrantedAccess = PreviouslyGrantedAccess;
-            *AccessStatus = STATUS_SUCCESS;
-            return TRUE;
-        }
+        SubjectSecurityContext->ClientToken :
SubjectSecurityContext->PrimaryToken;
+
+    /* Check for ACCESS_SYSTEM_SECURITY and WRITE_OWNER access */
+    Status = SePrivilegePolicyCheck(&RemainingAccess,
+                                    &PreviouslyGrantedAccess,
+                                    NULL,
+                                    Token,
+                                    NULL,
+                                    UserMode);
+    if (!NT_SUCCESS(Status))
+    {
+        *AccessStatus = Status;
+        return FALSE;
+    }
+
+    /* Succeed if there are no more rights to grant */
+    if (RemainingAccess == 0)
+    {
+        *GrantedAccess = PreviouslyGrantedAccess;
+        *AccessStatus = STATUS_SUCCESS;
+        return TRUE;
     }
 
     /* Get the DACL */
@@ -142,35 +129,6 @@
 #ifdef OLD_ACCESS_CHECK
     CurrentAccess = PreviouslyGrantedAccess;
 #endif
-
-    /* RULE 2: Check token for 'take ownership' privilege */
-    if (DesiredAccess & WRITE_OWNER)
-    {
-        Privilege.Luid = SeTakeOwnershipPrivilege;
-        Privilege.Attributes = SE_PRIVILEGE_ENABLED;
-
-        if (SepPrivilegeCheck(Token,
-                              &Privilege,
-                              1,
-                              PRIVILEGE_SET_ALL_NECESSARY,
-                              AccessMode))
-        {
-            /* Adjust access rights */
-            RemainingAccess &= ~WRITE_OWNER;
-            PreviouslyGrantedAccess |= WRITE_OWNER;
-#ifdef OLD_ACCESS_CHECK
-            CurrentAccess |= WRITE_OWNER;
-#endif
-
-            /* Succeed if there are no more rights to grant */
-            if (RemainingAccess == 0)
-            {
-                *GrantedAccess = PreviouslyGrantedAccess;
-                *AccessStatus = STATUS_SUCCESS;
-                return TRUE;
-            }
-        }
-    }
 
     /* Deny access if the DACL is empty */
     if (Dacl->AceCount == 0)

Modified: trunk/reactos/ntoskrnl/se/priv.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/priv.c?rev=620…
==============================================================================
--- trunk/reactos/ntoskrnl/se/priv.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/se/priv.c	[iso-8859-1] Sat Feb  8 20:01:09 2014
@@ -89,6 +89,9 @@
     /* Get the number of privileges that are required to match */
     Required = (PrivilegeControl & PRIVILEGE_SET_ALL_NECESSARY) ? PrivilegeCount :
1;
 
+    /* Acquire a shared token lock */
+    SepAcquireTokenLockShared(Token);
+
     /* Loop all requested privileges until we found the required ones */
     for (i = 0; i < PrivilegeCount; i++)
     {
@@ -96,8 +99,7 @@
         for (j = 0; j < Token->PrivilegeCount; j++)
         {
             /* Check if the LUIDs match */
-            if (Token->Privileges[j].Luid.LowPart == Privileges[i].Luid.LowPart
&&
-                Token->Privileges[j].Luid.HighPart == Privileges[i].Luid.HighPart)
+            if (RtlEqualLuid(&Token->Privileges[j].Luid,
&Privileges[i].Luid))
             {
                 DPRINT("Found privilege. Attributes: %lx\n",
                        Token->Privileges[j].Attributes);
@@ -112,6 +114,7 @@
                     if (Required == 0)
                     {
                         /* We're done! */
+                        SepReleaseTokenLock(Token);
                         return TRUE;
                     }
                 }
@@ -122,9 +125,131 @@
         }
     }
 
+    /* Release the token lock */
+    SepReleaseTokenLock(Token);
+
     /* When we reached this point, we did not find all privileges */
     NT_ASSERT(Required > 0);
     return FALSE;
+}
+
+NTSTATUS
+NTAPI
+SepSinglePrivilegeCheck(
+    LUID PrivilegeValue,
+    PTOKEN Token,
+    KPROCESSOR_MODE PreviousMode)
+{
+    LUID_AND_ATTRIBUTES Privilege;
+    PAGED_CODE();
+    ASSERT(!RtlEqualLuid(&PrivilegeValue, &SeTcbPrivilege));
+
+    Privilege.Luid = PrivilegeValue;
+    Privilege.Attributes = SE_PRIVILEGE_ENABLED;
+    return SepPrivilegeCheck(Token,
+                             &Privilege,
+                             1,
+                             PRIVILEGE_SET_ALL_NECESSARY,
+                             PreviousMode);
+}
+
+NTSTATUS
+NTAPI
+SePrivilegePolicyCheck(
+    _Inout_ PACCESS_MASK DesiredAccess,
+    _Inout_ PACCESS_MASK GrantedAccess,
+    _In_ PSECURITY_SUBJECT_CONTEXT SubjectContext,
+    _In_ PTOKEN Token,
+    _Out_opt_ PPRIVILEGE_SET *OutPrivilegeSet,
+    _In_ KPROCESSOR_MODE PreviousMode)
+{
+    SIZE_T PrivilegeSize;
+    PPRIVILEGE_SET PrivilegeSet;
+    ULONG PrivilegeCount = 0, Index = 0;
+    ACCESS_MASK AccessMask = 0;
+    PAGED_CODE();
+
+    /* Check if we have a security subject context */
+    if (SubjectContext != NULL)
+    {
+        /* Check if there is a client impersonation token */
+        if (SubjectContext->ClientToken != NULL)
+            Token = SubjectContext->ClientToken;
+        else
+            Token = SubjectContext->PrimaryToken;
+    }
+
+    /* Check if the caller wants ACCESS_SYSTEM_SECURITY access */
+    if (*DesiredAccess & ACCESS_SYSTEM_SECURITY)
+    {
+        /* Do the privilege check */
+        if (SepSinglePrivilegeCheck(SeSecurityPrivilege, Token, PreviousMode))
+        {
+            /* Remember this access flag */
+            AccessMask |= ACCESS_SYSTEM_SECURITY;
+            PrivilegeCount++;
+        }
+        else
+        {
+            return STATUS_PRIVILEGE_NOT_HELD;
+        }
+    }
+
+    /* Check if the caller wants WRITE_OWNER access */
+    if (*DesiredAccess & WRITE_OWNER)
+    {
+        /* Do the privilege check */
+        if (SepSinglePrivilegeCheck(SeTakeOwnershipPrivilege, Token, PreviousMode))
+        {
+            /* Remember this access flag */
+            AccessMask |= WRITE_OWNER;
+            PrivilegeCount++;
+        }
+    }
+
+    /* Update the access masks */
+    *GrantedAccess |= AccessMask;
+    *DesiredAccess &= ~AccessMask;
+
+    /* Does the caller want a privilege set? */
+    if (OutPrivilegeSet != NULL)
+    {
+        /* Do we have any privileges to report? */
+        if (PrivilegeCount > 0)
+        {
+            /* Calculate size and allocate the structure */
+            PrivilegeSize = FIELD_OFFSET(PRIVILEGE_SET, Privilege[PrivilegeCount]);
+            PrivilegeSet = ExAllocatePoolWithTag(PagedPool, PrivilegeSize,
'rPeS');
+            *OutPrivilegeSet = PrivilegeSet;
+            if (PrivilegeSet == NULL)
+            {
+                return STATUS_INSUFFICIENT_RESOURCES;
+            }
+
+            PrivilegeSet->PrivilegeCount = PrivilegeCount;
+            PrivilegeSet->Control = 0;
+
+            if (AccessMask & WRITE_OWNER)
+            {
+                PrivilegeSet->Privilege[Index].Luid = SeTakeOwnershipPrivilege;
+                PrivilegeSet->Privilege[Index].Attributes =
SE_PRIVILEGE_USED_FOR_ACCESS;
+                Index++;
+            }
+
+            if (AccessMask & ACCESS_SYSTEM_SECURITY)
+            {
+                PrivilegeSet->Privilege[Index].Luid = SeSecurityPrivilege;
+                PrivilegeSet->Privilege[Index].Attributes =
SE_PRIVILEGE_USED_FOR_ACCESS;
+            }
+        }
+        else
+        {
+            /* No privileges, no structure */
+            *OutPrivilegeSet = NULL;
+        }
+    }
+
+    return STATUS_SUCCESS;
 }
 
 NTSTATUS