Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What's the best choice for a non-root port for ASP.NET Core? #43149

Closed
richlander opened this issue Aug 8, 2022 · 21 comments
Closed

What's the best choice for a non-root port for ASP.NET Core? #43149

richlander opened this issue Aug 8, 2022 · 21 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions design-proposal This issue represents a design proposal for a different issue, linked in the description docker

Comments

@richlander
Copy link
Member

We're looking at supporting non-root container images. See: dotnet/dotnet-docker#3968

We're part way down this path already: https://github.com/dotnet/dotnet-docker/blob/2aeeffadbf4f31c062ba20d46efb4782d62e9a43/src/runtime-deps/6.0/cbl-mariner2.0-distroless/amd64/Dockerfile#L52

We've chosen port 8080 so far. That's a reasonable choice. We want to make a long-term decision on our default non-root port.

The leading contenders for HTTP and HTTPS are:

  • 5000 and 5001
  • 8080 and 8081

I have a bias to the 500x option since its what ASP.NET Core uses by default for localhost dev.

The request for this info is coming from the .NET container team. We feel like the ASP.NET Core team should (A) make this decision, and (B) ensure it remains a good decision long-term.

@mthalman @MichaelSimons @jander-msft @bradygaster @LadyNaggaga @davidfowl @glennc

@richlander richlander added the design-proposal This issue represents a design proposal for a different issue, linked in the description label Aug 8, 2022
@richlander
Copy link
Member Author

richlander commented Aug 8, 2022

I'm proposing that we move to a single set of ports for both rootful and non-root container images.

Here's what that would look like:

  • For .NET 7
    • Rootful: ASPNETCORE_URLS=http://+:80
    • Non-root: ASPNETCORE_URLS=http://+:5000
  • For .NET 8
    • Rootful: ASPNETCORE_URLS=http://+:80;http://+:5000
    • Non-root: ASPNETCORE_URLS=http://+:5000
  • For .NET 9
    • Rootful: ASPNETCORE_URLS=http://+:5000
    • Non-root: ASPNETCORE_URLS=http://+:5000

There is a break, but we give one release for folks to adapt in a compatible way. Or we could leave port 80 there forever and enable folks to map whatever of those ports they like. We could also leave more than one release before we pull 80, however, I think that's a less good option since the break will be more difficult to explain if we only remove it after 5 years.

If we decide that 800x or 808x is better, than that's what we'll use instead for non-root. For TLS, we'll recommend the 1 version, like 5001 or 8081.

@Tratcher
Copy link
Member

Tratcher commented Aug 8, 2022

Ports selection is pretty arbitrary. I can only think of two meaningful criteria:

  • Consistency with other systems, picking a default that's most likely to work with whatever is running in the container (Kestrel).
  • Avoids conflicts with other resources

@richlander
Copy link
Member Author

As a default, nothing else will use port 5000 since nothing else is in our containers by default. Also, we're more likely to see conflicts on the dev desktop than anywhere else.

@davidfowl
Copy link
Member

5000 and 5001 how about that 😄

@richlander
Copy link
Member Author

richlander commented Aug 9, 2022

Well, I like it!

What do you think of the plan, including adding two ports for .NET 8? We could do that for .NET 7, too, and then have two releases before pulling port 80 in .NET 9.

@bradygaster
Copy link
Member

This seems good. If @blowdart lacks concern for the approach, I think we can cancel the meeting now. :)

@blowdart
Copy link
Contributor

blowdart commented Aug 9, 2022

Have the meeting. And invite me.

@richlander
Copy link
Member Author

richlander commented Aug 9, 2022

Did some research with OpenShift with help from @omajid.

Both podman and OpenShift are rootless by default.

The Java example is the most interesting. Three ports are exposed:

  • 8080
  • 8443
  • 8778

I like the 8080, 8443 duo. They are self-descriptive as to what they are both for, much more so than 5000 and 5001. The third port is for monitoring with solutions like: https://jolokia.org/features-nb.html. We don't need this one, but it it is still interesting.

I think it is is compelling to just say "we're matching Red Hat here; that approach LGTM." Assuming that, then I'd propose we expose both 8080 and 8443 and by "expose", I mean "listen on".

@halter73
Copy link
Member

halter73 commented Aug 10, 2022

I looked at the linked OpenShift Dockerfiles and only 8080 is exposed by all but the Java one. Even the dotnet one uses just "http://*:8080" with no HTTPS enabled.

Could we stop exposing HTTPS by default in our docker images? It's a hassle dealing with certs and exposing two ports by default for functionality most containers do not need. Sure, the app might be doing HSTS, but it might not be either. Or it might be done outside of the container by a reverse proxy. Apparently, the people using the OpenShift dotnet images don't have a problem with no HTTPS by default. Maybe HTTPS could be opt-in. No HTTPS would mean no HTTP/2 or HTTP/3 to the container without some "prior knowledge" setup, but I'm not sure how common HTTP/2 or HTTP/3 connections into a container are.

I can see how given that we expose both HTTP and HTTPS ports from our docker images, ports 8080 and 8443 could make it clearer what each port is for at least to dotnet newcomers. Maybe we should have chosen these as our defaults instead of 5000 and 5001 long ago, but we made the 5000 the default port in Kestrel before we even added HTTPS support.

I think we stick to the default ports we already have elsewhere. 5001 and 5001. If it's a single non-HTTPS port, then can we just stick to 5000 and there's no confusion. I don't want to change our "non-root" docker binding away from 5000 if we don't have to, but I agree making it the same for root and non-root makes sense.

I think the bigger problem is the Kestrel binds only to loopback by default if no "URLS" config is passed in from hosting, but I guess that's a separate issue.

@davidfowl
Copy link
Member

I think the bigger problem is the Kestrel binds only to loopback by default of no "URLS" config is passed in from hosting, but I guess that's a seperate issue.

I think this is the least important case tbh.

I think we stick to the default ports we already have elsewhere. 5001 and 5001. If it's a single non-HTTPS port, then can we just stick to 5000 and there's no confusion. I don't want to change our "non-root" docker binding away from 5000 if we don't have to, but I agree making it the same for root and non-root makes sense.

+1, we can just default to 5000.

@DamianEdwards is out but I don't know what we decided for https based on docker

@richlander
Copy link
Member Author

richlander commented Aug 10, 2022

It's up to you guys. 500x and 8xxx are equally good for me.

I think registering both ports is good because it reduces the set of things you need to change to configure ASP.NET Core for common scenarios. Is there an actual downside to registering both ports?

Could we stop exposing HTTPS by default in our docker images?

We don't today. This is a proposed change to expose both HTTP/HTTPS. It's a separate decision from exposing non-root ports.

@halter73
Copy link
Member

halter73 commented Aug 10, 2022

I think this is the least important case tbh.

I see where you're coming from. It rarely falls back that far, but we should be defaulting to "localhost" less everywhere.

Is there an actual downside to registering both ports?

It adds to the cognitive load. There's more that can go wrong with cert loading and stuff. What cert would we even use? Not the dev cert, I assume.

@richlander
Copy link
Member Author

I see it the opposite. The cognitive load is needing to know all these knobs to turn. Our current HTTPS docs are pretty complicated. I would like to make our instructions simpler, more prescriptive, and have them work for more cases w/o users knowing stuff.

Dev cert vs real cert is immaterial for this conversation.

My feedback on ASPNETCORE_URLS is similar. How do we reduce the cognitive load of (what I see as) cryptic syntax.

@davidfowl
Copy link
Member

I would like to make our instructions simpler, more prescriptive, and have them work for more cases w/o users knowing stuff.

I'm afraid that can't be a thing for HTTPS, its complicated. Managing certificates is platform specific and an absolute PITA. That said, we can give people instructions on how to configure it. Hopefully our tooling does the needful when you launch in VS (injects stuff into the container so that it works).

My feedback on ASPNETCORE_URLS is similar. How do we reduce the cognitive load of (what I see as) cryptic syntax.

I was hoping this issue didn't turn into #43135 😄

@richlander
Copy link
Member Author

Here are our instructions on HTTPS: https://github.com/dotnet/dotnet-docker/blob/main/samples/host-aspnetcore-https.md. Yes, that's an involved exercise although the differences across OS are mostly pathing. I agree that exposing the port is just one small part.

Right, right. No need to re-create the other topic here.

@halter73
Copy link
Member

halter73 commented Aug 12, 2022

I'm really glad we don't currently expose HTTPS by default in our docker images, and I don't think we should. Most people do not need to terminate HTTPS in their docker container, and adding HTTPS support by default definitely adds to the cognitive load for a lot of developers who don't need this stuff right off the bat when they're learning how to use ASP.NET Core in docker.

Look at an HTTPS docker-compose.debug.yml:

version: '3.4'

services:
  webapp:
    image: mcr.microsoft.com/dotnet/core/samples:aspnetapp
    ports:
      - 80
      - 443
    environment:
      - ASPNETCORE_ENVIRONMENT=Development
      - ASPNETCORE_URLS=https://+:443;http://+:80
      - ASPNETCORE_Kestrel__Certificates__Default__Password=password
      - ASPNETCORE_Kestrel__Certificates__Default__Path=/https/aspnetapp.pfx
    volumes:
      - ~/.aspnet/https:/https:ro

Compared to a non-HTTPS docker-compose.debug.yml:

version: '3.4'

services:
  webapp:
    image: mcr.microsoft.com/dotnet/core/samples:aspnetapp
    ports:
      - 80
    environment:
      - ASPNETCORE_URLS=https://+:80
      - ASPNETCORE_ENVIRONMENT=Development

If we follow through with my suggestion of binding to "::" instead of "localhost" by default in Kestrel, we could remove the ASPNETCORE_URLS=https://+:80 part too and just expose port 5000.

version: '3.4'

services:
  webapp:
    image: mcr.microsoft.com/dotnet/core/samples:aspnetapp
    ports:
      - 5000
    environment:
      - ASPNETCORE_ENVIRONMENT=Development

And with the second version, you'd never have to touch dotnet dev-certs or do anything else with certs! Most developers would consider that a huge win 😆

Yes, that's an involved exercise although the differences across OS are mostly pathing.

I'd say the biggest difference is we give no guidance whatsoever about how to trust the certificate on Linux. I know it's hard because Linux is so fragmented and different applications can use different CA stores, but it is sad.

Our docker HTTPS docs also assume that you always have a pfx cert. Kestrel supports pem certs too, but the docker HTTPS docs don't cover it. We have other docs that do, but it just goes to show how HTTPS adds to the complexity.

@davidfowl
Copy link
Member

If we follow through with my suggestion of binding to "::" instead of "localhost" by default in Kestrel, we could remove the ASPNETCORE_URLS=https://+:80 part too and just expose port 5000.

Right, that only shows the docker scenario though and ignores all of the other ones. So we can't make that decision based on this issue.

@richlander
Copy link
Member Author

we could remove the ASPNETCORE_URLS=https://+:80 part too and just expose port 5000.

I'm unaware that you can expose a loopback port outside the container. Wouldn't that be odd?

I document that here:
dotnet/dotnet-docker#3968 (comment)

HTTPS

Let's remove that from that conversation. It's not the key point.

definitely adds to the cognitive load for a lot of developers

However, this is objectively not the case. AFAICT, basically zero developers using our containers realizes that they are relying on this ENV being set or look at it. There is zero cognitive load today because it is entirely mysterious.

To the defense of @davidfowl, this is exactly why he keeps on saying "maybe we just move it to code."

  • ASPNETCORE_ENVIRONMENT=Development

Are you proposing using the Development mode at prod? Or are you proposing that we make a new launchSettings.json for this?

I think of launchSettings.json as a dev-time feature primarily. I'd much rather we went the other direction and guided that developers should add this file to .dockerignore.

I think we're demonstrating that we don't have a good system here. It doesn't feel like a designed system.

mcr.microsoft.com/dotnet/core/samples:aspnetapp

That's old. Use mcr.microsoft.com/dotnet/samples:aspnetapp

@halter73
Copy link
Member

I'm unaware that you can expose a loopback port outside the container. Wouldn't that be odd?

The point is that we could start binding to all interfaces instead of loopback by default. Based on the journey you described in dotnet/dotnet-docker#3968 (comment), I think that binding to loopback instead of all interfaces was the most confusing problematic thing. You identified binding to loopback as one of the "three distinct choices here that led to this poor outcome", and I agree. I think it's the most fundamental issue.

@mthalman
Copy link
Member

Is there any more information necessary here to be able to make a decision? I'm happy to help provide anything that's still needed.

@wtgodbe wtgodbe added this to the .NET 8 Planning milestone Sep 7, 2022
@adityamandaleeka
Copy link
Member

@mthalman @richlander Looks like we had the discussion over at dotnet/dotnet-docker#3968 (comment) . Can this issue be closed now?

@ghost ghost locked as resolved and limited conversation to collaborators Mar 9, 2023
@amcasey amcasey added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions design-proposal This issue represents a design proposal for a different issue, linked in the description docker
Projects
None yet
Development

No branches or pull requests