On October 21, 2019 12:12 pm, Wolfgang Link wrote:
> comment inline
>
> On 10/18/19 11:25 AM, Fabian Grünbichler wrote:
>> On October 14, 2019 1:08 pm, Wolfgang Link wrote:
>>> ---
>>> src/PVE/ACME.pm | 16 ++++++++++++++++
>>> src/PVE/ACME/StandAlone.pm | 9 +--------
>>> 2 files changed, 17 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
>>> index d6b6e99..173af69 100644
>>> --- a/src/PVE/ACME.pm
>>> +++ b/src/PVE/ACME.pm
>>> @@ -76,6 +76,22 @@ sub fromjs($) {
>>> return from_json($_[0]);
>>> }
>>>
>>> +sub extract_challenge ($$) {
>>> + my ($challenges, $c_type) = @_;
>>> +
>>> + die "no challenges defined\n" if !$challenges;
>>> + die "no challenge type is defined \n" if !$c_type;
>>> +
>>> + my $tmp_challenges = [ grep {$_->{type} eq $c_type} @$challenges ];
>>> + die "no $c_type challenge defined in authorization\n"
>>> + if ! scalar $tmp_challenges;
>>> +
>>> + my $challenge = $tmp_challenges->[0];
>>> +
>>> + die "no token found in $c_type challenge\n" if !$challenge->{token};
>> strictly speaking, not all challenges must require a token. http-01 and
>> dns-01 do though ;)
> Would it be better to do an extra check if http-01 and dns-01 the token
> required?
I think it's okay for now, but maybe add a comment like
# only dns-01 and http-01 supported for now
# both require a challenge token
so that we can re-visit this if we add new challenge types or open this
up for third-party plugins in the future.
if you want you can also extend the condition to check for one of those
two types, but currently that should always evaluate to true ;)
>>> + return $challenge;
>>> +}
>>> +
>>> sub validating_url($$$$) {
>>> my ($acme, $auth, $auth_url, $node_config) = @_;
>>>
>>> diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
>>> index 965fb32..7910bfd 100644
>>> --- a/src/PVE/ACME/StandAlone.pm
>>> +++ b/src/PVE/ACME/StandAlone.pm
>>> @@ -49,14 +49,7 @@ sub validating_url {
>>> sub setup {
>>> my ($class, $acme, $authorization) = @_;
>>>
>>> - my $challenges = $authorization->{challenges};
>>> - die "no challenges defined in authorization\n" if !$challenges;
>>> -
>>> - my $http_challenges = [ grep {$_->{type} eq 'http-01'} @$challenges ];
>>> - die "no http-01 challenge defined in authorization\n"
>>> - if ! scalar $http_challenges;
>>> -
>>> - my $http_challenge = $http_challenges->[0];
>>> + my $http_challenge = extract_challenge($authorization->{challenges},
>>> "http-01");
>>>
>>> die "no token found in http-01 challenge\n" if
>>> !$http_challenge->{token};
>> this line should no longer be needed?
>>
>>>
>>> --
>>> 2.20.1
>>>
>>>
>>> _______________________________________________
>>> pve-devel mailing list
>>> pve-devel@pve.proxmox.com
>>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>>
>>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
