- Cleanup AccessCheck, and set the correct last error in the case where the check...
authorStefan Ginsberg <stefanginsberg@gmail.com>
Fri, 2 Jan 2009 17:39:45 +0000 (17:39 +0000)
committerStefan Ginsberg <stefanginsberg@gmail.com>
Fri, 2 Jan 2009 17:39:45 +0000 (17:39 +0000)
- 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

svn path=/trunk/; revision=38510

reactos/dll/win32/advapi32/token/token.c
reactos/ntoskrnl/se/semgr.c

index 4e05378..9cc8b57 100644 (file)
@@ -135,19 +135,21 @@ SetTokenInformation(HANDLE TokenHandle,
 /*
  * @implemented
  */
-BOOL WINAPI
-AccessCheck(PSECURITY_DESCRIPTOR pSecurityDescriptor,
-            HANDLE ClientToken,
-            DWORD DesiredAccess,
-            PGENERIC_MAPPING GenericMapping,
-            PPRIVILEGE_SET PrivilegeSet,
-            LPDWORD PrivilegeSetLength,
-            LPDWORD GrantedAccess,
-            LPBOOL AccessStatus)
+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 AccessStat;
+    NTSTATUS NtAccessStatus;
 
+    /* Do the access check */
     Status = NtAccessCheck(pSecurityDescriptor,
                            ClientToken,
                            DesiredAccess,
@@ -155,22 +157,30 @@ AccessCheck(PSECURITY_DESCRIPTOR pSecurityDescriptor,
                            PrivilegeSet,
                            (PULONG)PrivilegeSetLength,
                            (PACCESS_MASK)GrantedAccess,
-                           &AccessStat);
+                           &NtAccessStatus);
+
+    /* See if the access check operation succeeded */
     if (!NT_SUCCESS(Status))
     {
+        /* Check failed */
         SetLastError(RtlNtStatusToDosError(Status));
         return FALSE;
     }
 
-    if (!NT_SUCCESS(AccessStat))
+    /* Now check the access status  */
+    if (!NT_SUCCESS(NtAccessStatus))
     {
-        SetLastError(RtlNtStatusToDosError(Status));
+        /* Access denied */
+        SetLastError(RtlNtStatusToDosError(NtAccessStatus));
         *AccessStatus = FALSE;
-        return TRUE;
+    }
+    else
+    {
+        /* Access granted */
+        *AccessStatus = TRUE;
     }
 
-    *AccessStatus = TRUE;
-
+    /* Check succeeded */
     return TRUE;
 }
 
index 99f978e..e20c219 100644 (file)
@@ -619,33 +619,48 @@ SeAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
 
 /* 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,57 +672,43 @@ NtAccessCheck(IN PSECURITY_DESCRIPTOR SecurityDescriptor,
         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
 NtAccessCheckByType(IN PSECURITY_DESCRIPTOR SecurityDescriptor,