From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 11FEF92C8C for ; Wed, 14 Sep 2022 15:42:40 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 047082CE28 for ; Wed, 14 Sep 2022 15:42:40 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 14 Sep 2022 15:42:36 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 5C52344130 for ; Wed, 14 Sep 2022 15:42:36 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Wed, 14 Sep 2022 15:42:35 +0200 Message-Id: <20220914134235.3707811-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.088 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [accesscontrol.pm] Subject: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Sep 2022 13:42:40 -0000 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 --- 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