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

--always-force-new-domain-key should pre-generate the future domain key pair #3096

Closed
Delicates opened this issue Aug 9, 2020 · 33 comments
Closed

Comments

@Delicates
Copy link

--always-force-new-domain-key should pre-generate the future (next) domain key pair after the new certificate is provisioned, so that --reloadcmd can update TLSA records in advance of obtaining future certificates as part of the Current + Next DANE roll-over procedure.
Pre-generated keys (if they exist) should be used for all future --always-force-new-domain-key certificate provisioning.

Key rotation requires future planning for DANE TLSA roll-over to account for DNS propagation delays and TLSA records TTL.

See slides 20-21 from the 2018 ICANN61 presentation by Viktor Dukhovni.

The Current + Next DANE roll-over procedure is:

  • Generate next key when deploying current key and cert
  • Deploy new chain, and publish new TLSA records:
    _25._tcp.mx.example.com. IN TLSA 3 1 1 curr-pubkey-sha256
    _25._tcp.mx.example.com. IN TLSA 3 1 1 next-pubkey-sha256
  • Weeks later, obtain certificate for pre-generated next key
  • But first, make sure TLSA record is already in place
  • Repeat!
@acmesh-official acmesh-official deleted a comment from auto-comment bot Aug 9, 2020
@Delicates
Copy link
Author

Please note, with DNS-01 challenge, it also would be fairly trivial for acme.sh to implement the DANE roll-over procedure and manage shared trust anchor TLSA records itself via the already configured DNS API interface.

acme.sh could maintain Current + Next shared trust anchor TLSA records (user configurable), e.g. tlsa._dane.example.net. as per the example in Section 5.1 of RFC 7671.

All that sysadmins would have to do for different TLS servers that use the acme.sh provided certificate is set up CNAMEs pointing to the shared trust anchor TLSA record that acme.sh maintains.

This would be a nice-to-have cherry on top, leveraging all the work that has gone into acme.sh DNS API integrations, saving sysadmins from having to figure out how to work their corresponding DNS API themselves with --reloadcmd and supporting/promoting/automating best practice that improves security of the Internet ecosystem overall.

But the key to this all is to start pre-generating future keys in the first place.
Without it can't do Current + Next roll-over even with --reloadcmd.

@Neilpang
Copy link
Member

But the key to this all is to start pre-generating future keys in the first place.

The pre-generating is ok, but all the current dns api hooks are just to add/remove TXT record, they are not able to process TLSA record.

@Neilpang
Copy link
Member

please try with the tlsa branch.

acme.sh  --upgrade -b tlsa

There is a new env variable Le_Pre_Generated_Key when reloadcmd is run.

Let me know if this is what you want.

@Delicates
Copy link
Author

Thanks @Neilpang

I've tested it out and the general gist of it works, but needs some minor polish.

Please see attached the patch with the following minor polish updates:

  • _info and _err messages clean up.
  • Changed pre-generated key filename from *.key.prekey to *.next.key.
  • Replaced $Le_Pre_Generated_Key with $NEXT_KEY_PATH.
  • Exported NEXT_KEY_PATH also when running post hook and renew hook.
  • Added new info message: Your pre-generated next key for future cert key change is in.
  • Preserved permissions of the pre-generated key file if it exists by not removing it.

I've tested it successfully with a reloadcmd script along these lines:

# DANE-EE SPKI SHA2-512 (TLSA 3 1 2) hash generation
TLSA312_CURRENT=$(openssl pkey -in "$CERT_KEY_PATH" -pubout -outform DER 2>/dev/null | sha512sum | cut -f1 -d ' ')
TLSA312_NEXT=$(openssl pkey -in "$NEXT_KEY_PATH" -pubout -outform DER 2>/dev/null | sha512sum | cut -f1 -d ' ')

cat << EOF | nsupdate -k /path/nsupdate.key
update del tlsa._dane.$Le_Domain 60 TLSA
update add tlsa._dane.$Le_Domain 60 TLSA 3 1 2 $TLSA312_CURRENT
update add tlsa._dane.$Le_Domain 60 TLSA 3 1 2 $TLSA312_NEXT
send
EOF

I've also identified some major security issues throughout existing code and raised a separate issue #3127 for it, which also affects this code - so you might want to use this branch for fixing them as well.

Outstanding
A bit of further work is required to fix the unlikely but possible issue with the below code in line with how you decide to fix issue #3127:
cat "$NEXT_KEY_PATH" >"$CERT_KEY_PATH"

acme-tlsa.diff.txt

@Delicates
Copy link
Author

Also would be good to export CERT_KEY_PATH and NEXT_KEY_PATH for all different hooks (not just post and renew) and also for all hook calls.

The hooks could do validation of TLSA records against:

  • current certificate key
  • pre-generated next key
  • installed certificates

to make sure nothing got broken during/after acme.sh execution.

Example use case - it would be useful prior to generating a new certificate to run a pre-hook to check if a TLSA record matching the pre-generated next key already exists. If it doesn't exist, the pre-hook could:

  • Add missing TLSA record so it is there the next time acme.sh is run.
  • Abort new certificate generation to avoid service outage while DNS syncs up the missing entries.
  • Send a notification email to sysadmins.

Similarly deployment hook can be used to validate that everything is as it should be and nothing got broken.

I didn't look at all of them until now, and raised a new issue #3128 about hook variables passing inconsistency that needs to be fixed.

@Delicates
Copy link
Author

After some further testing - there's an issue.

The key files get rotated even when cert issuance fails.
This would lead to loss of the previously generated key that has been advertised in TLSA, which in turn would lead to an outage when the cert finally does succeed in being generated, until old TLSA record TTL expires and new TLSA records get propagated by DNS.

The pre-generation of new key should be happening after a successful cert issuance.
If cert issuance fails - the old pre-generated key file should be left untouched for the next attempt.

@Eagle3386
Copy link
Contributor

@Delicates I'm not that good at shell scripting/programming, so not capable of doing this myself, yet eagerly waiting to automate TLSA generation/population, too. 😅
So, may I ask if you could achieve any progress since your last comment?

@rdemendoza
Copy link

@Neilpang Really nice feature, do you plan to merge it into the master branch?

@Neilpang
Copy link
Member

@rdemendoza I need more time to check it.

@Eagle3386
Copy link
Contributor

@Neilpang Almost 1 year later - do you see any chance of getting that "more time to check it" within the next couple of weeks or at least months?
I'm really looking forward to switch from TLSA with 2-1-1 to TLSA's with 3-1-1 without custom post-LE-approval scripting on my box... 😉

@damousys
Copy link

I have also updated to the TLSA branche.
Run a --force to renew my cert, but didn't get a TLSA record.
Do I have to do something extra besides running this command ?

"/root/.acme.sh"/acme.sh --cron --home "/root/.acme.sh" --force

@redglobuli
Copy link

maybe something could be achieved with this new tool https://letsdns.org/index.html, could be called within a hook.

@Neilpang
Copy link
Member

Sorry guys, I will check it soon.

@GwynethLlewelyn
Copy link

Eagerly awaiting your checks, @Neilpang 😉

@Neilpang
Copy link
Member

@Eagle3386
Copy link
Contributor

@Neilpang Since the linked Wiki page isn't that much of an explanation regarding actual usage, can you please extend it?

I do get that the additional command line argument generates the new key & puts it into a file.
But how am I supposed to consume it in order to store it as an TXT record over at my domain provider?
Your current DNS API architecture only allows for the ACME challenge as an TXT record, not any TLSA key.

@bolemo
Copy link

bolemo commented Sep 12, 2023

@Neilpang Since the linked Wiki page isn't that much of an explanation regarding actual usage, can you please extend it?

I do get that the additional command line argument generates the new key & puts it into a file. But how am I supposed to consume it in order to store it as an TXT record over at my domain provider? Your current DNS API architecture only allows for the ACME challenge as an TXT record, not any TLSA key.

The way I do it is with a custom script through cron (every month) that:

  1. uses acme.sh to renew the certificate using --issue --dns mydnsservice -d my domain.com -d *.mydomain.com --server letsencrypt --ocsp --ecc -k ec-384 -ak ec-384 --always-force-new-domain-key --force
  2. generates TLSA/DANE keys using @Delicates method (look under # DANE-EE SPKI SHA2-512 (TLSA 3 1 2) hash generation on his post)
  3. pushes the TLSA records to my DNS Service using their API, deleting the old one (keeping current and next)
  4. copies/deploys the certificate to their effective location (local and remote if any)
  5. restarts the services (nginx, etc.) local and remote if any.

@Delicates
Copy link
Author

After 2 years I have finally moved from my own patched version to the latest master acme.sh and noticed the following problem:

@Neilpang On the initial certificate issuance, out of all variables passed to reloadcmd for some reason two are blank, including $Le_Next_Domain_Key (even though the nextkey file has been created).
On subsequent certificate renewals, the variables are not blank.

Also, looking at the logs, it seems the next key still gets pre-generated prior to successful certificate renewal, so I'm guessing the nasty key-loss issue I pointed out in my last message above remains?

@Eagle3386
Copy link
Contributor

@bolemo Thanks for the further help, but I still got some questions:

  1. Why do I have to run acme.sh --issue instead of acme.sh --renew?
  2. Please, can you tweak your comment so that it includes a whitespace between # & DANE-EE - otherwise, any (future) visitor of this issue might struggle finding @Delicates's comment.
  3. How do I actually push the TLSA records to my DNS Service using acme.sh, removing old, keeping current & adding new ones?
  4. Using your "tutorial", deploying can still be done via adding --deploy --deployhook <my hook> to the usual acme.sh --renew […] one-liner, right?

If the answer to the last question above would be true, then I'd be lucky, because the deploy hook I need to run already includes restarting Nginx. 😎

Really looking forward to your answers! 😍

@bolemo
Copy link

bolemo commented Sep 26, 2023

@bolemo Thanks for the further help, but I still got some questions:

  1. Why do I have to run acme.sh --issue instead of acme.sh --renew?
  2. Please, can you tweak your comment so that it includes a whitespace between # & DANE-EE - otherwise, any (future) visitor of this issue might struggle finding @Delicates's comment.
  3. How do I actually push the TLSA records to my DNS Service using acme.sh, removing old, keeping current & adding new ones?
  4. Using your "tutorial", deploying can still be done via adding --deploy --deployhook <my hook> to the usual acme.sh --renew […] one-liner, right?

If the answer to the last question above would be true, then I'd be lucky, because the deploy hook I need to run already includes restarting Nginx. 😎

Really looking forward to your answers! 😍

Hello @Eagle3386

  1. You don't have to use --issue, and I suppose --renew would work fine. This is just how I set things for me, and my script was given as an example open to customization :)
  2. I edited my post, and added a direct link to the post ;)
  3. I push the TLSA records directly from my script, using the Dynu API (since I use Dynu). Here is the portion of my script for that:
# Push the DANE/TLSA keys to Dynu
>&2 echo '*** DANE/TLSA key generation and send to DNS (Dynu)' 
# DANE-EE SPKI SHA2-256 (TLSA 3 1 2) hash generation
TLSA312_CURRENT=$(openssl pkey -in "$ACME_CRT_DIR/mydomain.com.key" -pubout -outform DER 2>/dev/null | sha256sum | cut -f1 -d ' ')
TLSA312_NEXT=$(openssl pkey -in "$ACME_CRT_DIR/mydomain.com.key.next" -pubout -outform DER 2>/dev/null | sha256sum | cut -f1 -d ' ')

>&2 echo 'Getting Authorization Token...'
Dynu_AT="$(curl -s -X GET https://api.dynu.com/v2/oauth2/token \
        -H "accept: application/json" \
        -u "$Dynu_ClientId:$Dynu_Secret" \
    | jq -r .access_token)"

>&2 echo 'Getting hostname ID...'
Dynu_ID="$(curl -s -X GET https://api.dynu.com/v2/dns/getroot/mydomain.com \
        -H "accept: application/json" \
        -H "Authorization: Bearer $Dynu_AT" \
    | jq -r .id)"

>&2 echo 'Getting TLSA Records...'
Dynu_TLSARecords="$(curl -s -X GET https://api.dynu.com/v2/dns/$Dynu_ID/record \
        -H "accept: application/json" \
        -H "Authorization: Bearer $Dynu_AT" \
    | jq ".dnsRecords[] | select(.recordType == \"TLSA\")")"

Dynu_TLSARecToDel=$(echo "$Dynu_TLSARecords" | jq ". | select(.certificateAssociatedData != \"${TLSA312_CURRENT^^}\") | .id")
Dynu_TLSACurExists=$(echo "$Dynu_TLSARecords" | jq ". | select(.certificateAssociatedData == \"${TLSA312_CURRENT^^}\") | .id")

Dynu_AddTLSARecord() {
  curl -s -X POST "https://api.dynu.com/v2/dns/$Dynu_ID/record" \
	-H "accept: application/json" \
	-H "Authorization: Bearer $Dynu_AT" \
	-H "Content-Type: application/json" \
	-d "{\"nodeName\":\"_443._tcp\",\"hostname\":\"_443._tcp.mydomain.com\",\"recordType\":\"TLSA\",\"ttl\":90,\"state\":true,\"certificateUsage\":3,\"selector\":1,\"matchingType\":1,\"certificateAssociatedData\":\"$1\"}"
}

Dynu_DelTLSARecord() {
  curl -s -X DELETE "https://api.dynu.com/v2/dns/$Dynu_ID/record/$1" \
	-H "accept: application/json" \
	-H "Authorization: Bearer $Dynu_AT"
}

if [ "$Dynu_TLSACurExists" ]
  then >&2 echo "Current TLSA Record exists."
  else >&2 echo "Current TLSA Record is missing, adding it..."
    Dynu_AddTLSARecord $TLSA312_CURRENT
fi

>&2 echo "Deleting records: $Dynu_TLSARecToDel"
for REC in "$Dynu_TLSARecToDel"; do Dynu_DelTLSARecord $REC; done

>&2 echo "Adding next TLSA Record..."
Dynu_AddTLSARecord $TLSA312_NEXT
# -------------------------------------
  1. I suppose you could use the acme.sh hook, but I decided to not use it personally, and using acme.sh only to renew the certificate, and manage the rest from my script (my script is calling acme.sh while the logic of the hook is to have acme.sh to call a custom script).

@Eagle3386
Copy link
Contributor

Eagle3386 commented Sep 26, 2023

@bolemo Awesome & almost near the finish line now, thanks! 👏🏻

Regarding your answers:

  1. Glad to hear that, thanks! 👍🏻
  2. Even better than expected, thanks a lot!
  3. Since I use a different DNS API (ArtFiles which I'm the maintainer of 😅), I might found the catch here: since ArtFiles has an awful API themselves (a GET gives you the full monty, yet a setting requires specifying "record pairs", e.g. TXT=<all current records that shall be kept + your new lines>, as form-encoded data of a POST request with that data not stored inside the request body, but within the URL as it would be a simple GET request… 🙈).
  4. So, given this "headache" of a DNS API, how am I supposed to use acme.sh for this?
    What I mean is which hook to use (--post-hook, but how to check if renew succeeded then, or --reloadcmd, --renew-hook or is there yet another hook I should be better using) & how to use acme.sh to call my DNS API plugin for setting the TLSA records?
    Granted, I need to modify the plugin by defaulting to TXT as the record type to be set, but also allow other types via adding an additional & optional argument to the corresponding method.

Honestly, I thought Neil would give us something like a new --dane argument that could then be used by supporting DNS API plugins to offer a method that gets the required values as arguments & just calls the DNS provider to set that data.
But right now, it seems like acme.sh only supports the very bare minimum & everything else is left to any user crazy enough to deal with such "tinkering" of pushing TLSA record updates.

@StrangePeanut
Copy link

The entire process looks like something that should be implemented in acme.sh directly. Why is this not the case? It would be great to have a --dane option or similar.

@Eagle3386
Copy link
Contributor

Eagle3386 commented Sep 27, 2023

@StrangePeanut That's pretty much what I said in my comment right above your one. 😅👍🏻

@Neilpang Any willingness to improve the situation here? Pretty please?! 🥹😍

@bolemo
Copy link

bolemo commented Sep 27, 2023

Technically, acme.sh could automate pushing the DANE records, at least for all the DNS providers it already supports (as it already has API functions).
It would however require time and testing (& motivation), as there are 160 supported DNS API to adjust and test for that (math: ("headache" of a DNS API) x 160)…
For the non supported DNS providers, a custom hook script (pretty much like some of you are doing) does the trick.

@Eagle3386, until acme.sh automates dane, if it ever does, you have no choice but to deal with ArtFiles API yourself, seems it is the only work left from you for this to work In your case.

@Eagle3386
Copy link
Contributor

@bolemo

Nice play on my "headache thing", therefore 😄.
However, the 👍🏻 is for acme.sh moving further, i.e. adding support for it.

But while we all wait for that to happen one day, can you please tell me (read: us Linux shell newbies) how I (we) can "call back" from such a "hook script" back into acme.sh & most importantly into a DNS API plugin script so that its methods are used to add & remove the old & new DANE records? Because I simply don't know how… 🙈

@bolemo
Copy link

bolemo commented Sep 27, 2023

@bolemo

Nice play on my "headache thing", therefore 😄. However, the 👍🏻 is for acme.sh moving further, i.e. adding support for it.

But while we all wait for that to happen one day, can you please tell me (read: us Linux shell newbies) how I (we) can "call back" from such a "hook script" back into acme.sh & most importantly into a DNS API plugin script so that its methods are used to add & remove the old & new DANE records? Because I simply don't know how… 🙈

Well, reread my post here.

I don't use acme.sh directly and I don't use any hook.
What I do is using every month (cron) a custom script (and that script calls acme.sh only to renew the certs in a dane compliant way, nothing else, and then my script deals with deployment and pushing TLSA to my DNS provider).

 my_custom_script
     |
     • —> acme.sh to generate/renew the certs
     |
     • —> deploy
     |
     • —> TLSA/DANE
     |
     • —> nginx, etc.
     |
     •
    exit

I am sure it can be done with little change with a post hook like --deployhook (using acme.sh primarily and it is acme.sh that calls a custom hook script to deal with TLSA…)

 acme.sh
    |
    • —> generate/renew the certs
    |
    • —> calling deploy hook script
    .          |
    .          • —> TLSA/DANE
    .          |
    .          • —> nginx, etc.
    .          |
    •  <— exit •
    |
    •
   exit

You said you already use successfully a deploy hook to restart your nginx. So it seems all you have to do is add in that existing deploy hook script the code to push TLSA to your DNS provider.
For that, you can use the logic of what I do in my script posted here and adapt the API calls for ArtFiles (the only headache part).

@bolemo
Copy link

bolemo commented Sep 27, 2023

But while we all wait for that to happen one day, can you please tell me (read: us Linux shell newbies) how I (we) can "call back" from such a "hook script" back into acme.sh & most importantly into a DNS API plugin script so that its methods are used to add & remove the old & new DANE records? Because I simply don't know how… 🙈

To reply to this more specifically, I don’t believe you can call back the acme.sh DNS API plugin yourself, as I don’t think they are able to deal with DANE specific TLSA records (what I was mentioning earlier about 160 x headaches).
Unfortunately, you would need to do it using the DNS API in your own hook script.

@Eagle3386
Copy link
Contributor

@bolemo Thanks so much, this helped a lot!

Last question: given your 2nd scheme of running everything (where TLSA is done via hook script), are those variables containing username & password for the DNS API available in that very hook script?
Because I neither want to have 2 files where they're stored nor want to deal with reading them for usage from disk twice.

@bolemo
Copy link

bolemo commented Sep 28, 2023

You are welcome!

Last question: given your 2nd scheme of running everything (where TLSA is done via hook script), are those variables containing username & password for the DNS API available in that very hook script? Because I neither want to have 2 files where they're stored nor want to deal with reading them for usage from disk twice.

This is a very good question.

In my case, before I call acme.sh in my script, I systematically set my dns provider credentials exporting the appropriate variables.
In fact, this has to be done only the first time, as after it is saved in ~/.acme.sh/account.conf as per acme.sh documentation (same for ArtFiles: https://github.com/acmesh-official/acme.sh/wiki/dnsapi2#dns_artfiles )

export AF_API_Username="api12345678"
export AF_API_Password="apiPassword"

So when using export … before calling acme.sh, the variables AF_API_Usernameand AF_API_Password will be available in the post hook script.

However, once they are set in ~/.acme.sh/account.conf and you don't use the export anymore, I believe it won't be available as is in the post hook.
So the best would be in your post hook script to add this line at the beginning (before you use the AF_API_… variables):

. ~/.acme.sh/account.conf

That would add the content of ~/.acme.sh/account.conf (hence the variables) to your post hook script, and AF_API_Username, AF_API_Password should then be available for you in your script.

@bolemo
Copy link

bolemo commented Oct 12, 2023

Any success @Eagle3386 ?

@Eagle3386
Copy link
Contributor

Didn't have time due to private life tasks of higher priority. Planning on trying this in the next couple of weeks & will report back ASAP.

@GwynethLlewelyn
Copy link

Planning on trying this in the next couple of weeks & will report back ASAP.

@Eagle3386, thanks so much for looking into it!

I had attempted to get a quick-and-dirty patch on PR #4087 — merging an old tlsa branch into what at the time was the current main branch — which essentially did everything that was required. It just required quite a bit of polishing and a lot of testing with different DNS providers.

I still have the old code... somewhere :) ... but I'm the first one to admit being a terrible amateurish programmer with little or no knowledge of what I'm doing :) Nevertheless, whatever I did back then did work. Imagine to my surprise after @Neilpang closed that PR saying that it was "fixed", I let the auto-updater run by itself, and after a few months, I suddenly realised that all my emails were bouncing with invalid key validation errors...

AFAIK, right now, we just get an extra private key for the upcoming rotation cycle, which is just ⅓ of the job: a new public key needs to be created as well, and, using the DNS API, a TLSA record with the fingerprint needs to be published. So there is really a bit more that needs to be done.

Also note that I'm extremely limited in the number of external DNS providers (and their APIs) that I have access to; as such, my own patches were only tested on Cloudflare. They might not work elsewhere, especially on providers which don't give access to TLSA records from their APIs. Such providers will unfortunately have to be left out of this scenario (and let their users put some pressure to support it!). Nevertheless, I believe that a working solution that works for many providers is better than the current state-of-the-art, where it doesn't work with any of them, unless, of course, you either patch the code yourself, or create the external scripts as hooks as described earlier in this (now long!) thread.

@nixigaj
Copy link

nixigaj commented Dec 10, 2023

For anyone interested using Cloudflare, I made a single binary tool in Go that takes in a current and next EC private key (generated by acme.sh) and automatically updates the TLSA records for a given domain using the Cloudflare API. I use it for my own mail server and it gives me a 100% score on Internet.nl's email test.

https://github.com/nixigaj/cf-tlsa-acmesh
Instructions are in the readme.

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

10 participants