search

outroll / vesta

VESTA Control Panel
http://vestacp.com
GNU General Public License v3.0
2.97k stars 1.03k forks source link

Vesta ACME client: support asynchronous finalization #2278

Open aarongable opened 1 year ago

aarongable commented 1 year ago

Operating System (OS/VERSION):

All

VestaCP Version:

All

Bug:

When issuing an SSL certificate from Let's Encrypt, the Vesta ACME Client does not properly implement finalization.

According to RFC 8555, making a "finalize" request does not guarantee immediate issuance. Instead, it will return an Order object with status "pending". The client should only expect a certificate to be issued (and therefore expect a "certificate" url field to appear in the Order obejct) once the Order's "status" field transitions to state "valid".

If the Order object returned by Finalize isn't already in state "valid", the client should poll the order object (preferably with exponential backoff!) until the state transitions to either "valid" or "invalid". If it transitions to "invalid", then issuance failed. If it transitions to "valid", then issuance succeeded, and the client should be able to download the certificate from the "certificate" url.

The code that needs to change is here: https://github.com/serghey-rodin/vesta/blob/73d60c45917514877f691f602273d50448453505/bin/v-add-letsencrypt-domain#L308-L314 Similar polling code already exists here: https://github.com/serghey-rodin/vesta/blob/73d60c45917514877f691f602273d50448453505/bin/v-add-letsencrypt-domain#L263-L290

Related Issues/Forum Threads:

https://community.letsencrypt.org/t/myvesta-hestiacp-vestacp-fail-issuance-with-async-finalization/195923

Other Notes:

It would be useful to add a useragent to the ACME curl requests as well, so that Let's Encrypt can identify Vesta-derived clients in the future.

It would also be useful for the Vesta project to regularly test issuance against Let's Encrypt's Staging API (https://letsencrypt.org/docs/staging-environment/), against Pebble (https://github.com/letsencrypt/pebble), or against other ACME Servers (such as ZeroSSL or Google Trust Services), any of which would have revealed this bug earlier.

jaapmarcus commented 1 year ago

https://github.com/hestiacp/hestiacp/pull/3442

Seems to solve it for Hestia :)

jaapmarcus commented 1 year ago

For VestaCP users who want to do slightly less work: https://github.com/myvesta/vesta/pull/157

@anton-reutov feel free to use it with a credit :)

Please note the Changes @aarongable makes will break existing Vesta setups after 24th of April when https://community.letsencrypt.org/t/enabling-asynchronous-order-finalization/193522 goes in effect (If not delayed)

myvesta commented 1 year ago

I think Vesta can take whole files: https://github.com/myvesta/vesta/blob/0.9.8-26-62/bin/v-add-letsencrypt-domain https://github.com/myvesta/vesta/blob/0.9.8-26-62/bin/v-add-letsencrypt-user ... since myVesta and VestaCP are still 100% compatible in bash segment.

Just change --user-agent 🙂

HestiaCP made the fix.