[pve-devel] [PATCH access-control 03/10] use PBS-like auth api call flow

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

The main difference here is that we have no separate api
path for verification. Instead, the ticket api call gets an
optional 'tfa-challenge' parameter.

This is opt-in: old pve-manager UI with new
pve-access-control will still work as expected, but users
won't be able to *update* their TFA settings.

Since the new tfa config parser will build a compatible
in-perl tfa config object as well, the old authentication
code is left unchanged for compatibility and will be removed
with pve-8, where the `new-format` parameter in the ticket
call will change its default to `1`.

The `change_tfa` call will simply die in this commit. It is
removed later when adding the pbs-style TFA API calls.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PVE/API2/AccessControl.pm |  79 ++++++++++++++----
 src/PVE/AccessControl.pm      | 153 +++++++++++++++++++++++++++++-----
 2 files changed, 199 insertions(+), 33 deletions(-)

diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm
index 6dec66c..8fa3606 100644
--- a/src/PVE/API2/AccessControl.pm
+++ b/src/PVE/API2/AccessControl.pm
@@ -105,8 +105,8 @@ __PACKAGE__->register_method ({
     }});
 
 
-my $verify_auth = sub {
-    my ($rpcenv, $username, $pw_or_ticket, $otp, $path, $privs) = @_;
+my sub verify_auth : prototype($$$$$$$) {
+    my ($rpcenv, $username, $pw_or_ticket, $otp, $path, $privs, $new_format) = @_;
 
     my $normpath = PVE::AccessControl::normalize_path($path);
 
@@ -117,7 +117,12 @@ my $verify_auth = sub {
     } elsif (PVE::AccessControl::verify_vnc_ticket($pw_or_ticket, $username, $normpath, 1)) {
 	# valid vnc ticket
     } else {
-	$username = PVE::AccessControl::authenticate_user($username, $pw_or_ticket, $otp);
+	$username = PVE::AccessControl::authenticate_user(
+	    $username,
+	    $pw_or_ticket,
+	    $otp,
+	    $new_format,
+	);
     }
 
     my $privlist = [ PVE::Tools::split_list($privs) ];
@@ -128,22 +133,45 @@ my $verify_auth = sub {
     return { username => $username };
 };
 
-my $create_ticket = sub {
-    my ($rpcenv, $username, $pw_or_ticket, $otp) = @_;
+my sub create_ticket_do : prototype($$$$$$) {
+    my ($rpcenv, $username, $pw_or_ticket, $otp, $new_format, $tfa_challenge) = @_;
+
+    die "TFA response should be in 'password', not 'otp' when 'tfa-challenge' is set\n"
+	if defined($otp) && defined($tfa_challenge);
+
+    my ($ticketuser, undef, $tfa_info);
+    if (!defined($tfa_challenge)) {
+	# We only verify this ticket if we're not responding to a TFA challenge, as in that case
+	# it is a TFA-data ticket and will be verified by `authenticate_user`.
+
+	($ticketuser, undef, $tfa_info) = PVE::AccessControl::verify_ticket($pw_or_ticket, 1);
+    }
 
-    my ($ticketuser, undef, $tfa_info) = PVE::AccessControl::verify_ticket($pw_or_ticket, 1);
     if (defined($ticketuser) && ($ticketuser eq 'root@pam' || $ticketuser eq $username)) {
 	if (defined($tfa_info)) {
 	    die "incomplete ticket\n";
 	}
 	# valid ticket. Note: root@pam can create tickets for other users
     } else {
-	($username, $tfa_info) = PVE::AccessControl::authenticate_user($username, $pw_or_ticket, $otp);
+	($username, $tfa_info) = PVE::AccessControl::authenticate_user(
+	    $username,
+	    $pw_or_ticket,
+	    $otp,
+	    $new_format,
+	    $tfa_challenge,
+	);
     }
 
     my %extra;
     my $ticket_data = $username;
-    if (defined($tfa_info)) {
+    my $aad;
+    if ($new_format) {
+	if (defined($tfa_info)) {
+	    $extra{NeedTFA} = 1;
+	    $ticket_data = "!tfa!$tfa_info";
+	    $aad = $username;
+	}
+    } elsif (defined($tfa_info)) {
 	$extra{NeedTFA} = 1;
 	if ($tfa_info->{type} eq 'u2f') {
 	    my $u2finfo = $tfa_info->{data};
@@ -159,7 +187,7 @@ my $create_ticket = sub {
 	}
     }
 
-    my $ticket = PVE::AccessControl::assemble_ticket($ticket_data);
+    my $ticket = PVE::AccessControl::assemble_ticket($ticket_data, $aad);
     my $csrftoken = PVE::AccessControl::assemble_csrf_prevention_token($username);
 
     return {
@@ -230,6 +258,20 @@ __PACKAGE__->register_method ({
 		optional => 1,
 		maxLength => 64,
 	    },
+	    'new-format' => {
+		type => 'boolean',
+		description =>
+		    'With webauthn the format of half-authenticated tickts changed.'
+		    .' New clients should pass 1 here and not worry about the old format.'
+		    .' The old format is deprecated and will be retired with PVE-8.0',
+		optional => 1,
+		default => 0,
+	    },
+	    'tfa-challenge' => {
+		type => 'string',
+                description => "The signed TFA challenge string the user wants to respond to.",
+		optional => 1,
+	    },
 	}
     },
     returns => {
@@ -257,10 +299,17 @@ __PACKAGE__->register_method ({
 	    $rpcenv->check_user_enabled($username);
 
 	    if ($param->{path} && $param->{privs}) {
-		$res = &$verify_auth($rpcenv, $username, $param->{password}, $param->{otp},
-				     $param->{path}, $param->{privs});
+		$res = verify_auth($rpcenv, $username, $param->{password}, $param->{otp},
+				   $param->{path}, $param->{privs}, $param->{'new-format'});
 	    } else {
-		$res = &$create_ticket($rpcenv, $username, $param->{password}, $param->{otp});
+		$res = create_ticket_do(
+		    $rpcenv,
+		    $username,
+		    $param->{password},
+		    $param->{otp},
+		    $param->{'new-format'},
+		    $param->{'tfa-challenge'},
+		);
 	    }
 	};
 	if (my $err = $@) {
@@ -476,6 +525,8 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
+	die "TODO!\n";
+
 	my $rpcenv = PVE::RPCEnvironment::get();
 	my $authuser = $rpcenv->get_user();
 
@@ -528,7 +579,7 @@ __PACKAGE__->register_method ({
 	    raise_param_exc({ 'response' => "confirm action requires the 'response' parameter to be set" })
 		if !defined($response);
 
-	    my ($type, $u2fdata) = PVE::AccessControl::user_get_tfa($userid, $realm);
+	    my ($type, $u2fdata) = PVE::AccessControl::user_get_tfa($userid, $realm, 'FIXME');
 	    raise("no u2f data available")
 		if (!defined($type) || $type ne 'u2f');
 
@@ -580,7 +631,7 @@ __PACKAGE__->register_method({
 	my $authuser = $rpcenv->get_user();
 	my ($username, undef, $realm) = PVE::AccessControl::verify_username($authuser);
 
-	my ($tfa_type, $tfa_data) = PVE::AccessControl::user_get_tfa($username, $realm);
+	my ($tfa_type, $tfa_data) = PVE::AccessControl::user_get_tfa($username, $realm, 0);
 	if (!defined($tfa_type)) {
 	    raise('no u2f data available');
 	}
diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 2fa2695..d61d7f4 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -338,16 +338,23 @@ my $get_ticket_age_range = sub {
     return ($min, $max);
 };
 
-sub assemble_ticket {
-    my ($data) = @_;
+sub assemble_ticket : prototype($;$) {
+    my ($data, $aad) = @_;
 
     my $rsa_priv = get_privkey();
 
-    return PVE::Ticket::assemble_rsa_ticket($rsa_priv, 'PVE', $data);
+    return PVE::Ticket::assemble_rsa_ticket($rsa_priv, 'PVE', $data, $aad);
 }
 
-sub verify_ticket {
-    my ($ticket, $noerr) = @_;
+# Returns the username, "age" and tfa info.
+#
+# Note that for the new-style outh, tfa info is never set, as it only uses the `/ticket` api call
+# via the new 'tfa-challenge' parameter, so this part can go with PVE-8.
+#
+# New-style auth still uses this function, but sets `$tfa_ticket` to true when validating the tfa
+# ticket.
+sub verify_ticket : prototype($;$$) {
+    my ($ticket, $noerr, $tfa_ticket_aad) = @_;
 
     my $now = time();
 
@@ -361,7 +368,7 @@ sub verify_ticket {
 	return undef if !defined($min);
 
 	return PVE::Ticket::verify_rsa_ticket(
-	    $rsa_pub, 'PVE', $ticket, undef, $min, $max, 1);
+	    $rsa_pub, 'PVE', $ticket, $tfa_ticket_aad, $min, $max, 1);
     };
 
     my ($data, $age) = $check->();
@@ -382,7 +389,21 @@ sub verify_ticket {
 	return $auth_failure->();
     }
 
+    if ($tfa_ticket_aad) {
+	# We're validating a ticket-call's 'tfa-challenge' parameter, so just return its data.
+	if ($data =~ /^!tfa!(.*)$/) {
+	    return $1;
+	}
+	die "bad ticket\n";
+    }
+
     my ($username, $tfa_info);
+    if ($data =~ /^!tfa!(.*)$/) {
+	# PBS style half-authenticated ticket, contains a json string form of a `TfaChallenge`
+	# object.
+	# This type of ticket does not contain the user name.
+	return { type => 'new', data => $1 };
+    }
     if ($data =~ m{^u2f!([^!]+)!([0-9a-zA-Z/.=_\-+]+)$}) {
 	# Ticket for u2f-users:
 	($username, my $challenge) = ($1, $2);
@@ -623,8 +644,8 @@ sub verify_one_time_pw {
 
 # password should be utf8 encoded
 # Note: some plugins delay/sleep if auth fails
-sub authenticate_user {
-    my ($username, $password, $otp) = @_;
+sub authenticate_user : prototype($$$$;$) {
+    my ($username, $password, $otp, $new_format, $tfa_challenge) = @_;
 
     die "no username specified\n" if !$username;
 
@@ -641,9 +662,28 @@ sub authenticate_user {
     my $cfg = $domain_cfg->{ids}->{$realm};
     die "auth domain '$realm' does not exist\n" if !$cfg;
     my $plugin = PVE::Auth::Plugin->lookup($cfg->{type});
+
+    if ($tfa_challenge) {
+	# This is the 2nd factor, use the password for the OTP response.
+	my $tfa_challenge = authenticate_2nd_new($username, $realm, $password, $tfa_challenge);
+	return wantarray ? ($username, $tfa_challenge) : $username;
+    }
+
     $plugin->authenticate_user($cfg, $realm, $ruid, $password);
 
-    my ($type, $tfa_data) = user_get_tfa($username, $realm);
+    if ($new_format) {
+	# This is the first factor with an optional immediate 2nd factor for TOTP:
+	my $tfa_challenge = authenticate_2nd_new($username, $realm, $otp, $tfa_challenge);
+	return wantarray ? ($username, $tfa_challenge) : $username;
+    } else {
+	return authenticate_2nd_old($username, $realm, $otp);
+    }
+}
+
+sub authenticate_2nd_old : prototype($$$) {
+    my ($username, $realm, $otp) = @_;
+
+    my ($type, $tfa_data) = user_get_tfa($username, $realm, 0);
     if ($type) {
 	if ($type eq 'u2f') {
 	    # Note that if the user did not manage to complete the initial u2f registration
@@ -671,6 +711,77 @@ sub authenticate_user {
     return wantarray ? ($username, $tfa_data) : $username;
 }
 
+# Returns a tfa challenge or undef.
+sub authenticate_2nd_new : prototype($$$$) {
+    my ($username, $realm, $otp, $tfa_challenge) = @_;
+
+    return lock_tfa_config(sub {
+	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};
+	if (defined($realm_type) && $realm_type eq 'yubico') {
+	    $tfa_cfg->set_yubico_config({
+		id => $realm_tfa->{id},
+		key => $realm_tfa->{key},
+		url => $realm_tfa->{url},
+	    });
+	}
+
+	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, $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";
+		}
+	    }
+	}
+
+	if ($must_save) {
+	    cfs_write_file('priv/tfa.cfg', $tfa_cfg);
+	}
+
+	return $tfa_challenge;
+    });
+}
+
+sub configure_u2f_and_wa : prototype($) {
+    my ($tfa_cfg) = @_;
+
+    my $dc = cfs_read_file('datacenter.cfg');
+    if (my $u2f = $dc->{u2f}) {
+	my $origin = $u2f->{origin};
+	if (!defined($origin)) {
+	    my $rpcenv = PVE::RPCEnvironment::get();
+	    $origin = $rpcenv->get_request_host(1);
+	    if ($origin) {
+		$origin = "https://$origin";
+	    } else {
+		die "failed to figure out u2f origin\n";
+	    }
+	}
+	$tfa_cfg->set_u2f_config({
+	    origin => $origin,
+	    appid => $u2f->{appid},
+	});
+    }
+    if (my $wa = $dc->{webauthn}) {
+	$tfa_cfg->set_webauthn_config($wa);
+    }
+}
+
 sub domain_set_password {
     my ($realm, $username, $password) = @_;
 
@@ -1630,8 +1741,8 @@ sub user_set_tfa {
     cfs_write_file('user.cfg', $user_cfg) if defined($user);
 }
 
-sub user_get_tfa {
-    my ($username, $realm) = @_;
+sub user_get_tfa : prototype($$$) {
+    my ($username, $realm, $new_format) = @_;
 
     my $user_cfg = cfs_read_file('user.cfg');
     my $user = $user_cfg->{users}->{$username}
@@ -1662,16 +1773,20 @@ sub user_get_tfa {
 	});
     } else {
 	my $tfa_cfg = cfs_read_file('priv/tfa.cfg');
-	my $tfa = $tfa_cfg->{users}->{$username};
-	return if !$tfa; # should not happen (user.cfg wasn't cleaned up?)
+	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?)
 
-	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};
-	}
+	    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});
+	    return ($tfa->{type}, $tfa->{data});
+	}
     }
 }
 
-- 
2.30.2