Pylons / waitress

Waitress - A WSGI server for Python 3
https://docs.pylonsproject.org/projects/waitress/en/latest/
Other
1.45k stars 176 forks source link

Fix X-Forwarded-Host #319

Closed invisibleroads closed 4 years ago

invisibleroads commented 4 years ago

Under the previous configuration proxy_set_header X-Forwarded-Host $host:$server_port, request.route_url does not respect a non-standard port such as http://example.com:8000

Under the proposed configuration proxy_set_header X-Forwarded-Host $http_host, request.route_url does respect a non-standard port such as http://example.com:8000

mmerickel commented 4 years ago

What's wrong with the original? Can we see some examples? At the very least it's odd that the next line showing X-Forwarded-Port is still using $server_port. So I'd like to know if that should be dropped or changed or something.

stevepiercy commented 4 years ago

I could not find any RFC that lists http_host as an HTTP header.

The PR should at least reference the SO post https://stackoverflow.com/questions/15414810/whats-the-difference-of-host-and-http-host-in-nginx, which unfortunately does not reference http_host outside the nginx docs or as an RFC. Another SO post goes into more detail https://stackoverflow.com/questions/2297403/what-is-the-difference-between-http-host-and-server-name-in-php

mmerickel commented 4 years ago

I could not find any RFC that lists http_host as an HTTP header.

@stevepiercy this is an issue about the nginx config in the waitress docs, and $http_host is an nginx variable representing the HTTP Host header.

invisibleroads commented 4 years ago

I'm trying to duplicate the behavior using a fresh configuration. Need a few minutes.

On Thu, Oct 8, 2020 at 4:52 PM Michael Merickel notifications@github.com wrote:

I could not find any RFC that lists http_host as an HTTP header.

@stevepiercy https://github.com/stevepiercy this is an issue about the nginx config in the waitress docs, and $http_host is an nginx variable representing the HTTP Host header.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Pylons/waitress/pull/319#issuecomment-705816795, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACBDLESOX5UDFJPXAULWV3SJYRBDANCNFSM4SIF3IAQ .

stevepiercy commented 4 years ago

@mmerickel I informed @invisibleroads of this nginx configuration in a project we both work on, referencing that SO post. It's hard to find that information. I thought maybe there was an HTTP header defined in the RFCs, but there is not. It's just a server variable. Apache and maybe IIS have it as well.

invisibleroads commented 4 years ago

@mmerickel It seems that $host:$server_port and $http_host behave identically when nginx is on the same machine as the pyramid app, but behave differently when nginx and the pyramid app are in different containers.

Here is an example configuration where you can duplicate the different behavior using podman.

https://github.com/invisibleroads/pyramid-reverse-proxy-test

vim /etc/hosts
    127.0.0.1 example.com

vim proxy/example.conf
    proxy_set_header X-Forwarded-Host $host:$server_port;
    # proxy_set_header X-Forwarded-Host $http_host;
bash run.sh
curl example.com:8000
# {"url": "http://example.com/"}

vim proxy/example.conf
    # proxy_set_header X-Forwarded-Host $host:$server_port;
    proxy_set_header X-Forwarded-Host $http_host;
bash run.sh
curl example.com:8000
# {"url": "http://example.com:8000/"}

@stevepiercy This is for a different project.

invisibleroads commented 4 years ago

Another possibility is that this only happens when nginx is served on a non-standard port as described here https://github.com/jupyterhub/jupyterhub/issues/1457 and https://stackoverflow.com/questions/1459739/php-serverhttp-host-vs-serverserver-name-am-i-understanding-the-ma/12046836#12046836.

Most people won't run into this issue unless they are trying to run nginx on a non-standard port for local development, so I reverted the original configuration, which might be more secure as it is does not rely on a request header sent by the client.

@stevepiercy I modified the pull request based on your feedback by adding the reference to the stackoverflow link, reverting the original configuration and adding a separate note for an alternate $http_host configuration for development configurations.

mmerickel commented 4 years ago

I think the issue only happens if you have more than one proxy in line with the app. They need to coordinate properly how they update the header, and the $server_port is only accurate for the proxy closest to the client/ingress. If there are any hops after that, it's no longer the right answer for those hops.

invisibleroads commented 4 years ago

@mmerickel Your explanation makes a lot of sense. That explains why $server_port is empty for the nginx container because it is serving port 80, though the pod is externally mapping port 8000 to the nginx container port 80.

mmerickel commented 4 years ago

I think you'd be better off recommending to not add the extra forwarding headers if you're not the ingress proxy closest to the client. That's where these headers need to be added to the request. There are standards (which this example is not following, nor are super simple, nor are super useful in 90% of scenarios) to cascade the headers into a comma-separated list, but it all depends on topology and we cannot give a full tutorial on how to configure nginx.

mmerickel commented 4 years ago

Sorry, @bertjwregeer pointed out to me what's actually happening here. I was focused on this double-proxy scenario, but in your example it's just an iptables redirect from port 8000 outside to port 80 in the container. The situation is thus that nginx is the first proxy, but it's listening on port 80 and thinks it's receiving packets on port 80 despite the client connecting to port 8000. I do not think there's any "safe" way to handle this and you have to decide what you're going to do based on your deployment because nginx simply cannot know what the original port was in this scenario with any guarantee and it depends on a lot of factors.

I don't think there's a perfect recommendation to suggest here.

digitalresistor commented 4 years ago

I am going to close this request. Waitress should not recommend a particular configuration that may be blindly copy and pasted that could lead to security issues for its users.

Using the Host header directly for the X-Forwarded-Host is not safe in a variety of cases. If you are using port remapping using iptables or some other firewall rule as is the case with docker's port exposing flags, you are responsible for configuring NGINX to correctly know about the port, and can pass it through correctly.