-
Notifications
You must be signed in to change notification settings - Fork 32
Fix bad geoip reporting #1022
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
base: master
Are you sure you want to change the base?
Fix bad geoip reporting #1022
Conversation
|
Is there an issue here where the client can inject an X-Forwarded-For header with a bogus IP? Any security-related use of X-Forwarded-For (such as for rate limiting or IP-based access control) must only use IP addresses added by a trusted proxy. If there is a (single) proxy as part of the ooni infrastructure, maybe it makes sense to drop the last proxy IP from the list. |
Hi @aagbsn, this code is fixing a metric that we use to check the relationship between the reported CC and ASN from the ones we can observe from the server using the reported client IP, the idea is precisely to see how trustworthy the reported data is:
However this data is not used for anything critical or even exposed to the public, we use it internally to study if there's an unusually high volume of inconsistencies and why. In case of a bogus IP, it would pollute the mismatch metric, but nothing more Do you see any case where this could still be an issue? |
This PR will fix the bad geoip reporting that was caused by how we parse the
X-Forwarded-Forheader:According to MDN:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-For#directives
And AWS:
https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/x-forwarded-headers.html#x-forwarded-for
But we're taking the right most header instead, the most recent proxy server (backend-fsn)
backend/ooniapi/services/ooniprobe/src/ooniprobe/utils.py
Line 127 in 30f3860
And looking at the logs I found the following value for
X-Forwarded-For:'xxx.xxx.xxx.xxx, 162.55.247.208'Where
162.55.247.208is backend-fsnSo the solution should be just taking the first value instead of the last one
closes #1003