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

[Fabian Grünbichler]

On Fri, Jun 22, 2018 at 10:45:02AM +0200, Stoiko Ivanov wrote:
> FWIW, tested it quickly with the current staging - both our current
> master, as well as the version patched with this worked and provided me
> with a certificate.

might work, should be racy though in master:

    PVE                 LE
- finalize
                order is processing --
                certificate is actually issued --
                order is valid --
- get order
- continue

vs

    PVE                 LE
- finalize
                order is processing --
- get order
- die
                certificate is actually issued --
                order is valid --

I don't think that part is new though, still needs to be fixed ASAP.

> On Thu, 21 Jun 2018 20:34:14 +0200
> Fabian Grünbichler <f.gruenbichler at proxmox.com> wrote:
> 
> > 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
> > > 
> > >   
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel at pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>