5

Closed

Thread.CurrentPrincipal Not Propagated

description

We have code that has a DelegatingHandler that sets Thread.CurrentPrincipal to an IClaimsPrincipal early in the pipeline. This has been working just fine for months. Recently we've enabled a custom TraceWriter using the following code:

httpConfiguration.Services.Replace(typeof(ITraceWriter), new TraceWriter());

This seems to cause a greater level of thread agility and Thread.CurrentPrincipal is not being propagated to the new thread. I'm presuming this also means that it might not get cleaned up properly so could result in privilege escalation...

We're running .NET 4.0 with the ASP.NET 4.0 Web API RC. Our code essentially looks like this (greatly simplified):
internal class AuthenticationMessageHandler : DelegatingHandler
{
    protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        Thread.CurrentPrincipal = ClaimsPrincipal.CreateFromIdentities(...);

        return base.SendAsync(request, cancellationToken);
    }
}
Then a controller accesses the IClaimsPrincipal as follows:

IClaimsPrincipal claimsPrincipal = Thread.CurrentPrincipal.AsClaimsPrincipal();

this returns null because Thread.CurrentPrincipal from the handler hasn't been propagated to the thread that the controller is running on...

file attachments

Closed Jun 7, 2013 at 8:34 PM by trdai

comments

adamiprinciples wrote Jun 14, 2012 at 11:24 AM

This is a pretty serious problem and is really hindering my progress on a couple of projects now.

HongmeiG wrote Jun 14, 2012 at 3:58 PM

Thanks for reporting the issue. We are looking into this issue.

As a temporary workaround, you can save your principal as a property in the request.Properties collection in our authentication message handler, and retrieve the principal from the request's properties later in your controller.

roncain wrote Jun 14, 2012 at 8:19 PM

I'm investigating now. Could I ask for more details on what your TraceWriter does? I just want to rule it out. I'll assume a do-nothing TraceWriter will do the same thing.

roncain wrote Jun 14, 2012 at 9:31 PM

Apparently I do need more details, such as how and when the ApiController is accessing CurrentPrincipal, what the TraceWriter does, exactly how you created the ClaimsPrincipal, what other message handlers are in play, using WebHost or SelfHost, etc.

I am attaching NoRepro.cs which is a console app using SelfHost to pass an IPrincipal to a WebApi action. On the current post-RC bits, this app does not lose the IPrincipal. Could I ask you to try it? If it passes, could I ask you to describe how you app differs from this one?

deanward wrote Jun 14, 2012 at 10:00 PM

Sure, the TraceWriter is the NLog one from https://github.com/dbettin/webapi.nlog/blob/master/src/WebApi.NLog/NLogTraceWriter.cs

We're currently hosting in IIS (WebHost) - I haven't tested using SelfHost yet but I'll grab your code and give it a go tomorrow morning (we're UK so I'll give it a run at 7am-ish GMT).

I've attached the AuthenticationHandler - it's primarily synchronous code so don't think it'll be an issue. From what I can see in our logs the problem occurs when a thread switch occurs between the handler and the controller - i.e. Thread.CurrentThread.ManagedThreadId is different. When we don't add the TraceWriter it is the same thread, when we do it changes thread between the handler and the controller.

roncain wrote Jun 14, 2012 at 10:06 PM

I can repro this in WebHost and am investigating...

roncain wrote Jun 15, 2012 at 9:08 PM

We can repro this on WebHost using IIS Express -- on some machines, in certain scenarios. I believe I see the bug but we are finding out what makes it repro and what allows it to succeed.

Do you have a rough idea of how frequently it fails?
How many cores to the machines that fail generally have?

Thanks for the information so far -- it is helping narrow this.

roncain wrote Jun 15, 2012 at 9:56 PM

This was fixed with http://aspnetwebstack.codeplex.com/SourceControl/changeset/changes/df47d044190b.

The issue was that Tracing async operations failed to honor SynchronizationContext to marshall its end trace onto the original thread. IPrincipal was gone because it was on a different thread.
More significantly, being on another thread lost the original SynchronizationContext for downstream consumers.

deanward wrote Jun 15, 2012 at 11:15 PM

Fantastic turnaround time guys, very impressed :o)