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

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 25/10/2023 um 15:07 schrieb Folke Gleumes:
> Changes v1 -> v2:
> Switched from including the eab credentials in the info hash,
> to passing them in their own variable. This still unfortunately still 
> breaks the api, but doesn't potentially expose secrets and is
> cleaner then purging the values from the hash afterwards.

yeah, IMO the signature of that method is still not really ideal, i.e.,
adding that as explicit param is a lateral move and still breaks ABI, so
meh, and I'd prefer

>  src/PVE/ACME.pm | 42 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
> index 3f66182..7b3840b 100644
> --- a/src/PVE/ACME.pm
> +++ b/src/PVE/ACME.pm

> @@ -251,6 +251,28 @@ sub jws {
>      };
>  }
>  
> +# EAB signing using the HS256 alg (HMAC/SHA256).
> +sub external_account_binding_jws {

should this be actually a private method? I.e., some

my sub external_account_binding_jws { }

or as code ref in a variable:

my $external_account_binding_jws = sub { }

FYI: For above examples you'd need to pass $self then explicitly
either way though.

As this is probably not intended to be called from the "outside"?
Albeit, that would be yet another way to avoid API breakage: let the
caller call this first and place the result into the %info hash, but
that would still keep that unchecked passing along of the %info hash.