From 26b0797bcb80e957d424b3138e8bfb0fe0b25add Mon Sep 17 00:00:00 2001 From: Sebastian Lackner Date: Sat, 23 Aug 2014 06:33:01 +0200 Subject: [PATCH] Rewrite part of server-CreateProcess_ACLs in order to fix incorrect error codes and memory leaks. --- patches/Makefile | 10 +- ...ending-process-and-thread-security-d.patch | 112 --------- ...ending-thread-and-process-security-d.patch | 128 ++++++++++ ...t-passing-a-process-security-descri.patch} | 171 +++++-------- ...dd-additional-tests-for-passing-a-th.patch | 96 ++++++++ ...-passing-a-thread-security-descripto.patch | 228 ------------------ patches/server-CreateProcess_ACLs/definition | 4 +- 7 files changed, 291 insertions(+), 458 deletions(-) delete mode 100644 patches/server-CreateProcess_ACLs/0002-server-Support-sending-process-and-thread-security-d.patch create mode 100644 patches/server-CreateProcess_ACLs/0002-server-Support-sending-thread-and-process-security-d.patch rename patches/server-CreateProcess_ACLs/{0003-server-implement-passing-a-process-security-descript.patch => 0003-kernel32-Implement-passing-a-process-security-descri.patch} (55%) create mode 100644 patches/server-CreateProcess_ACLs/0004-advapi32-tests-Add-additional-tests-for-passing-a-th.patch delete mode 100644 patches/server-CreateProcess_ACLs/0004-server-implement-passing-a-thread-security-descripto.patch diff --git a/patches/Makefile b/patches/Makefile index 1db81caa..fea4ebd8 100644 --- a/patches/Makefile +++ b/patches/Makefile @@ -635,7 +635,7 @@ server-Address_Change_Notification.ok: # Patchset server-CreateProcess_ACLs # | # | Included patches: -# | * Implement passing ACLs to CreateProcess. [by Joris van der Wel] +# | * Implement passing ACLs to CreateProcess. [rev 2, by Joris van der Wel / Sebastian Lackner] # | # | This patchset fixes the following Wine bugs: # | * [#22006] Support for process ACLs @@ -647,11 +647,11 @@ server-Address_Change_Notification.ok: .INTERMEDIATE: server-CreateProcess_ACLs.ok server-CreateProcess_ACLs.ok: $(call APPLY_FILE,server-CreateProcess_ACLs/0001-server-A-new-function-set_sd_defaults_from_token-tha.patch) - $(call APPLY_FILE,server-CreateProcess_ACLs/0002-server-Support-sending-process-and-thread-security-d.patch) - $(call APPLY_FILE,server-CreateProcess_ACLs/0003-server-implement-passing-a-process-security-descript.patch) - $(call APPLY_FILE,server-CreateProcess_ACLs/0004-server-implement-passing-a-thread-security-descripto.patch) + $(call APPLY_FILE,server-CreateProcess_ACLs/0002-server-Support-sending-thread-and-process-security-d.patch) + $(call APPLY_FILE,server-CreateProcess_ACLs/0003-kernel32-Implement-passing-a-process-security-descri.patch) + $(call APPLY_FILE,server-CreateProcess_ACLs/0004-advapi32-tests-Add-additional-tests-for-passing-a-th.patch) @( \ - echo '+ { "server-CreateProcess_ACLs", "Joris van der Wel", "Implement passing ACLs to CreateProcess." },'; \ + echo '+ { "server-CreateProcess_ACLs", "Joris van der Wel / Sebastian Lackner", "Implement passing ACLs to CreateProcess. [rev 2]" },'; \ ) > server-CreateProcess_ACLs.ok # Patchset server-Inherited_ACLs diff --git a/patches/server-CreateProcess_ACLs/0002-server-Support-sending-process-and-thread-security-d.patch b/patches/server-CreateProcess_ACLs/0002-server-Support-sending-process-and-thread-security-d.patch deleted file mode 100644 index f71d61c5..00000000 --- a/patches/server-CreateProcess_ACLs/0002-server-Support-sending-process-and-thread-security-d.patch +++ /dev/null @@ -1,112 +0,0 @@ -From 38c3a1a50ca4bdef0b5ec0cf120fd5da889954dd Mon Sep 17 00:00:00 2001 -From: Joris van der Wel -Date: Sun, 3 Aug 2014 12:52:14 +0200 -Subject: server: Support sending process and thread security descriptors for - the "new_process" request in the protocol - ---- - dlls/kernel32/process.c | 2 ++ - server/process.c | 42 +++++++++++++++++++++++++++++------------- - server/protocol.def | 4 ++++ - 3 files changed, 35 insertions(+), 13 deletions(-) - -diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c -index 301c64a..66e4a31 100644 ---- a/dlls/kernel32/process.c -+++ b/dlls/kernel32/process.c -@@ -2034,6 +2034,8 @@ 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->thread_sd_size = 0; - req->info_size = startup_info_size; - - wine_server_add_data( req, startup_info, startup_info_size ); -diff --git a/server/process.c b/server/process.c -index 7b9a3b2..d7220e1 100644 ---- a/server/process.c -+++ b/server/process.c -@@ -880,6 +880,24 @@ DECL_HANDLER(new_process) - struct process *process; - struct process *parent = current->process; - int socket_fd = thread_get_inflight_fd( current, req->socket_fd ); -+ const startup_info_t *req_info; -+ data_size_t req_info_size; -+ const WCHAR *req_env; -+ data_size_t req_env_size; -+ -+ if (req->process_sd_size > get_req_data_size() || -+ req->thread_sd_size > get_req_data_size() - req->process_sd_size || -+ req->info_size > get_req_data_size() - req->process_sd_size - req->thread_sd_size) -+ { -+ close( socket_fd ); -+ return; -+ } -+ -+ req_info = (const startup_info_t *) -+ ((char*)get_req_data() + req->process_sd_size + req->thread_sd_size); -+ req_env = (const WCHAR *) -+ ((char*)get_req_data() + req->process_sd_size + req->thread_sd_size + req->info_size); -+ req_env_size = get_req_data_size() - (req->process_sd_size + req->thread_sd_size + req->info_size); - - if (socket_fd == -1) - { -@@ -920,27 +938,25 @@ DECL_HANDLER(new_process) - !(info->exe_file = get_file_obj( current->process, req->exe_file, FILE_READ_DATA ))) - goto done; - -- info->data_size = get_req_data_size(); -- info->info_size = min( req->info_size, info->data_size ); -- - if (req->info_size < sizeof(*info->data)) - { - /* make sure we have a full startup_info_t structure */ -- data_size_t env_size = info->data_size - info->info_size; -- data_size_t info_size = min( req->info_size, FIELD_OFFSET( startup_info_t, curdir_len )); -- -- if (!(info->data = mem_alloc( sizeof(*info->data) + env_size ))) goto done; -- memcpy( info->data, get_req_data(), info_size ); -- memset( (char *)info->data + info_size, 0, sizeof(*info->data) - info_size ); -- memcpy( info->data + 1, (const char *)get_req_data() + req->info_size, env_size ); -- info->info_size = sizeof(startup_info_t); -- info->data_size = info->info_size + env_size; -+ info->info_size = sizeof(*info->data); -+ info->data_size = sizeof(*info->data) + req_env_size; -+ -+ req_info_size = min( req->info_size, FIELD_OFFSET( startup_info_t, curdir_len )); -+ if (!(info->data = mem_alloc( info->data_size ))) goto done; -+ memset( info->data, 0, info->data_size ); -+ memcpy( info->data, req_info, req_info_size ); -+ memcpy( info->data + 1, req_env, req_env_size ); - } - else - { - data_size_t pos = sizeof(*info->data); -+ info->info_size = req->info_size; -+ info->data_size = req->info_size + req_env_size; - -- if (!(info->data = memdup( get_req_data(), info->data_size ))) goto done; -+ if (!(info->data = memdup( req_info, info->data_size ))) goto done; - #define FIXUP_LEN(len) do { (len) = min( (len), info->info_size - pos ); pos += (len); } while(0) - FIXUP_LEN( info->data->curdir_len ); - FIXUP_LEN( info->data->dllpath_len ); -diff --git a/server/protocol.def b/server/protocol.def -index c9270ea..dca98a4 100644 ---- a/server/protocol.def -+++ b/server/protocol.def -@@ -670,7 +670,11 @@ struct rawinput_device - unsigned int thread_access; /* access rights for thread object */ - unsigned int thread_attr; /* attributes for thread object */ - cpu_type_t cpu; /* CPU that the new process will use */ -+ data_size_t process_sd_size;/* size of the process security descriptor */ -+ data_size_t thread_sd_size; /* size of the thread security descriptor */ - data_size_t info_size; /* size of startup info */ -+ VARARG(process_sd,security_descriptor,process_sd_size); /* security descriptor to set on the process */ -+ VARARG(thread_sd,security_descriptor,thread_sd_size); /* security descriptor to set on the thread */ - VARARG(info,startup_info,info_size); /* startup information */ - VARARG(env,unicode_str); /* environment for new process */ - @REPLY --- -1.7.9.5 - diff --git a/patches/server-CreateProcess_ACLs/0002-server-Support-sending-thread-and-process-security-d.patch b/patches/server-CreateProcess_ACLs/0002-server-Support-sending-thread-and-process-security-d.patch new file mode 100644 index 00000000..51048201 --- /dev/null +++ b/patches/server-CreateProcess_ACLs/0002-server-Support-sending-thread-and-process-security-d.patch @@ -0,0 +1,128 @@ +From 9a9b0d8a21af0e88e1a0af4f32bcf10fabad3e5a Mon Sep 17 00:00:00 2001 +From: Sebastian Lackner +Date: Sat, 23 Aug 2014 05:58:30 +0200 +Subject: server: Support sending thread and process security descriptors in + new_process wineserver call. + +Based on a patch by Joris van der Wel. The original patch was removed since it contained several mistakes in validating untrusted length arguments. +--- + dlls/kernel32/process.c | 2 ++ + server/process.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++- + server/protocol.def | 6 +++++- + 3 files changed, 55 insertions(+), 2 deletions(-) + +diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c +index 301c64a..5de8b59 100644 +--- a/dlls/kernel32/process.c ++++ b/dlls/kernel32/process.c +@@ -2035,6 +2035,8 @@ 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->info_size = startup_info_size; ++ req->env_size = (env_end - env) * sizeof(WCHAR); ++ req->process_sd_size = 0; + + wine_server_add_data( req, startup_info, startup_info_size ); + wine_server_add_data( req, env, (env_end - env) * sizeof(WCHAR) ); +diff --git a/server/process.c b/server/process.c +index 7b9a3b2..426bcca 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 *process_sd = NULL, *thread_sd = NULL; + + if (socket_fd == -1) + { +@@ -920,7 +921,7 @@ DECL_HANDLER(new_process) + !(info->exe_file = get_file_obj( current->process, req->exe_file, FILE_READ_DATA ))) + goto done; + +- info->data_size = get_req_data_size(); ++ info->data_size = min( get_req_data_size(), req->info_size + req->env_size ); + info->info_size = min( req->info_size, info->data_size ); + + if (req->info_size < sizeof(*info->data)) +@@ -953,6 +954,31 @@ DECL_HANDLER(new_process) + #undef FIXUP_LEN + } + ++ /* validate security descriptors (if any) */ ++ if (get_req_data_size() > req->info_size + req->env_size) ++ { ++ data_size_t sd_size, pos = req->info_size + req->env_size; ++ if ((sd_size = min( get_req_data_size() - pos, req->process_sd_size ))) ++ { ++ process_sd = (const struct security_descriptor *)((const char *)get_req_data() + pos); ++ if (!sd_is_valid( process_sd, sd_size )) ++ { ++ set_error( STATUS_INVALID_SECURITY_DESCR ); ++ goto done; ++ } ++ pos += sd_size; ++ } ++ if ((sd_size = get_req_data_size() - pos)) ++ { ++ thread_sd = (const struct security_descriptor *)((const char *)get_req_data() + pos); ++ if (!sd_is_valid( thread_sd, sd_size )) ++ { ++ set_error( STATUS_INVALID_SECURITY_DESCR ); ++ goto done; ++ } ++ } ++ } ++ + if (!(thread = create_process( socket_fd, current, req->inherit_all ))) goto done; + process = thread->process; + process->debug_children = (req->create_flags & DEBUG_PROCESS) +@@ -1004,6 +1030,27 @@ 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 ); + ++ if (process_sd) ++ { ++ default_set_sd( &process->obj, ++ process_sd, ++ OWNER_SECURITY_INFORMATION | ++ GROUP_SECURITY_INFORMATION | ++ DACL_SECURITY_INFORMATION | ++ SACL_SECURITY_INFORMATION ); ++ } ++ ++ if (thread_sd) ++ { ++ set_sd_defaults_from_token( &thread->obj, ++ thread_sd, ++ OWNER_SECURITY_INFORMATION | ++ GROUP_SECURITY_INFORMATION | ++ DACL_SECURITY_INFORMATION | ++ SACL_SECURITY_INFORMATION, ++ process->token ); ++ } ++ + done: + release_object( info ); + } +diff --git a/server/protocol.def b/server/protocol.def +index c9270ea..3f75375 100644 +--- a/server/protocol.def ++++ b/server/protocol.def +@@ -671,8 +671,12 @@ struct rawinput_device + unsigned int thread_attr; /* attributes for thread object */ + cpu_type_t cpu; /* CPU that the new process will use */ + data_size_t info_size; /* size of startup info */ ++ data_size_t env_size; /* size of the environment */ ++ data_size_t process_sd_size;/* size of the process security descriptor */ + VARARG(info,startup_info,info_size); /* startup information */ +- VARARG(env,unicode_str); /* environment for new process */ ++ VARARG(env,unicode_str,env_size); /* environment for new process */ ++ VARARG(process_sd,security_descriptor,process_sd_size); /* security descriptor to set on the process */ ++ VARARG(thread_sd,security_descriptor); /* security descriptor to set on the thread */ + @REPLY + obj_handle_t info; /* new process info handle */ + process_id_t pid; /* process id */ +-- +1.7.9.5 + diff --git a/patches/server-CreateProcess_ACLs/0003-server-implement-passing-a-process-security-descript.patch b/patches/server-CreateProcess_ACLs/0003-kernel32-Implement-passing-a-process-security-descri.patch similarity index 55% rename from patches/server-CreateProcess_ACLs/0003-server-implement-passing-a-process-security-descript.patch rename to patches/server-CreateProcess_ACLs/0003-kernel32-Implement-passing-a-process-security-descri.patch index 7eab4c9d..13843792 100644 --- a/patches/server-CreateProcess_ACLs/0003-server-implement-passing-a-process-security-descript.patch +++ b/patches/server-CreateProcess_ACLs/0003-kernel32-Implement-passing-a-process-security-descri.patch @@ -1,21 +1,14 @@ -From 31d68ddd963e008e73e31c661556cd76b78da17e Mon Sep 17 00:00:00 2001 -From: Joris van der Wel -Date: Sun, 3 Aug 2014 12:52:32 +0200 -Subject: server: implement passing a process security descriptor to - CreateProcess +From bcf14e35900209c3177b76ae9b1e368aa12d58e6 Mon Sep 17 00:00:00 2001 +From: Sebastian Lackner +Date: Sat, 23 Aug 2014 06:27:28 +0200 +Subject: kernel32: Implement passing a process security descriptor from + CreateProcess to the wineserver. -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. +Based on a patch by Joris van der Wel. --- dlls/advapi32/tests/security.c | 3 -- - dlls/kernel32/process.c | 85 +++++++++++++++++++++++++++++++++++++++- - server/process.c | 24 ++++++++++++ - 3 files changed, 108 insertions(+), 4 deletions(-) + dlls/kernel32/process.c | 102 +++++++++++++++++++++++++++++++++++++++- + 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index b44496a..b1b35aa 100644 @@ -41,7 +34,7 @@ index b44496a..b1b35aa 100644 /* Documented privilege elevation */ diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c -index 66e4a31..65e6978 100644 +index 5de8b59..7d28140 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, @@ -119,121 +112,77 @@ index 66e4a31..65e6978 100644 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 *process_sd = NULL, *thread_sd = NULL; ++ data_size_t process_sd_size = 0, thread_sd_size = 0; if ((cpu = get_process_cpu( filename, binary_info )) == -1) { -@@ -1946,10 +2012,22 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW +@@ -1993,12 +2059,41 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW return FALSE; } -+ if (psa && (psa->nLength >= sizeof(*psa)) && psa->lpSecurityDescriptor) ++ if (psa && (psa->nLength >= sizeof(*psa))) + { -+ status = create_struct_sd( psa->lpSecurityDescriptor, &psd, &psd_len ); ++ status = create_struct_sd( psa->lpSecurityDescriptor, &process_sd, &process_sd_size ); + if (status != STATUS_SUCCESS) + { -+ WARN("Invalid process security descriptor with status %x\n", status); ++ close( socketfd[0] ); ++ close( socketfd[1] ); ++ WARN("Invalid process security descriptor: 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 d7220e1..2119a08 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; -@@ -893,6 +894,16 @@ DECL_HANDLER(new_process) - return; - } - -+ if (req->process_sd_size) ++ if (tsa && (tsa->nLength >= sizeof(*tsa))) + { -+ req_psd = get_req_data(); -+ if (!sd_is_valid( req_psd, req->process_sd_size )) ++ status = create_struct_sd( tsa->lpSecurityDescriptor, &thread_sd, &thread_sd_size ); ++ if (status != STATUS_SUCCESS) + { -+ set_error( STATUS_INVALID_SECURITY_DESCR ); -+ return; ++ RtlFreeHeap(GetProcessHeap(), 0, process_sd); ++ close( socketfd[0] ); ++ close( socketfd[1] ); ++ WARN("Invalid thread security descriptor: Status %x\n", status); ++ SetLastError( RtlNtStatusToDosError(status) ); ++ return FALSE; + } + } + - req_info = (const startup_info_t *) - ((char*)get_req_data() + req->process_sd_size + req->thread_sd_size); - req_env = (const WCHAR *) -@@ -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 ); + RtlAcquirePebLock(); -+ /* note: alloc_handle might fail with access denied -+ * if the security descriptor is set before that call */ + if (!(startup_info = create_startup_info( filename, cmd_line, cur_dir, env, flags, startup, + &startup_info_size ))) + { + RtlReleasePebLock(); ++ RtlFreeHeap(GetProcessHeap(), 0, process_sd); ++ RtlFreeHeap(GetProcessHeap(), 0, thread_sd); + close( socketfd[0] ); + close( socketfd[1] ); + return FALSE; +@@ -2036,10 +2131,12 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW + req->cpu = cpu; + req->info_size = startup_info_size; + req->env_size = (env_end - env) * sizeof(WCHAR); +- req->process_sd_size = 0; ++ req->process_sd_size = process_sd_size; + + wine_server_add_data( req, startup_info, startup_info_size ); + wine_server_add_data( req, env, (env_end - env) * sizeof(WCHAR) ); ++ wine_server_add_data( req, process_sd, process_sd_size ); ++ wine_server_add_data( req, thread_sd, thread_sd_size ); + if (!(status = wine_server_call( req ))) + { + info->dwProcessId = (DWORD)reply->pid; +@@ -2052,6 +2149,9 @@ static BOOL create_process( HANDLE hFile, LPCWSTR filename, LPWSTR cmd_line, LPW + SERVER_END_REQ; + + RtlReleasePebLock(); ++ RtlFreeHeap(GetProcessHeap(), 0, process_sd); ++ RtlFreeHeap(GetProcessHeap(), 0, thread_sd); + -+ 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 ); - } + if (status) + { + switch (status) -- 1.7.9.5 diff --git a/patches/server-CreateProcess_ACLs/0004-advapi32-tests-Add-additional-tests-for-passing-a-th.patch b/patches/server-CreateProcess_ACLs/0004-advapi32-tests-Add-additional-tests-for-passing-a-th.patch new file mode 100644 index 00000000..0f164280 --- /dev/null +++ b/patches/server-CreateProcess_ACLs/0004-advapi32-tests-Add-additional-tests-for-passing-a-th.patch @@ -0,0 +1,96 @@ +From 3c5ee362799eef25543ef8a9787d97fcf8cef304 Mon Sep 17 00:00:00 2001 +From: Joris van der Wel +Date: Sun, 3 Aug 2014 12:52:44 +0200 +Subject: advapi32/tests: Add additional tests for passing a thread sd to + CreateProcess. + +--- + dlls/advapi32/tests/security.c | 44 ++++++++++++++++++++++++++++++++++++---- + 1 file changed, 40 insertions(+), 4 deletions(-) + +diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c +index b1b35aa..eb9b8cb 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) +-- +1.7.9.5 + diff --git a/patches/server-CreateProcess_ACLs/0004-server-implement-passing-a-thread-security-descripto.patch b/patches/server-CreateProcess_ACLs/0004-server-implement-passing-a-thread-security-descripto.patch deleted file mode 100644 index bd0cece9..00000000 --- a/patches/server-CreateProcess_ACLs/0004-server-implement-passing-a-thread-security-descripto.patch +++ /dev/null @@ -1,228 +0,0 @@ -From c4b089e56ea5ace923a69428c1a96c838e94a2aa Mon Sep 17 00:00:00 2001 -From: Joris van der Wel -Date: Sun, 3 Aug 2014 12:52:44 +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..eb9b8cb 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 65e6978..f2d11ba 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 2119a08..c0b82d1 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.7.9.5 - diff --git a/patches/server-CreateProcess_ACLs/definition b/patches/server-CreateProcess_ACLs/definition index 72e68063..45245156 100644 --- a/patches/server-CreateProcess_ACLs/definition +++ b/patches/server-CreateProcess_ACLs/definition @@ -1,4 +1,4 @@ -Author: Joris van der Wel +Author: Joris van der Wel / Sebastian Lackner Subject: Implement passing ACLs to CreateProcess. -Revision: 1 +Revision: 2 Fixes: [22006] Support for process ACLs