ip_address() in bootstrap.inc returns invalid ip address when using 2 proxies [#705718]

ip_address uses the last added ip_address in $_SERVER['HTTP_X_FORWARDED_FOR'], while this is not the correct remote ip address in double proxy configurations and isn't using trim as the drupal 7 version.

if (!empty($reverse_proxy_addresses) && in_array($ip_address, $reverse_proxy_addresses, TRUE)) {
        // If there are several arguments, we need to check the most
        // recently added one, i.e. the last one.
        $ip_address = array_pop(explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']));
      }

Possible solution

 if (!empty($reverse_proxy_addresses) && in_array($ip_address, $reverse_proxy_addresses, TRUE)) {
        // If there are several arguments, we need to check the most
        // recently added one, i.e. the last one.
        $ip_list = array_map('trim', explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']));
        $ip_address = array_pop($ip_list);
while(in_array($ip_address, $reverse_proxy_addresses)){
  $ip_address = trim(array_pop($ip_list));
  }
      }

Comment	File	Size	Author
#2	drupal-705718.patch	1.24 KB	mikeytown2
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

robpeek CreditAttribution: robpeek commented 5 February 2010 at 15:30

The trim method inside the while loop is not needed.

Comment #2

mikeytown2 CreditAttribution: mikeytown2 commented 7 May 2010 at 23:38

Version:

6.0

» 6.x-dev

File	Size
drupal-705718.patch	1.24 KB

Currently only returns the last one, need to search for last IP that is not in reverse_proxy_addresses.

Comment #3

teastburn85 CreditAttribution: teastburn85 commented 7 June 2010 at 23:57

Why does ip_address() use the rightmost entry from X-Forwarded-For header? Isn't the purpose of ip_address() to get the end-user/client's IP address? The documentation for X-Forwarded-For on wikipedia seems to state that the client's IP is the first mentioned IP (leftmost) and any other servers that touch the request append their IPs to the end of the list (rightmost). Am I misunderstanding how X-Forwarded-For is used in this situation?

Selecting the rightmost IP is breaking our Drupal setup where our servers are behind a CDN that passes the client's IP as the first IP in the X-Forwarded-For header.

Also posted this question on http://drupal.org/node/309586 and http://drupal.org/node/258397

Thanks in advance

Comment #4

mikeytown2 CreditAttribution: mikeytown2 commented 8 June 2010 at 01:48

@teastburn85
Drupal uses the right most one that doesn't match to avoid IP address spoofing. Firefox plugin to exploit the first IP given
https://addons.mozilla.org/en-US/firefox/addon/5948/

Comment #5

teastburn85 CreditAttribution: teastburn85 commented 8 June 2010 at 17:03

@mikeytown2
Thanks for the quick reply! I understand the spoofing aspect and realize it's rather easy when headers are just plain text easily sent by the requester. However, if the purpose of the ip_address() function is to get the client's IP address (please tell me if that's not really the purpose), then people will use it to try to track users (see the Poll module). But if the function returns the rightmost address, then it's just returning the proxy server's address, which would be the same for every user (or would vary by the number of servers in your rotation), totally nullifying the purpose of the function. Maybe I'm missing something here...
Thanks

Comment #6

mikeytown2 CreditAttribution: mikeytown2 commented 8 June 2010 at 17:15

IP addresses in drupal can be used to prevent access to drupal. Using the firefox addon, one can bypass the block you may have setup.

in your settings.php

$conf['reverse_proxy'] = TRUE;
$cpnf['reverse_proxy_addresses'] = array(
  '127.0.0.1',
  '192.168.1.1',
);

Using the modified ip_address() function from #2 it will discard 127.0.0.1 & 192.168.1.1 from the HTTP_X_FORWARDED_FOR string and get the next right most ip address.

Your free to do what you want, but unless you follow these guidelines odds are bad things will happen to your web server.

Comment #7

teastburn85 CreditAttribution: teastburn85 commented 9 June 2010 at 18:08

@mikeytown2
I appreciate your responses on this, and maybe I should have given a little more background. Our servers are behind a CDN that uses the X-Forwarded-For property to pass us the client's IP address and the IP of any of their servers that touched the request. So for tracking users (e.g. voting in the Poll module, blocking an IP, etc), the ip_address() function simply returns our CDN's IPs, which is not very helpful (for Polls, it only allows # of votes proportionate to CDN nodes; for banning, we do not want to ban the very servers that serve all of our content!).

The $conf['reverse_proxy_addresses'] variable would indeed fix this, if not for a CDN having hundreds of servers across the world that we neither want to know nor should know the IPs for. Anyways, I added a $conf variable to allow bypassing the reverse_proxy_addresses and take the leftmost IP from the list. Maybe it would be nice to make a hook for this for people in my situation? Or maybe not... :)

Thanks again