diff --git a/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Build/Attributes.pm b/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Build/Attributes.pm index 5ff9ae575..1214183a2 100644 --- a/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Build/Attributes.pm +++ b/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Build/Attributes.pm @@ -851,27 +851,24 @@ sub attributes { documentation => 'Enable brute force attack protection', }, bruteForceProtectionTempo => { - default => 30, - type => 'int', - documentation => - 'Brute force attack protection -> Tempo before try again', + default => 30, + type => 'int', + documentation => 'Lock time', }, bruteForceProtectionMaxAge => { - default => 300, - type => 'int', - documentation => -'Brute force attack protection -> Max age between last and first allowed failed login', + default => 300, + type => 'int', + documentation => 'Max age between current and first failed login', }, bruteForceProtectionMaxFailed => { - default => 3, - type => 'int', - documentation => - 'Brute force attack protection -> Max allowed failed login', + default => 3, + type => 'int', + documentation => 'Max allowed failed login', }, bruteForceProtectionMaxLockTime => { default => 900, type => 'int', - documentation => 'Brute force attack protection -> Max lock time', + documentation => 'Max lock time', }, bruteForceProtectionIncrementalTempo => { default => 0, @@ -882,7 +879,7 @@ sub attributes { }, bruteForceProtectionLockTimes => { type => 'text', - default => '5, 15, 60, 300, 600', + default => '15, 30, 60, 300, 600', documentation => 'Incremental lock time values for brute force attack protection', }, diff --git a/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Conf/Tests.pm b/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Conf/Tests.pm index 3a580cf78..4b03bfb9d 100644 --- a/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Conf/Tests.pm +++ b/lemonldap-ng-manager/lib/Lemonldap/NG/Manager/Conf/Tests.pm @@ -689,19 +689,19 @@ sub tests { '"History" plugin is required to enable "BruteForceProtection" plugin' ) unless ( $conf->{loginHistoryEnabled} ); return ( 1, -'Number of failed logins must be higher than 2 to enable "BruteForceProtection" plugin' - ) unless ( $conf->{failedLoginNumber} > 2 ); +'Number of failed logins must be higher than 1 to enable "BruteForceProtection" plugin' + ) unless ( $conf->{failedLoginNumber} > 1 ); return ( 1, -'Number of failed logins history must be higher than allowed failed logins plus lock time values' +'Number of failed logins history must be higher or equal than allowed failed logins plus lock time values' ) if ( $conf->{bruteForceProtectionIncrementalTempo} - && $conf->{failedLoginNumber} <= + && $conf->{failedLoginNumber} < $conf->{bruteForceProtectionMaxFailed} + $conf->{bruteForceProtectionLockTimes} ); return ( 1, -'Number of failed logins history must be higher than allowed failed logins' +'Number of failed logins history must be higher or equal than allowed failed logins' ) - unless ( $conf->{failedLoginNumber} > + unless ( $conf->{failedLoginNumber} >= $conf->{bruteForceProtectionMaxFailed} ); return 1; }, diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm index 63f717fd4..95d9b9d1a 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Process.pm @@ -185,7 +185,7 @@ sub checkUnauthLogout { $self->userLogger->info('Unauthenticated logout request'); $self->logger->debug('Cleaning pdata'); $self->logger->debug("Removing $self->{conf}->{cookieName} cookie"); - $req->pdata({}); + $req->pdata( {} ); $req->addCookie( $self->cookie( name => $self->conf->{cookieName}, @@ -449,12 +449,13 @@ sub setGroups { } sub setPersistentSessionInfo { - my ( $self, $req ) = @_; + + # $user passed by BruteForceProtection plugin + my ( $self, $req, $user ) = @_; # Do not restore infos if session already opened unless ( $req->id ) { - my $key = $req->{sessionInfo}->{ $self->conf->{whatToTrace} }; - + my $key = $req->{sessionInfo}->{ $self->conf->{whatToTrace} } || $user; return PE_OK unless ( $key and length($key) ); my $persistentSession = $self->getPersistentSession($key); @@ -593,9 +594,9 @@ sub secondFactor { } sub storeHistory { - my ( $self, $req ) = @_; + my ( $self, $req, $uid ) = @_; # $uid passed by BruteForceProtection plugin if ( $self->conf->{loginHistoryEnabled} ) { - $self->registerLogin($req); + $self->registerLogin( $req, $uid ); } PE_OK; } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm index 3689eeba9..5474826a0 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm @@ -1037,7 +1037,9 @@ sub tplParams { } sub registerLogin { - my ( $self, $req ) = @_; + + # $user passed by BruteForceProtection plugin + my ( $self, $req, $uid ) = @_; return unless ( $self->conf->{loginHistoryEnabled} and defined $req->authResult ); @@ -1067,7 +1069,8 @@ sub registerLogin { } } } - $self->updatePersistentSession( $req, { 'loginHistory' => undef } ); + $self->updatePersistentSession( $req, { 'loginHistory' => undef }, + $uid ); delete $req->sessionInfo->{loginHistory}; } @@ -1092,7 +1095,7 @@ sub registerLogin { if ( scalar @{ $history->{$type} } > $self->conf->{ $type . "Number" } ); # Save into persistent session - $self->updatePersistentSession( $req, { _loginHistory => $history, } ); + $self->updatePersistentSession( $req, { _loginHistory => $history }, $uid ); PE_OK; } diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/BruteForceProtection.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/BruteForceProtection.pm index 5ee3988c0..2fcaf9648 100644 --- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/BruteForceProtection.pm +++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Plugins/BruteForceProtection.pm @@ -12,7 +12,7 @@ our $VERSION = '2.0.10'; extends 'Lemonldap::NG::Portal::Main::Plugin'; # INITIALIZATION -use constant afterSub => { storeHistory => 'run' }; +use constant aroundSub => { authenticate => 'check' }; has lockTimes => ( is => 'rw', @@ -25,8 +25,15 @@ has maxAge => ( isa => 'Int' ); +has maxFailed => ( + is => 'rw', + isa => 'Int' +); + sub init { my ($self) = @_; + $self->maxFailed( abs $self->conf->{bruteForceProtectionMaxFailed} ); + if ( $self->conf->{disablePersistentStorage} ) { $self->logger->error( '"BruteForceProtection" plugin enabled WITHOUT persistent session storage"' @@ -40,13 +47,11 @@ sub init { return 0; } - unless ( $self->conf->{failedLoginNumber} > - $self->conf->{bruteForceProtectionMaxFailed} ) - { + unless ( $self->conf->{failedLoginNumber} > $self->maxFailed ) { $self->logger->error( 'Number of failed logins history (' . $self->conf->{failedLoginNumber} . ') must be higher than allowed failed logins attempt (' - . $self->conf->{bruteForceProtectionMaxFailed} + . $self->maxFailed . ')' ); return 0; } @@ -62,21 +67,16 @@ sub init { split /\s*,\s*/, $self->conf->{bruteForceProtectionLockTimes}; unless ($lockTimes) { - @{ $self->lockTimes } = ( 5, 15, 60, 300, 600 ); + @{ $self->lockTimes } = ( 15, 30, 60, 300, 600 ); $lockTimes = 5; } - for ( - my $i = 1 ; - $i <= $self->conf->{bruteForceProtectionMaxFailed} ; - $i++ - ) - { + for ( my $i = 1 ; $i < $self->maxFailed ; $i++ ) { unshift @{ $self->lockTimes }, 0; $lockTimes++; } - unless ( $lockTimes < $self->conf->{failedLoginNumber} ) { + unless ( $lockTimes <= $self->conf->{failedLoginNumber} ) { $self->logger->warn( 'Number failed logins history (' . $self->conf->{failedLoginNumber} . ') must be higher than incremental lock time values plus allowed failed logins attempt (' @@ -91,76 +91,83 @@ sub init { $self->maxAge($sum); } else { - $self->maxAge( $self->conf->{bruteForceProtectionMaxAge} ); + $self->maxAge( $self->conf->{bruteForceProtectionMaxAge} * + ( 1 + $self->maxFailed ) ); } - + return 1; } # RUNNING METHOD -sub run { - my ( $self, $req ) = @_; - my $now = time; +sub check { + my ( $self, $sub, $req ) = @_; + my $now = time; + $self->p->setSessionInfo($req); + $self->logger->debug("Retrieve $req->{user} logins history"); + $self->p->setPersistentSessionInfo( $req, $req->{user} ); + my $countFailed = my @failedLogins = - map { ( $now - $_->{_utime} ) < $self->maxAge ? $_ : () } + map { ( $now - $_->{_utime} ) <= $self->maxAge ? $_ : () } @{ $req->sessionInfo->{_loginHistory}->{failedLogin} }; - $self->logger->debug( ' Failed login maxAge = ' . $self->maxAge ); + $self->logger->debug( ' -> Failed login maxAge = ' . $self->maxAge ); $self->logger->debug( - " Number of failed login(s) to take into account = $countFailed"); + "Number of failed login(s) to take into account = $countFailed"); + my $lastFailedLoginEpoch = $failedLogins[0]->{_utime} || undef; if ( $self->conf->{bruteForceProtectionIncrementalTempo} ) { - my $lastFailedLoginEpoch = $failedLogins[0]->{_utime} || undef; - - return PE_OK unless $lastFailedLoginEpoch; + return $sub->($req) unless $lastFailedLoginEpoch; + # Delta between current attempt and last failed login my $delta = $now - $lastFailedLoginEpoch; $self->logger->debug(" -> Delta = $delta"); + + # Time to wait my $waitingTime = $self->lockTimes->[ $countFailed - 1 ] // $self->conf->{bruteForceProtectionMaxLockTime}; + + # Reach last tempo. Stop to increase waiting time + if ( $countFailed >= scalar @{ $self->lockTimes } ) { + $self->userLogger->warn( + "BruteForceProtection: Last lock time has been reached"); + $self->logger->debug("Force waitingTime to last value"); + $waitingTime = + $self->lockTimes->[ scalar @{ $self->lockTimes } - 1 ]; + } $self->logger->debug(" -> Waiting time = $waitingTime"); - if ( $waitingTime && $delta <= $waitingTime ) { - $self->logger->debug("BruteForceProtection enabled"); - $req->lockTime($waitingTime); + + # Delta < waitingTime => wait + if ( $waitingTime && $delta < $waitingTime ) { + $self->userLogger->warn("BruteForceProtection enabled"); + $req->authResult(PE_WAIT); + + # Do not store failed login if last tempo or max tempo is reached + $self->p->registerLogin( $req, $req->{user} ) + if ( $waitingTime < $self->conf->{bruteForceProtectionMaxLockTime} + && $waitingTime < + $self->lockTimes->[ scalar @{ $self->lockTimes } - 1 ] ); + $req->lockTime( $waitingTime - $delta ); return PE_WAIT; } - return PE_OK; + return $sub->($req); } - return PE_OK - if ( $countFailed <= $self->conf->{bruteForceProtectionMaxFailed} ); + return $sub->($req) + if ( $countFailed < $self->maxFailed ); - my @lastFailedLoginEpoch = (); - my $MaxAge = $self->maxAge + 1; - - # Auth_N-2 failed login epoch - foreach ( 0 .. $self->conf->{bruteForceProtectionMaxFailed} - 1 ) { - push @lastFailedLoginEpoch, - $req->sessionInfo->{_loginHistory}->{failedLogin}->[$_]->{_utime} - if ( $req->sessionInfo->{_loginHistory}->{failedLogin}->[$_] ); - } - - # If Auth_N-MaxFailed older than MaxAge -> another try allowed - $MaxAge = - $lastFailedLoginEpoch[0] - - $lastFailedLoginEpoch[ $self->conf->{bruteForceProtectionMaxFailed} - 1 ] - if $self->conf->{bruteForceProtectionMaxFailed}; - $self->logger->debug(" -> MaxAge = $MaxAge"); - - return PE_OK - if ( $MaxAge > $self->maxAge ); - - # Delta between the two last failed logins -> Auth_N - Auth_N-1 - my $delta = - defined $lastFailedLoginEpoch[1] ? $now - $lastFailedLoginEpoch[1] : 0; + # Delta between current attempt and last failed login + my $delta = $lastFailedLoginEpoch ? $now - $lastFailedLoginEpoch : 0; $self->logger->debug(" -> Delta = $delta"); - # Delta between the two last failed logins < Tempo => wait - return PE_OK - unless ( $delta <= $self->conf->{bruteForceProtectionTempo} ); + # Delta < Tempo => wait + return $sub->($req) + unless ( $delta < $self->conf->{bruteForceProtectionTempo} + && $countFailed ); # Account locked - $self->logger->debug("BruteForceProtection enabled"); - $req->lockTime( $self->conf->{bruteForceProtectionTempo} ); + $self->userLogger->warn("BruteForceProtection enabled"); + $self->logger->debug( + " -> Waiting time = $self->{conf}->{bruteForceProtectionTempo}"); + $req->lockTime( $self->conf->{bruteForceProtectionTempo} - $delta ); return PE_WAIT; } diff --git a/lemonldap-ng-portal/t/61-BruteForceProtection-with-Incremental-lockTimes.t b/lemonldap-ng-portal/t/61-BruteForceProtection-with-Incremental-lockTimes.t index a3d7a3b36..f5dac9f0c 100644 --- a/lemonldap-ng-portal/t/61-BruteForceProtection-with-Incremental-lockTimes.t +++ b/lemonldap-ng-portal/t/61-BruteForceProtection-with-Incremental-lockTimes.t @@ -10,7 +10,7 @@ my $res; my $client = LLNG::Manager::Test->new( { ini => { - logLevel => 'error', + logLevel => 'debug', authentication => 'Demo', userDB => 'Same', loginHistoryEnabled => 1, @@ -118,11 +118,32 @@ ok( ), '2nd Bad Auth query' ); +ok( $res->[2]->[0] =~ /<\/span>/, + 'Rejected -> Protection enabled' ) + or print STDERR Dumper( $res->[2]->[0] ); +ok( $res->[2]->[0] =~ m%5 seconds%, + 'LockTime = 5' ) + or print STDERR Dumper( $res->[2]->[0] ); +count(3); + +# Waiting +Time::Fake->offset("+6s"); + +## Third failed connection +ok( + $res = $client->_post( + '/', + IO::String->new('user=dwho&password=ohwd'), + length => 23, + accept => 'text/html', + ), + '3rd Bad Auth query' +); ok( $res->[2]->[0] =~ /<\/span>/, 'Rejected -> Protection enabled' ) or print STDERR Dumper( $res->[2]->[0] ); ok( $res->[2]->[0] =~ m%10 seconds%, - 'LockTime = 10' ) + 'LockTime = 10**************' ) or print STDERR Dumper( $res->[2]->[0] ); count(3); diff --git a/lemonldap-ng-portal/t/61-BruteForceProtection.t b/lemonldap-ng-portal/t/61-BruteForceProtection.t index df2ee8441..baeffa886 100644 --- a/lemonldap-ng-portal/t/61-BruteForceProtection.t +++ b/lemonldap-ng-portal/t/61-BruteForceProtection.t @@ -224,14 +224,20 @@ $id1 = expectCookie($res); ok( $res->[2]->[0] =~ /trspan="lastLogins"/, 'History found' ) or print STDERR Dumper( $res->[2]->[0] ); +ok( $res->[2]->[0] =~ //, 'History found' ) + or print STDERR Dumper( $res->[2]->[0] ); +ok( $res->[2]->[0] =~ //, 'History found' ) + or print STDERR Dumper( $res->[2]->[0] ); my @c = ( $res->[2]->[0] =~ /127.0.0.1/gs ); my @cf = ( $res->[2]->[0] =~ /PE5<\/td>/gs ); -# History with 10 entries -ok( @c == 10, ' -> Ten entries found' ); -ok( @cf == 6, " -> Six 'failedLogin' entries found" ); -count(3); +# History with 8 entries +ok( @c == 8, ' -> Eight entries found' ) + or print STDERR Dumper( $res->[2]->[0] ); +ok( @cf == 4, " -> Four 'failedLogin' entries found" ) + or print STDERR Dumper( $res->[2]->[0] ); +count(5); $client->logout($id1); clean_sessions();