[PATCH v7 0/5] MR9843: wtsapi32: Fix session id and desktop handling when using CreateProcessAsUserW with tokens from WTSQueryUserToken
This Merge request resolves [Wine-Bug #58588](https://bugs.winehq.org/show_bug.cgi?id=58588). The underlying problem was that services started on their own, non-interactive winstation, but within the current user's session. This is trivially not the case for system services on Windows: they run, by design, in other user contexts (e.g. NTAUTHORITY\SYSTEM). Whenever a service wants to start a process within another user's graphical session, it needs to set the session id to the correct value. This may be done using the WTS API. This MR implements this functionality in a (hopefully) wine-compatible way: 1. Patch wineserver so tokens can be transferred to other sessions 2. Implement NtSetInformationToken for TokenSessionId 3. Change the session of non-interactive services to session 0 (from the default session 1) in addition to setting their desktop to `__wineservice_winstation\Default` 4. Let CreateProcessAsUserW reset the desktop to the default value `__winsta0\Default` if the session of the to-be-created process is not a service session. This avoids implementing all user and session handling in-depth while still allowing the underlying functionality to work. A real-world user of this mechanism is the Dassault 3DEXPERIENCE launcher, which runs as a system service and wants to open interactive child processes within a user's scope. -- v7: kernelbase: Let CreateProcessInternalW overwrite the desktop if needed https://gitlab.winehq.org/wine/wine/-/merge_requests/9843
From: Katharina Bogad <katharina@hacked.xyz> Add a server protocol message to set session ids for process tokens. --- server/protocol.def | 5 +++++ server/token.c | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/server/protocol.def b/server/protocol.def index 0ac64297026..9f210a8504d 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3550,6 +3550,11 @@ enum caret_state VARARG(acl,acl); /* default dacl to set */ @END +@REQ(set_token_session_id) + obj_handle_t handle; /* handle to the token */ + unsigned int session_id; /* session_id the token should become */ +@END + @REQ(set_security_object) obj_handle_t handle; /* handle to the object */ unsigned int security_info; /* which parts of security descriptor to set */ diff --git a/server/token.c b/server/token.c index 5ce7298211e..b8a214fd21e 100644 --- a/server/token.c +++ b/server/token.c @@ -1602,6 +1602,20 @@ DECL_HANDLER(set_token_default_dacl) } } +DECL_HANDLER(set_token_session_id) +{ + struct token *token; + + if((token = (struct token *)get_handle_obj( current->process, req->handle, + TOKEN_ADJUST_DEFAULT, + &token_ops))) + { + token->session_id = req->session_id; + + release_object( token ); + } +} + DECL_HANDLER(create_linked_token) { struct token *token, *linked; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9843
From: Katharina Bogad <katharina@hacked.xyz> --- dlls/ntdll/unix/security.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/security.c b/dlls/ntdll/unix/security.c index 949af82f783..aff8763ee75 100644 --- a/dlls/ntdll/unix/security.c +++ b/dlls/ntdll/unix/security.c @@ -702,7 +702,7 @@ NTSTATUS WINAPI NtSetInformationToken( HANDLE token, TOKEN_INFORMATION_CLASS cla break; case TokenSessionId: - if (length < sizeof(DWORD)) + if (length < sizeof(ULONG)) { ret = STATUS_INFO_LENGTH_MISMATCH; break; @@ -712,7 +712,13 @@ NTSTATUS WINAPI NtSetInformationToken( HANDLE token, TOKEN_INFORMATION_CLASS cla ret = STATUS_ACCESS_VIOLATION; break; } - FIXME("TokenSessionId stub!\n"); + SERVER_START_REQ( set_token_session_id ) + { + req->handle = wine_server_obj_handle( token ); + req->session_id = *((ULONG *)info); + ret = wine_server_call(req); + } + SERVER_END_REQ; ret = STATUS_SUCCESS; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9843
From: Katharina Bogad <katharina@hacked.xyz> On Windows, WTSQueryUserToken called with a session_id returns a token that is in that user's current session and winstation. Since WINE lacks handling for multiple users, make WTSQueryUserToken return the duplicated token with the requested session_id set. --- dlls/wtsapi32/wtsapi32.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/wtsapi32/wtsapi32.c b/dlls/wtsapi32/wtsapi32.c index 5a1a87f416e..0a2e25bb9cd 100644 --- a/dlls/wtsapi32/wtsapi32.c +++ b/dlls/wtsapi32/wtsapi32.c @@ -653,9 +653,13 @@ BOOL WINAPI WTSQueryUserToken(ULONG session_id, PHANDLE token) return FALSE; } - return DuplicateHandle(GetCurrentProcess(), GetCurrentProcessToken(), + if(!DuplicateHandle(GetCurrentProcess(), GetCurrentProcessToken(), GetCurrentProcess(), token, - 0, FALSE, DUPLICATE_SAME_ACCESS); + 0, FALSE, DUPLICATE_SAME_ACCESS)) + return FALSE; + + + return SetTokenInformation(*token, TokenSessionId, &session_id, sizeof(ULONG)); } /************************************************************ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9843
From: Katharina Bogad <katharina@hacked.xyz> Services run in a non-interactive session by default; this session is also not enumeratable in WTSEnumerateSessions. Previously, the first part was handled by overwriting the winstation to __wineservice_winstation\Default while leaving the session id to the default 1. Change the default service session to session id 0 when starting a service and fixup WTSEnumerateSessions to never return the service session id. --- dlls/wtsapi32/wtsapi32.c | 5 +++++ programs/services/services.c | 21 +++++++++++++++++---- server/user.h | 1 + server/winstation.c | 6 ++++-- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/dlls/wtsapi32/wtsapi32.c b/dlls/wtsapi32/wtsapi32.c index 0a2e25bb9cd..3db47731945 100644 --- a/dlls/wtsapi32/wtsapi32.c +++ b/dlls/wtsapi32/wtsapi32.c @@ -364,6 +364,11 @@ BOOL WINAPI WTSEnumerateSessionsW(HANDLE server, DWORD reserved, DWORD version, WTSFreeMemory(*session_info); return FALSE; } + if ((*session_info)->SessionId == 0) + { + FIXME("replacing service session with default\n"); + (*session_info)->SessionId = 1 /* default_session_id, server/process.h */; + } *count = 1; (*session_info)->State = WTSActive; (*session_info)->pWinStationName = (WCHAR *)((char *)*session_info + sizeof(**session_info)); diff --git a/programs/services/services.c b/programs/services/services.c index dff3cd60040..933e8bb2689 100644 --- a/programs/services/services.c +++ b/programs/services/services.c @@ -918,7 +918,7 @@ static DWORD service_start_process(struct service_entry *service_entry, struct p BOOL is_wow64 = FALSE; HANDLE token; WCHAR *path; - DWORD err; + DWORD err, service_session_id = 0; BOOL r; service_lock(service_entry); @@ -1028,7 +1028,10 @@ found: si.lpDesktop = (WCHAR *)L"__wineservice_winstation\\Default"; } - if (!environment && OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY | TOKEN_DUPLICATE, &token)) + r = DuplicateHandle(GetCurrentProcess(), GetCurrentProcessToken(), + GetCurrentProcess(), &token, 0, FALSE, DUPLICATE_SAME_ACCESS); + TRACE("DuplicateHandle %u %lu\n", r, GetLastError()); + if (!environment && r ) { WCHAR val[16]; CreateEnvironmentBlock(&environment, token, FALSE); @@ -1040,7 +1043,6 @@ found: RtlInitUnicodeString( &value, val ); RtlSetEnvironmentVariable( (WCHAR **)&environment, &name, &value ); } - CloseHandle(token); } service_entry->status.dwCurrentState = SERVICE_START_PENDING; @@ -1053,8 +1055,19 @@ found: process->use_count++; service_unlock(service_entry); - r = CreateProcessW(NULL, path, NULL, NULL, FALSE, CREATE_UNICODE_ENVIRONMENT | DETACHED_PROCESS, environment, NULL, &si, &pi); + r = SetTokenInformation(token, TokenSessionId, &service_session_id, sizeof(service_session_id)); + + if(!r) + { + err = GetLastError(); + process_terminate(process); + release_process(process); + return err; + } + + r = CreateProcessAsUserW(token, NULL, path, NULL, NULL, FALSE, CREATE_UNICODE_ENVIRONMENT | DETACHED_PROCESS, environment, NULL, &si, &pi); free(path); + CloseHandle(token); if (!r) { err = GetLastError(); diff --git a/server/user.h b/server/user.h index 9f6c3dcaf0f..d2063f19aa9 100644 --- a/server/user.h +++ b/server/user.h @@ -49,6 +49,7 @@ struct winstation unsigned int monitor_count; /* number of monitors */ struct monitor_info *monitors; /* window station monitors */ unsigned __int64 monitor_serial; /* winstation monitor update counter */ + unsigned int session_id; /* session id this winstation belongs to */ }; struct key_repeat diff --git a/server/winstation.c b/server/winstation.c index a36253e20db..f6793f3ebba 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -136,7 +136,7 @@ static const struct object_ops desktop_ops = /* create a winstation object */ static struct winstation *create_winstation( struct object *root, const struct unicode_str *name, - unsigned int attr, unsigned int flags ) + unsigned int attr, unsigned int flags, unsigned int session_id ) { struct winstation *winstation; @@ -152,6 +152,7 @@ static struct winstation *create_winstation( struct object *root, const struct u winstation->monitors = NULL; winstation->monitor_count = 0; winstation->monitor_serial = 1; + winstation->session_id = session_id; list_add_tail( &winstation_list, &winstation->entry ); list_init( &winstation->desktops ); if (!(winstation->desktop_names = create_namespace( 7 ))) @@ -612,7 +613,7 @@ DECL_HANDLER(create_winstation) reply->handle = 0; if (req->rootdir && !(root = get_directory_obj( current->process, req->rootdir ))) return; - if ((winstation = create_winstation( root, &name, req->attributes, req->flags ))) + if ((winstation = create_winstation( root, &name, req->attributes, req->flags, current->process->session_id ))) { reply->handle = alloc_handle( current->process, winstation, req->access, req->attributes ); release_object( winstation ); @@ -987,6 +988,7 @@ DECL_HANDLER(enum_winstation) unsigned int access = WINSTA_ENUMERATE; if (!(name = winstation->obj.name)) continue; if (!check_object_access( NULL, &winstation->obj, &access )) continue; + if (current->process->session_id != winstation->session_id) continue; reply->count++; reply->total += name->len + sizeof(WCHAR); if (reply->total <= size) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9843
From: Katharina Bogad <katharina@hacked.xyz> Calling CreateProcessAsUserW with a token that has session_id != 0 set should result in the program being started on the token's assoiciated desktop. Currently, WINE doesn't handle multiple users and multiple winstations, so for the moment, if the associated token has a session_id != 0, overwrite the desktop to the default value. This allows serivces started with session_id 0 running non-interactively to get a session_id from wtsapi32 belonging to an interactive session and use CreateProcessAsUserW to open child processes in an interactive environment. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58588 --- dlls/kernelbase/process.c | 27 ++++++++++++++++++++++++++- server/winstation.c | 19 ++++++++++++++++--- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 3656e40280d..d7cd32a4944 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -519,6 +519,11 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR ULONG nt_flags = 0; USHORT machine = 0; NTSTATUS status; + DWORD dwSessionId = 0; + DWORD dwReturnLength; + STARTUPINFOW local_startup_info; + BOOL success; + /* Process the AppName and/or CmdLine to get module name and path */ @@ -541,6 +546,26 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR app_name = name; } + /* Fixup startupinfo->lpDesktop if needed */ + + memcpy(&local_startup_info, startup_info, sizeof(STARTUPINFOW)); + if(token != NULL) + { + success = GetTokenInformation(token, TokenSessionId, &dwSessionId, sizeof(dwSessionId), &dwReturnLength); + + TRACE("success = %u, dwSessionId = %lu\n", success, dwSessionId); + + + if (success && dwSessionId != 0) + { + /* the created process should be attached to an interactive session. + * So we override the winstation for this process. */ + FIXME( "Child process will be started with default desktop\n"); + + local_startup_info.lpDesktop = (WCHAR*)L"winsta0\\default"; + } + } + /* Warn if unsupported features are used */ if (flags & (IDLE_PRIORITY_CLASS | HIGH_PRIORITY_CLASS | REALTIME_PRIORITY_CLASS | @@ -560,7 +585,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH CreateProcessInternalW( HANDLE token, const WCHAR info->hThread = info->hProcess = 0; info->dwProcessId = info->dwThreadId = 0; - if (!(params = create_process_params( app_name, tidy_cmdline, cur_dir, env, flags, startup_info ))) + if (!(params = create_process_params( app_name, tidy_cmdline, cur_dir, env, flags, &local_startup_info ))) { status = STATUS_NO_MEMORY; goto done; diff --git a/server/winstation.c b/server/winstation.c index f6793f3ebba..a9dde71930e 100644 --- a/server/winstation.c +++ b/server/winstation.c @@ -500,12 +500,13 @@ void set_process_default_desktop( struct process *process, struct desktop *deskt void connect_process_winstation( struct process *process, struct unicode_str *desktop_path, struct thread *parent_thread, struct process *parent_process ) { - struct unicode_str desktop_name = *desktop_path, winstation_name = {0}; + struct unicode_str desktop_name = *desktop_path, winstation_name = {0}, root_name = {0}, full_winstation_name = {0}; const int attributes = OBJ_CASE_INSENSITIVE | OBJ_OPENIF; struct winstation *winstation = NULL; struct desktop *desktop = NULL; const WCHAR *wch, *end; obj_handle_t handle; + char ascii_root[50]; /* len of root path + len(str(2**32)) + 4 (to be safe) */ for (wch = desktop_name.str, end = wch + desktop_name.len / sizeof(WCHAR); wch != end; wch++) { @@ -524,9 +525,20 @@ void connect_process_winstation( struct process *process, struct unicode_str *de { winstation = (struct winstation *)get_handle_obj( process, handle, 0, &winstation_ops ); } - else if (winstation_name.len && (winstation = open_named_object( NULL, &winstation_ops, &winstation_name, attributes ))) + else if (winstation_name.len) { - handle = alloc_handle( process, winstation, STANDARD_RIGHTS_REQUIRED | WINSTA_ALL_ACCESS, 0 ); + snprintf(ascii_root, sizeof(ascii_root), "\\Sessions\\%u\\Windows\\WindowStations\\", process->session_id); + ascii_to_unicode_str(ascii_root, &root_name); + + full_winstation_name.len = root_name.len + winstation_name.len; + full_winstation_name.str = malloc(full_winstation_name.len + 2); + memcpy((void*)full_winstation_name.str, root_name.str, root_name.len); + memcpy((void*)(full_winstation_name.str + root_name.len / sizeof(WCHAR)), winstation_name.str, winstation_name.len); + + if((winstation = open_named_object( NULL, &winstation_ops, &full_winstation_name, attributes ))) + { + handle = alloc_handle( process, winstation, STANDARD_RIGHTS_REQUIRED | WINSTA_ALL_ACCESS, 0 ); + } } else if (parent_process->winstation) { @@ -563,6 +575,7 @@ void connect_process_winstation( struct process *process, struct unicode_str *de } if (handle) set_process_default_desktop( process, desktop, handle ); + done: if (desktop) release_object( desktop ); if (winstation) release_object( winstation ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9843
Thanks for the patches! I'd merge 1/5 and 2/5 together; new server calls are usually done that way. 3/5 could use a test. Also, is there an application that wants it? The wtsapi32 change in 4/5 seems suspect; what needs this? The server change in 4/5 seems like a separate change that should be split. Dealing with 5/5 by changing kernelbase seems wrong; I would think we should just find the correct desktop from the token in the server. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9843#note_126393
Thank you for your feedback! Re: merging 1/5 and 2/5 - yeah, will do. Re: 3/5 - I have a reproducer for the underlying bug; WTSQueryUserToken should return a token in the user's context when run from a Service. This is also related to the changes in 4/5. Beginning with Vista, services are moved to session 0 - see [this change announcement from Microsoft](https://learn.microsoft.com/en-us/previous-versions/bb756986(v=msdn.10)?redi...). This session is non-interactive, which WINE currently can't express. Personally, I think this is better left as a FIXME at the moment. Re: 5/5 - Muddling around in the supplied startupinfo structure isn't really ideal, I agree. The correct way to do this would be to make connect_process_windowstation aware of the new processes' session_id and reset the windowstation to the first station of this session, if the parent process is in another session. AFAICT, there is currently no clear way of enumerating directories on the server side, so this would currently run into the same issue as my also very hacky off-brand wchar strcat. All of these changes are needed to run the Dassault Systems 3DEXPERIENCE Launcher. This is a program / cloud connector for the 3D CAD Program SolidWorks. They implement three components; A service running under NTAUTHORITY\SYSTEM using WTSEnumerateSessions to find the currently active sessions, using the session handle to obtain the User's Token from WTSQueryUserToken, and runs a program in the users's graphical session with CreateProcessAsUserW. This necessitates moving services to session 0 (as documented also the case in Windows), since the interactive user session without RDP involved is session 1. The rest is more or less fixing the mess that this creates: The non-interactive session should not be reported from WTSEnumerateSessions; instead it needs to report the correct interactive user session so CreateProcessAsUser / whatever comes later can find the correct winstation and desktop for the session. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9843#note_126396
participants (3)
-
Elizabeth Figura (＠zfigura) -
Katharina Bogad
-
Katharina Bogad (＠hackathi)