[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [v7 1/5] server: Named pipe query SD from pipe object
From: Jacek Caban <jacek () codeweavers ! com>
Date: 2017-09-29 14:04:54
Message-ID: 3e874a78-6ba6-dfc4-3188-89faf17f8364 () codeweavers ! com
[Download RAW message or body]
Hi Jonathan,
On 29.09.2017 14:35, Jonathan Doron wrote:
> Hi Jackek,
>
> I went over your patch but I believe it might have few issues that i
> wanted to raise before we change anything
> 1. For some reason when you are doing the "ok" test after
> CreateNamedPipe you do against != NULL should != INVALID_HANDLE_VALUE
> this is critical... since the 2nd server might have failed with
> OBJECT_NAME_ALREADY_EXISTS
> 2. When you create the 2nd server instance against the "ok" check is
> against NULL but not only that, you check against "server" and not
> "server2"
Good catch, thanks.
> The reason I'm raising these issues is because from my understanding
> of Windows, there can be only 1 instance for a given named pipe instance,
There may be as many instances as you specify in CreateNamedPipe call.
I fixed tests to handle that properly (see attachment). And, indeed, it
looks like SD is shared between all pipe instances.
Thanks,
Jacek
["patch.diff" (text/x-patch)]
diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c
index 84db14e814..636c40f7d6 100644
--- a/dlls/kernel32/tests/pipe.c
+++ b/dlls/kernel32/tests/pipe.c
@@ -3057,6 +3057,181 @@ static void test_overlapped_transport(BOOL msg_mode, BOOL msg_read_mode)
CloseHandle(client);
}
+static PSECURITY_DESCRIPTOR get_security_descriptor(HANDLE handle)
+{
+ SECURITY_DESCRIPTOR *sec_desc;
+ ULONG length = 0;
+ NTSTATUS status;
+
+ status = NtQuerySecurityObject(handle, GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION,
+ NULL, 0, &length);
+ ok(status == STATUS_BUFFER_TOO_SMALL,
+ "Failed to query object security descriptor length: %08x\n", status);
+ ok(length != 0, "length = 0\n");
+
+ sec_desc = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, length);
+ status = NtQuerySecurityObject(handle, GROUP_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION,
+ sec_desc, length, &length);
+ ok(status == STATUS_SUCCESS, "Failed to query object security descriptor: %08x\n", status);
+
+ return sec_desc;
+}
+
+static TOKEN_OWNER *get_current_owner(void)
+{
+ TOKEN_OWNER *owner;
+ ULONG length = 0;
+ HANDLE token;
+ BOOL ret;
+
+ ret = OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &token);
+ ok(ret, "Failed to get process token: %u\n", GetLastError());
+
+ ret = GetTokenInformation(token, TokenOwner, NULL, 0, &length);
+ ok(!ret && GetLastError() == ERROR_INSUFFICIENT_BUFFER,
+ "GetTokenInformation failed: %u\n", GetLastError());
+ ok(length != 0, "Failed to get token owner information length: %u\n", GetLastError());
+
+ owner = HeapAlloc(GetProcessHeap(), 0, length);
+ ret = GetTokenInformation(token, TokenOwner, owner, length, &length);
+ ok(ret, "Failed to get token owner information: %u)\n", GetLastError());
+
+ CloseHandle(token);
+ return owner;
+}
+
+static TOKEN_PRIMARY_GROUP *get_current_group(void)
+{
+ TOKEN_PRIMARY_GROUP *group;
+ ULONG length = 0;
+ HANDLE token;
+ BOOL ret;
+
+ ret = OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &token);
+ ok(ret, "Failed to get process token: %u\n", GetLastError());
+
+ ret = GetTokenInformation(token, TokenPrimaryGroup, NULL, 0, &length);
+ ok(!ret && GetLastError() == ERROR_INSUFFICIENT_BUFFER,
+ "GetTokenInformation failed: %u\n", GetLastError());
+ ok(length != 0, "Failed to get primary group token information length: %u\n", GetLastError());
+
+ group = HeapAlloc(GetProcessHeap(), 0, length);
+ ret = GetTokenInformation(token, TokenPrimaryGroup, group, length, &length);
+ ok(ret, "Failed to get primary group token information: %u\n", GetLastError());
+
+ CloseHandle(token);
+ return group;
+}
+
+static SID *well_known_sid(WELL_KNOWN_SID_TYPE sid_type)
+{
+ DWORD size = SECURITY_MAX_SID_SIZE;
+ SID *sid;
+ BOOL ret;
+
+ sid = HeapAlloc(GetProcessHeap(), 0, size);
+ ret = CreateWellKnownSid(sid_type, NULL, sid, &size);
+ ok(ret, "CreateWellKnownSid failed: %u\n", GetLastError());
+ return sid;
+}
+
+#define test_group(a,b) _test_group(__LINE__,a,b)
+static void _test_group(unsigned line, HANDLE handle, SID *expected_sid)
+{
+ SECURITY_DESCRIPTOR *sec_desc;
+ BOOLEAN defaulted;
+ PSID group_sid;
+ NTSTATUS status;
+
+ sec_desc = get_security_descriptor(handle);
+
+ status = RtlGetGroupSecurityDescriptor(sec_desc, &group_sid, &defaulted);
+ ok_(__FILE__,line)(status == STATUS_SUCCESS,
+ "Failed to query group from security descriptor: %08x\n", status);
+ ok_(__FILE__,line)(EqualSid(group_sid, expected_sid), "SIDs are not equal\n");
+
+ HeapFree(GetProcessHeap(), 0, sec_desc);
+}
+
+static void test_security(void)
+{
+ char sec_desc[SECURITY_DESCRIPTOR_MIN_LENGTH];
+ TOKEN_PRIMARY_GROUP *process_group;
+ SECURITY_ATTRIBUTES sec_attr;
+ TOKEN_OWNER *process_owner;
+ HANDLE server, client, server2;
+ SID *world_sid, *local_sid;
+ NTSTATUS status;
+ BOOL ret;
+
+ trace("security tests...\n");
+
+ process_owner = get_current_owner();
+ process_group = get_current_group();
+ world_sid = well_known_sid(WinWorldSid);
+ local_sid = well_known_sid(WinLocalSid);
+
+ ret = InitializeSecurityDescriptor(sec_desc, SECURITY_DESCRIPTOR_REVISION);
+ ok(ret, "InitializeSecurityDescriptor failed\n");
+
+ ret = SetSecurityDescriptorOwner(sec_desc, process_owner->Owner, FALSE);
+ ok(ret, "SetSecurityDescriptorOwner failed\n");
+
+ ret = SetSecurityDescriptorGroup(sec_desc, process_group->PrimaryGroup, FALSE);
+ ok(ret, "SetSecurityDescriptorGroup failed\n");
+
+ sec_attr.nLength = sizeof(sec_attr);
+ sec_attr.lpSecurityDescriptor = sec_desc;
+ sec_attr.bInheritHandle = FALSE;
+ server = CreateNamedPipeA(PIPENAME, PIPE_ACCESS_DUPLEX | WRITE_OWNER, PIPE_TYPE_BYTE, 10,
+ 0x20000, 0x20000, 0, NULL);
+ ok(server != INVALID_HANDLE_VALUE, "CreateNamedPipe failed: %u\n", GetLastError());
+
+ client = CreateFileA(PIPENAME, GENERIC_ALL, 0, NULL, OPEN_EXISTING, 0, NULL);
+ ok(client != INVALID_HANDLE_VALUE, "CreateFile failed: %u\n", GetLastError());
+
+ test_group(server, process_group->PrimaryGroup);
+ test_group(client, process_group->PrimaryGroup);
+
+ /* set server group, client changes as well */
+ ret = SetSecurityDescriptorGroup(sec_desc, world_sid, FALSE);
+ ok(ret, "SetSecurityDescriptorGroup failed\n");
+
+ status = NtSetSecurityObject(server, GROUP_SECURITY_INFORMATION, sec_desc);
+ ok(status == STATUS_SUCCESS, "NtSetSecurityObject failed: %08x\n", status);
+
+ test_group(server, world_sid);
+ test_group(client, world_sid);
+
+ /* new instance of pipe server has its own security descriptor */
+ server2 = CreateNamedPipeA(PIPENAME, PIPE_ACCESS_DUPLEX, PIPE_TYPE_BYTE, 10,
+ 0x20000, 0x20000, 0, NULL);
+ ok(server2 != INVALID_HANDLE_VALUE, "CreateNamedPipe failed: %u\n", GetLastError());
+ test_group(server2, world_sid);
+ CloseHandle(server2);
+
+ /* set client group, server changes as well */
+ ret = SetSecurityDescriptorGroup(sec_desc, local_sid, FALSE);
+ ok(ret, "SetSecurityDescriptorGroup failed\n");
+
+ status = NtSetSecurityObject(server, GROUP_SECURITY_INFORMATION, sec_desc);
+ ok(status == STATUS_SUCCESS, "NtSetSecurityObject failed: %08x\n", status);
+
+ test_group(server, local_sid);
+ test_group(client, local_sid);
+
+ CloseHandle(server);
+
+ /* SD is preserved after closing server object */
+ test_group(client, local_sid);
+ CloseHandle(client);
+
+ HeapFree(GetProcessHeap(), 0, process_owner);
+ HeapFree(GetProcessHeap(), 0, process_group);
+ HeapFree(GetProcessHeap(), 0, world_sid);
+ HeapFree(GetProcessHeap(), 0, local_sid);
+}
+
START_TEST(pipe)
{
char **argv;
@@ -3095,6 +3270,7 @@ START_TEST(pipe)
test_readfileex_pending();
test_overlapped_transport(TRUE, FALSE);
test_overlapped_transport(TRUE, TRUE);
+ test_security();
if (broken(1)) /* FIXME: Remove once Wine is ready. */
test_overlapped_transport(FALSE, FALSE);
}
[Attachment #4 (text/plain)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic