[pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Sep 15 09:01:56 CEST 2022
I think this is https://bugzilla.proxmox.com/show_bug.cgi?id=3739 ;)
@wolfgang could you take a look at this?
On September 14, 2022 3:42 pm, Dominik Csapak wrote:
> 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
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list