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

clientIP: add trustedproxy, return first untrusted IP instead of the last one #2892

Merged
merged 3 commits into from Oct 21, 2019

Conversation

phy25
Copy link
Collaborator

@phy25 phy25 commented Oct 21, 2019

This fixes #2828, where malicious clients passed in customized HTTP header to keep its IP address off records.

This is inspired by Sympony's Request::setTrustedProxies, but I don't want to implement everything including IP CIDR matching (IPv4 + IPv6), so I decided to reuse the local IP checker in place powered by regexp. Now admins can customize this "local" (trusted) proxy list using $conf['trustedproxy'], and by default it will allow any local IPs.

If in the future there is a need to implement array-based CIDR matching, $conf['trustedproxies'] can be used for the new config name.

We cannot trust any IP not included in $conf['trustedproxy'].
…last one

This fixes #2828, where malicious clients passed in customized HTTP header to keep its IP address off records.

This is inspired by Sympony's Request::setTrustedProxies, but I don't want to implement everything including IP CIDR matching (IPv4 + IPv6), so I decided to reuse the local IP checker in place powered by regexp. Now admins can customize this "local" (trusted) proxy list using $conf['trustedproxy'], and by default it will allow any local IPs.

If in the future there is a need to implement array-based CIDR matching, $conf['trustedproxies'] can be used for the new config name.
@splitbrain splitbrain merged commit 57cf3ea into master Oct 21, 2019
@splitbrain splitbrain deleted the trusted-proxies branch October 21, 2019 18:56
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.

A way to bypass the ban
2 participants