Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
robocoder opened this Issue · 14 comments

2 participants

@robocoder
Collaborator

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
Collaborator

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

@robocoder
Collaborator

Rolling requirements into #43.

@robocoder
Collaborator

Re-opening as a separate ticket.

@robocoder
Collaborator

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

@mattab
Owner

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

@robocoder
Collaborator

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

@robocoder
Collaborator

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

@robocoder
Collaborator

(In [3211]) fixes #567

@robocoder
Collaborator

(In [3225]) refs #567

@robocoder
Collaborator

(In [3226]) refs #567

@robocoder
Collaborator

(In [3232]) refs #567

@robocoder
Collaborator

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

@robocoder
Collaborator

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

@robocoder
Collaborator

(In [3463]) fixes #567

@robocoder robocoder added this to the Piwik 1.1 milestone
@robocoder robocoder self-assigned this
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.