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

diff --git a/Bugzilla/Auth/Login/Cookie.pm b/Bugzilla/Auth/Login/Cookie.pm
index 0b002168ece5fd2cc3a5831f1c9597aaaeeb4580..570988f7e2e140aca6b22174ea0308df32db6be6 100644 (file)
--- 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 1e1b3a87178253feb0faa5fb7fba2f7fc3b04d6b..232212075878870da8b01c1cfe7e9f8a83916de8 100644 (file)
--- 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 51943f03f7ca42303c8d172c1e632028688b6395..1acf76f38bfa487959cefeb023706e70c3fe13a2 100644 (file)
--- a/Bugzilla/Config/Advanced.pm
+++ b/Bugzilla/Config/Advanced.pm
@@ -41,6 +41,12 @@ use constant get_param_list => (
    default => ''
   },
 
+  {
+   name => 'inbound_proxies',
+   type => 't',
+   default => ''
+  },
+
   {
    name => 'proxy_url',
    type => 't',

diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index dbd9688a9938c512865664047dc28741897e3718..4e312b3edf0e6c9417bc2a5b814abeffe08b82fc 100644 (file)
--- 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 4aee0561ec497cbf8db73cd8e5078de05f4d105b..2cd9e3f9c4cef39f3be556e1d8b115bff292ab07 100644 (file)
--- 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 e8ea2878ef6af20dcebd0e5cccc5f70fa9ae64d2..75a4fcf1de620974023625fea848ba81b2dc4455 100644 (file)
--- 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 00f9b0a05bab55a70db2e789a922e6621b04ba57..ca2506ffa3c628cc6a72d4305c50678bc104b426 100644 (file)
--- 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 ''

diff --git a/template/en/default/admin/params/advanced.html.tmpl b/template/en/default/admin/params/advanced.html.tmpl
index d3fe449f829598c6aa423d7ed9c297a18fcddac7..4caa2f1dc56004e75de052ebd05ea5b116be2773 100644 (file)
--- a/template/en/default/admin/params/advanced.html.tmpl
+++ b/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)."