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

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=dkl, a=mkanat

7 files changed, 25 insertions, 14 deletions

diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm
index 0b002168e..570988f7e 100644
--- a/Bugzilla/Auth/Login/Cookie.pm
+++ b/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");
 
diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm
index 1e1b3a871..232212075 100644
--- a/Bugzilla/Auth/Persist/Cookie.pm
+++ b/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);
diff --git a/Bugzilla/Config/Advanced.pm b/Bugzilla/Config/Advanced.pm
index 51943f03f..1acf76f38 100644
--- a/Bugzilla/Config/Advanced.pm
+++ b/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 => ''
diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index dbd9688a9..4e312b3ed 100644
--- a/Bugzilla/Error.pm
+++ b/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";
diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index 4aee0561e..2cd9e3f9c 100644
--- a/Bugzilla/Token.pm
+++ b/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;
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index e8ea2878e..75a4fcf1d 100644
--- a/Bugzilla/User.pm
+++ b/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
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 00f9b0a05..ca2506ffa 100644
--- a/Bugzilla/Util.pm
+++ b/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 ''