They can be spammy. Also clarify these debug prints, because some people
think that "failed to grant access rights" means there's something wrong
in the core access check functions.
Certain apps such as AIM installer passes an empty generic mapping (this can
be understood with their generic masks set to 0) and our code tries to map
the access right from an ACE with the mapping provided by AccessCheck.
This can lead to a bug where we would not be able to decode the generic right
from an ACE as we need a proper generic mapping in order to do so. A mask
right that is not decoded it cannot be used to mask out the remaining rights,
further resulting into a denied access right.
What Windows does instead is they are mapping the ACE's rights in another place,
presumably when setting security data to an object, and they are using the
generic mapping passed by the kernel.
What we can do for the time being is to temporarily grant access to the client,
but only if they are an administrator.
CORE-18576
- Implement SepDenyAccessObjectTypeResultList, SepAllowAccessObjectTypeResultList,
SepDenyAccessObjectTypeList and SepAllowAccessObjectTypeList. These routines will
be used to grant or deny access to sub-objects of an object in the list.
- Refactor SepAnalyzeAcesFromDacl and SepAccessCheck to accomodate the newly
implemented access check by type mechanism.
- SepAccessCheck will now be SepAccessCheckWorker, a worker helper function that further
abstracts the access check mechanism in the kernel. Whereas the SepAccessCheck name will be
used as a centralized function used by the access check NT system calls.
- Deprecate SepGetSDOwner and SepGetSDGroup in favor of SepGetOwnerFromDescriptor and
SepGetGroupFromDescriptor. The former functions were buggy as they might potentially
return garbage data if either the owner or group were passed as NULL to a security
descriptor, hence a second chance exception fault. This was caught when writing tests
for NtAccessCheckByType.
- Shorten the debug prints by removing the name of the functions, the person who reads
the debugger output has to look at the source code anyway.
Access check is an expensive operation, that is, whenever an access to an
object is performed an access check has to be done to ensure the access
can be allowed to the calling thread who attempts to access such object.
Currently SepAnalyzeAcesFromDacl allocates a block of pool memory for
access check rights, nagging the Memory Manager like a desperate naughty
creep. So instead initialize the access rights as a simple variable in
SepAccessCheck and pass it out as an address to SepAnalyzeAcesFromDacl so
that the function will fill it up with access rights. This helps with
performance, avoiding wasting a few bits of memory just to hold these
access rights.
In addition to that, add a few asserts and fix the copyright header on
both se.h and accesschk.c, to reflect the Coding Style rules.
The "failed to grant access rights" message isn't enough to understand what kind of access rights haven't been granted and why. Dumping information of the captured security descriptor, the ACL and its ACEs with mask rights and token SIDs should be enough to understand the reason of the failure in question.
There are two fundamental problems when it comes to access checks in ReactOS. First, the internal function SepAccessCheck which is the heart and brain of the whole access checks logic of the kernel warrants access to the calling thread of a process to an object even though access could not be given.
This can potentially leave security issues as we literally leave objects to be touched indiscriminately by anyone regardless of their ACEs in the DACL of a security descriptor. Second, the current access check code doesn't take into account the fact that an access token can have restricted SIDs. In such scenario we must perform additional access checks by iterating over the restricted SIDs of the primary token by comparing the SID equality and see if the group can be granted certain rights based on the ACE policy that represents the same SID.
Part of SepAccessCheck's code logic will be split for a separate private kernel routine, SepAnalyzeAcesFromDacl. The reasons for this are primarily two -- such code is subject to grow eventually as we'll support different type ACEs and handle them accordingly -- and we avoid further code duplicates. On Windows Server 2003 there are 5 different type of ACEs that are supported for access checks:
- ACCESS_DENIED_ACE_TYPE (supported by ReactOS)
- ACCESS_ALLOWED_ACE_TYPE (supported by ReactOS)
- ACCESS_DENIED_OBJECT_ACE_TYPE
- ACCESS_ALLOWED_OBJECT_ACE_TYPE
- ACCESS_ALLOWED_COMPOUND_ACE_TYPE
This gives the opportunity for us to have a semi serious kernel where security of objects are are taken into account, rather than giving access to everyone.
CORE-9174
CORE-9175
CORE-9184
CORE-14520
When performing access security check, use the security descriptor that we've captured it to determine based on that descriptor if the client can be granted access or not.
