Let the possibility to specify a wildcard for the truted_proxy var

[dulacp]

Here is a suggestion to use the X_FORWARDED_PROTO on cloud based reversed proxies with no fixed ip.

I don't see any personal objection on allowing the wildcard if it is documented well enough for this particular case.

Cheers

[tseaver]

This feature introduces a huge security hole: allowing any host which can see the server set those headers opens a site up to phishing, etc.

I'd be more willing to merge a feature which allowed either a list of trusted proxies, or perhaps a way to spell a subnet (e.g., using the py-ipaddress distribution on Python2.

[mmerickel]

pyramid_debugtoolbar works for subnets including 0.0.0.0/0 as well as specific ip addresses. This is probably a good approach to copy.

[dulacp]

My first approach was indeed to use ip subnet masks, but I didn't want to introduce a dependency without first submitting the idea of trusting not only fixed ip addresses.

I agree that it may introduce a hole, but in the case of Heroku you can't receive wrong headers from Nginx. Well that's what I understand so far from reading the documentation.

I will make the changes then with the ipaddress package (and natively for Python 3).

[tseaver]

Hmm, looking at https://pypi.python.org/pypi/py2-ipaddress, it doesn't appear to support Python 3.2 / 3.3, which we need to support.

[tseaver]

https://pypi.python.org/pypi/ipaddress looks to be a better choice.

[mmerickel]

It's probably a reasonable dependency for a wsgi server to be able to parse ip addresses properly. Alternatively pyramid_debugtoolbar just vendored the ipaddr module.

https://github.com/Pylons/pyramid_debugtoolbar/blob/master/pyramid_debugtoolbar/ipaddr.py

[dulacp]

Exactly, the package ipaddress is needed for Python 2.6, 2.7 until 3.3 where it was included as a built-in module.

[dulacp]

I have replaced the wildcard use by a subnet mask.

So you can specify a trusted_proxy configuration as 192.168.0.1/24 or even could use values like 0.0.0.0/0 if you want to authorize every ip.

This way people can granularly choose the security they need.

Is the implement good for you ?
If it is I could fix the documentation to reflect the changes

Cheers

[dulacp]

The tests failed under Python 3.2, but I can't find the reason why the SyntaxError is raised. If anyone knows Python 3.2 specifics syntax tricks

[dulacp]

I finally found how to fix the tests !

[mmerickel]

Is there a reason you have pinned ipaddress < 1.1? We tend to avoid pinning in packages without reason, allowing users themselves to do the pinning if there is a problem.

Also I believe ipaddress is a backport of a python 3.3 feature, so it should not be depended upon for 3.3+ so a conditional install_requires is necessary.

[dulacp]

Very well, I removed the pinned statement, it is indeed better this way !
And I have only included ipaddress for Python version strictly below 3.3.

[mmerickel]

Could you add a test for the new behavior? It's good to see the current tests pass unchanged for the trusted_proxy setting. We also need to update the docs to reflect the new features that trusted_proxy supports.

[digitalresistor]

I really think this ought to not live in waitress, and instead should be middleware. There's some work that was done in WebOb and I need to take a look at it again, but I think solving it there will be much more useful as a WSGI middleware than special support in the HTTP server.

[mmerickel]

I already argued for this a bit when the feature was added. It's in now, so we can only try to improve it. Point-being I think that your comment is not constructive to this particular PR. :-)

[marcinkuzminski]

hmm

[marcinkuzminski]

ok

[mmerickel]

@dulaccc I think this PR is good but there are outstanding issues on it going back to 2015. We cannot use this code unless you sign the contributors agreement so I must close this. If you'd like to at least add your name to CONTRIBUTORS.txt we can have someone else work on the rest.