CertMagic fails when _acme-challenge is its own zone, no memory of presenting a DNS record

[mjm]

What version of the package are you using?

0.21.3, via Caddy 2.8.4

What are you trying to do?

I'm using a separate DNS zone for my _acme-challenge records, as with my DNS provider (deSEC) this makes it possible to give Caddy a token that is restricted to only be able to update those DNS records rather than any records for my entire domain. So instead of creating an _acme-challenge record in the mydomain.com zone, I should have a @ record in the _acme-challenge.mydomain.com zone. Certmagic does not seem to correctly handle this situation, as it seems to get confused about whether it created the DNS record or not (though it very much does create the record).

What steps did you take?

1. Given a domain mydomain.com, create an _acme-challenge.mydomain.com zone for ACME challenge records.
2. Configure Caddy to use DNS challenges to try to get a cert for mydomain.com. I'm not sure if provider matters here, but I'm using deSEC.
3. Start Caddy.

What did you expect to happen, and what actually happened instead?

I apologize that I hit this issue like two months ago and worked around it, and am only just now finding the time to file the issue, so I don't have the exact logs anymore, but the main error that was present there said:

no memory of presenting a DNS record for "_acme-challenge.mydomain.com"

If I looked in my DNS provider, I could see quite a few records being correctly added.

I expected that the DNS validation would proceed successfully, and I would get certs and Caddy would use them.

How do you think this should be fixed?

I spent a fair bit of time debugging, and found a patch to certmagic that seemed to resolve this issue for me. Add this code here in solvers.go:

if rec.Name == "" {
  rec.Name = "@"
}

I'm not sure if this is the right place to do this or if there is somewhere better, but it addresses the problem. If I remember right, the issue is that somewhere along the way, the empty string used for the name of the record will get correctly translated to "@", and then when CertMagic tries to look up the record it presented later, it will be given "@" as the name, but it won't find the relevant record under that name.

Changing the name to "@" before creating it addresses this, but maybe that could also be done in libdns which constructs the name in the first place. Or alternatively, getDNSPresentMemory could have logic for the "@" case to also check "".

Please link to any related issues, pull requests, and/or discussion

I haven't opened a PR yet (though I can) in case there is a better place/way to handle this.

Bonus: What do you use CertMagic for, and do you find it useful?

I use CertMagic via Caddy for automatic certs! It's very nice to have this functionality integrated into the webserver, so yes, I definitely find it useful. I used to use NixOS's ACME support with Nginx, but I'm much happier doing this through Caddy instead.

[mholt]

Thanks for the issue -- sorry I have been behind on a lot of things.

Hmm, this doesn't quite sound like the right fix in general.

This:

Or alternatively, getDNSPresentMemory could have logic for the "@" case to also check "".

causes me to wonder if there's something about the deSec implementation that is inconsistent with regards to "" or "@". I believe whatever CertMagic sees at first is what it should see again, if that makes sense.

Have you tried seeing if a patch needs to or could be made in the deSec provider library?

[mjm]

It looks to me that the deSEC implementation is conforming to libdns's expectations, which are that records should use @ as the name for records at the root of the zone. In fact, deSEC's own API seems to use an empty string, but it gets converted to @ intentionally because that's what libdns wants. This suggests to me that the deSEC provider is not the thing that ought to be patched.

Cloudflare's API uses @ for this already (and therefore doesn't need an explicit conversion in its provider), so I suspect it would hit the same issue. Presumably any provider that is conforming to this libdns expectation would hit it.

[mholt]

It looks to me that the deSEC implementation is conforming to libdns's expectations, which are that records should use @ as the name for records at the root of the zone.

I'm not sure we have set a standard for that anywhere, actually. 🤔 (It's been a while since I've worked on this though.) Are you seeing that somewhere?

Definitely open to getting this fixed but I want to ensure I understand what's happening first.

[mjm]

I'm not sure we have set a standard for that anywhere, actually. 🤔 (It's been a while since I've worked on this though.) Are you seeing that somewhere?

Mostly just in the fact that the deSEC provider explicitly does a conversion there in the function I linked to in my previous comment. I'm assuming that they wouldn't have done that conversion if it wasn't needed/expected. The function comment says "libdns uses @". That's all I'm really going on though.

[mholt]

Hm, strange. Nothing in libdns/libdns "uses @" -- the only place we do is a single condition like x == "" || x == "@", i.e. we support both. libdns doesn't set any of the fields though.

[mjm]

Maybe this line is the actual issue then:

certmagic/solvers.go

Lines 557 to 559 in ed73243

	if memory.zoneRec.record.Name == "" {
	return dnsPresentMemory{}, fmt.Errorf("no memory of presenting a DNS record for %q (usually OK if presenting also failed)", dnsName)
	}

After looking at it more closely, it does seem like what you say is true: it shouldn't matter which value is used, since CertMagic is using information from the challenge to both save and lookup the record. But the line linked above will cause it to think it didn't find a saved record if the record it did find has an empty string for its name. So maybe that's the right place to fix this.

[mholt]

Ah, I see; that actually would make sense; maybe if it changed to use a separate found variable to differentiate empty name versus not found, then the error wouldn't be erroneous :)

[francislavoie]

Oh, good change @mholt that would explain why we saw that so much in threads on the forums 🙈

[mholt]

Yeah, who knew 🤯

[mjm]

I updated to Caddy v2.9.0-beta.3, removed my workaround patch for certmagic, and moved aside my existing cert storage, and I was able to get a new certificate issued without issue. Thanks for the fix!

[mholt]

Great! Thanks for the update.