Expose underlying connection when leveraging Kestrel for IPC

[urielginsburg]

Is there an existing issue for this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

For its simplicity and cross platform portability, I am very interested in leveraging Kestrel for IPC over named pipes and *nix domain sockets, both of which are supported by Kestrel and endorsed by Microsoft here.

However, because transport is completely abstracted from services, the endpoints and services cannot access the identity of the client, even though it is readily available at the transport layer without mucking about with application-level authentication or TLS.

It would be very beneficial, if not considered essential to expose the identity of the client, as well as the ability to impersonate the client where and when it's applicable in IPC scenarios.

Describe the solution you'd like

two virtual methods could be added to the ConnectionInfo class:

public virtual NamedPipeServerStream? DangerousGetNamedPipeServerStream() { return null; }
public virtual Socket? DangerousGetUnixDomainSocket() { return null; }

Additionally, an IConnectionContextFeature interface that exposes the TransportConnection property could be added and implemented by the KestrelConnection class. The constructor of KestrelConnection<T> could set it as it does other features:

/// <summary>
/// A feature that represents the connection context.
/// </summary>
public interface IConnectionContextFeature
{
    /// <summary>
    /// Gets the underlying <seealso cref="BaseConnectionContext"/>
    /// </summary>
    public BaseConnectionContext ConnectionContext { get; }
}
...
public KestrelConnection( ... ) : base( ... )
{
    _connectionDelegate = connectionDelegate;
    _transportConnection = connectionContext;
    connectionContext.Features.Set<IConnectionHeartbeatFeature>(this);
    ...
    connectionContext.Features.Set<IConnectionContextFeature>(this);
}

Additional context

Strictly as a PoC, the following code gets the client identity of the current HttpContext on Windows:

public static class HttpContextExtensions
{
    private static readonly PropertyInfo _connectionLifetimeProperty;
    private static readonly PropertyInfo _transportConnectionProperty;
    private static readonly PropertyInfo _namedPipeProperty;

    static HttpContextExtensions()
    {
        var assembly = typeof(Microsoft.AspNetCore.Http.HttpContextAccessor).Assembly;
        var defaultConnectionInfoType = assembly.GetType("Microsoft.AspNetCore.Http.DefaultConnectionInfo");

        assembly = typeof(Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes.NamedPipeTransportOptions).Assembly;
        var namedPipeConnectionType = assembly.GetType("Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes.Internal.NamedPipeConnection");

        assembly = typeof(Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServer).Assembly;
        var kestrelConnectionType = assembly.GetType("Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure.KestrelConnection");

        _connectionLifetimeProperty = defaultConnectionInfoType.GetProperty("ConnectionLifetime", BindingFlags.Instance | BindingFlags.NonPublic);
        _namedPipeProperty = namedPipeConnectionType.GetProperty("NamedPipe", BindingFlags.Instance | BindingFlags.Public);

        _transportConnectionProperty = kestrelConnectionType.GetProperty("TransportConnection", BindingFlags.Instance | BindingFlags.Public);
    }

    public static WindowsIdentity? GetClientIdentity(this HttpContext context)
    {
        if (context == null)
            throw new ArgumentNullException(nameof(context));

        var connectionLifetime = (IConnectionLifetimeNotificationFeature)_connectionLifetimeProperty.GetValue(context.Connection);
        var transportConnection = (BaseConnectionContext)_transportConnectionProperty.GetValue(connectionLifetime);
        var namedPipeStream = (NamedPipeServerStream)_namedPipeProperty.GetValue(transportConnection);
        WindowsIdentity rc = null;
        // this will throw if the client hasn't written anything to the pipe or connected anonymously
        namedPipeStream.RunAsUser(() => rc = WindowsIdentity.GetCurrent());
        return rc;
    }
}

[urielginsburg]

@JamesNK Hi, as the lead who brought gRPC over named pipes/unix domains sockets to aspnetcore, I would love to hear your thoughts about bringing over some of the benefits that are inherent to local IPC. Specifically, identifying the peer of the connection without the need for client certificates or complex application level authentication and (on Windows) the ability to impersonate the peer. I guess impersonation could be implemented on *nix as well, but that would be a complex and potentially platform-specific task.

This ticket suggests exposing the actual transport - the named pipe or socket - of the underlying KestrelConnection and this is a major change, considering the abstraction between the application and transport layers in aspnetcore. I still went this way, mainly because the ability to directly call NamedPipeServerThread.RunAsUser. Another option could be adding extension methods to HttpContext:

[SupportedOSPlatform("windows")]
void RunAsPeer(this HttpContext httpContext);

[SupportedOSPlatform("windows")]
WindowsIdentity GetNamedPipePeerIdentity(this HttpContext httpContext);

[UnsupportedOSPlatform("windows")]
UnixSocketPeer GetUnixDomainSocketPeerIdentity(this HttpContext httpContext);

struct UnixSocketPeer
{
    uint ProcessId;
    uint UserId;
    List<uint> Groups;
}

As I said, I would love to hear your thoughts on the subject.

cheers, Uriel

[amcasey]

James is definitely the expert here, but I have some naive questions of my own. Why does this need to be exposed on ConnectionInfo? I would have thought it would be like ITlsConnectionFeature, where connections that have sensible values can expose the feature and others can not. Would that be impractical to implement/consume?

[urielginsburg]

@amcasey I thought of ConnectionInfo because my suggestion was to expose the actual connection object - the underlying NamedPipeServerStream or Socket - so that seemed like a logical place to me. I realize it's a big leap from the current level of abstraction between the application and transport layers, so I am open to any suggestion... The point is that IPC has inherent features that should be exposed to implementations, IMHO:

• NamedPipeServerSocket has the RunAsClient and GetImpersonationUserName methods.
• In *nix, libc has the LOCAL_PEERCRED socket option that retrieves the client process's pid, uid and gid's.

I see two ways to go about implementing this:

1. Expose the Socket or NamedPipeServerStream objects. Pros: gives implementers the freedom they expect when implementing IPC mechanisms (which are inherently different from what server applications normally do in a distributed environment). Cons: it's a big leap from the current paradigm of separating the application from the transport.

2. Implement platform specific functions:

   • On Windows, implement methods like GetClientWindowsIdentity() and RunAsClient()
   • On *nix, implement something like GetClientUnixIdentity(). Pros: more focused on the functionality, keeps the abstraction in place. Cons: there may be IPC use cases we're not thinking of that would require access to the Socket or NamedPipeServerStream.

I guess a third option could be to do both :)

[amcasey]

Sorry, I think my question was unclear. What I meant was, if we were going to expose the NamedPipeServerSocket, why wouldn't it be on an INamedPipeConnectionFeature? That feels more idiomatic than putting virtual methods on base types.

[urielgin]

You're absolutely right... I just thought that since exposing the underlying transport is such a 180 from the existing model of separation it deservesa method call with a 'Dangerous' prefix, similarly to SafeHandle.DangerousGetHandle etc. I placed it in ConnectionInfo because it's a part of the connection - both the socket and the pipe are members of a KestrelConnection<T> derivative - it just seemed like the correct place. I did add a feature for this in my branch when I was plsying around with this.

However, since I'm not a contributor (yet! :) I will go with whatever style and conventions are usually employed in aspnetcore for stuff like this. I would love for this to be my first contribution.

[amcasey]

That's great! If you'd like to move forward, I think the way to go would be to follow the model we used for SslStream. You can see that https://github.com/dotnet/aspnetcore/pull/46824 started off in the same way - with a request for access to backing objects. It was followed by an API proposal (I'll tag this issue so the bot provides instructions) and then the PR that addressed both. In my experience, it's best to have at least a prototype implementation before creating the API proposal - something to convince yourself that the API can be implemented efficiently - but you have to balance that against the fact that API review will likely suggest changes and may even reject the proposal. How does that sound?

[amcasey]

Hmm, I definitely thought the bot would react to that label...

In any case, you basically just file a new issue, but use the API Proposal template.

[urielgin]

Ok great, I'm super excited about this opportunity to contribute.

How about this - I will submit an API proposal with two alternatives:

1. Exposing the actual NamedPipeServerStream and Socket objects, each in its own feature interface, added by each respective connection object.
2. Exposing only specific methods, again in features added by each respective connection object:
   • An INamedPipeServerStreamFeature interface exposing RunAsClient and GetImpersonationUserName.
   • An ILocalPeerCredFeature exposing the xucred struct or a facsimile thereof and implemented by calling getsockopt(...LOCAL_PEERCRED...) on *nix systems.
3. Maybe a third option would be to implement both - 1 as suggested above and 2 as extension methods rather than features.

As an insider, which do you think is more in line with existing style and conventions and is more likely to pass review?

Thanks! Uriel

[amcasey]

In fact, the API proposal template has a specific section for alternative approaches, so I think providing multiple solutions is a great way to go. 😄

I think having optionally-available features is the way to go, but I don't have strong feelings about whether those features should expose the underlying objects or just their most relevant functionality. Personally, I'd probably lean towards just exposing the objects directly, so that we don't have a stream of requests to add their members one-by-one, but there's a reasonable argument to be made for exposing as little as possible without a definite use case.

[amcasey]

Whoops, before you get too far, take a look at https://github.com/dotnet/aspnetcore/blob/3e168fe85e2330621ed12d9c9524c80e0f743dc6/src/Servers/Connections.Abstractions/src/Features/IConnectionNamedPipeFeature.cs and make sure it's not already doing what you need. There's probably one for sockets as well.

[urielgin]

Oh poop on a stick! Now I'm embarrassed. There's a socket one too.

I guess this has been a pipe dream. Har har har.

[amcasey]

No worries. Looking forward to your first contribution. 😃