From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: w.bumiller@proxmox.com
Subject: Re: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
Date: Thu, 15 Sep 2022 09:01:56 +0200 [thread overview]
Message-ID: <1663225247.uuvq80f077.astroid@nora.none> (raw)
In-Reply-To: <20220914134235.3707811-1-d.csapak@proxmox.com>
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@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@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
next prev parent reply other threads:[~2022-09-15 7:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-14 13:42 Dominik Csapak
2022-09-15 7:01 ` Fabian Grünbichler [this message]
2022-09-15 10:43 ` Thomas Lamprecht
2022-09-15 12:40 ` Dominik Csapak
2022-09-15 12:53 ` Thomas Lamprecht
2022-09-15 13:12 ` Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1663225247.uuvq80f077.astroid@nora.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=w.bumiller@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox