public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: pmg-devel@lists.proxmox.com
Subject: [pmg-devel] [PATCH api 5/6] implement tfa authentication
Date: Fri, 26 Nov 2021 14:55:09 +0100	[thread overview]
Message-ID: <20211126135524.117846-6-w.bumiller@proxmox.com> (raw)
In-Reply-To: <20211126135524.117846-1-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





  parent reply	other threads:[~2021-11-26 13:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 1/6] add tfa.json and its lock methods Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 2/6] add PMG::TFAConfig module Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 3/6] add TFA API Wolfgang Bumiller
2021-11-26 17:29   ` Stoiko Ivanov
2021-11-26 13:55 ` [pmg-devel] [PATCH api 4/6] add tfa config api Wolfgang Bumiller
2021-11-26 13:55 ` Wolfgang Bumiller [this message]
2021-11-26 13:55 ` [pmg-devel] [PATCH api 6/6] provide qrcode.min.js from libjs-qrcodejs Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH gui] add TFA components Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 1/7] pve: bump perlmod to 0.9 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 2/7] pve: update to proxmox-tfa 2.0 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 3/7] pve: bump d/control Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 4/7] import pmg-rs Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 5/7] pmg: bump perlmod to 0.9 Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 6/7] pmg: add tfa module Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 7/7] pmg: bump d/control Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 1/6] tfa: fix typo in docs Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 2/6] tfa: add WebauthnConfig::digest method Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 3/6] tfa: let OriginUrl deref to its inner Url, add FromStr impl Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 4/6] tfa: make configured webauthn origin optional Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 5/6] tfa: clippy fixes Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 6/6] bump proxmox-tfa to 2.0.0-1 Wolfgang Bumiller
2021-11-26 17:34 ` [pmg-devel] [PATCH multiple 0/7] PMG TFA support Stoiko Ivanov
2021-11-28 21:17 ` [pmg-devel] applied-series: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211126135524.117846-6-w.bumiller@proxmox.com \
    --to=w.bumiller@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal