[pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured

[Dominik Csapak]

With change to our new tfa mechanism, we now lock the tfa config
when verifying the second factor and when creating the challenge for it
that makes sense, since at least one tfa type can change the config
(recovery keys must be deleted from there).

The downside is that we cannot authenticate users anymore without quorum
(since locking requires write access to pmxcfs), even for users without
tfa configured (and also for clusters without any tfa configured at all).

With this patch, we loosen that restriction a bit, by checking if the
user has tfa configured outside the lock. We still query it again
inside the lock to get the current config inside the lock again.
(slightly out of the diff context)

There is a minimal (IMHO unimportant) race here:
if a user is logging in and for this user tfa is configured
simultaneously, it may happen that during the first check tfa is still
not present.

This is not a real problem though, since a logged in user will not be
logged out when a tfa is configured, so it's the same as when the user
would have logged in before the tfa is being configured.

There were quite a bit confused users that ran into that, and preventing
them from logging in at all because of a not quorate server (when
there is not a good technical reason for it) seems bad

It's still not possible to login when tfa is enabled though, but at
least for simple setups it should be a bit better

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
 src/PVE/AccessControl.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index c32dcc3..52ad84b 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -790,6 +790,12 @@ sub authenticate_2nd_old : prototype($$$) {
 sub authenticate_2nd_new : prototype($$$$) {
     my ($username, $realm, $otp, $tfa_challenge) = @_;
 
+    my ($tfa_cfg) = user_get_tfa($username, $realm, 1);
+
+    if (!defined($tfa_cfg)) {
+	return undef;
+    }
+
     my $result = lock_tfa_config(sub {
 	my ($tfa_cfg, $realm_tfa) = user_get_tfa($username, $realm, 1);
 
-- 
2.30.2