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
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).
https://github.com/koajs/koa/blob/53a44461230199fd175a9f1c77ba66c7943bfaf5/lib/application.js#L174
I think the docs should be updated to reflect how the code works. Thoughts?
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.
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.
Well this is interesting. First of all [0] was done intentionally as per the source code comments:
https://github.com/koajs/koa/blob/c699c75c52bc96ba6c3dfc0202a55b76832d7192/lib/request.js#L313-L323
The problem is that there is no established standard for X-Forwarded-For header.
X-Forwarded-For: <client>, <proxy1>, <proxy2> which corresponds to koa implementationnginx 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 sameForwarded header as XFF replacement and it also presumes appending to the end of the listThus 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.
@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.
Most helpful comment
Well this is interesting. First of all
[0]was done intentionally as per the source code comments:https://github.com/koajs/koa/blob/c699c75c52bc96ba6c3dfc0202a55b76832d7192/lib/request.js#L313-L323
The problem is that there is no established standard for
X-Forwarded-Forheader.X-Forwarded-For: <client>, <proxy1>, <proxy2>which corresponds tokoaimplementationnginxalso appends IP addresses to the end leaving client IP as first element in the list (can be checked here).squidwhere XFF was introduced does the sameForwardedheader as XFF replacement and it also presumes appending to the end of the listThus 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
koadocs. 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.