From dfc33c89b8cac25951867a2e6821120c92bb055b Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Mon, 4 Jul 2016 11:43:44 -0400 Subject: Bug 1284277 - allow inbound_proxy to be set to '*' r=dkl --- Bugzilla/Config/Advanced.pm | 13 +++- Bugzilla/Util.pm | 35 ++++------ t/013remote_ip.t | 81 ++++++++++++++++++++++ .../en/default/admin/params/advanced.html.tmpl | 4 +- 4 files changed, 111 insertions(+), 22 deletions(-) create mode 100644 t/013remote_ip.t diff --git a/Bugzilla/Config/Advanced.pm b/Bugzilla/Config/Advanced.pm index 75afe7b22..b3968a254 100644 --- 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 e673a920e..dc41652f7 100644 --- 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 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 1f24c7d06..1aedc9f9b 100644 --- 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" _ " X-Forwarded-For 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" -- cgit v1.2.3-24-g4f1b