]> git.proxmox.com Git - pve-access-control.git/blobdiff - src/PVE/AccessControl.pm
tfa: upgrade check: be less strict about version format
[pve-access-control.git] / src / PVE / AccessControl.pm
index 0b0084787ca7a2f4f77f215fa231628398dcecec..c5be63bc8b49867f642dcdfe5b87837dbe2be9eb 100644 (file)
@@ -7,11 +7,14 @@ use Crypt::OpenSSL::Random;
 use Crypt::OpenSSL::RSA;
 use Net::SSLeay;
 use Net::IP;
+use MIME::Base32;
 use MIME::Base64;
 use Digest::SHA;
 use IO::File;
 use File::stat;
 use JSON;
+use Scalar::Util 'weaken';
+use URI::Escape;
 
 use PVE::OTP;
 use PVE::Ticket;
@@ -68,10 +71,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 +93,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;
@@ -735,29 +751,41 @@ sub authenticate_2nd_new : prototype($$$$) {
        }
 
        my $realm_type = $realm_tfa && $realm_tfa->{type};
-       if (defined($realm_type) && $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;
+       $realm_type = 'totp' if $realm_type eq 'oath'; # we used to call it that
+       # verify realm type unless using recovery keys:
+       if (defined($realm_type)) {
+           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);
                }
-               return to_json($challenge);
-           }
 
-           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);
-               };
+               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,
+                       );
+                   };
+               }
            }
 
-           # Beside the realm configured auth we only allow recovery keys:
-           if ($otp !~ /^recovery:/) {
-               die "realm requires yubico authentication\n";
+           my $response_type;
+           if (defined($otp)) {
+               if ($otp !~ /^([^:]+):/) {
+                   die "bad otp response\n";
+               }
+               $response_type = $1;
            }
+
+           die "realm requires $realm_type authentication\n"
+               if $response_type && $response_type ne 'recovery' && $response_type ne $realm_type;
        }
 
        configure_u2f_and_wa($tfa_cfg);
@@ -1553,8 +1581,8 @@ sub parse_priv_tfa_config {
 sub write_priv_tfa_config {
     my ($filename, $cfg) = @_;
 
-    # FIXME: Only allow this if the complete cluster has been upgraded to understand the json
-    # config format.
+    assert_new_tfa_config_available();
+
     return $cfg->write();
 }
 
@@ -1737,7 +1765,31 @@ my $USER_CONTROLLED_TFA_TYPES = {
 };
 
 sub assert_new_tfa_config_available() {
-    # FIXME: Assert cluster-wide new-tfa-config support!
+    PVE::Cluster::cfs_update();
+    my $version_info = PVE::Cluster::get_node_kv('version-info');
+    die "cannot update tfa config, please make sure all cluster nodes are up to date\n"
+       if !$version_info;
+    my $members = PVE::Cluster::get_members();
+    my $old = '';
+    foreach my $node (keys $members->%*) {
+       my $info = $version_info->{$node};
+       if (!$info) {
+           $old .= "cluster node '$node' is too old, did not broadcast its version info\n";
+           next;
+       }
+       $info = from_json($info);
+       my $ver = $info->{version};
+       if ($ver !~ /^(\d+\.\d+)-(\d+)/) {
+           $old .= "cluster node '$node' provided an invalid version string: '$ver'\n";
+           next;
+       }
+       my ($maj, $rel) = ($1, $2);
+       if (!($maj > 7.0 || ($maj == 7.0 && $rel >= 15))) {
+           $old .= "cluster node '$node' is too old\n";
+           next;
+       }
+    }
+    die $old if length($old);
 }
 
 sub user_remove_tfa : prototype($) {
@@ -1750,6 +1802,78 @@ sub user_remove_tfa : prototype($) {
     cfs_write_file('priv/tfa.cfg', $tfa_cfg);
 }
 
+my sub add_old_yubico_keys : prototype($$$) {
+    my ($userid, $tfa_cfg, $keys) = @_;
+
+    my $count = 0;
+    foreach my $key (split_list($keys)) {
+       my $description = "<old userconfig key $count>";
+       ++$count;
+       $tfa_cfg->add_yubico_entry($userid, $description, $key);
+    }
+}
+
+my sub normalize_totp_secret : prototype($) {
+    my ($key) = @_;
+
+    my $binkey;
+    # See PVE::OTP::oath_verify_otp:
+    if ($key =~ /^v2-0x([0-9a-fA-F]+)$/) {
+       # v2, hex
+       $binkey = pack('H*', $1);
+    } elsif ($key =~ /^v2-([A-Z2-7=]+)$/) {
+       # v2, base32
+       $binkey = MIME::Base32::decode_rfc3548($1);
+    } elsif ($key =~ /^[A-Z2-7=]{16}$/) {
+       $binkey = MIME::Base32::decode_rfc3548($key);
+    } elsif ($key =~ /^[A-Fa-f0-9]{40}$/) {
+       $binkey = pack('H*', $key);
+    } else {
+       return undef;
+    }
+
+    return MIME::Base32::encode_rfc3548($binkey);
+}
+
+my sub add_old_totp_keys : prototype($$$$) {
+    my ($userid, $tfa_cfg, $realm_tfa, $keys) = @_;
+
+    my $issuer = 'Proxmox%20VE';
+    my $account = uri_escape("Old key for $userid");
+    my $digits = $realm_tfa->{digits} || 6;
+    my $step = $realm_tfa->{step} || 30;
+    my $uri = "otpauth://totp/$issuer:$account?digits=$digits&period=$step&algorithm=SHA1&secret=";
+
+    my $count = 0;
+    foreach my $key (split_list($keys)) {
+       $key = normalize_totp_secret($key);
+       # and just skip invalid keys:
+       next if !defined($key);
+
+       my $description = "<old userconfig key $count>";
+       ++$count;
+       eval { $tfa_cfg->add_totp_entry($userid, $description, $uri . $key) };
+       warn $@ if $@;
+    }
+}
+
+sub add_old_keys_to_realm_tfa : prototype($$$$) {
+    my ($userid, $tfa_cfg, $realm_tfa, $keys) = @_;
+
+    # if there's no realm tfa configured, we don't know what the keys mean, so we just ignore
+    # them...
+    return if !$realm_tfa;
+
+    my $type = $realm_tfa->{type};
+    if ($type eq 'oath') {
+       add_old_totp_keys($userid, $tfa_cfg, $realm_tfa, $keys);
+    } elsif ($type eq 'yubico') {
+       add_old_yubico_keys($userid, $tfa_cfg, $keys);
+    } else {
+       # invalid keys, we'll just drop them now...
+    }
+}
+
 sub user_get_tfa : prototype($$$) {
     my ($username, $realm, $new_format) = @_;
 
@@ -1772,6 +1896,14 @@ sub user_get_tfa : prototype($$$) {
        die "missing required 2nd keys\n";
     }
 
+    if ($new_format) {
+       my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
+       if (defined($keys) && $keys !~ /^x(?:!.*)$/) {
+           add_old_keys_to_realm_tfa($username, $tfa_cfg, $realm_tfa, $keys);
+       }
+       return ($tfa_cfg, $realm_tfa);
+    }
+
     # new style config starts with an 'x' and optionally contains a !<type> suffix
     if ($keys !~ /^x(?:!.*)?$/) {
        # old style config, find the type via the realm
@@ -1782,20 +1914,16 @@ sub user_get_tfa : prototype($$$) {
        });
     } else {
        my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
-       if ($new_format) {
-           return ($tfa_cfg, $realm_tfa);
-       } else {
-           my $tfa = $tfa_cfg->{users}->{$username};
-           return if !$tfa; # should not happen (user.cfg wasn't cleaned up?)
+       my $tfa = $tfa_cfg->{users}->{$username};
+       return if !$tfa; # should not happen (user.cfg wasn't cleaned up?)
 
-           if ($realm_tfa) {
-               # if the realm has a tfa setting we need to verify the type:
-               die "auth domain '$realm' and user have mismatching TFA settings\n"
-                   if $realm_tfa && $realm_tfa->{type} ne $tfa->{type};
-           }
-
-           return ($tfa->{type}, $tfa->{data});
+       if ($realm_tfa) {
+           # if the realm has a tfa setting we need to verify the type:
+           die "auth domain '$realm' and user have mismatching TFA settings\n"
+               if $realm_tfa && $realm_tfa->{type} ne $tfa->{type};
        }
+
+       return ($tfa->{type}, $tfa->{data});
     }
 }