From: Wolfgang Bumiller <w.bumiller@proxmox.com>
Date: Wed, 10 Nov 2021 12:49:00 +0000 (+0100)
Subject: assert tfa/user config lock order
X-Git-Url: https://git.proxmox.com/?p=pve-access-control.git;a=commitdiff_plain;h=2ee46655a533d82cafc9afa7413e02f01e76c4ef

assert tfa/user config lock order

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 0b00847..49cef94 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -12,6 +12,7 @@ use Digest::SHA;
 use IO::File;
 use File::stat;
 use JSON;
+use Scalar::Util 'weaken';
 
 use PVE::OTP;
 use PVE::Ticket;
@@ -68,10 +69,20 @@ sub pve_verify_realm {
     PVE::Auth::Plugin::pve_verify_realm(@_);
 }
 
+# Locking both config files together is only ever allowed in one order:
+#  1) tfa config
+#  2) user config
+# If we permit the other way round, too, we might end up deadlocking!
+my $user_config_locked;
 sub lock_user_config {
     my ($code, $errmsg) = @_;
 
+    my $locked = 1;
+    $user_config_locked = \$locked;
+    weaken $user_config_locked; # make this scope guard signal safe...
+
     cfs_lock_file("user.cfg", undef, $code);
+    $user_config_locked = undef;
     if (my $err = $@) {
 	$errmsg ? die "$errmsg: $err" : die $err;
     }
@@ -80,6 +91,9 @@ sub lock_user_config {
 sub lock_tfa_config {
     my ($code, $errmsg) = @_;
 
+    die "tfa config lock cannot be acquired while holding user config lock\n"
+	if ($user_config_locked && $$user_config_locked);
+
     my $res = cfs_lock_file("priv/tfa.cfg", undef, $code);
     if (my $err = $@) {
 	$errmsg ? die "$errmsg: $err" : die $err;