Bug 1284277 - allow inbound_proxy to be set to '*'

r=dkl

diff --git a/Bugzilla/Config/Advanced.pm b/Bugzilla/Config/Advanced.pm
index 75afe7b22d844225134c255cf95a55b4b0bc93c2..b3968a25464876e8163e21dca3026c2972b0bf0d 100644 (file)
--- a/Bugzilla/Config/Advanced.pm
+++ b/Bugzilla/Config/Advanced.pm
@@ -26,7 +26,7 @@ use constant get_param_list => (
    name => 'inbound_proxies',
    type => 't',
    default => '',
-   checker => \&check_ip
+   checker => \&check_inbound_proxies
   },
 
   {
@@ -44,4 +44,15 @@ use constant get_param_list => (
   },
 );
 
+sub check_inbound_proxies {
+    my $inbound_proxies = shift;
+
+    return "" if $inbound_proxies eq "*";
+    my @proxies = split(/[\s,]+/, $inbound_proxies);
+    foreach my $proxy (@proxies) {
+        validate_ip($proxy) || return "$proxy is not a valid IPv4 or IPv6 address";
+    }
+    return "";
+}
+
 1;

diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index e673a920eea059bd2e84a4cf23b35c11a15afd7c..dc41652f7a141e6f6fe7cfc7b573d2fb04e00dff 100644 (file)
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -34,7 +34,7 @@ use Date::Parse;
 use Date::Format;
 use Digest;
 use Email::Address;
-use List::Util qw(first);
+use List::MoreUtils qw(none);
 use Scalar::Util qw(tainted blessed);
 use Text::Wrap;
 use Encode qw(encode decode resolve_alias);
@@ -284,28 +284,23 @@ sub correct_urlbase {
     }
 }
 
+# Returns the real remote address of the client,
 sub remote_ip {
-    my $ip = $ENV{'REMOTE_ADDR'} || '127.0.0.1';
-    my @proxies = split(/[\s,]+/, Bugzilla->params->{'inbound_proxies'});
-
-    # If the IP address is one of our trusted proxies, then we look at
-    # the X-Forwarded-For header to determine the real remote IP address.
-    if ($ENV{'HTTP_X_FORWARDED_FOR'} && first { $_ eq $ip } @proxies) {
-        my @ips = split(/[\s,]+/, $ENV{'HTTP_X_FORWARDED_FOR'});
-        # This header can contain several IP addresses. We want the
-        # IP address of the machine which connected to our proxies as
-        # all other IP addresses may be fake or internal ones.
-        # Note that this may block a whole external proxy, but we have
-        # no way to determine if this proxy is malicious or trustable.
-        foreach my $remote_ip (reverse @ips) {
-            if (!first { $_ eq $remote_ip } @proxies) {
-                # Keep the original IP address if the remote IP is invalid.
-                $ip = validate_ip($remote_ip) || $ip;
-                last;
-            }
+    my $remote_ip       = $ENV{'REMOTE_ADDR'} || '127.0.0.1';
+    my @proxies         = split(/[\s,]+/, Bugzilla->params->{inbound_proxies});
+    my @x_forwarded_for = split(/[\s,]+/, $ENV{HTTP_X_FORWARDED_FOR} // '');
+
+    return $remote_ip unless @x_forwarded_for;
+    return $x_forwarded_for[0] if $proxies[0] eq '*';
+    return $remote_ip if none { $_ eq $remote_ip } @proxies;
+
+    foreach my $ip (reverse @x_forwarded_for) {
+        if (none { $_ eq $ip } @proxies) {
+            # Keep the original IP address if the remote IP is invalid.
+            return validate_ip($ip) || $remote_ip;
         }
     }
-    return $ip;
+    return $remote_ip;
 }
 
 sub validate_ip {

diff --git a/t/013remote_ip.t b/t/013remote_ip.t
new file mode 100644 (file)
index 0000000..1cc832a
--- /dev/null
+++ b/t/013remote_ip.t
@@ -0,0 +1,81 @@
+#!/usr/bin/perl
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+use strict;
+use lib qw(. lib t);
+use Test::More qw(no_plan);
+use Bugzilla;
+use Bugzilla::Util qw(remote_ip);
+
+my $params = Bugzilla->params;
+
+{
+    local $params->{inbound_proxies} = '10.0.0.1,10.0.0.2';
+    local $ENV{REMOTE_ADDR} = '10.0.0.2';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42';
+
+    is(remote_ip(), '10.42.42.42', "from proxy 2");
+}
+
+{
+    local $params->{inbound_proxies} = '10.0.0.1,10.0.0.2';
+    local $ENV{REMOTE_ADDR} = '10.0.0.1';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42';
+
+    is(remote_ip(), '10.42.42.42', "from proxy 1");
+}
+
+{
+    local $params->{inbound_proxies} = '10.0.0.1,10.0.0.2';
+    local $ENV{REMOTE_ADDR} = '10.0.0.3';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42';
+
+    is(remote_ip(), '10.0.0.3', "not a proxy");
+}
+
+{
+    local $params->{inbound_proxies} = '*';
+    local $ENV{REMOTE_ADDR} = '10.0.0.3';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42,1.4.9.2';
+
+    is(remote_ip(), '10.42.42.42', "always proxy");
+}
+
+{
+    local $params->{inbound_proxies} = '';
+    local $ENV{REMOTE_ADDR} = '10.9.8.7';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42,1.4.9.2';
+
+    is(remote_ip(), '10.9.8.7', "never proxy");
+}
+
+
+{
+    local $params->{inbound_proxies} = '10.0.0.1,2600:cafe::cafe:ffff:bf42:4998';
+    local $ENV{REMOTE_ADDR} = '2600:cafe::cafe:ffff:bf42:4998';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '2600:cafe::cafe:ffff:bf42:BEEF';
+
+    is(remote_ip(), '2600:cafe::cafe:ffff:bf42:BEEF', "from proxy ipv6");
+}
+
+{
+    local $params->{inbound_proxies} = '10.0.0.1,2600:cafe::cafe:ffff:bf42:4998';
+    local $ENV{REMOTE_ADDR} = '2600:cafe::cafe:ffff:bf42:DEAD';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '2600:cafe::cafe:ffff:bf42:BEEF';
+
+    is(remote_ip(), '2600:cafe::cafe:ffff:bf42:DEAD', "invalid proxy ipv6");
+}
+
+
+{
+    local $params->{inbound_proxies} = '*';
+    local $ENV{REMOTE_ADDR} = '2600:cafe::cafe:ffff:bf42:DEAD';
+    local $ENV{HTTP_X_FORWARDED_FOR} = '';
+
+    is(remote_ip(), '2600:cafe::cafe:ffff:bf42:DEAD', "always proxy ipv6");
+}

diff --git a/template/en/default/admin/params/advanced.html.tmpl b/template/en/default/admin/params/advanced.html.tmpl
index 1f24c7d06e683a9650652757d33002a329655504..1aedc9f9b097dad30ee0ee3d5eb4092fb54add72 100644 (file)
--- a/template/en/default/admin/params/advanced.html.tmpl
+++ b/template/en/default/admin/params/advanced.html.tmpl
@@ -54,7 +54,9 @@
     _ " user is the IP address of the proxy. If you enter a comma-separated"
     _ " list of IPs in this parameter, then 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.",
+    _ " and use the value of that header as the end user's IP address."
+    _ " If set to a *, $terms.Bugzilla will trust the first value in the "
+    _ " X-Forwarded-For header.",
 
   proxy_url => 
     "Bugzilla may have to access the web to get notifications about"