RequireLocalPort and related extensions for IEndpointConventionBuilder
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 281
Description
## Background and Motivation
This is basically a continuation of the discussion in #46057 which got closed due to missing author feedback.
Currently the asp.net core API surface offers no straight-forward way to restrict access to an endpoint to a specific port or host. In many locations on the net, including Microsoft documentation like [Health-Checks](https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/health-checks?view=aspnetcore-7.0#require-host) patterns like
```csharp
app.MapHealthChecks("/healthz")
.RequireHost("*:5001");
```
are suggested to enforce a port restriction. However this is an unsafe way to implement such a restriction as host matching is only performed based on the often user controllable Host header of the request. This is especially dangerous as the restriction appears to work when using a benign client like a browser.
https://learn.microsoft.com/en-us/aspnet/core/fundamentals/routing?view=aspnetcore-7.0#host-matching-in-routes-with-requirehost and the extension method documentation in https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.builder.routingendpointconventionbuilderextensions.requirehost?view=aspnetcore-7.0 also do not make it clear that this relies on the potentially user controlled Host header field.
To make it easy for users to get correct and safe behavior when trying to restrict access to a specific port, having extension methods to match on `HttpContext.Connection.LocalPort` would be helpful. A related though probably rarer and more complicated problem would be to want to require connections from a specific ip (or interface) if multiple are bound. The basic ip restriction version could match on `HttpContext.Connection.LocalIpAddress` . To get full control the ability to match exactly what the server actually [listens on](https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.core.kestrelserveroptions.listen?view=aspnetcore-7.0) (e.g. could even be a unix domain socket) would be needed.
I reported the specific health check documentation issue in https://github.com/dotnet/AspNetCore.Docs/issues/29399 and was asked to create a more general issue here.
## Proposed API
As suggested in #46057 in addition to improving the documentation on `RequireHost`, there could be a `RequireLocalPort` extension method for `IEndpointConventionBuilder`.
Additionally a corresponding `RequireLocalIpAddress` could be added.
```diff
namespace Microsoft.AspNetCore.Builder;
public static class RoutingEndpointConventionBuilderExtensions
{
+ public static TBuilder RequireLocalPort(this TBuilder builder, params int[] ports) where TBuilder : IEndpointConventionBuilder { ... };
+ public static TBuilder RequireLocalIpAddress(this TBuilder builder, params string[] ips) where TBuilder : IEndpointConventionBuilder { ... };
}
```
To give full control from where a connection is coming from, additional extension methods matching the corresponding listen endpoint possibilities could be added. However I do not think I have enough framework knowledge to propose good and implementable api for these. Maybe something in the direction of:
```diff
namespace Microsoft.AspNetCore.Builder;
public static class RoutingEndpointConventionBuilderExtensions
{
+ public static TBuilder RequireNetEndPoint(this TBuilder builder, params System.Net.EndPoint[] socketEndpoint) where TBuilder : IEndpointConventionBuilder { ... };
}
```
## Usage Examples
```csharp
app.MapGet( "/only5001", () =>"Only reachable on port 5001")
.RequireLocalPort(5001);
app.MapGet( "/onlyloopback", () => "Only reachable through loopback")
.RequireLocalIpAddress("127.0.0.1", "[::1]");
```
```csharp
UnixDomainSocketEndPoint domainSocket = ...;
app.MapGet( "/onlydomainsocket", () => "Only reachable on domain socket")
.RequireNetEndPoint(domainSocket);
```
## Alternative Designs
- To simplify usage something like a `RequireLocalHost` analog to `RequireHost` could be introduced that understands things like `localhost:1234`, `*:80`, `127.0.0.1.:*`. However this is easy to confuse with `RequireHost` and due to the flexible interface it is not so simple to answer what should happen if it were given a DNS name or any other current/future options `RequireHost` might support.
- `RequireLocalIpAddress` could take an actual `IPAddress` object instead of a string. In practice I think this would just lead to the vast majority of users having to add a `IPAddress.Parse` into the call. Of course both overloads could be offered.
- The methods could be abbreviated to `RequirePort` and `RequireIpAddress`. This makes it harder to understand that this matches a specific part of the `HttpContext.Connection` and if "remote" were added later could be slightly harder to understand. On the other hand the "Local" part might confuse some.
cc: @guardrex https://github.com/dotnet/AspNetCore.Docs/issues/29399
Contributor guide
Assessment
This issue has not been assessed yet.