Issue with IP address

[aschempp]

Today I had an issue with Environment::ip(), it returned a comma separated list of IP addresses.

Apparently, HTTP_X_FORWARDED_FOR can be a comma separated list, but Environment::ip() should always return only one IP...

https://www.google.ch/search?client=safari&rls=en&q=http_x_forwarded_for+multiple+addresses&ie=UTF-8&oe=UTF-8&redir_esc=&ei=xC2fUb3BGYeXPe7wgSA

[BugBuster1701]

I use the first IP in my modules
http://en.wikipedia.org/wiki/X-Forwarded-For#Format

[aschempp]

I've notice the fiest IP was also mine in the list. However multiple sources say you should use the last one...

[backbone87]

From what you've told, the last one should be the one of the last proxy before the requesting proxy.

[PC] -> [Proxy 1] -> [Proxy 2] -> [Contao]

So first of HTTP_X_FORWARDED_FOR would be IP of PC and second (last) would be IP of Proxy 1. REMOTE_ADDR is Proxy 2.

(Thats just my logic out of what you told, to be confirmed)

Edit: A @BugBuster1701 's wiki link confirms it.

@aschempp
What sources recommend using the last one? IMHO it depends on the use-case. If you're building a transparent HTTP application (like Proxy's) using the last one, would probably make sense, but in Contao context using the origin creator of the HTTP request is fine?

[aschempp]

That is correct. However, the sources say that you can only trust the last IP (the one your proxy server added), all other could be faked...

[leofeyer]

The actual question is why you want ip() to return only one address. A comma separated list seems just fine to me.

[aschempp]

Method description:

/**
 * Return the real REMOTE_ADDR even if a proxy server is used
 *
 * @return string The IP address of the client
 */

Do I need to say something else? ^^

[leofeyer]

The comment just says that the function will try to return the real remote address ([PC] in the example above) instead of just the IP of the last proxy ([Proxy 2] in the example above), which is stored in the $_SERVER['REMOTE_ADDR'] variable. It does however not say that it will only return one single IP address. It might as well return [PC], [Proxy 1], [Proxy 2], which will make debugging a lot easier than with the proxy addresses being stripped.

[aschempp]

1. REMOTE_ADDR is only one address
2. "The IP address" is definitely a singular statement
3. Methods like System::anonymizeIp assume a single IPv4 address (or the anonymization would not work)
4. Sessions are bound to the remote IP by default. If there are multiple proxy servers, this would cause the login session to fail.

[leofeyer]

Fixed in 414a81e then.

[aschempp]

thanks!

[aschempp]

I just came against this again. The issue with the first IP is, that clients can fake this in their browsers. This makes the IP address unusable for any verifications. I think we should discuss again if to use the first or last IP...

[backbone87]

You can not trust any HTTP_X_* header.
The only relieable address is the REMOTE_ADDR, which comes from the TCP/IP stack of the servers network interface.

[leofeyer]

Exactly what I said.

[leofeyer]

We have come up with a solution in the @contao/workgroup-core call:

1. Define a list of trusted proxy IPs in the local configuration.
2. Add the remote address to the list of X-Forwarded-For IPs.
3. Strip all trusted IPs from the end of the IP list and return the next last IP from the remaining list.

[aschempp]

if ($_SERVER['HTTP_X_FORWARDED_FOR'] != '') {
    $_SERVER['HTTP_X_FORWARDED_FOR'] = array_map('trim', explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']));
    $_SERVER['HTTP_X_FORWARDED_FOR'] = array_diff($_SERVER['HTTP_X_FORWARDED_FOR'], array('1.2.3.4', '2.3.4.5', '.3.4.5.6', '4.5.6.7'));
    $_SERVER['HTTP_X_FORWARDED_FOR'] = array_pop($_SERVER['HTTP_X_FORWARDED_FOR']);
}

[Toflar]

As a hint: https://github.com/symfony/HttpFoundation/blob/master/Request.php#L752
No need to reinvent the wheel :)

[backbone87]

This is good, but the docblock of the getClientIps is wrong. In their Impl the most trusted one is the left most because of the array_reverse at the end (this is done for easy access in the getClientIp method)

[leofeyer]

Implemented in c84bc16.