[sginsberg] 38510: - Cleanup AccessCheck, and set the correct last error in the case where the check succeeds but access is denied - Cleanup NtAccessCheck, properly set desired access when previous mode is kernel, remove a duplicate check that is performed in SeAccessCheck, and don't fail with STATUS_ACCESS_DENIED when the check succeeds but denies access -- the result of the access check is returned in the 'AccessStatus' parameter

Author: sginsberg
Date: Fri Jan  2 11:39:45 2009
New Revision: 38510

URL: http://svn.reactos.org/svn/reactos?rev=38510&view=rev
Log:
- Cleanup AccessCheck, and set the correct last error in the case where the check succeeds but access is denied
- Cleanup NtAccessCheck, properly set desired access when previous mode is kernel, remove a duplicate check that is performed in SeAccessCheck, and don't fail with STATUS_ACCESS_DENIED when the check succeeds but denies access -- the result of the access check is returned in the 'AccessStatus' parameter

Modified:
    trunk/reactos/dll/win32/advapi32/token/token.c
    trunk/reactos/ntoskrnl/se/semgr.c

Modified: trunk/reactos/dll/win32/advapi32/token/token.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/token/to...
==============================================================================
--- trunk/reactos/dll/win32/advapi32/token/token.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/advapi32/token/token.c [iso-8859-1] Fri Jan  2 11:39:45 2009
@@ -135,19 +135,21 @@
 /*
  * @implemented
  */
-BOOL WINAPI
-AccessCheck(PSECURITY_DESCRIPTOR pSecurityDescriptor,
-            HANDLE ClientToken,
-            DWORD DesiredAccess,
-            PGENERIC_MAPPING GenericMapping,
-            PPRIVILEGE_SET PrivilegeSet,
-            LPDWORD PrivilegeSetLength,
-            LPDWORD GrantedAccess,
-            LPBOOL AccessStatus)
-{
-    NTSTATUS Status;
-    NTSTATUS AccessStat;
-
+BOOL
+WINAPI
+AccessCheck(IN PSECURITY_DESCRIPTOR pSecurityDescriptor,
+            IN HANDLE ClientToken,
+            IN DWORD DesiredAccess,
+            IN PGENERIC_MAPPING GenericMapping,
+            OUT PPRIVILEGE_SET PrivilegeSet OPTIONAL,
+            IN OUT LPDWORD PrivilegeSetLength,
+            OUT LPDWORD GrantedAccess,
+            OUT LPBOOL AccessStatus)
+{
+    NTSTATUS Status;
+    NTSTATUS NtAccessStatus;
+
+    /* Do the access check */
     Status = NtAccessCheck(pSecurityDescriptor,
                            ClientToken,
                            DesiredAccess,
@@ -155,22 +157,30 @@
                            PrivilegeSet,
                            (PULONG)PrivilegeSetLength,
                            (PACCESS_MASK)GrantedAccess,
-                           &AccessStat);
-    if (!NT_SUCCESS(Status))
-    {
-        SetLastError(RtlNtStatusToDosError(Status));
-        return FALSE;
-    }
-
-    if (!NT_SUCCESS(AccessStat))
-    {
-        SetLastError(RtlNtStatusToDosError(Status));
+                           &NtAccessStatus);
+
+    /* See if the access check operation succeeded */
+    if (!NT_SUCCESS(Status))
+    {
+        /* Check failed */
+        SetLastError(RtlNtStatusToDosError(Status));
+        return FALSE;
+    }
+
+    /* Now check the access status  */
+    if (!NT_SUCCESS(NtAccessStatus))
+    {
+        /* Access denied */
+        SetLastError(RtlNtStatusToDosError(NtAccessStatus));
         *AccessStatus = FALSE;
-        return TRUE;
-    }
-
-    *AccessStatus = TRUE;
-
+    }
+    else
+    {
+        /* Access granted */
+        *AccessStatus = TRUE;
+    }
+
+    /* Check succeeded */
     return TRUE;
 }

Modified: trunk/reactos/ntoskrnl/se/semgr.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/se/semgr.c?rev=385...
==============================================================================
--- trunk/reactos/ntoskrnl/se/semgr.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/se/semgr.c [iso-8859-1] Fri Jan  2 11:39:45 2009
@@ -619,33 +619,48 @@

/* SYSTEM CALLS ***************************************************************/

-NTSTATUS NTAPI
+/*
+ * @implemented
+ */
+NTSTATUS
+NTAPI
 NtAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
               IN HANDLE TokenHandle,
               IN ACCESS_MASK DesiredAccess,
               IN PGENERIC_MAPPING GenericMapping,
-              OUT PPRIVILEGE_SET PrivilegeSet,
-              OUT PULONG ReturnLength,
+              OUT PPRIVILEGE_SET PrivilegeSet OPTIONAL,
+              IN OUT PULONG PrivilegeSetLength,
               OUT PACCESS_MASK GrantedAccess,
               OUT PNTSTATUS AccessStatus)
 {
-    SECURITY_SUBJECT_CONTEXT SubjectSecurityContext = { NULL, 0, NULL, NULL };
-    KPROCESSOR_MODE PreviousMode;
+    SECURITY_SUBJECT_CONTEXT SubjectSecurityContext;
+    KPROCESSOR_MODE PreviousMode = ExGetPreviousMode();
     PTOKEN Token;
     NTSTATUS Status;
-    
     PAGED_CODE();

-    DPRINT("NtAccessCheck() called\n");
-    
-    PreviousMode = KeGetPreviousMode();
+    /* Check if this is kernel mode */
     if (PreviousMode == KernelMode)
     {
-        *GrantedAccess = DesiredAccess;
+        /* Check if kernel wants everything */
+        if (DesiredAccess & MAXIMUM_ALLOWED)
+        {
+            /* Give it */
+            *GrantedAccess = GenericMapping->GenericAll;
+            *GrantedAccess |= (DesiredAccess &~ MAXIMUM_ALLOWED);
+        }
+        else
+        {
+            /* Just give the desired access */
+            *GrantedAccess = DesiredAccess;
+        }
+        
+        /* Success */
         *AccessStatus = STATUS_SUCCESS;
         return STATUS_SUCCESS;
     }
-    
+
+    /* Reference the token */
     Status = ObReferenceObjectByHandle(TokenHandle,
                                        TOKEN_QUERY,
                                        SepTokenObjectType,
@@ -657,56 +672,42 @@
         DPRINT1("Failed to reference token (Status %lx)\n", Status);
         return Status;
     }
-    
+
     /* Check token type */
     if (Token->TokenType != TokenImpersonation)
     {
         DPRINT1("No impersonation token\n");
         ObDereferenceObject(Token);
-        return STATUS_ACCESS_VIOLATION;
-    }
-    
-    /* Check impersonation level */
-    if (Token->ImpersonationLevel < SecurityIdentification)
-    {
-        DPRINT1("Invalid impersonation level\n");
-        ObDereferenceObject(Token);
-        return STATUS_ACCESS_VIOLATION;
-    }
-    
+        return STATUS_ACCESS_DENIED;
+    }
+
+    /* Set up the subject context, and lock it */
     SubjectSecurityContext.ClientToken = Token;
     SubjectSecurityContext.ImpersonationLevel = Token->ImpersonationLevel;
-    
-    /* Lock subject context */
+    SubjectSecurityContext.PrimaryToken = NULL;
+    SubjectSecurityContext.ProcessAuditId = NULL;
     SeLockSubjectContext(&SubjectSecurityContext);
-    
-    if (SeAccessCheck(SecurityDescriptor,
-                      &SubjectSecurityContext,
-                      TRUE,
-                      DesiredAccess,
-                      0,
-                      &PrivilegeSet,
-                      GenericMapping,
-                      PreviousMode,
-                      GrantedAccess,
-                      AccessStatus))
-    {
-        Status = *AccessStatus;
-    }
-    else
-    {
-        Status = STATUS_ACCESS_DENIED;
-    }
-    
-    /* Unlock subject context */
+
+    /* Now perform the access check */
+    SeAccessCheck(SecurityDescriptor,
+                  &SubjectSecurityContext,
+                  TRUE,
+                  DesiredAccess,
+                  0,
+                  &PrivilegeSet, //FIXME
+                  GenericMapping,
+                  PreviousMode,
+                  GrantedAccess,
+                  AccessStatus);
+
+    /* Unlock subject context and dereference the token */
     SeUnlockSubjectContext(&SubjectSecurityContext);
-    
     ObDereferenceObject(Token);
-    
-    DPRINT("NtAccessCheck() done\n");
-    
-    return Status;
-}
+
+    /* Check succeeded */
+    return STATUS_SUCCESS;
+}
+

NTSTATUS
 NTAPI