Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using ConPTY with CreateProcessWithLogon and CreateProcessWithToken #11865

Open
sajagi opened this issue Dec 2, 2021 · 4 comments
Open

Using ConPTY with CreateProcessWithLogon and CreateProcessWithToken #11865

sajagi opened this issue Dec 2, 2021 · 4 comments
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@sajagi
Copy link

sajagi commented Dec 2, 2021

The documentation only covers same-user scenario with CreateProcess function. There is no mention whether CreatePseudoConsole also works with CreateProcessWithLogon, CreateProcessWithToken and similar functions.

It seems as if this scenario is not supported. CreateProcessWithLogon fails with ERROR_INVALID_PARAMETER (87) message. However, given how cryptic the message is, the underlying issue might be also elsewhere.

@sajagi sajagi added the Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs label Dec 2, 2021
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Dec 2, 2021
@zadjii-msft
Copy link
Member

You know, we wrote CreatePseudoConsoleAsUser

extern "C" HRESULT ConptyCreatePseudoConsoleAsUser(_In_ HANDLE hToken,

a while ago, but I don't think we ever added it to the public SDK. We should probably do that.

(this might also be an argument in favor of the conpty nuget package, #3577/#1130 so folks don't need to rely on SDK version updates)

@sajagi
Copy link
Author

sajagi commented Dec 2, 2021

Thanks for the quick reply.

Do I read it correctly that such scenario is then not supported and the only way to host another user's process in pseudoconsole is to call CreatePseudoConsole and CreateProcess from a process already running under that user?

@zadjii-msft
Copy link
Member

Right now, yea, it's not technically supported. All the code is there and ready to be used, we just never exposed the public function signature for it (mostly because we just forgot to).

If we do ever get around to shipping a conpty nuget, then we could ship the updated API surface out-of-band, without needing to wait for the OS to update. That might be a quicker and easier solution than trying to get this exposed in the OS.

@zadjii-msft zadjii-msft added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conpty For console issues specifically related to conpty and removed Issue-Docs It's a documentation issue that really should be on MicrosoftDocs/Console-Docs labels Dec 2, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 2, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Dec 2, 2021
@eryksun
Copy link

eryksun commented Dec 2, 2021

The linked code just calls CreateProcessAsUserW() to spawn the console host. But CreateProcessWithLogonW() and CreateProcessWithTokenW() are delegated to the Secondary Logon service (seclogon). Delegation is required in order to allow an unprivileged user to spawn a process as another user (e.g. via runas.exe).

Unless the new token is related to the caller's token (e.g. a restricted token, or UWP app token), directly calling CreateProcessAsUserW() requires SeAssignPrimaryTokenPrivilege and possibly SeIncreaseQuotaPrivilege. By default, even administrators don't have SeAssignPrimaryTokenPrivilege.

Also, for CreateProcessWithLogonW(), the Secondary Logon service enables access to the client's interactive window station and desktop by adding the client's logon-session group to the new token. (A logon-session group has the attribute SE_GROUP_LOGON_ID.) Without this access, the spawned process (e.g. conhost.exe) will fail with STATUS_DLL_INIT_FAILED (0xC0000142) when user32.dll loads, since it tries and fails to open the window station and desktop. Creating a token with the client's logon-session group requires calling LsaLogonUser() or LogonUserExExW() with SeTcbPrivilege (act as part of the OS) granted and enabled, which is why seclogon runs as a SYSTEM service. The alternative is to open the client's window station and desktop objects to add a DACL entry that grants all access to the new logon-session group.

@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@carlos-zamora carlos-zamora removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

4 participants