[PATCH v5 0/2] MR414: fltmgr.sys: Implement FltBuildDefaultSecurityDescriptor

[Alistair Leslie-Hughes (＠alesliehughes)]

Signed-off-by: Alistair Leslie-Hughes

--
  v5: ntoskrnl.exe: Add FltBuildDefaultSecurityDescriptor test

https://gitlab.winehq.org/wine/wine/-/merge_requests/414

[PATCH v5 1/2] fltmgr.sys: Implement FltBuildDefaultSecurityDescriptor

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

Signed-off-by: Alistair Leslie-Hughes 
---
 dlls/fltmgr.sys/Makefile.in     |  1 +
 dlls/fltmgr.sys/fltmgr.sys.spec |  4 +-
 dlls/fltmgr.sys/main.c          | 71 +++++++++++++++++++++++++++++++++
 include/ddk/fltkernel.h         |  3 +-
 4 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/dlls/fltmgr.sys/Makefile.in b/dlls/fltmgr.sys/Makefile.in
index ba106a43831..bb1f34b4896 100644
--- a/dlls/fltmgr.sys/Makefile.in
+++ b/dlls/fltmgr.sys/Makefile.in
@@ -1,5 +1,6 @@
 MODULE    = fltmgr.sys
 EXTRADLLFLAGS = -Wl,--subsystem,native
+IMPORTS = ntoskrnl

C_SRCS = \
    main.c
diff --git a/dlls/fltmgr.sys/fltmgr.sys.spec b/dlls/fltmgr.sys/fltmgr.sys.spec
index 39ce6798178..8943b9f85cf 100644
--- a/dlls/fltmgr.sys/fltmgr.sys.spec
+++ b/dlls/fltmgr.sys/fltmgr.sys.spec
@@ -10,7 +10,7 @@
 @ stub FltAllocatePoolAlignedWithTag
 @ stub FltAttachVolume
 @ stub FltAttachVolumeAtAltitude
-@ stub FltBuildDefaultSecurityDescriptor
+@ stdcall FltBuildDefaultSecurityDescriptor(ptr long)
 @ stub FltCancelFileOpen
 @ stub FltCancelIo
 @ stub FltCbdqDisable
@@ -60,7 +60,7 @@
 @ stub FltFreeFileLock
 @ stub FltFreeGenericWorkItem
 @ stub FltFreePoolAlignedWithTag
-@ stub FltFreeSecurityDescriptor
+@ stdcall FltFreeSecurityDescriptor(ptr)
 @ stub FltFsControlFile
 @ stub FltGetBottomInstance
 @ stub FltGetContexts
diff --git a/dlls/fltmgr.sys/main.c b/dlls/fltmgr.sys/main.c
index e1016a4989c..ea9685b4308 100644
--- a/dlls/fltmgr.sys/main.c
+++ b/dlls/fltmgr.sys/main.c
@@ -93,3 +93,74 @@ void* WINAPI FltGetRoutineAddress(LPCSTR name)

return func;
 }
+
+NTSTATUS WINAPI FltBuildDefaultSecurityDescriptor(PSECURITY_DESCRIPTOR *descriptor, ACCESS_MASK access)
+{
+    PACL dacl;
+    NTSTATUS ret = STATUS_INSUFFICIENT_RESOURCES;
+    ULONG sid_len;
+    PSID sid;
+    PSID sid_system;
+    PSECURITY_DESCRIPTOR sec_desc = NULL;
+    SID_IDENTIFIER_AUTHORITY auth = { SECURITY_NULL_SID_AUTHORITY };
+
+    *descriptor = NULL;
+
+    ret = RtlAllocateAndInitializeSid(&auth, 2, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_GROUP_RID_ADMINS,
+                                         0, 0, 0, 0, 0, 0, &sid);
+    if (ret != STATUS_SUCCESS)
+        goto done;
+
+    ret = RtlAllocateAndInitializeSid(&auth, 1, SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0, &sid_system);
+    if (ret != STATUS_SUCCESS)
+        goto done;
+
+    sid_len = SECURITY_DESCRIPTOR_MIN_LENGTH + sizeof(ACL) +
+            sizeof(ACCESS_ALLOWED_ACE) + RtlLengthSid(sid) +
+            sizeof(ACCESS_ALLOWED_ACE) + RtlLengthSid(sid_system);
+
+    sec_desc = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, sid_len);
+    if (!sec_desc)
+    {
+        ret = STATUS_NO_MEMORY;
+        goto done;
+    }
+
+    ret = RtlCreateSecurityDescriptor(sec_desc, SECURITY_DESCRIPTOR_REVISION);
+    if (ret != STATUS_SUCCESS)
+        goto done;
+
+    dacl = (PACL)((char*)sec_desc + SECURITY_DESCRIPTOR_MIN_LENGTH);
+    ret = RtlCreateAcl(dacl, sid_len - SECURITY_DESCRIPTOR_MIN_LENGTH, ACL_REVISION);
+    if (ret != STATUS_SUCCESS)
+        goto done;
+
+    ret = RtlAddAccessAllowedAce(dacl, ACL_REVISION, access, sid);
+    if (ret != STATUS_SUCCESS)
+        goto done;
+
+    ret = RtlAddAccessAllowedAce(dacl, ACL_REVISION, access, sid_system);
+    if (ret != STATUS_SUCCESS)
+        goto done;
+
+    ret = RtlSetDaclSecurityDescriptor(sec_desc, 1, dacl, 0);
+    if (ret == STATUS_SUCCESS)
+        *descriptor = sec_desc;
+
+done:
+    if (ret != STATUS_SUCCESS && sec_desc != NULL)
+        RtlFreeHeap(GetProcessHeap(), 0, sec_desc);
+
+    if (sid != NULL)
+        RtlFreeHeap(GetProcessHeap(), 0, sid);
+
+    if (sid_system != NULL)
+        RtlFreeHeap(GetProcessHeap(), 0, sid_system);
+
+    return ret;
+}
+
+void WINAPI FltFreeSecurityDescriptor(PSECURITY_DESCRIPTOR descriptor)
+{
+    RtlFreeHeap(GetProcessHeap(), 0, descriptor);
+}
\ No newline at end of file
diff --git a/include/ddk/fltkernel.h b/include/ddk/fltkernel.h
index 8ebebfa2e81..9ece0990810 100644
--- a/include/ddk/fltkernel.h
+++ b/include/ddk/fltkernel.h
@@ -653,7 +653,8 @@ typedef struct _FLT_REGISTRATION
     PFLT_SECTION_CONFLICT_NOTIFICATION_CALLBACK SectionNotificationCallback;
 } FLT_REGISTRATION, *PFLT_REGISTRATION;

-
+NTSTATUS WINAPI FltBuildDefaultSecurityDescriptor(PSECURITY_DESCRIPTOR *, ACCESS_MASK);
+void     WINAPI FltFreeSecurityDescriptor(PSECURITY_DESCRIPTOR);
 void*    WINAPI FltGetRoutineAddress(LPCSTR name);
 NTSTATUS WINAPI FltRegisterFilter(PDRIVER_OBJECT, const FLT_REGISTRATION *, PFLT_FILTER *);
 NTSTATUS WINAPI FltStartFiltering(PFLT_FILTER);

[PATCH v5 2/2] ntoskrnl.exe: Add FltBuildDefaultSecurityDescriptor test

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

Signed-off-by: Alistair Leslie-Hughes 
---
 dlls/ntoskrnl.exe/tests/driver.c | 76 ++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c
index a80bef78fab..16cdc2e69ea 100644
--- a/dlls/ntoskrnl.exe/tests/driver.c
+++ b/dlls/ntoskrnl.exe/tests/driver.c
@@ -32,6 +32,7 @@
 #include "ddk/ntddk.h"
 #include "ddk/ntifs.h"
 #include "ddk/wdm.h"
+#include "ddk/fltkernel.h"

#include "driver.h"

@@ -2326,6 +2327,80 @@ static void test_driver_object_extension(void)
     ok(get_obj_ext == NULL, "got %p\n", get_obj_ext);
 }

+static void test_default_security(void)
+{
+    NTSTATUS (WINAPI *pFltBuildDefaultSecurityDescriptor)(PSECURITY_DESCRIPTOR *, ACCESS_MASK);
+    void (WINAPI *pFltFreeSecurityDescriptor)(PSECURITY_DESCRIPTOR);
+    PSECURITY_DESCRIPTOR sd = NULL;
+    NTSTATUS status;
+    PSID group = NULL, owner = NULL;
+    BOOLEAN isdefault, present;
+    PACL acl = NULL;
+    PACCESS_ALLOWED_ACE ace;
+    SID_IDENTIFIER_AUTHORITY auth = { SECURITY_NULL_SID_AUTHORITY };
+    PSID sid1, sid2;
+
+    pFltBuildDefaultSecurityDescriptor = get_proc_address("FltBuildDefaultSecurityDescriptor");
+    pFltFreeSecurityDescriptor = get_proc_address("FltFreeSecurityDescriptor");
+
+    if (!pFltBuildDefaultSecurityDescriptor || !pFltFreeSecurityDescriptor)
+    {
+        win_skip("FltBuildDefaultSecurityDescriptor is not available.\n");
+        return;
+    }
+
+    status = pFltBuildDefaultSecurityDescriptor(&sd, STANDARD_RIGHTS_ALL);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+    ok(sd != NULL, "Failed to return descriptor\n");
+
+    status = RtlGetGroupSecurityDescriptor(sd, &group, &isdefault);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+    ok(group == NULL, "group isn't NULL\n");
+
+    status = RtlGetOwnerSecurityDescriptor(sd, &owner, &isdefault);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+    ok(owner == NULL, "owner isn't NULL\n");
+
+    status = RtlGetDaclSecurityDescriptor(sd, &present, &acl, &isdefault);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+    ok(acl != NULL, "acl is NULL\n");
+    ok(acl->AceCount == 2, "got %d\n", acl->AceCount);
+
+    sid1 = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, RtlLengthRequiredSid(2));
+    RtlInitializeSid(sid1, &auth, 2);
+    *RtlSubAuthoritySid(sid1, 0)  = SECURITY_BUILTIN_DOMAIN_RID;
+    *RtlSubAuthoritySid(sid1, 1) = DOMAIN_GROUP_RID_ADMINS;
+
+    sid2 = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, RtlLengthRequiredSid(1));
+    RtlInitializeSid(sid2, &auth, 1);
+    *RtlSubAuthoritySid(sid2, 0)  = SECURITY_LOCAL_SYSTEM_RID;
+
+    /* SECURITY_BUILTIN_DOMAIN_RID */
+    status = RtlGetAce(acl, 0, (void**)&ace);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+
+    ok(ace->Header.AceType == ACCESS_ALLOWED_ACE_TYPE, "got %#x\n", ace->Header.AceType);
+    ok(ace->Header.AceFlags == 0, "got %#x\n", ace->Header.AceFlags);
+    ok(ace->Mask == STANDARD_RIGHTS_ALL, "got %#lx\n", ace->Mask);
+
+    ok(RtlEqualSid(sid1, (PSID)&ace->SidStart), "SID not equal\n");
+
+    /* SECURITY_LOCAL_SYSTEM_RID */
+    status = RtlGetAce(acl, 1, (void**)&ace);
+    ok(status == STATUS_SUCCESS, "got %#lx\n", status);
+
+    ok(ace->Header.AceType == ACCESS_ALLOWED_ACE_TYPE, "got %#x\n", ace->Header.AceType);
+    ok(ace->Header.AceFlags == 0, "got %#x\n", ace->Header.AceFlags);
+    ok(ace->Mask == STANDARD_RIGHTS_ALL, "got %#lx\n", ace->Mask);
+
+    ok(RtlEqualSid(sid2, (PSID)&ace->SidStart), "SID not equal\n");
+
+    RtlFreeHeap(GetProcessHeap(), 0, sid1);
+    RtlFreeHeap(GetProcessHeap(), 0, sid2);
+
+    pFltFreeSecurityDescriptor(sd);
+}
+
 static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *stack)
 {
     void *buffer = irp->AssociatedIrp.SystemBuffer;
@@ -2370,6 +2445,7 @@ static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *st
     test_process_memory(test_input);
     test_permanence();
     test_driver_object_extension();
+    test_default_security();

IoMarkIrpPending(irp);
     IoQueueWorkItem(work_item, main_test_task, DelayedWorkQueue, irp);

[PATCH v5 2/2] ntoskrnl.exe: Add FltBuildDefaultSecurityDescriptor test

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=130321

Your paranoid android.

=== w7u_2qxl (32 bit report) ===

Report validation errors:
ntoskrnl.exe:ntoskrnl is missing some skip messages

=== w7u_el (32 bit report) ===

Report validation errors:
ntoskrnl.exe:ntoskrnl is missing some skip messages

=== w8 (32 bit report) ===

Report validation errors:
ntoskrnl.exe:ntoskrnl is missing some skip messages

=== w1064_tsign (64 bit report) ===

Report validation errors:
ntoskrnl.exe:ntoskrnl is missing some skip messages

=== debian11 (32 bit report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit ar:MA report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit de report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit fr report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit he:IL report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit hi:IN report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit ja:JP report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11 (32 bit zh:CN report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.

=== debian11b (64 bit WoW report) ===

ntoskrnl.exe:
driver.c:2348: Test failed: FltBuildDefaultSecurityDescriptor is not available.