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
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Milestone

Comments

@robocoder
Copy link
Contributor

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

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

Rolling requirements into #43.

@robocoder
Copy link
Contributor Author

Re-opening as a separate ticket.

@robocoder
Copy link
Contributor Author

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 commented Mar 26, 2010

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

@robocoder
Copy link
Contributor Author

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

@robocoder
Copy link
Contributor Author

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

@robocoder
Copy link
Contributor Author

(In [3211]) fixes #567

@robocoder
Copy link
Contributor Author

(In [3225]) refs #567

@robocoder
Copy link
Contributor Author

(In [3226]) refs #567

@robocoder
Copy link
Contributor Author

(In [3232]) refs #567

@robocoder
Copy link
Contributor Author

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

@robocoder
Copy link
Contributor Author

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

@robocoder
Copy link
Contributor Author

(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
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

2 participants