https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d72025649b769061b5f9f…
commit d72025649b769061b5f9f38aa57742ef17ae7c9e
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sun Oct 1 15:02:48 2023 +0200
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Wed Oct 4 18:04:30 2023 +0200
[NTOS:SE] Mute the access denied DPRINTs
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.
---
ntoskrnl/se/accesschk.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/ntoskrnl/se/accesschk.c b/ntoskrnl/se/accesschk.c
index 858bfeef994..ad931da38fb 100644
--- a/ntoskrnl/se/accesschk.c
+++ b/ntoskrnl/se/accesschk.c
@@ -1174,7 +1174,7 @@ SepAccessCheckWorker(
RemainingAccess &= ~(MAXIMUM_ALLOWED | AccessCheckRights.GrantedAccessRights);
if (RemainingAccess != 0)
{
- DPRINT1("Failed to grant access rights. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", RemainingAccess, DesiredAccess);
+ DPRINT("Failed to grant access rights, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", RemainingAccess, DesiredAccess);
PreviouslyGrantedAccess = 0;
Status = STATUS_ACCESS_DENIED;
goto ReturnCommonStatus;
@@ -1188,7 +1188,7 @@ SepAccessCheckWorker(
}
else
{
- DPRINT1("Failed to grant access rights. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess);
+ DPRINT("Failed to grant access rights, access denied. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess);
Status = STATUS_ACCESS_DENIED;
}
@@ -1217,7 +1217,7 @@ SepAccessCheckWorker(
RemainingAccess &= ~(MAXIMUM_ALLOWED | GrantedRights);
if (RemainingAccess != 0)
{
- DPRINT1("Failed to grant access rights to the whole object hierarchy list. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n",
+ DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n",
RemainingAccess, DesiredAccess);
PreviouslyGrantedAccess = 0;
Status = STATUS_ACCESS_DENIED;
@@ -1232,7 +1232,7 @@ SepAccessCheckWorker(
}
else
{
- DPRINT1("Failed to grant access rights to the whole object hierarchy list. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n",
+ DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n",
DesiredAccess);
Status = STATUS_ACCESS_DENIED;
}
@@ -1262,7 +1262,7 @@ SepAccessCheckWorker(
RemainingAccess = (~GrantedRights & WantedRights);
if (RemainingAccess != 0)
{
- DPRINT1("Failed to grant access rights at specific object at index %lu. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n",
+ DPRINT("Failed to grant access rights at specific object at index %lu, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n",
ObjectTypeIndex, RemainingAccess, DesiredAccess);
AccessStatusList[ObjectTypeIndex] = STATUS_ACCESS_DENIED;
}
@@ -1274,7 +1274,7 @@ SepAccessCheckWorker(
else
{
/* No access is given */
- DPRINT1("Failed to grant access rights at specific object at index %lu. No access is given\n", ObjectTypeIndex);
+ DPRINT("Failed to grant access rights at specific object at index %lu. No access is given\n", ObjectTypeIndex);
AccessStatusList[ObjectTypeIndex] = STATUS_ACCESS_DENIED;
}
@@ -1324,7 +1324,7 @@ SepAccessCheckWorker(
/* Fail if some rights have not been granted */
if (AccessCheckRights.RemainingAccessRights != 0)
{
- DPRINT1("Failed to grant access rights. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess);
+ DPRINT("Failed to grant access rights, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess);
PreviouslyGrantedAccess = 0;
Status = STATUS_ACCESS_DENIED;
goto ReturnCommonStatus;
@@ -1349,7 +1349,7 @@ SepAccessCheckWorker(
if (!AccessIsGranted)
{
- DPRINT1("Failed to grant access rights to the whole object hierarchy list. DesiredAccess = 0x%08lx\n", DesiredAccess);
+ DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. DesiredAccess = 0x%08lx\n", DesiredAccess);
PreviouslyGrantedAccess = 0;
Status = STATUS_ACCESS_DENIED;
goto ReturnCommonStatus;
@@ -1381,7 +1381,7 @@ SepAccessCheckWorker(
/* Fail if some rights have not been granted */
if (AccessCheckRights.RemainingAccessRights != 0)
{
- DPRINT1("Failed to grant access rights. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess);
+ DPRINT("Failed to grant access rights, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess);
PreviouslyGrantedAccess = 0;
Status = STATUS_ACCESS_DENIED;
goto ReturnCommonStatus;
@@ -1410,7 +1410,7 @@ SepAccessCheckWorker(
if (!AccessIsGranted)
{
- DPRINT1("Failed to grant access rights to the whole object hierarchy list. DesiredAccess = 0x%08lx\n", DesiredAccess);
+ DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. DesiredAccess = 0x%08lx\n", DesiredAccess);
PreviouslyGrantedAccess = 0;
Status = STATUS_ACCESS_DENIED;
goto ReturnCommonStatus;
@@ -1424,7 +1424,7 @@ SepAccessCheckWorker(
/* Fail if no rights have been granted */
if (PreviouslyGrantedAccess == 0)
{
- DPRINT1("Failed to grant access rights. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess);
+ DPRINT("Failed to grant access rights, access denied. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess);
Status = STATUS_ACCESS_DENIED;
goto ReturnCommonStatus;
}
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=09bfd96f3bdb8a190ddf5…
commit 09bfd96f3bdb8a190ddf51100204be19cbbc0b8c
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sun Oct 1 14:01:22 2023 +0200
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Wed Oct 4 18:04:30 2023 +0200
[NTOS:SE] HACK: Temporarily add the Local group SID to the system token
Temporarily add the local group to the system token so that Virtualbox
GA services can properly set up network drives for shared folders.
What happens is that a security descriptor has a DACL with only one ACE
that grants access to Local SID (presumably coming from Vbox?)
but the client token is that of the service which is a SYSTEM token.
Perhaps we are not impersonating the right user or whatever else.
This is only a temporary placebo, until a proper solution is found.
CORE-18250
---
ntoskrnl/se/token.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c
index dae5a411dff..83335415669 100644
--- a/ntoskrnl/se/token.c
+++ b/ntoskrnl/se/token.c
@@ -1784,12 +1784,14 @@ SepCreateSystemProcessToken(VOID)
{
{SeAliasAdminsSid, OwnerAttributes},
{SeWorldSid, GroupAttributes},
- {SeAuthenticatedUsersSid, GroupAttributes}
+ {SeAuthenticatedUsersSid, GroupAttributes},
+ {SeLocalSid, SE_GROUP_ENABLED} // HACK: Temporarily add the local group. See CORE-18250.
};
GroupsLength = sizeof(SID_AND_ATTRIBUTES) +
SeLengthSid(Groups[0].Sid) +
SeLengthSid(Groups[1].Sid) +
- SeLengthSid(Groups[2].Sid);
+ SeLengthSid(Groups[2].Sid) +
+ SeLengthSid(Groups[3].Sid); // HACK
ASSERT(GroupsLength <= (sizeof(Groups) * sizeof(ULONG)));
/* Setup the privileges */
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4b4638dc5599d5061a01a…
commit 4b4638dc5599d5061a01a21ed04a1ce9b68e732d
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Sep 30 20:51:12 2023 +0200
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Wed Oct 4 18:04:29 2023 +0200
[NTOS:SE] HACK: Temporarily grant access to the client if empty generic mapping was passed
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
---
ntoskrnl/se/accesschk.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/ntoskrnl/se/accesschk.c b/ntoskrnl/se/accesschk.c
index eaf159f8ce1..858bfeef994 100644
--- a/ntoskrnl/se/accesschk.c
+++ b/ntoskrnl/se/accesschk.c
@@ -1058,6 +1058,34 @@ SepAccessCheckWorker(
goto ReturnCommonStatus;
}
+ /*
+ * HACK: Temporary hack that checks if the caller passed an empty
+ * generic mapping. In such cases we cannot mask out the remaining
+ * access rights without a proper mapping so the only option we
+ * can do is to check if the client is an administrator,
+ * since they are powerful users.
+ *
+ * See CORE-18576 for information.
+ */
+ if (GenericMapping->GenericRead == 0 &&
+ GenericMapping->GenericWrite == 0 &&
+ GenericMapping->GenericExecute == 0 &&
+ GenericMapping->GenericAll == 0)
+ {
+ if (SeTokenIsAdmin(Token))
+ {
+ /* Grant him access */
+ PreviouslyGrantedAccess |= RemainingAccess;
+ Status = STATUS_SUCCESS;
+ goto ReturnCommonStatus;
+ }
+
+ /* It's not an admin so bail out */
+ PreviouslyGrantedAccess = 0;
+ Status = STATUS_ACCESS_DENIED;
+ goto ReturnCommonStatus;
+ }
+
/* Get the DACL */
Status = RtlGetDaclSecurityDescriptor(SecurityDescriptor,
&Present,