generic-worker: spawn child processes using CreateProcessAsUser or CreateProcessWithLogonW Windows system calls

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: pmoore, Assigned: pmoore)

Tracking

Details

(Assignee)

Description

3 years ago
Windows users are created dynamically per task by the generic worker. For each task, a new user is created, used, and destroyed.

The generic worker runs as a Windows service, and as such runs under a different user to the task users that get created. This means the generic worker needs to spawn processes under a different user account than the generic worker is running under.

In go there is no trivial way to do this for Windows. The options are:

1) using RUNAS in cmd.exe - this requires cmd.exe to run, and a password to be dynamically provided, the password cannot be a command line option. I have not succeeded in passing this in dynamically - providing it via standard in is not sufficient. Also requires cmd.exe, so not optimal

2) using PsExec - this was working fine, until running as a service. Once running as a service, the calls to PsExec fail with STATUS_DLL_INIT_FAILED == -1073741502 == 0xC0000142. I've investigated many possible paths to solve this but to no avail. Again having a dependency on PsExec library is not ideal.

3) using Powershell. Here there are also troubles, see: http://stackoverflow.com/questions/30917132/why-does-powershell-start-process-not-work-when-called-from-go-golang (currently unanswered). I raised this today to see if somebody might be able to help.

4) using the go syscall package to make windows system calls directly to spawn processes. This is the one I'd like to go with.

This will require working through e.g.

* https://msdn.microsoft.com/en-us/library/windows/desktop/ms682429%28v=vs.85%29.aspx
* https://golang.org/src/os/exec_windows.go

to work out how to get this working from go.

Might even be worth contributing back to the go project, so that it can be done in quite a trivial way as it can in unix, e.g.: http://stackoverflow.com/a/21706757/5023934
(Assignee)

Comment 2

3 years ago
(In reply to Pete Moore [:pmoore][:pete] from comment #1)
> In particular:
> https://golang.org/src/syscall/exec_windows.go?s=4805:4904#L228

Here the golang exec code is calling CreateProcess (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425%28v=vs.85%29.aspx) rather than CreateProcessAsUser or CreateProcessWithLogonW. Therefore in order to solve this, comparing the syscall differences between CreateProcess and CreateProcessAsUser/CreateProcessWithLogonW on the microsoft site should provide enough insight to work out how to implement a modified version of StartProcess (https://golang.org/src/syscall/exec_windows.go?s=4805:4904#L228) in go that correctly handles starting the process as an alternative user.

We might be able to use https://golang.org/src/os/user/lookup_windows.go for querying the user data - although there is a bug reported here which we may also first need to fix ("// BUG(brainman): Lookup and LookupId functions do not set Gid and HomeDir fields in the User struct returned on windows.").
(Assignee)

Updated

3 years ago
Blocks: 1178657
(Assignee)

Comment 3

3 years ago
Assigning all generic worker bugs to myself for now. If anyone wants to take this bug, feel free to add a comment to request it. I can provide context.
Assignee: nobody → pmoore
(Assignee)

Updated

3 years ago
Component: TaskCluster → Generic-Worker
Product: Testing → Taskcluster
If you are looking to spawn a new process only to run as a different user, you might want to consider the impersonation API https://msdn.microsoft.com/en-us/library/cc246062.aspx
(Assignee)

Comment 6

3 years ago
(In reply to Wander Lairson Costa [:wcosta] from comment #5)
> If you are looking to spawn a new process only to run as a different user,
> you might want to consider the impersonation API
> https://msdn.microsoft.com/en-us/library/cc246062.aspx

I think we'll need to spawn a new process in any case, to run the task, keeping the generic worker running in its own process separately. Is the impersonation API more about security context switching (e.g. impersonating a user in an already-running process), or also something to be used when creating new processes?
(In reply to Pete Moore [:pmoore][:pete] from comment #6)
> (In reply to Wander Lairson Costa [:wcosta] from comment #5)
> > If you are looking to spawn a new process only to run as a different user,
> > you might want to consider the impersonation API
> > https://msdn.microsoft.com/en-us/library/cc246062.aspx
> 
> I think we'll need to spawn a new process in any case, to run the task,
> keeping the generic worker running in its own process separately. Is the
> impersonation API more about security context switching (e.g. impersonating
> a user in an already-running process), or also something to be used when
> creating new processes?

Impersonating API would be more useful in a multithread environment, not so much useful regarding multiprocess.
(Assignee)

Comment 8

3 years ago
Here are some pointers I received on the go-nuts google group:
https://groups.google.com/forum/#!searchin/golang-nuts/1073741502/golang-nuts/2W2jVhMdKkk/7QF65sES9TsJ
(Assignee)

Comment 9

3 years ago
So I've done a lot of work on this today. Long story short - the system calls are buried deep inside the standard library, and the only way to change them (to use CreateProcessWithLogonW instead of CreateProcess) was to lift (copy) several functions and types in the standard package - essentially everything from the API method down through the stack until the system calls - and place this inside the generic worker.

Long term it may be better for everyone if this was fed back upstream to be available as part of the standard library. It would certainly be cleaner. There were no abstractions in the standard library that allowed essentially swapping out the system calls only, e.g. no interface / IoC that would have enabled this.

For the first pass, I'm just going to rebuild all, run tests, and make sure that the "lifted" libraries function the same as the original. After that is all ok, I will be switching out CreateProcess call for CreateProcessWithLogonW call).

Changes in first pass:
https://github.com/taskcluster/generic-worker/compare/be89283f41aca219f046d65bbd12f804414141c4...c8f6d8301ff0f5caed48a5e4b2ebe370459060ed
Status: NEW → ASSIGNED
(Assignee)

Comment 10

3 years ago
So this is fixed now in release 2.0.0alpha14:
https://github.com/taskcluster/generic-worker/releases/tag/v2.0.0alpha14

I'd still like to rework this into the standard library, and contribute it back upstream.

For now I forked the standard library functions and types, and placed them inside the generic worker, and massaged them to accept username/password for windows and use the alternative system call.

When this is contributed back, I'll update https://golang.org/pkg/syscall/#Credential to allow including a password when on Windows. I didn't do it this way in the current implementation, since it meant forking even more types/methods/functions, so I tried to keep the code lifting to a minimum by adding it directly in https://golang.org/pkg/os/exec/#Cmd.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

3 years ago
Frustratingly, somewhere between getting it working, and creating a new release, I seem to have broken it, and at the moment I can't work out where.

Now I'm getting exit status 3221225794. :(

Looking into this...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

3 years ago
From Windows Event logs...

WIN-H9V5NDSAFG5	1000	Error	Application Error	Application	12/18/2015 11:32:44 AM
Faulting application name: cmd.exe, version: 6.3.9600.17415, time stamp: 0x545042b1
Faulting module name: KERNELBASE.dll, version: 6.3.9600.18146, time stamp: 0x5650b9bb
Exception code: 0xc0000142
Fault offset: 0x00000000000ec540
Faulting process id: 0xb30
Faulting application start time: 0x01d13987cfd887be
Faulting application path: C:\Windows\system32\cmd.exe
Faulting module path: KERNELBASE.dll
Report Id: 0d8c74ed-a57b-11e5-8114-0aa5466b941d
Faulting package full name: 
Faulting package-relative application ID: 

WIN-H9V5NDSAFG5	1000	Error	Application Error	Application	12/18/2015 11:32:44 AM
Faulting application name: conhost.exe, version: 6.3.9600.17415, time stamp: 0x5450410b
Faulting module name: USER32.dll, version: 6.3.9600.18146, time stamp: 0x5650b9bb
Exception code: 0xc0000142
Fault offset: 0x00000000000ec540
Faulting process id: 0x4d4
Faulting application start time: 0x01d13987cfd8d42c
Faulting application path: C:\Windows\system32\conhost.exe
Faulting module path: USER32.dll
Report Id: 0d865ac2-a57b-11e5-8114-0aa5466b941d
Faulting package full name: 
Faulting package-relative application ID: 


If I rerun a task, I consistently get these two entries in the application event logs.
(Assignee)

Comment 13

3 years ago
So running from a console, I don't get this problem, so it seems to be related to the way I have the generic worker running as a service.

I notice when I run from the console, that it opens up its own console window for the task, which might be not allowed due to https://github.com/taskcluster/generic-worker/blob/0baabc69bd5c741b3797934658643eda8c84beac/plat_windows.go#L466 so I'll try changing that, and see if that fixes things.

Here is a task running **successfully** when the generic worker is invoked from the console as Administrator, rather than from a windows service as Generic Worker user:

  * https://tools.taskcluster.net/task-inspector/#M3wNmR3KTDejy2WzOzLgxQ/0

To contrast, here is it failing when run as a service:

  * https://tools.taskcluster.net/task-inspector/#JAyVlGrHRQy0dyaB1aEQ-A/0

I have a meeting now, so I'll try the above suggested change in the morning.
(Assignee)

Comment 14

3 years ago
Another option might be disabling CREATE_NEW_CONSOLE and/or setting CREATE_NO_WINDOW (see https://msdn.microsoft.com/en-us/library/windows/desktop/ms682431%28v=vs.85%29.aspx) or some other subtle flags.

Of course there is still CreateProcessAsUser that might work, or another CreateProcess variant, in the worst case.

Some maybe relevant information here:
http://blogs.msdn.com/b/winsdk/archive/2009/07/14/launching-an-interactive-process-from-windows-service-in-windows-vista-and-later.aspx
(Assignee)

Comment 15

3 years ago
^^ These suggestions are based on the premise that the reason that the error occurs when running from a windows service is that a console tries to attach to a desktop, and that there is not one available, or it does not have permission to attach to the one it tries to attach to. The hope is that if I can disable a console window from opening, I avoid the problem, or alternatively if I can allow the windows service to access a desktop, that would also work. The benefit of allowing it to open a console or attach to a desktop is that later we will have tests that will want to do this - so this would be preferred to just preventing a window from getting created which will not help us later when tests require desktop elements...
(Assignee)

Updated

3 years ago
Depends on: 1240031
(Assignee)

Comment 16

3 years ago
Created a separate bug (bug 1240031) for this issue ^^^
(Assignee)

Updated

3 years ago
Depends on: 1267992
No longer depends on: 1240031
(Assignee)

Updated

3 years ago
See Also: → bug 1240031
(Assignee)

Comment 17

3 years ago
This was resolved in bug 1267992! \o/
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.