aspnet / BasicMiddleware

[Archived] Basic middleware components for ASP.NET Core. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
169 stars 84 forks source link

template/HttpsRedirectionMiddleware assumes https is always available #315

Closed tmds closed 6 years ago

tmds commented 6 years ago

The HttpsRedirectionMiddleware is enabled in the ASP.NET Core templates. This middleware assumes https is always available. This means when deploying application with http only (e.g. Kubernetes or behind ssl terminating load balancer) the application becomes unusable.

The middleware could be smarter and not perform a redirect when there are no https server addresses. Or maybe it should not be in the template.

CC @davidfowl

Tratcher commented 6 years ago

The middleware should not be disabled unless the reverse proxy / load balancer enforces the same behavior. Also, when forwarders are configured correctly then you won't have to disable the middleware. https://github.com/aspnet/Docs/issues/2384

In Kubernetes is ssl also being terminated externally? Or are you really running without https?

tmds commented 6 years ago

In Kubernetes is ssl also being terminated externally? Or are you really running without https?

It could be either. The service is deployed in a Docker container which could be accessed via a private network via http or externally through an ssl terminating load balancer.

We set ASPNETCORE_URLS=http://*:8080 to inform ASP.NET Core we want an http endpoint at port 8080.

Tratcher commented 6 years ago

https://github.com/aspnet/templating/issues/322

javiercn commented 6 years ago

The problem in general here is that we don't know what environment the app is being deployed on. As long as the app is either behind a reverse proxy that forwards headers or exposed directly to external traffic, the redirect middleware does its job and this covers the majority of scenarios.

There are some scenarios in which the app can be exposed to external traffic (behind a reverse proxy, where the HTTPS connections are terminated) and to internal traffic (other services, for example traffic inside a cluster without https)

The app will do the right thing for traffic coming from the outside (will use the forwarded headers and redirect/detect that the traffic was HTTPS)

For internal traffic however, the app will always try to redirect because there are no forwarded headers for internal services communicating through HTTP.

In this case you can simply alter the pipeline as follows:

app.MapWhen(ctx => /* Logic to detect internal traffic */, app.UseHttpsRedirection())

An option to separate internal traffic from external traffic can be just to look at the host header.

tmds commented 6 years ago

The problem in general here is that we don't know what environment the app is being deployed on.

ASPNETCORE_URLS is what the app gets from the environment to figure this out, right?

internal traffic (other services, for example traffic inside a cluster without https)

Which is a common pattern in microservice architecture

In this case you can simply alter the pipeline as follows:

... so it is bad UX if you need to change the template to get it to work.

Also you only get the failure when you deploy, while you are developing things looks like they work fine.

Just thinking ... I find it strange the default template does https. Do you think the majority of ASP.NET Core web apps handle SSL? Or is there some other reason for this choice?

davidfowl commented 6 years ago

Just thinking ... I find it strange the default template does https. Do you think the majority of ASP.NET Core web apps handle SSL? Or is there some other reason for this choice?

It's to make your prod and dev environments look more similar so you can test and experience more things before seeing first time failures at deployment time.

tmds commented 6 years ago

It's to make your prod and dev environments look more similar so you can test and experience more things before seeing first time failures at deployment time.

In this case it has the opposite effect.

I think it would make sense HttpsRedirectionMiddleware figures out there is no https port from ASPNETCORE_URLS.

Tratcher commented 6 years ago

That fails for most reverse proxy scenarios which we still want to support. For something as important as https enforcement we don't want it to silently turn off, you need to know when it's off.

tmds commented 6 years ago

That fails for most reverse proxy scenarios which we still want to support.

I don't understand. Can you elaborate on what this breaks?

For something as important as https enforcement we don't want it to silently turn off, you need to know when it's off.

I agree.

It's to make your prod and dev environments look more similar

What do you think is the common use-case? https handled in ASP.NET Core or externally?

javiercn commented 6 years ago

I don't understand. Can you elaborate on what this breaks?

This is for when you are doing HTTPS termination at the ingress. You will be receiving HTTPS traffic up until it reaches IIS but then communicating internally to Kestrel through HTTP with a given set of forwarded headers.

What do you think is the common use-case? https handled in ASP.NET Core or externally?

There are two aspects to this: 1) Where you are physically handling/terminating https. 2) Where you are logically handling HTTP/S traffic.

The redirect middleware (like the HSTS middleware) handles the logical piece. Doing so has 3 key benefits. 1) Its independent of where you are deploying (whether is Kestrel on the edge, behind IIS, nginx, etc). 2) All the logical configuration for your app is in one place, your app and you have the guarantee that it will work in the same way independent of the environment. 3) It makes it way easier to verify that your app works as you expect at the HTTP level, as you can use things like TestServer to write in-memory integration tests.

so it is bad UX if you need to change the template to get it to work

Templates are a starting point meant to get you started, not a solution that will work across all possible topologies and environments without changes.

tmds commented 6 years ago

This is for when you are doing HTTPS termination at the ingress. You will be receiving HTTPS traffic up until it reaches IIS but then communicating internally to Kestrel through HTTP with a given set of forwarded headers.

I understand the use-case. Can you make more clear what the role is here for the HttpsRedirectionMiddleware?

Templates are a starting point meant to get you started, not a solution that will work across all possible topologies and environments without changes.

Right, templates can't handle every scenario. In microservice architecture, it is common to use http services. The default template fails (in production) for that use-case. It seems like an important use-case where the default template breaks.

Tratcher commented 6 years ago

@tmds for template specific behavior I suggest continuing this on https://github.com/aspnet/templating/issues/322

muratg commented 6 years ago

Closing based on the discussion at https://github.com/aspnet/templating/issues/404