Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vesta ACME client: support asynchronous finalization #2278

Open
aarongable opened this issue Apr 5, 2023 · 3 comments
Open

Vesta ACME client: support asynchronous finalization #2278

aarongable opened this issue Apr 5, 2023 · 3 comments
Assignees

Comments

@aarongable
Copy link

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
Copy link
Contributor

hestiacp/hestiacp#3442

Seems to solve it for Hestia :)

@jaapmarcus
Copy link
Contributor

jaapmarcus commented Apr 6, 2023

For VestaCP users who want to do slightly less work:
myvesta#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
Copy link

myvesta commented Apr 6, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants