X-Git-Url: https://git.proxmox.com/?a=blobdiff_plain;f=src%2FPVE%2FAccessControl.pm;h=3d77bed0650a682c2f3eb2cb822695eba1c7fee5;hb=61565fb2c543092658f4ab449f8cb35688a770ff;hp=a2e0458c30cbb6ad41e72e18208e0b2d1c29569c;hpb=7d23b7cac8c82d85138422b036a961ebee50eb30;p=pve-access-control.git

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index a2e0458..3d77bed 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -203,7 +203,16 @@ sub rotate_authkey {
     return if $authkey_lifetime == 0;
 
     PVE::Cluster::cfs_lock_authkey(undef, sub {
-	# re-check with lock to avoid double rotation in clusters
+	# stat() calls might be answered from the kernel page cache for up to
+	# 1s, so this special dance is needed to avoid a double rotation in
+	# clusters *despite* the cfs_lock context..
+
+	# drop in-process cache hash
+	$pve_auth_key_cache = {};
+	# force open/close of file to invalidate page cache entry
+	get_pubkey();
+	# now re-check with lock held and page cache invalidated so that stat()
+	# does the right thing, and any key updates by other nodes are visible.
 	return if check_authkey();
 
 	my $old = get_pubkey();
@@ -498,6 +507,8 @@ my $assemble_short_lived_ticket = sub {
 
     $path = normalize_path($path);
 
+    die "invalid ticket path\n" if !defined($path);
+
     my $secret_data = "$username:$path";
 
     return PVE::Ticket::assemble_rsa_ticket(
@@ -509,6 +520,8 @@ my $verify_short_lived_ticket = sub {
 
     $path = normalize_path($path);
 
+    die "invalid ticket path\n" if !defined($path);
+
     my $secret_data = "$username:$path";
 
     my ($rsa_pub, $rsa_mtime) = get_pubkey();
@@ -773,79 +786,90 @@ sub authenticate_2nd_old : prototype($$$) {
     return wantarray ? ($username, $tfa_data) : $username;
 }
 
-# Returns a tfa challenge or undef.
-sub authenticate_2nd_new : prototype($$$$) {
-    my ($username, $realm, $otp, $tfa_challenge) = @_;
-
-    my $result = lock_tfa_config(sub {
-	my ($tfa_cfg, $realm_tfa) = user_get_tfa($username, $realm, 1);
+sub authenticate_2nd_new_do : prototype($$$$) {
+    my ($username, $realm, $tfa_response, $tfa_challenge) = @_;
+    my ($tfa_cfg, $realm_tfa) = user_get_tfa($username, $realm, 1);
 
-	if (!defined($tfa_cfg)) {
-	    return undef;
-	}
-
-	my $realm_type = $realm_tfa && $realm_tfa->{type};
-	# verify realm type unless using recovery keys:
-	if (defined($realm_type)) {
-	    $realm_type = 'totp' if $realm_type eq 'oath'; # we used to call it that
-	    if ($realm_type eq 'yubico') {
-		# Yubico auth will not be supported in rust for now...
-		if (!defined($tfa_challenge)) {
-		    my $challenge = { yubico => JSON::true };
-		    # Even with yubico auth we do allow recovery keys to be used:
-		    if (my $recovery = $tfa_cfg->recovery_state($username)) {
-			$challenge->{recovery} = $recovery;
-		    }
-		    return to_json($challenge);
-		}
+    if (!defined($tfa_cfg)) {
+	return undef;
+    }
 
-		if ($otp =~ /^yubico:(.*)$/) {
-		    $otp = $1;
-		    # Defer to after unlocking the TFA config:
-		    return sub {
-			authenticate_yubico_new(
-			    $tfa_cfg, $username, $realm_tfa, $tfa_challenge, $otp,
-			);
-		    };
+    my $realm_type = $realm_tfa && $realm_tfa->{type};
+    # verify realm type unless using recovery keys:
+    if (defined($realm_type)) {
+	$realm_type = 'totp' if $realm_type eq 'oath'; # we used to call it that
+	if ($realm_type eq 'yubico') {
+	    # Yubico auth will not be supported in rust for now...
+	    if (!defined($tfa_challenge)) {
+		my $challenge = { yubico => JSON::true };
+		# Even with yubico auth we do allow recovery keys to be used:
+		if (my $recovery = $tfa_cfg->recovery_state($username)) {
+		    $challenge->{recovery} = $recovery;
 		}
+		return to_json($challenge);
 	    }
 
-	    my $response_type;
-	    if (defined($otp)) {
-		if ($otp !~ /^([^:]+):/) {
-		    die "bad otp response\n";
-		}
-		$response_type = $1;
+	    if ($tfa_response =~ /^yubico:(.*)$/) {
+		$tfa_response = $1;
+		# Defer to after unlocking the TFA config:
+		return sub {
+		    authenticate_yubico_new(
+			$tfa_cfg, $username, $realm_tfa, $tfa_challenge, $tfa_response,
+		    );
+		};
 	    }
+	}
 
-	    die "realm requires $realm_type authentication\n"
-		if $response_type && $response_type ne 'recovery' && $response_type ne $realm_type;
+	my $response_type;
+	if (defined($tfa_response)) {
+	    if ($tfa_response !~ /^([^:]+):/) {
+		die "bad otp response\n";
+	    }
+	    $response_type = $1;
 	}
 
-	configure_u2f_and_wa($tfa_cfg);
+	die "realm requires $realm_type authentication\n"
+	    if $response_type && $response_type ne 'recovery' && $response_type ne $realm_type;
+    }
 
-	my $must_save = 0;
-	if (defined($tfa_challenge)) {
-	    $tfa_challenge = verify_ticket($tfa_challenge, 0, $username);
-	    $must_save = $tfa_cfg->authentication_verify($username, $tfa_challenge, $otp);
-	    $tfa_challenge = undef;
-	} else {
-	    $tfa_challenge = $tfa_cfg->authentication_challenge($username);
-	    if (defined($otp)) {
-		if (defined($tfa_challenge)) {
-		    $must_save = $tfa_cfg->authentication_verify($username, $tfa_challenge, $otp);
-		} else {
-		    die "no such challenge\n";
-		}
+    configure_u2f_and_wa($tfa_cfg);
+
+    my $must_save = 0;
+    if (defined($tfa_challenge)) {
+	$tfa_challenge = verify_ticket($tfa_challenge, 0, $username);
+	$must_save = $tfa_cfg->authentication_verify($username, $tfa_challenge, $tfa_response);
+	$tfa_challenge = undef;
+    } else {
+	$tfa_challenge = $tfa_cfg->authentication_challenge($username);
+	if (defined($tfa_response)) {
+	    if (defined($tfa_challenge)) {
+		$must_save = $tfa_cfg->authentication_verify($username, $tfa_challenge, $tfa_response);
+	    } else {
+		die "no such challenge\n";
 	    }
 	}
+    }
 
-	if ($must_save) {
-	    cfs_write_file('priv/tfa.cfg', $tfa_cfg);
-	}
+    if ($must_save) {
+	cfs_write_file('priv/tfa.cfg', $tfa_cfg);
+    }
 
-	return $tfa_challenge;
-    });
+    return $tfa_challenge;
+}
+
+# Returns a tfa challenge or undef.
+sub authenticate_2nd_new : prototype($$$$) {
+    my ($username, $realm, $tfa_response, $tfa_challenge) = @_;
+
+    my $result;
+
+    if (defined($tfa_response) && $tfa_response =~ m/^recovery:/) {
+	$result = lock_tfa_config(sub {
+	    authenticate_2nd_new_do($username, $realm, $tfa_response, $tfa_challenge);
+	});
+    } else {
+	$result = authenticate_2nd_new_do($username, $realm, $tfa_response, $tfa_challenge);
+    }
 
     # Yubico auth returns the authentication sub:
     if (ref($result) eq 'CODE') {
@@ -908,13 +932,8 @@ sub configure_u2f_and_wa : prototype($) {
 	warn "u2f unavailable, configuration error: $@\n" if $@;
     }
     if (my $wa = $dc->{webauthn}) {
-	eval {
-	    $tfa_cfg->set_webauthn_config({
-		origin => $wa->{origin} // $get_origin->(),
-		rp => $wa->{rp},
-		id => $wa->{id},
-	    });
-	};
+	$wa->{origin} //= $get_origin->();
+	eval { $tfa_cfg->set_webauthn_config({%$wa}) };
 	warn "webauthn unavailable, configuration error: $@\n" if $@;
     }
 }
@@ -1166,6 +1185,8 @@ sub lookup_username {
 sub normalize_path {
     my $path = shift;
 
+    return undef if !$path;
+
     $path =~ s|/+|/|g;
 
     $path =~ s|/$||;
@@ -1641,6 +1662,12 @@ sub roles {
 
     return 'Administrator' if $user eq 'root@pam'; # root can do anything
 
+    if (!defined($path)) {
+	# this shouldn't happen!
+	warn "internal error: ACL check called for undefined ACL path!\n";
+	return {};
+    }
+
     if (pve_verify_tokenid($user, 1)) {
 	my $tokenid = $user;
 	my ($username, $token) = split_tokenid($tokenid);