@timdream opened this Issue on February 7th 2021

Problem

Considering the current implementation: https://github.com/matomo-org/matomo/blob/a31fd86b64c2c7d2deadcfca71540fb4b0152c10/core/IP.php#L99-L125

With the following setup:

[client] -> [reverse proxy 1] -> [reverse proxy 2] -> [matomo Nginx]

Since each proxy appends the IP of the incoming connection according to RFC7239, X-Forwarded-For will be this by the time the connection reaches Nginx (1):

X-Forwarded-For: (client), (proxy 1)

Matomo currently extracts the first IP from the list, since #10404. This is correct until you realized the "client" can pretended to be a proxy as well, by including its own X-Forwarded-For header, like this:

curl -i -H"X-Forwarded-For: 8.8.8.8" "http://example.com/matomo/matomo.php?..."

When this happens, proxy 1 will be faithfully preserve the header passed in and append the client IP after it (2):

X-Forwarded-For: 8.8.8.8, (client), (proxy 1)

And Matomo will extract 8.8.8.8 as the client IP.

Solutions

Solution 1: Trust the "transparent proxies"

If Matomo decided the first "client" IP can always be trusted, we can still extract the first IP address, and this issue can be closed as "works as intended." But it will in risk of having the analytics data being polluted. Brutal force login detection can be tricked too.

Solution 2: Extract the last IP address

This would involve reverting #10404 and go back to extract the last IP address. Since the original proxy_ips[] configuration is still intact, when facing a header like (2), above, a correctly configured Matomo will be able to exclude the "proxy 1" IP address.

Notes

  • There can be a solution 1.1 where Mamoto will rely on proxy 1 (the first public-facing proxy) to remove the spoofed header. However this is not always possible (not possible for Apache AFAIK)
  • It is unclear to me what other client IP headers (HTTP_CF_CONNECTING_IP, HTTP_CLIENT_IP, etc) behaves and if Matomo have to do things differently between these and the X-Forwarded-For header.
  • #7060 is a staled discussion related to this, but the discussion talks about the header processing in the context before #10342.
  • #16379 have since promoted proxy_ips[] pretty well in the documentation. We may consider doubling down on the documentation effort when we pick either solution for this issue.
  • Lastly, one tiny nitpick: #10404 updated the name of the function IP::getFirstIpFromList but it didn't change the word "last" in the function comment right above it :)

Thanks!

@diosmosis commented on February 7th 2021 Member

Hi @timdream, thanks for creating this issue. Is this behavior in Matomo causing a problem for you specifically? Do you know of a client that does this and is causing a problem?

@timdream commented on February 8th 2021

@diosmosis It did not cause a problem for me! I am just trying to document what I've found that may be problematic in terms of spoofing and attack vector. I did not spot such attack/spoofing taking place on my own instance.

@tsteur commented on February 8th 2021 Member

Thanks @timdream would this be kind of a duplicate or related to https://github.com/matomo-org/matomo/issues/7060 ?

@timdream commented on February 9th 2021

@tsteur Yes the issue is related (see note), but I wouldn't say it is a duplicate because the research I made here contains new information on the current implementation.

@tsteur commented on February 10th 2021 Member

There can be a solution 1.1 where Mamoto will rely on proxy 1 (the first public-facing proxy) to remove the spoofed header. However this is not always possible (not possible for Apache AFAIK)

that was my first thought would be the preferred solution as AFAIK in https://github.com/matomo-org/matomo/pull/10404 it says

The RFC at https://tools.ietf.org/html/rfc7239 makes it clear that we should extract the first IP from the header HTTP_X_FORWARDED_FOR

So changing it to last IP could potentially break things again etc.

Another workaround might be to use other headers depending on what can be configured that can only be set by the public facing proxy? I don't know if that's the same problem for Apache.

Mostly seeing this so far as a documentation issue to mention it in the docs for setting up the proxy if it's not mentioned yet.

Powered by GitHub Issue Mirror