SAML-Toolkits / python3-saml

MIT License
683 stars 304 forks source link

Optionally modify request based on X-Forwarded headers in demo_pyramid #230

Closed daxxog closed 3 years ago

daxxog commented 3 years ago

This PR helps when running a reverse proxy in front of the pyramid demo by overriding request.scheme and request.server_port when the proper X-Forwarded headers are set.

pitbulk commented 3 years ago

Can we add those lines commented? I don't want to force everyone to have that enabled.

daxxog commented 3 years ago

@pitbulk They are conditional and wouldn't be used unless those header(s) are set. What would be the downside of having these in here uncommented?

e.g. in nginx

        proxy_set_header        Host               $host;
        proxy_set_header        X-Real-IP          $remote_addr;
        proxy_set_header        X-Forwarded-For    $proxy_add_x_forwarded_for;
        proxy_set_header        X-Forwarded-Host   $host:443;
        proxy_set_header        X-Forwarded-Server $host;
        proxy_set_header        X-Forwarded-Port   443;
        proxy_set_header        X-Forwarded-Proto  https;
pitbulk commented 3 years ago

The X-Forwarded-For, X-Forwarded-By, X-Forwarded-Proto, and X-Forwarded-Host headers are not trusted for security reasons, because it is not possible to know the order in which already existing fields were added (as per Forwarded HTTP Extension).

https://devcenter.heroku.com/articles/http-routing#heroku-headers

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.

https://tools.ietf.org/html/rfc7239.html 8.1. Header Validity and Integrity

daxxog commented 3 years ago

Make sense, I will update this PR in a few days to reflect your suggested changes and put some comments so users know what this bit is for.