[PATCH 1/5] advapi32/tests: Add basic tests for CreateProcessAsUser().
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/tests/security.c | 118 +++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index 6919ea64ce9..bd8cc5f061b 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -7765,6 +7765,121 @@ static void test_duplicate_handle_access_child(void) CloseHandle(process); } +#define join_process(a) join_process_(__LINE__, a) +static void join_process_(int line, const PROCESS_INFORMATION *pi) +{ + DWORD ret = WaitForSingleObject(pi->hProcess, 1000); + ok_(__FILE__, line)(!ret, "wait failed\n"); + CloseHandle(pi->hProcess); + CloseHandle(pi->hThread); +} + +static void test_create_process_token(void) +{ + char cmdline[300], acl_buffer[200], sid_buffer[100]; + SECURITY_ATTRIBUTES sa = {.nLength = sizeof(sa)}; + ACL *acl = (ACL *)acl_buffer; + SID *sid = (SID *)sid_buffer; + SID_AND_ATTRIBUTES sid_attr; + HANDLE event, token, token2; + PROCESS_INFORMATION pi; + SECURITY_DESCRIPTOR sd; + STARTUPINFOA si = {0}; + DWORD size; + BOOL ret; + + size = sizeof(sid_buffer); + ret = CreateWellKnownSid(WinLocalSid, NULL, sid, &size); + ok(ret, "got error %u\n", GetLastError()); + ret = InitializeAcl(acl, sizeof(acl_buffer), ACL_REVISION); + ok(ret, "got error %u\n", GetLastError()); + ret = AddAccessAllowedAce(acl, ACL_REVISION, EVENT_MODIFY_STATE, sid); + ok(ret, "got error %u\n", GetLastError()); + InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); + ret = SetSecurityDescriptorDacl(&sd, TRUE, acl, FALSE); + ok(ret, "got error %u\n", GetLastError()); + sa.lpSecurityDescriptor = &sd; + event = CreateEventA(&sa, TRUE, TRUE, "test_event"); + ok(!!event, "got error %u\n", GetLastError()); + + sprintf(cmdline, "%s security restricted 0", myARGV[0]); + + ret = CreateProcessAsUserA(NULL, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "got error %u\n", GetLastError()); + join_process(&pi); + + ret = CreateProcessAsUserA(GetCurrentProcessToken(), NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + todo_wine ok(!ret, "expected failure\n"); + todo_wine ok(GetLastError() == ERROR_INVALID_HANDLE, "got error %u\n", GetLastError()); + if (ret) join_process(&pi); + + ret = OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY, &token); + ok(ret, "got error %u\n", GetLastError()); + ret = CreateProcessAsUserA(token, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret || broken(GetLastError() == ERROR_ACCESS_DENIED) /* < 7 */, "got error %u\n", GetLastError()); + if (ret) join_process(&pi); + CloseHandle(token); + + ret = OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token); + ok(ret, "got error %u\n", GetLastError()); + ret = CreateProcessAsUserA(token, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + todo_wine ok(!ret, "expected failure\n"); + todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + if (ret) join_process(&pi); + CloseHandle(token); + + ret = OpenProcessToken(GetCurrentProcess(), TOKEN_ASSIGN_PRIMARY, &token); + ok(ret, "got error %u\n", GetLastError()); + ret = CreateProcessAsUserA(token, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + todo_wine ok(!ret, "expected failure\n"); + todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + if (ret) join_process(&pi); + CloseHandle(token); + + ret = OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY | TOKEN_DUPLICATE, &token); + ok(ret, "got error %u\n", GetLastError()); + + ret = DuplicateTokenEx(token, TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY, NULL, + SecurityImpersonation, TokenImpersonation, &token2); + ok(ret, "got error %u\n", GetLastError()); + ret = CreateProcessAsUserA(token2, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret || broken(GetLastError() == ERROR_BAD_TOKEN_TYPE) /* < 7 */, "got error %u\n", GetLastError()); + if (ret) join_process(&pi); + CloseHandle(token2); + + sprintf(cmdline, "%s security restricted 1", myARGV[0]); + sid_attr.Sid = sid; + sid_attr.Attributes = 0; + ret = CreateRestrictedToken(token, 0, 1, &sid_attr, 0, NULL, 0, NULL, &token2); + ok(ret, "got error %u\n", GetLastError()); + ret = CreateProcessAsUserA(token2, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); + ok(ret, "got error %u\n", GetLastError()); + join_process(&pi); + CloseHandle(token2); + + CloseHandle(token); + + CloseHandle(event); +} + +static void test_create_process_token_child(void) +{ + HANDLE event; + + SetLastError(0xdeadbeef); + event = OpenEventA(EVENT_MODIFY_STATE, FALSE, "test_event"); + if (!atoi(myARGV[3])) + { + ok(!!event, "got error %u\n", GetLastError()); + CloseHandle(event); + } + else + { + todo_wine ok(!event, "expected failure\n"); + todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + } +} + START_TEST(security) { init(); @@ -7778,6 +7893,8 @@ START_TEST(security) test_process_security_child(); else if (!strcmp(myARGV[2], "duplicate")) test_duplicate_handle_access_child(); + else if (!strcmp(myARGV[2], "restricted")) + test_create_process_token_child(); return; } test_kernel_objects_security(); @@ -7824,6 +7941,7 @@ START_TEST(security) test_GetExplicitEntriesFromAclW(); test_BuildSecurityDescriptorW(); test_duplicate_handle_access(); + test_create_process_token(); /* Must be the last test, modifies process token */ test_token_security_descriptor(); -- 2.28.0
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ntdll/process.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/process.c b/dlls/ntdll/process.c index b6b0f10b9b0..4cf83753a79 100644 --- a/dlls/ntdll/process.c +++ b/dlls/ntdll/process.c @@ -56,12 +56,12 @@ NTSTATUS WINAPI RtlCreateUserProcess( UNICODE_STRING *path, ULONG attributes, RTL_USER_PROCESS_PARAMETERS *params, SECURITY_DESCRIPTOR *process_descr, SECURITY_DESCRIPTOR *thread_descr, - HANDLE parent, BOOLEAN inherit, HANDLE debug, HANDLE exception, + HANDLE parent, BOOLEAN inherit, HANDLE debug, HANDLE token, RTL_USER_PROCESS_INFORMATION *info ) { OBJECT_ATTRIBUTES process_attr, thread_attr; PS_CREATE_INFO create_info; - ULONG_PTR buffer[offsetof( PS_ATTRIBUTE_LIST, Attributes[5] ) / sizeof(ULONG_PTR)]; + ULONG_PTR buffer[offsetof( PS_ATTRIBUTE_LIST, Attributes[6] ) / sizeof(ULONG_PTR)]; PS_ATTRIBUTE_LIST *attr = (PS_ATTRIBUTE_LIST *)buffer; UINT pos = 0; @@ -98,6 +98,14 @@ NTSTATUS WINAPI RtlCreateUserProcess( UNICODE_STRING *path, ULONG attributes, attr->Attributes[pos].ReturnLength = NULL; pos++; } + if (token) + { + attr->Attributes[pos].Attribute = PS_ATTRIBUTE_TOKEN; + attr->Attributes[pos].Size = sizeof(token); + attr->Attributes[pos].ValuePtr = token; + attr->Attributes[pos].ReturnLength = NULL; + pos++; + } attr->TotalLength = offsetof( PS_ATTRIBUTE_LIST, Attributes[pos] ); InitializeObjectAttributes( &process_attr, NULL, 0, NULL, process_descr ); -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=79245 Your paranoid android. === debiant (build log) === The task timed out === debiant (build log) === The task timed out
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/kernelbase/process.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 648174b2a7a..e7998c00f28 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -263,14 +263,14 @@ struct _PROC_THREAD_ATTRIBUTE_LIST /*********************************************************************** * create_nt_process */ -static NTSTATUS create_nt_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, +static NTSTATUS create_nt_process( HANDLE token, SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, BOOL inherit, DWORD flags, RTL_USER_PROCESS_PARAMETERS *params, RTL_USER_PROCESS_INFORMATION *info, HANDLE parent, const struct proc_thread_attr *handle_list ) { OBJECT_ATTRIBUTES process_attr, thread_attr; PS_CREATE_INFO create_info; - ULONG_PTR buffer[offsetof( PS_ATTRIBUTE_LIST, Attributes[5] ) / sizeof(ULONG_PTR)]; + ULONG_PTR buffer[offsetof( PS_ATTRIBUTE_LIST, Attributes[6] ) / sizeof(ULONG_PTR)]; PS_ATTRIBUTE_LIST *attr = (PS_ATTRIBUTE_LIST *)buffer; UNICODE_STRING nameW; NTSTATUS status; @@ -315,6 +315,14 @@ static NTSTATUS create_nt_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES attr->Attributes[pos].ReturnLength = NULL; pos++; } + if (token) + { + attr->Attributes[pos].Attribute = PS_ATTRIBUTE_TOKEN; + attr->Attributes[pos].Size = sizeof(token); + attr->Attributes[pos].ValuePtr = token; + attr->Attributes[pos].ReturnLength = NULL; + pos++; + } attr->TotalLength = offsetof( PS_ATTRIBUTE_LIST, Attributes[pos] ); InitializeObjectAttributes( &process_attr, NULL, 0, NULL, psa ? psa->lpSecurityDescriptor : NULL ); @@ -335,7 +343,7 @@ static NTSTATUS create_nt_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES /*********************************************************************** * create_vdm_process */ -static NTSTATUS create_vdm_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, +static NTSTATUS create_vdm_process( HANDLE token, SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, BOOL inherit, DWORD flags, RTL_USER_PROCESS_PARAMETERS *params, RTL_USER_PROCESS_INFORMATION *info ) { @@ -356,7 +364,7 @@ static NTSTATUS create_vdm_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTE winevdm, params->ImagePathName.Buffer, params->CommandLine.Buffer ); RtlInitUnicodeString( ¶ms->ImagePathName, winevdm ); RtlInitUnicodeString( ¶ms->CommandLine, newcmdline ); - status = create_nt_process( psa, tsa, inherit, flags, params, info, NULL, NULL ); + status = create_nt_process( token, psa, tsa, inherit, flags, params, info, NULL, NULL ); HeapFree( GetProcessHeap(), 0, newcmdline ); return status; } @@ -365,7 +373,7 @@ static NTSTATUS create_vdm_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTE /*********************************************************************** * create_cmd_process */ -static NTSTATUS create_cmd_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, +static NTSTATUS create_cmd_process( HANDLE token, SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTES *tsa, BOOL inherit, DWORD flags, RTL_USER_PROCESS_PARAMETERS *params, RTL_USER_PROCESS_INFORMATION *info ) { @@ -384,7 +392,7 @@ static NTSTATUS create_cmd_process( SECURITY_ATTRIBUTES *psa, SECURITY_ATTRIBUTE swprintf( newcmdline, len, L"%s /s/c \"%s\"", comspec, params->CommandLine.Buffer ); RtlInitUnicodeString( ¶ms->ImagePathName, comspec ); RtlInitUnicodeString( ¶ms->CommandLine, newcmdline ); - status = create_nt_process( psa, tsa, inherit, flags, params, info, NULL, NULL ); + status = create_nt_process( token, psa, tsa, inherit, flags, params, info, NULL, NULL ); RtlFreeHeap( GetProcessHeap(), 0, newcmdline ); return status; } @@ -500,7 +508,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR TRACE( "app %s cmdline %s\n", debugstr_w(app_name), debugstr_w(cmd_line) ); - if (token) FIXME( "Creating a process with a token is not yet implemented\n" ); if (new_token) FIXME( "No support for returning created process token\n" ); if (app_name) @@ -585,7 +592,8 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR } } - status = create_nt_process( process_attr, thread_attr, inherit, flags, params, &rtl_info, parent, handle_list ); + status = create_nt_process( token, process_attr, thread_attr, inherit, + flags, params, &rtl_info, parent, handle_list ); switch (status) { case STATUS_SUCCESS: @@ -594,7 +602,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR case STATUS_INVALID_IMAGE_NE_FORMAT: case STATUS_INVALID_IMAGE_PROTECT: TRACE( "starting %s as Win16/DOS binary\n", debugstr_w(app_name) ); - status = create_vdm_process( process_attr, thread_attr, inherit, flags, params, &rtl_info ); + status = create_vdm_process( token, process_attr, thread_attr, inherit, flags, params, &rtl_info ); break; case STATUS_INVALID_IMAGE_NOT_MZ: /* check for .com or .bat extension */ @@ -602,12 +610,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR if (!wcsicmp( p, L".com" ) || !wcsicmp( p, L".pif" )) { TRACE( "starting %s as DOS binary\n", debugstr_w(app_name) ); - status = create_vdm_process( process_attr, thread_attr, inherit, flags, params, &rtl_info ); + status = create_vdm_process( token, process_attr, thread_attr, inherit, flags, params, &rtl_info ); } else if (!wcsicmp( p, L".bat" ) || !wcsicmp( p, L".cmd" )) { TRACE( "starting %s as batch binary\n", debugstr_w(app_name) ); - status = create_cmd_process( process_attr, thread_attr, inherit, flags, params, &rtl_info ); + status = create_cmd_process( token, process_attr, thread_attr, inherit, flags, params, &rtl_info ); } break; } -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=79246 Your paranoid android. === debiant (build log) === The task timed out === debiant (build log) === The task timed out
From: Michael Müller <michael(a)fds-team.de> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/tests/security.c | 24 +++++++++++------------- dlls/ntdll/unix/process.c | 2 +- server/process.c | 17 +++++++++++++---- server/process.h | 2 +- server/protocol.def | 1 + server/request.c | 2 +- server/security.h | 1 + server/token.c | 5 +++++ 8 files changed, 34 insertions(+), 20 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index bd8cc5f061b..c9318bef928 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -7732,16 +7732,16 @@ static void test_duplicate_handle_access_child(void) SetLastError(0xdeadbeef); event2 = OpenEventA(EVENT_MODIFY_STATE, FALSE, "test_dup"); - todo_wine ok(!event2, "expected failure\n"); - todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + ok(!event2, "expected failure\n"); + ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); ret = DuplicateHandle(process, event, process, &event2, EVENT_MODIFY_STATE, FALSE, 0); - ok(ret, "got error %u\n", GetLastError()); + todo_wine ok(ret, "got error %u\n", GetLastError()); SetLastError(0xdeadbeef); ret = DuplicateHandle(process, event, GetCurrentProcess(), &event2, EVENT_MODIFY_STATE, FALSE, 0); - todo_wine ok(!ret, "expected failure\n"); - todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + ok(!ret, "expected failure\n"); + ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); ret = DuplicateHandle(process, (HANDLE)(ULONG_PTR)_atoi64(myARGV[5]), GetCurrentProcess(), &token, 0, FALSE, DUPLICATE_SAME_ACCESS); @@ -7823,17 +7823,15 @@ static void test_create_process_token(void) ret = OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token); ok(ret, "got error %u\n", GetLastError()); ret = CreateProcessAsUserA(token, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); - todo_wine ok(!ret, "expected failure\n"); - todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); - if (ret) join_process(&pi); + ok(!ret, "expected failure\n"); + ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); CloseHandle(token); ret = OpenProcessToken(GetCurrentProcess(), TOKEN_ASSIGN_PRIMARY, &token); ok(ret, "got error %u\n", GetLastError()); ret = CreateProcessAsUserA(token, NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi); - todo_wine ok(!ret, "expected failure\n"); - todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); - if (ret) join_process(&pi); + ok(!ret, "expected failure\n"); + ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); CloseHandle(token); ret = OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY | TOKEN_DUPLICATE, &token); @@ -7875,8 +7873,8 @@ static void test_create_process_token_child(void) } else { - todo_wine ok(!event, "expected failure\n"); - todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + ok(!event, "expected failure\n"); + ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); } } diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index a6be00646a8..5ccf435e9ff 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -894,7 +894,6 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ TRACE( "%s image %s cmdline %s parent %p\n", debugstr_us( &path ), debugstr_us( ¶ms->ImagePathName ), debugstr_us( ¶ms->CommandLine ), parent ); if (debug) FIXME( "debug port %p not supported yet\n", debug ); - if (token) FIXME( "token %p not supported yet\n", token ); unixdir = get_unix_curdir( params ); @@ -942,6 +941,7 @@ NTSTATUS WINAPI NtCreateUserProcess( HANDLE *process_handle_ptr, HANDLE *thread_ SERVER_START_REQ( new_process ) { + req->token = wine_server_obj_handle( token ); req->parent_process = wine_server_obj_handle( parent ); req->inherit_all = !!(process_flags & PROCESS_CREATE_FLAGS_INHERIT_HANDLES); req->create_flags = params->DebugFlags; /* hack: creation flags stored in DebugFlags for now */ diff --git a/server/process.c b/server/process.c index 1650bd8f354..26958308ca7 100644 --- a/server/process.c +++ b/server/process.c @@ -504,7 +504,7 @@ static void start_sigkill_timer( struct process *process ) /* if the function fails the fd is closed */ struct process *create_process( int fd, struct process *parent, int inherit_all, const struct security_descriptor *sd, const obj_handle_t *handles, - unsigned int handle_count ) + unsigned int handle_count, struct token *token ) { struct process *process; @@ -581,7 +581,7 @@ struct process *create_process( int fd, struct process *parent, int inherit_all, : alloc_handle_table( process, 0 ); /* Note: for security reasons, starting a new process does not attempt * to use the current impersonation token for the new process */ - process->token = token_duplicate( parent->token, TRUE, 0, NULL, NULL, 0, NULL, 0 ); + process->token = token_duplicate( token ? token : parent->token, TRUE, 0, NULL, NULL, 0, NULL, 0 ); process->affinity = parent->affinity; } if (!process->handles || !process->token) goto error; @@ -1102,6 +1102,7 @@ DECL_HANDLER(new_process) const struct security_descriptor *sd; const struct object_attributes *objattr = get_req_object_attributes( &sd, &name, NULL ); struct process *process = NULL; + struct token *token = NULL; struct process *parent; struct thread *parent_thread = current; int socket_fd = thread_get_inflight_fd( current, req->socket_fd ); @@ -1220,7 +1221,14 @@ DECL_HANDLER(new_process) #undef FIXUP_LEN } - if (!(process = create_process( socket_fd, parent, req->inherit_all, sd, handles, req->handles_size / sizeof(*handles) ))) + if (req->token && !(token = get_token_obj( current->process, req->token, TOKEN_QUERY | TOKEN_ASSIGN_PRIMARY ))) + { + close( socket_fd ); + goto done; + } + + if (!(process = create_process( socket_fd, parent, req->inherit_all, sd, + handles, req->handles_size / sizeof(*handles), token ))) goto done; process->startup_info = (struct startup_info *)grab_object( info ); @@ -1284,6 +1292,7 @@ DECL_HANDLER(new_process) done: if (process) release_object( process ); + if (token) release_object( token ); release_object( parent ); release_object( info ); } @@ -1316,7 +1325,7 @@ DECL_HANDLER(exec_process) close( socket_fd ); return; } - if (!(process = create_process( socket_fd, NULL, 0, NULL, NULL, 0 ))) return; + if (!(process = create_process( socket_fd, NULL, 0, NULL, NULL, 0, NULL ))) return; create_thread( -1, process, NULL ); release_object( process ); } diff --git a/server/process.h b/server/process.h index 7facc4f6dfa..bc81d563b47 100644 --- a/server/process.h +++ b/server/process.h @@ -110,7 +110,7 @@ extern void free_ptid( unsigned int id ); extern void *get_ptid_entry( unsigned int id ); extern struct process *create_process( int fd, struct process *parent, int inherit_all, const struct security_descriptor *sd, const obj_handle_t *handles, - unsigned int handle_count ); + unsigned int handle_count, struct token *token ); extern data_size_t init_process( struct thread *thread ); extern struct thread *get_process_first_thread( struct process *process ); extern struct process *get_process_from_id( process_id_t id ); diff --git a/server/protocol.def b/server/protocol.def index 9437a303501..6534092f2cc 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -787,6 +787,7 @@ struct rawinput_device /* Create a new process from the context of the parent */ @REQ(new_process) + obj_handle_t token; /* process token */ obj_handle_t parent_process; /* parent process */ int inherit_all; /* inherit all handles from parent */ unsigned int create_flags; /* creation flags */ diff --git a/server/request.c b/server/request.c index 3ef4a244b84..40e9478e09e 100644 --- a/server/request.c +++ b/server/request.c @@ -583,7 +583,7 @@ static void master_socket_poll_event( struct fd *fd, int event ) int client = accept( get_unix_fd( master_socket->fd ), (struct sockaddr *) &dummy, &len ); if (client == -1) return; fcntl( client, F_SETFL, O_NONBLOCK ); - if ((process = create_process( client, NULL, 0, NULL, NULL, 0 ))) + if ((process = create_process( client, NULL, 0, NULL, NULL, 0, NULL ))) { create_thread( -1, process, NULL ); release_object( process ); diff --git a/server/security.h b/server/security.h index 7c35300afe4..bece7f54048 100644 --- a/server/security.h +++ b/server/security.h @@ -53,6 +53,7 @@ extern const PSID security_high_label_sid; /* token functions */ +extern struct token *get_token_obj( struct process *process, obj_handle_t handle, unsigned int access ); extern struct token *token_create_admin(void); extern int token_assign_label( struct token *token, PSID label ); extern struct token *token_duplicate( struct token *src_token, unsigned primary, diff --git a/server/token.c b/server/token.c index 8404e651196..8a106b7d34f 100644 --- a/server/token.c +++ b/server/token.c @@ -835,6 +835,11 @@ int token_assign_label( struct token *token, PSID label ) return ret; } +struct token *get_token_obj( struct process *process, obj_handle_t handle, unsigned int access ) +{ + return (struct token *)get_handle_obj( process, handle, access, &token_ops ); +} + struct token *token_create_admin( void ) { struct token *token = NULL; -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=79247 Your paranoid android. === debiant (32 bit report) === ntdll: om.c:2019: Test failed: got 89 === debiant (build log) === The task timed out === debiant (build log) === The task timed out
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/advapi32/tests/security.c | 6 +++--- server/handle.c | 13 ++++++++++--- server/security.h | 2 +- server/token.c | 6 ++++-- server/winstation.c | 4 ++-- 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index c9318bef928..eaaa29866bb 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -7704,8 +7704,8 @@ static void test_duplicate_handle_access(void) SetLastError(0xdeadbeef); ret = DuplicateHandle(GetCurrentProcess(), sync_event, pi.hProcess, &event2, EVENT_MODIFY_STATE, FALSE, 0); - todo_wine ok(!ret, "expected failure\n"); - todo_wine ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); + ok(!ret, "expected failure\n"); + ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); ret = WaitForSingleObject(pi.hProcess, 1000); ok(!ret, "wait failed\n"); @@ -7736,7 +7736,7 @@ static void test_duplicate_handle_access_child(void) ok(GetLastError() == ERROR_ACCESS_DENIED, "got error %u\n", GetLastError()); ret = DuplicateHandle(process, event, process, &event2, EVENT_MODIFY_STATE, FALSE, 0); - todo_wine ok(ret, "got error %u\n", GetLastError()); + ok(ret, "got error %u\n", GetLastError()); SetLastError(0xdeadbeef); ret = DuplicateHandle(process, event, GetCurrentProcess(), &event2, EVENT_MODIFY_STATE, FALSE, 0); diff --git a/server/handle.c b/server/handle.c index 90f9ea63d62..d4df33f4730 100644 --- a/server/handle.c +++ b/server/handle.c @@ -286,7 +286,7 @@ obj_handle_t alloc_handle( struct process *process, void *ptr, unsigned int acce { struct object *obj = ptr; access = obj->ops->map_access( obj, access ) & ~RESERVED_ALL; - if (access && !check_object_access( obj, &access )) return 0; + if (access && !check_object_access( NULL, obj, &access )) return 0; return alloc_handle_entry( process, ptr, access, attr ); } @@ -308,7 +308,7 @@ static obj_handle_t alloc_global_handle_no_access_check( void *obj, unsigned int /* return the handle, or 0 on error */ static obj_handle_t alloc_global_handle( void *obj, unsigned int access ) { - if (access && !check_object_access( obj, &access )) return 0; + if (access && !check_object_access( NULL, obj, &access )) return 0; return alloc_global_handle_no_access_check( obj, access ); } @@ -558,10 +558,17 @@ obj_handle_t duplicate_handle( struct process *src, obj_handle_t src_handle, str /* asking for the more access rights than src_access? */ if (access & ~src_access) { + if ((current->token && !check_object_access( current->token, obj, &access )) || + !check_object_access( dst->token, obj, &access )) + { + release_object( obj ); + return 0; + } + if (options & DUP_HANDLE_MAKE_GLOBAL) res = alloc_global_handle( obj, access ); else - res = alloc_handle( dst, obj, access, attr ); + res = alloc_handle_no_access_check( dst, obj, access, attr ); } else { diff --git a/server/security.h b/server/security.h index bece7f54048..08bdb8de805 100644 --- a/server/security.h +++ b/server/security.h @@ -86,7 +86,7 @@ static inline int security_equal_sid( const SID *sid1, const SID *sid2 ) extern void security_set_thread_token( struct thread *thread, obj_handle_t handle ); extern const SID *security_unix_uid_to_sid( uid_t uid ); -extern int check_object_access( struct object *obj, unsigned int *access ); +extern int check_object_access( struct token *token, struct object *obj, unsigned int *access ); static inline int thread_single_check_privilege( struct thread *thread, const LUID *priv) { diff --git a/server/token.c b/server/token.c index 8a106b7d34f..26d9708f2cd 100644 --- a/server/token.c +++ b/server/token.c @@ -1208,13 +1208,15 @@ const SID *token_get_primary_group( struct token *token ) return token->primary_group; } -int check_object_access(struct object *obj, unsigned int *access) +int check_object_access(struct token *token, struct object *obj, unsigned int *access) { GENERIC_MAPPING mapping; - struct token *token = current->token ? current->token : current->process->token; unsigned int status; int res; + if (!token) + token = current->token ? current->token : current->process->token; + mapping.GenericAll = obj->ops->map_access( obj, GENERIC_ALL ); if (!obj->sd) diff --git a/server/winstation.c b/server/winstation.c index b0142908549..c9c85e50fff 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -717,7 +717,7 @@ DECL_HANDLER(enum_winstation) { unsigned int access = WINSTA_ENUMERATE; if (req->index > index++) continue; - if (!check_object_access( &winsta->obj, &access )) continue; + if (!check_object_access( NULL, &winsta->obj, &access )) continue; clear_error(); reply->next = index; if ((name = get_object_name( &winsta->obj, &len ))) @@ -746,7 +746,7 @@ DECL_HANDLER(enum_desktop) unsigned int access = DESKTOP_ENUMERATE; if (req->index > index++) continue; if (!desktop->obj.name) continue; - if (!check_object_access( &desktop->obj, &access )) continue; + if (!check_object_access( NULL, &desktop->obj, &access )) continue; if ((name = get_object_name( &desktop->obj, &len ))) set_reply_data( name, min( len, get_reply_max_size() )); release_object( winstation ); -- 2.28.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=79248 Your paranoid android. === debiant (32 bit report) === ntdll: om.c:2004: Test failed: got 86 === debiant (build log) === The task timed out === debiant (build log) === The task timed out
participants (2)
-
Marvin -
Zebediah Figura