Re: [pve-devel] [PATCH manager 2/5] fix #4497: acme: add support for external account bindings

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On October 23, 2023 3:18 pm, Folke Gleumes wrote:
> Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com>
> ---
>  PVE/API2/ACMEAccount.pm | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/ACMEAccount.pm b/PVE/API2/ACMEAccount.pm
> index b790843a..daae18d8 100644
> --- a/PVE/API2/ACMEAccount.pm
> +++ b/PVE/API2/ACMEAccount.pm
> @@ -115,6 +115,16 @@ __PACKAGE__->register_method ({
>  		default => $acme_default_directory_url,
>  		optional => 1,
>  	    }),
> +	    eab_kid => {
> +		type => 'string',
> +		description => 'Key Identifier for External Account Binding.',
> +		optional => 1,
> +	    },
> +	    eab_hmac_key => {
> +		type => 'string',
> +		description => 'HMAC key for External Account Binding.',
> +		optional => 1,
> +	    },

Nit: s/_/-/ for new parameters :)
>  	},
>      },
>      returns => {
> @@ -130,8 +140,15 @@ __PACKAGE__->register_method ({
>  	my $account_file = "${acme_account_dir}/${account_name}";
>  	mkdir $acme_account_dir if ! -e $acme_account_dir;
>  
> +	my $eab_kid = extract_param($param, 'eab_kid');
> +	my $eab_hmac_key = extract_param($param, 'eab_hmac_key');
> +
>  	raise_param_exc({'name' => "ACME account config file '${account_name}' already exists."})
>  	    if -e $account_file;
> +	raise_param_exc({'eab_kid' => "'eab_hmac_key' must be defined if 'eab_kid' is set."})
> +	    if defined($eab_kid) and not defined($eab_hmac_key);
> +	raise_param_exc({'eab_hmac_key' => "'eab_kid' must be defined if 'eab_hmac_key' is set."})
> +	    if defined($eab_hmac_key) and not defined($eab_kid);

these two checks can be encoded directly in the schema by adding

requires => "name-of-require-parameter"

to both definitions, pointing at the other one. if a caller only
provides either of them and not both (or none), the schema check will
error:

eab_hmac_key: missing property - 'eab_kid' requires this property

without needing any manual handling in the API endpoint handler sub.

>  
>  	my $directory = extract_param($param, 'directory') // $acme_default_directory_url;
>  	my $contact = $account_contact_from_param->($param);
> @@ -145,7 +162,15 @@ __PACKAGE__->register_method ({
>  		print "Generating ACME account key..\n";
>  		$acme->init(4096);
>  		print "Registering ACME account..\n";
> -		eval { $acme->new_account($param->{tos_url}, contact => $contact); };
> +		my $info = {contact => $contact};
> +		if (defined($eab_kid) and defined($eab_hmac_key)) {
> +		    $info->{eab} = {
> +			kid => $eab_kid,
> +			hmac_key => $eab_hmac_key
> +		    };
> +		}
> +
> +		eval { $acme->new_account($param->{tos_url}, $info); };

if you switch this line to %$info or $info->%*, the new_account sub can
still take the hash directly instead of a reference, but see comments on
the proxmox-acme patch for possibly nicer signatures.

>  		if (my $err = $@) {
>  		    unlink $account_file;
>  		    die "Registration failed: $err\n";
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>