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

WindowsIdentity.RunImpersonated has issues resolving libraries if the library wasn't already used in the program #91771

Closed
catmanjan opened this issue Sep 6, 2023 · 21 comments
Labels
area-System.Security help wanted [up-for-grabs] Good issue for external contributors investigate
Milestone

Comments

@catmanjan
Copy link

catmanjan commented Sep 6, 2023

This issue was reported here: dotnet/core#5021 (comment)

It seems that if your WindowsIdentity.RunImpersonated action contains references to functions in libraries that weren't used before the impersonation occurs, dotnet is not resolving those libraries as expected.

The simplest reproduction steps:

using System.Security.Principal;

//Console.WriteLine("This works");

WindowsIdentity.RunImpersonated(new WindowsIdentity("[email protected]").AccessToken, () => { Console.WriteLine("Hello"); });

You receive this error:

Unhandled exception. System.IO.FileLoadException:
File name: 'System.Console, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at Program.<>c.<<Main>$>b__0_0()
   at System.Security.Principal.WindowsIdentity.<>c__DisplayClass73_0.<RunImpersonatedInternal>b__0(Object <p0>)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Security.Principal.WindowsIdentity.RunImpersonatedInternal(SafeAccessTokenHandle token, Action action)
   at System.Security.Principal.WindowsIdentity.RunImpersonated(SafeAccessTokenHandle safeAccessTokenHandle, Action action)
   at Program.<Main>$(String[] args)

Uncomment Console.WriteLine("This works"); and the program runs as expected.

@buyaa-n buyaa-n transferred this issue from dotnet/core Sep 8, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 8, 2023
@ghost
Copy link

ghost commented Sep 8, 2023

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

This issue was reported here: dotnet/core#5021 (comment)

It seems that if your WindowsIdentity.RunImpersonated action contains references to functions in libraries that weren't used before the impersonation occurs, dotnet is not resolving those libraries as expected.

The simplest reproduction steps:

using System.Security.Principal;

//Console.WriteLine("This works");

WindowsIdentity.RunImpersonated(new WindowsIdentity("[email protected]").AccessToken, () => { Console.WriteLine("Hello"); });

You receive this error:

Unhandled exception. System.IO.FileLoadException:
File name: 'System.Console, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at Program.<>c.<<Main>$>b__0_0()
   at System.Security.Principal.WindowsIdentity.<>c__DisplayClass73_0.<RunImpersonatedInternal>b__0(Object <p0>)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Security.Principal.WindowsIdentity.RunImpersonatedInternal(SafeAccessTokenHandle token, Action action)
   at System.Security.Principal.WindowsIdentity.RunImpersonated(SafeAccessTokenHandle safeAccessTokenHandle, Action action)
   at Program.<Main>$(String[] args)

Uncomment Console.WriteLine("This works"); and the program runs as expected.

Author: catmanjan
Assignees: -
Labels:

area-System.Security

Milestone: -

@jozkee
Copy link
Member

jozkee commented Sep 8, 2023

Hmm works for me although I had to use WindowsIdentity.GetCurrent() instead of a UPN because I was getting the error "Name provided not a properly formed account name" from WindowsIdentity.ctor(string).

//Console.WriteLine("This works");
WindowsIdentity.RunImpersonated(WindowsIdentity.GetCurrent().AccessToken, () => { 
    string json = JsonSerializer.Serialize(42);
    Console.WriteLine("Hello " + json); });

@jozkee jozkee added this to the Future milestone Sep 8, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 8, 2023
@catmanjan
Copy link
Author

@jozkee you will need to replace the [email protected] in my code sample to match your environment, it must be a user other than your local user

@catmanjan
Copy link
Author

bump

@catmanjan
Copy link
Author

@jozkee any chance I can get this and #93537 escalated?

@jozkee
Copy link
Member

jozkee commented Oct 18, 2023

This issue still needs investigation as I am unable to exactly match your scenario with UPN; yes, I did replace "[email protected]" while trying.

cc @jeffhandley.

@catmanjan
Copy link
Author

@jozkee I think you will also need to be on a domain joined machine, running as a domain account, impersonating a different user from that domain

@jozkee jozkee added help wanted [up-for-grabs] Good issue for external contributors investigate labels Oct 20, 2023
@wfurt
Copy link
Member

wfurt commented Oct 31, 2023

Would it also fail if you leave the first Console.WriteLine in? Runtime will load assemblies as needed on first call. I have seen cases when the impersonated user does not have access to the actual files.

@catmanjan
Copy link
Author

catmanjan commented Oct 31, 2023

@wfurt no, it works fine as long as you don't reference any new dlls while impersonating - I understand why it might be happening at a technical level, but here's why it is bad:

  • it didn't work this way in .NET framework
  • it is unexpected that library DLLs wouldn't be loaded as the original identity
  • it is tedious and ugly to have to call each DLL that you may need while impersonating beforehand just to load them into memory

P.S. I just checked, and even if you are impersonating a user who does have permission to load the DLLs it still doesn't work, but I think this is more related to the fact that impersonation in dotnet core isn't really working at all, see #93537

@wfurt
Copy link
Member

wfurt commented Oct 31, 2023

I cannot speak for Framework - but to load all dlls at start is expensive. The "expectation" is questionable. We can perhaps make it more clear - and our docs repo takes contributions.

I think the correct solution is to make sure the new identity has access. If it does not we should investigate. I did look at #93537 but it it hard to tell what is going on without getting repro setup.

@catmanjan
Copy link
Author

@wfurt yes it is expensive - do you realize that is what you currently have to do today?

I am not sure what your position is here, do you think this is working as intended?

Regarding #93537, all you need to repro is a domain joined machine BTW

@jeffhandley
Copy link
Member

jeffhandley commented Nov 2, 2023

Hello @catmanjan. We appreciate all the effort you've done to narrow this down and verify this can be observed as a difference between .NET Framework 4.8 and .NET 6. It's apparent how much effort you've put into the investigation and we appreciate you searching through old issues, StackOverflow, and other resources before submitting these issues.

To set expectations though, we do not have an SLA for investigation or resolution of issues submitted on GitHub. At this time, we do not know if the difference in behavior is intended.

From what you've shared, I gather this observed difference is blocking a migration from .NET Framework to .NET 6+. We certainly want to support you on that migration and we are progressing with our investigation as swiftly as we can. I also understand that from your perspective, the repro should be easy. Since this scenario relies on domain configuration details though, a typical machine does not match your environment. We are exploring this further, but again we cannot promise any sort of turnaround time.

If you need support on this issue more quickly than our team is able to provide, Microsoft Support might be a good avenue.

Thanks for your understanding and patience. We'll get back with you as soon as we can with next steps.

@wfurt
Copy link
Member

wfurt commented Nov 2, 2023

I'm not even sure there is fundamental difference. AFAIK the error comes from OS not .NET. The console may work since on Framework that is part of mscorlib.dll but other methods may still fail.

On that note, do you use shared framework or self-contained app @catmanjan ?

@jozkee
Copy link
Member

jozkee commented Nov 3, 2023

I think the correct solution is to make sure the new identity has access.

As @wfurt mentioned, the error is likely due to the impersonated acct lack of permissions to the dlls. See https://stackoverflow.com/q/69302526/4231460. @catmanjan can you please check that?

I was finally able to repro and I can see no exception on .NET Framework as you describe, however as soon as I call a type whose dll wasn't unloaded e.g. BigInteger, I do get the exception. This aligns with @wfurt's comment.

The console may work since on Framework that is part of mscorlib.dll but other methods may still fail.

@catmanjan can you please give a try to this in .NET Framework so you can confirm this is not a regression?

var identity = new WindowsIdentity(username@domain.com); 
WindowsIdentity.RunImpersonated(
    identity.AccessToken,
    () => { Console.WriteLine(BigInteger.Zero); });

@catmanjan
Copy link
Author

catmanjan commented Nov 5, 2023

@jozkee I tried your sample in .NET 4.8 and it executes as expected so it seems like it is a regression

image

@catmanjan
Copy link
Author

On that note, do you use shared framework or self-contained app @catmanjan ?

I've tried most every combination I can think of, including shared framework/self-contained+packaging sdk etc etc - in all cases dotnet6 consistenly has issues with impersonation

@catmanjan
Copy link
Author

@jeffhandley I'd like to log a support ticket but I can't work out how to from that link, it seems to be a search portal for self help - my company is a silver partner and deals a lot with aus government so I believe we have an entitlement to do so?

@jeffhandley
Copy link
Member

jeffhandley commented Nov 6, 2023

@catmanjan Here's the entry point for submitting a new support ticket: https://support.serviceshub.microsoft.com/supportforbusiness/create

This link is available from https://support.microsoft.com under the "Enterprise Support" section, listed as "Support for business"

@jozkee
Copy link
Member

jozkee commented Nov 10, 2023

@jozkee I tried your sample in .NET 4.8 and it executes as expected so it seems like it is a regression

That's not what I see on my end.

@wfurt
Copy link
Member

wfurt commented Nov 10, 2023

I'm wondering if #49292 is relevant e.g. system privilege vs client impersonation.

@catmanjan
Copy link
Author

catmanjan commented Nov 13, 2023

@wfurt yes running as system fixes the impersonation and dll usage issue - so does the TCB hack described in that ticket

Perhaps there needs to be something in the documentation to explain the new requirements around RunImpersonated

For now we will use the TCB reflection hack as admins are hesitant to run anything as system

@github-actions github-actions bot locked and limited conversation to collaborators Dec 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security help wanted [up-for-grabs] Good issue for external contributors investigate
Projects
None yet
Development

No branches or pull requests

4 participants