]> git.proxmox.com Git - pve-access-control.git/blobdiff - src/PVE/AccessControl.pm
authenticate_2nd_new: rename $otp to $tfa_response
[pve-access-control.git] / src / PVE / AccessControl.pm
index a2e0458c30cbb6ad41e72e18208e0b2d1c29569c..3d77bed0650a682c2f3eb2cb822695eba1c7fee5 100644 (file)
@@ -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);