Consider exposing Bedrock's "Connection Features" on HttpContext

[analogrelay]

I'm filing this to track some ideas that came up while @Tratcher was looking at Kerberos/NTLM auth. The exact requirements are still coming but I wanted to file this to start some parallel discussion on things that may help make the implementation smooth.

NTLM requires mandatory caching storage of security context information between requests occurring on the same connection. So, given a connection C and NTLM-authenticated requests R1 and R2 (with R2 following after R1), it is not possible to authenticate R2 without using cached stored data from the authentication process in R1.

Using current features, it is relatively simple to implement this by caching this data keyed off the Connection ID. However this has a few problems:

1. It's a little clunky to have to maintain a separate cache dictionary when there is generally a connection state object in the server
2. It is difficult to reliably expire this cache clean-up unnecessary contexts unless the server exposes a "Connection Ended" event of some kind.

public interface INtlmConnectionStateFeature
{
    // ... ntlmy data ...
}

The auth middleware can implement this feature entirely, however, it needs to be able to store it somewhere that is guaranteed to live across the entire connection. Bedrock's "Connection Features" is a perfect place for this, however it is not exposed up through the stack.

If we had a way to Get/Set connection-level features, we could implement this in the auth handler with pseudo-code like this:

var currentState = context.Connection.Features.Get<INtlmConnectionStateFeature>();
if (currentState == null)
{
    currentState = new NtlmConnectionState();
    context.Connections.Features.Set(currentState);
}
PerformAuthentication(currentState);

My proposal is this:

1. Add a new feature to the request features: IHttpConnectionFeaturesFeature (name can be bikeshed later)

public interface IHttpConnectionFeaturesFeature
{
    IFeatureCollection ConnectionFeatures { get; }
}

2. Add read-only Features property to ConnectionInfo which returns null if there is no IHttpConnectionFeaturesFeature present.

3. Implement IHttpConnectionFeaturesFeature in Kestrel to expose the underlying Connection's feature collection.

The NTLM authentication logic will require this feature be present in order to function, and will throw a useful exception if it isn't present. This way, servers which do not support this feature are not "broken", but they can't be used with NTLM auth. Since even the "custom dictionary" method requires a server change in order to detect the end of the connection, this seems like a reasonable requirement to make.

We can consider implementing the feature in IIS and HttpSysServer as well, though since they have integrated Windows Auth, it may not be as necessary at the moment.

Let the discussion begin! @davidfowl @halter73

[davidfowl]

What does it mean for HTTP/2 connections? In that scenario if you set a connection level feature during a request it can interfere with other requests on that same connection (since they overlap in execution).

[Tratcher]

@davidfowl good point. Windows/Kerberos auth isn't supported on HTTP/2 for similar reasons. We could skip HTTP/2 connections for the time being. Or you have to make that feature collection concurrent.

[analogrelay]

What does it mean for HTTP/2 connections?

Yep, this is a good point.

We could skip HTTP/2 connections for the time being

For NTLM, this is sufficient, since as @Tratcher says, you can't do NTLM over HTTP/2.

[analogrelay]

Btw, the alternative feature (one of the two options is basically required for NTLM) is something like this:

public interface IHttpConnectionLifetimeFeature
{
    CancellationToken ConnectionTerminated { get; }
}

That would allow the auth handler to empty it's stored context (I want to avoid using "cache" since it's not an optimization, it's mandatory).

[halter73]

If we had a way to Get/Set connection-level features

You can already get connection-level features, just not set them. As a temporary hack, you could have a mutable INtlmConnectionStateFeature pre-set by the server. Or less hacky, we could have a the server add a non-mutable IHttpConnectionLifetimeFeature.

https://github.com/aspnet/AspNetCore/blob/212ba91a5c1eaf51c0205fc4422a92c027ffeaec/src/Servers/Kestrel/tools/CodeGenerator/FeatureCollectionGenerator.cs#L110

https://github.com/aspnet/AspNetCore/blob/212ba91a5c1eaf51c0205fc4422a92c027ffeaec/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs#L259

[Tratcher]

Additional requirement: Dispose a given object when cleaning up a connection. This is the connection equivalent of HttpResponse.OnCompleted/RegisterForDispose.

Kerberos/NTLM auth on windows produces a WindowsIdentity that holds an OS handle. The finalizer will take care of it eventually, but in a high churn situation you could exhaust OS handles before the finalizer kicks in.

The NTAuth connection state will also need to be disposed.

[Tratcher]

I was able to implement this using three existing features:

• IConnectionLifetimeFeature for disposing the auth connection state
• IConnectionItemsFeature for storing the auth connection state
• HttpResponse.RegisterForDispose to dispose the per-request copy of the WindowsIdentity

IConnectionLifetimeFeature isn't ideal because it could fire mid-request. We'll want to replace it with the a connection level equivalent of RegisterForDispose.

[NinoFloris]

This would be very useful for some of our own downstream request context objects too, which are currently pooled via ObjectPool.

EDIT: For HTTP/2 we wouldn't indeed be able to have a single cached reference but a super small pool on a connection to very naturally segment, otherwise global, contention.

[analogrelay]

Kestrel currently does push Connection Features through to Request Features (you can't add new ones, but you can access any existing connection features), so we're good for 3.0. Moving to the backlog.

[rizi]

I was able to implement this using three existing features:

• IConnectionLifetimeFeature for disposing the auth connection state
• IConnectionItemsFeature for storing the auth connection state
• HttpResponse.RegisterForDispose to dispose the per-request copy of the WindowsIdentity

IConnectionLifetimeFeature isn't ideal because it could fire mid-request. We'll want to replace it with the a connection level equivalent of RegisterForDispose.

@Tratcher we exactly need this feature, currently two users get the same windows identity (the one from the user who logged in first), could you provide some help how this can be solved?

Do you have a code sample?

br

[Tratcher]

I was able to implement this using three existing features:

• IConnectionLifetimeFeature for disposing the auth connection state
• IConnectionItemsFeature for storing the auth connection state
• HttpResponse.RegisterForDispose to dispose the per-request copy of the WindowsIdentity

IConnectionLifetimeFeature isn't ideal because it could fire mid-request. We'll want to replace it with the a connection level equivalent of RegisterForDispose.

@Tratcher we exactly need this feature, currently two users get the same windows identity (the one from the user who logged in first), could you provide some help how this can be solved?

Do you have a code sample?

br

That sounds like a client bug, two users should never attempt to use the same connection with NTLM/Kerb and UnsafeNTLMConnectionSharing. Consider disabling NegotiateOptions.PersistNTLM/KerberosCredentials.

[rizi]

NegotiateOptions.PersistNTLM/KerberosCredentials.

@Tratcher
In this case the client is a "normal" browser (Edge, Chrome, Brave), where can this options be found " NegotiateOptions.PersistNTLM/KerberosCredentials"?

FYI: It's not happening always, but we found out (with the logs of YARP and the logs of our server application) that once userA is authenticated and then right after that userB tries to authenticate via NTLM using asp.net mvc (.net 4.7.2) hosted in IIS the windows identity of userA is used instead of userBs, it's only happening when using YARP

[Tratcher]

NTLM must not be used with TLS terminating proxies like YARP since external connections from different users can share the same connection to the backend.
https://microsoft.github.io/reverse-proxy/articles/authn-authz.html#windows-negotiate-ntlm-kerberos

[rizi]

NTLM must not be used with TLS terminating proxies like YARP since external connections from different users can share the same connection to the backend. https://microsoft.github.io/reverse-proxy/articles/authn-authz.html#windows-negotiate-ntlm-kerberos

@Tratcher thx for the clarification 👍

Br