Opened 5 years ago

Closed 3 years ago

#567 closed New feature (fixed)

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

Reported by: vipsoft Owned by: vipsoft
Priority: normal Milestone: Piwik 1.1
Component: Core Keywords:
Cc: Sensitive: no

Description (last modified by vipsoft)

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.

Change History (22)

comment:1 Changed 5 years ago by vipsoft (robocoder)

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

comment:2 Changed 5 years ago by vipsoft (robocoder)

  • Resolution set to duplicate
  • Sensitive unset
  • Status changed from new to closed

Rolling requirements into #43.

comment:3 Changed 5 years ago by koteiko

comment:4 Changed 4 years ago by vipsoft (robocoder)

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Re-opening as a separate ticket.

comment:5 Changed 4 years ago by vipsoft (robocoder)

  • Owner set to vipsoft
  • Status changed from reopened to new

comment:6 Changed 4 years ago by vipsoft (robocoder)

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

comment:7 Changed 4 years ago by matt (mattab)

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

comment:8 Changed 4 years ago by vipsoft (robocoder)

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

comment:9 Changed 4 years ago by vipsoft (robocoder)

  • Description modified (diff)
  • Keywords outofscope added
  • Resolution set to wontfix
  • Status changed from new to closed
  • Summary changed from For proxies, Piwik_Common::getIp() should get the first public IP address to Piwik_Common::getIP() - enhancements
  • Type changed from Bug to New feature

comment:10 Changed 4 years ago by vipsoft (robocoder)

  • Description modified (diff)

comment:11 Changed 4 years ago by vipsoft (robocoder)

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

comment:12 Changed 4 years ago by vipsoft (robocoder)

  • Keywords outofscope removed
  • Milestone changed from Features requests 1.x or 2.x to 1.1 - Piwik 1.1
  • Resolution wontfix deleted
  • Status changed from closed to reopened

comment:13 Changed 4 years ago by vipsoft (robocoder)

  • Description modified (diff)

comment:14 Changed 4 years ago by vipsoft (robocoder)

  • Summary changed from Piwik_Common::getIP() - enhancements to Piwik_Common::getIP() - filter for public IP or from trusted proxy

comment:15 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [3211]) fixes #567

comment:19 Changed 3 years ago by vipsoft (robocoder)

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:20 Changed 3 years ago by vipsoft (robocoder)

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

comment:21 Changed 3 years ago by vipsoft (robocoder)

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

comment:22 Changed 3 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [3463]) fixes #567

Note: See TracTickets for help on using tickets.