From fbd7681b844c91c7c48e503fce6b076cc8243d34 Mon Sep 17 00:00:00 2001 From: Stefan Ginsberg Date: Fri, 2 Jan 2009 17:39:45 +0000 Subject: [PATCH] - 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 svn path=/trunk/; revision=38510 --- reactos/dll/win32/advapi32/token/token.c | 42 ++++++---- reactos/ntoskrnl/se/semgr.c | 99 ++++++++++++------------ 2 files changed, 76 insertions(+), 65 deletions(-) diff --git a/reactos/dll/win32/advapi32/token/token.c b/reactos/dll/win32/advapi32/token/token.c index 4e053787c20..9cc8b575611 100644 --- a/reactos/dll/win32/advapi32/token/token.c +++ b/reactos/dll/win32/advapi32/token/token.c @@ -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; } diff --git a/reactos/ntoskrnl/se/semgr.c b/reactos/ntoskrnl/se/semgr.c index 99f978e7625..e20c219dd0d 100644 --- a/reactos/ntoskrnl/se/semgr.c +++ b/reactos/ntoskrnl/se/semgr.c @@ -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; + return STATUS_ACCESS_DENIED; } - - /* Check impersonation level */ - if (Token->ImpersonationLevel < SecurityIdentification) - { - DPRINT1("Invalid impersonation level\n"); - ObDereferenceObject(Token); - return STATUS_ACCESS_VIOLATION; - } - + + /* 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,