[pve-devel] [PATCH manager v2] ACMEv2 order "ready" status update

[Fabian Grünbichler]

LGTM, besides on nit inline (but haven't tested from home - currently
staging has the feature enabled, production does not, so this should be
quick on your end ;)).

On Wed, Jun 20, 2018 at 11:56:05AM +0200, Dominik Csapak wrote:
> since letsencrypt updates their implementation to the ACMEv2 spec [1],
> we should correctly parse the order status
> 
> 1: https://community.letsencrypt.org/t/acmev2-order-ready-status/62866
> 
> note that we (for now) try to be compatbile to both versions,
> with and without ready state, this can be changed when all letsencrypt
> apis have changed
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> changes from v1:
> * try finalizing during 'pending' state with max 5 tries
> * change sleep to 5 seconds after finalizing
>  PVE/API2/ACME.pm | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/ACME.pm b/PVE/API2/ACME.pm
> index 3c85458b..b1bb6261 100644
> --- a/PVE/API2/ACME.pm
> +++ b/PVE/API2/ACME.pm
> @@ -90,14 +90,36 @@ my $order_certificate = sub {
>      print "\nCreating CSR\n";
>      my ($csr, $key) = PVE::Certificate::generate_csr(identifiers => $order->{identifiers});
>  
> -    print "Finalizing order\n";
> -    $acme->finalize_order($order, PVE::Certificate::pem_to_der($csr));
> -
> +    my $finalize_error_cnt = 0;
>      print "Checking order status\n";
>      while (1) {
>  	$order = $acme->get_order($order_url);
>  	if ($order->{status} eq 'pending') {
> -	    print "still pending, trying again in 30 seconds\n";
> +	    print "still pending, trying to finalize order\n";
> +	    # FIXME
> +	    # to be compatible with and without the order ready state
> +	    # we try to finalize even at the 'pending' state
> +	    # and give up after 5 unsuccessful tries
> +	    # this can be removed when the letsencrypt api
> +	    # definitely has implemented the 'ready' state
> +	    eval {
> +		$acme->finalize_order($order, PVE::Certificate::pem_to_der($csr));
> +	    };
> +	    if (my $err = $@) {
> +		die $err if $finalize_error_cnt >= 5;
> +
> +		$finalize_error_cnt++;
> +		warn $err;

I don't think we need multiple attempts here - the logic in LE's CA
software calculates the order status based on the authorizations. at
this point we have already checked all the authorizations, so if it is
"pending", the "order status ready" feature is not enabled, and we only
need one finalization attempt just like before. if the feature is
enabled, at this point the status must be "ready" anyway.

> +	    }
> +	    sleep 5;
> +	    next;
> +	} elsif ($order->{status} eq 'ready') {
> +	    print "Order is ready, finalizing order\n";
> +	    $acme->finalize_order($order, PVE::Certificate::pem_to_der($csr));
> +	    sleep 5;
> +	    next;
> +	} elsif ($order->{status} eq 'processing') {
> +	    print "still processing, trying again in 30 seconds\n";
>  	    sleep 30;
>  	    next;
>  	} elsif ($order->{status} eq 'valid') {
> -- 
> 2.11.0
> 
>