dotnet / dotnet/aspnetcore

Consider exposing Bedrock's "Connection Features" on HttpContext

Open
#9,213 14 comments 1 reaction 0 assignees View on GitHub
affected-very-few area-networking enhancement Needs: Design severity-nice-to-have
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

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](https://github.com/aspnet/AspNetCore/blob/4134d02dab44717d049d1e55abb473ec04ddf47d/src/Http/Http.Abstractions/src/ConnectionInfo.cs#L16). 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
1. It is difficult to reliably ~~expire this cache~~ clean-up unnecessary contexts unless the server exposes a "Connection Ended" event of some kind.

```csharp
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:

```csharp
var currentState = context.Connection.Features.Get();
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)

```csharp
public interface IHttpConnectionFeaturesFeature
{
IFeatureCollection ConnectionFeatures { get; }
}
```

2. Add **read-only** `Features` property to [`ConnectionInfo`](https://github.com/aspnet/AspNetCore/blob/4134d02dab44717d049d1e55abb473ec04ddf47d/src/Http/Http.Abstractions/src/ConnectionInfo.cs#L11) 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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.