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

Piwik_Common::getIP() - filter for public IP or from trusted proxy #567

Closed
robocoder opened this issue Feb 26, 2009 · 14 comments
Closed

Piwik_Common::getIP() - filter for public IP or from trusted proxy #567

robocoder opened this issue Feb 26, 2009 · 14 comments
Assignees
Milestone

Comments

@robocoder
Copy link
Contributor

@robocoder robocoder commented Feb 26, 2009

Currently, getIp() only returns a single client IP address, looking at HTTP_CLIENT_IP, HTTP_X_FORWARD_FOR (XFF), and REMOTE_ADDR (in that order).

It’s possible that getIp() returns a private IP address. We should make it configurable to return the first “public” IP address which can be geolocated, unless you want the current behavior (e.g., #1054 intranet subnet identification).

These are some private IP address ranges:
- 10.0.0.0 – 10.255.255.255
- 172.16.0.0 – 172.31.255.255
- 192.168.0.0 – 192.168.255.255

Another consideration is XFF spoofing (increasing popular with various browser addons). Perhaps we should log both the result from getIp() and REMOTE_ADDR?

(Above two scenarios may or may not involve a reverse proxy.)

Another consideration is #1553 … the IP address from PiwikTracker should override any logic here.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Feb 26, 2009

Also, it looks like there are a couple of unreachable codepaths in the current implementation of getIp(). [be reviewed](to)

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Aug 9, 2009

Rolling requirements into #43.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Mar 22, 2010

Re-opening as a separate ticket.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Mar 26, 2010

For intranets, this may be undesirable. So, I'm guessing we'd want to make this configureable. See #1054 use case.

@mattab
Copy link
Member

@mattab mattab commented Mar 26, 2010

Why is it not desirable for intranets? I'm afraid my network knowledge is limited.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Mar 26, 2010

intranets tend to use ip addresses in the private ip address ranges; excluding these would be bad unless configurable.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Mar 29, 2010

(In [2013]) refs #567 / comment🎫567:1 - clean up getIp()

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Oct 3, 2010

(In [3211]) fixes #567

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Oct 4, 2010

(In [3225]) refs #567

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Oct 4, 2010

(In [3226]) refs #567

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Oct 4, 2010

(In [3232]) refs #567

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Dec 19, 2010

This fix was undone by work in #1897, and needs to be revisited.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Dec 19, 2010

The fix is to use the last IP in the comma separated list.

@robocoder
Copy link
Contributor Author

@robocoder robocoder commented Dec 19, 2010

(In [3463]) fixes #567

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

Successfully merging a pull request may close this issue.

None yet
2 participants