Closed qtc-de closed 4 years ago
Hi @qtc-de , thank you so much for all of those improvements!
I will run some tests and check if something break some edge cases i know about.
About the Environment Block there are some considerations to be taken. Usually the environment block make sense only if an interactive logon is done. This because when you do that lsass will call LoadUserProfile that will load all the user data for that authentication. On other logon type lsass won't load the user profile and that is also one of the reason the interactive logon is the slowest (consider that if the user has never logon to that machine all the profile should be downloaded from domain controller and this could take also several minutes). The function CreateEnvironmentBlock does not load the user profile but try to read the data associated with the authentication in the token and in the case, i.e. network logon, it just contains the system variables.
Just as an example: if the user "user1" that has never logon to a specific machine try a network logon on that machine it will sucessfully logon on the machine and the directory C:\Users\user1 won't exist. This because all the user profile is loaded to a machine only with interactive logons. This to say that particular scripts/exe should be run within an interactive authentication either for profile creation and also because the logon type change also the SIDs associated with the processes and by consequence permissions for specific actions (ie. only interactive sids can start/stop/read some services).
The case where you manually add the USERPROFILE variable is a nice improvement but i think it won't cover all the cases and it makes sense only if the logon type is not interactive because if you use LogonUser with logon type 2 i'm almost sure that the CreateEnvironmentBlock return all the user profile envs correctly. Another thing i need to check (and maybe change before merge) is the unsafe code. I personally don't like the unsafe code in C# and i think it's possible to count that array bytes with some "while" trick using Marshal.PtrToStringUni(IntPtr) and increment the pointer casting to int and adding the string size.
For all other points you have added i see no particular problem and they are all great improvements/bugfixes.
I will merge asap, thanks!
Thanks for the feedback :)
if you use LogonUser with logon type 2 i'm almost sure that the CreateEnvironmentBlock return all the user profile envs correctly.
I just looked into this and it is not the case. Even when using logon type 2 explicitly the profile data of the corresponding user is not loaded into HKEY_USERS
. Therefore, CreateEnvironmentBlock
still ony returns the basic-environment. This is kind of odd, because looking at the token clearly shows the interactive logon:
GROUP INFORMATION
-----------------
Group Name Type SID Attributes
====================================== ================ ============ ==================================================
Everyone Well-known group S-1-1-0 Mandatory group, Enabled by default, Enabled group
BUILTIN\Users Alias S-1-5-32-545 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\INTERACTIVE Well-known group S-1-5-4 Mandatory group, Enabled by default, Enabled group
CONSOLE LOGON Well-known group S-1-2-1 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\Authenticated Users Well-known group S-1-5-11 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\This Organization Well-known group S-1-5-15 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\Local account Well-known group S-1-5-113 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\NTLM Authentication Well-known group S-1-5-64-10 Mandatory group, Enabled by default, Enabled group
Mandatory Label\Medium Mandatory Level Label S-1-16-8192
When comparing LogonUser
with logon type 2 and CreateProcessWithLogonW
reveals that the sids are the same for both methods. It seems that there is just a fundamental difference on how the OS handles these different API calls. I guess LogonUser
just never calls LoadUserProfile
, independent of the logon type you choose (even Microsofts documentation explains it differently: When a user logs on interactively, the system automatically loads the user's profile.
...). I guess this is maybe the reason why the CreateProcessWithLogonW
API call requires an additional service?
I personally don't like the unsafe code in C# and i think it's possible to count that array bytes with some "while" trick using Marshal.PtrToStringUni(IntPtr) and increment the pointer casting to int and adding the string size.
I agree. This seems to be smarter. Thanks for already implementing it :+1:
For all other points you have added i see no particular problem and they are all great improvements/bugfixes.
You are welcome and thanks for the awesome initial work :)
Merged!
I have reverse the two functions CreateProcessWithLogonW and CreateProcessWithTokenW from advapi32.dll. They both call CreateProcessWithLogonCommonW and this function make an rpc call to the seclogon service (running under svchost.exe with the seclogon.dll) calling the function SlrCreateProcessWithLogon. The difference between the two is that when a token is passed this function use it, otherwise if credentials are provided it will call LsaLogonUser to get the authentication token. In both cases the LoadUserProfile is called if the flag LOGON_WITH_PROFILE is specified. Even if a network logon is specified the profile is created/loaded. Obviously the seclogon service is run with high privileges and can call LoadUserProfile, but this is not the average case of the user running RunasCs.
I really can't figure out why the CreateEnvironmentBlock behave differently on those two very similar function. In any case the code you have added to manually change the USERPROFILE is a nice improvement that fix almost all scenarios. I see that also the TMP directory are changed to the one under the user profile when using the CreateProcessWithLogonW, but at the moment i think the USERPROFILE is enough. If any bugs will occur with also the TMP variable we could consider to add also those env variables.
The problem with LOGON_WITH_PROFILE is that it's really noisy (and slow if the domain controllers are busy). If a user never logged on the machine it will create all the profile directory under the Users folder and this is not what you would do (if the folder already exists it will change the modification time so still forensics traces). But if you need a proper process with all the environment variables this is needed. I left it as an additional flag that can be chosen based on the needing.
I have created also the dev branch if you want to add/fix anything else --> https://github.com/antonioCoco/RunasCs/tree/dev
I will soon publish also new compiled binaries under the release page and the changelog of all the improvements.
Again thanks for this PR! :)
Haha, really funny! Today I noticed the LOGON_WITH_PROFILE
option in CreateProcessWithTokenW
and thought it might be a good addition to add an additional -p
flag to RunasCs to set this value to 1. Wanted to implement this tomorrow and was a little bit disappointed once I saw your tweet about v1.3 already being released. Happy to see that you had the same idea ;)
but at the moment i think the USERPROFILE is enough
Me too. This was the only one that caused problems for me in the past and if we encounter problems with other variables they can be added quite easily.
Thanks for the quick merge and the discussions around the PR. I will keep an eye on this project and report or contribute if I encounter any problems or see additional improvements :+1:
Hi :)
With this pull request I want to add some improvements to RunasCs. Normally I would have created it for your development branch, but there is none :)
1. Add a simple command line parser
Up to now RunasCs used only positional arguments, which is obviously not very user friendly. Since the tool is relatively small, adding an argparse-library would be an overkill. Therefore, I added a small and stupid command line parser manually, which allows specifying arguments like the
domain
orlogonType
using optionals.2. Create an Environment Block
One problem of
CreateProcessAsUserA
andCreateProcessWithTokenW
is that they do not load the correct environment for the desired user.CreateProcessWithLogonW
uses the secondary logon service, which just creates the ordinary user environment, but the otherCreateProcess...
functions inherit the environment from the caller.Here is an example where the command
set
is called withCreateProcessWithLogonW
:And the same again for
CreateProcessAsUserA
:Why this matters
Well, to be honest in 90% of the cases it is probably fine to inherit the environment. However, I encountered situations where the wrong environment variables caused issues with other applications. E.g. recently I was not able to launch PowerShell on a server, since the
USERPROFILE
variable was not setup correctly. Also other software may rely on correctly set environment variables.How to correct this
Well, in theory it is simple.
CreateProcessAsUserA
andCreateProcessWithTokenW
both accept anlpEnvironment
variable. Therefore, following calls can be made to setup the correct environment:This works fine, but only when the corresponding user is not currently logged in. In that case, the
CreateEnvironmentBlock
function returnsAccess Denied
. Not 100% sure why, but maybeCreateEnvironmentBlock
attempts to read the logged on users environment. However, as we already have a token fromLogonUserA
that can be used for impersonation, we can impersonate the user before obtaining the environment:Now we should be fine, right? Unfortunately not yet. When the targeted user account is currently not logged in,
CreateEnvironmentBlock
only creates a very basic environment. Again, this is probably fine in 90% of the cases, but as mentioned above, I encountered situations where theUSERPROFILE
variable was required to launch PowerShell. This is one of the variables that is not set byCreateEnvironmentBlock
.To get around this limitation, one can extend the
lpEnvironment
block. Unfortunately, this is a really odd structure which (best to my knowledge) cannot easily be converted to managed code. Therefore, we need someunsafe
code to parse it:After parsing it one can use
GetUserProfileDirectory
to extend the structure and finally create the process with an (almost) correct environment (if other environment variables are required they can be added in a pretty simple manner). So the final procedure is:This is the corresponding result when using the
set
command withCreateProcessAsUserA
:3. Add Remote Host Support
Another change was to add the
-r
or--remote
option. This option allows redirecting stdin, stdout and stderr to a remote host. Just in case you want to launch an interactive executable.And on the remote host:
4. Smooth Exit from PowerShell
The
Invoke-RunasCs.ps1
script is a nice addition, but it does not work that well together with the current implementation ofRunasCs.cs
. The problem is thatRunasCs.cs
usesSystem.Environment.Exit()
in different locations. When called from PowerShell, this will cause PowerShell to exit.To resolve this issue, I added an exception based error handling. Instead of terminating the program, errors will bubble up to the main function. There they are caught and returned as ordinary program output. From the users perspective there is no difference apart from the error code and I guess nobody cares about that one anyway. With these changes, RunasCs can be used from PowerShell without the risk of terminating the shell.
Changing this required modifications in several different places. I hope that I did not forgot or broke anything. So far, I did not encounter any errors.
5. Process Timeout
The process timeout implementation missed an edge case. When the executed command not produces any output befor finishing, the process timeout is not effective. A simple (but stupid) example is:
This call will block forever, as
nc.exe
does not produce any output before finishing. The underlying reason is well described by Microsofts Documentation of theCreatePipe
function:If no write operation is done by the process on the other side of the pipe, the
ReadFile
call hangs forever. To prevent this, my pull request uses a call toSetNamedPipeHandleState
to set the pipe handle toPIPE_NOWAIT
. In this state,ReadFile
will always return immediately, (hopefully) resulting in the expected behavior.Final Remarks
Sorry that this PR got so lengthy. Initially I just wanted to make a small change, but somehow it escalated a little bit. During my changes I kept attention to not break anything and to leave the project
.NETv2
compatible. However, I'm not that familar with Windows API orC#
programming. I tested the version from this PR on my dev maschine as well as a Windows server. So far no errors encountered. However, it would be great if you could review the changes befor merging them.Best Regards Tobias