Bugzilla – Attachment 6703 Details for
Bug 6800
Koha authentication should handle proxies better
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 6800: Handle X-Forwarded-For headers
Bug-6800-Handle-X-Forwarded-For-headers.patch (text/plain), 18.23 KB, created by
Jared Camins-Esakov
on 2011-12-11 00:29:19 UTC
(
hide
)
Description:
Bug 6800: Handle X-Forwarded-For headers
Filename:
MIME Type:
Creator:
Jared Camins-Esakov
Created:
2011-12-11 00:29:19 UTC
Size:
18.23 KB
patch
obsolete
>From 3bfeff39d2cfcbf015503af4b4f6bbb5c72c0e31 Mon Sep 17 00:00:00 2001 >From: Jared Camins-Esakov <jcamins@cpbibliography.com> >Date: Sat, 27 Aug 2011 12:10:24 -0400 >Subject: [PATCH] Bug 6800: Handle X-Forwarded-For headers >Content-Type: text/plain; charset="UTF-8" > >Previously Koha always used the remote address for its sessions. This is a >problem where a sizable percentage of sessions are being routed through the >same proxy (for example, in the case of load balancers or reverse proxies, >or even a corporate proxy). This commit adds support for pulling the >client's IP address out of the X-Forwarded-For HTTP header, so that sessions >will be keyed to the client and not the proxy. > >Although X-Forwarded-For can be spoofed, in situations where all clients >would have the same immediate REMOTE_ADDRESS (e.g. load balancing, reverse >proxy, corporate firewall), using X-Forwarded-For seems the lesser of two >evils (if you're running the proxy, you can guarantee that the most recent >entry in X-Forwarded-For is accurate, hence the behavior when the syspref is >set to require a routable IP). > >=== SYSPREFS === >This commit adds the syspref HandleXForwardedFor with the following options: >* Always use the address of the machine connecting to Koha as the client IP > for authenticated sessions. This is appropriate for configurations with > no reverse proxy or load balancer, and is exactly the same as the > previous behavior. >* Always use the address of the machine with the web browser as the client > IP for authenticated sessions. This is appropriate for configurations > that are contained entirely within a LAN, and therefore non-routable IPs > can be mapped to specific computers. >* Use the first routable address or the address of the last hop before the > proxy as the client IP for authenticated sessions. This is appropriate > for configurations that include a reverse proxy or load balancer exposed > via the public Internet. Anyone connecting through an additional proxy > will have their session linked to that proxy's IP. > >=== API CHANGES === >This commit adds the get_clientip method to C4::Auth to handle >identification of the client IP: > > my $clientip = get_clientip($remote_addr, $forwarded_for, $require_routable); > >Parses the remote IP address (passed to the function in $remote_addr), the >X-Forwarded-For header (passed to the function in $forwarded_for), and >retrieves the IP address of the client, returning a string representation of >the IP address. If $require_routable is set to "first", this function will >always return the most-distant IP address. If $require_routable is set to >"routable", this function will choose the first routable IP address in the >list of relays, or the address immediately before the closest proxy. If >$require_routable is set to "ignore", this function will always return the >most recent hop (i.e. the remote address). "Ignore" is the default, if >$require_routable is not set. > >=== TESTING INSTRUCTIONS === >The problem with the current configuration in Koha can be seen by >configuring Koha to listen on 127.0.0.1 and setting up a Squid proxy with >the following configuration options on the same server: > > # BEGIN SQUID CONFIGURATION > # The next two lines must go at the top of the squid configuration file: >http_port ${PUBLIC_IP}:80 accel defaultsite=${YOUR_DOMAIN} vhost >cache_peer 127.0.0.1 parent 80 0 no-query originserver name=myAccel > > # The next four lines must go AFTER the line "acl CONNECT method CONNECT >acl our_sites dstdomain .${YOUR_DOMAIN} >http_access allow our_sites >cache_peer_access myAccel allow our_sites >cache_peer_access myAccel deny all > # END SQUID CONFIGURATION > >If you view the session log after connecting via ${PUBLIC_IP}:80, you will >see an entry for 127.0.0.1. This is the default behavior after this patch is >applied as well, but by changing the syspref HandleXForwardedFor to "Always >use the address of the originating machine," you can ensure that the IP that >shows up will always be the IP address of the machine with the web browser, >or by setting the syspref to "Use the first routable address or address of >last hop before proxy," you can ensure that the IP will always be either the >first routable address or the address of the system connecting to the >reverse proxy. On a LAN, the difference between those two options can be >tested by daisy-chaining a second squid proxy to the first, and connecting >through that. > >In addition to these steps for testing, several tests have been added to >confirm that C4::Auth::get_clientip correctly handles valid input. >--- > C4/Auth.pm | 85 ++++++++++++++++---- > installer/data/mysql/sysprefs.sql | 2 +- > installer/data/mysql/updatedatabase.pl | 9 ++- > .../prog/en/modules/admin/preferences/admin.pref | 8 ++ > t/Auth.t | 17 ++++ > 5 files changed, 104 insertions(+), 17 deletions(-) > create mode 100644 t/Auth.t > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index e360e10..977985c 100755 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -646,6 +646,7 @@ sub checkauth { > $timeout = $1 * 86400; > }; > $timeout = 600 unless $timeout; >+ my $clientip = get_clientip($ENV{'REMOTE_ADDR'}, $ENV{'HTTP_X_FORWARDED_FOR'}, C4::Context->preference('HandleXForwardedFor')); > > _version_check($type,$query); > # state variables >@@ -720,10 +721,10 @@ sub checkauth { > $userid = undef; > $sessionID = undef; > } >- elsif ( $ip ne $ENV{'REMOTE_ADDR'} ) { >+ elsif ($ip ne $clientip) { > # Different ip than originally logged in from > $info{'oldip'} = $ip; >- $info{'newip'} = $ENV{'REMOTE_ADDR'}; >+ $info{'newip'} = $clientip; > $info{'different_ip'} = 1; > $session->delete(); > C4::Context->_unset_userenv($sessionID); >@@ -765,7 +766,7 @@ sub checkauth { > $userid = $retuserid if ($retuserid ne ''); > } > if ($return) { >- _session_log(sprintf "%20s from %16s logged in at %30s.\n", $userid,$ENV{'REMOTE_ADDR'},(strftime '%c', localtime)); >+ _session_log(sprintf "%20s from %16s logged in at %30s.\n", $userid,$clientip,(strftime '%c', localtime)); > if ( $flags = haspermission( $userid, $flagsrequired ) ) { > $loggedin = 1; > } >@@ -813,7 +814,6 @@ sub checkauth { > # launch a sequence to check if we have a ip for the branch, i > # if we have one we replace the branchcode of the userenv by the branch bound in the ip. > >- my $ip = $ENV{'REMOTE_ADDR'}; > # if they specify at login, use that > if ($query->param('branch')) { > $branchcode = $query->param('branch'); >@@ -823,7 +823,7 @@ sub checkauth { > if (C4::Context->boolean_preference('IndependantBranches') && C4::Context->boolean_preference('Autolocation')){ > # we have to check they are coming from the right ip range > my $domain = $branches->{$branchcode}->{'branchip'}; >- if ($ip !~ /^$domain/){ >+ if ($clientip !~ /^$domain/){ > $loggedin=0; > $info{'wrongip'} = 1; > } >@@ -833,7 +833,7 @@ sub checkauth { > foreach my $br ( keys %$branches ) { > # now we work with the treatment of ip > my $domain = $branches->{$br}->{'branchip'}; >- if ( $domain && $ip =~ /^$domain/ ) { >+ if ( $domain && $clientip =~ /^$domain/ ) { > $branchcode = $branches->{$br}->{'branchcode'}; > > # new op dev : add the branchprinter and branchname in the cookie >@@ -850,7 +850,7 @@ sub checkauth { > $session->param('branchname',$branchname); > $session->param('flags',$userflags); > $session->param('emailaddress',$emailaddress); >- $session->param('ip',$session->remote_addr()); >+ $session->param('ip',$clientip); > $session->param('lasttime',time()); > $debug and printf STDERR "AUTH_4: (%s)\t%s %s - %s\n", map {$session->param($_)} qw(cardnumber firstname surname branch) ; > } >@@ -866,7 +866,7 @@ sub checkauth { > $session->param('branchname','NO_LIBRARY_SET'); > $session->param('flags',1); > $session->param('emailaddress', C4::Context->preference('KohaAdminEmailAddress')); >- $session->param('ip',$session->remote_addr()); >+ $session->param('ip',$clientip); > $session->param('lasttime',time()); > } > C4::Context::set_userenv( >@@ -917,7 +917,7 @@ sub checkauth { > C4::Context::set_shelves_userenv('tot',$total); > > # setting a couple of other session vars... >- $session->param('ip',$session->remote_addr()); >+ $session->param('ip',$clientip); > $session->param('lasttime',time()); > $session->param('sessiontype','anon'); > } >@@ -1088,6 +1088,7 @@ sub check_api_auth { > my $dbh = C4::Context->dbh; > my $timeout = C4::Context->preference('timeout'); > $timeout = 600 unless $timeout; >+ my $clientip = get_clientip($ENV{'REMOTE_ADDR'}, $ENV{'HTTP_X_FORWARDED_FOR'}, C4::Context->preference('HandleXForwardedFor')); > > unless (C4::Context->preference('Version')) { > # database has not been installed yet >@@ -1139,7 +1140,7 @@ sub check_api_auth { > $userid = undef; > $sessionID = undef; > return ("expired", undef, undef); >- } elsif ( $ip ne $ENV{'REMOTE_ADDR'} ) { >+ } elsif ( $ip ne $clientip ) { > # IP address changed > $session->delete(); > C4::Context->_unset_userenv($sessionID); >@@ -1229,7 +1230,6 @@ sub check_api_auth { > } > } > >- my $ip = $ENV{'REMOTE_ADDR'}; > # if they specify at login, use that > if ($query->param('branch')) { > $branchcode = $query->param('branch'); >@@ -1240,7 +1240,7 @@ sub check_api_auth { > foreach my $br ( keys %$branches ) { > # now we work with the treatment of ip > my $domain = $branches->{$br}->{'branchip'}; >- if ( $domain && $ip =~ /^$domain/ ) { >+ if ( $domain && $clientip =~ /^$domain/ ) { > $branchcode = $branches->{$br}->{'branchcode'}; > > # new op dev : add the branchprinter and branchname in the cookie >@@ -1257,7 +1257,7 @@ sub check_api_auth { > $session->param('branchname',$branchname); > $session->param('flags',$userflags); > $session->param('emailaddress',$emailaddress); >- $session->param('ip',$session->remote_addr()); >+ $session->param('ip',$clientip); > $session->param('lasttime',time()); > } elsif ( $return == 2 ) { > #We suppose the user is the superlibrarian >@@ -1270,7 +1270,7 @@ sub check_api_auth { > $session->param('branchname','NO_LIBRARY_SET'); > $session->param('flags',1); > $session->param('emailaddress', C4::Context->preference('KohaAdminEmailAddress')); >- $session->param('ip',$session->remote_addr()); >+ $session->param('ip',$clientip); > $session->param('lasttime',time()); > } > C4::Context::set_userenv( >@@ -1321,6 +1321,7 @@ sub check_cookie_auth { > my $dbh = C4::Context->dbh; > my $timeout = C4::Context->preference('timeout'); > $timeout = 600 unless $timeout; >+ my $clientip = get_clientip($ENV{'REMOTE_ADDR'}, $ENV{'HTTP_X_FORWARDED_FOR'}, C4::Context->preference('HandleXForwardedFor')); > > unless (C4::Context->preference('Version')) { > # database has not been installed yet >@@ -1371,7 +1372,7 @@ sub check_cookie_auth { > $userid = undef; > $sessionID = undef; > return ("expired", undef); >- } elsif ( $ip ne $ENV{'REMOTE_ADDR'} ) { >+ } elsif ( $ip ne $clientip ) { > # IP address changed > $session->delete(); > C4::Context->_unset_userenv($sessionID); >@@ -1396,6 +1397,60 @@ sub check_cookie_auth { > } > } > >+=head2 get_clientip >+ >+ my $clientip = get_clientip($remote_addr, $forwarded_for, $require_routable); >+ >+Parses the remote IP address (passed to the function in $remote_addr), the >+X-Forwarded-For header (passed to the function in $forwarded_for), and retrieves >+the IP address of the client, returning a string representation of the IP >+address. If $require_routable is set to "first", this function will always >+return the most-distant IP address. If $require_routable is set to "routable", >+this function will choose the first routable IP address in the list of relays, >+or the address immediately before the closest proxy. If $require_routable is set >+to "ignore", this function will always return the most recent hop (i.e. the >+remote address). "Ignore" is the default. >+ >+=cut >+ >+sub get_clientip { >+ my $remote_addr = shift; >+ my $forwarded_for = shift; >+ my $require_routable = shift; >+ my $clientip; >+ >+ $require_routable ||= 'ignore'; >+ >+ if ($require_routable eq 'ignore' || !length $forwarded_for) { >+ $clientip = $remote_addr; >+ } else { >+ my @ips = split(', ', $forwarded_for); >+ $ips[$#ips + 1] = $remote_addr; >+ if ($require_routable eq 'first') { >+ $clientip = $ips[0]; >+ } else { # $require_routable eq 'routable' >+ foreach (@ips) { >+ if ( $_ !~ /^(192\.|10\.|127\.|0\.|169\.254\.|2([345][0-9|2[4-9])\.|172\.(1[6-9]|2[0-9]|3[0-1])\.)/) { >+ $clientip = $_; >+ last; >+ # We've found the client if the IP is not in any of: >+ # * 192.0.0.0/8 >+ # * 10.0.0.0/8 >+ # * 127.0.0.0/8 >+ # * 169.254.0.0/16 >+ # * 224.0.0.0/4 or 240.0.0.0/4 >+ # * 172.16.0.0/12 >+ } >+ } >+ if (!length $clientip) { >+ $clientip = $ips[$#ips - 1]; >+ } >+ } >+ } >+ >+ return $clientip; >+} >+ > =head2 get_session > > use CGI::Session; >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index ae2c1cb..f74a717 100755 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -328,4 +328,4 @@ INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES(' > INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES ('OpacKohaUrl','1',"Show 'Powered by Koha' text on OPAC footer.",NULL,NULL); > INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('EasyAnalyticalRecords','0','If on, display in the catalogue screens tools to easily setup analytical record relationships','','YesNo'); > INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('OpacShowRecentComments',0,'If ON a link to recent comments will appear in the OPAC masthead',NULL,'YesNo'); >- >+INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('HandleXForwardedFor','ignore','Specify how to handle client IPs and the X-Forwarded-For header.','ignore|first|routable','Choice'); >diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl >index 0063a75..0a16a09 100755 >--- a/installer/data/mysql/updatedatabase.pl >+++ b/installer/data/mysql/updatedatabase.pl >@@ -4438,7 +4438,6 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { > SetVersion($DBversion); > } > >- > $DBversion = "3.05.00.011"; > if (C4::Context->preference("Version") < TransformToNum($DBversion)) { > $dbh->do("INSERT INTO `systempreferences` (variable,value,explanation,options,type) VALUES('OPACResultsSidebar','','Define HTML to be included on the search results page, underneath the facets sidebar','70|10','Textarea')"); >@@ -4578,6 +4577,14 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { > SetVersion($DBversion); > } > >+$DBversion = "3.07.00.XXX"; >+if (C4::Context->preference("Version") < TransformToNum($DBversion)) { >+ $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('HandleXForwardedFor','ignore','Specify how to handle client IPs and the X-Forwarded-For header.','ignore|first|routable','Choice');"); >+ print "Upgrade to $DBversion done (add HandleXForwardedFor syspref- change if you are using a reverse proxy or load balancer)\n"; >+ SetVersion($DBversion); >+} >+ >+ > =head1 FUNCTIONS > > =head2 DropAllForeignKeys($table) >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref >index f026c7e..3ccc833 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref >@@ -71,6 +71,14 @@ Administration: > tmp: as temporary files. > memcached: in a memcached server. > - >+ - pref: HandleXForwardedFor >+ default: ignore >+ choices: >+ ignore: Always use the address of the machine connecting to Koha >+ routable: Use the first routable address or address of the last hop before the proxy >+ first: Always use the address of the machine with the web browser >+ - as the client IP for authenticated sessions. >+ - > - pref: IndependantBranches > default: 0 > choices: >diff --git a/t/Auth.t b/t/Auth.t >new file mode 100644 >index 0000000..c265c3c >--- /dev/null >+++ b/t/Auth.t >@@ -0,0 +1,17 @@ >+#!/usr/bin/perl >+# >+# This Koha test module is a stub! >+# Add more tests here!!! >+ >+use strict; >+use warnings; >+ >+use Test::More tests => 5; >+ >+BEGIN { >+ use_ok('C4::Auth'); >+ ok(C4::Auth::get_clientip('192.168.101.2', '192.168.101.3, 192.168.101.4', 'ignore') eq '192.168.101.2', 'get_clientip: Ignore proxies for client IP'); >+ ok(C4::Auth::get_clientip('192.168.101.2', '192.168.101.3, 192.168.101.4', 'first') eq '192.168.101.3', 'get_clientip: Always use most remote client IP'); >+ ok(C4::Auth::get_clientip('192.168.101.2', '127.0.0.1, 192.168.102.3, 10.0.0.1, 53.42.191.136', 'routable') eq '53.42.191.136', 'get_clientip: Find routable client IP w/reverse proxy'); >+ ok(C4::Auth::get_clientip('192.168.101.2', '127.0.0.1, 192.168.102.3, 10.0.0.1', 'routable') eq '10.0.0.1', 'get_clientip: Handle no-routable IPs w/reverse proxy'); >+} >-- >1.7.2.5
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 6800
:
5185
|
6703
|
6894