Skip to content

Commit 175f9c1

Browse files
dylanwhdylanwh
committed
Bug 1217536 - allow inbound_proxy supports '*'
1 parent f521f52 commit 175f9c1

File tree

5 files changed

+112
-32
lines changed

5 files changed

+112
-32
lines changed

‎Bugzilla/Config/Advanced.pm‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ use constant get_param_list => (
4747
name => 'inbound_proxies',
4848
type => 't',
4949
default => '',
50-
checker => \&check_ip
50+
checker => \&check_inbound_proxies
5151
},
5252

5353
{
@@ -108,4 +108,15 @@ use constant get_param_list => (
108108
},
109109
);
110110

111+
sub check_inbound_proxies {
112+
my $inbound_proxies = shift;
113+
114+
return "" if $inbound_proxies eq "*";
115+
my @proxies = split(/[\s,]+/, $inbound_proxies);
116+
foreach my $proxy (@proxies) {
117+
validate_ip($proxy) || return "$proxy is not a valid IPv4 or IPv6 address";
118+
}
119+
return "";
120+
}
121+
111122
1;

‎Bugzilla/Config/Common.pm‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ use base qw(Exporter);
4848
qw(check_multi check_numeric check_regexp check_url check_group
4949
check_sslbase check_priority check_severity check_platform
5050
check_opsys check_shadowdb check_urlbase check_webdotbase
51-
check_user_verify_class check_ip
51+
check_user_verify_class
5252
check_mail_delivery_method check_notification check_utf8
5353
check_bug_status check_smtp_auth check_theschwartz_available
5454
check_maxattachmentsize check_email
@@ -130,15 +130,6 @@ sub check_sslbase {
130130
return "";
131131
}
132132

133-
sub check_ip {
134-
my $inbound_proxies = shift;
135-
my @proxies = split(/[\s,]+/, $inbound_proxies);
136-
foreach my $proxy (@proxies) {
137-
validate_ip($proxy) || return "$proxy is not a valid IPv4 or IPv6 address";
138-
}
139-
return "";
140-
}
141-
142133
sub check_utf8 {
143134
my $utf8 = shift;
144135
# You cannot turn off the UTF-8 parameter if you've already converted

‎Bugzilla/Util.pm‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use DateTime;
5757
use DateTime::TimeZone;
5858
use Digest;
5959
use Email::Address;
60-
use List::Util qw(first);
60+
use List::MoreUtils qw(none);
6161
use Scalar::Util qw(tainted blessed);
6262
use Text::Wrap;
6363
use Encode qw(encode decode resolve_alias);
@@ -305,28 +305,23 @@ sub correct_urlbase {
305305
}
306306
}
307307

308+
# Returns the real remote address of the client,
308309
sub remote_ip {
309-
my $ip = $ENV{'REMOTE_ADDR'} || '127.0.0.1';
310-
my @proxies = split(/[\s,]+/, Bugzilla->params->{'inbound_proxies'});
311-
312-
# If the IP address is one of our trusted proxies, then we look at
313-
# the X-Forwarded-For header to determine the real remote IP address.
314-
if ($ENV{'HTTP_X_FORWARDED_FOR'} && first { $_ eq $ip } @proxies) {
315-
my @ips = split(/[\s,]+/, $ENV{'HTTP_X_FORWARDED_FOR'});
316-
# This header can contain several IP addresses. We want the
317-
# IP address of the machine which connected to our proxies as
318-
# all other IP addresses may be fake or internal ones.
319-
# Note that this may block a whole external proxy, but we have
320-
# no way to determine if this proxy is malicious or trustable.
321-
foreach my $remote_ip (reverse @ips) {
322-
if (!first { $_ eq $remote_ip } @proxies) {
323-
# Keep the original IP address if the remote IP is invalid.
324-
$ip = validate_ip($remote_ip) || $ip;
325-
last;
326-
}
310+
my $remote_ip = $ENV{'REMOTE_ADDR'} || '127.0.0.1';
311+
my @proxies = split(/[\s,]+/, Bugzilla->params->{inbound_proxies});
312+
my @x_forwarded_for = split(/[\s,]+/, $ENV{HTTP_X_FORWARDED_FOR} // '');
313+
314+
return $remote_ip unless @x_forwarded_for;
315+
return $x_forwarded_for[0] if $proxies[0] eq '*';
316+
return $remote_ip if none { $_ eq $remote_ip } @proxies;
317+
318+
foreach my $ip (reverse @x_forwarded_for) {
319+
if (none { $_ eq $ip } @proxies) {
320+
# Keep the original IP address if the remote IP is invalid.
321+
return validate_ip($ip) || $remote_ip;
327322
}
328323
}
329-
return $ip;
324+
return $remote_ip;
330325
}
331326

332327
sub validate_ip {

‎t/013remote_ip.t‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#!/usr/bin/perl
2+
# This Source Code Form is subject to the terms of the Mozilla Public
3+
# License, v. 2.0. If a copy of the MPL was not distributed with this
4+
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
5+
#
6+
# This Source Code Form is "Incompatible With Secondary Licenses", as
7+
# defined by the Mozilla Public License, v. 2.0.
8+
9+
use strict;
10+
use lib qw(. lib t);
11+
use Test::More qw(no_plan);
12+
use Bugzilla;
13+
use Bugzilla::Util qw(remote_ip);
14+
15+
my $params = Bugzilla->params;
16+
17+
{
18+
local $params->{inbound_proxies} = '10.0.0.1,10.0.0.2';
19+
local $ENV{REMOTE_ADDR} = '10.0.0.2';
20+
local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42';
21+
22+
is(remote_ip(), '10.42.42.42', "from proxy 2");
23+
}
24+
25+
{
26+
local $params->{inbound_proxies} = '10.0.0.1,10.0.0.2';
27+
local $ENV{REMOTE_ADDR} = '10.0.0.1';
28+
local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42';
29+
30+
is(remote_ip(), '10.42.42.42', "from proxy 1");
31+
}
32+
33+
{
34+
local $params->{inbound_proxies} = '10.0.0.1,10.0.0.2';
35+
local $ENV{REMOTE_ADDR} = '10.0.0.3';
36+
local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42';
37+
38+
is(remote_ip(), '10.0.0.3', "not a proxy");
39+
}
40+
41+
{
42+
local $params->{inbound_proxies} = '*';
43+
local $ENV{REMOTE_ADDR} = '10.0.0.3';
44+
local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42,1.4.9.2';
45+
46+
is(remote_ip(), '10.42.42.42', "always proxy");
47+
}
48+
49+
{
50+
local $params->{inbound_proxies} = '';
51+
local $ENV{REMOTE_ADDR} = '10.9.8.7';
52+
local $ENV{HTTP_X_FORWARDED_FOR} = '10.42.42.42,1.4.9.2';
53+
54+
is(remote_ip(), '10.9.8.7', "never proxy");
55+
}
56+
57+
58+
{
59+
local $params->{inbound_proxies} = '10.0.0.1,2600:cafe::cafe:ffff:bf42:4998';
60+
local $ENV{REMOTE_ADDR} = '2600:cafe::cafe:ffff:bf42:4998';
61+
local $ENV{HTTP_X_FORWARDED_FOR} = '2600:cafe::cafe:ffff:bf42:BEEF';
62+
63+
is(remote_ip(), '2600:cafe::cafe:ffff:bf42:BEEF', "from proxy ipv6");
64+
}
65+
66+
{
67+
local $params->{inbound_proxies} = '10.0.0.1,2600:cafe::cafe:ffff:bf42:4998';
68+
local $ENV{REMOTE_ADDR} = '2600:cafe::cafe:ffff:bf42:DEAD';
69+
local $ENV{HTTP_X_FORWARDED_FOR} = '2600:cafe::cafe:ffff:bf42:BEEF';
70+
71+
is(remote_ip(), '2600:cafe::cafe:ffff:bf42:DEAD', "invalid proxy ipv6");
72+
}
73+
74+
75+
{
76+
local $params->{inbound_proxies} = '*';
77+
local $ENV{REMOTE_ADDR} = '2600:cafe::cafe:ffff:bf42:DEAD';
78+
local $ENV{HTTP_X_FORWARDED_FOR} = '';
79+
80+
is(remote_ip(), '2600:cafe::cafe:ffff:bf42:DEAD', "always proxy ipv6");
81+
}

‎template/en/default/admin/params/advanced.html.tmpl‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@
6767
_ " user is the IP address of the proxy. If you enter a comma-separated"
6868
_ " list of IPs in this parameter, then $terms.Bugzilla will trust any"
6969
_ " <code>X-Forwarded-For</code> header sent from those IPs,"
70-
_ " and use the value of that header as the end user's IP address.",
70+
_ " and use the value of that header as the end user's IP address."
71+
_ " If set to a *, $terms.Bugzilla will trust the first value in the "
72+
_ " X-Forwarded-For header.",
7173

7274
proxy_url =>
7375
"$terms.Bugzilla may have to access the web to get notifications about"

0 commit comments

Comments
 (0)