[pve-devel] [RFC access-control 1/2] fix #2079: add periodic auth key rotation
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Mar 6 08:21:39 CET 2019
and modify checks to accept still valid tickets generated using the
previous auth key.
the slightly complicated caching mechanism is needed for reading the key and
its modification timestamp in one go while only reading and parsing it again if
it has changed.
the +- 300 seconds fuzzing is kept for slightly out-of-sync clusters, since the
time encoded in the tickets is the result of time() on whichever node the
ticket API call got forwarded to.
Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
Notes:
VERSIONED DEPENDENCY: on pve-cluster with cfs_lock_authkey needed
it would also be possible to extend pve-common's Ticket.pm to support a list of
keys + age ranges, but that makes the interface rather non-straight-forward..
if there are no objections to this approach in general, extending it for VNC
tickets should be straight-forward - although they are only valid for a minute
anyway ;)
a similar approach could also work for CSRF tokens if desired..
the auth key lifetime can of course also be set to a bigger value (e.g., 24h)
PVE/AccessControl.pm | 211 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 191 insertions(+), 20 deletions(-)
diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
index bdadfd2..ff6413c 100644
--- a/PVE/AccessControl.pm
+++ b/PVE/AccessControl.pm
@@ -9,6 +9,8 @@ use Net::SSLeay;
use Net::IP;
use MIME::Base64;
use Digest::SHA;
+use IO::File;
+use File::stat;
use PVE::OTP;
use PVE::Ticket;
@@ -33,11 +35,19 @@ PVE::Auth::Plugin->init();
# $authdir must be writable by root only!
my $confdir = "/etc/pve";
my $authdir = "$confdir/priv";
-my $authprivkeyfn = "$authdir/authkey.key";
-my $authpubkeyfn = "$confdir/authkey.pub";
+
my $pve_www_key_fn = "$confdir/pve-www.key";
+my $pve_auth_key_files = {
+ priv => "$authdir/authkey.key",
+ pub => "$confdir/authkey.pub",
+ pubold => "$confdir/authkey.pub.old",
+};
+
+my $pve_auth_key_cache = {};
+
my $ticket_lifetime = 3600*2; # 2 hours
+my $authkey_lifetime = 3600*3; # 3 hours
Crypt::OpenSSL::RSA->import_random_seed();
@@ -62,16 +72,131 @@ sub lock_user_config {
}
}
-my $pve_auth_pub_key;
+my $cache_read_key = sub {
+ my ($type) = @_;
+
+ my $path = $pve_auth_key_files->{$type};
+
+ my $read_key_and_mtime = sub {
+ my $fh = IO::File->new($path, "r");
+
+ return undef if !defined($fh);
+
+ my $st = stat($fh);
+ my $pem = PVE::Tools::safe_read_from($fh, 0, 0, $path);
+
+ close $fh;
+
+ my $key;
+ if ($type eq 'pub' || $type eq 'pubold') {
+ $key = eval { Crypt::OpenSSL::RSA->new_public_key($pem); };
+ } elsif ($type eq 'priv') {
+ $key = eval { Crypt::OpenSSL::RSA->new_private_key($pem); };
+ } else {
+ die "Invalid authkey type '$type'\n";
+ }
+
+ return { key => $key, mtime => $st->mtime };
+ };
+
+ if (!defined($pve_auth_key_cache->{$type})) {
+ $pve_auth_key_cache->{$type} = $read_key_and_mtime->();
+ } else {
+ my $st = stat($path);
+ if (!$st || $st->mtime != $pve_auth_key_cache->{$type}->{mtime}) {
+ $pve_auth_key_cache->{$type} = $read_key_and_mtime->();
+ }
+ }
+
+ return $pve_auth_key_cache->{$type};
+};
+
sub get_pubkey {
+ my ($old) = @_;
+
+ my $type = $old ? 'pubold' : 'pub';
+
+ my $res = $cache_read_key->($type);
+ return undef if !defined($res);
+
+ return wantarray ? ($res->{key}, $res->{mtime}) : $res->{key};
+}
+
+sub get_privkey {
+ my $res = $cache_read_key->('priv');
+
+ if (!defined($res) || !check_authkey(1)) {
+ rotate_authkey();
+ $res = $cache_read_key->('priv');
+ }
+
+ return wantarray ? ($res->{key}, $res->{mtime}) : $res->{key};
+}
+
+sub check_authkey {
+ my ($quiet) = @_;
+
+ my ($pub_key, $mtime) = get_pubkey();
+ if (!$pub_key) {
+ warn('auth key pair missing, generating new one..') if !$quiet;
+ return 0;
+ } else {
+ if (time() - $mtime >= $authkey_lifetime) {
+ warn('auth key pair too old, rotating..') if !$quiet;;
+ return 0;
+ } else {
+ warn('auth key new enough, skipping rotation') if !$quiet;;
+ return 1;
+ }
+ }
+}
- return $pve_auth_pub_key if $pve_auth_pub_key;
+sub rotate_authkey {
+ my $res = PVE::Cluster::cfs_lock_authkey(undef, sub {
+ # re-check with lock to avoid double rotation in clusters
+ return if check_authkey();
- my $input = PVE::Tools::file_get_contents($authpubkeyfn);
+ my $old = get_pubkey();
- $pve_auth_pub_key = Crypt::OpenSSL::RSA->new_public_key($input);
+ if ($old) {
+ eval {
+ my $pem = $old->get_public_key_x509_string();
+ PVE::Tools::file_set_contents($pve_auth_key_files->{pubold}, $pem);
+ };
+ die "Failed to store old auth key: $@\n" if $@;
+ }
+
+ my $new = Crypt::OpenSSL::RSA->generate_key(2048);
+ eval {
+ my $pem = $new->get_public_key_x509_string();
+ PVE::Tools::file_set_contents($pve_auth_key_files->{pub}, $pem);
+ };
+ if ($@) {
+ if ($old) {
+ warn "Failed to store new auth key - $@\n";
+ warn "Reverting to previous auth key\n";
+ eval {
+ my $pem = $old->get_public_key_x509_string();
+ PVE::Tools::file_set_contents($pve_auth_key_files->{pub}, $pem);
+ };
+ die "Failed to restore old auth key: $@\n" if $@;
+ } else {
+ die "Failed to store new auth key - $@\n";
+ }
+ }
- return $pve_auth_pub_key;
+ eval {
+ my $pem = $new->get_private_key_string();
+ PVE::Tools::file_set_contents($pve_auth_key_files->{priv}, $pem);
+ };
+ if ($@) {
+ warn "Failed to store new auth key - $@\n";
+ warn "Deleting auth key to force regeneration\n";
+ unlink $pve_auth_key_files->{pub};
+ unlink $pve_auth_key_files->{priv};
+ }
+ });
+ die $@ if (!defined($res) && $@);
}
my $csrf_prevention_secret;
@@ -100,17 +225,33 @@ sub verify_csrf_prevention_token {
$secret, $username, $token, -300, $ticket_lifetime, $noerr);
}
-my $pve_auth_priv_key;
-sub get_privkey {
+my $get_ticket_age_range = sub {
+ my ($now, $mtime, $rotated) = @_;
- return $pve_auth_priv_key if $pve_auth_priv_key;
+ my $key_age = $now - $mtime;
+ $key_age = 0 if $key_age < 0;
- my $input = PVE::Tools::file_get_contents($authprivkeyfn);
+ my ($min, $max);
- $pve_auth_priv_key = Crypt::OpenSSL::RSA->new_private_key($input);
+ if ($rotated) {
+ # ticket creation after rotation is not allowed
+ $min = $key_age - 300;
+ } else {
+ if ($key_age > $authkey_lifetime) {
+ # key should have been rotated, clamp range accordingly
+ $min = $key_age - $authkey_lifetime;
+ } else {
+ $min = -300;
+ }
+ }
- return $pve_auth_priv_key;
-}
+ return undef if $min > $ticket_lifetime;
+
+ $max = $ticket_lifetime;
+ $max = $key_age + 300 if (!$rotated && $key_age < $ticket_lifetime);
+
+ return ($min, $max);
+};
sub assemble_ticket {
my ($username) = @_;
@@ -123,12 +264,34 @@ sub assemble_ticket {
sub verify_ticket {
my ($ticket, $noerr) = @_;
- my $rsa_pub = get_pubkey();
+ my $now = time();
- my ($username, $age) = PVE::Ticket::verify_rsa_ticket(
- $rsa_pub, 'PVE', $ticket, undef, -300, $ticket_lifetime, $noerr);
+ my $check = sub {
+ my ($old) = @_;
- return undef if $noerr && !defined($username);
+ my ($rsa_pub, $rsa_mtime) = get_pubkey($old);
+ return undef if !$rsa_pub;
+
+ my ($min, $max) = $get_ticket_age_range->($now, $rsa_mtime, $old);
+ return undef if !$min;
+
+ return PVE::Ticket::verify_rsa_ticket(
+ $rsa_pub, 'PVE', $ticket, undef, $min, $max, 1);
+ };
+
+ my ($username, $age) = $check->();
+
+ # check with old, rotated key if current key failed
+ ($username, $age) = $check->(1) if !defined($username);
+
+ if (!defined($username)) {
+ if ($noerr) {
+ return undef;
+ } else {
+ # raise error via undef ticket
+ PVE::Ticket::verify_rsa_ticket(undef, 'PVE');
+ }
+ }
return undef if !PVE::Auth::Plugin::verify_username($username, $noerr);
@@ -154,10 +317,18 @@ sub assemble_vnc_ticket {
sub verify_vnc_ticket {
my ($ticket, $username, $path, $noerr) = @_;
- my $rsa_pub = get_pubkey();
-
my $secret_data = "$username:$path";
+ my ($rsa_pub, $rsa_mtime) = get_pubkey();
+ if (!$rsa_pub || (time() - $rsa_mtime > $authkey_lifetime)) {
+ if ($noerr) {
+ return undef;
+ } else {
+ # raise error via undef ticket
+ PVE::Ticket::verify_rsa_ticket($rsa_pub, 'PVEVNC');
+ }
+ }
+
return PVE::Ticket::verify_rsa_ticket(
$rsa_pub, 'PVEVNC', $ticket, $secret_data, -20, 40, $noerr);
}
--
2.20.1
More information about the pve-devel
mailing list