benoitc / gunicorn

gunicorn 'Green Unicorn' is a WSGI HTTP Server for UNIX, fast clients and sleepy applications.
http://www.gunicorn.org
Other
9.89k stars 1.76k forks source link

Support the Forwarded header (RFC 7239) #1544

Closed vfaronov closed 2 years ago

vfaronov commented 7 years ago

Gunicorn can determine wsgi.url_scheme from a number of non-standard headers, such as X-Forwarded-Proto, controlled by the secure_scheme_headers setting.

RFC 7239 defines a standard Forwarded header to incorporate all the information that is typically spread all over the various X-Forwarded-* and Via headers. For example, Forwarded: for=123.45.67.89;proto=https means that the last inbound server on the chain received the request with the https scheme.

It would be nice if Gunicorn could determine wsgi.url_scheme from this header as well. Unfortunately, the secure_scheme_headers setting does not suffice for this, because the value of Forwarded is not a fixed string; it has to be carefully parsed.

benoitc commented 7 years ago

The forward scheme is not the same as supporting secure scheme headers coming from the server.

We used to support part of the Forward api but it has been removed: https://github.com/benoitc/gunicorn/commit/c4873681299212d6082cd9902740eef18c2f14f1

also if we support it we need to be able to whilelist the forward addresses we can accept

8.1. Header Validity and Integrity

The "Forwarded" HTTP header field cannot be relied upon to be correct, as it may be modified, whether mistakenly or for malicious reasons, by every node on the way to the server, including the client making the request.

One approach to ensure that the "Forwarded" HTTP header field is correct is to verify the correctness of proxies and to whitelist them as trusted. This approach has at least two weaknesses. First, the chain of IP addresses listed before the request came to the proxy cannot be trusted. Second, unless the communication between proxies and the endpoint is secured, the data can be modified by an attacker with access to the network.

vfaronov commented 7 years ago

@benoitc

The forward scheme is not the same as supporting secure scheme headers coming from the server.

I’m not sure what you mean? The reverse proxy uses a special header to pass the scheme information to the upstream server. It may use a non-standard header such as X-Forwarded-Proto, or it may use the standardized Forwarded: proto. You don’t have to trust everything else a Forwarded header might contain.

also if we support it we need to be able to whilelist the forward addresses we can accept

I think a better approach would be a setting to control which index in the Forwarded header to look at, sort of like the num_proxies parameter to Werkzeug’s ProxyFix. For example, if Gunicorn is sitting behind two reverse proxies, it might look at the proto value in the second-last element of Forwarded. Any forwarded_allow_ips still applies.

benoitc commented 7 years ago

this is what did the code (looking at the end of the proxy chain). and this what make the difference with using the non standard headers filled by the server on top. We had only one to check.

Also as a side note, one reason to the removal was also the existence of application codes/middlewares that were already using the forwarded headers for their own purpose. So their must be a way to disable the support of the forward protocol if needed. And we should probably discuss what should be the default since it's a breaking change.

Anyway I'm not saying we shouldn't implement it, on the contrary, i'm just noting things we should take care for it. Mind to send a PR ? :)