[pmg-devel] [PATCH api 5/6] implement tfa authentication

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

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/PMG/API2/AccessControl.pm | 76 +++++++++++++++++++++++++++++------
 src/PMG/API2/TFA.pm           |  4 +-
 src/PMG/AccessControl.pm      | 29 +++++++++----
 src/PMG/HTTPServer.pm         |  5 ++-
 src/PMG/Service/pmgproxy.pm   |  2 +-
 src/PMG/Ticket.pm             | 30 ++++++++++----
 6 files changed, 114 insertions(+), 32 deletions(-)

diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm
index 942f8dc..5774fab 100644
--- a/src/PMG/API2/AccessControl.pm
+++ b/src/PMG/API2/AccessControl.pm
@@ -63,11 +63,11 @@ __PACKAGE__->register_method ({
 	return $res;
     }});
 
-
-my $create_or_verify_ticket = sub {
-    my ($rpcenv, $username, $pw_or_ticket, $otp, $path) = @_;
+my sub create_or_verify_ticket : prototype($$$$$$) {
+    my ($rpcenv, $username, $pw_or_ticket, $path, $otp, $tfa_challenge) = @_;
 
     my $ticketuser;
+    my $aad;
 
     if ($pw_or_ticket =~ m/^PMGQUAR:/) {
 	my $ticketuser = PMG::Ticket::verify_quarantine_ticket($pw_or_ticket);
@@ -85,13 +85,22 @@ my $create_or_verify_ticket = sub {
 
     my $role = PMG::AccessControl::check_user_enabled($rpcenv->{usercfg}, $username);
 
-    if (($ticketuser = PMG::Ticket::verify_ticket($pw_or_ticket, 1)) &&
-	($ticketuser eq 'root@pam' || $ticketuser eq $username)) {
-	# valid ticket. Note: root@pam can create tickets for other users
-    } elsif ($path && PMG::Ticket::verify_vnc_ticket($pw_or_ticket, $username, $path, 1)) {
-	# valid vnc ticket for $path
-    } else {
-	$username = PMG::AccessControl::authenticate_user($username, $pw_or_ticket, $otp);
+    my $tfa_challenge_is_ticket = 1;
+
+    if (!$tfa_challenge) {
+	$tfa_challenge_is_ticket = 0;
+	($ticketuser, undef, $tfa_challenge) = PMG::Ticket::verify_ticket($pw_or_ticket, undef, 1);
+	die "No ticket\n" if $tfa_challenge;
+
+	if ($ticketuser && ($ticketuser eq 'root@pam' || $ticketuser eq $username)) {
+	    # valid ticket. Note: root@pam can create tickets for other users
+	} elsif ($path && PMG::Ticket::verify_vnc_ticket($pw_or_ticket, $username, $path, 1)) {
+	    # valid vnc ticket for $path
+	} else {
+	    ($username, $tfa_challenge) =
+		PMG::AccessControl::authenticate_user($username, $pw_or_ticket, 0);
+	    $pw_or_ticket = $otp;
+	}
     }
 
     if (defined($path)) {
@@ -99,7 +108,42 @@ my $create_or_verify_ticket = sub {
 	return { username => $username };
     }
 
-    my $ticket = PMG::Ticket::assemble_ticket($username);
+    if ($tfa_challenge && $pw_or_ticket) {
+	if ($tfa_challenge_is_ticket) {
+	    (undef, undef, $tfa_challenge) = PMG::Ticket::verify_ticket($tfa_challenge, $username, 0);
+	}
+	PMG::TFAConfig::lock_config(sub {
+	    my $tfa_cfg = PMG::TFAConfig->new();
+
+	    my $origin = undef;
+	    if (!$tfa_cfg->has_webauthn_origin()) {
+		my $rpcenv = PMG::RESTEnvironment->get();
+		$origin = 'https://'.$rpcenv->get_request_host(1);
+	    }
+	    my $must_save = $tfa_cfg->authentication_verify(
+		$username,
+		$tfa_challenge,
+		$pw_or_ticket,
+		$origin,
+	    );
+
+	    $tfa_cfg->write() if $must_save;
+	});
+
+	$tfa_challenge = undef;
+    }
+
+    my $ticket_data;
+    my %extra;
+    if ($tfa_challenge) {
+	$ticket_data = '!tfa!' . $tfa_challenge;
+	$aad = $username;
+	$extra{NeedTFA} = 1;
+    } else {
+	$ticket_data = $username;
+    }
+
+    my $ticket = PMG::Ticket::assemble_ticket($ticket_data, $aad);
     my $csrftoken = PMG::Ticket::assemble_csrf_prevention_token($username);
 
     return {
@@ -107,6 +151,7 @@ my $create_or_verify_ticket = sub {
 	ticket => $ticket,
 	username => $username,
 	CSRFPreventionToken => $csrftoken,
+	%extra,
     };
 };
 
@@ -160,6 +205,11 @@ __PACKAGE__->register_method ({
 		optional => 1,
 		maxLength => 64,
 	    },
+	    'tfa-challenge' => {
+		type => 'string',
+		description => "The signed TFA challenge string the user wants to respond to.",
+		optional => 1,
+	    },
 	}
     },
     returns => {
@@ -187,8 +237,8 @@ __PACKAGE__->register_method ({
 
 	my $res;
 	eval {
-	    $res = &$create_or_verify_ticket($rpcenv, $username,
-		    $param->{password}, $param->{otp}, $param->{path});
+	    $res = create_or_verify_ticket($rpcenv, $username,
+		    $param->{password}, $param->{path}, $param->{otp}, $param->{'tfa-challenge'});
 	};
 	if (my $err = $@) {
 	    my $clientip = $rpcenv->get_client_ip() || '';
diff --git a/src/PMG/API2/TFA.pm b/src/PMG/API2/TFA.pm
index 626d4f8..33c718f 100644
--- a/src/PMG/API2/TFA.pm
+++ b/src/PMG/API2/TFA.pm
@@ -132,7 +132,7 @@ my sub check_permission_password : prototype($$$$) {
 	raise_param_exc({ 'password' => 'password is required to modify TFA data' })
 	    if !defined($password);
 
-	PMG::AccessControl::authenticate_user($userid, $password);
+	PMG::AccessControl::authenticate_user($userid, $password, 1);
     }
 
     return wantarray ? ($userid, $realm) : $userid;
@@ -381,7 +381,7 @@ __PACKAGE__->register_method ({
 	    set_user_tfa_enabled($userid, $realm, $tfa_cfg);
 	    my $origin = undef;
 	    if (!$tfa_cfg->has_webauthn_origin()) {
-		$origin = $rpcenv->get_request_host(1);
+		$origin = 'https://'.$rpcenv->get_request_host(1);
 	    }
 
 	    my $response = $tfa_cfg->api_add_tfa_entry(
diff --git a/src/PMG/AccessControl.pm b/src/PMG/AccessControl.pm
index 1461335..b093666 100644
--- a/src/PMG/AccessControl.pm
+++ b/src/PMG/AccessControl.pm
@@ -26,8 +26,10 @@ sub normalize_path {
 
 # password should be utf8 encoded
 # Note: some plugins delay/sleep if auth fails
-sub authenticate_user {
-    my ($username, $password, $otp) = @_;
+#
+# returns ($username, $tfa_challenge)
+sub authenticate_user : prototype($$$) {
+    my ($username, $password, $skip_tfa) = @_;
 
     die "no username specified\n" if !$username;
 
@@ -38,24 +40,35 @@ sub authenticate_user {
     if ($realm eq 'pam') {
 	die "invalid pam user (only root allowed)\n" if $ruid ne 'root';
 	authenticate_pam_user($ruid, $password);
-	return $username;
     } elsif ($realm eq 'pmg') {
 	my $usercfg = PMG::UserConfig->new();
 	$usercfg->authenticate_user($username, $password);
-	return $username;
     } elsif ($realm eq 'quarantine') {
 	my $ldap_cfg = PMG::LDAPConfig->new();
 	my $ldap = PMG::LDAPSet->new_from_ldap_cfg($ldap_cfg, 1);
 
 	if (my $ldapinfo = $ldap->account_info($ruid, $password)) {
 	    my $pmail = $ldapinfo->{pmail};
-	    return $pmail . '@quarantine';
-	} else {
-	    die "ldap login failed\n";
+	    return ($pmail . '@quarantine', undef);
 	}
+	die "ldap login failed\n";
+    } else {
+	die "no such realm '$realm'\n";
     }
 
-    die "no such realm '$realm'\n";
+    return ($username, undef) if $skip_tfa;
+
+    my $tfa = PMG::TFAConfig->new();
+
+    my $origin = undef;
+    if (!$tfa->has_webauthn_origin()) {
+	my $rpcenv = PMG::RESTEnvironment->get();
+	$origin = 'https://'.$rpcenv->get_request_host(1);
+    }
+
+    my $tfa_challenge = $tfa->authentication_challenge($username, $origin);
+
+    return ($username, $tfa_challenge);
 }
 
 sub set_user_password {
diff --git a/src/PMG/HTTPServer.pm b/src/PMG/HTTPServer.pm
index 3dc9655..b6c50d9 100755
--- a/src/PMG/HTTPServer.pm
+++ b/src/PMG/HTTPServer.pm
@@ -76,7 +76,10 @@ sub auth_handler {
 	    $rpcenv->set_user($username);
 	    $rpcenv->set_role('quser');
 	} else {
-	    ($username, $age) = PMG::Ticket::verify_ticket($ticket);
+	    ($username, $age, my $tfa) = PMG::Ticket::verify_ticket($ticket, undef, 0);
+	    # TFA tickets don't return a username, and return a tfa challenge, either is enough to
+	    # fail here:
+	    die "No ticket\n" if !$username || $tfa;
 	    my $role = PMG::AccessControl::check_user_enabled($self->{usercfg}, $username);
 	    $rpcenv->set_user($username);
 	    $rpcenv->set_role($role);
diff --git a/src/PMG/Service/pmgproxy.pm b/src/PMG/Service/pmgproxy.pm
index 89efa6a..8a8a9d0 100755
--- a/src/PMG/Service/pmgproxy.pm
+++ b/src/PMG/Service/pmgproxy.pm
@@ -199,7 +199,7 @@ sub get_index {
 	if ($ticket =~ m/^PMGQUAR:/) {
 	    $username = PMG::Ticket::verify_quarantine_ticket($ticket, 1);
 	} else {
-	    $username = PMG::Ticket::verify_ticket($ticket, 1);
+	    $username = PMG::Ticket::verify_ticket($ticket, undef, 1);
 	}
     } else {
 	if (defined($args->{ticket})) {
diff --git a/src/PMG/Ticket.pm b/src/PMG/Ticket.pm
index 344e784..0c2ec0b 100644
--- a/src/PMG/Ticket.pm
+++ b/src/PMG/Ticket.pm
@@ -164,22 +164,38 @@ sub assemble_csrf_prevention_token {
     return PVE::Ticket::assemble_csrf_prevention_token ($secret, $username);
 }
 
-sub assemble_ticket {
-    my ($username) = @_;
+sub assemble_ticket : prototype($;$) {
+    my ($data, $aad) = @_;
 
     my $rsa_priv = PVE::INotify::read_file('auth_priv_key');
 
-    return PVE::Ticket::assemble_rsa_ticket($rsa_priv, 'PMG', $username);
+    return PVE::Ticket::assemble_rsa_ticket($rsa_priv, 'PMG', $data, $aad);
 }
 
-sub verify_ticket {
-    my ($ticket, $noerr) = @_;
+# Returns (username, age, tfa-challenge) or just the username in scalar context.
+# Note that in scalar context, tfa tickets return `undef`.
+sub verify_ticket : prototype($$$) {
+    my ($ticket, $aad, $noerr) = @_;
 
     my $rsa_pub = PVE::INotify::read_file('auth_pub_key');
 
-    return PVE::Ticket::verify_rsa_ticket(
-	$rsa_pub, 'PMG', $ticket, undef,
+    my $tfa_challenge;
+    my ($data, $age) = PVE::Ticket::verify_rsa_ticket(
+	$rsa_pub, 'PMG', $ticket, $aad,
 	$min_ticket_lifetime, $max_ticket_lifetime, $noerr);
+
+    if ($noerr && !$data) {
+	# if $noerr was set $data can be undef:
+	return wantarray ? (undef, undef, undef) : undef;
+    }
+
+
+    if ($data =~ /^!tfa!(.*)$/) {
+	return (undef, $age, $1) if wantarray;
+	return undef if $noerr;
+	die "second factor required\n";
+    }
+    return wantarray ? ($data, $age, undef) : $data;
 }
 
 # VNC tickets
-- 
2.30.2