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

Renew LetsEncrypt certificates #641

Merged

Conversation

iulianbarbu
Copy link
Contributor

@iulianbarbu iulianbarbu commented Feb 19, 2023

Closes #621 .

Description

This PR adds functionality that enables the periodical renewal of LetsEncrypt certificates. These certificates have a validity window of 90 days and is recommended to be renewed 30 days before expiration. The functionality will cover renewal for:

  1. certificates emitted for a custom domain project when requested through the admin gateway API "/admin/acme/request/:project_name/:fqdn" or through shuttle-admin CLI.
  2. the gateway certificate, which is loaded when starting the gateway service or created based on a set of ACME credentials present on the local filesystem.

WIP testing

  1. End-to-end manual test of the certificate renewal with all shuttle components deployed locally. I have some issues with the shuttle local deployment that blocks me while testing. I need some more time to understand how to enable shuttle locally, with pebble deployed locally as CA.

  2. Unit testing for the introduced business logic where it makes sense and is possible. Spent lots of time on 1) and I'll need to come back at this.

I welcome any suggestions on steps/guides/docs for how to enable the shuttle components locally to test the certificates renewal.

Documentation

Haven't seen much documentation on how to test the admin functionality related to the certificates requesting, so I hope I'll be able to compile something worth mentioning after finishing this.

Review wise

Besides code review, I am curious at the beginning to understand if this PR covers what's asked from #621 functionality-wise. I would be happy to follow up with adjustments code-wise if there are suggestions for improvement. Also, in parallel, I'll try to come back with information on the testing aspect.

@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch 4 times, most recently from 4dd4956 to a6a669e Compare February 19, 2023 21:55
@brokad brokad self-requested a review February 20, 2023 08:53
@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch from a6a669e to 861b868 Compare February 20, 2023 09:08
@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented Feb 20, 2023

Testing update

I am blocked when requesting a certificate by the fact that my pebble local deployment finalizes an order but it's not returning the certificate url, needed by the ACME client to download the certificate. Can't say what's the order status after finalizing, I'll need to debug by working through the instance-acme dependency code.

So far, the pebble CA log looks like this:

Pebble 2023/02/20 17:55:06 ACME directory available at: https://0.0.0.0:14000/dir
Pebble 2023/02/20 17:56:55 GET /dir -> calling handler()
Pebble 2023/02/20 17:56:55 HEAD /nonce-plz -> calling handler()
Pebble 2023/02/20 17:56:55 POST /sign-me-up -> calling handler()
Pebble 2023/02/20 17:56:55 There are now 1 accounts in memory
Pebble 2023/02/20 17:57:19 HEAD /nonce-plz -> calling handler()
Pebble 2023/02/20 17:57:19 POST /order-plz -> calling handler()
Pebble 2023/02/20 17:57:19 There are now 1 authorizations in the db
Pebble 2023/02/20 17:57:19 Added order "niAlrdD77-VhHeU6raADNxxxYuZSjCqQujLmUTCmirI" to the db
Pebble 2023/02/20 17:57:19 There are now 1 orders in the db
Pebble 2023/02/20 17:57:19 POST /authZ/ -> calling handler()
Pebble 2023/02/20 17:57:19 POST /chalZ/ -> calling handler()
Pebble 2023/02/20 17:57:19 Pulled a task from the Tasks queue: &va.vaTask{Identifier:acme.Identifier{Type:"dns", Value:"hello-world-rocket-app.unstable.shuttleapp.rs"}, Challenge:(*core.Challenge)(0xc0001c8280), Account:(*core.Account)(0xc0000785a0)}
Pebble 2023/02/20 17:57:19 Starting 3 validations.
Pebble 2023/02/20 17:57:19 Sleeping for 3s seconds before validating
Pebble 2023/02/20 17:57:19 Sleeping for 2s seconds before validating
Pebble 2023/02/20 17:57:19 Sleeping for 2s seconds before validating
Pebble 2023/02/20 17:57:20 POST /my-order/ -> calling handler()
Pebble 2023/02/20 17:57:20 POST /my-order/ -> calling handler()
Pebble 2023/02/20 17:57:21 POST /my-order/ -> calling handler()
Pebble 2023/02/20 17:57:21 Attempting to validate w/ HTTP: http://hello-world-rocket-app.unstable.shuttleapp.rs:7999/.well-known/acme-challenge/trr79hstEGoLoSkdJfdQWxAZq7Ufl1BDG4PmKzsMVks
Pebble 2023/02/20 17:57:21 Attempting to validate w/ HTTP: http://hello-world-rocket-app.unstable.shuttleapp.rs:7999/.well-known/acme-challenge/trr79hstEGoLoSkdJfdQWxAZq7Ufl1BDG4PmKzsMVks
Pebble 2023/02/20 17:57:22 Attempting to validate w/ HTTP: http://hello-world-rocket-app.unstable.shuttleapp.rs:7999/.well-known/acme-challenge/trr79hstEGoLoSkdJfdQWxAZq7Ufl1BDG4PmKzsMVks
Pebble 2023/02/20 17:57:22 authz UFddOjhDm8lo3tgfjdAvU4pJYo8l8iIgTYFrIE4IsvE set VALID by completed challenge -lhkvnOJCIG4rRoDj6b-ijmsfUD_GiKAXKkl1ekwmo8
Pebble 2023/02/20 17:57:23 POST /my-order/ -> calling handler()
**Pebble 2023/02/20 17:57:23 POST /finalize-order/ -> calling handler()
Pebble 2023/02/20 17:57:23 Order niAlrdD77-VhHeU6raADNxxxYuZSjCqQujLmUTCmirI is fully authorized. Processing finalization
Pebble 2023/02/20 17:57:23 Issued certificate serial 05cf8f20a311617a for order niAlrdD77-VhHeU6raADNxxxYuZSjCqQujLmUTCmirI**

It seems that the order finalization succeeded with a certificate issued, but the finalization response doesn't contain the certificate URL, or the logs don't mention it sadly.

The gateway logs look like this:

Screenshot 2023-02-20 at 18 27 22

The place where this fails is in instant-acme finalize order method:
Screenshot 2023-02-20 at 18 28 47

gateway/src/main.rs Outdated Show resolved Hide resolved
gateway/src/main.rs Outdated Show resolved Hide resolved
gateway/src/api/latest.rs Outdated Show resolved Hide resolved
@chesedo
Copy link
Contributor

chesedo commented Feb 21, 2023

Hey @iulianbarbu, thanks for the PR 😄

It seems that the order finalization succeeded with a certificate issued, but the finalization response doesn't contain the certificate URL, or the logs don't mention it sadly.

Gateway should create a trace event with "got authorization" (a few lines earlier). Iirc, this log will contain some challenge URL that can help with the debugging

@iulianbarbu
Copy link
Contributor Author

Hey @chesedo ! Thanks for taking a look. On the pebble side, the above log I sent contains this at the end, but haven't checked the corresponding trace events on the gateway side. I'll try again.

Pebble 2023/02/20 17:57:23 POST /my-order/ -> calling handler()
Pebble 2023/02/20 17:57:23 POST /finalize-order/ -> calling handler()
Pebble 2023/02/20 17:57:23 Order niAlrdD77-VhHeU6raADNxxxYuZSjCqQujLmUTCmirI is fully authorized. Processing finalization
Pebble 2023/02/20 17:57:23 Issued certificate serial 05cf8f20a311617a for order niAlrdD77-VhHeU6raADNxxxYuZSjCqQujLmUTCmirI

@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch 2 times, most recently from 7af6922 to 5ad20c0 Compare February 21, 2023 12:49
@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented Feb 21, 2023

@brokad , I've addressed your comments here: 8b9f484. Let me know in case you have other concerns.

Other than this, I will take another stab at the end-to-end manual test and then unit testing + docs.

@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch 3 times, most recently from 8b9f484 to 2d81d54 Compare February 22, 2023 17:52
@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch 4 times, most recently from 31e7d4a to 87afe4f Compare March 3, 2023 18:22
Copy link
Contributor

@chesedo chesedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates @iulianbarbu

admin/src/args.rs Show resolved Hide resolved
@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch 5 times, most recently from 03dfb60 to 6bbc4da Compare March 21, 2023 13:16
@iulianbarbu iulianbarbu changed the title [RFC] Renew LetsEncrypt certificates Renew LetsEncrypt certificates Mar 21, 2023
Copy link
Contributor

@chesedo chesedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Containerfile Outdated Show resolved Hide resolved
gateway/src/acme.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@oddgrd oddgrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this Iulian, LGTM to me too! I left a few comments but mostly just nice things and a question or two 😄

Makefile Outdated Show resolved Hide resolved
admin/README.md Outdated Show resolved Hide resolved
gateway/Cargo.toml Show resolved Hide resolved
gateway/src/acme.rs Outdated Show resolved Hide resolved
gateway/src/main.rs Outdated Show resolved Hide resolved
...and gateway certificates renewal.

* added a new `request-gateway-certificate` admin command to
renew the gateway certificate on-deman.

* added gateway admin APIs for custom domains and gateway
certificates renewal.

Note: requesting renewal for the gateway certificate requires inserting
manually a DNS TXT record to complete the ACME DNS-01 challenge.

Signed-off-by: Iulian Barbu <iulianbarbu2@gmail.com>
...and removed Makefile containters additional build args for
panamax and postgres. It seems there is a 'PLATFORMS' env variable
in the Makefile which can be set before building the images and used
by `buildx` to build against the mentioned target platforms.

The `PLATFORMS` env variable is not needed on my machine because I use
Docker Desktop, which supports running containers built for other arch
using behind the scenes a LinuxKit VM that intercepts [1][2] the binaries
and runs them using binfmt_misc, through QEMU [1].

[1] https://www.docker.com/blog/the-magic-behind-the-scenes-of-docker-desktop/
[2] https://stackoverflow.com/questions/72444103/what-does-running-the-multiarch-qemu-user-static-does-before-building-a-containe

Signed-off-by: Iulian Barbu <iulianbarbu2@gmail.com>
Simplified the functions' signatures to use only the `AccountCredentials`,
which are received from the API requests and read as well from the gateway
state when needed.
...when needing to request/renew custom-domain and gateway certificates
on the local environment.
Strings that are returned by gateway API requests must
be wrapped within double quotes to be deserialized with
serde_json.
@iulianbarbu iulianbarbu force-pushed the renew_lets_encrypt_certificates_621 branch from 6bbc4da to b177ac1 Compare March 21, 2023 22:02
Copy link
Contributor

@chesedo chesedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the edits @iulianbarbu. LGTM

Containerfile Show resolved Hide resolved
@iulianbarbu iulianbarbu merged commit 6843874 into shuttle-hq:main Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Renew LetsEncrypt certificates arriving at expiration
4 participants