[security:low] afterData plugins (grantSession) cannot prevent session establishment when 2FA is in use
Concerned version
Version: %2.0
Summary
- Configure a 2F factor (Ext or Mail)
- Configure the following grant session rule:
$uid ne "dwho"
- Login as dwho
- Fill second factor
- The grant session error is displayed
- Click "OK" or wait for timeout
- You are logged in
Cause
Consider the following code in ::Portal::Main::SecondFactor
# Else restore session
[ ... ]
$self->p->rebuildCookies($req);
$req->mustRedirect(1);
$self->userLogger->notice( $self->prefix
. '2F verification for '
. $req->sessionInfo->{ $self->conf->{whatToTrace} } );
[ ... ]
$req->authResult(PE_SENDRESPONSE);
return $self->p->do(
$req,
[
@{ $self->p->afterData },
$self->p->validSession,
@{ $self->p->endAuth },
sub { PE_OK }
]
);
afterData
is called after the cookies are set in the request object, which means that whatever happens in afterData, a SSO cookie (built when the first factor succeeded) is going to be sent to the user.
This prevent any plugins that apply restrictions (only grantSession for now) from working in the context of 2FA
Possible fixes
Delaying the setting of cookies to after afterData
seems to work:
diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm
index 55a29caf7..7cc51ecb8 100644
--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm
+++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/SecondFactor.pm
@@ -118,7 +118,6 @@ sub _verify {
$req->id( delete $req->sessionInfo->{_2fRealSession} );
$req->urldc( delete $req->sessionInfo->{_2fUrldc} );
$req->{sessionInfo}->{_utime} = delete $req->{sessionInfo}->{_2fUtime};
- $self->p->rebuildCookies($req);
$req->mustRedirect(1);
$self->userLogger->notice( $self->prefix
. '2F verification for '
@@ -133,6 +132,7 @@ sub _verify {
[
@{ $self->p->afterData },
$self->p->validSession,
+ 'rebuildCookies',
@{ $self->p->endAuth },
sub { PE_OK }
]
But I'll leave the last word to @guimard
Score
Scored "low" because in most environments, accounts who might get filtered by GrantSession typically wouldn't be assigned a second factor in the first place