[pve-devel] [PATCH common 1/2] JSONSchema: add pve-tfa-secret option and format

Wolfgang Bumiller w.bumiller at proxmox.com
Mon Oct 28 15:13:40 CET 2019


On Mon, Oct 28, 2019 at 02:26:28PM +0100, Thomas Lamprecht wrote:
> On 10/28/19 12:20 PM, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
> > ---
> >  src/PVE/JSONSchema.pm | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> > index db38d44..3712872 100644
> > --- a/src/PVE/JSONSchema.pm
> > +++ b/src/PVE/JSONSchema.pm
> > @@ -530,6 +530,30 @@ PVE::JSONSchema::register_standard_option('pve-startup-order', {
> >      typetext => '[[order=]\d+] [,up=\d+] [,down=\d+] ',
> >  });
> >  
> > +register_format('pve-tfa-secret', \&pve_verify_tfa_secret);
> > +sub pve_verify_tfa_secret {
> > +    my ($key, $noerr) = @_;
> > +
> > +    # The old format used 16 base32 chars or 40 hex digits. Since they have a common subset it's
> > +    # hard to distinguish them without the our previous length constraints, so add a 'v2' of the
> > +    # format to support arbitrary lengths properly:
> > +    if ($key =~ /^v2-0x[0-9a-fA-F]{16,128}$/ || # hex
> > +        $key =~ /^v2-[A-Z2-7=]{16,128}$/ ||     # base32
> > +        $key =~ /^(?:[A-Z2-7=]{16}|[A-Fa-f0-9]{40})$/) # and the old pattern copy&pasted
> > +    {
> > +	return $key;
> > +    }
> > +
> > +    return undef if $noerr;
> > +
> > +    die "unable to decode TFA secret\n";
> > +}
> > +
> > +register_standard_option('pve-tfa-secret', {
> > +    description => "A TFA secret, base32 encoded or hexadecimal.",
> > +    type => 'string', format => 'pve-tfa-secret',
> > +});
> > +
> 
> Why do you register a standard option but then do not use it? 
> But actually, I like using the format more, IMO this is essential to
> PVE/MG, and thus should not be a standard-option at all, so I'd rather
> just remove the registering here, and keep the access-control API patch
> as is.

Right, I did the pve-common change first and thought it would make sense
as an option (as the description would be used in pve & pmg), but then
in pve-access-control thought the meaning of the actual API parameter
might change in the future with updates/changes to second factors and
then did not remove it afterwards, sorry.

Should I resend or will you fix it up when applying?




More information about the pve-devel mailing list