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

ip rl: improved method of getting the IP address #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asoorm
Copy link
Member

@asoorm asoorm commented Jun 20, 2024

Initial implementation was problematic, because if you use any form of reverse proxy or load balancer, the RemoteAddr would appear as the IP address of the load balancer. We need to throw this away as it is not always useful for us.

This PR tweaks the code slightly to check X-Forwarded-For and X-Real-Ip headers.

We still have a shortcoming in that we don't check for private IP subnets. This means that outgoing requests from a client, going via a proxy or gateway, may have a private sub-net in the X-Forwarded-For - these should be filtered out.

Finally, we should probably walk backwards through the X-Forwarded-For header, not forwards.

Initial implementation was problematic, because if you use any form of
reverse proxy or load balancer, the RemoteAddr would appear as the IP
address of the load balancer.

We need to throw this away as it is not useful for us.

This PR checks X-Forwarded-For and X-Real-Ip headers.

We still have a shortcoming in that we don't check for private IP
subnets. This means that outgoing requests from a client, going via a
proxy or gateway, may have a private sub-net in the X-Forwarded-For -
these should be filtered out.

Finally, we should probably walk backwards through the X-Forwarded-For
header, not forwards.
@asoorm asoorm requested a review from evert44k June 20, 2024 07:12
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

Successfully merging this pull request may close these issues.

1 participant