wrap-forwarded-remote-addr ELB compatibility

[g-k]

Can you elaborate on the commit message for https://github.com/ring-clojure/ring-headers/commit/2b3ea70584ae61c1414953180938d2b14153850d?

Middleware function incorrectly used the first rather than last value present in the X-Forwarded-For header. This could result in attackers being able to spoof the :remote-addr key if this middleware was used. Reported by Daniel Compton desk@danielcompton.net.

Shouldn't a load balancer should either set the X-Forwarded-For header or not forward it at all?

Since that commit breaks compatibility with ELB could we make using the first or last IP configurable?

[weavejester]

Does ELB proxy the request through more than one server? One option is to allow a setting where the :remote-addr is set n before the last. So if we know ELB proxies through two servers, we can use the penultimate address instead.

We don't want to make it use the first IP, because the first IP can be set by the client to whatever they want, making it unreliable.

[g-k]

Does ELB proxy the request through more than one server?

Not in general, but requests that go client -> ELB -> API gateway/facade -> ELB -> microservice with every layer conforming to the AWS X-Forwarded-For format have multiple IPs.

I understand that clients can send packets with a spoofed origin IP, but I'm still not clear on why the last IP would be more or less reliable (maybe it depends on the LB?).

Regardless, an option to pull the nth-to-last IP would be great since the number of proxies should be constant. I can make a PR for that change (I figure it'd return nil when there aren't enough IPs in the header).

[weavejester]

Proxies always add IP addresses onto the end of the X-Forwarded-For header. So if you have one proxy then you know that the last address in X-Forwarded-For was set by a service under your control. If you have two proxies, then the last two addresses were set by services under your control, and so forth.

This means that taking the nth-from-last address, where n is the number of proxies, is reliable as it was set by a service under your control. Taking the first address is unreliable, because it can be set by a service out of your control.

I'd accept an additional argument on the middleware, n, defaulting to 0, and representing the number of positions from the last address. So 0 is the last address, 1 is the penultimate address, and so on.

[g-k]

OK for some reason I thought wrap-forwarded-remote-addr was supposed to set remote-addr to the client IP and not the last proxy.

https://github.com/ring-clojure/ring/wiki/Concepts#requests

[weavejester]

The middleware sets :remote-addr to the IP address that sent the request to the last proxy. If there is only one proxy, then the IP address is equivalent to the client address.

The wiki describes Ring's default behavior, not the behavior of the middleware.

[g-k]

No problem, thanks for clearing that up for me.

On Thu, Jul 14, 2016 at 1:29 PM, James Reeves notifications@github.com wrote:

It sets :remote-addr to the IP address that sent the request to the last proxy. If there is only one proxy, then the IP address is equivalent to the client address. The wiki is worded a little incorrectly, so I'll fix it.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ring-clojure/ring-headers/issues/7#issuecomment-232734602, or mute the thread https://github.com/notifications/unsubscribe/AAN1LSLtleA4m9R9Bj4ztkMGr-70My_oks5qVnHogaJpZM4JJff1 .

[seancorfield]

I just ran into this problem. We have F5 Big-IP and the way this middleware works, it always returns the IP of the load balancer, not the client IP (which is what we wanted). Now I understand what the intent is, I'll switch to using a different middleware.

[glenjamin]

I just ran into this too, an arg for "trust N proxies" would be handy 👍