Bug 527586: Use X-Forwarded-For instead of REMOTE_ADDR for trusted proxies

Bugzilla/Auth/Login/Cookie.pm

@@ -35,7 +35,7 @@ sub get_login_info {
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
 
-    my $ip_addr      = $cgi->remote_addr();
+    my $ip_addr      = remote_ip();
     my $login_cookie = $cgi->cookie("Bugzilla_logincookie");
     my $user_id      = $cgi->cookie("Bugzilla_login");
 

Bugzilla/Auth/Persist/Cookie.pm

@@ -52,7 +52,7 @@ sub persist_login {
 
     my $ip_addr;
     if ($input_params->{'Bugzilla_restrictlogin'}) {
-        $ip_addr = $cgi->remote_addr;
+        $ip_addr = remote_ip();
         # The IP address is valid, at least for comparing with itself in a
         # subsequent login
         trick_taint($ip_addr);

Bugzilla/Config/Advanced.pm

@@ -42,6 +42,12 @@ use constant get_param_list => (
   },
 
   {
+   name => 'inbound_proxies',
+   type => 't',
+   default => ''
+  },
+
+  {
    name => 'proxy_url',
    type => 't',
    default => ''

Bugzilla/Error.pm

@@ -64,7 +64,7 @@ sub _throw_error {
         for (1..75) { $mesg .= "-"; };
         $mesg .= "\n[$$] " . time2str("%D %H:%M:%S ", time());
         $mesg .= "$name $error ";
-        $mesg .= "$ENV{REMOTE_ADDR} " if $ENV{REMOTE_ADDR};
+        $mesg .= remote_ip();
         $mesg .= Bugzilla->user->login;
         $mesg .= (' actually ' . Bugzilla->sudoer->login) if Bugzilla->sudoer;
         $mesg .= "\n";

Bugzilla/Token.pm

@@ -142,7 +142,7 @@ sub IssuePasswordToken {
 
     ThrowUserError('too_soon_for_new_token', {'type' => 'password'}) if $too_soon;
 
-    my ($token, $token_ts) = _create_token($user->id, 'password', $::ENV{'REMOTE_ADDR'});
+    my ($token, $token_ts) = _create_token($user->id, 'password', remote_ip());
 
     # Mail the user the token along with instructions for using it.
     my $template = Bugzilla->template_inner($user->settings->{'lang'}->{'value'});
@@ -283,7 +283,7 @@ sub Cancel {
     my $user = new Bugzilla::User($userid);
 
     $vars->{'emailaddress'} = $userid ? $user->email : $eventdata;
-    $vars->{'remoteaddress'} = $::ENV{'REMOTE_ADDR'};
+    $vars->{'remoteaddress'} = remote_ip();
     $vars->{'token'} = $token;
     $vars->{'tokentype'} = $tokentype;
     $vars->{'issuedate'} = $issuedate;

Bugzilla/User.pm

@@ -65,11 +65,6 @@ use base qw(Bugzilla::Object Exporter);
 # Constants
 #####################################################################
 
-# Used as the IP for authentication failures for password-lockout purposes
-# when there is no IP (for example, if we're doing authentication from the
-# command line for some reason).
-use constant NO_IP => '255.255.255.255';
-
 use constant USER_MATCH_MULTIPLE => -1;
 use constant USER_MATCH_FAILED   => 0;
 use constant USER_MATCH_SUCCESS  => 1;
@@ -1681,7 +1676,7 @@ sub account_is_locked_out {
 
 sub note_login_failure {
     my $self = shift;
-    my $ip_addr = Bugzilla->cgi->remote_addr || NO_IP;
+    my $ip_addr = remote_ip();
     trick_taint($ip_addr);
     Bugzilla->dbh->do("INSERT INTO login_failure (user_id, ip_addr, login_time)
                        VALUES (?, ?, LOCALTIMESTAMP(0))",
@@ -1691,7 +1686,7 @@ sub note_login_failure {
 
 sub clear_login_failures {
     my $self = shift;
-    my $ip_addr = Bugzilla->cgi->remote_addr || NO_IP;
+    my $ip_addr = remote_ip();
     trick_taint($ip_addr);
     Bugzilla->dbh->do(
         'DELETE FROM login_failure WHERE user_id = ? AND ip_addr = ?',
@@ -1703,7 +1698,7 @@ sub account_ip_login_failures {
     my $self = shift;
     my $dbh = Bugzilla->dbh;
     my $time = $dbh->sql_interval(LOGIN_LOCKOUT_INTERVAL, 'MINUTE');
-    my $ip_addr = Bugzilla->cgi->remote_addr || NO_IP;
+    my $ip_addr = remote_ip();
     trick_taint($ip_addr);
     $self->{account_ip_login_failures} ||= Bugzilla->dbh->selectall_arrayref(
         "SELECT login_time, ip_addr, user_id FROM login_failure

Bugzilla/Util.pm

@@ -35,7 +35,7 @@ use base qw(Exporter);
                              detaint_signed
                              html_quote url_quote xml_quote
                              css_class_quote html_light_quote url_decode
-                             i_am_cgi correct_urlbase
+                             i_am_cgi correct_urlbase remote_ip
                              lsearch do_ssl_redirect_if_required use_attachbase
                              diff_arrays
                              trim wrap_hard wrap_comment find_wrap_point
@@ -54,6 +54,7 @@ use DateTime;
 use DateTime::TimeZone;
 use Digest;
 use Email::Address;
+use List::Util qw(first);
 use Scalar::Util qw(tainted);
 use Template::Filters;
 use Text::Wrap;
@@ -289,6 +290,15 @@ sub correct_urlbase {
     }
 }
 
+sub remote_ip {
+    my $ip = $ENV{'REMOTE_ADDR'} || '127.0.0.1';
+    my @proxies = split(/[\s,]+/, Bugzilla->params->{'inbound_proxies'});
+    if (first { $_ eq $ip } @proxies) {
+        $ip = $ENV{'HTTP_X_FORWARDED_FOR'} if $ENV{'HTTP_X_FORWARDED_FOR'};
+    }
+    return $ip;
+}
+
 sub use_attachbase {
     my $attachbase = Bugzilla->params->{'attachment_base'};
     return ($attachbase ne ''

template/en/default/admin/params/advanced.html.tmpl

@@ -32,6 +32,14 @@
     _ " one hostname pointing at the same web server, and you"
     _ " want them to share the $terms.Bugzilla cookie.",
 
+  inbound_proxies =>
+    "When inbound traffic to $terms.Bugzilla goes through a proxy,"
+    _ " $terms.Bugzilla thinks that the IP address of every single"
+    _ " user is the IP address of the proxy. If you enter a comma-separated"
+    _ " list of IPs in this parameter, then $terms.Bugzilla will trust any"
+    _ " <code>X-Forwarded-For</code> header sent from those IPs,"
+    _ " and use the value of that header as the end user's IP address.",
+
   proxy_url => 
     "$terms.Bugzilla may have to access the web to get notifications about"
     _ " new releases (see the <tt>upgrade_notification</tt> parameter)."