addons icon indicating copy to clipboard operation
addons copied to clipboard

Fix DuckDNS Lets Encrypt certificate creation/renewal failing with "Incorrect TXT record" error

Open lildude opened this issue 3 years ago • 1 comments

As detailed in https://github.com/home-assistant/addons/issues/2505, the DuckDNS extension will fail to issue or renew a certificate and fail with the error "Incorrect TXT record"

Processing my-ha.duckdns.org with alternative names: my-ha.cooldomain.cz
 + Signing domains...
 + Generating private key...
 + Generating signing request...
 + Requesting new certificate order from CA...
 + Received 2 authorizations URLs from the CA
 + Handling authorization for my-ha.duckdns.org
 + Handling authorization for my-ha.cooldomain.cz
 + 2 pending challenge(s)
 + Deploying challenge tokens...
OKOK + Responding to challenge for my-ha.duckdns.org authorization...
 + Cleaning challenge tokens...
OKOK + Challenge validation has failed :(
ERROR: Challenge is invalid! (returned: invalid) (result: ["type"]	"dns-01"
["status"]	"invalid"
["error","type"]	"urn:ietf:params:acme:error:unauthorized"
["error","detail"]	"Incorrect TXT record \"1g4FgZoGt2y9WaBs_7TQL7v7jb7lUJz8xNrlixCEuLQ\" found at _acme-challenge.my-ha.duckdns.org"
["error","status"]	403
["error"]	{"type":"urn:ietf:params:acme:error:unauthorized","detail":"Incorrect TXT record \"1g4FgZoGt2y9WaBs_7TQL7v7jb7lUJz8xNrlixCEuLQ\" found at _acme-challenge.my-ha.duckdns.org","status":403}
["url"]	"https://acme-v02.api.letsencrypt.org/acme/chall-v3/114079207846/9uci7g"
["token"]	"mtVWXobHYyfKU8XgjdLUYj6ebiZNqZ89Dh2kYpfLS7g"
["validated"]	"2022-05-30T05:50:26Z")
[07:55:30] INFO: OK

As I've detailed in the issue https://github.com/home-assistant/addons/issues/2505#issuecomment-1242714582:

The issue here is dehydrated that is used for getting/renewing the certificates deploys the challenge tokens for all the domains and then performs the validation for each domain.

This causes a problem with DuckDNS as it only has a single TXT record which will always be overwritten by the challenge for the last domain in the list.

This PR fixes that by requesting the certificates sequentially for each of the aliases. This isn't ideal, but it does the trick and should be fine for most users. Users with a lot of domains or aliases might need to try a few times because of Lets Encrypt's rate limits, but I suspect there won't be many people hitting these.

Whilst I'm at it, I've also made the OK lines in the output clearer by adding more context to what the hooks are actually doing so the output now looks like this:

[13:10:21] INFO: Renew certificate for domains: example.duckdns.org and aliases: example.com 
[13:10:21] INFO: No certificate found for example.duckdns.org, requesting new certificate.
# INFO: Using main config file /data/workdir/config
 + Creating chain cache directory /data/workdir/chains
Processing example.duckdns.org
 + Creating new directory /data/letsencrypt/example.duckdns.org ...
 + Signing domains...
 + Generating private key...
 + Generating signing request...
 + Requesting new certificate order from CA...
 + Received 1 authorizations URLs from the CA
 + Handling authorization for example.duckdns.org
 + 1 pending challenge(s)
 + Deploying challenge tokens...
OK - setting challenge token for example.duckdns.org
 + Responding to challenge for example.duckdns.org authorization...
 + Challenge is valid!
 + Cleaning challenge tokens...
OK - removing challenge token for example.duckdns.org
 + Requesting certificate...
 + Checking certificate...
 + Done!
 + Creating fullchain.pem...
 + Done!
# INFO: Using main config file /data/workdir/config
Processing example.duckdns.org with alternative names: example.com
 + Checking domain name(s) of existing cert... changed!
 + Domain name(s) are not matching!
 + Names in old certificate: example.duckdns.org
 + Configured names: example.com example.duckdns.org
 + Forcing renew.
 + Checking expire date of existing cert...
 + Valid till Dec  9 11:10:28 2022 GMT (Longer than 30 days). Ignoring because renew was forced!
 + Signing domains...
 + Generating private key...
 + Generating signing request...
 + Requesting new certificate order from CA...
 + Received 2 authorizations URLs from the CA
 + Handling authorization for example.duckdns.org
 + Found valid authorization for example.duckdns.org
 + Handling authorization for example.com
 + 1 pending challenge(s)
 + Deploying challenge tokens...
OK - setting challenge token for example.com
 + Responding to challenge for example.com authorization...
 + Challenge is valid!
 + Cleaning challenge tokens...
OK - removing challenge token for example.com
 + Requesting certificate...
 + Checking certificate...
 + Done!
 + Creating fullchain.pem...
 + Done!

And a renew attempt for an already valid certificate will look like this:

[13:11:37] INFO: Renew certificate for domains: example.duckdns.org and aliases: example.com 
[13:11:37] INFO: Checking existing cert...
[13:11:37] INFO: Certificate still valid. Skipping renew!

I've also fixed an annoyance where first configuring the extension would always have an empty invalid domain name added.

Fixes https://github.com/home-assistant/addons/issues/2505

lildude avatar Sep 10 '22 12:09 lildude

Hi @lildude,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

homeassistant avatar Sep 10 '22 12:09 homeassistant

can this be merged?

lozzd avatar Oct 20 '22 14:10 lozzd

@homeassistant merge 👍

thomasgeens avatar Oct 27 '22 07:10 thomasgeens

ping for merge...

jackrosenthal avatar Nov 06 '22 01:11 jackrosenthal

@SimplyLinn

lozzd avatar Nov 08 '22 13:11 lozzd

@lozzd

I'm not a member of this project, I have no authority to merge, I just threw my review into the mix because I had the same issue this will solve and looked at the code and it seemed sound to my eyes.

I was also waiting on a merge up until last month when I moved away from duckdns as a solution and managed my certificates using the core_letsencrypt addon which had support for my new registrar I've recently migrated to.

SimplyLinn avatar Nov 08 '22 14:11 SimplyLinn

ah. my apologies.

lozzd avatar Nov 08 '22 14:11 lozzd

Will it merge...

KurtMar avatar Nov 08 '22 19:11 KurtMar

@SimplyLinn Thanks for approving the PR. However this is still not merged. Do you know who to contact to make this happen? Thanks.

diamant-x avatar Nov 09 '22 11:11 diamant-x

@diamant-x https://github.com/home-assistant/addons/graphs/contributors

One if the top contributors would be my best guess, I am a user of HA, not a dev

SimplyLinn avatar Nov 09 '22 11:11 SimplyLinn

Hi @frenck or @pvizeli Could this PR be merged for the DuckDNS official addon as contains a bugfix for a long-standing issues. Both were the last merger of a PR for the addons, or the last merger for the last change to the DuckDNS addon. Thanks!

diamant-x avatar Nov 09 '22 13:11 diamant-x

I have the same problem.

kklem0 avatar Dec 15 '22 14:12 kklem0

Hi @pvizeli, Could this passed and signed PR be merged to solve a multi-year long standing issue with DuckDNS official Addon? Thanks,

diamant-x avatar Dec 15 '22 15:12 diamant-x

This is ridiculous! Dealing with a Home Assistant outage every couple of months just because of this! Please merge into master and release before I lose my mind!

christhementalist avatar Dec 21 '22 12:12 christhementalist

Hi @frenck @scop or @ludeeus or @pvizeli as above can we please please please merge this fix, its affecting a lot of us and causing regular outages to our HA instances!

mattclar avatar Jan 05 '23 00:01 mattclar

Closing in favour of removing support for aliases in https://github.com/home-assistant/addons/pull/2964 as suggested by @mdegat01 here.

lildude avatar Apr 02 '23 12:04 lildude