On Fri, Mar 17, 2017 at 2:52 AM, Ilya Skriblovsky <ilyaskriblovsky@gmail.com> wrote:
The code calling request.URLPath(), in a given Resource, or application, is highly unlikely to know whether it wants to honor (x-)forwarded-for.
You are right, I haven't thought about it.
But I'm in doubt whether trusting X-Forwarded-* by default can damage security if Twisted app is running with naked HTTP(S) port exposed without reverse proxy that handles these headers.
There are three headers:
1. X-Forwarded-For specifying original client IP and IPs of proxies
2. X-Forwarded-Host specifying original Host header from the client
3. X-Forwarded-Proto specifying original client's scheme
(there is also new-style "Forwarded:" header but it is not widely used yet, AFAIK)

X-Forwarded-For definetly can't be trusted if comes from untrusted client client. Fortunately we don't need it at all for generating URLs :) It will be in question when refactoring getClientIP() somewhen later.

But can we trust X-Forwarded-Host & X-Forwarded-Proto? From the first glance it isn't a problem since we are using them to display URLs for the same client, so nasty client will get his nasty URLs, that's it. But if app is doing something like storing URL in DB or (more likely) sending an email with a link to another client, this would be an issue.

It's not safe to enable support for X-Forwarded-For and friends by default, since you can't know how application code will use that information and in many configurations it may be spoofed by clients.

A proper deployment which sets these headers looks like this:

1. Configure your frontend reverse-proxy (nginx or whatever) to discard incoming X-Forwarded-* headers and set X-Forwarded-* as appropriate. Note that X-Forwarded-For is actually a list of hops[1], so if you do this naively your server may append to the list! [2]
2. Configure your backend services to respect exactly the headers passed by your frontend.

Note that this requires administrative action in two places, and that if you don't do the frontend config it will probably pass the headers through, allowing them to be spoofed.

A non-exhaustive list of possible bad stuff I client could use spoofing for:

1. Evade IP-based access control (a reasonable defense-in-depth measure).
2. Evade pinning of user sessions to IP addresses or subnets.
2. Evade IP-based rate limiting, e.g. as discussed at [3].

It's also probably worth noting that Django used to offer support for X-Forwarded-For and removed it[4]. X-Forwarded-* and friends just too varied in the wild to reasonably support.

If Twisted is to support this in any way, I think that it should be opt-in support for the Forwarded header as specified in RFC 7239. This should be a parameter applicable to all of twisted.web.server rather than per-method call, since it's something the administrator needs to set.

—Tom

[1]: Standardized as Forwarded in https://tools.ietf.org/html/rfc7239
[2]: Your frontend proxy should also validate Host is a domain you control to prevent cookie theft. Plus lots of other stuff. Web security is hard, and every default everywhere sets users up for failure.
[3]: http://django-ratelimit.readthedocs.io/en/v1.0.0/security.html
[4]: https://www.djangoproject.com/weblog/2009/jul/28/security/#secondary-issue