* [pmg-devel] [PATCH api 1/6] add tfa.json and its lock methods
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 2/6] add PMG::TFAConfig module Wolfgang Bumiller
` (20 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
src/PMG/Cluster.pm | 1 +
src/PMG/UserConfig.pm | 32 +++++++++++++++++++++++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index d82a392..31384b2 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -459,6 +459,7 @@ sub sync_config_from_master {
'pmg-csrf.key',
'ldap.conf',
'user.conf',
+ 'tfa.json',
'domains',
'mynetworks',
'transport',
diff --git a/src/PMG/UserConfig.pm b/src/PMG/UserConfig.pm
index 42a7d20..b9a83a7 100644
--- a/src/PMG/UserConfig.pm
+++ b/src/PMG/UserConfig.pm
@@ -2,8 +2,9 @@ package PMG::UserConfig;
use strict;
use warnings;
-use Data::Dumper;
+
use Clone 'clone';
+use Scalar::Util 'weaken';
use PVE::Tools;
use PVE::INotify;
@@ -15,6 +16,9 @@ use PMG::Utils;
my $inotify_file_id = 'pmg-user.conf';
my $config_filename = '/etc/pmg/user.conf';
+my $tfa_inotify_file_id = 'pmg-tfa.json';
+my $tfa_config_filename = '/etc/pmg/tfa.json';
+
sub new {
my ($type) = @_;
@@ -32,14 +36,40 @@ sub write {
}
my $lockfile = "/var/lock/pmguser.lck";
+my $tfa_lockfile = "/var/lock/pmgtfa.lck";
+# Locking both config files together is only ever allowed in one order:
+# 1) tfa config
+# 2) user config
+# If we permit the other way round, too, we might end up deadlocking!
+my $user_config_locked;
sub lock_config {
my ($code, $errmsg) = @_;
+ my $locked = 1;
+ $user_config_locked = \$locked;
+ weaken $user_config_locked; # make this scope guard signal safe...
+
my $p = PVE::Tools::lock_file($lockfile, undef, $code);
+ $user_config_locked = undef;
+ if (my $err = $@) {
+ $errmsg ? die "$errmsg: $err" : die $err;
+ }
+}
+
+# This lives here in order to enforce lock order.
+sub lock_tfa_config {
+ my ($code, $errmsg) = @_;
+
+ die "tfa config lock cannot be acquired while holding user config lock\n"
+ if ($user_config_locked && $$user_config_locked);
+
+ my $res = PVE::Tools::lock_file($tfa_lockfile, undef, $code);
if (my $err = $@) {
$errmsg ? die "$errmsg: $err" : die $err;
}
+
+ return $res;
}
my $schema = {
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH api 2/6] add PMG::TFAConfig module
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 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 3/6] add TFA API Wolfgang Bumiller
` (19 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
src/Makefile | 1 +
src/PMG/TFAConfig.pm | 80 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 81 insertions(+)
create mode 100644 src/PMG/TFAConfig.pm
diff --git a/src/Makefile b/src/Makefile
index eac682b..de05aa0 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -82,6 +82,7 @@ LIBSOURCES = \
PMG/Quarantine.pm \
PMG/Report.pm \
PMG/SACustom.pm \
+ PMG/TFAConfig.pm \
PMG/RuleDB/Group.pm \
PMG/RuleDB/Rule.pm \
PMG/RuleDB/Object.pm \
diff --git a/src/PMG/TFAConfig.pm b/src/PMG/TFAConfig.pm
new file mode 100644
index 0000000..998e266
--- /dev/null
+++ b/src/PMG/TFAConfig.pm
@@ -0,0 +1,80 @@
+package PMG::TFAConfig;
+
+use strict;
+use warnings;
+
+use PVE::Tools;
+use PVE::INotify;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Exception qw(raise);
+
+use PMG::Utils;
+use PMG::UserConfig;
+
+use base 'PMG::RS::TFA';
+
+my $inotify_file_id = 'pmg-tfa.json';
+my $config_filename = '/etc/pmg/tfa.json';
+
+sub new {
+ my ($type) = @_;
+
+ my $class = ref($type) || $type;
+
+ my $cfg = PVE::INotify::read_file($inotify_file_id);
+
+ return bless $cfg, $class;
+}
+
+sub write {
+ my ($self) = @_;
+
+ PVE::INotify::write_file($inotify_file_id, $self);
+}
+
+# This lives in `UserConfig` in order to enforce lock order.
+sub lock_config {
+ return PMG::UserConfig::lock_tfa_config(@_);
+}
+
+my sub read_tfa_conf : prototype($$) {
+ my ($filename, $fh) = @_;
+
+ my $raw;
+ if ($fh) {
+ $raw = do { local $/ = undef; <$fh> };
+ } else {
+ $raw = '{}';
+ }
+
+ my $cfg = PMG::RS::TFA->new($raw);
+
+ # Purge invalid users:
+ my $usercfg = PMG::UserConfig->new();
+ foreach my $user ($cfg->users()->@*) {
+ if (!$usercfg->lookup_user_data($user, 1)) {
+ $cfg->remove_user($user);
+ }
+ }
+
+ return $cfg;
+}
+
+my sub write_tfa_conf : prototype($$$) {
+ my ($filename, $fh, $cfg) = @_;
+
+ chmod(0600, $fh);
+
+ PVE::Tools::safe_print($filename, $fh, $cfg->SUPER::write());
+}
+
+PVE::INotify::register_file($inotify_file_id, $config_filename,
+ \&read_tfa_conf,
+ \&write_tfa_conf,
+ undef,
+ always_call_parser => 1,
+ # the parser produces a rust TfaConfig object,
+ # Clone::clone would break this
+ noclone => 1);
+
+1;
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH api 3/6] add TFA API
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 ` 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
` (18 subsequent siblings)
21 siblings, 1 reply; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
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.
+ 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',
+ },
+ 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;
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [pmg-devel] [PATCH api 3/6] add TFA API
2021-11-26 13:55 ` [pmg-devel] [PATCH api 3/6] add TFA API Wolfgang Bumiller
@ 2021-11-26 17:29 ` Stoiko Ivanov
0 siblings, 0 replies; 24+ messages in thread
From: Stoiko Ivanov @ 2021-11-26 17:29 UTC (permalink / raw)
To: Wolfgang Bumiller; +Cc: pmg-devel
two small notes inline:
On Fri, 26 Nov 2021 14:55:07 +0100
Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> 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;
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pmg-devel] [PATCH api 4/6] add tfa config api
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (2 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH api 3/6] add TFA API Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 5/6] implement tfa authentication Wolfgang Bumiller
` (17 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
src/Makefile | 1 +
src/PMG/API2/Config.pm | 6 ++
src/PMG/API2/TFAConfig.pm | 142 ++++++++++++++++++++++++++++++++++++++
3 files changed, 149 insertions(+)
create mode 100644 src/PMG/API2/TFAConfig.pm
diff --git a/src/Makefile b/src/Makefile
index c2bf2c9..f08be0f 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -149,6 +149,7 @@ LIBSOURCES = \
PMG/API2/Quarantine.pm \
PMG/API2/AccessControl.pm \
PMG/API2/TFA.pm \
+ PMG/API2/TFAConfig.pm \
PMG/API2/ObjectGroupHelpers.pm \
PMG/API2/Rules.pm \
PMG/API2/RuleDB.pm \
diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm
index c5697e1..19ae8f1 100644
--- a/src/PMG/API2/Config.pm
+++ b/src/PMG/API2/Config.pm
@@ -27,6 +27,7 @@ use PMG::API2::DKIMSign;
use PMG::API2::SACustom;
use PMG::API2::PBS::Remote;
use PMG::API2::ACME;
+use PMG::API2::TFAConfig;
use base qw(PVE::RESTHandler);
@@ -105,6 +106,11 @@ __PACKAGE__->register_method ({
path => 'acme',
});
+__PACKAGE__->register_method ({
+ subclass => "PMG::API2::TFAConfig",
+ path => 'tfa',
+});
+
__PACKAGE__->register_method ({
name => 'index',
path => '',
diff --git a/src/PMG/API2/TFAConfig.pm b/src/PMG/API2/TFAConfig.pm
new file mode 100644
index 0000000..dbe8969
--- /dev/null
+++ b/src/PMG/API2/TFAConfig.pm
@@ -0,0 +1,142 @@
+package PMG::API2::TFAConfig;
+
+use strict;
+use warnings;
+
+use PVE::Exception qw(raise raise_perm_exc raise_param_exc);
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::Tools qw(extract_param);
+
+use PMG::AccessControl;
+use PMG::RESTEnvironment;
+use PMG::TFAConfig;
+use PMG::UserConfig;
+use PMG::Utils;
+
+use base qw(PVE::RESTHandler);
+
+my $wa_config_schema = {
+ type => 'object',
+ properties => {
+ rp => {
+ type => 'string',
+ description =>
+ "Relying party name. Any text identifier.\n"
+ ."Changing this *may* break existing credentials.",
+ },
+ origin => {
+ type => 'string',
+ optional => 1,
+ description =>
+ 'Site origin. Must be a `https://` URL (or `http://localhost`).'
+ .' Should contain the address users type in their browsers to access the web'
+ ." interface.\n"
+ .'Changing this *may* break existing credentials.',
+ },
+ id => {
+ type => 'string',
+ description =>
+ "Relying part ID. Must be the domain name without protocol, port or location.\n"
+ .'Changing this *will* break existing credentials.',
+ },
+ },
+};
+
+my %return_properties = $wa_config_schema->{properties}->%*;
+$return_properties{$_}->{optional} = 1 for keys %return_properties;
+
+my $wa_config_return_schema = {
+ type => 'object',
+ properties => \%return_properties,
+};
+
+__PACKAGE__->register_method({
+ name => 'get_webauthn_config',
+ path => 'webauthn',
+ method => 'GET',
+ protected => 1,
+ permissions => { user => 'all' },
+ description => "Read the webauthn configuration.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => {
+ optional => 1,
+ $wa_config_schema->%*,
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $cfg = PMG::TFAConfig->new();
+ return $cfg->get_webauthn_config();
+ }});
+
+__PACKAGE__->register_method({
+ name => 'update_webauthn_config',
+ path => 'webauthn',
+ method => 'PUT',
+ protected => 1,
+ proxyto => 'master',
+ permissions => { check => [ 'admin' ] },
+ description => "Read the webauthn configuration.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ $wa_config_schema->{properties}->%*,
+ delete => {
+ type => 'string', enum => [keys $wa_config_schema->{properties}->%*],
+ description => "A list of settings you want to delete.",
+ optional => 1,
+ },
+ digest => {
+ type => 'string',
+ description => 'Prevent changes if current configuration file has different SHA1 digest.'
+ .' This can be used to prevent concurrent modifications.',
+ maxLength => 40,
+ optional => 1,
+ },
+ },
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ my $digest = extract_param($param, 'digest');
+ my $delete = extract_param($param, 'delete');
+
+ PMG::TFAConfig::lock_config(sub {
+ my $cfg = PMG::TFAConfig->new();
+
+ my ($config_digest, $wa) = $cfg->get_webauthn_config();
+ if (defined($digest)) {
+ PVE::Tools::assert_if_modified($digest, $config_digest);
+ }
+
+ foreach my $opt (PVE::Tools::split_list($delete)) {
+ delete $wa->{$opt};
+ }
+ foreach my $opt (keys %$param) {
+ my $value = $param->{$opt};
+ if (length($value)) {
+ $wa->{$opt} = $value;
+ } else {
+ delete $wa->{$opt};
+ }
+ }
+
+ # to remove completely, pass `undef`:
+ if (!%$wa) {
+ $wa = undef;
+ }
+
+ $cfg->set_webauthn_config($wa);
+
+ $cfg->write();
+ });
+
+ return;
+ }});
+
+1;
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH api 5/6] implement tfa authentication
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (3 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH api 4/6] add tfa config api Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH api 6/6] provide qrcode.min.js from libjs-qrcodejs Wolfgang Bumiller
` (16 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
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
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH api 6/6] provide qrcode.min.js from libjs-qrcodejs
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (4 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH api 5/6] implement tfa authentication Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH gui] add TFA components Wolfgang Bumiller
` (15 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
debian/control | 1 +
src/PMG/Service/pmgproxy.pm | 3 +++
2 files changed, 4 insertions(+)
diff --git a/debian/control b/debian/control
index c643e5a..8695996 100644
--- a/debian/control
+++ b/debian/control
@@ -55,6 +55,7 @@ Depends: apt (>= 2~),
libhtml-parser-perl,
libhtml-scrubber-perl,
libhtml-tree-perl,
+ libjs-qrcodejs (>= 1.20201119),
liblockfile-simple-perl,
libmail-spf-perl,
libmime-tools-perl,
diff --git a/src/PMG/Service/pmgproxy.pm b/src/PMG/Service/pmgproxy.pm
index 8a8a9d0..0efde23 100755
--- a/src/PMG/Service/pmgproxy.pm
+++ b/src/PMG/Service/pmgproxy.pm
@@ -118,6 +118,9 @@ sub init {
'/proxmoxlib.js' => {
file => '/usr/share/javascript/proxmox-widget-toolkit/proxmoxlib.js',
},
+ '/qrcode.min.js' => {
+ file => '/usr/share/javascript/qrcodejs/qrcode.min.js',
+ },
},
dirs => $dirs,
};
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH gui] add TFA components
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (5 preceding siblings ...)
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 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 1/7] pve: bump perlmod to 0.9 Wolfgang Bumiller
` (14 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
js/LoginView.js | 61 +++++++++++++++++++++++++++++++++-----------
js/Makefile | 1 +
js/UserManagement.js | 8 ++++--
js/UserSelector.js | 13 ++++++++++
pmg-index.html.tt | 1 +
5 files changed, 67 insertions(+), 17 deletions(-)
create mode 100644 js/UserSelector.js
diff --git a/js/LoginView.js b/js/LoginView.js
index 7ad695c..63f4099 100644
--- a/js/LoginView.js
+++ b/js/LoginView.js
@@ -44,7 +44,7 @@ Ext.define('PMG.LoginView', {
me.submitForm();
},
- submitForm: function() {
+ submitForm: async function() {
let me = this;
let view = me.getView();
let loginForm = me.lookupReference('loginForm');
@@ -67,23 +67,54 @@ Ext.define('PMG.LoginView', {
sp.set(saveunField.getStateId(), saveunField.getValue());
}
- loginForm.submit({
- success: function(form, action) {
- // save login data and create cookie
- PMG.Utils.updateLoginData(action.result.data);
- PMG.app.changeView(view.targetview);
- },
- failure: function(form, action) {
- loginForm.unmask();
- Ext.MessageBox.alert(
- gettext('Error'),
- gettext('Login failed. Please try again'),
- );
- },
- });
+ let creds = loginForm.getValues();
+
+ try {
+ let resp = await Proxmox.Async.api2({
+ url: '/api2/extjs/access/ticket',
+ params: creds,
+ method: 'POST',
+ });
+
+ let data = resp.result.data;
+ if (data.ticket.startsWith('PMG:!tfa!')) {
+ data = await me.performTFAChallenge(data);
+ }
+ PMG.Utils.updateLoginData(data);
+ PMG.app.changeView(view.targetview);
+ } catch (error) {
+ Proxmox.Utils.authClear();
+ loginForm.unmask();
+ Ext.MessageBox.alert(
+ gettext('Error'),
+ gettext('Login failed. Please try again'),
+ );
+ }
}
},
+ performTFAChallenge: async function(data) {
+ let me = this;
+
+ let userid = data.username;
+ let ticket = data.ticket;
+ let challenge = JSON.parse(decodeURIComponent(
+ ticket.split(':')[1].slice("!tfa!".length),
+ ));
+
+ let resp = await new Promise((resolve, reject) => {
+ Ext.create('Proxmox.window.TfaLoginWindow', {
+ userid,
+ ticket,
+ challenge,
+ onResolve: value => resolve(value),
+ onReject: reject,
+ }).show();
+ });
+
+ return resp.result.data;
+ },
+
openQuarantineLinkWindow: function() {
let me = this;
me.lookup('loginwindow').setVisible(false);
diff --git a/js/Makefile b/js/Makefile
index 672f61e..f4b7630 100644
--- a/js/Makefile
+++ b/js/Makefile
@@ -73,6 +73,7 @@ JSSRC= \
FetchmailEdit.js \
FetchmailView.js \
UserManagement.js \
+ UserSelector.js \
ViewMailHeaders.js \
PostfixQShape.js \
PostfixMailQueue.js \
diff --git a/js/UserManagement.js b/js/UserManagement.js
index 85e41e5..d81a4cc 100644
--- a/js/UserManagement.js
+++ b/js/UserManagement.js
@@ -27,7 +27,11 @@ Ext.define('PMG.UserManagement', {
itemId: 'pop',
iconCls: 'fa fa-reply-all',
},
+ {
+ xtype: 'pmxTfaView',
+ title: 'Two Factor',
+ itemId: 'tfa',
+ iconCls: 'fa fa-key',
+ },
],
});
-
-
diff --git a/js/UserSelector.js b/js/UserSelector.js
new file mode 100644
index 0000000..8fb31d7
--- /dev/null
+++ b/js/UserSelector.js
@@ -0,0 +1,13 @@
+Ext.define('pmx-users', {
+ extend: 'Ext.data.Model',
+ fields: [
+ 'userid', 'firstname', 'lastname', 'email', 'comment',
+ { type: 'boolean', name: 'enable' },
+ { type: 'date', dateFormat: 'timestamp', name: 'expire' },
+ ],
+ proxy: {
+ type: 'proxmox',
+ url: "/api2/json/access/users",
+ },
+ idProperty: 'userid',
+});
diff --git a/pmg-index.html.tt b/pmg-index.html.tt
index 4a29ba2..4e9f1af 100644
--- a/pmg-index.html.tt
+++ b/pmg-index.html.tt
@@ -24,6 +24,7 @@
[% ELSE %]
<script type="text/javascript" src="/pve2/ext6/ext-all.js"></script>
<script type="text/javascript" src="/pve2/ext6/charts.js"></script>
+ <script type="text/javascript" src="/qrcode.min.js"></script>
[% END %]
<script type="text/javascript">
Proxmox = {
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 1/7] pve: bump perlmod to 0.9
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (6 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH gui] add TFA components Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 2/7] pve: update to proxmox-tfa 2.0 Wolfgang Bumiller
` (13 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
pve-rs/Cargo.toml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
index 7f0bf05..f7cbd8a 100644
--- a/pve-rs/Cargo.toml
+++ b/pve-rs/Cargo.toml
@@ -26,7 +26,7 @@ serde = "1.0"
serde_bytes = "0.11"
serde_json = "1.0"
-perlmod = { version = "0.8.1", features = [ "exporter" ] }
+perlmod = { version = "0.9", features = [ "exporter" ] }
proxmox-apt = "0.8"
proxmox-openid = "0.9"
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 2/7] pve: update to proxmox-tfa 2.0
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (7 preceding siblings ...)
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 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 3/7] pve: bump d/control Wolfgang Bumiller
` (12 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
pve-rs/Cargo.toml | 3 ++-
pve-rs/debian/control | 4 ++--
pve-rs/src/tfa.rs | 24 +++++++++++++++++++-----
3 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
index f7cbd8a..74f45e3 100644
--- a/pve-rs/Cargo.toml
+++ b/pve-rs/Cargo.toml
@@ -25,9 +25,10 @@ openssl = "0.10"
serde = "1.0"
serde_bytes = "0.11"
serde_json = "1.0"
+url = "2"
perlmod = { version = "0.9", features = [ "exporter" ] }
proxmox-apt = "0.8"
proxmox-openid = "0.9"
-proxmox-tfa = { version = "1.3.2", features = ["api"] }
+proxmox-tfa = { version = "2", features = ["api"] }
diff --git a/pve-rs/debian/control b/pve-rs/debian/control
index 4988e33..62ab4cb 100644
--- a/pve-rs/debian/control
+++ b/pve-rs/debian/control
@@ -17,8 +17,8 @@ Build-Depends: debhelper (>= 12),
librust-perlmod-0.8+exporter-dev (>= 0.8.1-~~),
librust-proxmox-apt-0.8+default-dev,
librust-proxmox-openid-0.9+default-dev,
- librust-proxmox-tfa-1+api-dev (>= 1.3-~~),
- librust-proxmox-tfa-1+default-dev (>= 1.3-~~),
+ librust-proxmox-tfa-2+api-dev,
+ librust-proxmox-tfa-2+default-dev,
librust-serde-1+default-dev,
librust-serde-bytes-0.11+default-dev,
librust-serde-json-1+default-dev,
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index ecc5eb0..cc53118 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -31,6 +31,7 @@ mod export {
use anyhow::{bail, format_err, Error};
use serde_bytes::ByteBuf;
+ use url::Url;
use perlmod::Value;
use proxmox_tfa::api::methods;
@@ -243,10 +244,15 @@ mod export {
#[raw] raw_this: Value,
//#[try_from_ref] this: &Tfa,
userid: &str,
+ origin: Option<Url>,
) -> Result<Option<String>, Error> {
let this: &Tfa = (&raw_this).try_into()?;
let mut inner = this.inner.lock().unwrap();
- match inner.authentication_challenge(UserAccess::new(&raw_this)?, userid)? {
+ match inner.authentication_challenge(
+ UserAccess::new(&raw_this)?,
+ userid,
+ origin.as_ref(),
+ )? {
Some(challenge) => Ok(Some(serde_json::to_string(&challenge)?)),
None => Ok(None),
}
@@ -278,13 +284,20 @@ mod export {
userid: &str,
challenge: &str, //super::TfaChallenge,
response: &str,
+ origin: Option<Url>,
) -> Result<bool, Error> {
let this: &Tfa = (&raw_this).try_into()?;
let challenge: super::TfaChallenge = serde_json::from_str(challenge)?;
let response: super::TfaResponse = response.parse()?;
let mut inner = this.inner.lock().unwrap();
inner
- .verify(UserAccess::new(&raw_this)?, userid, &challenge, response)
+ .verify(
+ UserAccess::new(&raw_this)?,
+ userid,
+ &challenge,
+ response,
+ origin.as_ref(),
+ )
.map(|save| save.needs_saving())
}
@@ -342,6 +355,7 @@ mod export {
value: Option<String>,
challenge: Option<String>,
ty: methods::TfaType,
+ origin: Option<Url>,
) -> Result<methods::TfaUpdateInfo, Error> {
let this: &Tfa = (&raw_this).try_into()?;
methods::add_tfa_entry(
@@ -353,6 +367,7 @@ mod export {
value,
challenge,
ty,
+ origin.as_ref(),
)
}
@@ -864,11 +879,10 @@ impl proxmox_tfa::api::OpenUserChallengeData for UserAccess {
Err(err) => {
eprintln!(
"failed to parse challenge data for user {}: {}",
- userid,
- err
+ userid, err
);
Default::default()
- },
+ }
}
};
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 3/7] pve: bump d/control
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (8 preceding siblings ...)
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 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 4/7] import pmg-rs Wolfgang Bumiller
` (11 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
pve-rs/debian/control | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/pve-rs/debian/control b/pve-rs/debian/control
index 62ab4cb..3e9c3e9 100644
--- a/pve-rs/debian/control
+++ b/pve-rs/debian/control
@@ -13,8 +13,8 @@ Build-Depends: debhelper (>= 12),
librust-libc-0.2+default-dev,
librust-nix-0.19+default-dev,
librust-openssl-0.10+default-dev,
- librust-perlmod-0.8+default-dev (>= 0.8.1-~~),
- librust-perlmod-0.8+exporter-dev (>= 0.8.1-~~),
+ librust-perlmod-0.9+default-dev,
+ librust-perlmod-0.9+exporter-dev,
librust-proxmox-apt-0.8+default-dev,
librust-proxmox-openid-0.9+default-dev,
librust-proxmox-tfa-2+api-dev,
@@ -22,6 +22,7 @@ Build-Depends: debhelper (>= 12),
librust-serde-1+default-dev,
librust-serde-bytes-0.11+default-dev,
librust-serde-json-1+default-dev,
+ librust-url-2+default-dev,
Maintainer: Proxmox Support Team <support@proxmox.com>
Standards-Version: 4.5.1
Vcs-Git: git://git.proxmox.com/git/proxmox.git
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 4/7] import pmg-rs
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (9 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 3/7] pve: bump d/control Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 5/7] pmg: bump perlmod to 0.9 Wolfgang Bumiller
` (10 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
Cargo.toml | 1 +
Makefile | 7 +-
pmg-rs/Cargo.toml | 33 +++
pmg-rs/Makefile | 75 ++++++
pmg-rs/debian/changelog | 50 ++++
pmg-rs/debian/compat | 1 +
pmg-rs/debian/control | 27 +++
pmg-rs/debian/copyright | 16 ++
pmg-rs/debian/debcargo.toml | 10 +
pmg-rs/debian/rules | 7 +
pmg-rs/debian/source/format | 1 +
pmg-rs/debian/triggers | 1 +
pmg-rs/src/acme.rs | 430 +++++++++++++++++++++++++++++++++
pmg-rs/src/apt/mod.rs | 1 +
pmg-rs/src/apt/repositories.rs | 162 +++++++++++++
pmg-rs/src/csr.rs | 24 ++
pmg-rs/src/lib.rs | 3 +
pmg-rs/test.pl | 172 +++++++++++++
18 files changed, 1018 insertions(+), 3 deletions(-)
create mode 100644 pmg-rs/Cargo.toml
create mode 100644 pmg-rs/Makefile
create mode 100644 pmg-rs/debian/changelog
create mode 100644 pmg-rs/debian/compat
create mode 100644 pmg-rs/debian/control
create mode 100644 pmg-rs/debian/copyright
create mode 100644 pmg-rs/debian/debcargo.toml
create mode 100755 pmg-rs/debian/rules
create mode 100644 pmg-rs/debian/source/format
create mode 100644 pmg-rs/debian/triggers
create mode 100644 pmg-rs/src/acme.rs
create mode 100644 pmg-rs/src/apt/mod.rs
create mode 100644 pmg-rs/src/apt/repositories.rs
create mode 100644 pmg-rs/src/csr.rs
create mode 100644 pmg-rs/src/lib.rs
create mode 100644 pmg-rs/test.pl
diff --git a/Cargo.toml b/Cargo.toml
index 8556b45..6b869d4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -2,6 +2,7 @@
exclude = [ "build", "perl-*" ]
members = [
"pve-rs",
+ "pmg-rs",
]
[patch.crates-io]
diff --git a/Makefile b/Makefile
index f8dd85a..2cc02fe 100644
--- a/Makefile
+++ b/Makefile
@@ -28,14 +28,15 @@ build:
echo system >build/rust-toolchain
cp -a ./perl-* ./build/
cp -a ./pve-rs ./build
+ cp -a ./pmg-rs ./build
pve-deb: build
cd ./build/pve-rs && dpkg-buildpackage -b -uc -us
touch $@
-# pmg-deb: build
-# cd ./build/pmg-rs && dpkg-buildpackage -b -uc -us
-# touch $@
+pmg-deb: build
+ cd ./build/pmg-rs && dpkg-buildpackage -b -uc -us
+ touch $@
%-upload: %-deb
cd build; \
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
new file mode 100644
index 0000000..02f59de
--- /dev/null
+++ b/pmg-rs/Cargo.toml
@@ -0,0 +1,33 @@
+[package]
+name = "pmg-rs"
+version = "0.3.2"
+authors = [
+ "Proxmox Support Team <support@proxmox.com>",
+ "Wolfgang Bumiller <w.bumiller@proxmox.com>",
+ "Fabian Ebner <f.ebner@proxmox.com>",
+]
+edition = "2018"
+license = "AGPL-3"
+description = "PMG parts which have been ported to rust"
+exclude = [
+ "build",
+ "debian",
+ "PMG",
+]
+
+[lib]
+crate-type = [ "cdylib" ]
+
+[dependencies]
+anyhow = "1.0"
+hex = "0.4"
+openssl = "0.10.32"
+serde = "1.0"
+serde_bytes = "0.11.3"
+serde_json = "1.0"
+
+perlmod = { version = "0.8.1", features = [ "exporter" ] }
+
+proxmox-acme-rs = { version = "0.3.1", features = ["client"] }
+
+proxmox-apt = "0.8.0"
diff --git a/pmg-rs/Makefile b/pmg-rs/Makefile
new file mode 100644
index 0000000..a290544
--- /dev/null
+++ b/pmg-rs/Makefile
@@ -0,0 +1,75 @@
+include /usr/share/dpkg/default.mk
+
+PACKAGE=libpmg-rs-perl
+
+ARCH:=$(shell dpkg-architecture -qDEB_BUILD_ARCH)
+export GITVERSION:=$(shell git rev-parse HEAD)
+
+PERL_INSTALLVENDORARCH != perl -MConfig -e 'print $$Config{installvendorarch};'
+PERL_INSTALLVENDORLIB != perl -MConfig -e 'print $$Config{installvendorlib};'
+
+MAIN_DEB=${PACKAGE}_${DEB_VERSION}_${ARCH}.deb
+DBGSYM_DEB=${PACKAGE}-dbgsym_${DEB_VERSION}_${ARCH}.deb
+DEBS=$(MAIN_DEB) $(DBGSYM_DEB)
+
+DESTDIR=
+
+PM_DIRS := \
+ PMG/RS/APT
+
+PM_FILES := \
+ PMG/RS/Acme.pm \
+ PMG/RS/APT/Repositories.pm \
+ PMG/RS/CSR.pm
+
+ifeq ($(BUILD_MODE), release)
+CARGO_BUILD_ARGS += --release
+endif
+
+all:
+ifneq ($(BUILD_MODE), skip)
+ cargo build $(CARGO_BUILD_ARGS)
+endif
+
+# always re-create this dir
+# but also copy the local target/ and PMG/ dirs as a build-cache
+.PHONY: build
+build:
+ rm -rf build
+ cargo build --release
+ rsync -a debian Makefile Cargo.toml Cargo.lock src target PMG build/
+
+.PHONY: install
+install: target/release/libpmg_rs.so
+ install -d -m755 $(DESTDIR)$(PERL_INSTALLVENDORARCH)/auto
+ install -m644 target/release/libpmg_rs.so $(DESTDIR)$(PERL_INSTALLVENDORARCH)/auto/libpmg_rs.so
+ install -d -m755 $(DESTDIR)$(PERL_INSTALLVENDORLIB)/PMG/RS
+ for i in $(PM_DIRS); do \
+ install -d -m755 $(DESTDIR)$(PERL_INSTALLVENDORLIB)/$$i; \
+ done
+ for i in $(PM_FILES); do \
+ install -m644 $$i $(DESTDIR)$(PERL_INSTALLVENDORLIB)/$$i; \
+ done
+
+.PHONY: deb
+deb: $(MAIN_DEB)
+$(MAIN_DEB): build
+ cd build; dpkg-buildpackage -b -us -uc --no-pre-clean
+ lintian $(DEBS)
+
+distclean: clean
+
+clean:
+ cargo clean
+ rm -rf *.deb *.dsc *.tar.gz *.buildinfo *.changes Cargo.lock build
+ find . -name '*~' -exec rm {} ';'
+
+.PHONY: dinstall
+dinstall: ${DEBS}
+ dpkg -i ${DEBS}
+
+.PHONY: upload
+upload: ${DEBS}
+ # check if working directory is clean
+ git diff --exit-code --stat && git diff --exit-code --stat --staged
+ tar cf - ${DEBS} | ssh -X repoman@repo.proxmox.com upload --product pmg --dist bullseye
diff --git a/pmg-rs/debian/changelog b/pmg-rs/debian/changelog
new file mode 100644
index 0000000..afc3e60
--- /dev/null
+++ b/pmg-rs/debian/changelog
@@ -0,0 +1,50 @@
+libpmg-rs-perl (0.3.2) bullseye; urgency=medium
+
+ * acme: add proxy support
+
+ -- Proxmox Support Team <support@proxmox.com> Thu, 18 Nov 2021 11:18:01 +0100
+
+libpmg-rs-perl (0.3.1) bullseye; urgency=medium
+
+ * update to proxmox-acme-rs 0.3
+
+ -- Proxmox Support Team <support@proxmox.com> Thu, 21 Oct 2021 13:13:46 +0200
+
+libpmg-rs-perl (0.3.0) bullseye; urgency=medium
+
+ * update proxmox-apt to 0.6.0
+
+ -- Proxmox Support Team <support@proxmox.com> Fri, 30 Jul 2021 10:56:35 +0200
+
+libpmg-rs-perl (0.2.0-1) bullseye; urgency=medium
+
+ * add bindings for proxmox-apt
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 13 Jul 2021 12:48:04 +0200
+
+libpmg-rs-perl (0.1.3-1) bullseye; urgency=medium
+
+ * re-build for Proxmox Mail Gateway 7 / Debian 11 Bullseye
+
+ -- Proxmox Support Team <support@proxmox.com> Thu, 27 May 2021 19:58:08 +0200
+
+libpmg-rs-perl (0.1.2-1) buster; urgency=medium
+
+ * update proxmox-acme-rs to 0.1.4 to store the 'created' account field if it
+ is available
+
+ * set account file permission to 0700
+
+ -- Proxmox Support Team <support@proxmox.com> Mon, 29 Mar 2021 11:22:54 +0200
+
+libpmg-rs-perl (0.1.1-1) unstable; urgency=medium
+
+ * update proxmox-acme-rs to 0.1.3 to fix ecsda signature padding
+
+ -- Proxmox Support Team <support@proxmox.com> Wed, 17 Mar 2021 13:43:12 +0100
+
+libpmg-rs-perl (0.1-1) unstable; urgency=medium
+
+ * initial release
+
+ -- Proxmox Support Team <support@proxmox.com> Mon, 22 Feb 2021 13:40:10 +0100
diff --git a/pmg-rs/debian/compat b/pmg-rs/debian/compat
new file mode 100644
index 0000000..48082f7
--- /dev/null
+++ b/pmg-rs/debian/compat
@@ -0,0 +1 @@
+12
diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
new file mode 100644
index 0000000..be632ba
--- /dev/null
+++ b/pmg-rs/debian/control
@@ -0,0 +1,27 @@
+Source: libpmg-rs-perl
+Section: perl
+Priority: optional
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Build-Depends:
+ debhelper (>= 12),
+ librust-anyhow-1+default-dev,
+ librust-hex-0.4+default-dev,
+ librust-openssl-0.10+default-dev (>= 0.10.32-~~),
+ librust-perlmod-0.8+default-dev,
+ librust-perlmod-0.8+exporter-dev,
+ librust-proxmox-acme-rs-0.3+client-dev (>= 0.3.1-~~),
+ librust-proxmox-acme-rs-0.3+default-dev (>= 0.3.1-~~),
+ librust-proxmox-apt-0.8+default-dev,
+ librust-serde-1+default-dev,
+ librust-serde-bytes-0.11+default-dev (>= 0.11.3-~~),
+ librust-serde-json-1+default-dev,
+Standards-Version: 4.3.0
+Homepage: https://www.proxmox.com
+
+Package: libpmg-rs-perl
+Architecture: any
+Depends: ${perl:Depends},
+ ${shlibs:Depends},
+Description: Components of Proxmox Mail Gateway which have been ported to Rust.
+ Contains parts of Proxmox Mail Gateway which have been ported to, or newly
+ implemented in the Rust programming language.
diff --git a/pmg-rs/debian/copyright b/pmg-rs/debian/copyright
new file mode 100644
index 0000000..477c305
--- /dev/null
+++ b/pmg-rs/debian/copyright
@@ -0,0 +1,16 @@
+Copyright (C) 2020-2021 Proxmox Server Solutions GmbH
+
+This software is written by Proxmox Server Solutions GmbH <support@proxmox.com>
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program. If not, see <http://www.gnu.org/licenses/>.
diff --git a/pmg-rs/debian/debcargo.toml b/pmg-rs/debian/debcargo.toml
new file mode 100644
index 0000000..8aa085f
--- /dev/null
+++ b/pmg-rs/debian/debcargo.toml
@@ -0,0 +1,10 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+section = "perl"
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
+
+[packages.libpmg-rs-perl]
diff --git a/pmg-rs/debian/rules b/pmg-rs/debian/rules
new file mode 100755
index 0000000..0f5be05
--- /dev/null
+++ b/pmg-rs/debian/rules
@@ -0,0 +1,7 @@
+#!/usr/bin/make -f
+
+#export DH_VERBOSE=1
+export BUILD_MODE=release
+
+%:
+ dh $@
diff --git a/pmg-rs/debian/source/format b/pmg-rs/debian/source/format
new file mode 100644
index 0000000..89ae9db
--- /dev/null
+++ b/pmg-rs/debian/source/format
@@ -0,0 +1 @@
+3.0 (native)
diff --git a/pmg-rs/debian/triggers b/pmg-rs/debian/triggers
new file mode 100644
index 0000000..59dd688
--- /dev/null
+++ b/pmg-rs/debian/triggers
@@ -0,0 +1 @@
+activate-noawait pve-api-updates
diff --git a/pmg-rs/src/acme.rs b/pmg-rs/src/acme.rs
new file mode 100644
index 0000000..0429a0d
--- /dev/null
+++ b/pmg-rs/src/acme.rs
@@ -0,0 +1,430 @@
+//! `PMG::RS::Acme` perl module.
+//!
+//! The functions in here are perl bindings.
+
+use std::fs::OpenOptions;
+use std::io::{self, Write};
+use std::os::unix::fs::OpenOptionsExt;
+
+use anyhow::{format_err, Error};
+use serde::{Deserialize, Serialize};
+
+use proxmox_acme_rs::account::AccountData as AcmeAccountData;
+use proxmox_acme_rs::{Account, Client};
+
+/// Our on-disk format inherited from PVE's proxmox-acme code.
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct AccountData {
+ /// The account's location URL.
+ location: String,
+
+ /// The account dat.
+ account: AcmeAccountData,
+
+ /// The private key as PEM formatted string.
+ key: String,
+
+ /// ToS URL the user agreed to.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ tos: Option<String>,
+
+ #[serde(skip_serializing_if = "is_false", default)]
+ debug: bool,
+
+ /// The directory's URL.
+ directory_url: String,
+}
+
+#[inline]
+fn is_false(b: &bool) -> bool {
+ !*b
+}
+
+struct Inner {
+ client: Client,
+ account_path: Option<String>,
+ tos: Option<String>,
+ debug: bool,
+}
+
+impl Inner {
+ pub fn new(api_directory: String) -> Result<Self, Error> {
+ Ok(Self {
+ client: Client::new(api_directory),
+ account_path: None,
+ tos: None,
+ debug: false,
+ })
+ }
+
+ pub fn load(account_path: String) -> Result<Self, Error> {
+ let data = std::fs::read(&account_path)?;
+ let data: AccountData = serde_json::from_slice(&data)?;
+
+ let mut client = Client::new(data.directory_url);
+ client.set_account(Account::from_parts(data.location, data.key, data.account));
+
+ Ok(Self {
+ client,
+ account_path: Some(account_path),
+ tos: data.tos,
+ debug: data.debug,
+ })
+ }
+
+ pub fn new_account(
+ &mut self,
+ account_path: String,
+ tos_agreed: bool,
+ contact: Vec<String>,
+ rsa_bits: Option<u32>,
+ ) -> Result<(), Error> {
+ self.tos = if tos_agreed {
+ self.client.terms_of_service_url()?.map(str::to_owned)
+ } else {
+ None
+ };
+
+ let _account = self.client.new_account(contact, tos_agreed, rsa_bits)?;
+ let file = OpenOptions::new()
+ .write(true)
+ .create(true)
+ .mode(0o600)
+ .open(&account_path)
+ .map_err(|err| format_err!("failed to open {:?} for writing: {}", account_path, err))?;
+ self.write_to(file).map_err(|err| {
+ format_err!(
+ "failed to write acme account to {:?}: {}",
+ account_path,
+ err
+ )
+ })?;
+ self.account_path = Some(account_path);
+
+ Ok(())
+ }
+
+ /// Convenience helper around `.client.account().ok_or_else(||...)`
+ fn account(&self) -> Result<&Account, Error> {
+ self.client
+ .account()
+ .ok_or_else(|| format_err!("missing account"))
+ }
+
+ fn to_account_data(&self) -> Result<AccountData, Error> {
+ let account = self.account()?;
+
+ Ok(AccountData {
+ location: account.location.clone(),
+ key: account.private_key.clone(),
+ account: AcmeAccountData {
+ only_return_existing: false, // don't actually write this out in case it's set
+ ..account.data.clone()
+ },
+ tos: self.tos.clone(),
+ debug: self.debug,
+ directory_url: self.client.directory_url().to_owned(),
+ })
+ }
+
+ fn write_to<T: io::Write>(&mut self, out: T) -> Result<(), Error> {
+ let data = self.to_account_data()?;
+
+ Ok(serde_json::to_writer_pretty(out, &data)?)
+ }
+
+ pub fn update_account<T: Serialize>(&mut self, data: &T) -> Result<(), Error> {
+ let account_path = self
+ .account_path
+ .as_deref()
+ .ok_or_else(|| format_err!("missing account path"))?;
+ self.client.update_account(data)?;
+
+ let tmp_path = format!("{}.tmp", account_path);
+ // FIXME: move proxmox::tools::replace_file & make_temp out into a nice *little* crate...
+ let mut file = OpenOptions::new()
+ .write(true)
+ .create(true)
+ .mode(0o600)
+ .open(&tmp_path)
+ .map_err(|err| format_err!("failed to open {:?} for writing: {}", tmp_path, err))?;
+ self.write_to(&mut file).map_err(|err| {
+ format_err!("failed to write acme account to {:?}: {}", tmp_path, err)
+ })?;
+ file.flush().map_err(|err| {
+ format_err!("failed to flush acme account file {:?}: {}", tmp_path, err)
+ })?;
+
+ // re-borrow since we needed `self` as mut earlier
+ let account_path = self.account_path.as_deref().unwrap();
+ std::fs::rename(&tmp_path, account_path).map_err(|err| {
+ format_err!(
+ "failed to rotate temp file into place ({:?} -> {:?}): {}",
+ &tmp_path,
+ account_path,
+ err
+ )
+ })?;
+ drop(file);
+ Ok(())
+ }
+
+ pub fn revoke_certificate(&mut self, data: &[u8], reason: Option<u32>) -> Result<(), Error> {
+ Ok(self.client.revoke_certificate(data, reason)?)
+ }
+
+ pub fn set_proxy(&mut self, proxy: String) {
+ self.client.set_proxy(proxy)
+ }
+}
+
+#[perlmod::package(name = "PMG::RS::Acme", lib = "pmg_rs")]
+pub mod export {
+ use std::collections::HashMap;
+ use std::convert::TryFrom;
+ use std::sync::Mutex;
+
+ use anyhow::Error;
+ use serde_bytes::{ByteBuf, Bytes};
+
+ use perlmod::Value;
+ use proxmox_acme_rs::directory::Meta;
+ use proxmox_acme_rs::order::OrderData;
+ use proxmox_acme_rs::{Authorization, Challenge, Order};
+
+ use super::{AccountData, Inner};
+
+ const CLASSNAME: &str = "PMG::RS::Acme";
+
+ /// An Acme client instance.
+ pub struct Acme {
+ inner: Mutex<Inner>,
+ }
+
+ impl<'a> TryFrom<&'a Value> for &'a Acme {
+ type Error = Error;
+
+ fn try_from(value: &'a Value) -> Result<&'a Acme, Error> {
+ Ok(unsafe { value.from_blessed_box(CLASSNAME)? })
+ }
+ }
+
+ fn bless(class: Value, mut ptr: Box<Acme>) -> Result<Value, Error> {
+ let value = Value::new_pointer::<Acme>(&mut *ptr);
+ let value = Value::new_ref(&value);
+ let this = value.bless_sv(&class)?;
+ let _perl = Box::leak(ptr);
+ Ok(this)
+ }
+
+ /// Create a new ACME client instance given an account path and an API directory URL.
+ #[export(raw_return)]
+ pub fn new(#[raw] class: Value, api_directory: String) -> Result<Value, Error> {
+ bless(
+ class,
+ Box::new(Acme {
+ inner: Mutex::new(Inner::new(api_directory)?),
+ }),
+ )
+ }
+
+ /// Load an existing account.
+ #[export(raw_return)]
+ pub fn load(#[raw] class: Value, account_path: String) -> Result<Value, Error> {
+ bless(
+ class,
+ Box::new(Acme {
+ inner: Mutex::new(Inner::load(account_path)?),
+ }),
+ )
+ }
+
+ #[export(name = "DESTROY")]
+ fn destroy(#[raw] this: Value) {
+ perlmod::destructor!(this, Acme: CLASSNAME);
+ }
+
+ /// Create a new account.
+ ///
+ /// `tos_agreed` is usually not optional, but may be set later via an update.
+ /// The `contact` list should be a list of `mailto:` strings (or others, if the directory
+ /// allows the).
+ ///
+ /// In case an RSA key should be generated, an `rsa_bits` parameter should be provided.
+ /// Otherwise a P-256 EC key will be generated.
+ #[export]
+ pub fn new_account(
+ #[try_from_ref] this: &Acme,
+ account_path: String,
+ tos_agreed: bool,
+ contact: Vec<String>,
+ rsa_bits: Option<u32>,
+ ) -> Result<(), Error> {
+ this.inner
+ .lock()
+ .unwrap()
+ .new_account(account_path, tos_agreed, contact, rsa_bits)
+ }
+
+ /// Get the directory's meta information.
+ #[export]
+ pub fn get_meta(#[try_from_ref] this: &Acme) -> Result<Option<Meta>, Error> {
+ match this.inner.lock().unwrap().client.directory()?.meta() {
+ Some(meta) => Ok(Some(meta.clone())),
+ None => Ok(None),
+ }
+ }
+
+ /// Get the account's directory URL.
+ #[export]
+ pub fn directory(#[try_from_ref] this: &Acme) -> Result<String, Error> {
+ Ok(this.inner.lock().unwrap().client.directory()?.url.clone())
+ }
+
+ /// Serialize the account data.
+ #[export]
+ pub fn account(#[try_from_ref] this: &Acme) -> Result<AccountData, Error> {
+ this.inner.lock().unwrap().to_account_data()
+ }
+
+ /// Get the account's location URL.
+ #[export]
+ pub fn location(#[try_from_ref] this: &Acme) -> Result<String, Error> {
+ Ok(this.inner.lock().unwrap().account()?.location.clone())
+ }
+
+ /// Get the account's agreed-to ToS URL.
+ #[export]
+ pub fn tos_url(#[try_from_ref] this: &Acme) -> Option<String> {
+ this.inner.lock().unwrap().tos.clone()
+ }
+
+ /// Get the debug flag.
+ #[export]
+ pub fn debug(#[try_from_ref] this: &Acme) -> bool {
+ this.inner.lock().unwrap().debug
+ }
+
+ /// Get the debug flag.
+ #[export]
+ pub fn set_debug(#[try_from_ref] this: &Acme, on: bool) {
+ this.inner.lock().unwrap().debug = on;
+ }
+
+ /// Place a new order.
+ #[export]
+ pub fn new_order(
+ #[try_from_ref] this: &Acme,
+ domains: Vec<String>,
+ ) -> Result<(String, OrderData), Error> {
+ let order: Order = this.inner.lock().unwrap().client.new_order(domains)?;
+ Ok((order.location, order.data))
+ }
+
+ /// Get the authorization info given an authorization URL.
+ ///
+ /// This should be an URL found in the `authorizations` array in the `OrderData` returned from
+ /// `new_order`.
+ #[export]
+ pub fn get_authorization(
+ #[try_from_ref] this: &Acme,
+ url: &str,
+ ) -> Result<Authorization, Error> {
+ Ok(this.inner.lock().unwrap().client.get_authorization(url)?)
+ }
+
+ /// Query an order given its URL.
+ ///
+ /// The corresponding URL is returned as first value from the `new_order` call.
+ #[export]
+ pub fn get_order(#[try_from_ref] this: &Acme, url: &str) -> Result<OrderData, Error> {
+ Ok(this.inner.lock().unwrap().client.get_order(url)?)
+ }
+
+ /// Get the key authorization string for a challenge given a token.
+ #[export]
+ pub fn key_authorization(#[try_from_ref] this: &Acme, token: &str) -> Result<String, Error> {
+ Ok(this.inner.lock().unwrap().client.key_authorization(token)?)
+ }
+
+ /// Get the key dns-01 TXT challenge value for a token.
+ #[export]
+ pub fn dns_01_txt_value(#[try_from_ref] this: &Acme, token: &str) -> Result<String, Error> {
+ Ok(this.inner.lock().unwrap().client.dns_01_txt_value(token)?)
+ }
+
+ /// Request validation of a challenge by URL.
+ ///
+ /// Given an `Authorization`, it'll contain `challenges`. These contain `url`s pointing to a
+ /// method used to request challenge authorization. This is the URL used for this method,
+ /// *after* performing the necessary steps to satisfy the challenge. (Eg. after setting up a
+ /// DNS TXT entry using the `dns-01` type challenge's key authorization.
+ #[export]
+ pub fn request_challenge_validation(
+ #[try_from_ref] this: &Acme,
+ url: &str,
+ ) -> Result<Challenge, Error> {
+ Ok(this
+ .inner
+ .lock()
+ .unwrap()
+ .client
+ .request_challenge_validation(url)?)
+ }
+
+ /// Request finalization of an order.
+ ///
+ /// The `url` should be the 'finalize' URL of the order.
+ #[export]
+ pub fn finalize_order(
+ #[try_from_ref] this: &Acme,
+ url: &str,
+ csr: &Bytes,
+ ) -> Result<(), Error> {
+ Ok(this.inner.lock().unwrap().client.finalize(url, csr)?)
+ }
+
+ /// Download the certificate for an order.
+ ///
+ /// The `url` should be the 'certificate' URL of the order.
+ #[export]
+ pub fn get_certificate(#[try_from_ref] this: &Acme, url: &str) -> Result<ByteBuf, Error> {
+ Ok(ByteBuf::from(
+ this.inner.lock().unwrap().client.get_certificate(url)?,
+ ))
+ }
+
+ /// Update account data.
+ ///
+ /// This can be used for example to deactivate an account or agree to ToS later on.
+ #[export]
+ pub fn update_account(
+ #[try_from_ref] this: &Acme,
+ data: HashMap<String, serde_json::Value>,
+ ) -> Result<(), Error> {
+ this.inner.lock().unwrap().update_account(&data)?;
+ Ok(())
+ }
+
+ /// Revoke an existing certificate using the certificate in PEM or DER form.
+ #[export]
+ pub fn revoke_certificate(
+ #[try_from_ref] this: &Acme,
+ data: &[u8],
+ reason: Option<u32>,
+ ) -> Result<(), Error> {
+ this.inner
+ .lock()
+ .unwrap()
+ .revoke_certificate(&data, reason)?;
+ Ok(())
+ }
+
+ /// Set a proxy
+ #[export]
+ pub fn set_proxy(#[try_from_ref] this: &Acme, proxy: String) {
+ this.inner.lock().unwrap().set_proxy(proxy)
+ }
+
+}
diff --git a/pmg-rs/src/apt/mod.rs b/pmg-rs/src/apt/mod.rs
new file mode 100644
index 0000000..574c1a7
--- /dev/null
+++ b/pmg-rs/src/apt/mod.rs
@@ -0,0 +1 @@
+mod repositories;
diff --git a/pmg-rs/src/apt/repositories.rs b/pmg-rs/src/apt/repositories.rs
new file mode 100644
index 0000000..75207c7
--- /dev/null
+++ b/pmg-rs/src/apt/repositories.rs
@@ -0,0 +1,162 @@
+#[perlmod::package(name = "PMG::RS::APT::Repositories", lib = "pmg_rs")]
+mod export {
+ use std::convert::TryInto;
+
+ use anyhow::{bail, Error};
+ use serde::{Deserialize, Serialize};
+
+ use proxmox_apt::repositories::{
+ APTRepositoryFile, APTRepositoryFileError, APTRepositoryHandle, APTRepositoryInfo,
+ APTStandardRepository,
+ };
+
+ #[derive(Deserialize, Serialize)]
+ #[serde(rename_all = "kebab-case")]
+ /// Result for the repositories() function
+ pub struct RepositoriesResult {
+ /// Successfully parsed files.
+ pub files: Vec<APTRepositoryFile>,
+
+ /// Errors for files that could not be parsed or read.
+ pub errors: Vec<APTRepositoryFileError>,
+
+ /// Common digest for successfully parsed files.
+ pub digest: String,
+
+ /// Additional information/warnings about repositories.
+ pub infos: Vec<APTRepositoryInfo>,
+
+ /// Standard repositories and their configuration status.
+ pub standard_repos: Vec<APTStandardRepository>,
+ }
+
+ #[derive(Deserialize, Serialize)]
+ #[serde(rename_all = "kebab-case")]
+ /// For changing an existing repository.
+ pub struct ChangeProperties {
+ /// Whether the repository should be enabled or not.
+ pub enabled: Option<bool>,
+ }
+
+ /// Get information about configured and standard repositories.
+ #[export]
+ pub fn repositories() -> Result<RepositoriesResult, Error> {
+ let (files, errors, digest) = proxmox_apt::repositories::repositories()?;
+ let digest = hex::encode(&digest);
+
+ let suite = proxmox_apt::repositories::get_current_release_codename()?;
+
+ let infos = proxmox_apt::repositories::check_repositories(&files, suite);
+ let standard_repos = proxmox_apt::repositories::standard_repositories(&files, "pmg", suite);
+
+ Ok(RepositoriesResult {
+ files,
+ errors,
+ digest,
+ infos,
+ standard_repos,
+ })
+ }
+
+ /// Add the repository identified by the `handle`.
+ /// If the repository is already configured, it will be set to enabled.
+ ///
+ /// The `digest` parameter asserts that the configuration has not been modified.
+ #[export]
+ pub fn add_repository(handle: &str, digest: Option<&str>) -> Result<(), Error> {
+ let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
+
+ let handle: APTRepositoryHandle = handle.try_into()?;
+ let suite = proxmox_apt::repositories::get_current_release_codename()?;
+
+ if let Some(digest) = digest {
+ let expected_digest = hex::decode(digest)?;
+ if expected_digest != current_digest {
+ bail!("detected modified configuration - file changed by other user? Try again.");
+ }
+ }
+
+ // check if it's already configured first
+ for file in files.iter_mut() {
+ for repo in file.repositories.iter_mut() {
+ if repo.is_referenced_repository(handle, "pmg", &suite.to_string()) {
+ if repo.enabled {
+ return Ok(());
+ }
+
+ repo.set_enabled(true);
+ file.write()?;
+
+ return Ok(());
+ }
+ }
+ }
+
+ let (repo, path) = proxmox_apt::repositories::get_standard_repository(handle, "pmg", suite);
+
+ if let Some(error) = errors.iter().find(|error| error.path == path) {
+ bail!(
+ "unable to parse existing file {} - {}",
+ error.path,
+ error.error,
+ );
+ }
+
+ if let Some(file) = files.iter_mut().find(|file| file.path == path) {
+ file.repositories.push(repo);
+
+ file.write()?;
+ } else {
+ let mut file = match APTRepositoryFile::new(&path)? {
+ Some(file) => file,
+ None => bail!("invalid path - {}", path),
+ };
+
+ file.repositories.push(repo);
+
+ file.write()?;
+ }
+
+ Ok(())
+ }
+
+ /// Change the properties of the specified repository.
+ ///
+ /// The `digest` parameter asserts that the configuration has not been modified.
+ #[export]
+ pub fn change_repository(
+ path: &str,
+ index: usize,
+ options: ChangeProperties,
+ digest: Option<&str>,
+ ) -> Result<(), Error> {
+ let (mut files, errors, current_digest) = proxmox_apt::repositories::repositories()?;
+
+ if let Some(digest) = digest {
+ let expected_digest = hex::decode(digest)?;
+ if expected_digest != current_digest {
+ bail!("detected modified configuration - file changed by other user? Try again.");
+ }
+ }
+
+ if let Some(error) = errors.iter().find(|error| error.path == path) {
+ bail!("unable to parse file {} - {}", error.path, error.error);
+ }
+
+ if let Some(file) = files.iter_mut().find(|file| file.path == path) {
+ if let Some(repo) = file.repositories.get_mut(index) {
+ if let Some(enabled) = options.enabled {
+ repo.set_enabled(enabled);
+ }
+
+ file.write()?;
+ } else {
+ bail!("invalid index - {}", index);
+ }
+ } else {
+ bail!("invalid path - {}", path);
+ }
+
+ Ok(())
+ }
+}
diff --git a/pmg-rs/src/csr.rs b/pmg-rs/src/csr.rs
new file mode 100644
index 0000000..961a2cf
--- /dev/null
+++ b/pmg-rs/src/csr.rs
@@ -0,0 +1,24 @@
+#[perlmod::package(name = "PMG::RS::CSR", lib = "pmg_rs")]
+pub mod export {
+ use std::collections::HashMap;
+
+ use anyhow::Error;
+ use serde_bytes::ByteBuf;
+
+ use proxmox_acme_rs::util::Csr;
+
+ /// Generates a CSR and its accompanying private key.
+ ///
+ /// The CSR is DER formatted, the private key is a PEM formatted pkcs8 private key.
+ #[export]
+ pub fn generate_csr(
+ identifiers: Vec<&str>,
+ attributes: HashMap<String, &str>,
+ ) -> Result<(ByteBuf, ByteBuf), Error> {
+ let csr = Csr::generate(&identifiers, &attributes)?;
+ Ok((
+ ByteBuf::from(csr.data),
+ ByteBuf::from(csr.private_key_pem),
+ ))
+ }
+}
diff --git a/pmg-rs/src/lib.rs b/pmg-rs/src/lib.rs
new file mode 100644
index 0000000..47e61b5
--- /dev/null
+++ b/pmg-rs/src/lib.rs
@@ -0,0 +1,3 @@
+pub mod acme;
+pub mod apt;
+pub mod csr;
diff --git a/pmg-rs/test.pl b/pmg-rs/test.pl
new file mode 100644
index 0000000..3d2a6df
--- /dev/null
+++ b/pmg-rs/test.pl
@@ -0,0 +1,172 @@
+#!/usr/bin/env perl
+
+use v5.28.0;
+use Data::Dumper;
+
+use lib '.';
+use PMG::RS::Acme;
+use PMG::RS::CSR;
+
+# "Config:" The Acme server URL:
+my $DIR = 'https://acme-staging-v02.api.letsencrypt.org/directory';
+
+# Useage:
+#
+# * Create a new account:
+# | ~/ $ ./test.pl ./account.json new 'somebody@example.invalid"
+#
+# The `./account.json` will be created using an EC P-256 key.
+# Optionally an RSA key size can be passed as additional parameter to generate
+# an account with an RSA key instead.
+#
+# From here on out the `./account.json` file must already exist:
+#
+# * Place a new order:
+# | ~/ $ ./test.pl ./account.json new-order my.domain.com
+# | $VAR1 = {
+# | ... order data ...
+# | 'authorizations' => [
+# | 'https://acme.example/auths/1244',
+# | ... possibly more ...
+# | ]
+# | }
+# | Order URL: https://acme.example/order/1793
+#
+# Note: This ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
+# URL will be used later for finalization and certifiate download.
+# The `$VAR1` dump contains the order JSON data.
+# The 'authorizations' URLs are going to be used next.
+#
+# * Get authorization info
+# | ~/ $ ./test.pl ./account.json get-auth 'https://acme.example/auths/1244'
+# | $VAR1 = {
+# | ... auth data ...
+# | 'challenges' => [
+# | {
+# | 'type' => 'dns-01',
+# | 'url' => 'https://acme.example/challenge/8188/dns1'
+# | }
+# | ... likely more ...
+# | ]
+# | }
+# | Key Authorization = SuperVeryMegaLongValue
+# | dns-01 TXT value = ShorterValue
+#
+# Now perform the things you need to for the challenge, eg. setup the DNS
+# entry using the provided TXT value.
+# Then use the correct challenge's URL with req-auth
+#
+# * Request challenge validation
+# | ~/ $ ./test.pl ./account.json \
+# | req-challenge 'https://acme.example/challenge/8188/dns1
+#
+# * Repeat the above 2 steps for all authorizations.
+# * Wait for the order to be valid via `get-order`
+# | ~/ $ ./test.pl ./account.json get-order 'https://acme.example/order/1793'
+# | $VAR1 = {
+# | 'status' => 'valid',
+# | 'finalize' => 'some URL',
+# | ... order data ...
+# | }
+# | Order URL: https://acme.example/order/1793
+#
+# * Finalize the order via the *Order URL* and a private key to sign the
+# request with (eg. generated via `openssl genrsa` or `openssl ecparam`).
+# | ~/ $ ./test.pl ./account.json \
+# | finalize my.domain.com ./my-private-key.pem \
+# | 'https://acme.example/order/1793'
+#
+# * Wait for a 'certificate' property to pop up in the order
+# (check via 'get-order')
+#
+# * Grab the certificate with the Order URL and a destination file name:
+# | ~/ $ ./test.pl ./account.json get-cert \
+# | 'https://acme.example/order/1793' \
+# | ./my-cert.pem
+
+
+my $account = shift // die "missing account file\n";
+my $cmd = shift // die "missing account file\n";
+
+sub load : prototype($) {
+ my ($file) = @_;
+ open(my $fh, '<', $file) or die "open($file): $!\n";
+ my $data = do {
+ local $/ = undef;
+ <$fh>
+ };
+ close($fh);
+ return $data;
+}
+
+sub store : prototype($$) {
+ my ($file, $data) = @_;
+ open(my $fh, '>', $file) or die "open($file): $!\n";
+ syswrite($fh, $data) == length($data)
+ or die "failed to write data to $file: $!\n";
+ close($fh);
+}
+
+if ($cmd eq 'new') {
+ my $mail = shift // die "missing mail address\n";
+ my $rsa_bits = shift;
+ if (defined($rsa_bits)) {
+ $rsa_bits = int($rsa_bits);
+ }
+ my $acme = PMG::RS::Acme->new($DIR);
+ $acme->new_account($account, 1, ["mailto:$mail"], undef);
+} elsif ($cmd eq 'get-meta') {
+ #my $acme = PMG::RS::Acme->new($DIR);
+ my $acme = PMG::RS::Acme->new('https%3A%2F%2Facme-v02.api.letsencrypt.org%2Fdirectory');
+ my $data = $acme->get_meta();
+ say Dumper($data);
+} elsif ($cmd eq 'new-order') {
+ my $domain = shift // die "missing domain\n";
+ my $acme = PMG::RS::Acme->load($account);
+ my ($url, $order) = $acme->new_order([$domain]);
+ say Dumper($order);
+ say "Order URL: $url\n";
+} elsif ($cmd eq 'get-auth') {
+ my $url = shift // die "missing url\n";
+ my $acme = PMG::RS::Acme->load($account);
+ my $auth = $acme->get_authorization($url);
+ say Dumper($auth);
+ for my $challenge ($auth->{challenges}->@*) {
+ next if $challenge->{type} ne 'dns-01';
+ say "Key Authorization = ".$acme->key_authorization($challenge->{token});
+ say "dns-01 TXT value = ".$acme->dns_01_txt_value($challenge->{token});
+ }
+} elsif ($cmd eq 'req-challenge') {
+ my $url = shift // die "missing url\n";
+ my $acme = PMG::RS::Acme->load($account);
+ my $challenge = $acme->request_challenge_validation($url);
+ say Dumper($challenge);
+} elsif ($cmd eq 'finalize') {
+ my $domain = shift // die 'missing domain\n';
+ my $pkfile = shift // die "missing private key file\n";
+ my $order_url = shift // die "missing order URL\n";
+ my ($csr_der, $pkey_pem) = PMG::RS::CSR::generate_csr([$domain], {});
+ store($pkfile, $pkey_pem);
+ my $acme = PMG::RS::Acme->load($account);
+ my $order = $acme->get_order($order_url);
+ say Dumper($order);
+ die "order not ready\n" if $order->{status} ne 'ready';
+ $acme->finalize_order($order->{finalize}, $csr_der);
+} elsif ($cmd eq 'get-order') {
+ my $order_url = shift // die "missing order URL\n";
+ my $acme = PMG::RS::Acme->load($account);
+ my $order = $acme->get_order($order_url);
+ say Dumper($order);
+} elsif ($cmd eq 'get-cert') {
+ my $order_url = shift // die "missing order URL\n";
+ my $file_name = shift // die "missing destination file name\n";
+ my $acme = PMG::RS::Acme->load($account);
+ my $order = $acme->get_order($order_url);
+ my $cert_url = $order->{certificate};
+ die "certificate not ready\n" if !$cert_url;
+ say Dumper($order);
+ my $cert = $acme->get_certificate($cert_url);
+ store($file_name, $cert);
+} else {
+ die "unknown command '$cmd'\n";
+}
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 5/7] pmg: bump perlmod to 0.9
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (10 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 4/7] import pmg-rs Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 6/7] pmg: add tfa module Wolfgang Bumiller
` (9 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
pmg-rs/Cargo.toml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
index 02f59de..659456c 100644
--- a/pmg-rs/Cargo.toml
+++ b/pmg-rs/Cargo.toml
@@ -26,7 +26,7 @@ serde = "1.0"
serde_bytes = "0.11.3"
serde_json = "1.0"
-perlmod = { version = "0.8.1", features = [ "exporter" ] }
+perlmod = { version = "0.9", features = [ "exporter" ] }
proxmox-acme-rs = { version = "0.3.1", features = ["client"] }
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 6/7] pmg: add tfa module
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (11 preceding siblings ...)
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 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 7/7] pmg: bump d/control Wolfgang Bumiller
` (8 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
pmg-rs/Cargo.toml | 5 +-
pmg-rs/Makefile | 5 +-
pmg-rs/debian/control | 9 +-
pmg-rs/src/lib.rs | 1 +
pmg-rs/src/tfa.rs | 603 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 618 insertions(+), 5 deletions(-)
create mode 100644 pmg-rs/src/tfa.rs
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
index 659456c..1d66bb5 100644
--- a/pmg-rs/Cargo.toml
+++ b/pmg-rs/Cargo.toml
@@ -21,13 +21,16 @@ crate-type = [ "cdylib" ]
[dependencies]
anyhow = "1.0"
hex = "0.4"
+libc = "0.2"
+nix = "0.19"
openssl = "0.10.32"
serde = "1.0"
serde_bytes = "0.11.3"
serde_json = "1.0"
+url = "2"
perlmod = { version = "0.9", features = [ "exporter" ] }
proxmox-acme-rs = { version = "0.3.1", features = ["client"] }
-
proxmox-apt = "0.8.0"
+proxmox-tfa = { version = "2", features = ["api"] }
diff --git a/pmg-rs/Makefile b/pmg-rs/Makefile
index a290544..69b2798 100644
--- a/pmg-rs/Makefile
+++ b/pmg-rs/Makefile
@@ -18,9 +18,10 @@ PM_DIRS := \
PMG/RS/APT
PM_FILES := \
- PMG/RS/Acme.pm \
PMG/RS/APT/Repositories.pm \
- PMG/RS/CSR.pm
+ PMG/RS/Acme.pm \
+ PMG/RS/CSR.pm \
+ PMG/RS/TFA.pm
ifeq ($(BUILD_MODE), release)
CARGO_BUILD_ARGS += --release
diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
index be632ba..8c22fae 100644
--- a/pmg-rs/debian/control
+++ b/pmg-rs/debian/control
@@ -6,15 +6,20 @@ Build-Depends:
debhelper (>= 12),
librust-anyhow-1+default-dev,
librust-hex-0.4+default-dev,
+ librust-libc-0.2+default-dev,
+ librust-nix-0.19+default-dev,
librust-openssl-0.10+default-dev (>= 0.10.32-~~),
- librust-perlmod-0.8+default-dev,
- librust-perlmod-0.8+exporter-dev,
+ librust-perlmod-0.8+default-dev (>= 0.8.1-~~),
+ librust-perlmod-0.8+exporter-dev (>= 0.8.1-~~),
librust-proxmox-acme-rs-0.3+client-dev (>= 0.3.1-~~),
librust-proxmox-acme-rs-0.3+default-dev (>= 0.3.1-~~),
librust-proxmox-apt-0.8+default-dev,
+ librust-proxmox-tfa-2+api-dev,
+ librust-proxmox-tfa-2+default-dev,
librust-serde-1+default-dev,
librust-serde-bytes-0.11+default-dev (>= 0.11.3-~~),
librust-serde-json-1+default-dev,
+ librust-url-2+default-dev,
Standards-Version: 4.3.0
Homepage: https://www.proxmox.com
diff --git a/pmg-rs/src/lib.rs b/pmg-rs/src/lib.rs
index 47e61b5..e3c9593 100644
--- a/pmg-rs/src/lib.rs
+++ b/pmg-rs/src/lib.rs
@@ -1,3 +1,4 @@
pub mod acme;
pub mod apt;
pub mod csr;
+pub mod tfa;
diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
new file mode 100644
index 0000000..404ddb2
--- /dev/null
+++ b/pmg-rs/src/tfa.rs
@@ -0,0 +1,603 @@
+//! This implements the `tfa.cfg` parser & TFA API calls for PMG.
+//!
+//! The exported `PMG::RS::TFA` perl package provides access to rust's `TfaConfig`.
+//! Contrary to the PVE implementation, this does not need to provide any backward compatible
+//! entries.
+//!
+//! NOTE: In PMG the tfa config is behind `PVE::INotify`'s `ccache`, so PMG sets it to `noclone` in
+//! order to avoid losing the rust magic-ref.
+
+use std::fs::File;
+use std::io::{self, Read};
+use std::os::unix::fs::OpenOptionsExt;
+use std::os::unix::io::{AsRawFd, RawFd};
+use std::path::{Path, PathBuf};
+
+use anyhow::{bail, format_err, Error};
+use nix::errno::Errno;
+use nix::sys::stat::Mode;
+
+pub(self) use proxmox_tfa::api::{
+ RecoveryState, TfaChallenge, TfaConfig, TfaResponse, U2fConfig, WebauthnConfig,
+};
+
+#[perlmod::package(name = "PMG::RS::TFA")]
+mod export {
+ use std::convert::TryInto;
+ use std::sync::Mutex;
+
+ use anyhow::{bail, format_err, Error};
+ use serde_bytes::ByteBuf;
+ use url::Url;
+
+ use perlmod::Value;
+ use proxmox_tfa::api::methods;
+
+ use super::{TfaConfig, UserAccess};
+
+ perlmod::declare_magic!(Box<Tfa> : &Tfa as "PMG::RS::TFA");
+
+ /// A TFA Config instance.
+ pub struct Tfa {
+ inner: Mutex<TfaConfig>,
+ }
+
+ /// Prevent 'dclone'.
+ #[export(name = "STORABLE_freeze", raw_return)]
+ fn storable_freeze(#[try_from_ref] _this: &Tfa, _cloning: bool) -> Result<Value, Error> {
+ bail!("freezing TFA config not supported!");
+ }
+
+ /// Parse a TFA configuration.
+ #[export(raw_return)]
+ fn new(#[raw] class: Value, config: &[u8]) -> Result<Value, Error> {
+ let mut inner: TfaConfig = serde_json::from_slice(config)
+ .map_err(|err| format_err!("failed to parse TFA file: {}", err))?;
+
+ // PMG does not support U2F.
+ inner.u2f = None;
+ Ok(perlmod::instantiate_magic!(
+ &class, MAGIC => Box::new(Tfa { inner: Mutex::new(inner) })
+ ))
+ }
+
+ /// Write the configuration out into a JSON string.
+ #[export]
+ fn write(#[try_from_ref] this: &Tfa) -> Result<serde_bytes::ByteBuf, Error> {
+ let inner = this.inner.lock().unwrap();
+ Ok(ByteBuf::from(serde_json::to_vec(&*inner)?))
+ }
+
+ /// Debug helper: serialize the TFA user data into a perl value.
+ #[export]
+ fn to_perl(#[try_from_ref] this: &Tfa) -> Result<Value, Error> {
+ let inner = this.inner.lock().unwrap();
+ Ok(perlmod::to_value(&*inner)?)
+ }
+
+ /// Get a list of all the user names in this config.
+ /// PMG uses this to verify users and purge the invalid ones.
+ #[export]
+ fn users(#[try_from_ref] this: &Tfa) -> Result<Vec<String>, Error> {
+ Ok(this.inner.lock().unwrap().users.keys().cloned().collect())
+ }
+
+ /// Remove a user from the TFA configuration.
+ #[export]
+ fn remove_user(#[try_from_ref] this: &Tfa, userid: &str) -> Result<bool, Error> {
+ Ok(this.inner.lock().unwrap().users.remove(userid).is_some())
+ }
+
+ /// Get the TFA data for a specific user.
+ #[export(raw_return)]
+ fn get_user(#[try_from_ref] this: &Tfa, userid: &str) -> Result<Value, perlmod::Error> {
+ perlmod::to_value(&this.inner.lock().unwrap().users.get(userid))
+ }
+
+ /// Add a u2f registration. This modifies the config (adds the user to it), so it needs be
+ /// written out.
+ #[export]
+ fn add_u2f_registration(
+ #[raw] raw_this: Value,
+ //#[try_from_ref] this: &Tfa,
+ userid: &str,
+ description: String,
+ ) -> Result<String, Error> {
+ let this: &Tfa = (&raw_this).try_into()?;
+ let mut inner = this.inner.lock().unwrap();
+ inner.u2f_registration_challenge(UserAccess::new(&raw_this)?, userid, description)
+ }
+
+ /// Finish a u2f registration. This updates temporary data in `/run` and therefore the config
+ /// needs to be written out!
+ #[export]
+ fn finish_u2f_registration(
+ #[raw] raw_this: Value,
+ //#[try_from_ref] this: &Tfa,
+ userid: &str,
+ challenge: &str,
+ response: &str,
+ ) -> Result<String, Error> {
+ let this: &Tfa = (&raw_this).try_into()?;
+ let mut inner = this.inner.lock().unwrap();
+ inner.u2f_registration_finish(UserAccess::new(&raw_this)?, userid, challenge, response)
+ }
+
+ /// Check if a user has any TFA entries of a given type.
+ #[export]
+ fn has_type(#[try_from_ref] this: &Tfa, userid: &str, typename: &str) -> Result<bool, Error> {
+ Ok(match this.inner.lock().unwrap().users.get(userid) {
+ Some(user) => match typename {
+ "totp" | "oath" => !user.totp.is_empty(),
+ "u2f" => !user.u2f.is_empty(),
+ "webauthn" => !user.webauthn.is_empty(),
+ "yubico" => !user.yubico.is_empty(),
+ "recovery" => match &user.recovery {
+ Some(r) => r.count_available() > 0,
+ None => false,
+ },
+ _ => bail!("unrecognized TFA type {:?}", typename),
+ },
+ None => false,
+ })
+ }
+
+ /// Generates a space separated list of yubico keys of this account.
+ #[export]
+ fn get_yubico_keys(#[try_from_ref] this: &Tfa, userid: &str) -> Result<Option<String>, Error> {
+ Ok(this.inner.lock().unwrap().users.get(userid).map(|user| {
+ user.enabled_yubico_entries()
+ .fold(String::new(), |mut s, k| {
+ if !s.is_empty() {
+ s.push(' ');
+ }
+ s.push_str(k);
+ s
+ })
+ }))
+ }
+
+ #[export]
+ fn set_u2f_config(#[try_from_ref] this: &Tfa, config: Option<super::U2fConfig>) {
+ this.inner.lock().unwrap().u2f = config;
+ }
+
+ #[export]
+ fn set_webauthn_config(
+ #[try_from_ref] this: &Tfa,
+ config: Option<super::WebauthnConfig>,
+ ) -> Result<(), Error> {
+ this.inner.lock().unwrap().webauthn = config.map(TryInto::try_into).transpose()?;
+ Ok(())
+ }
+
+ #[export]
+ fn get_webauthn_config(
+ #[try_from_ref] this: &Tfa,
+ ) -> Result<(Option<String>, Option<super::WebauthnConfig>), Error> {
+ Ok(match this.inner.lock().unwrap().webauthn.clone() {
+ Some(config) => (Some(hex::encode(&config.digest())), Some(config.into())),
+ None => (None, None),
+ })
+ }
+
+ #[export]
+ fn has_webauthn_origin(#[try_from_ref] this: &Tfa) -> bool {
+ match &this.inner.lock().unwrap().webauthn {
+ Some(wa) => wa.origin.is_some(),
+ None => false,
+ }
+ }
+
+ /// Create an authentication challenge.
+ ///
+ /// Returns the challenge as a json string.
+ /// Returns `undef` if no second factor is configured.
+ #[export]
+ fn authentication_challenge(
+ #[raw] raw_this: Value,
+ //#[try_from_ref] this: &Tfa,
+ userid: &str,
+ origin: Option<Url>,
+ ) -> Result<Option<String>, Error> {
+ let this: &Tfa = (&raw_this).try_into()?;
+ let mut inner = this.inner.lock().unwrap();
+ match inner.authentication_challenge(
+ UserAccess::new(&raw_this)?,
+ userid,
+ origin.as_ref(),
+ )? {
+ Some(challenge) => Ok(Some(serde_json::to_string(&challenge)?)),
+ None => Ok(None),
+ }
+ }
+
+ /// Get the recovery state (suitable for a challenge object).
+ #[export]
+ fn recovery_state(#[try_from_ref] this: &Tfa, userid: &str) -> Option<super::RecoveryState> {
+ this.inner
+ .lock()
+ .unwrap()
+ .users
+ .get(userid)
+ .and_then(|user| {
+ let state = user.recovery_state();
+ state.is_available().then(move || state)
+ })
+ }
+
+ /// Takes the TFA challenge string (which is a json object) and verifies ther esponse against
+ /// it.
+ ///
+ /// NOTE: This returns a boolean whether the config data needs to be *saved* after this call
+ /// (to use up recovery keys!).
+ #[export]
+ fn authentication_verify(
+ #[raw] raw_this: Value,
+ //#[try_from_ref] this: &Tfa,
+ userid: &str,
+ challenge: &str, //super::TfaChallenge,
+ response: &str,
+ origin: Option<Url>,
+ ) -> Result<bool, Error> {
+ let this: &Tfa = (&raw_this).try_into()?;
+ let challenge: super::TfaChallenge = serde_json::from_str(challenge)?;
+ let response: super::TfaResponse = response.parse()?;
+ let mut inner = this.inner.lock().unwrap();
+ inner
+ .verify(
+ UserAccess::new(&raw_this)?,
+ userid,
+ &challenge,
+ response,
+ origin.as_ref(),
+ )
+ .map(|save| save.needs_saving())
+ }
+
+ /// DEBUG HELPER: Get the current TOTP value for a given TOTP URI.
+ #[export]
+ fn get_current_totp_value(otp_uri: &str) -> Result<String, Error> {
+ let totp: proxmox_tfa::totp::Totp = otp_uri.parse()?;
+ Ok(totp.time(std::time::SystemTime::now())?.to_string())
+ }
+
+ #[export]
+ fn api_list_user_tfa(
+ #[try_from_ref] this: &Tfa,
+ userid: &str,
+ ) -> Result<Vec<methods::TypedTfaInfo>, Error> {
+ methods::list_user_tfa(&this.inner.lock().unwrap(), userid)
+ }
+
+ #[export]
+ fn api_get_tfa_entry(
+ #[try_from_ref] this: &Tfa,
+ userid: &str,
+ id: &str,
+ ) -> Option<methods::TypedTfaInfo> {
+ methods::get_tfa_entry(&this.inner.lock().unwrap(), userid, id)
+ }
+
+ /// Returns `true` if the user still has other TFA entries left, `false` if the user has *no*
+ /// more tfa entries.
+ #[export]
+ fn api_delete_tfa(#[try_from_ref] this: &Tfa, userid: &str, id: String) -> Result<bool, Error> {
+ let mut this = this.inner.lock().unwrap();
+ match methods::delete_tfa(&mut this, userid, &id) {
+ Ok(has_entries_left) => Ok(has_entries_left),
+ Err(methods::EntryNotFound) => bail!("no such entry"),
+ }
+ }
+
+ #[export]
+ fn api_list_tfa(
+ #[try_from_ref] this: &Tfa,
+ authid: &str,
+ top_level_allowed: bool,
+ ) -> Result<Vec<methods::TfaUser>, Error> {
+ methods::list_tfa(&this.inner.lock().unwrap(), authid, top_level_allowed)
+ }
+
+ #[export]
+ fn api_add_tfa_entry(
+ #[raw] raw_this: Value,
+ //#[try_from_ref] this: &Tfa,
+ userid: &str,
+ description: Option<String>,
+ totp: Option<String>,
+ value: Option<String>,
+ challenge: Option<String>,
+ ty: methods::TfaType,
+ origin: Option<Url>,
+ ) -> Result<methods::TfaUpdateInfo, Error> {
+ let this: &Tfa = (&raw_this).try_into()?;
+ methods::add_tfa_entry(
+ &mut this.inner.lock().unwrap(),
+ UserAccess::new(&raw_this)?,
+ userid,
+ description,
+ totp,
+ value,
+ challenge,
+ ty,
+ origin.as_ref(),
+ )
+ }
+
+ /// Add a totp entry without validating it, used for user.cfg keys.
+ /// Returns the ID.
+ #[export]
+ fn add_totp_entry(
+ #[try_from_ref] this: &Tfa,
+ userid: &str,
+ description: String,
+ totp: String,
+ ) -> Result<String, Error> {
+ Ok(this
+ .inner
+ .lock()
+ .unwrap()
+ .add_totp(userid, description, totp.parse()?))
+ }
+
+ /// Add a yubico entry without validating it, used for user.cfg keys.
+ /// Returns the ID.
+ #[export]
+ fn add_yubico_entry(
+ #[try_from_ref] this: &Tfa,
+ userid: &str,
+ description: String,
+ yubico: String,
+ ) -> String {
+ this.inner
+ .lock()
+ .unwrap()
+ .add_yubico(userid, description, yubico)
+ }
+
+ #[export]
+ fn api_update_tfa_entry(
+ #[try_from_ref] this: &Tfa,
+ userid: &str,
+ id: &str,
+ description: Option<String>,
+ enable: Option<bool>,
+ ) -> Result<(), Error> {
+ match methods::update_tfa_entry(
+ &mut this.inner.lock().unwrap(),
+ userid,
+ id,
+ description,
+ enable,
+ ) {
+ Ok(()) => Ok(()),
+ Err(methods::EntryNotFound) => bail!("no such entry"),
+ }
+ }
+}
+
+/// Attach the path to errors from [`nix::mkir()`].
+pub(crate) fn mkdir<P: AsRef<Path>>(path: P, mode: libc::mode_t) -> Result<(), Error> {
+ let path = path.as_ref();
+ match nix::unistd::mkdir(path, unsafe { Mode::from_bits_unchecked(mode) }) {
+ Ok(()) => Ok(()),
+ Err(nix::Error::Sys(Errno::EEXIST)) => Ok(()),
+ Err(err) => bail!("failed to create directory {:?}: {}", path, err),
+ }
+}
+
+#[cfg(debug_assertions)]
+#[derive(Clone)]
+#[repr(transparent)]
+pub struct UserAccess(perlmod::Value);
+
+#[cfg(debug_assertions)]
+impl UserAccess {
+ #[inline]
+ fn new(value: &perlmod::Value) -> Result<Self, Error> {
+ value
+ .dereference()
+ .ok_or_else(|| format_err!("bad TFA config object"))
+ .map(Self)
+ }
+
+ #[inline]
+ fn is_debug(&self) -> bool {
+ self.0
+ .as_hash()
+ .and_then(|v| v.get("-debug"))
+ .map(|v| v.iv() != 0)
+ .unwrap_or(false)
+ }
+}
+
+#[cfg(not(debug_assertions))]
+#[derive(Clone, Copy)]
+#[repr(transparent)]
+pub struct UserAccess;
+
+#[cfg(not(debug_assertions))]
+impl UserAccess {
+ #[inline]
+ const fn new(_value: &perlmod::Value) -> Result<Self, std::convert::Infallible> {
+ Ok(Self)
+ }
+
+ #[inline]
+ const fn is_debug(&self) -> bool {
+ false
+ }
+}
+
+/// Build the path to the challenge data file for a user.
+fn challenge_data_path(userid: &str, debug: bool) -> PathBuf {
+ if debug {
+ PathBuf::from(format!("./local-tfa-challenges/{}", userid))
+ } else {
+ PathBuf::from(format!("/run/pmg-private/tfa-challenges/{}", userid))
+ }
+}
+
+impl proxmox_tfa::api::OpenUserChallengeData for UserAccess {
+ type Data = UserChallengeData;
+
+ fn open(&self, userid: &str) -> Result<UserChallengeData, Error> {
+ if self.is_debug() {
+ mkdir("./local-tfa-challenges", 0o700)?;
+ } else {
+ mkdir("/run/pmg-private", 0o700)?;
+ mkdir("/run/pmg-private/tfa-challenges", 0o700)?;
+ }
+
+ let path = challenge_data_path(userid, self.is_debug());
+
+ let mut file = std::fs::OpenOptions::new()
+ .create(true)
+ .read(true)
+ .write(true)
+ .truncate(false)
+ .mode(0o600)
+ .open(&path)
+ .map_err(|err| format_err!("failed to create challenge file {:?}: {}", &path, err))?;
+
+ UserChallengeData::lock_file(file.as_raw_fd())?;
+
+ // the file may be empty, so read to a temporary buffer first:
+ let mut data = Vec::with_capacity(4096);
+
+ file.read_to_end(&mut data).map_err(|err| {
+ format_err!("failed to read challenge data for user {}: {}", userid, err)
+ })?;
+
+ let inner = if data.is_empty() {
+ Default::default()
+ } else {
+ match serde_json::from_slice(&data) {
+ Ok(inner) => inner,
+ Err(err) => {
+ eprintln!(
+ "failed to parse challenge data for user {}: {}",
+ userid, err
+ );
+ Default::default()
+ }
+ }
+ };
+
+ Ok(UserChallengeData {
+ inner,
+ path,
+ lock: file,
+ })
+ }
+
+ /// `open` without creating the file if it doesn't exist, to finish WA authentications.
+ fn open_no_create(&self, userid: &str) -> Result<Option<UserChallengeData>, Error> {
+ let path = challenge_data_path(userid, self.is_debug());
+
+ let mut file = match std::fs::OpenOptions::new()
+ .read(true)
+ .write(true)
+ .truncate(false)
+ .mode(0o600)
+ .open(&path)
+ {
+ Ok(file) => file,
+ Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None),
+ Err(err) => return Err(err.into()),
+ };
+
+ UserChallengeData::lock_file(file.as_raw_fd())?;
+
+ let inner = serde_json::from_reader(&mut file).map_err(|err| {
+ format_err!("failed to read challenge data for user {}: {}", userid, err)
+ })?;
+
+ Ok(Some(UserChallengeData {
+ inner,
+ path,
+ lock: file,
+ }))
+ }
+
+ fn remove(&self, userid: &str) -> Result<bool, Error> {
+ let path = challenge_data_path(userid, self.is_debug());
+ match std::fs::remove_file(&path) {
+ Ok(()) => Ok(true),
+ Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(false),
+ Err(err) => Err(err.into()),
+ }
+ }
+}
+
+/// Container of `TfaUserChallenges` with the corresponding file lock guard.
+///
+/// Basically provides the TFA API to the REST server by persisting, updating and verifying active
+/// challenges.
+pub struct UserChallengeData {
+ inner: proxmox_tfa::api::TfaUserChallenges,
+ path: PathBuf,
+ lock: File,
+}
+
+impl proxmox_tfa::api::UserChallengeAccess for UserChallengeData {
+ fn get_mut(&mut self) -> &mut proxmox_tfa::api::TfaUserChallenges {
+ &mut self.inner
+ }
+
+ fn save(self) -> Result<(), Error> {
+ UserChallengeData::save(self)
+ }
+}
+
+impl UserChallengeData {
+ fn lock_file(fd: RawFd) -> Result<(), Error> {
+ let rc = unsafe { libc::flock(fd, libc::LOCK_EX) };
+
+ if rc != 0 {
+ let err = io::Error::last_os_error();
+ bail!("failed to lock tfa user challenge data: {}", err);
+ }
+
+ Ok(())
+ }
+
+ /// Rewind & truncate the file for an update.
+ fn rewind(&mut self) -> Result<(), Error> {
+ use std::io::{Seek, SeekFrom};
+
+ let pos = self.lock.seek(SeekFrom::Start(0))?;
+ if pos != 0 {
+ bail!(
+ "unexpected result trying to rewind file, position is {}",
+ pos
+ );
+ }
+
+ let rc = unsafe { libc::ftruncate(self.lock.as_raw_fd(), 0) };
+ if rc != 0 {
+ let err = io::Error::last_os_error();
+ bail!("failed to truncate challenge data: {}", err);
+ }
+
+ Ok(())
+ }
+
+ /// Save the current data. Note that we do not replace the file here since we lock the file
+ /// itself, as it is in `/run`, and the typical error case for this particular situation
+ /// (machine loses power) simply prevents some login, but that'll probably fail anyway for
+ /// other reasons then...
+ ///
+ /// This currently consumes selfe as we never perform more than 1 insertion/removal, and this
+ /// way also unlocks early.
+ fn save(mut self) -> Result<(), Error> {
+ self.rewind()?;
+
+ serde_json::to_writer(&mut &self.lock, &self.inner).map_err(|err| {
+ format_err!("failed to update challenge file {:?}: {}", self.path, err)
+ })?;
+
+ Ok(())
+ }
+}
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH perl-rs 7/7] pmg: bump d/control
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (12 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 6/7] pmg: add tfa module Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 1/6] tfa: fix typo in docs Wolfgang Bumiller
` (7 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
pmg-rs/debian/control | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pmg-rs/debian/control b/pmg-rs/debian/control
index 8c22fae..0817571 100644
--- a/pmg-rs/debian/control
+++ b/pmg-rs/debian/control
@@ -9,8 +9,8 @@ Build-Depends:
librust-libc-0.2+default-dev,
librust-nix-0.19+default-dev,
librust-openssl-0.10+default-dev (>= 0.10.32-~~),
- librust-perlmod-0.8+default-dev (>= 0.8.1-~~),
- librust-perlmod-0.8+exporter-dev (>= 0.8.1-~~),
+ librust-perlmod-0.9+default-dev,
+ librust-perlmod-0.9+exporter-dev,
librust-proxmox-acme-rs-0.3+client-dev (>= 0.3.1-~~),
librust-proxmox-acme-rs-0.3+default-dev (>= 0.3.1-~~),
librust-proxmox-apt-0.8+default-dev,
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH proxmox 1/6] tfa: fix typo in docs
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (13 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH perl-rs 7/7] pmg: bump d/control Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 2/6] tfa: add WebauthnConfig::digest method Wolfgang Bumiller
` (6 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
proxmox-tfa/src/api/webauthn.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs
index c691e42..d4f8acb 100644
--- a/proxmox-tfa/src/api/webauthn.rs
+++ b/proxmox-tfa/src/api/webauthn.rs
@@ -43,7 +43,7 @@ pub struct WebauthnConfig {
/// Changing this *may* break existing credentials.
pub origin: OriginUrl,
- /// Relying part ID. Must be the domain name without protocol, port or location.
+ /// Relying party ID. Must be the domain name without protocol, port or location.
///
/// Changing this *will* break existing credentials.
pub id: String,
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH proxmox 2/6] tfa: add WebauthnConfig::digest method
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (14 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 1/6] tfa: fix typo in docs Wolfgang Bumiller
@ 2021-11-26 13:55 ` 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
` (5 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
proxmox-tfa/src/api/webauthn.rs | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs
index d4f8acb..6453808 100644
--- a/proxmox-tfa/src/api/webauthn.rs
+++ b/proxmox-tfa/src/api/webauthn.rs
@@ -49,6 +49,18 @@ pub struct WebauthnConfig {
pub id: String,
}
+impl WebauthnConfig {
+ pub fn digest(&self) -> [u8; 32] {
+ let data = format!(
+ "rp={:?}\norigin={:?}\nid={:?}\n",
+ self.rp,
+ self.origin.0.as_str(),
+ self.id,
+ );
+ openssl::sha::sha256(data.as_bytes())
+ }
+}
+
/// For now we just implement this on the configuration this way.
///
/// Note that we may consider changing this so `get_origin` returns the `Host:` header provided by
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH proxmox 3/6] tfa: let OriginUrl deref to its inner Url, add FromStr impl
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (15 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 2/6] tfa: add WebauthnConfig::digest method Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 4/6] tfa: make configured webauthn origin optional Wolfgang Bumiller
` (4 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
proxmox-tfa/src/api/webauthn.rs | 35 +++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs
index 6453808..486e4f5 100644
--- a/proxmox-tfa/src/api/webauthn.rs
+++ b/proxmox-tfa/src/api/webauthn.rs
@@ -1,14 +1,13 @@
//! Webauthn configuration and challenge data.
+use anyhow::Error;
use serde::{Deserialize, Serialize};
+use url::Url;
+use webauthn_rs::proto::{COSEKey, Credential, CredentialID, UserVerificationPolicy};
#[cfg(feature = "api-types")]
use proxmox_schema::{api, Updater, UpdaterType};
-use url::Url;
-
-use webauthn_rs::proto::{COSEKey, Credential, CredentialID, UserVerificationPolicy};
-
use super::IsExpired;
#[derive(Clone, Deserialize, Serialize)]
@@ -20,6 +19,34 @@ impl UpdaterType for OriginUrl {
type Updater = Option<Self>;
}
+impl std::str::FromStr for OriginUrl {
+ type Err = Error;
+
+ fn from_str(s: &str) -> Result<Self, Error> {
+ Ok(Self(s.parse()?))
+ }
+}
+
+impl std::ops::Deref for OriginUrl {
+ type Target = Url;
+
+ fn deref(&self) -> &Url {
+ &self.0
+ }
+}
+
+impl std::ops::DerefMut for OriginUrl {
+ fn deref_mut(&mut self) -> &mut Url {
+ &mut self.0
+ }
+}
+
+impl Into<String> for OriginUrl {
+ fn into(self) -> String {
+ self.0.into()
+ }
+}
+
#[cfg_attr(feature = "api-types", api(
properties: {
rp: { type: String },
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH proxmox 4/6] tfa: make configured webauthn origin optional
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (16 preceding siblings ...)
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 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 5/6] tfa: clippy fixes Wolfgang Bumiller
` (3 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
and add a webauthn origin override parameter to all methods
accessing it
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
proxmox-tfa/src/api/methods.rs | 21 ++++++++++++---
proxmox-tfa/src/api/mod.rs | 45 +++++++++++++++++++++------------
proxmox-tfa/src/api/webauthn.rs | 45 +++++++++++++++++++++++----------
3 files changed, 79 insertions(+), 32 deletions(-)
diff --git a/proxmox-tfa/src/api/methods.rs b/proxmox-tfa/src/api/methods.rs
index 56971f1..b63d56e 100644
--- a/proxmox-tfa/src/api/methods.rs
+++ b/proxmox-tfa/src/api/methods.rs
@@ -317,6 +317,7 @@ pub fn add_tfa_entry<A: OpenUserChallengeData>(
value: Option<String>,
challenge: Option<String>,
r#type: TfaType,
+ origin: Option<&url::Url>,
) -> Result<TfaUpdateInfo, Error> {
match r#type {
TfaType::Totp => {
@@ -331,7 +332,15 @@ pub fn add_tfa_entry<A: OpenUserChallengeData>(
bail!("'totp' parameter is invalid for 'webauthn' entries");
}
- add_webauthn(config, access, userid, description, challenge, value)
+ add_webauthn(
+ config,
+ access,
+ userid,
+ description,
+ challenge,
+ value,
+ origin,
+ )
}
TfaType::U2f => {
if totp.is_some() {
@@ -436,10 +445,16 @@ fn add_webauthn<A: OpenUserChallengeData>(
description: Option<String>,
challenge: Option<String>,
value: Option<String>,
+ origin: Option<&url::Url>,
) -> Result<TfaUpdateInfo, Error> {
match challenge {
None => config
- .webauthn_registration_challenge(access, &userid, need_description(description)?)
+ .webauthn_registration_challenge(
+ access,
+ &userid,
+ need_description(description)?,
+ origin,
+ )
.map(|c| TfaUpdateInfo {
challenge: Some(c),
..Default::default()
@@ -449,7 +464,7 @@ fn add_webauthn<A: OpenUserChallengeData>(
format_err!("missing 'value' parameter (webauthn challenge response missing)")
})?;
config
- .webauthn_registration_finish(access, &userid, &challenge, &value)
+ .webauthn_registration_finish(access, &userid, &challenge, &value, origin)
.map(TfaUpdateInfo::id)
}
}
diff --git a/proxmox-tfa/src/api/mod.rs b/proxmox-tfa/src/api/mod.rs
index e0084b9..b591a23 100644
--- a/proxmox-tfa/src/api/mod.rs
+++ b/proxmox-tfa/src/api/mod.rs
@@ -9,6 +9,7 @@ use std::collections::HashMap;
use anyhow::{bail, format_err, Error};
use serde::{Deserialize, Serialize};
use serde_json::Value;
+use url::Url;
use webauthn_rs::{proto::UserVerificationPolicy, Webauthn};
@@ -28,6 +29,7 @@ pub mod methods;
pub use recovery::RecoveryState;
pub use u2f::U2fConfig;
+use webauthn::WebauthnConfigInstance;
pub use webauthn::{WebauthnConfig, WebauthnCredential};
#[cfg(feature = "api-types")]
@@ -93,15 +95,22 @@ fn check_u2f(u2f: &Option<U2fConfig>) -> Result<u2f::U2f, Error> {
/// Helper to get a `Webauthn` instance from a `WebauthnConfig`, or `None` if there isn't one
/// configured.
-fn get_webauthn(waconfig: &Option<WebauthnConfig>) -> Option<Webauthn<WebauthnConfig>> {
- waconfig.clone().map(Webauthn::new)
+fn get_webauthn<'a, 'config: 'a, 'origin: 'a>(
+ waconfig: &'config Option<WebauthnConfig>,
+ origin: Option<&'origin Url>,
+) -> Option<Result<Webauthn<WebauthnConfigInstance<'a>>, Error>> {
+ Some(waconfig.as_ref()?.instantiate(origin).map(Webauthn::new))
}
/// Helper to get a u2f instance from a u2f config.
///
/// This is outside of `TfaConfig` to not borrow its `&self`.
-fn check_webauthn(waconfig: &Option<WebauthnConfig>) -> Result<Webauthn<WebauthnConfig>, Error> {
- get_webauthn(waconfig).ok_or_else(|| format_err!("no webauthn configuration available"))
+fn check_webauthn<'a, 'config: 'a, 'origin: 'a>(
+ waconfig: &'config Option<WebauthnConfig>,
+ origin: Option<&'origin Url>,
+) -> Result<Webauthn<WebauthnConfigInstance<'a>>, Error> {
+ get_webauthn(waconfig, origin)
+ .ok_or_else(|| format_err!("no webauthn configuration available"))?
}
impl TfaConfig {
@@ -142,8 +151,9 @@ impl TfaConfig {
access: A,
user: &str,
description: String,
+ origin: Option<&Url>,
) -> Result<String, Error> {
- let webauthn = check_webauthn(&self.webauthn)?;
+ let webauthn = check_webauthn(&self.webauthn, origin)?;
self.users
.entry(user.to_owned())
@@ -158,8 +168,9 @@ impl TfaConfig {
userid: &str,
challenge: &str,
response: &str,
+ origin: Option<&Url>,
) -> Result<String, Error> {
- let webauthn = check_webauthn(&self.webauthn)?;
+ let webauthn = check_webauthn(&self.webauthn, origin)?;
let response: webauthn_rs::proto::RegisterPublicKeyCredential =
serde_json::from_str(response)
@@ -208,12 +219,13 @@ impl TfaConfig {
&mut self,
access: A,
userid: &str,
+ origin: Option<&Url>,
) -> Result<Option<TfaChallenge>, Error> {
match self.users.get_mut(userid) {
Some(udata) => udata.challenge(
access,
userid,
- get_webauthn(&self.webauthn),
+ get_webauthn(&self.webauthn, origin),
get_u2f(&self.u2f).as_ref(),
),
None => Ok(None),
@@ -227,6 +239,7 @@ impl TfaConfig {
userid: &str,
challenge: &TfaChallenge,
response: TfaResponse,
+ origin: Option<&Url>,
) -> Result<NeedsSaving, Error> {
match self.users.get_mut(userid) {
Some(user) => match response {
@@ -239,7 +252,7 @@ impl TfaConfig {
None => bail!("no u2f factor available for user '{}'", userid),
},
TfaResponse::Webauthn(value) => {
- let webauthn = check_webauthn(&self.webauthn)?;
+ let webauthn = check_webauthn(&self.webauthn, origin)?;
user.verify_webauthn(access.clone(), userid, webauthn, value)
}
TfaResponse::Recovery(value) => {
@@ -424,7 +437,7 @@ impl TfaUserData {
fn webauthn_registration_challenge<A: OpenUserChallengeData>(
&mut self,
access: A,
- webauthn: Webauthn<WebauthnConfig>,
+ webauthn: Webauthn<WebauthnConfigInstance>,
userid: &str,
description: String,
) -> Result<String, Error> {
@@ -463,7 +476,7 @@ impl TfaUserData {
fn webauthn_registration_finish<A: OpenUserChallengeData>(
&mut self,
access: A,
- webauthn: Webauthn<WebauthnConfig>,
+ webauthn: Webauthn<WebauthnConfigInstance>,
userid: &str,
challenge: &str,
response: webauthn_rs::proto::RegisterPublicKeyCredential,
@@ -555,11 +568,11 @@ impl TfaUserData {
}
/// Generate a generic TFA challenge. See the [`TfaChallenge`] description for details.
- pub fn challenge<A: OpenUserChallengeData>(
+ fn challenge<A: OpenUserChallengeData>(
&mut self,
access: A,
userid: &str,
- webauthn: Option<Webauthn<WebauthnConfig>>,
+ webauthn: Option<Result<Webauthn<WebauthnConfigInstance>, Error>>,
u2f: Option<&u2f::U2f>,
) -> Result<Option<TfaChallenge>, Error> {
if self.is_empty() {
@@ -570,7 +583,7 @@ impl TfaUserData {
totp: self.totp.iter().any(|e| e.info.enable),
recovery: RecoveryState::from(&self.recovery),
webauthn: match webauthn {
- Some(webauthn) => self.webauthn_challenge(access.clone(), userid, webauthn)?,
+ Some(webauthn) => self.webauthn_challenge(access.clone(), userid, webauthn?)?,
None => None,
},
u2f: match u2f {
@@ -591,7 +604,7 @@ impl TfaUserData {
&mut self,
access: A,
userid: &str,
- webauthn: Webauthn<WebauthnConfig>,
+ webauthn: Webauthn<WebauthnConfigInstance>,
) -> Result<Option<webauthn_rs::proto::RequestChallengeResponse>, Error> {
if self.webauthn.is_empty() {
return Ok(None);
@@ -703,7 +716,7 @@ impl TfaUserData {
&mut self,
access: A,
userid: &str,
- webauthn: Webauthn<WebauthnConfig>,
+ webauthn: Webauthn<WebauthnConfigInstance>,
mut response: Value,
) -> Result<(), Error> {
let expire_before = proxmox_time::epoch_i64() - CHALLENGE_TIMEOUT_SECS;
@@ -995,7 +1008,7 @@ impl TfaUserChallenges {
/// `webauthn_registration_challenge`. The response should come directly from the client.
fn webauthn_registration_finish(
&mut self,
- webauthn: Webauthn<WebauthnConfig>,
+ webauthn: Webauthn<WebauthnConfigInstance>,
challenge: &str,
response: webauthn_rs::proto::RegisterPublicKeyCredential,
existing_registrations: &[TfaEntry<WebauthnCredential>],
diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs
index 486e4f5..aed2885 100644
--- a/proxmox-tfa/src/api/webauthn.rs
+++ b/proxmox-tfa/src/api/webauthn.rs
@@ -1,6 +1,6 @@
//! Webauthn configuration and challenge data.
-use anyhow::Error;
+use anyhow::{format_err, Error};
use serde::{Deserialize, Serialize};
use url::Url;
use webauthn_rs::proto::{COSEKey, Credential, CredentialID, UserVerificationPolicy};
@@ -50,7 +50,7 @@ impl Into<String> for OriginUrl {
#[cfg_attr(feature = "api-types", api(
properties: {
rp: { type: String },
- origin: { type: String },
+ origin: { type: String, optional: true },
id: { type: String },
}
))]
@@ -68,7 +68,8 @@ pub struct WebauthnConfig {
/// users type in their browsers to access the web interface.
///
/// Changing this *may* break existing credentials.
- pub origin: OriginUrl,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub origin: Option<OriginUrl>,
/// Relying party ID. Must be the domain name without protocol, port or location.
///
@@ -78,31 +79,49 @@ pub struct WebauthnConfig {
impl WebauthnConfig {
pub fn digest(&self) -> [u8; 32] {
- let data = format!(
- "rp={:?}\norigin={:?}\nid={:?}\n",
- self.rp,
- self.origin.0.as_str(),
- self.id,
- );
+ let mut data = format!("rp={:?}\nid={:?}\n", self.rp, self.id,);
+ if let Some(origin) = &self.origin {
+ data.push_str(&format!("origin={:?}\n", origin.as_str()));
+ }
openssl::sha::sha256(data.as_bytes())
}
+
+ /// Instantiate a usable webauthn configuration instance.
+ pub(super) fn instantiate<'a, 'this: 'a, 'origin: 'a>(
+ &'this self,
+ origin: Option<&'origin Url>,
+ ) -> Result<WebauthnConfigInstance<'a>, Error> {
+ Ok(WebauthnConfigInstance {
+ origin: origin
+ .or_else(|| self.origin.as_ref().map(|u| &u.0))
+ .ok_or_else(|| format_err!("missing webauthn origin"))?,
+ rp: &self.rp,
+ id: &self.id,
+ })
+ }
+}
+
+pub(super) struct WebauthnConfigInstance<'a> {
+ rp: &'a str,
+ origin: &'a Url,
+ id: &'a str,
}
/// For now we just implement this on the configuration this way.
///
/// Note that we may consider changing this so `get_origin` returns the `Host:` header provided by
/// the connecting client.
-impl webauthn_rs::WebauthnConfig for WebauthnConfig {
+impl<'a> webauthn_rs::WebauthnConfig for WebauthnConfigInstance<'a> {
fn get_relying_party_name(&self) -> &str {
- &self.rp
+ self.rp
}
fn get_origin(&self) -> &Url {
- &self.origin.0
+ self.origin
}
fn get_relying_party_id(&self) -> &str {
- &self.id
+ self.id
}
}
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH proxmox 5/6] tfa: clippy fixes
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (17 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 4/6] tfa: make configured webauthn origin optional Wolfgang Bumiller
@ 2021-11-26 13:55 ` Wolfgang Bumiller
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 6/6] bump proxmox-tfa to 2.0.0-1 Wolfgang Bumiller
` (2 subsequent siblings)
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
proxmox-tfa/src/api/methods.rs | 20 ++++++++++----------
proxmox-tfa/src/api/mod.rs | 6 +++---
proxmox-tfa/src/api/recovery.rs | 4 ++--
proxmox-tfa/src/api/serde_tools.rs | 8 ++++----
proxmox-tfa/src/api/webauthn.rs | 6 +++---
proxmox-tfa/src/totp.rs | 14 ++++++++------
proxmox-tfa/src/u2f.rs | 2 +-
7 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/proxmox-tfa/src/api/methods.rs b/proxmox-tfa/src/api/methods.rs
index b63d56e..08905da 100644
--- a/proxmox-tfa/src/api/methods.rs
+++ b/proxmox-tfa/src/api/methods.rs
@@ -142,7 +142,7 @@ pub fn get_tfa_entry(config: &TfaConfig, userid: &str, id: &str) -> Option<Typed
Some(
match {
// scope to prevent the temporary iter from borrowing across the whole match
- let entry = tfa_id_iter(&user_data).find(|(_ty, _index, entry_id)| id == *entry_id);
+ let entry = tfa_id_iter(user_data).find(|(_ty, _index, entry_id)| id == *entry_id);
entry.map(|(ty, index, _)| (ty, index))
} {
Some((TfaType::Recovery, _)) => match user_data.recovery() {
@@ -155,21 +155,20 @@ pub fn get_tfa_entry(config: &TfaConfig, userid: &str, id: &str) -> Option<Typed
Some((TfaType::Totp, index)) => {
TypedTfaInfo {
ty: TfaType::Totp,
- // `into_iter().nth()` to *move* out of it
- info: user_data.totp.iter().nth(index).unwrap().info.clone(),
+ info: user_data.totp.get(index).unwrap().info.clone(),
}
}
Some((TfaType::Webauthn, index)) => TypedTfaInfo {
ty: TfaType::Webauthn,
- info: user_data.webauthn.iter().nth(index).unwrap().info.clone(),
+ info: user_data.webauthn.get(index).unwrap().info.clone(),
},
Some((TfaType::U2f, index)) => TypedTfaInfo {
ty: TfaType::U2f,
- info: user_data.u2f.iter().nth(index).unwrap().info.clone(),
+ info: user_data.u2f.get(index).unwrap().info.clone(),
},
Some((TfaType::Yubico, index)) => TypedTfaInfo {
ty: TfaType::Yubico,
- info: user_data.yubico.iter().nth(index).unwrap().info.clone(),
+ info: user_data.yubico.get(index).unwrap().info.clone(),
},
None => return None,
},
@@ -195,7 +194,7 @@ pub fn delete_tfa(config: &mut TfaConfig, userid: &str, id: &str) -> Result<bool
match {
// scope to prevent the temporary iter from borrowing across the whole match
- let entry = tfa_id_iter(&user_data).find(|(_, _, entry_id)| id == *entry_id);
+ let entry = tfa_id_iter(user_data).find(|(_, _, entry_id)| id == *entry_id);
entry.map(|(ty, index, _)| (ty, index))
} {
Some((TfaType::Recovery, _)) => user_data.recovery = None,
@@ -308,6 +307,7 @@ fn need_description(description: Option<String>) -> Result<String, Error> {
/// Permissions for accessing `userid` must have been verified by the caller.
///
/// The caller must have already verified the user's password!
+#[allow(clippy::too_many_arguments)]
pub fn add_tfa_entry<A: OpenUserChallengeData>(
config: &mut TfaConfig,
access: A,
@@ -354,7 +354,7 @@ pub fn add_tfa_entry<A: OpenUserChallengeData>(
bail!("generating recovery tokens does not allow additional parameters");
}
- let recovery = config.add_recovery(&userid)?;
+ let recovery = config.add_recovery(userid)?;
Ok(TfaUpdateInfo {
id: Some("recovery".to_string()),
@@ -451,7 +451,7 @@ fn add_webauthn<A: OpenUserChallengeData>(
None => config
.webauthn_registration_challenge(
access,
- &userid,
+ userid,
need_description(description)?,
origin,
)
@@ -464,7 +464,7 @@ fn add_webauthn<A: OpenUserChallengeData>(
format_err!("missing 'value' parameter (webauthn challenge response missing)")
})?;
config
- .webauthn_registration_finish(access, &userid, &challenge, &value, origin)
+ .webauthn_registration_finish(access, userid, &challenge, &value, origin)
.map(TfaUpdateInfo::id)
}
}
diff --git a/proxmox-tfa/src/api/mod.rs b/proxmox-tfa/src/api/mod.rs
index b591a23..1f9fb2c 100644
--- a/proxmox-tfa/src/api/mod.rs
+++ b/proxmox-tfa/src/api/mod.rs
@@ -247,13 +247,13 @@ impl TfaConfig {
TfaResponse::U2f(value) => match &challenge.u2f {
Some(challenge) => {
let u2f = check_u2f(&self.u2f)?;
- user.verify_u2f(access.clone(), userid, u2f, &challenge.challenge, value)
+ user.verify_u2f(access, userid, u2f, &challenge.challenge, value)
}
None => bail!("no u2f factor available for user '{}'", userid),
},
TfaResponse::Webauthn(value) => {
let webauthn = check_webauthn(&self.webauthn, origin)?;
- user.verify_webauthn(access.clone(), userid, webauthn, value)
+ user.verify_webauthn(access, userid, webauthn, value)
}
TfaResponse::Recovery(value) => {
user.verify_recovery(&value)?;
@@ -587,7 +587,7 @@ impl TfaUserData {
None => None,
},
u2f: match u2f {
- Some(u2f) => self.u2f_challenge(access.clone(), userid, u2f)?,
+ Some(u2f) => self.u2f_challenge(access, userid, u2f)?,
None => None,
},
yubico: self.yubico.iter().any(|e| e.info.enable),
diff --git a/proxmox-tfa/src/api/recovery.rs b/proxmox-tfa/src/api/recovery.rs
index 9af2873..92c0e9d 100644
--- a/proxmox-tfa/src/api/recovery.rs
+++ b/proxmox-tfa/src/api/recovery.rs
@@ -12,7 +12,7 @@ fn getrandom(mut buffer: &mut [u8]) -> Result<(), io::Error> {
libc::getrandom(
buffer.as_mut_ptr() as *mut libc::c_void,
buffer.len() as libc::size_t,
- 0 as libc::c_uint,
+ 0,
)
};
@@ -49,7 +49,7 @@ impl Recovery {
getrandom(&mut secret)?;
let mut this = Self {
- secret: hex::encode(&secret).to_string(),
+ secret: hex::encode(&secret),
entries: Vec::with_capacity(10),
created: proxmox_time::epoch_i64(),
};
diff --git a/proxmox-tfa/src/api/serde_tools.rs b/proxmox-tfa/src/api/serde_tools.rs
index 1f307a2..b9f73d7 100644
--- a/proxmox-tfa/src/api/serde_tools.rs
+++ b/proxmox-tfa/src/api/serde_tools.rs
@@ -11,7 +11,7 @@ use serde::Deserialize;
pub struct FoldSeqVisitor<T, Out, F, Init>
where
Init: FnOnce(Option<usize>) -> Out,
- F: Fn(&mut Out, T) -> (),
+ F: Fn(&mut Out, T),
{
init: Option<Init>,
closure: F,
@@ -22,7 +22,7 @@ where
impl<T, Out, F, Init> FoldSeqVisitor<T, Out, F, Init>
where
Init: FnOnce(Option<usize>) -> Out,
- F: Fn(&mut Out, T) -> (),
+ F: Fn(&mut Out, T),
{
pub fn new(expecting: &'static str, init: Init, closure: F) -> Self {
Self {
@@ -37,7 +37,7 @@ where
impl<'de, T, Out, F, Init> serde::de::Visitor<'de> for FoldSeqVisitor<T, Out, F, Init>
where
Init: FnOnce(Option<usize>) -> Out,
- F: Fn(&mut Out, T) -> (),
+ F: Fn(&mut Out, T),
T: Deserialize<'de>,
{
type Value = Out;
@@ -104,7 +104,7 @@ pub fn fold<'de, T, Out, Init, Fold>(
) -> FoldSeqVisitor<T, Out, Fold, Init>
where
Init: FnOnce(Option<usize>) -> Out,
- Fold: Fn(&mut Out, T) -> (),
+ Fold: Fn(&mut Out, T),
T: Deserialize<'de>,
{
FoldSeqVisitor::new(expected, init, fold)
diff --git a/proxmox-tfa/src/api/webauthn.rs b/proxmox-tfa/src/api/webauthn.rs
index aed2885..4e90007 100644
--- a/proxmox-tfa/src/api/webauthn.rs
+++ b/proxmox-tfa/src/api/webauthn.rs
@@ -41,9 +41,9 @@ impl std::ops::DerefMut for OriginUrl {
}
}
-impl Into<String> for OriginUrl {
- fn into(self) -> String {
- self.0.into()
+impl From<OriginUrl> for String {
+ fn from(url: OriginUrl) -> String {
+ url.0.into()
}
}
diff --git a/proxmox-tfa/src/totp.rs b/proxmox-tfa/src/totp.rs
index d2e009b..1e342c4 100644
--- a/proxmox-tfa/src/totp.rs
+++ b/proxmox-tfa/src/totp.rs
@@ -20,9 +20,9 @@ pub enum Algorithm {
Sha512,
}
-impl Into<MessageDigest> for Algorithm {
- fn into(self) -> MessageDigest {
- match self {
+impl From<Algorithm> for MessageDigest {
+ fn from(algo: Algorithm) -> MessageDigest {
+ match algo {
Algorithm::Sha1 => MessageDigest::sha1(),
Algorithm::Sha256 => MessageDigest::sha256(),
Algorithm::Sha512 => MessageDigest::sha512(),
@@ -343,7 +343,7 @@ impl std::str::FromStr for Totp {
// FIXME: Also split on "%3A" / "%3a"
let mut account = account.splitn(2, |&b| b == b':');
let first_part = percent_decode(
- &account
+ account
.next()
.ok_or_else(|| anyhow!("missing account in otpauth uri"))?,
)
@@ -364,13 +364,13 @@ impl std::str::FromStr for Totp {
for parts in uri.split(|&b| b == b'&') {
let mut parts = parts.splitn(2, |&b| b == b'=');
let key = percent_decode(
- &parts
+ parts
.next()
.ok_or_else(|| anyhow!("bad key in otpauth uri"))?,
)
.decode_utf8()?;
let value = percent_decode(
- &parts
+ parts
.next()
.ok_or_else(|| anyhow!("bad value in otpauth uri"))?,
);
@@ -467,6 +467,8 @@ impl PartialEq<&str> for TotpValue {
return false;
}
+ // I don't trust that `.parse()` never starts accepting `0x` prefixes so:
+ #[allow(clippy::from_str_radix_10)]
match u32::from_str_radix(*other, 10) {
Ok(value) => self.value() == value,
Err(_) => false,
diff --git a/proxmox-tfa/src/u2f.rs b/proxmox-tfa/src/u2f.rs
index 84fea41..9175e14 100644
--- a/proxmox-tfa/src/u2f.rs
+++ b/proxmox-tfa/src/u2f.rs
@@ -579,7 +579,7 @@ mod bytes_as_base64url_nopad {
pub fn serialize<S: Serializer>(data: &[u8], serializer: S) -> Result<S::Ok, S::Error> {
serializer.serialize_str(&base64::encode_config(
- data.as_ref(),
+ data,
base64::URL_SAFE_NO_PAD,
))
}
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] [PATCH proxmox 6/6] bump proxmox-tfa to 2.0.0-1
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (18 preceding siblings ...)
2021-11-26 13:55 ` [pmg-devel] [PATCH proxmox 5/6] tfa: clippy fixes Wolfgang Bumiller
@ 2021-11-26 13:55 ` 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
21 siblings, 0 replies; 24+ messages in thread
From: Wolfgang Bumiller @ 2021-11-26 13:55 UTC (permalink / raw)
To: pmg-devel
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
proxmox-tfa/Cargo.toml | 2 +-
proxmox-tfa/debian/changelog | 12 +++++++
proxmox-tfa/debian/control | 66 ++++++++++++++++++------------------
3 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/proxmox-tfa/Cargo.toml b/proxmox-tfa/Cargo.toml
index b153ce3..616e7aa 100644
--- a/proxmox-tfa/Cargo.toml
+++ b/proxmox-tfa/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "proxmox-tfa"
-version = "1.3.2"
+version = "2.0.0"
authors = ["Proxmox Support Team <support@proxmox.com>"]
edition = "2018"
license = "AGPL-3"
diff --git a/proxmox-tfa/debian/changelog b/proxmox-tfa/debian/changelog
index 9dcf7b8..9553d17 100644
--- a/proxmox-tfa/debian/changelog
+++ b/proxmox-tfa/debian/changelog
@@ -1,3 +1,15 @@
+rust-proxmox-tfa (2.0.0-1) stable; urgency=medium
+
+ * add Webauthn::digest
+
+ * let OriginUrl deref to its inner Url and add FromStr/TryFrom/Into<String>
+ impls
+
+ * make configured webauthn origin optional, allow users to pass an
+ origin-override
+
+ -- Proxmox Support Team <support@proxmox.com> Tue, 23 Nov 2021 16:19:16 +0100
+
rust-proxmox-tfa (1.3.2-1) stable; urgency=medium
* fix instantiation of u2f context, use origin instead of always replacing
diff --git a/proxmox-tfa/debian/control b/proxmox-tfa/debian/control
index a7858bf..452f7c8 100644
--- a/proxmox-tfa/debian/control
+++ b/proxmox-tfa/debian/control
@@ -46,12 +46,12 @@ Suggests:
librust-proxmox-tfa+webauthn-rs-dev (= ${binary:Version})
Provides:
librust-proxmox-tfa+default-dev (= ${binary:Version}),
- librust-proxmox-tfa-1-dev (= ${binary:Version}),
- librust-proxmox-tfa-1+default-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+default-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+default-dev (= ${binary:Version})
+ librust-proxmox-tfa-2-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2+default-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+default-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+default-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - Rust source code
This package contains the source for the Rust proxmox-tfa crate, packaged by
debcargo for use with cargo and dh-cargo.
@@ -67,9 +67,9 @@ Depends:
librust-proxmox-uuid-1+default-dev,
librust-webauthn-rs-0.3+default-dev
Provides:
- librust-proxmox-tfa-1+api-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+api-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+api-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+api-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+api-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+api-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "api"
This metapackage enables feature "api" for the Rust proxmox-tfa crate, by
pulling in any additional dependencies needed by that feature.
@@ -84,12 +84,12 @@ Depends:
librust-proxmox-schema-1+default-dev
Provides:
librust-proxmox-tfa+proxmox-schema-dev (= ${binary:Version}),
- librust-proxmox-tfa-1+api-types-dev (= ${binary:Version}),
- librust-proxmox-tfa-1+proxmox-schema-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+api-types-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+proxmox-schema-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+api-types-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+proxmox-schema-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+api-types-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2+proxmox-schema-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+api-types-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+proxmox-schema-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+api-types-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+proxmox-schema-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "api-types" and 1 more
This metapackage enables feature "api-types" for the Rust proxmox-tfa crate, by
pulling in any additional dependencies needed by that feature.
@@ -104,9 +104,9 @@ Depends:
librust-proxmox-tfa-dev (= ${binary:Version}),
librust-libc-0.2+default-dev
Provides:
- librust-proxmox-tfa-1+libc-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+libc-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+libc-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+libc-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+libc-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+libc-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "libc"
This metapackage enables feature "libc" for the Rust proxmox-tfa crate, by
pulling in any additional dependencies needed by that feature.
@@ -119,9 +119,9 @@ Depends:
librust-proxmox-tfa-dev (= ${binary:Version}),
librust-proxmox-time-1+default-dev
Provides:
- librust-proxmox-tfa-1+proxmox-time-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+proxmox-time-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+proxmox-time-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+proxmox-time-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+proxmox-time-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+proxmox-time-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "proxmox-time"
This metapackage enables feature "proxmox-time" for the Rust proxmox-tfa crate,
by pulling in any additional dependencies needed by that feature.
@@ -134,9 +134,9 @@ Depends:
librust-proxmox-tfa-dev (= ${binary:Version}),
librust-proxmox-uuid-1+default-dev
Provides:
- librust-proxmox-tfa-1+proxmox-uuid-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+proxmox-uuid-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+proxmox-uuid-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+proxmox-uuid-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+proxmox-uuid-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+proxmox-uuid-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "proxmox-uuid"
This metapackage enables feature "proxmox-uuid" for the Rust proxmox-tfa crate,
by pulling in any additional dependencies needed by that feature.
@@ -149,9 +149,9 @@ Depends:
librust-proxmox-tfa-dev (= ${binary:Version}),
librust-serde-json-1+default-dev
Provides:
- librust-proxmox-tfa-1+serde-json-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+serde-json-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+serde-json-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+serde-json-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+serde-json-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+serde-json-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "serde_json"
This metapackage enables feature "serde_json" for the Rust proxmox-tfa crate,
by pulling in any additional dependencies needed by that feature.
@@ -166,9 +166,9 @@ Depends:
librust-serde-1+derive-dev,
librust-serde-json-1+default-dev
Provides:
- librust-proxmox-tfa-1+u2f-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+u2f-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+u2f-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+u2f-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+u2f-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+u2f-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "u2f"
This metapackage enables feature "u2f" for the Rust proxmox-tfa crate, by
pulling in any additional dependencies needed by that feature.
@@ -181,9 +181,9 @@ Depends:
librust-proxmox-tfa-dev (= ${binary:Version}),
librust-webauthn-rs-0.3+default-dev
Provides:
- librust-proxmox-tfa-1+webauthn-rs-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3+webauthn-rs-dev (= ${binary:Version}),
- librust-proxmox-tfa-1.3.2+webauthn-rs-dev (= ${binary:Version})
+ librust-proxmox-tfa-2+webauthn-rs-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0+webauthn-rs-dev (= ${binary:Version}),
+ librust-proxmox-tfa-2.0.0+webauthn-rs-dev (= ${binary:Version})
Description: Tfa implementation for totp and u2f - feature "webauthn-rs"
This metapackage enables feature "webauthn-rs" for the Rust proxmox-tfa crate,
by pulling in any additional dependencies needed by that feature.
--
2.30.2
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [pmg-devel] [PATCH multiple 0/7] PMG TFA support
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (19 preceding siblings ...)
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 ` Stoiko Ivanov
2021-11-28 21:17 ` [pmg-devel] applied-series: " Thomas Lamprecht
21 siblings, 0 replies; 24+ messages in thread
From: Stoiko Ivanov @ 2021-11-26 17:34 UTC (permalink / raw)
To: Wolfgang Bumiller; +Cc: pmg-devel
Huge thanks for your work on this!
* gave it a quick spin on a freshly setup PMG from ISO (by adding a user
and setting up TOTP for it) - works as advertised
* quickly glanced over the code (e.g. diffing API2/TFA.pm between PVE
and PMG) - sent a mail with 2 minor notes to one of the patches
* will do a more thorough review on Monday
Thanks again!
On Fri, 26 Nov 2021 14:55:04 +0100
Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:
> This touches multiple repos as it required some more ground-work on the
> rust side:
>
> 1) proxmox-tfa
> Aside from fixups and maintenance, patch 4 is the important one:
> The `origin` in the webauthn configuration is now *optional*.
>
> Note that the origin is generally required for webauthn, however, we
> also have clusters where the origin shouldn't be pinned cluster-wide.
>
> This does not really affect PVE as there we store the webauthn
> configuration separately and apply it only when it is used, but in
> PBS it's kept directly in tfa.json, and PMG for now does this too,
> although we *could* move it to pmg.conf or some other synced file if
> we wanted?
> That would in theory remove the need for this, but I think this is
> actually a more appropriate API anyway, since the two other parts of
> the config stay the same across a cluster, and the origin can simply
> be provided as an overriding parameter to the methods which actually
> make use of it.
>
> 2) proxmox-perl-rs
> pmg-rs is now moved into here, also, this contains fixups for the
> proxmox-tfa-crate-using pve-side.
> Since the newly introduced parameters are at the end and optional,
> and perlmod 0.9 supports trailing Option<> parameters as actual
> *optional* parameters, this may in theory even be API compatible with
> PVE, so hopefully no `Breaks` on old pve-access-control is required,
> but we'll see.
Option<> at the end translating to optional parameters in perl sounds very
nice! (thinking about replacing the set_proxy method for my ACME-proxy
series)
>
> 3) pmg-api
> Same login & TFA api updates as in PVE. The config API path is
> different, but that's not shared code anyway ;-)
> API2/TFA.pm is very similar to PVE, I think I got the method schemas
> wright, but I'm not used to the permissions system in PMG so please
> double-check this.
> The actual changes to the login code path is much shorter than in PVE
> since we did not actually have TFA support in there yet.
>
> 4) pmg-gui
> For now this only adds TFA login and the `TfaView` from WTK. The
> config (which in this case only means webauthn settings) part isn't
> there yet.
>
>
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread* [pmg-devel] applied-series: Re: [PATCH multiple 0/7] PMG TFA support
2021-11-26 13:55 [pmg-devel] [PATCH multiple 0/7] PMG TFA support Wolfgang Bumiller
` (20 preceding siblings ...)
2021-11-26 17:34 ` [pmg-devel] [PATCH multiple 0/7] PMG TFA support Stoiko Ivanov
@ 2021-11-28 21:17 ` Thomas Lamprecht
21 siblings, 0 replies; 24+ messages in thread
From: Thomas Lamprecht @ 2021-11-28 21:17 UTC (permalink / raw)
To: Wolfgang Bumiller, pmg-devel
On 26/11/2021 14:55, Wolfgang Bumiller wrote:
> This touches multiple repos as it required some more ground-work on the
> rust side:
applied series, thanks!
Made a few follow ups:
- restricted permissions of tfa add/modify to all but qusers, while the gui wouldn't
allow it for those, it still could be set up over the API and that'd be confusing at
best, as it would have any effect if I remember the quarantine user login flow somewhat
- added tfa to the API index of /config and added one for webauthn in tfa, makes pmgsh
work, among other things
- implemented a webauthn config button, UX could surely get improved a bit but it does
the job for now.
- added libqrcode js dependency also in pmg-gui, as that's the actual user, albeit pmg-api
can be fine too, it's not that clear with the split main-api and main-gui packages that
we only have in PMG.
- docs, well as at least some reference to the biggest newest feature in the docs may be
expected by some users ;)
^ permalink raw reply [flat|nested] 24+ messages in thread