diff options
| -rw-r--r-- | Bugzilla/Config/Advanced.pm | 13 | ||||
| -rw-r--r-- | Bugzilla/Config/Common.pm | 11 | ||||
| -rw-r--r-- | Bugzilla/Util.pm | 35 | ||||
| -rw-r--r-- | t/013remote_ip.t | 81 | ||||
| -rw-r--r-- | template/en/default/admin/params/advanced.html.tmpl | 4 |
5 files changed, 112 insertions, 32 deletions
diff --git a/Bugzilla/Config/Advanced.pm b/Bugzilla/Config/Advanced.pm index 4b57df24d..893213b02 100644 --- a/Bugzilla/Config/Advanced.pm +++ b/Bugzilla/Config/Advanced.pm @@ -47,7 +47,7 @@ use constant get_param_list => ( name => 'inbound_proxies', type => 't', default => '', - checker => \&check_ip + checker => \&check_inbound_proxies }, { @@ -108,4 +108,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/Config/Common.pm b/Bugzilla/Config/Common.pm index 35a70994b..4c9994e0e 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -48,7 +48,7 @@ use base qw(Exporter); qw(check_multi check_numeric check_regexp check_url check_group check_sslbase check_priority check_severity check_platform check_opsys check_shadowdb check_urlbase check_webdotbase - check_user_verify_class check_ip + check_user_verify_class check_mail_delivery_method check_notification check_utf8 check_bug_status check_smtp_auth check_theschwartz_available check_maxattachmentsize check_email @@ -130,15 +130,6 @@ sub check_sslbase { return ""; } -sub check_ip { - my $inbound_proxies = shift; - my @proxies = split(/[\s,]+/, $inbound_proxies); - foreach my $proxy (@proxies) { - validate_ip($proxy) || return "$proxy is not a valid IPv4 or IPv6 address"; - } - return ""; -} - sub check_utf8 { my $utf8 = shift; # You cannot turn off the UTF-8 parameter if you've already converted diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index d80ab9569..f793ed727 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -57,7 +57,7 @@ use DateTime; use DateTime::TimeZone; 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); @@ -305,28 +305,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 index 000000000..1cc832a9b --- /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 a2103c652..6cd13a49d 100644 --- a/template/en/default/admin/params/advanced.html.tmpl +++ b/template/en/default/admin/params/advanced.html.tmpl @@ -67,7 +67,9 @@ _ " 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.", + _ " 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 => "$terms.Bugzilla may have to access the web to get notifications about" |