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

Should use the rightmost ip in x-forwarded-for, if used at all #213

Closed
jarlandre opened this issue Jan 24, 2023 · 7 comments
Closed

Should use the rightmost ip in x-forwarded-for, if used at all #213

jarlandre opened this issue Jan 24, 2023 · 7 comments

Comments

@jarlandre
Copy link

jarlandre commented Jan 24, 2023

https://adam-p.ca/blog/2022/03/x-forwarded-for/

For something as important as rate limiting, the rightmost ip in the header, separated by comma, should be used. Not the leftmost, even if that is "closest" to the client. Because it can be spoofed.

Im going to fork this repo and make the get getIPFromXFFHeader function return the last ip in the list. So that i can at least use this library

@jarlandre jarlandre reopened this Jan 24, 2023
@jarlandre
Copy link
Author

Just tell if its interesting that i make a PR into this repo from this MaindeckAS@a8425b3

@novln
Copy link
Contributor

novln commented Jan 24, 2023

Hello,

As described here (https://github.com/ulule/limiter#limiter-behind-a-reverse-proxy) you should use what is best for your network topology. That's why it's disabled by default.

But word of caution, if you use the rightmost IP in the header you could rate limit your load balancer or CDN.

Just tell if its interesting that i make a PR into this repo [...]

No thank you, I plan to make a customizable system for the next major version.
I just need to find the motivation for this, I haven't been active recently.

@novln
Copy link
Contributor

novln commented Jan 24, 2023

But thank you for mentioning your fork, I may use it when (or if) I'll implement a rightmost ip parser.

@jarlandre
Copy link
Author

But word of caution, if you use the rightmost IP in the header you could rate limit your load balancer or CDN.

No, because aws alb appends client ip to XFF header. I dont know from where you have this idea, but its not valid in aws :) but no stress

@jarlandre
Copy link
Author

btw the most funny part is i might not need to use this fix at all. Because i made a middleware that fixes XFF header to rightmost. So i can use all kinds of libraries like this without caring too much

@novln
Copy link
Contributor

novln commented Jan 27, 2023

No, because aws alb appends client ip to XFF header. I dont know from where you have this idea, but its not valid in aws :)

As said before, that depend of your network topology: not everyone use AWS.
You could have an nginx with a public ip and then other nginx inside a VLAN, private network or an hypervisor that routes that to the correct VM / containers.

@jarlandre
Copy link
Author

Yep, you are right :) Ill close this now, but feel free to use as inspiration, or not. I am currently using another solution for this problem.

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

No branches or pull requests

2 participants