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 1DA5C82077 for ; Fri, 26 Nov 2021 18:30:01 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 0DE821BBCD for ; Fri, 26 Nov 2021 18:30:01 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 958C31BBBF for ; Fri, 26 Nov 2021 18:29:59 +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 640BB44C7B for ; Fri, 26 Nov 2021 18:29:59 +0100 (CET) Date: Fri, 26 Nov 2021 18:29:57 +0100 From: Stoiko Ivanov To: Wolfgang Bumiller Cc: pmg-devel@lists.proxmox.com Message-ID: <20211126182957.215bc14c@rosa.proxmox.com> In-Reply-To: <20211126135524.117846-4-w.bumiller@proxmox.com> References: <20211126135524.117846-1-w.bumiller@proxmox.com> <20211126135524.117846-4-w.bumiller@proxmox.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.294 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. [objectgrouphelpers.pm, accesscontrol.pm, rules.pm, quarantine.pm, postfix.pm, tfa.pm, ruledb.pm] Subject: Re: [pmg-devel] [PATCH api 3/6] add TFA API 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 17:30:01 -0000 two small notes inline: On Fri, 26 Nov 2021 14:55:07 +0100 Wolfgang Bumiller wrote: > Signed-off-by: Wolfgang Bumiller > --- > src/Makefile | 1 + > src/PMG/API2/AccessControl.pm | 6 + > src/PMG/API2/TFA.pm | 462 ++++++++++++++++++++++++++++++++++ > 3 files changed, 469 insertions(+) > create mode 100644 src/PMG/API2/TFA.pm > > diff --git a/src/Makefile b/src/Makefile > index de05aa0..c2bf2c9 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -148,6 +148,7 @@ LIBSOURCES = \ > PMG/API2/Postfix.pm \ > PMG/API2/Quarantine.pm \ > PMG/API2/AccessControl.pm \ > + PMG/API2/TFA.pm \ > PMG/API2/ObjectGroupHelpers.pm \ > PMG/API2/Rules.pm \ > PMG/API2/RuleDB.pm \ > diff --git a/src/PMG/API2/AccessControl.pm b/src/PMG/API2/AccessControl.pm > index cb8daa5..942f8dc 100644 > --- a/src/PMG/API2/AccessControl.pm > +++ b/src/PMG/API2/AccessControl.pm > @@ -13,6 +13,7 @@ use PMG::Utils; > use PMG::UserConfig; > use PMG::AccessControl; > use PMG::API2::Users; > +use PMG::API2::TFA; > > use Data::Dumper; > > @@ -23,6 +24,11 @@ __PACKAGE__->register_method ({ > path => 'users', > }); > > +__PACKAGE__->register_method ({ > + subclass => "PMG::API2::TFA", > + path => 'tfa', > +}); > + > __PACKAGE__->register_method ({ > name => 'index', > path => '', > diff --git a/src/PMG/API2/TFA.pm b/src/PMG/API2/TFA.pm > new file mode 100644 > index 0000000..626d4f8 > --- /dev/null > +++ b/src/PMG/API2/TFA.pm > @@ -0,0 +1,462 @@ > +package PMG::API2::TFA; > + > +use strict; > +use warnings; > + > +use HTTP::Status qw(:constants); > + > +use PVE::Exception qw(raise raise_perm_exc raise_param_exc); > +use PVE::JSONSchema qw(get_standard_option); > +use PVE::RESTHandler; > + > +use PMG::AccessControl; > +use PMG::RESTEnvironment; > +use PMG::TFAConfig; > +use PMG::UserConfig; > +use PMG::Utils; > + > +use base qw(PVE::RESTHandler); > + > +my $OPTIONAL_PASSWORD_SCHEMA = { > + description => "The current password.", > + type => 'string', > + optional => 1, # Only required if not root@pam > + minLength => 5, > + maxLength => 64 > +}; > + > +my $TFA_TYPE_SCHEMA = { > + type => 'string', > + description => 'TFA Entry Type.', > + enum => [qw(totp u2f webauthn recovery)], > +}; > + > +my %TFA_INFO_PROPERTIES = ( > + id => { > + type => 'string', > + description => 'The id used to reference this entry.', > + }, > + description => { > + type => 'string', > + description => 'User chosen description for this entry.', > + }, > + created => { > + type => 'integer', > + description => 'Creation time of this entry as unix epoch.', > + }, > + enable => { > + type => 'boolean', > + description => 'Whether this TFA entry is currently enabled.', > + optional => 1, > + default => 1, > + }, > +); > + > +my $TYPED_TFA_ENTRY_SCHEMA = { > + type => 'object', > + description => 'TFA Entry.', > + properties => { > + type => $TFA_TYPE_SCHEMA, > + %TFA_INFO_PROPERTIES, > + }, > +}; > + > +my $TFA_ID_SCHEMA = { > + type => 'string', > + description => 'A TFA entry id.', > +}; > + > +my $TFA_UPDATE_INFO_SCHEMA = { > + type => 'object', > + properties => { > + id => { > + type => 'string', > + description => 'The id of a newly added TFA entry.', > + }, > + challenge => { > + type => 'string', > + optional => 1, > + description => > + 'When adding u2f entries, this contains a challenge the user must respond to in' > + .' order to finish the registration.' > + }, > + recovery => { > + type => 'array', > + optional => 1, > + description => > + 'When adding recovery codes, this contains the list of codes to be displayed to' > + .' the user', > + items => { > + type => 'string', > + description => 'A recovery entry.' > + }, > + }, > + }, > +}; > + > +# Set TFA to enabled if $tfa_cfg is passed, or to disabled if $tfa_cfg is undef, > +# When enabling we also merge the old user.cfg keys into the $tfa_cfg. > +my sub set_user_tfa_enabled : prototype($$$) { > + my ($userid, $realm, $tfa_cfg) = @_; > + > + PMG::UserConfig::lock_config(sub { > + my $cfg = PMG::UserConfig->new(); > + my $user = $cfg->lookup_user_data($userid); > + > + # We had the 'keys' property available in PMG for a while, but never used it. > + # If the keys property had been used by someone, let's just error out here. Wasn't aware of the 'keys' property until now - but just so that it's not forgotten - we should remove it from the User Add/Edit window as well > + my $keys = $user->{keys}; > + die "user has an unsupported 'keys' value, please remove\n" > + if defined($keys) && $keys ne 'x'; > + > + $user->{keys} = $tfa_cfg ? 'x' : undef; > + > + $cfg->write(); > + }, "enabling/disabling TFA for the user failed"); > +} > + > +# Only root may modify root, regular users need to specify their password. > +# > +# Returns the userid returned from `verify_username`. > +# Or ($userid, $realm) in list context. > +my sub check_permission_password : prototype($$$$) { > + my ($rpcenv, $authuser, $userid, $password) = @_; > + > + ($userid, my $ruid, my $realm) = PMG::Utils::verify_username($userid); > + raise("no access from quarantine\n") if $realm eq 'quarantine'; > + > + raise_perm_exc() if $userid eq 'root@pam' && $authuser ne 'root@pam'; > + > + # Regular users need to confirm their password to change TFA settings. > + if ($authuser ne 'root@pam') { > + raise_param_exc({ 'password' => 'password is required to modify TFA data' }) > + if !defined($password); > + > + PMG::AccessControl::authenticate_user($userid, $password); > + } > + > + return wantarray ? ($userid, $realm) : $userid; > +} > + > +my sub check_permission_self : prototype($$) { > + my ($rpcenv, $userid) = @_; > + > + my $authuser = $rpcenv->get_user(); > + > + ($userid, my $ruid, my $realm) = PMG::Utils::verify_username($userid); > + raise("no access from quarantine\n") if $realm eq 'quarantine'; > + > + if ($authuser eq 'root@pam') { > + # OK - root can change anything > + } else { > + if ($realm eq 'pmg' && $authuser eq $userid) { > + # OK - each enable user can see their own data > + PMG::AccessControl::check_user_enabled($rpcenv->{usercfg}, $userid); > + } else { > + raise_perm_exc(); > + } > + } > +} > + > +__PACKAGE__->register_method ({ > + name => 'list_user_tfa', > + path => '{userid}', > + method => 'GET', > + proxyto => 'master', > + permissions => { > + description => > + 'Each user is allowed to view their own TFA entries.' > + .' Only root can view entries of another user.', > + user => 'all', > + }, most other API methods use `permissions => { check => [ 'admin', 'qmanager', 'audit'] },` to exclude quarantine users from it (but this can easily be fixed at a later point as well (not necessarily by you), and is not material to the series) > + protected => 1, # else we can't access shadow files > + allowtoken => 0, # we don't want tokens to change the regular user's TFA settings > + description => 'List TFA configurations of users.', > + parameters => { > + additionalProperties => 0, > + properties => { > + userid => get_standard_option('userid'), > + } > + }, > + returns => { > + description => "A list of the user's TFA entries.", > + type => 'array', > + items => $TYPED_TFA_ENTRY_SCHEMA, > + }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PMG::RESTEnvironment->get(); > + check_permission_self($rpcenv, $param->{userid}); > + > + my $tfa_cfg = PMG::TFAConfig->new(); > + return $tfa_cfg->api_list_user_tfa($param->{userid}); > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'get_tfa_entry', > + path => '{userid}/{id}', > + method => 'GET', > + proxyto => 'master', > + permissions => { > + description => > + 'Each user is allowed to view their own TFA entries.' > + .' Only root can view entries of another user.', > + user => 'all', > + }, > + protected => 1, # else we can't access shadow files > + allowtoken => 0, # we don't want tokens to change the regular user's TFA settings > + description => 'Fetch a requested TFA entry if present.', > + parameters => { > + additionalProperties => 0, > + properties => { > + userid => get_standard_option('userid'), > + id => $TFA_ID_SCHEMA, > + } > + }, > + returns => $TYPED_TFA_ENTRY_SCHEMA, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PMG::RESTEnvironment->get(); > + check_permission_self($rpcenv, $param->{userid}); > + > + my $tfa_cfg = PMG::TFAConfig->new(); > + my $id = $param->{id}; > + my $entry = $tfa_cfg->api_get_tfa_entry($param->{userid}, $id); > + raise("No such tfa entry '$id'", code => HTTP::Status::HTTP_NOT_FOUND) if !$entry; > + return $entry; > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'delete_tfa', > + path => '{userid}/{id}', > + method => 'DELETE', > + proxyto => 'master', > + permissions => { > + description => > + 'Each user is allowed to modify their own TFA entries.' > + .' Only root can modify entries of another user.', > + user => 'all', > + }, > + protected => 1, # else we can't access shadow files > + allowtoken => 0, # we don't want tokens to change the regular user's TFA settings > + description => 'Delete a TFA entry by ID.', > + parameters => { > + additionalProperties => 0, > + properties => { > + userid => get_standard_option('userid'), > + id => $TFA_ID_SCHEMA, > + password => $OPTIONAL_PASSWORD_SCHEMA, > + } > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PMG::RESTEnvironment->get(); > + check_permission_self($rpcenv, $param->{userid}); > + > + my $authuser = $rpcenv->get_user(); > + my $userid = > + check_permission_password($rpcenv, $authuser, $param->{userid}, $param->{password}); > + > + my $has_entries_left = PMG::TFAConfig::lock_config(sub { > + my $tfa_cfg = PMG::TFAConfig->new(); > + my $has_entries_left = $tfa_cfg->api_delete_tfa($userid, $param->{id}); > + $tfa_cfg->write(); > + return $has_entries_left; > + }); > + > + if (!$has_entries_left) { > + set_user_tfa_enabled($userid, undef, undef); > + } > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'list_tfa', > + path => '', > + method => 'GET', > + proxyto => 'master', > + permissions => { > + description => "Returns all or just the logged-in user, depending on privileges.", > + user => 'all', > + }, > + protected => 1, # else we can't access shadow files > + allowtoken => 0, # we don't want tokens to change the regular user's TFA settings > + description => 'List TFA configurations of users.', > + parameters => { > + additionalProperties => 0, > + properties => {} > + }, > + returns => { > + description => "The list tuples of user and TFA entries.", > + type => 'array', > + items => { > + type => 'object', > + properties => { > + userid => { > + type => 'string', > + description => 'User this entry belongs to.', > + }, > + entries => { > + type => 'array', > + items => $TYPED_TFA_ENTRY_SCHEMA, > + }, > + }, > + }, > + }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PMG::RESTEnvironment->get(); > + my $authuser = $rpcenv->get_user(); > + my $top_level_allowed = ($authuser eq 'root@pam'); > + > + my $tfa_cfg = PMG::TFAConfig->new(); > + return $tfa_cfg->api_list_tfa($authuser, $top_level_allowed); > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'add_tfa_entry', > + path => '{userid}', > + method => 'POST', > + proxyto => 'master', > + permissions => { > + description => > + 'Each user is allowed to modify their own TFA entries.' > + .' Only root can modify entries of another user.', > + user => 'all', > + }, > + protected => 1, # else we can't access shadow files > + allowtoken => 0, # we don't want tokens to change the regular user's TFA settings > + description => 'Add a TFA entry for a user.', > + parameters => { > + additionalProperties => 0, > + properties => { > + userid => get_standard_option('userid'), > + type => $TFA_TYPE_SCHEMA, > + description => { > + type => 'string', > + description => 'A description to distinguish multiple entries from one another', > + maxLength => 255, > + optional => 1, > + }, > + totp => { > + type => 'string', > + description => "A totp URI.", > + optional => 1, > + }, > + value => { > + type => 'string', > + description => > + 'The current value for the provided totp URI, or a Webauthn/U2F' > + .' challenge response', > + optional => 1, > + }, > + challenge => { > + type => 'string', > + description => 'When responding to a u2f challenge: the original challenge string', > + optional => 1, > + }, > + password => $OPTIONAL_PASSWORD_SCHEMA, > + }, > + }, > + returns => $TFA_UPDATE_INFO_SCHEMA, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PMG::RESTEnvironment->get(); > + check_permission_self($rpcenv, $param->{userid}); > + my $authuser = $rpcenv->get_user(); > + my ($userid, $realm) = > + check_permission_password($rpcenv, $authuser, $param->{userid}, $param->{password}); > + > + my $type = delete $param->{type}; > + my $value = delete $param->{value}; > + > + return PMG::TFAConfig::lock_config(sub { > + my $tfa_cfg = PMG::TFAConfig->new(); > + > + set_user_tfa_enabled($userid, $realm, $tfa_cfg); > + my $origin = undef; > + if (!$tfa_cfg->has_webauthn_origin()) { > + $origin = $rpcenv->get_request_host(1); > + } > + > + my $response = $tfa_cfg->api_add_tfa_entry( > + $userid, > + $param->{description}, > + $param->{totp}, > + $value, > + $param->{challenge}, > + $type, > + $origin, > + ); > + > + $tfa_cfg->write(); > + > + return $response; > + }); > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'update_tfa_entry', > + path => '{userid}/{id}', > + method => 'PUT', > + proxyto => 'master', > + permissions => { > + description => > + 'Each user is allowed to modify their own TFA entries.' > + .' Only root can modify entries of another user.', > + user => 'all', > + }, > + protected => 1, # else we can't access shadow files > + allowtoken => 0, # we don't want tokens to change the regular user's TFA settings > + description => 'Add a TFA entry for a user.', > + parameters => { > + additionalProperties => 0, > + properties => { > + userid => get_standard_option('userid', { > + completion => \&PVE::AccessControl::complete_username, > + }), > + id => $TFA_ID_SCHEMA, > + description => { > + type => 'string', > + description => 'A description to distinguish multiple entries from one another', > + maxLength => 255, > + optional => 1, > + }, > + enable => { > + type => 'boolean', > + description => 'Whether the entry should be enabled for login.', > + optional => 1, > + }, > + password => $OPTIONAL_PASSWORD_SCHEMA, > + }, > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PMG::RESTEnvironment->get(); > + check_permission_self($rpcenv, $param->{userid}); > + my $authuser = $rpcenv->get_user(); > + my $userid = > + check_permission_password($rpcenv, $authuser, $param->{userid}, $param->{password}); > + > + PMG::TFAConfig::lock_config(sub { > + my $tfa_cfg = PMG::TFAConfig->new(); > + > + $tfa_cfg->api_update_tfa_entry( > + $userid, > + $param->{id}, > + $param->{description}, > + $param->{enable}, > + ); > + > + $tfa_cfg->write(); > + }); > + }}); > + > +1;