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

Jump to bottom

syscall: StartProcess should accept Desktop property for StartupInfo for Windows #42836

Open
iangallOlive opened this issue Nov 25, 2020 · 5 comments
Open

syscall: StartProcess should accept Desktop property for StartupInfo for Windows #42836

iangallOlive opened this issue Nov 25, 2020 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Unplanned

Comments

@iangallOlive
Copy link

iangallOlive commented Nov 25, 2020 •
edited

Feature Request

Purpose

This is meant to be a feature request. I'm trying to gauge whether people agree this would be an acceptable/beneficial addition, and if the team would be willing to work on it or if it would be generally accepted by an outside contributor (me) if a PR was submitted.

Problem Statement

To give a quick context: os/exec's Command's Run func calls os.StartProcess, which ends up calling syscall.StartProcess.

For Windows, syscall.StartProcess calls syscall.CreateProcess (or syscall.CreateProcessAsUser), which calls the CreateProcessW Win32 API method.

CreateProcessW accepts a STARTUP_INFOW param, which is nicely constructed in the syscall.StartProcess func via a syscall.StartupInfo. One of STARTUP_INFOW's properties is a LPWSTR lpDesktop. Unfortunately, nothing around syscall.StartProcess allows for this to be provided or set on the underlying syscall.StartupInfo, even though that struct already has the property defined.

I've had to generally rewrite the overall logic of syscall.StartProcess in my company's application because we need to be able to set the lpDesktop property. It would be nice to have it part of Go so we can use os/exec's Command.

Proposed Solution

Of the syscall.StartProcess func, In the attr (*syscall.ProcAttr) param, there is a Sys (*syscall.SysProcAttr) property. I think a Desktop string property should be added there. This will be an optional value and only used in the syscall.StartupInfo if it's provided, which should make it fully backwards compatible.

So, in syscall.StartProcess, something like the following can be done:

var desktopp *uint16
if len(sys.Desktop) != 0 {
	desktopp, err = UTF16PtrFromString(sys.Desktop)
	if err != nil {
		return 0, 0, err
	}
}

and then used like this:

si.Desktop = desktopp

and I think that should be it!
@ianlancetaylor
Copy link
Contributor

CC @alexbrainman @mattn @zx2c4

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows labels Nov 26, 2020
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Nov 26, 2020
@zx2c4
Copy link
Contributor

zx2c4 commented Nov 26, 2020

Seems reasonable to me and easy to implement.

One thing I'm wondering is: what are you using this parameter for and why? Usually it's possible to get a token to an interactive user with the desktop, and simply pass that token using the existing mechanism for that. Is there a reason why that doesn't work for you?

@networkimprov
Copy link

ping @iangallOlive re above Q

@iangallOlive
Copy link
Author

Apologies for not responding very quickly at all! But appreciate the ping.

@zx2c4 You're absolutely right, and I think we're already doing that - but as far I can remember (because it's been awhile since we implemented this...I'm only now circling back to see if the concept could be added to Go), if you don't set this property, it doesn't matter where the token is from. I could be wrong, but I feel like this was true.

Just to give more context, here's some more info:

  • The app is running as a Windows service (running as the Local System user).
  • We use SendSAS to bring up the login desktop (screen), then fill in the username/password fields with specific creds we know of. This effectively logs the user in and gives them an interactive desktop (which is the whole point - we need a GUI).
  • Then we get the "explorer.exe" process (OpenProcess), get its token (OpenProcessToken), duplicate it (DuplicateTokenEx), load the user profile (LoadUserProfileW), create an environment block (CreateEnvironmentBlock), and eventually create the child process (CreateProcessAsUserW). And obviously we set the StartupInfo's Desktop property to "winsta0\default".

I know we went through several iterations trying to figure out this whole concept, some of which I wasn't involved in.

Please let me know if there's something I'm missing, that we could be doing to avoid the need to set the StartupInfo's Desktop property. And please let me know if I can clarify anything more, or try anything, or do whatever.

@zx2c4
Copy link
Contributor

zx2c4 commented Dec 7, 2020

If you're stealing a token from explorer.exe, you shouldn't need to set desktop, afaik. But actually there's a more robust way of doing this from a service:

You can use windows.WTSEnumerateSessions to enumerate current sessions and use the svc.SessionChange event from the service's <-chan svc.ChangeRequest channel argument to get notifications of new sessions that come up. The former returns an array of windows.WTS_SESSION_INFO objects. The latter raises a WTS_SESSION_LOGON and you can then cast EventData to windows.WTSSESSION_NOTIFICATION. Both windows.WTS_SESSION_INFO and windows.WTSSESSION_NOTIFICATION have a field called SessionID, which is the magic ID you're looking for. (Alternatively you could stick with your explorer.exe approach and extract the Session ID from the Process ID, but the above is likely more robust.)

Once you have the Session ID, you call windows.WTSQueryUserToken, and voila, you get a user token for that session ID, which you can then pass to syscall.SysProcAttr's Token field. And whambam: you've got a UI in that user's session. This way you get a fresh token that's tied to the session, rather than hoping the explorer token you got works the way you were hoping.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@zx2c4 @networkimprov @ianlancetaylor @gopherbot @iangallOlive