Replace useXForwardedFor option with trustedProxies (#559)

debian/control

@@ -105,6 +105,7 @@ Depends: ${misc:Depends},
          libxml-simple-perl,
          libxml-libxml-perl,
          libxml-libxslt-perl,
+         libnet-cidr-lite-perl
 Recommends: libdbi-perl,
             libsoap-lite-perl
 Description: Lemonldap::NG common files

lemonldap-ng-common/META.yml

@@ -27,6 +27,7 @@ requires:
     XML::Simple:          0
     XML::LibXML:          0
     XML::LibXSLT:         0
+    Net::CIDR::Lite:      0
 no_index:
     directory:
         - t

lemonldap-ng-common/Makefile.PL

@@ -41,6 +41,7 @@ WriteMakefile(
         'Crypt::OpenSSL::RSA'    => 0,
         'Crypt::OpenSSL::X509'   => 0,
         'Crypt::OpenSSL::Bignum' => 0,
+        'Net::CIDR::Lite'        => 0,
     },    # e.g., Module::Name => 1.1
           #EXE_FILES => [ 'scripts/convertConfig', ],
     (

lemonldap-ng-common/lib/Lemonldap/NG/Common/CGI.pm

@@ -13,10 +13,11 @@ use Time::Local;
 use CGI;
 use utf8;
 use Encode;
+use Net::CIDR::Lite;
 
 #parameter syslog Indicates syslog facility for logging user actions
 
-our $VERSION = '1.2.2';
+our $VERSION = '1.2.3';
 
 use base qw(CGI);
 
@@ -296,7 +297,7 @@ sub userLog {
 # @param $mess string to log
 sub userInfo {
     my ( $self, $mess ) = @_;
-    $mess = "Lemonldap::NG : $mess ($ENV{REMOTE_ADDR})";
+    $mess = "Lemonldap::NG : $mess (" . $self->ipAddr . ")";
     $self->userLog( $mess, 'info' );
 }
 
@@ -306,7 +307,7 @@ sub userInfo {
 # @param $mess string to log
 sub userNotice {
     my ( $self, $mess ) = @_;
-    $mess = "Lemonldap::NG : $mess ($ENV{REMOTE_ADDR})";
+    $mess = "Lemonldap::NG : $mess (" . $self->ipAddr . ")";
     $self->userLog( $mess, 'notice' );
 }
 
@@ -316,7 +317,7 @@ sub userNotice {
 # @param $mess string to log
 sub userError {
     my ( $self, $mess ) = @_;
-    $mess = "Lemonldap::NG : $mess ($ENV{REMOTE_ADDR})";
+    $mess = "Lemonldap::NG : $mess (" . $self->ipAddr . ")";
     $self->userLog( $mess, 'warn' );
 }
 
@@ -406,6 +407,43 @@ sub quit {
     exit;
 }
 
+##@method string ipAddr()
+# Retrieve client IP address from remote address or X-FORWARDED-FOR header
+#@return client IP
+sub ipAddr {
+    my $self = shift;
+
+    unless ( $self->{ipAddr} ) {
+        $self->{ipAddr} = $ENV{REMOTE_ADDR};
+        if ( my $xheader = $ENV{HTTP_X_FORWARDED_FOR} ) {
+            if (   $self->{trustedProxies} =~ /\*/
+                or $self->{useXForwardedForIP} )
+            {
+                $self->{ipAddr} = $1 if ( $xheader =~ /^([^,]*)/ );
+            }
+            elsif ( $self->{trustedProxies} ) {
+                my $localIP =
+                  Net::CIDR::Lite->new("127.0.0.0/8"); # TODO: add IPv6 local IP
+                my $trustedIP =
+                  Net::CIDR::Lite->new( split /\s+/, $self->{trustedProxies} );
+                while (
+                    (
+                           $localIP->find( $self->{ipAddr} )
+                        or $trustedIP->find( $self->{ipAddr} )
+                    )
+                    and $xheader =~ s/[,\s]*([^,\s]+)$//
+                  )
+                {
+
+                    # because it is of no use to store a local IP as client IP
+                    $self->{ipAddr} = $1 unless ( $localIP->find($1) );
+                }
+            }
+        }
+    }
+    return $self->{ipAddr};
+}
+
 1;
 
 __END__

lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Notifications.pm

@@ -645,8 +645,6 @@ notifications
           type=>'File',
           dirName=>"/tmp/",
         },
-        # Force the use of X-FORWARDED-FOR for IP
-        useXForwardedForIP => 1,
         # Optionnal
         protection    => 'rule: $uid eq "admin"',
         # Or to use rules from manager

lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Uploader.pm

@@ -358,7 +358,7 @@ s/^(samlSPMetaDataXML|samlSPMetaDataExportedAttributes|samlSPMetaDataOptions)\/(
 
     # 1.5 Author attributes for accounting
     $newConf->{cfgAuthor}   = $ENV{REMOTE_USER} || 'anonymous';
-    $newConf->{cfgAuthorIP} = $ENV{REMOTE_ADDR};
+    $newConf->{cfgAuthorIP} = $self->ipAddr;
     $newConf->{cfgDate}     = time();
 
     # 1.6 Global tests

lemonldap-ng-manager/lib/Lemonldap/NG/Manager/_Struct.pm

@@ -9,7 +9,7 @@ use strict;
 use Lemonldap::NG::Common::Conf::SAML::Metadata;
 use Lemonldap::NG::Common::Regexp;
 
-our $VERSION = '1.2.2';
+our $VERSION = '1.2.3';
 
 ## @method protected hashref cstruct(hashref h,string k)
 # Merge $h with the structure produced with $k and return it.
@@ -715,11 +715,11 @@ sub struct {
 
             # LOGS PARAMETERS
             logParams => {
-                _nodes => [qw(syslog useXForwardedForIP whatToTrace)],
+                _nodes => [qw(syslog trustedProxies whatToTrace)],
                 _help  => 'logs',
                 syslog => 'text:/syslog',
-                useXForwardedForIP => 'bool:/useXForwardedForIP',
-                whatToTrace        => 'text:/whatToTrace',
+                trustedProxies => 'text:/trustedProxies',
+                whatToTrace    => 'text:/whatToTrace',
             },
 
             # COOKIE PARAMETERS
@@ -1603,6 +1603,7 @@ m{^(?:ldapi://[^/]*/?|\w[\w\-\.]*(?::\d{1,5})?|ldap(?:s|\+tls)?://\w[\w\-\.]*(?:
             test    => qr/^\d*$/,
             msgFail => 'Bad number',
         },
+        trustedProxies => $testNotDefined,
         userControl => {
             test    => $pcre,
             msgFail => 'Bad regular expression',
@@ -1614,7 +1615,6 @@ m{^(?:ldapi://[^/]*/?|\w[\w\-\.]*(?::\d{1,5})?|ldap(?:s|\+tls)?://\w[\w\-\.]*(?:
         useRedirectOnError     => $boolean,
         useRedirectOnForbidden => $boolean,
         useSafeJail            => $boolean,
-        useXForwardedForIP     => $boolean,
         variables              => $testNotDefined,
         vhostOptions           => {
             keyTest    => Lemonldap::NG::Common::Regexp::HOSTNAME(),
@@ -1962,13 +1962,13 @@ sub defaultConf {
         syslog                      => '',
         timeout                     => '72000',
         timeoutActivity             => '0',
+        trustedProxies              => '',
         userControl                 => '^[\w\.\-@]+$',
         userDB                      => 'LDAP',
         passwordDB                  => 'LDAP',
         useRedirectOnError          => '1',
         useRedirectOnForbidden      => '0',
         useSafeJail                 => '1',
-        useXForwardedForIP          => '0',
         vhostPort                   => '-1',
         vhostHttps                  => '-1',
         vhostMaintenance            => '0',

lemonldap-ng-manager/lib/Lemonldap/NG/Manager/_i18n.pm

@@ -338,6 +338,7 @@ sub en {
         timeout                     => 'Sessions timeout',
         timeoutActivity             => 'Sessions activity timeout',
         trustedDomains              => 'Trusted domains',
+        trustedProxies              => 'Trusted proxies IP',
         twitterAppName              => 'Application name',
         twitterAuthnLevel           => 'Authentication level',
         twitterKey                  => 'API key',
@@ -354,7 +355,6 @@ sub en {
         useRedirectOnError          => 'Redirect on error',
         useRedirectOnForbidden      => 'Redirect on forbidden',
         useSafeJail                 => 'Use Safe jail',
-        useXForwardedForIP          => "Use X-Forwarded-For",
         variables                   => "Variables",
         vhostHttps                  => 'HTTPS',
         vhostMaintenance            => 'Maintenance mode',
@@ -793,6 +793,7 @@ sub fr {
         timeout            => 'Durée de vie maximale des sessions',
         timeoutActivity    => 'Délai d\'expiration des sessions',
         trustedDomains     => 'Domaines approuvés',
+        trustedProxies     => 'IP des proxys de confiance',
         twitterAppName     => 'Nom de l\'application',
         twitterAuthnLevel  => 'Niveau d\'authentification',
         twitterKey         => 'Clé de l\'API',
@@ -809,7 +810,6 @@ sub fr {
         useRedirectOnError     => 'Redirection pour les erreurs',
         useRedirectOnForbidden => 'Redirection pour les accès interdits',
         useSafeJail            => 'Utiliser la cage Safe',
-        useXForwardedForIP     => "Utiliser X-Forwarded-For",
         variables              => "Variables",
         vhostHttps             => 'HTTPS',
         vhostMaintenance       => 'Mode maintenance',

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/AuthSSL.pm

@@ -9,7 +9,7 @@ use strict;
 use Lemonldap::NG::Portal::Simple;
 use Lemonldap::NG::Portal::AuthNull;
 
-our $VERSION = '1.2.2';
+our $VERSION = '1.2.3';
 our @ISA     = qw(Lemonldap::NG::Portal::AuthNull);
 
 *authenticate = *extractFormInfo;
@@ -35,7 +35,7 @@ sub extractFormInfo {
     }
     else {
         $self->_sub( 'userError',
-            "No certificate found for $ENV{REMOTE_ADDR}" );
+            "No certificate found for " . $self->ipAddr );
         return PE_CERTIFICATEREQUIRED;
     }
 }

lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Simple.pm

@@ -971,7 +971,7 @@ sub getApacheSession {
 
         # Session not available (expired ?)
         if ($id) {
-            $self->lmLog( "Session $id isn't yet available ($ENV{REMOTE_ADDR})",
+            $self->lmLog( "Session $id can't be retrieved (" . $self->ipAddr . ")",
                 'info' )
               unless $noInfo;
         }
@@ -1212,7 +1212,7 @@ sub _subProcess {
 sub updateStatus {
     my $self = shift;
     print $Lemonldap::NG::Handler::Simple::statusPipe (
-        $self->{user} ? $self->{user} : $ENV{REMOTE_ADDR} )
+        $self->{user} ? $self->{user} : $self->ipAddr )
       . " => $ENV{SERVER_NAME}$ENV{SCRIPT_NAME} "
       . $self->{error} . "\n"
       if ($Lemonldap::NG::Handler::Simple::statusPipe);
@@ -2006,7 +2006,7 @@ sub modifyPassword {
 }
 
 ##@apmethod int setSessionInfo()
-# Set ipAddr, xForwardedForAddr, startTime, updateTime, _utime and _userDB
+# Set ipAddr, startTime, updateTime, _utime and _userDB
 # Call setSessionInfo() in UserDB* module
 #@return Lemonldap::NG::Portal constant
 sub setSessionInfo {
@@ -2016,15 +2016,7 @@ sub setSessionInfo {
     $self->{sessionInfo}->{_userDB} = $self->get_module("user");
 
     # Store IP address from remote address or X-FORWARDED-FOR header
-    my $xheader = $ENV{HTTP_X_FORWARDED_FOR};
-    $xheader =~ s/(.*?)(\,)+.*/$1/ if $xheader;
-
-    if ( $xheader and $self->{useXForwardedForIP} ) {
-        $self->{sessionInfo}->{ipAddr} = $xheader;
-    }
-    else {
-        $self->{sessionInfo}->{ipAddr} = $ENV{REMOTE_ADDR};
-    }
+    $self->{sessionInfo}->{ipAddr} = $self->ipAddr;
 
     # Date and time
     if ( $self->{updateSession} ) {
@@ -2236,7 +2228,7 @@ sub removeOther {
     if ( $self->{singleUserByIP} ) {
         my $sessions =
           $self->{globalStorage}->searchOn( $self->{globalStorageOptions},
-            'ipAddr', $ENV{REMOTE_ADDR} );
+            'ipAddr', $self->ipAddr );
         foreach my $id ( keys %$sessions ) {
             next if ( $current and $current eq $id );
             my $h = $self->getApacheSession( $id, 1 ) or next;

rpm/lemonldap-ng.spec

@@ -87,8 +87,9 @@ BuildRequires:  perl(Crypt::OpenSSL::X509)
 BuildRequires:  perl(Class::Inspector)
 BuildRequires:  perl(Test::MockObject)
 BuildRequires:  perl(Clone)
+BuildRequires:  perl(Net::CIDR::Lite)
 %else
-BuildRequires:  perl-Regexp-Assemble, perl-Authen-Captcha, perl-Cache-Cache, perl-Apache-Session, perl-LDAP, perl-CGI-Session, perl-IO-String, perl-MIME-Lite, perl-SOAP-Lite, perl-XML-Simple, perl-XML-LibXSLT, perl-String-Random, perl-Email-Date-Format, perl-Crypt-Rijndael, perl-HTML-Template, perl-JSON, perl-Config-IniFiles, perl-Crypt-OpenSSL-RSA, perl-Crypt-OpenSSL-X509, perl-Class-Inspector, perl-Test-MockObject, perl-Clone
+BuildRequires:  perl-Regexp-Assemble, perl-Authen-Captcha, perl-Cache-Cache, perl-Apache-Session, perl-LDAP, perl-CGI-Session, perl-IO-String, perl-MIME-Lite, perl-SOAP-Lite, perl-XML-Simple, perl-XML-LibXSLT, perl-String-Random, perl-Email-Date-Format, perl-Crypt-Rijndael, perl-HTML-Template, perl-JSON, perl-Config-IniFiles, perl-Crypt-OpenSSL-RSA, perl-Crypt-OpenSSL-X509, perl-Class-Inspector, perl-Test-MockObject, perl-Clone, perl-Net-CIDR-Lite
 %endif
 
 %if 0%{?rhl}%{?rhel}%{?fedora}