https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f48191b4b5b2f5d5498ff…
commit f48191b4b5b2f5d5498ff77a651d062a59dc546b
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Feb 5 22:21:14 2022 +0100
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri May 6 10:09:53 2022 +0200
[NTOS:SE] Enable support for principal and restricted SIDs
SepSidInTokenEx function already provides the necessary mechanism to handle scenario where a token has restricted SIDs or a principal SID is given to the call. There's no reason to have these redundant ASSERTs anymore.
In addition to that make sure if the SID is not a restricted and if that SID is the first element on the array and it's enabled, this is the primary user.
---
ntoskrnl/se/access.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/ntoskrnl/se/access.c b/ntoskrnl/se/access.c
index 7e6eb23d136..98b00a70d1e 100644
--- a/ntoskrnl/se/access.c
+++ b/ntoskrnl/se/access.c
@@ -37,7 +37,7 @@ ERESOURCE SepSubjectContextLock;
*
* @param[in] Restricted
* If set to TRUE, the caller expects that a SID in a token is
- * restricted.
+ * restricted (by the general definition, a token is restricted).
*
* @return
* Returns TRUE if the specified SID in the call is present in the token,
@@ -52,7 +52,7 @@ SepSidInTokenEx(
_In_ BOOLEAN Deny,
_In_ BOOLEAN Restricted)
{
- ULONG i;
+ ULONG SidIndex;
PTOKEN Token = (PTOKEN)_Token;
PISID TokenSid, Sid = (PISID)_Sid;
PSID_AND_ATTRIBUTES SidAndAttributes;
@@ -60,10 +60,6 @@ SepSidInTokenEx(
USHORT SidMetadata;
PAGED_CODE();
- /* Not yet supported */
- ASSERT(PrincipalSelfSid == NULL);
- ASSERT(Restricted == FALSE);
-
/* Check if a principal SID was given, and this is our current SID already */
if ((PrincipalSelfSid) && (RtlEqualSid(SePrincipalSelfSid, Sid)))
{
@@ -91,7 +87,7 @@ SepSidInTokenEx(
SidMetadata = *(PUSHORT)&Sid->Revision;
/* Loop every SID */
- for (i = 0; i < SidCount; i++)
+ for (SidIndex = 0; SidIndex < SidCount; SidIndex++)
{
TokenSid = (PISID)SidAndAttributes->Sid;
#if SE_SID_DEBUG
@@ -106,8 +102,15 @@ SepSidInTokenEx(
/* Check if the SID data matches */
if (RtlEqualMemory(Sid, TokenSid, SidLength))
{
- /* Check if the group is enabled, or used for deny only */
- if ((!(i) && !(SidAndAttributes->Attributes & SE_GROUP_USE_FOR_DENY_ONLY)) ||
+ /*
+ * Check if the group is enabled, or used for deny only.
+ * Otherwise we have to check if this is the first user.
+ * We understand that by looking if this SID is not
+ * restricted, this is the first element we are iterating
+ * and that it doesn't have SE_GROUP_USE_FOR_DENY_ONLY
+ * attribute.
+ */
+ if ((!Restricted && (SidIndex == 0) && !(SidAndAttributes->Attributes & SE_GROUP_USE_FOR_DENY_ONLY)) ||
(SidAndAttributes->Attributes & SE_GROUP_ENABLED) ||
((Deny) && (SidAndAttributes->Attributes & SE_GROUP_USE_FOR_DENY_ONLY)))
{
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=bac67a65f26df384f5962…
commit bac67a65f26df384f5962e85f001f5984caa2b66
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Feb 5 22:01:39 2022 +0100
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri May 6 10:09:53 2022 +0200
[NTOS:SE] Implement SepGetSidFromAce
This function will be used to retrieve a security identifier from a valid access control entry in the kernel. Mostly and exclusively used within access checks related code and such.
---
ntoskrnl/se/sid.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/ntoskrnl/se/sid.c b/ntoskrnl/se/sid.c
index 509b3777488..13aeba2662b 100644
--- a/ntoskrnl/se/sid.c
+++ b/ntoskrnl/se/sid.c
@@ -412,6 +412,77 @@ SepReleaseSid(
}
}
+/**
+ * @brief
+ * Captures a security identifier from a
+ * given access control entry. This identifier
+ * is valid for the whole of its lifetime.
+ *
+ * @param[in] AceType
+ * The type of an access control entry. This
+ * type that is given by the calling thread
+ * must coincide with the actual ACE that is
+ * given in the second parameter otherwise this
+ * can potentially lead to UNDEFINED behavior!
+ *
+ * @param[in] Ace
+ * A pointer to an access control entry, which
+ * can be obtained from a DACL.
+ *
+ * @return
+ * Returns a pointer to a security identifier (SID),
+ * otherwise NULL is returned if an unsupported ACE
+ * type was given to the function.
+ */
+PSID
+NTAPI
+SepGetSidFromAce(
+ _In_ UCHAR AceType,
+ _In_ PACE Ace)
+{
+ PSID Sid;
+ PAGED_CODE();
+
+ /* Sanity check */
+ ASSERT(Ace);
+
+ /* Initialize the SID */
+ Sid = NULL;
+
+ /* Obtain the SID based upon ACE type */
+ switch (AceType)
+ {
+ case ACCESS_DENIED_ACE_TYPE:
+ {
+ Sid = (PSID)&((PACCESS_DENIED_ACE)Ace)->SidStart;
+ break;
+ }
+
+ case ACCESS_ALLOWED_ACE_TYPE:
+ {
+ Sid = (PSID)&((PACCESS_ALLOWED_ACE)Ace)->SidStart;
+ break;
+ }
+
+ case ACCESS_DENIED_OBJECT_ACE_TYPE:
+ {
+ Sid = (PSID)&((PACCESS_DENIED_OBJECT_ACE)Ace)->SidStart;
+ break;
+ }
+
+ case ACCESS_ALLOWED_OBJECT_ACE_TYPE:
+ {
+ Sid = (PSID)&((PACCESS_ALLOWED_OBJECT_ACE)Ace)->SidStart;
+ break;
+ }
+
+ default:
+ break;
+ }
+
+ return Sid;
+}
+
/**
* @brief
* Captures a SID with attributes.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f559f63063a64aba3f489…
commit f559f63063a64aba3f489aef210a9848a0850d48
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sun Feb 13 19:12:19 2022 +0100
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri May 6 10:09:52 2022 +0200
[SERVICES] Assign a World identity authority for Everyone SID, not Null authority
The current code allocates memory and initializes the Everyone "World" security identifier but with a Null authority identifier. This is utterly wrong on so many levels, more so partly because a Null authority identifier is 0 so after the Everyone SID is initialized, it is actually initialized as S-1-0-0 instead of S-1-1-0.
---
base/system/services/security.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/base/system/services/security.c b/base/system/services/security.c
index b2639e95a20..2e8f284a714 100644
--- a/base/system/services/security.c
+++ b/base/system/services/security.c
@@ -55,6 +55,7 @@ DWORD
ScmCreateSids(VOID)
{
SID_IDENTIFIER_AUTHORITY NullAuthority = {SECURITY_NULL_SID_AUTHORITY};
+ SID_IDENTIFIER_AUTHORITY WorldAuthority = {SECURITY_WORLD_SID_AUTHORITY};
SID_IDENTIFIER_AUTHORITY NtAuthority = {SECURITY_NT_AUTHORITY};
PULONG pSubAuthority;
ULONG ulLength1 = RtlLengthRequiredSid(1);
@@ -78,7 +79,7 @@ ScmCreateSids(VOID)
return ERROR_OUTOFMEMORY;
}
- RtlInitializeSid(pWorldSid, &NullAuthority, 1);
+ RtlInitializeSid(pWorldSid, &WorldAuthority, 1);
pSubAuthority = RtlSubAuthoritySid(pWorldSid, 0);
*pSubAuthority = SECURITY_WORLD_RID;
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f340524ea4d103221a7fd…
commit f340524ea4d103221a7fd2eaca51787ea02461c7
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Mon Feb 21 11:12:48 2022 +0100
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri May 6 10:09:51 2022 +0200
[SERVICES] Grant ReactOS Setup component SYSTEM access
ReactOS Setup is an integral component that is part of the operating system responsible for the installation of ROS during 2nd installation stage. The situation with current master branch is like this -- the Services component always tries to create the process
on behalf of the logged in user with its own security context. That user doesn't have the privileges and access rights like SYSTEM thus the Services component tries to create the process but it fails to do so because of lacking of required access right, TOKEN_DUPLICATE, in order for the calling thread to impersonate as self.
---
base/system/services/database.c | 2 +-
base/system/services/services.c | 24 ++++++++++++++++++++++++
base/system/services/services.h | 1 +
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/base/system/services/database.c b/base/system/services/database.c
index d773a44e102..8dbf148d43a 100644
--- a/base/system/services/database.c
+++ b/base/system/services/database.c
@@ -370,7 +370,7 @@ ScmLogonService(
DPRINT("ScmLogonService(%p %p)\n", pService, pImage);
DPRINT("Service %S\n", pService->lpServiceName);
- if (ScmIsLocalSystemAccount(pImage->pszAccountName) || ScmLiveSetup)
+ if (ScmIsLocalSystemAccount(pImage->pszAccountName) || ScmLiveSetup || ScmSetupInProgress)
return ERROR_SUCCESS;
/* Get the user and domain names */
diff --git a/base/system/services/services.c b/base/system/services/services.c
index b45ac07b745..fec54827b87 100644
--- a/base/system/services/services.c
+++ b/base/system/services/services.c
@@ -28,6 +28,7 @@ int WINAPI RegisterServicesProcess(DWORD ServicesProcessId);
BOOL ScmInitialize = FALSE;
BOOL ScmShutdown = FALSE;
BOOL ScmLiveSetup = FALSE;
+BOOL ScmSetupInProgress = FALSE;
static HANDLE hScmShutdownEvent = NULL;
static HANDLE hScmSecurityServicesEvent = NULL;
@@ -55,6 +56,7 @@ CheckForLiveCD(VOID)
WCHAR CommandLine[MAX_PATH];
HKEY hSetupKey;
DWORD dwSetupType;
+ DWORD dwSetupInProgress;
DWORD dwType;
DWORD dwSize;
DWORD dwError;
@@ -107,6 +109,28 @@ CheckForLiveCD(VOID)
ScmLiveSetup = TRUE;
}
+ /* Read the SystemSetupInProgress value */
+ dwSize = sizeof(DWORD);
+ dwError = RegQueryValueExW(hSetupKey,
+ L"SystemSetupInProgress",
+ NULL,
+ &dwType,
+ (LPBYTE)&dwSetupInProgress,
+ &dwSize);
+ if (dwError != ERROR_SUCCESS ||
+ dwType != REG_DWORD ||
+ dwSize != sizeof(DWORD) ||
+ dwSetupType == 0)
+ {
+ goto done;
+ }
+
+ if (dwSetupInProgress == 1)
+ {
+ DPRINT1("ReactOS Setup currently in progress!\n");
+ ScmSetupInProgress = TRUE;
+ }
+
done:
RegCloseKey(hSetupKey);
diff --git a/base/system/services/services.h b/base/system/services/services.h
index cbaa4a93329..204375c17c6 100644
--- a/base/system/services/services.h
+++ b/base/system/services/services.h
@@ -102,6 +102,7 @@ extern LIST_ENTRY ImageListHead;
extern BOOL ScmInitialize;
extern BOOL ScmShutdown;
extern BOOL ScmLiveSetup;
+extern BOOL ScmSetupInProgress;
extern PSECURITY_DESCRIPTOR pPipeSD;