Bugzilla – Attachment 106647 Details for
Bug 25950
REMOTE_ADDR set to null if client_ip in X-Forwarded-For matches a koha_trusted_proxies value
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25950: Remove <client> in X-Forwarded-For from proxy tests
Bug-25950-Remove-client-in-X-Forwarded-For-from-pr.patch (text/plain), 10.05 KB, created by
David Cook
on 2020-07-08 07:32:00 UTC
(
hide
)
Description:
Bug 25950: Remove <client> in X-Forwarded-For from proxy tests
Filename:
MIME Type:
Creator:
David Cook
Created:
2020-07-08 07:32:00 UTC
Size:
10.05 KB
patch
obsolete
>From 1e3058f38d8754428044eb3d5626ee54497dd039 Mon Sep 17 00:00:00 2001 >From: David Cook <dcook@prosentient.com.au> >Date: Wed, 8 Jul 2020 07:10:50 +0000 >Subject: [PATCH] Bug 25950: Remove <client> in X-Forwarded-For from proxy > tests > >This patch removes the <client> ip address in the X-Forwarded-For >header from being tested against koha_trusted_proxies. > >Without this patch, REMOTE_ADDR will be set to null, if the <client> >ip address matches against koha_trusted_proxies. > >To Test: >1. Run the unit test t/Koha/Middleware/RealIP.t >--- > Koha/Middleware/RealIP.pm | 3 +- > t/Koha/Middleware/RealIP.t | 152 +++++++++++++++++++++++++++++---------------- > 2 files changed, 100 insertions(+), 55 deletions(-) > >diff --git a/Koha/Middleware/RealIP.pm b/Koha/Middleware/RealIP.pm >index 88a843dbcf..fc9f280ff6 100644 >--- a/Koha/Middleware/RealIP.pm >+++ b/Koha/Middleware/RealIP.pm >@@ -81,9 +81,10 @@ sub get_real_ip { > > my $trusted_proxies = get_trusted_proxies(); > >+ #X-Forwarded-For: <client>, <proxy1>, <proxy2> >+ my $real_ip = shift @forwarded_for; > my @unconfirmed = ( @forwarded_for, $remote_addr ); > >- my $real_ip; > while (my $addr = pop @unconfirmed) { > my $has_matched = 0; > foreach my $netmask (@$trusted_proxies) { >diff --git a/t/Koha/Middleware/RealIP.t b/t/Koha/Middleware/RealIP.t >index 34bc02af26..c60994b189 100644 >--- a/t/Koha/Middleware/RealIP.t >+++ b/t/Koha/Middleware/RealIP.t >@@ -20,7 +20,7 @@ > > use strict; > use warnings; >-use Test::More tests => 13; >+use Test::More tests => 11; > use Test::Warn; > > use t::lib::Mocks; >@@ -28,62 +28,106 @@ use_ok("Koha::Middleware::RealIP"); > > my ($remote_address,$x_forwarded_for_header,$address); > >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = ""; >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'1.1.1.1',"There is no X-Forwarded-For header, so just use the remote address"); >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2"; >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'1.1.1.1',"Don't trust 1.1.1.1 as a proxy, so use it as the remote address"); >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.1'); >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'2.2.2.2',"Trust proxy (1.1.1.1), so use the X-Forwarded-For header for the remote address"); >- >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2,3.3.3.3"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.1 3.3.3.3'); >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'2.2.2.2',"Trust multiple proxies (1.1.1.1 and 3.3.3.3), so use the X-Forwaded-For <client> portion for the remote address"); >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2,3.3.3.3"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad configuration'); >-warnings_are { >+subtest "No X-Forwarded-For header" => sub { >+ plan tests => 1; >+ $remote_address = "1.1.1.1"; >+ $x_forwarded_for_header = ""; > $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-} ["could not parse bad","could not parse configuration"],"Warn on misconfigured koha_trusted_proxies"; >-is($address,'1.1.1.1',"koha_trusted_proxies is misconfigured so ignore the X-Forwarded-For header"); >+ is($address,'1.1.1.1',"There is no X-Forwarded-For header, so just use the remote address"); >+}; >+ >+subtest "No configuration" => sub { >+ plan tests => 1; >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'2.2.2.2',"No trusted proxies available, so don't trust 2.2.2.2 as a proxy, instead use it as the remote address"); >+}; >+ >+subtest "Bad configuration" => sub { >+ plan tests => 4; >+ $remote_address = "1.1.1.1"; >+ $x_forwarded_for_header = "2.2.2.2,3.3.3.3"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad configuration'); >+ warnings_are { >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ } ["could not parse bad","could not parse configuration"],"Warn on misconfigured koha_trusted_proxies"; >+ is($address,'1.1.1.1',"koha_trusted_proxies is misconfigured so ignore the X-Forwarded-For header"); >+ >+ $remote_address = "1.1.1.1"; >+ $x_forwarded_for_header = "2.2.2.2"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad 1.1.1.1'); >+ warning_is { >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ } "could not parse bad","Warn on partially misconfigured koha_trusted_proxies"; >+ is($address,'2.2.2.2',"koha_trusted_proxies contains an invalid value but still includes one correct value, which is relevant, so use X-Forwarded-For header"); >+}; >+ >+subtest "Fail proxy isn't trusted" => sub { >+ plan tests => 1; >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '3.3.3.3'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'2.2.2.2',"The 2.2.2.2 proxy isn't in our trusted list, so use it as the remote address"); >+}; >+ >+subtest "The most recent proxy is trusted but the proxy before it is not trusted" => sub { >+ plan tests => 1; >+ $remote_address = "3.3.3.3"; >+ $x_forwarded_for_header = "1.1.1.1, 2.2.2.2"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '3.3.3.3'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'2.2.2.2',"We trust 3.3.3.3, but we don't trust 2.2.2.2, so use 2.2.2.2 as the remote address"); >+}; >+ >+subtest "Success one proxy" => sub { >+ plan tests => 1; >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.2'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'1.1.1.1',"Trust proxy (2.2.2.2), so use the client IP from the X-Forwarded-For header for the remote address"); >+}; >+ >+subtest "Success multiple proxies" => sub { >+ plan tests => 1; >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1,3.3.3.3"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.2 3.3.3.3'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'1.1.1.1',"Trust multiple proxies (2.2.2.2 and 3.3.3.3), so use the X-Forwaded-For <client> portion for the remote address"); >+}; >+ >+subtest "Test alternative configuration styles" => sub { >+ plan tests => 3; >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.0/24'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'1.1.1.1',"Trust proxy (2.2.2.2) using CIDR notation, so use the X-Forwarded-For header for the remote address"); > >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', 'bad 1.1.1.1'); >-warning_is { >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2'); > $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-} "could not parse bad","Warn on partially misconfigured koha_trusted_proxies"; >-is($address,'2.2.2.2',"koha_trusted_proxies contains an invalid value but still includes one correct value, which is relevant, so use X-Forwarded-For header"); >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.0/24'); >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'2.2.2.2',"Trust proxy (1.1.1.1) using CIDR notation, so use the X-Forwarded-For header for the remote address"); >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1'); >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'2.2.2.2',"Trust proxy (1.1.1.1) using abbreviated notation, so use the X-Forwarded-For header for the remote address"); >- >-$remote_address = "1.1.1.1"; >-$x_forwarded_for_header = "2.2.2.2"; >-t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.0:255.255.255.0'); >-$address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >-is($address,'2.2.2.2',"Trust proxy (1.1.1.1) using an IP address and netmask separated by a colon, so use the X-Forwarded-For header for the remote address"); >+ is($address,'1.1.1.1',"Trust proxy (2.2.2.2) using abbreviated notation, so use the X-Forwarded-For header for the remote address"); >+ >+ $remote_address = "2.2.2.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '2.2.2.0:255.255.255.0'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'1.1.1.1',"Trust proxy (2.2.2.2) using an IP address and netmask separated by a colon, so use the X-Forwarded-For header for the remote address"); >+}; >+ >+subtest "Client IP is properly processed even if it is in koha_trusted_proxies" => sub { >+ $remote_address = "1.1.1.2"; >+ $x_forwarded_for_header = "1.1.1.1"; >+ t::lib::Mocks::mock_config('koha_trusted_proxies', '1.1.1.0/24'); >+ $address = Koha::Middleware::RealIP::get_real_ip( $remote_address, $x_forwarded_for_header ); >+ is($address,'1.1.1.1',"The X-Forwarded-For value matches the koha_trusted_proxies, but there is no proxy specified"); >+ done_testing(1); >+}; > > subtest "IPv6 support" => sub { > my ($remote_address,$x_forwarded_for_header,$address); >-- >2.11.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25950
:
106646
|
106647
|
106722
|
106763