Rewrite part of server-CreateProcess_ACLs in order to fix incorrect error codes and memory leaks.

This commit is contained in:
Sebastian Lackner 2014-08-23 06:33:01 +02:00
parent bba9e118d9
commit 26b0797bcb
7 changed files with 291 additions and 458 deletions

View File

@ -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

View File

@ -1,112 +0,0 @@
From 38c3a1a50ca4bdef0b5ec0cf120fd5da889954dd Mon Sep 17 00:00:00 2001
From: Joris van der Wel <joris@jorisvanderwel.com>
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

View File

@ -0,0 +1,128 @@
From 9a9b0d8a21af0e88e1a0af4f32bcf10fabad3e5a Mon Sep 17 00:00:00 2001
From: Sebastian Lackner <sebastian@fds-team.de>
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

View File

@ -1,21 +1,14 @@
From 31d68ddd963e008e73e31c661556cd76b78da17e Mon Sep 17 00:00:00 2001
From: Joris van der Wel <joris@jorisvanderwel.com>
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 <sebastian@fds-team.de>
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

View File

@ -0,0 +1,96 @@
From 3c5ee362799eef25543ef8a9787d97fcf8cef304 Mon Sep 17 00:00:00 2001
From: Joris van der Wel <joris@jorisvanderwel.com>
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

View File

@ -1,228 +0,0 @@
From c4b089e56ea5ace923a69428c1a96c838e94a2aa Mon Sep 17 00:00:00 2001
From: Joris van der Wel <joris@jorisvanderwel.com>
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

View File

@ -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