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

Allow custom ACME server #204

Closed
wants to merge 6 commits into from
Closed

Conversation

tashian
Copy link

@tashian tashian commented Jan 11, 2022


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

Support for a custom ACME server and CA bundle.
I've added ACMESERVER and ACMECABUNDLE variables to support this.
The root CA bundle in ACMECABUNDLE is expected to be a base64 encoded PEM file.
The config init script decodes and saves the CA bundle to /config/cabundle.pem.
The variable REQUESTS_CA_BUNDLE is exported for all calls to certbot, so that certbot can establish CA trust.

Benefits of this PR and context:

Internal ACME servers have become more popular and should be supported.
Closes #186.

How Has This Been Tested?

Not yet tested.

Source / References:

There's no formal documentation that I know of for the REQUESTS_CA_BUNDLE env variable passed into certbot, but this blog post shows it in action.

Copy link

@github-actions github-actions bot 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 opening this pull request! Be sure to follow the pull request template!

@tashian
Copy link
Author

tashian commented Jan 11, 2022

Just wanted to note that I don't love having to send a base64-encoded root CA PEM into the container via an env variable.
Maybe there is another approach to getting this file into the container?

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@tashian
Copy link
Author

tashian commented Feb 11, 2022

Bump

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@benoitj
Copy link

benoitj commented Apr 15, 2022

@tashian on alpine you can pkg add ca-certificates, store a ca cert in /usr/local/share/ca-certificates directory and run update-ca-certificates. this seems to make the ca cert available for all tools.

tested this on a alpine docker container, makes curl work on my custome ACME server

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@brycied00d
Copy link

Bumping this to demonstrate my own interest in this feature.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@tashian
Copy link
Author

tashian commented Aug 26, 2022

bump

@coreyramirezgomez
Copy link
Contributor

I am very interested in seeing this added as a feature. I have an internal CA (via step-ca + ACME provisioner) I have tested against for which I can provide results that this indeed does work (minus one change which I have already highlighted).

@aptalca
Copy link
Member

aptalca commented Sep 8, 2022

We've been working on a big rebase of our nginx baseimage and new features were put on hold. We're very close to completing that so we'll take a look at this soon.

@aptalca
Copy link
Member

aptalca commented Sep 22, 2022

@tashian
our big nginx rebase just got merged to SWAG so we are now reviewing PRs for new features again.

If you rebase this, we'll review and merge soon

Thanks

@tashian tashian force-pushed the custom-acme branch 3 times, most recently from af45143 to 20e8893 Compare September 26, 2022 16:24
@tashian
Copy link
Author

tashian commented Sep 26, 2022

Hi @aptalca, thanks for the update. I've rebased this PR.

@tashian
Copy link
Author

tashian commented Sep 26, 2022

@coreyramirezgomez thanks for testing on your setup! I committed your suggestion

2 similar comments
@aptalca
Copy link
Member

aptalca commented Sep 26, 2022

Thanks for the PR. It looks good to me except for a couple of things.

  1. We should keep the external and interval vars separate. Using ACMESERVER as both an env var set in docker arguments and an internal var that is set and used by the init script can lead to unexpected issues. It would be better to have a separate var set in docker arguments and in the init, check that and set ACMESERVER accordingly.
  2. I'd rather not list the two new env vars for this functionality in the var list in the readme. This functionality would appeal to a very small percentage of the users who are more advanced, but those additional vars will most definitely confuse many regular users who often try to set whatever var we list even if they're marked optional. I'm afraid it would lead to lots of users trying to set them to Let's Encrypt endpoints (and incorrectly) and lead to many support requests for us. It would be much better if the vars are mentioned in the description for CERTPROVIDER as required vars for the custom option. And they can be described in more detail in the application setup section of the readme.

@tashian
Copy link
Author

tashian commented Sep 26, 2022

Ok, I've addressed these issues, thanks for the feedback.

I also updated the logic to allow a file-mounted CA bundle. I think this is more straightforward than providing a base64-encoding PEM file in an env variable, but I know this option won't be available in some container environments.

1 similar comment
@@ -229,7 +248,7 @@ else
fi

# Check if the cert is using the old LE root cert, revoke and regen if necessary
if [ -f "/config/keys/letsencrypt/chain.pem" ] && ([ "${CERTPROVIDER}" == "letsencrypt" ] || [ "${CERTPROVIDER}" == "" ]) && [ "${STAGING}" != "true" ] && ! openssl x509 -in /config/keys/letsencrypt/chain.pem -noout -issuer | grep -q "ISRG Root X"; then
if [ -f "/config/keys/letsencrypt/chain.pem" ] && ([ "${CERTPROVIDER}" = "letsencrypt" ] || ([ "${CERTPROVIDER}" = "" ] && [ -z "$ACMECABUNDLE" ])) && [ "${STAGING}" != "true" ] && ! openssl x509 -in /config/keys/letsencrypt/chain.pem -noout -issuer | grep -q "ISRG Root X"; then
Copy link
Member

Choose a reason for hiding this comment

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

This new logic is incorrect. In fact, this logic does not need to be modified at all, as it is just a backwards compatibility shim for a breaking change from well over a year ago.

The part that needs to be modified is the following, to make sure that the changes to the custom ACME related vars result in revoking the old cert and forcing revalidation:

# checking for changes in cert variables, revoking certs if necessary
if [ ! "$URL" = "$ORIGURL" ] || [ ! "$SUBDOMAINS" = "$ORIGSUBDOMAINS" ] || [ ! "$ONLY_SUBDOMAINS" = "$ORIGONLY_SUBDOMAINS" ] || [ ! "$EXTRA_DOMAINS" = "$ORIGEXTRA_DOMAINS" ] || [ ! "$VALIDATION" = "$ORIGVALIDATION" ] || [ ! "$DNSPLUGIN" = "$ORIGDNSPLUGIN" ] || [ ! "$PROPAGATION" = "$ORIGPROPAGATION" ] || [ ! "$STAGING" = "$ORIGSTAGING" ] || [ ! "$DUCKDNSTOKEN" = "$ORIGDUCKDNSTOKEN" ] || [ ! "$CERTPROVIDER" = "$ORIGCERTPROVIDER" ]; then
echo "Different validation parameters entered than what was used before. Revoking and deleting existing certificate, and an updated one will be created"
if [ "$ORIGONLY_SUBDOMAINS" = "true" ] && [ ! "$ORIGSUBDOMAINS" = "wildcard" ]; then
ORIGDOMAIN="$(echo "$ORIGSUBDOMAINS" | tr ',' ' ' | awk '{print $1}').${ORIGURL}"
else
ORIGDOMAIN="$ORIGURL"
fi
if [ "$ORIGCERTPROVIDER" = "zerossl" ] && [ -n "$ORIGEMAIL" ]; then
REV_EAB_CREDS=$(curl -s https://api.zerossl.com/acme/eab-credentials-email --data "email=$ORIGEMAIL")
REV_ZEROSSL_EAB_KID=$(echo "$REV_EAB_CREDS" | python3 -c "import sys, json; print(json.load(sys.stdin)['eab_kid'])")
REV_ZEROSSL_EAB_HMAC_KEY=$(echo "$REV_EAB_CREDS" | python3 -c "import sys, json; print(json.load(sys.stdin)['eab_hmac_key'])")
if [ -z "$REV_ZEROSSL_EAB_KID" ] || [ -z "$REV_ZEROSSL_EAB_HMAC_KEY" ]; then
echo "Unable to retrieve EAB credentials from ZeroSSL. Check the outgoing connections to api.zerossl.com and dns. Sleeping."
sleep infinity
fi
REV_ACMESERVER="https://acme.zerossl.com/v2/DV90 --eab-kid ${REV_ZEROSSL_EAB_KID} --eab-hmac-key ${REV_ZEROSSL_EAB_HMAC_KEY}"
elif [ "$ORIGSTAGING" = "true" ]; then
REV_ACMESERVER="https://acme-staging-v02.api.letsencrypt.org/directory"
else
REV_ACMESERVER="https://acme-v02.api.letsencrypt.org/directory"
fi
if [[ -f /config/etc/letsencrypt/live/"$ORIGDOMAIN"/fullchain.pem ]]; then
certbot revoke --non-interactive --cert-path /config/etc/letsencrypt/live/"$ORIGDOMAIN"/fullchain.pem --server $REV_ACMESERVER
fi
rm -rf /config/etc/letsencrypt
mkdir -p /config/etc/letsencrypt
fi

The section for revoking might need some edits as well.

Copy link
Author

Choose a reason for hiding this comment

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

I see. Ok, I've made these edits.

As far as I can tell, there's no EAB credential state stored in the ORIG variables, so I can't do EAB revocation using a custom ACME server. The way it works for ZeroSSL is via a custom lookup API that maps the email to an EAB account. But, that seems to be a ZeroSSL thing, not part of the ACME standard.

So, for now, maybe EAB should not be allowed for custom ACME servers?

@tashian tashian changed the title Allow custom ACME server (Draft fix of #186) Allow custom ACME server Sep 27, 2022
Comment on lines +225 to +227
if [ -n "$ORIGACMECABUNDLE" ]; then
echo "$ORIGACMECABUNDLE" | base64 -d - > /config/origcabundle.pem
export REQUESTS_CA_BUNDLE="/config/origcabundle.pem"
Copy link
Author

@tashian tashian Sep 27, 2022

Choose a reason for hiding this comment

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

Hi @aptalca,

For revocation, I've exported the original CA bundle to /config/origcabundle.pem here and I've set REQUESTS_CA_BUNDLE so that it's picked up by certbot/curl.

Will I need to re-export REQUESTS_CA_BUNDLE after revocation, so it points to the contents of /config/cabundle.pem again?

1 similar comment
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

github-actions bot commented May 5, 2023

This pull request is locked due to inactivity

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support custom ACME servers
6 participants