"trust proxy" setting about to change

[dougwilson]

@defunctzombie I'm about to change how the "trust proxy" setting works. Do you want me to send a pull request with the changes here or should I have commit access?

[defunctzombie]

Documentation or actually a breaking change in express?

[dougwilson]

Just documentation of new options and to not encourage people to set it to "true" any more, no breaking change in express :)

[defunctzombie]

Why do we not encourage setting to true? Is there a better way to get
client IP now?
On May 7, 2014 11:51 AM, "Douglas Christopher Wilson" <
[email protected]> wrote:

Just documentation of new options and to not encourage people to set it to
"true" any more, no breaking change in express :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/152#issuecomment-42467532
.

[dougwilson]

See expressjs/express#2099 for the change rational. Basically, setting it to true will use the left-most X-Forwaded-For address, which is not safe. "trust proxy" is currently does not work in a dual environment where requests may come through a proxy or directly.

Soon you will give "trust proxy" the list of addresses you trust to be your proxy and it will return the appropriate trusted value. For example with the header X-Forwared-For: 10.0.0.1, 10.0.0.2, 10.0.0.3 and the socket's remoteAddress is 10.0.0.4:

• true gives you 10.0.0.1 (the left-most)
• "10.0.0.4" gives you 10.0.0.3 (the right-most because you only trust your direct connection)
• ["10.0.0.4", "10.0.0.3"] gives you 10.0.0.2 because you trust the first proxy and the direct connection.

A simple example is if you have "trust proxy" set to true and have Apache or nginx as your reverse proxy, and set the request with X-Forwaded-For: 0.0.0.0, the request from nginx/Apache will have X-Forwarded-For: 0.0.0.0, 10.0.0.1 where 10.0.0.1 is the real client, but you get back 0.0.0.0 in express because all the headers were blindly trusted.

[defunctzombie]

I am not sure I follow this description. What if you don’t know the address of your proxy server? (This seems common to me).

Is there really no way to get the client ip address without all this mess or configuration of the express app?

On May 7, 2014 at 12:01:15 PM, Douglas Christopher Wilson ([email protected]) wrote:

See expressjs/express#2099 for the change rational. Basically, setting it to true will use the left-most X-Forwaded-For address, which is not safe. "trust proxy" is currently does not work in a dual environment where requests may come through a proxy or directly.

Soon you will give "trust proxy" the list of addresses you trust to be your proxy and it will return the appropriate trusted value. For example with the header X-Forwared-For: 10.0.0.1, 10.0.0.2, 10.0.0.3 and the socket's remoteAddress is 10.0.0.4:

true gives you 10.0.0.1 (the left-most)
"10.0.0.4" gives you 10.0.0.3 (the right-most because you only trust your direct connection)
["10.0.0.4", "10.0.0.3"] gives you 10.0.0.2 because you trust the first proxy and the direct connection.
A simple example is if you have "trust proxy" set to true and have Apache or nginx as your reverse proxy, and set the request with X-Forwaded-For: 0.0.0.0, the request from nginx/Apache will have X-Forwarded-For: 0.0.0.0, 10.0.0.1 where 10.0.0.1 is the real client, but you get back 0.0.0.0 in express because all the headers were blindly trusted.

—
Reply to this email directly or view it on GitHub.

[tj]

I've never had this issue, we have nginx just clobber any which may be spoofed. If there are other solutions for the less common cases that's cool but we should maintain what makes sense now for the typical single-reverse proxy use-case

[defunctzombie]

I agree with @visionmedia here, I think this will just lead to more confusion. Your reverse proxy needs to be configured to send the proper client ip. Anything else and it seems like that is a bug/misconfiguration of the reverse proxy (unless I misunderstand this issue).

[dougwilson]

When your server is setup within your corporate environment, your employees can typically call it directly and send false X-Forwarded-For headers, which would default logging the IP address in your express app, would it not? Not even employees, but a compromised server within the firewall as well could make requests to your express app with false X-Forwarded-For headers.

Also, if you put your server behind a AWS Elastic Load Balancer, it will just append to X-Forwarded-For, making it impossible to use "trust proxy" behind that.

[tj]

could be wrong but it seems like the simplest solution would just be trusting N hops

[defunctzombie]

When your server is setup within your corporate environment, your employees can typically call it directly

Why? This is a choice, doesn't have to be this way at all, they can still use some reverse proxy. I personally would not expose the app directly.

AWS Elastic Load Balancer

What do you mean by "impossible"? Does it not have a way to make the first IP the correct client IP?

Are we able to always just use the last ip then instead of the first?

[dougwilson]

Does it not have a way to make the first IP the correct client IP?

The first IP will always be the cleint ID that called to AWS or whatever was in the X-Forwarded-For from the client, which the client can make a request with some other IP in there and express will use that, rather than what IP called AWS.

Are we able to always just use the last ip then instead of the first?

As long as there are not multiple proxy hops. Our main environment has both 2 hops and 1 hop, depending on the part of the world the request came from.

This is a choice, doesn't have to be this way at all, they can still use some reverse proxy

Unless you put a firewall between every single server on the network, typically production servers can all talk to each other, so a single compromised server can make fake direct calls or if employees can access the inner production ring, they can also make bad calls.

[tj]

As long as there are not multiple proxy hops. Our main environment has both 2 hops and 1 hop, depending on the part of the world the request came from.

ahhh ok, makes sense, kind of a shitty situation but makes sense. I would almost rather structure apps in such a way that private/internal apis are separated from public-facing stuff to avoid issues like this, but maybe that's not a reasonable solution haha

[tj]

tempted to just remove this shit and do x-ip or something, I've never made use of the list in an app

[dougwilson]

tempted to just remove this shit and do x-ip or something, I've never made use of the list in an app

Yea, the list does suck. At least Mashery just sends a single Client-IP header and doesn't involve this whole X-Forwarded-For chain crap.

kind of a shitty situation but makes sense

I know. The proxying stuff is done by a third-party that won't change because they say "setting the chain in X-Forwarded-For is the proper behavior".

My main problem (maybe the same problem that the guy from the express issue has) is that the corporate security required us to remedy the situation where they could call our services with a forged X-Forwarded-For header and people using req.ip in their apps where seeing the forged address.

[tj]

yea we're using a single for ours as well right now which is why ELB is no big deal, maybe removing it and delegating weird solutions is the way to go hahaha. tough call

[dougwilson]

Yea. I was allowed to take our X-Forwarded-For parsing solution and open source it, which currently lives at https://github.com/expressjs/proxy-addr which allowed us to configure the proper trust chain and satisfy security, but some of our teams want to use req.ip, req.host, req.protocol, etc. lol (especially since there are third-party modules reading the value).

[tj]

yea, req.protocol is probably the most useful one there hmm

[dougwilson]

@visionmedia would you be interested in seeing the WIP for possibly integrating into express 3.x? It keeps things like app.enable('trust proxy') fully working and completely trusting.

[tj]

Do you think we should add just simple trust-N support as well? so much configuration :( haha

[dougwilson]

Do you think we should add just simple trust-N support as well?

We sure can as a plain number, if you want :)

so much configuration :(

I feel ya :(

[dougwilson]

Any further comments on this? I would like to support these scenarios:

// current functionality (staying the same)
app.enable('trust proxy'); // trust all
app.set('trust proxy', true); // same as above
app.disable('trust proxy'); // trust none; default
app.set('trust proxy', false); // same as above

// possible new functionality
app.set('trust proxy', 1); // trust 1 hop blindly
app.set('trust proxy', '127.0.0.1'); // trust localhost only; good for app listening on 80 and separate ssl terminator listening on 443
app.set('trust proxy', '127.0.0.1,10.0.0.1'); // trust multiple hosts
app.set('trust proxy', '10.0.0.1/26'); // trust some subnet, like an entire prod environment
app.set('trust proxy', function(addr){ /* do whatever you want */ }); // trust however you like

[tj]

SGTM

[crandmck]

I would like to close this issue, since the code changes have been made and #180 covers it more succinctly. Is there a reason to keep this open as well?