From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; Wed, 14 Sep 2022 15:42:36 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
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 <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=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 <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