[pve-devel] [Patch V2 acme 09/12] Add function setup and teardown.

[Fabian Grünbichler]

On March 31, 2020 12:08 pm, Wolfgang Link wrote:
> These are the two main functions that a plugin should offer.
> Setup creates the endpoint at which Letsencrypt does the validation, teardown does the cleanup.
> 
> Signed-off-by: Wolfgang Link <w.link at proxmox.com>
> ---
>  src/PVE/ACME.pm            | 43 ++++++++++++++++++++++++++++++++++++++
>  src/PVE/ACME/StandAlone.pm | 16 +++++---------
>  2 files changed, 48 insertions(+), 11 deletions(-)
> 
> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
> index 114eb41..c10dca2 100644
> --- a/src/PVE/ACME.pm
> +++ b/src/PVE/ACME.pm
> @@ -23,6 +23,9 @@ file_set_contents
>  file_get_contents
>  );
>  
> +use PVE::ACME::Challenge;
> +use PVE::ACME::StandAlone;
> +
>  Crypt::OpenSSL::RSA->import_random_seed();
>  
>  my $LETSENCRYPT_STAGING = 'https://acme-staging-v02.api.letsencrypt.org/directory';
> @@ -493,6 +496,46 @@ sub request_challenge_validation {
>      return $return;
>  }
>  
> +# Setup the challange
> +# At the moment two plugin types are supproted

s/supproted/supported

> +# standalone: start an webserver an wait for the challenge
> +# dns: add an txt record over the given API

technically not true yet at this point, but not that important ;)

> +sub setup {
> +    my ($acme, $auth, $node_config) = @_;

I'd rather pass in a hash like described in the comment to the patch 
that calls this, although I have to say I am still not 100% convinced we 
even need this here.


my $plugin_conf = cfs_read_file('priv/acme_plugins.cfg');

foreach $auth
  my $domain = $auth->{identifier}->{value};
  my $domain_info = $hash->{$domain};
  die if !$domain_info;

  my $plugin_id = $domain_info->{plugin} // 'default';

  my $plugin_data = $plugin_conf->{ids}->{$plugin_id};
  die if !$plugin_data;

  my $plugin = PVE::ACME::Challenge->lookup($plugin_data->{tpye};
  $plugin->setup($acme, $auth, $domain_info->{alias} // $domain, $plugin_data);

is short enough, and only needed in one place $order_certificate (in 
pve-manager)


> +
> +    my $fqdn = $auth->{identifier}->{value};
> +    my $plugin_data = {};
> +    my $plugin_type = "standalone";
> +    my $plugin_conf = PVE::ACME::Challenge->load_config();
> +
> +    # default is standalone if no plugin is set (old config)
> +    my $index = 0;
> +    while (defined($node_config->{$index})) {
> +	if ($node_config->{$index}->{domain} eq $fqdn &&
> +	    defined($node_config->{$index}->{plugin})) {
> +	    $plugin_data = $plugin_conf->{ids}->{$node_config->{$index}->{plugin}};
> +	    $plugin_type = $plugin_data->{type};
> +
> +	    # Alias mode is only supported for DNSChallange
> +	    $plugin_data->{alias} = $node_config->{$index}->{alias} if
> +		$node_config->{$index}->{alias};
> +	    last;
> +	}
> +	$index++;
> +    }
> +    die if $plugin_type eq "standalone";
> +    my $plugin = PVE::ACME::Challenge->lookup($plugin_data->{type});
> +    return $plugin->setup($acme, $auth, $plugin_data);
> +}
> +
> +#
> +#
> +sub teardown {
> +    my ($self) = @_;

this is a misuse - $self refers to a blessed plugin here, not to an 
instance of PVE::ACME.. should be dropped as well, see above.

> +
> +    $self->teardown();
> +}
> +
>  # actually 'do' a $method request on $url
>  # $data: input for JWS, optional
>  # $use_jwk: use JWK instead of KID in JWD (see sub jws)
> diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
> index ac75184..8fc8dc9 100644
> --- a/src/PVE/ACME/StandAlone.pm
> +++ b/src/PVE/ACME/StandAlone.pm
> @@ -31,15 +31,8 @@ sub options {
>  sub setup {
>      my ($class, $acme, $authorization) = @_;

since this patch adds a new parameter, it would be good to add it here 
as well to keep the signatures in sync.

>  
> -    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];
> -
> +    print "Setting up webserver\n";

could add $domain to this message if passed in?

> +    my $http_challenge = PVE::ACME::extract_challenge($authorization->{challenges}, "http-01");
>      die "no token found in http-01 challenge\n" if !$http_challenge->{token};
>  
>      my $key_authorization = $acme->key_authorization($http_challenge->{token});
> @@ -47,7 +40,7 @@ sub setup {
>      my $server = HTTP::Daemon->new(
>  	LocalPort => 80,
>  	ReuseAddr => 1,
> -    ) or die "Failed to initialize HTTP daemon\n";
> +	) or die "Failed to initialize HTTP daemon\n";
>      my $pid = fork() // die "Failed to fork HTTP daemon - $!\n";
>      if ($pid) {
>  	my $self = {
> @@ -62,7 +55,8 @@ sub setup {
>      } else {
>  	while (my $c = $server->accept()) {
>  	    while (my $r = $c->get_request()) {
> -		if ($r->method() eq 'GET' and $r->uri->path eq "/.well-known/acme-challenge/$http_challenge->{token}") {
> +		if ($r->method() eq 'GET' and
> +		    $r->uri->path eq "/.well-known/acme-challenge/$http_challenge->{token}") {
>  		    my $resp = HTTP::Response->new(200, 'OK', undef, $key_authorization);
>  		    $resp->request($r);
>  		    $c->send_response($resp);
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>