Am 24/10/2023 um 10:32 schrieb Fabian Grünbichler:
>> # Optionally pass $tos_url to agree to the given Terms of Service
>> # POST to newAccount endpoint
>> # Expects a '201 Created' reply
>> # Saves and returns the account data
>> sub new_account {
>> - my ($self, $tos_url, %info) = @_;
>> + my ($self, $tos_url, $info) = @_;
>> my $url = $self->_method('newAccount');
>>
>> + if ($info->{'eab'}) {
>> + my $eab_hmac_key = decode_base64($info->{'eab'}->{hmac_key});
>> + $info->{externalAccountBinding} = $self->eab($info->{'eab'}->{kid},
>> $eab_hmac_key, $url);
>
> this means that `info` now contains both the binding, but also the input
> including the KID (okay, this is contained in the binding as well, so
> just duplicate info) and the HMAC key, which is supposed to be secret.
> granted, it is a secret given to the user by the CA over some channel,
> and we only send it back to the CA, but some ACME implementations might
> still reject the request because of the unexpected contents. and if the
> user ever mixes up the CAs they are talking to, they might accidentally
> leak the secret to the wrong entitiy.
passing %info direct around seems like a bad ABI anyway, so why not
stop doing that and construct a new hash here that only takes the
properties from info out that we actually care about?
>
> since `info` is directly translated to the new_account request contents,
> it might be better to pass in the EAB parameters on their own, and make
> this sub take
>
> my ($self, $tos_url, $eab, %info) = @_;
>
> if $eab is undef -> no EAB. if it is set, generate the binding and put
> it into %info for further passing to the ACME provider.
IMO this is not really a better general API (the unconstrained passing
around of %info, while requiring further ABI breakage on any future
parameter addition, is still there).
>
> alternatively, it would also work to combine $tos_url and $eab into a
> new $account_params or $params hash, if we think that more parameters
> might be added in the future, or if we plan on merging new_account and
> init, like we do in PMG/PBS.
>
> or, as third option, we could switch the public sub new_account to take
>
> my ($self, $tos_url, $contact, $eab) = @_;
>
> or
>
> my ($self, $tos_url, $contact, $eab, $rsa_bits) = @_;
>
> which would be more aligned with how PMG and PBS look like.
Aligning more towards PBS/PMG has it's merits, FWIW, we could also do
so with a new method, and then transform the existing one to just transform
the parameters as needed and call into that.
>
> almost all of the variants are a breaking change (except if we keep the
> signature as is, and properly extract the eab member if it is set) that
> require a double check for any reverse dependencies. there's at least
> one internal tool that uses this as well that would need to be updated,
> for example.
>
Yeah, that or adding a new method would be preferred from my side.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
