[security:low] removal of registrable 2F does not test the current authn level
Environment
LemonLDAP::NG version: 2.0.9
Summary
Given the following conditions
- A registrable (UBK,U2F,TOTP) 2F is used
- Self-registration is enabled on first login (sfRequired)
- Authentication levels are used to filter access to certain applications depending on the SF in use
Then an attacker can:
- Login with a "low-security" method, (for example: using stolen credentials)
- Remove the existing second factor (in portal 2FA menu)
- Login again, and be prompted to register a new device they control
- Access high-security applications
This problem is especially noticeable when using the newly introduced sfOnlyUpgrade
feature is used, because the 2F will not be asked if the user directly logs into the portal. But it can also happen when using a choice menu, or 2F conditions in general, or when multiple 2F are offered with different authnLevels.
Possible fixes
Before allowing a 2F to be removed, we must check that the user asking for removal has an authnLevel equals or greater than the authnLevel granted by the removed 2F.
The current state of the code does not make this very easy. All 2F modules handle removal themselves entirely. Perhaps we should wrap a test around it.
We should also maybe refactor registrable 2F plugins to have a better API, right now they look like this
sub run {
if ($action eq "add") {
}
elsif ($action eq "remove") {
}
etc.
}
they should probably look like this instead:
sub add {
}
sub remove {
}
Security impact
Probably quite low, it takes a very complicated setup to be affected before 2.0.9, and sfOnlyUpgrade is marked as beta in the doc