[PATCH 0/1] MR6602: win32u: Always launch explorer for the desktop with a default token.
Testing on Windows shows that the owner of GetDesktopWindow() always has a default admin token with `TokenElevationTypeDefault`, no matter the token of the process/thread that is responsible for creating it. We've had issues in this area in the past - 99e2fad1 was a case where it was important that explorer not inherit the token of the *process* spawning it, but instead the token of the *thread*. This patch keeps that app working, since now explorer will get a default token regardless. In addition to the privilege issues from 99e2fad1, it is a relatively common pattern to duplicate the token of the owner of GetDesktopWindow to acquire a default token. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6602
From: Tim Clem <tclem(a)codeweavers.com> --- dlls/win32u/winstation.c | 17 ++++++++++++++++- server/protocol.def | 8 ++++++++ server/token.c | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/winstation.c b/dlls/win32u/winstation.c index d2386fd5af5..8df1c09a7eb 100644 --- a/dlls/win32u/winstation.c +++ b/dlls/win32u/winstation.c @@ -771,6 +771,7 @@ HWND get_desktop_window(void) WCHAR desktop[MAX_PATH]; PEB *peb = NtCurrentTeb()->Peb; HANDLE process, thread; + HANDLE token; unsigned int status; SERVER_START_REQ( set_user_object_info ) @@ -789,6 +790,19 @@ HWND get_desktop_window(void) } SERVER_END_REQ; + SERVER_START_REQ( create_primary_admin_token ) + { + req->elevation = TokenElevationTypeDefault; + if (!wine_server_call( req )) + token = wine_server_ptr_handle( reply->token ); + else + { + WARN( "couldn't get a default token; starting explorer with current thread's\n" ); + token = GetCurrentThreadEffectiveToken(); + } + } + SERVER_END_REQ; + params.Flags = PROCESS_PARAMS_FLAG_NORMALIZED; params.Environment = peb->ProcessParameters->Environment; params.EnvironmentSize = peb->ProcessParameters->EnvironmentSize; @@ -806,7 +820,7 @@ HWND get_desktop_window(void) ps_attr->Attributes[1].Attribute = PS_ATTRIBUTE_TOKEN; ps_attr->Attributes[1].Size = sizeof(HANDLE); - ps_attr->Attributes[1].ValuePtr = GetCurrentThreadEffectiveToken(); + ps_attr->Attributes[1].ValuePtr = token; ps_attr->Attributes[1].ReturnLength = NULL; ps_attr->TotalLength = offsetof( PS_ATTRIBUTE_LIST, Attributes[2] ); @@ -823,6 +837,7 @@ HWND get_desktop_window(void) status = NtCreateUserProcess( &process, &thread, PROCESS_ALL_ACCESS, THREAD_ALL_ACCESS, NULL, NULL, 0, THREAD_CREATE_FLAGS_CREATE_SUSPENDED, ¶ms, &create_info, ps_attr ); + NtClose( token ); if (!status) { NtResumeThread( thread, NULL ); diff --git a/server/protocol.def b/server/protocol.def index f2db46bd87d..74858083dcc 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3279,6 +3279,14 @@ enum caret_state @END +/* Create a new primary token with the default admin properties. */ +(a)REQ(create_primary_admin_token) + int elevation; /* elevation type */ +(a)REPLY + obj_handle_t token; /* handle to the token */ +(a)END + + /* Open a security token */ @REQ(open_token) obj_handle_t handle; /* handle to the thread or process */ diff --git a/server/token.c b/server/token.c index 48ee1eca8fe..94281ff699a 100644 --- a/server/token.c +++ b/server/token.c @@ -1209,6 +1209,31 @@ DECL_HANDLER(create_token) } +/* create a new default, primary token with the given elevation */ +DECL_HANDLER(create_primary_admin_token) +{ + struct token *token; + + if (req->elevation != TokenElevationTypeDefault && + req->elevation != TokenElevationTypeFull && + req->elevation != TokenElevationTypeLimited) + { + set_error( STATUS_INVALID_PARAMETER ); + return; + } + + token = token_create_admin( TRUE, SecurityIdentification, req->elevation, default_session_id ); + if (!token) + { + set_error( STATUS_UNSUCCESSFUL ); + return; + } + + reply->token = alloc_handle( current->process, token, TOKEN_ALL_ACCESS, 0 ); + release_object( token ); +} + + /* open a security token */ DECL_HANDLER(open_token) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6602
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148807 Your paranoid android. === debian11b (64 bit WoW report) === d3d9: d3d9ex.c:3230: Test failed: Expected message 0x18 for window 0, but didn't receive it, i=0.
For what it's worth, it would help to do the same when launching services.exe from wineboot. At the moment (ultimately via e92ba2de43d7afbe0704b11b29f7c30f44dfaeca), it gets a limited token, which all services inherit. Some services (GOG again) get quite upset if they don't have an admin token. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6602#note_84097
participants (3)
-
Marvin -
Tim Clem -
Tim Clem (＠tclem)