Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AWS ELB sets the real IP address at the end of ctx.request.ips #1094

Closed
nitrocode opened this issue Nov 12, 2017 · 5 comments
Closed

AWS ELB sets the real IP address at the end of ctx.request.ips #1094

nitrocode opened this issue Nov 12, 2017 · 5 comments
Labels

Comments

@nitrocode
Copy link

nitrocode commented Nov 12, 2017

We were relying on ctx.request.ip to get the real IP which is really just the IP returned in the X-Forwarded-For header. Noticed that if we spoofed the header, AWS's ELB would forward it along with the real IP. Multiple spoofed headers were ignored. The most recent spoofed IP and the real IP was always the last IP in an array with 2 values.

$ curl -H 'X-Forwarded-For: 1.1.1.1' -H 'X-Forwarded-For: 2.2.2.2' elburl
X-Forwarded-For: 1.1.1.1,68.32.48.12

To correct this behavior we now use the last value in ctx.request.ips if it exists, if not, it will use ctx.request.ip. This seems to have prevented spoofing IPs with curl.

Reference: Elastic Load Balancer X-FORWARDED-FOR

@nitrocode
Copy link
Author

nitrocode commented Nov 14, 2017

According to the docs ctx.request.ip is ordered upstream to downstream and I interpret that as upstream is what the IP was where it originated and downstream is what the IP became before it reached the server (left to right).

request.ip = request.ips[0] || req.socket.remoteAddress || '';

I think the docs should be updated to reflect how the code works. Thoughts?

@nitrocode nitrocode changed the title Note: AWS ELB sets the real IP address at the end of ctx.request.ips AWS ELB sets the real IP address at the end of ctx.request.ips Nov 14, 2017
@garcia556
Copy link

Taking into account overall laconic style of koa documentation request.ip description looks fine IMO. It says Request remote address. Supports X-Forwarded-For when app.proxy is true..

Remote address makes clear that koa will try to get actual client IP address by all means: from HTTP headers or directly from the socket connected. If you try to cover more details like ips order you'll have to explain everything. And that brings you too close to the implementation. In such case it's easier to check the source code itself.

As for the terminology upstream/downstream actually might be confusing, here's perfect explanation. In short when you are looking at koa request upstream is where the data comes from thus actual client. That might contradict with upstream setting in something like nginx unless you change the perspective from application server request handler to the reverse proxy. For it downstream is client and next proxy in chain is upstream.

@nitrocode
Copy link
Author

nitrocode commented Nov 20, 2017

Thanks for the comment @garcia556 .

Our code has app.proxy set to true and request.ip returns the first item in the array vs the last item in the array. In my case, the last item in the array is the real IP address. request.ip shouldn't return the spoofed IP when we have the real IP. I shouldn't have to dig into request.ips each time.

If the docs are updated to show that the client's IP is the last IP in the array and / or request.ip was modified to return the last IP in the array, then I think the code and docs would be a lot more intuitive. Otherwise you have a lot of people assuming request.ip is the real IP address and they have no idea that anyone can spoof that.

The docs can also be updated to include your helpful link on upstream vs downstream depending on context.

@garcia556
Copy link

garcia556 commented Nov 20, 2017

Well this is interesting. First of all [0] was done intentionally as per the source code comments:

koa/lib/request.js

Lines 313 to 323 in c699c75

/**
* When `app.proxy` is `true`, parse
* the "X-Forwarded-For" ip address list.
*
* For example if the value were "client, proxy1, proxy2"
* you would receive the array `["client", "proxy1", "proxy2"]`
* where "proxy2" is the furthest down-stream.
*
* @return {Array}
* @api public
*/

The problem is that there is no established standard for X-Forwarded-For header.

  • Mozilla doc describes list as X-Forwarded-For: <client>, <proxy1>, <proxy2> which corresponds to koa implementation
  • nginx also appends IP addresses to the end leaving client IP as first element in the list (can be checked here).
  • squid where XFF was introduced does the same
  • there is a proposed RFC for Forwarded header as XFF replacement and it also presumes appending to the end of the list
  • another RFC regarding proxies also says the value should be appended

Thus ELB is a red herring here, as per the doc it indeed adds client IP address to the end of the list in XFF: X-Forwarded-For: ip-address-1, ip-address-2, client-ip-address.

It looks like a conflict of de-facto XFF standard vs. ELB particular implementation. It would also be hard to find the right place for this in koa docs. It seems to be a little bit off-topic IMO, all that XFF related stuff. Especially without XFF being recognized as actual standard and its vulnerability to spoofing.

@nitrocode
Copy link
Author

nitrocode commented Nov 22, 2017

@garcia556 that is informative! Thank you so much for that.

Since the ELB is indeed a red herring, other than submitting an issue with them or creating a npm module for every load balancer's XFF implementation, it might be safer to just document the code we use now to get the real IP knowing that we use ELB:

  // To avoid X-Forwarded-For spoofing
  // https://forums.aws.amazon.com/message.jspa?messageID=160282
  // If available this retrieves the last item in the array of IPs, if not, it returns the normal koa ip
  ctx.state.ipaddr = ctx.request.ips.slice(-1)[0] || ctx.request.ip;

I'm now curious how other competitor load balancers like Azure have implemented this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants