[pve-devel] [common 6/9] command composer for acme.sh

[Fabian Grünbichler]

On October 21, 2019 12:11 pm, Wolfgang Link wrote:
> comment inline
> 
> On 10/18/19 11:27 AM, Fabian Grünbichler wrote:
>> On October 14, 2019 1:08 pm, Wolfgang Link wrote:
>>> This composer supports two different operations.
>>> pve-setup:     this operation adds the DNS TXT record.
>>> pve-teardown:  this operation removes the DNS TXT record
>>> ---
>>>   src/PVE/ACME/ACME_sh.pm | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/src/PVE/ACME/ACME_sh.pm b/src/PVE/ACME/ACME_sh.pm
>>> index db8af9a..40be772 100644
>>> --- a/src/PVE/ACME/ACME_sh.pm
>>> +++ b/src/PVE/ACME/ACME_sh.pm
>>> @@ -38,6 +38,22 @@ my $get_dnsapi_conf = sub {
>>>       return ($api_plugin, "$API_CRED_DIR/$api_plugin.cred");
>>>   };
>>>   
>>> +my $compose_cmd = sub {
>>> +    my ($op, $token, $domain, $alias) = @_;
>> $token is not the token from the challenge, but the base64url-encoded,
>> hashed key_authorization? please name variables for what they actually
>> contain..
>>
>>> +
>>> +    my ($dns_api_plugin, $cred_file_path) = &$get_dnsapi_conf();
>>> +
>>> +    # valid operations for this composer are pve-setup and pve-teardown
>>> +    my @cmd = ('/usr/sbin/acme', "--$op");
>>> +    push @cmd, '--webroot', $dns_api_plugin;
>> huh? webroot is something different altogether, why use this term here?
> Internal at the acme.sh script not.

no. they call their 'mode of validation' variable _webroot since that is 
what they implemented first. the actual values are '--webroot' 
'--standalone' '--alpn' '--stateless' '--apache' '--dns PLUGIN' and 
'--nginx'. so this should be '--dns', not '--webroot'?

we control the definition of 'acme.sh pve-setup', and there is no need 
to put any reference to webroot there. webroot is an entirely different 
validation method which re-uses an existing webserver. none of the other 
non-webroot modes use that parameter (see the docs for nginx, apache, 
dnsapi modes..). we can also add our own custom parameters to _process 
if the existing ones are too limiting.

>>> +    push @cmd, '--domain', "_acme-challenge.$domain";
>> either the domain is $domain (if it is still used to derive some
>> validation response value somehow?)
>>
>>> +    push @cmd, '--token', $token;
>> same here..
>>
>>> +    push @cmd, '--accountconf', $cred_file_path;
>>> +    push @cmd, '--challenge-alias', $alias if defined($alias);
>> or the domain should be replaced with the aliased domain, since it just
>> signifies under which key the TXT record is created?
>>
>> this command is supposed to be just a thin wrapper around the DNS API
>> plugins, I'd expect the following:
>>
>> acme --pve-setup --plugin-conf $cred_file_path --plugin foo --domain $fulldomain --txtvalue $txtvalue
>>
>> where $fulldomain is either the regular domain, or the alias.. or am I
>> missing something here?
> 
> The FQDN Letsencrypt is locking for is
> 
> _acme-challenge.[subdomain.]<Domain>.<TLD>
> 
> This can be a CNAME record or a TXT record.
> 
> The CNAME must be a redirection to the TXT record.
> 
> If _acme-challenge. is set on our site or in the wrapper makes no mater.

we don't need to pass in both the certificate domain and the alias 
domain. we just want to set a single TXT record
- for _acme-challenge.$domain if no alias is set
- for $alias if an alias is set (depending on which of the alias modes 
  we want to support, there are two ;))

regular acme.sh needs both, since they store the config/certificate/... 
using the domain as key, but do the DNS validation using the alias. we 
just do the DNS validation here, the rest is already handled further up 
the stack.

> 
>>> +
>>> +    return \@cmd;
>>> +};
>>> +
>>>   sub validating_url {
>>>       my ($class, $acme, $auth, $auth_url, $node_config) = @_;
>>>   
>>> -- 
>>> 2.20.1
>>>
>>>
>>> _______________________________________________
>>> pve-devel mailing list
>>> pve-devel at pve.proxmox.com
>>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>>
>>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>