From: Joris van der Wel Subject: [4/4] server: implement passing a thread security descriptor to CreateProcess Message-Id: Date: Sun, 3 Aug 2014 12:52:44 +0200 server: implement passing a thread security descriptor to CreateProcess --- dlls/advapi32/tests/security.c | 44 ++++++++++++++++++++++++++++++++++++++---- dlls/kernel32/process.c | 24 ++++++++++++++++++++--- server/process.c | 25 ++++++++++++++++++++++-- 3 files changed, 84 insertions(+), 9 deletions(-) From 1730bed78f0df8d2a7568791de15030920d3c45a Mon Sep 17 00:00:00 2001 From: Joris van der Wel Date: Thu, 31 Jul 2014 23:44:22 +0200 Subject: server: implement passing a thread security descriptor to CreateProcess --- dlls/advapi32/tests/security.c | 44 ++++++++++++++++++++++++++++++++++++++---- dlls/kernel32/process.c | 24 ++++++++++++++++++++--- server/process.c | 25 ++++++++++++++++++++++-- 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index b1b35aa..244844f 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -2532,12 +2532,12 @@ static void test_process_security(void) PTOKEN_OWNER owner; PTOKEN_PRIMARY_GROUP group; PSID AdminSid = NULL, UsersSid = NULL; - PACL Acl = NULL; - SECURITY_DESCRIPTOR *SecurityDescriptor = NULL; + PACL Acl = NULL, ThreadAcl = NULL; + SECURITY_DESCRIPTOR *SecurityDescriptor = NULL, *ThreadSecurityDescriptor = NULL; char buffer[MAX_PATH]; PROCESS_INFORMATION info; STARTUPINFOA startup; - SECURITY_ATTRIBUTES psa; + SECURITY_ATTRIBUTES psa, tsa; HANDLE token, event; DWORD size; SID_IDENTIFIER_AUTHORITY SIDAuthWorld = { SECURITY_WORLD_SID_AUTHORITY }; @@ -2658,11 +2658,36 @@ static void test_process_security(void) psa.lpSecurityDescriptor = SecurityDescriptor; psa.bInheritHandle = TRUE; + ThreadSecurityDescriptor = HeapAlloc(GetProcessHeap(), 0, SECURITY_DESCRIPTOR_MIN_LENGTH); + res = InitializeSecurityDescriptor(ThreadSecurityDescriptor, SECURITY_DESCRIPTOR_REVISION); + ok(res, "InitializeSecurityDescriptor failed with error %d\n", GetLastError()); + + ThreadAcl = HeapAlloc(GetProcessHeap(), 0, 256); + res = InitializeAcl(ThreadAcl, 256, ACL_REVISION); + ok(res, "InitializeAcl failed with error %d\n", GetLastError()); + res = AddAccessDeniedAce(ThreadAcl, ACL_REVISION, THREAD_SET_THREAD_TOKEN, AdminSid); + ok(res, "AddAccessDeniedAce failed with error %d\n", GetLastError()); + res = AddAccessAllowedAce(ThreadAcl, ACL_REVISION, THREAD_ALL_ACCESS, AdminSid); + ok(res, "AddAccessAllowedAce failed with error %d\n", GetLastError()); + + res = SetSecurityDescriptorOwner(ThreadSecurityDescriptor, AdminSid, FALSE); + ok(res, "SetSecurityDescriptorOwner failed with error %d\n", GetLastError()); + res = SetSecurityDescriptorGroup(ThreadSecurityDescriptor, UsersSid, FALSE); + ok(res, "SetSecurityDescriptorGroup failed with error %d\n", GetLastError()); + res = SetSecurityDescriptorDacl(ThreadSecurityDescriptor, TRUE, ThreadAcl, FALSE); + ok(res, "SetSecurityDescriptorDacl failed with error %d\n", GetLastError()); + + tsa.nLength = sizeof(tsa); + tsa.lpSecurityDescriptor = ThreadSecurityDescriptor; + tsa.bInheritHandle = TRUE; + /* Doesn't matter what ACL say we should get full access for ourselves */ - res = CreateProcessA( NULL, buffer, &psa, NULL, FALSE, 0, NULL, NULL, &startup, &info ); + res = CreateProcessA( NULL, buffer, &psa, &tsa, FALSE, 0, NULL, NULL, &startup, &info ); ok(res, "CreateProcess with err:%d\n", GetLastError()); TEST_GRANTED_ACCESS2( info.hProcess, PROCESS_ALL_ACCESS_NT4, STANDARD_RIGHTS_ALL | SPECIFIC_RIGHTS_ALL ); + TEST_GRANTED_ACCESS2( info.hThread, THREAD_ALL_ACCESS_NT4, + STANDARD_RIGHTS_ALL | SPECIFIC_RIGHTS_ALL ); winetest_wait_child_process( info.hProcess ); FreeSid(EveryoneSid); @@ -2673,6 +2698,8 @@ static void test_process_security(void) HeapFree(GetProcessHeap(), 0, owner); HeapFree(GetProcessHeap(), 0, Acl); HeapFree(GetProcessHeap(), 0, SecurityDescriptor); + HeapFree(GetProcessHeap(), 0, ThreadAcl); + HeapFree(GetProcessHeap(), 0, ThreadSecurityDescriptor); } static void test_process_security_child(void) @@ -2728,6 +2755,15 @@ static void test_process_security_child(void) TEST_GRANTED_ACCESS( handle1, PROCESS_VM_READ ); CloseHandle( handle1 ); CloseHandle( handle ); + + + handle = OpenThread( THREAD_TERMINATE, FALSE, GetCurrentThreadId() ); + ok(handle != NULL, "OpenThread(THREAD_TERMINATE) with err:%d\n", GetLastError()); + TEST_GRANTED_ACCESS( handle, PROCESS_TERMINATE ); + CloseHandle( handle ); + + handle = OpenThread( THREAD_SET_THREAD_TOKEN, FALSE, GetCurrentThreadId() ); + ok(handle == NULL, "OpenThread(THREAD_SET_THREAD_TOKEN) should have failed\n"); } static void test_impersonation_level(void) diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index 44e7711..dd21100 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -2003,8 +2003,8 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW int socketfd[2], stdin_fd = -1, stdout_fd = -1; pid_t pid; int err, cpu; - struct security_descriptor *psd = NULL; - data_size_t psd_len = 0; + struct security_descriptor *psd = NULL, *tsd = NULL; + data_size_t psd_len = 0, tsd_len = 0; if ((cpu = get_process_cpu( filename, binary_info )) == -1) { @@ -2022,12 +2022,26 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW return FALSE; } } + if (tsa && (tsa->nLength >= sizeof(*tsa)) && tsa->lpSecurityDescriptor) + { + status = create_struct_sd( tsa->lpSecurityDescriptor, &tsd, &tsd_len ); + + if (status != STATUS_SUCCESS) + { + RtlFreeHeap(GetProcessHeap(), 0, psd); + RtlFreeHeap(GetProcessHeap(), 0, tsd); + WARN("Invalid thread security descriptor with status %x\n", status); + SetLastError( RtlNtStatusToDosError(status) ); + return FALSE; + } + } /* create the socket for the new process */ if (socketpair( PF_UNIX, SOCK_STREAM, 0, socketfd ) == -1) { RtlFreeHeap(GetProcessHeap(), 0, psd); + RtlFreeHeap(GetProcessHeap(), 0, tsd); SetLastError( ERROR_TOO_MANY_OPEN_FILES ); return FALSE; } @@ -2068,6 +2082,7 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW } close( socketfd[0] ); RtlFreeHeap(GetProcessHeap(), 0, psd); + RtlFreeHeap(GetProcessHeap(), 0, tsd); SetLastError( RtlNtStatusToDosError( status )); return FALSE; } @@ -2081,6 +2096,7 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW close( socketfd[0] ); close( socketfd[1] ); RtlFreeHeap(GetProcessHeap(), 0, psd); + RtlFreeHeap(GetProcessHeap(), 0, tsd); return FALSE; } if (!env) env = NtCurrentTeb()->Peb->ProcessParameters->Environment; @@ -2115,10 +2131,11 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW req->thread_attr = (tsa && (tsa->nLength >= sizeof(*tsa)) && tsa->bInheritHandle) ? OBJ_INHERIT : 0; req->cpu = cpu; req->process_sd_size= psd_len; - req->thread_sd_size = 0; + req->thread_sd_size = tsd_len; req->info_size = startup_info_size; wine_server_add_data( req, psd, psd_len ); + wine_server_add_data( req, tsd, tsd_len ); wine_server_add_data( req, startup_info, startup_info_size ); wine_server_add_data( req, env, (env_end - env) * sizeof(WCHAR) ); if (!(status = wine_server_call( req ))) @@ -2133,6 +2150,7 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW SERVER_END_REQ; RtlFreeHeap(GetProcessHeap(), 0, psd); + RtlFreeHeap(GetProcessHeap(), 0, tsd); RtlReleasePebLock(); if (status) diff --git a/server/process.c b/server/process.c index 571f05a..52c04f7 100644 --- a/server/process.c +++ b/server/process.c @@ -880,7 +880,7 @@ DECL_HANDLER(new_process) struct process *process; struct process *parent = current->process; int socket_fd = thread_get_inflight_fd( current, req->socket_fd ); - const struct security_descriptor *req_psd = NULL; + const struct security_descriptor *req_psd = NULL, *req_tsd = NULL; const startup_info_t *req_info; data_size_t req_info_size; const WCHAR *req_env; @@ -903,7 +903,17 @@ DECL_HANDLER(new_process) return; } } + if (req->thread_sd_size) + { + req_tsd = (const struct security_descriptor *) + ((char*)get_req_data() + req->process_sd_size); + if (!sd_is_valid( req_tsd, req->thread_sd_size )) + { + set_error( STATUS_INVALID_SECURITY_DESCR ); + return; + } + } req_info = (const startup_info_t *) ((char*)get_req_data() + req->process_sd_size + req->thread_sd_size); req_env = (const WCHAR *) @@ -1043,7 +1053,18 @@ DECL_HANDLER(new_process) DACL_SECURITY_INFORMATION| SACL_SECURITY_INFORMATION ); } - + if (req_tsd) + { + /* In CreateProcess the thread defaults come from the process token, + * (this is not the case during CreateThread however) */ + set_sd_defaults_from_token( &thread->obj, + req_tsd, + OWNER_SECURITY_INFORMATION| + GROUP_SECURITY_INFORMATION| + DACL_SECURITY_INFORMATION| + SACL_SECURITY_INFORMATION, + process->token ); + } done: release_object( info ); } -- 1.9.1