From: Joris van der Wel Subject: [3/4] server: implement passing a process security descriptor to CreateProcess Message-Id: Date: Sun, 3 Aug 2014 12:52:32 +0200 server: implement passing a process security descriptor to CreateProcess. For now the function "NTDLL_create_struct_sd" has been duplicated in kernel32. This is needed because kernel32 makes the server call. Kernel32 currently makes the server call because NtCreateProcess(Ex) has not been implemented in ntdll. When NtCreateProcessEx (and NtCreateThreadEx) gets implemented, the server call will be made from within ntdll instead, and this extra function in kernel32 will no longer be needed. --- dlls/advapi32/tests/security.c | 3 -- dlls/kernel32/process.c | 85 +++++++++++++++++++++++++++++++++++++++++- server/process.c | 24 ++++++++++++ 3 files changed, 108 insertions(+), 4 deletions(-) From 5d75c0684b29d79c65e43a4973d0afe9c1759abc Mon Sep 17 00:00:00 2001 From: Joris van der Wel Date: Thu, 31 Jul 2014 23:08:15 +0200 Subject: server: implement passing a process security descriptor to CreateProcess. For now the function "NTDLL_create_struct_sd" has been duplicated in kernel32. This is needed because kernel32 makes the server call. Kernel32 currently makes the server call because NtCreateProcess(Ex) has not been implemented in ntdll. When NtCreateProcessEx (and NtCreateThreadEx) gets implemented, the server call will be made from within ntdll instead, and this extra function in kernel32 will no longer be needed. --- dlls/advapi32/tests/security.c | 3 -- dlls/kernel32/process.c | 85 +++++++++++++++++++++++++++++++++++++++++- server/process.c | 24 ++++++++++++ 3 files changed, 108 insertions(+), 4 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index b44496a..b1b35aa 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -2696,7 +2696,6 @@ static void test_process_security_child(void) ret = DuplicateHandle( GetCurrentProcess(), handle, GetCurrentProcess(), &handle1, PROCESS_ALL_ACCESS, TRUE, 0 ); err = GetLastError(); - todo_wine ok(!ret && err == ERROR_ACCESS_DENIED, "duplicating handle should have failed " "with STATUS_ACCESS_DENIED, instead of err:%d\n", err); @@ -2704,10 +2703,8 @@ static void test_process_security_child(void) /* These two should fail - they are denied by ACL */ handle = OpenProcess( PROCESS_VM_READ, FALSE, GetCurrentProcessId() ); - todo_wine ok(handle == NULL, "OpenProcess(PROCESS_VM_READ) should have failed\n"); handle = OpenProcess( PROCESS_ALL_ACCESS, FALSE, GetCurrentProcessId() ); - todo_wine ok(handle == NULL, "OpenProcess(PROCESS_ALL_ACCESS) should have failed\n"); /* Documented privilege elevation */ diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index efd0e84..44e7711 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -1916,6 +1916,70 @@ static pid_t exec_loader( LPCWSTR cmd_line, unsigned int flags, int socketfd, return pid; } +/* creates a struct security_descriptor and contained information in one contiguous piece of memory */ +static NTSTATUS create_struct_sd(PSECURITY_DESCRIPTOR nt_sd, struct security_descriptor **server_sd, + data_size_t *server_sd_len) +{ + unsigned int len; + PSID owner, group; + ACL *dacl, *sacl; + BOOLEAN owner_present, group_present, dacl_present, sacl_present; + BOOLEAN defaulted; + NTSTATUS status; + unsigned char *ptr; + + if (!nt_sd) + { + *server_sd = NULL; + *server_sd_len = 0; + return STATUS_SUCCESS; + } + + len = sizeof(struct security_descriptor); + + status = RtlGetOwnerSecurityDescriptor(nt_sd, &owner, &owner_present); + if (status != STATUS_SUCCESS) return status; + status = RtlGetGroupSecurityDescriptor(nt_sd, &group, &group_present); + if (status != STATUS_SUCCESS) return status; + status = RtlGetSaclSecurityDescriptor(nt_sd, &sacl_present, &sacl, &defaulted); + if (status != STATUS_SUCCESS) return status; + status = RtlGetDaclSecurityDescriptor(nt_sd, &dacl_present, &dacl, &defaulted); + if (status != STATUS_SUCCESS) return status; + + if (owner_present) + len += RtlLengthSid(owner); + if (group_present) + len += RtlLengthSid(group); + if (sacl_present && sacl) + len += sacl->AclSize; + if (dacl_present && dacl) + len += dacl->AclSize; + + /* fix alignment for the Unicode name that follows the structure */ + len = (len + sizeof(WCHAR) - 1) & ~(sizeof(WCHAR) - 1); + *server_sd = RtlAllocateHeap(GetProcessHeap(), 0, len); + if (!*server_sd) return STATUS_NO_MEMORY; + + (*server_sd)->control = ((SECURITY_DESCRIPTOR *)nt_sd)->Control & ~SE_SELF_RELATIVE; + (*server_sd)->owner_len = owner_present ? RtlLengthSid(owner) : 0; + (*server_sd)->group_len = group_present ? RtlLengthSid(group) : 0; + (*server_sd)->sacl_len = (sacl_present && sacl) ? sacl->AclSize : 0; + (*server_sd)->dacl_len = (dacl_present && dacl) ? dacl->AclSize : 0; + + ptr = (unsigned char *)(*server_sd + 1); + memcpy(ptr, owner, (*server_sd)->owner_len); + ptr += (*server_sd)->owner_len; + memcpy(ptr, group, (*server_sd)->group_len); + ptr += (*server_sd)->group_len; + memcpy(ptr, sacl, (*server_sd)->sacl_len); + ptr += (*server_sd)->sacl_len; + memcpy(ptr, dacl, (*server_sd)->dacl_len); + + *server_sd_len = len; + + return STATUS_SUCCESS; +} + /*********************************************************************** * create_process * @@ -1939,17 +2003,31 @@ 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; if ((cpu = get_process_cpu( filename, binary_info )) == -1) { SetLastError( ERROR_BAD_EXE_FORMAT ); return FALSE; } + + if (psa && (psa->nLength >= sizeof(*psa)) && psa->lpSecurityDescriptor) + { + status = create_struct_sd( psa->lpSecurityDescriptor, &psd, &psd_len ); + if (status != STATUS_SUCCESS) + { + WARN("Invalid process 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); SetLastError( ERROR_TOO_MANY_OPEN_FILES ); return FALSE; } @@ -1989,6 +2067,7 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW winedebug, binary_info, TRUE ); } close( socketfd[0] ); + RtlFreeHeap(GetProcessHeap(), 0, psd); SetLastError( RtlNtStatusToDosError( status )); return FALSE; } @@ -2001,6 +2080,7 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW RtlReleasePebLock(); close( socketfd[0] ); close( socketfd[1] ); + RtlFreeHeap(GetProcessHeap(), 0, psd); return FALSE; } if (!env) env = NtCurrentTeb()->Peb->ProcessParameters->Environment; @@ -2034,10 +2114,11 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW req->thread_access = THREAD_ALL_ACCESS; req->thread_attr = (tsa && (tsa->nLength >= sizeof(*tsa)) && tsa->bInheritHandle) ? OBJ_INHERIT : 0; req->cpu = cpu; - req->process_sd_size= 0; + req->process_sd_size= psd_len; req->thread_sd_size = 0; req->info_size = startup_info_size; + wine_server_add_data( req, psd, psd_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 ))) @@ -2051,6 +2132,8 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW } SERVER_END_REQ; + RtlFreeHeap(GetProcessHeap(), 0, psd); + RtlReleasePebLock(); if (status) { diff --git a/server/process.c b/server/process.c index 110a38f..571f05a 100644 --- a/server/process.c +++ b/server/process.c @@ -880,6 +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 startup_info_t *req_info; data_size_t req_info_size; const WCHAR *req_env; @@ -892,6 +893,16 @@ DECL_HANDLER(new_process) close( socket_fd ); return; } + + if (req->process_sd_size) + { + req_psd = get_req_data(); + if (!sd_is_valid( req_psd, req->process_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); @@ -1020,6 +1031,19 @@ DECL_HANDLER(new_process) reply->phandle = alloc_handle( parent, process, req->process_access, req->process_attr ); reply->thandle = alloc_handle( parent, thread, req->thread_access, req->thread_attr ); + /* note: alloc_handle might fail with access denied + * if the security descriptor is set before that call */ + + if (req_psd) + { + default_set_sd( &process->obj, + req_psd, + OWNER_SECURITY_INFORMATION| + GROUP_SECURITY_INFORMATION| + DACL_SECURITY_INFORMATION| + SACL_SECURITY_INFORMATION ); + } + done: release_object( info ); } -- 1.9.1