From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 922A381F26 for ; Fri, 26 Nov 2021 14:55:54 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8F55B1942D for ; Fri, 26 Nov 2021 14:55:54 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7932C191F4 for ; Fri, 26 Nov 2021 14:55:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 543BB44CA1 for ; Fri, 26 Nov 2021 14:55:42 +0100 (CET) From: Wolfgang Bumiller To: pmg-devel@lists.proxmox.com Date: Fri, 26 Nov 2021 14:55:09 +0100 Message-Id: <20211126135524.117846-6-w.bumiller@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211126135524.117846-1-w.bumiller@proxmox.com> References: <20211126135524.117846-1-w.bumiller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.431 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [httpserver.pm, pmgproxy.pm, tfa.pm, accesscontrol.pm, ticket.pm] Subject: [pmg-devel] [PATCH api 5/6] implement tfa authentication X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Nov 2021 13:55:54 -0000 Signed-off-by: Wolfgang Bumiller --- 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