DNS01Solver fails to cleanup _acme-challenge in AWS Route53

aidansteele commented 1 year ago

What version of the package are you using?

github.com/caddyserver/certmagic v0.17.1 github.com/libdns/libdns v0.2.1

What are you trying to do?

I am trying to use AWS Route53 as my DNS provider for solving the DNS challenge in certmagic. It successfully creates the DNS records, passes the challenge and stores the certificates in Storage. When it later needs to request new certs, it fails with this error:

obtaining certificate: [${mydomain}] Obtain: [${mydomain}] solving challenges: presenting for challenge: adding temporary record for zone "${myzone}": InvalidChangeBatch: operation error Route 53: ChangeResourceRecordSets, https response error StatusCode: 400, RequestID: 3c97c377-1265-4712-a7f0-58d3f1d40b18, InvalidChangeBatch: [Tried to create resource record set [name='_acme-challenge.${mydomain}', type='TXT'] but it already exists] (order=https://acme-staging-v02.api.letsencrypt.org/acme/order/71216624/4429947744) (ca=https://acme-staging-v02.api.letsencrypt.org/directory)

This is because the _acme-challenge subdomain was never deleted during the first challenge.

What steps did you take?

This is the configuration I have written to use Route 53 for DNS:

import (
    "github.com/caddyserver/certmagic"
    "github.com/libdns/route53"
)

certmagic.DefaultACME.DNS01Solver = &certmagic.DNS01Solver{
    DNSProvider: &route53.Provider{WaitForPropagation: true},
}

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

The problem appears on this line: https://github.com/caddyserver/certmagic/blob/2e8dd4496aaa09347eef2b71f9f259d9a161eb81/solvers.go#L451

The issue is that mem.rec.Value has a value of "MOjGHqNXFLaUFiGfzdcOnXKZZeAx8r4E9G3x47vlI6g", whereas keyAuth has a value of MOjGHqNXFLaUFiGfzdcOnXKZZeAx8r4E9G3x47vlI6g, i.e. without the quotes. Because of this, the (*DNS01Solver).CleanUp method exits early here:

https://github.com/caddyserver/certmagic/blob/2e8dd4496aaa09347eef2b71f9f259d9a161eb81/solvers.go#L404-L407

It never gets a chance to clean up the record later on in that function:

https://github.com/caddyserver/certmagic/blob/2e8dd4496aaa09347eef2b71f9f259d9a161eb81/solvers.go#L420

How do you think this should be fixed?

I'm not sure to be honest. I don't know if this is a Route53-specific issue, a libdns issue or a CertMagic issue. I assume the answer isn't just to add strings.Trim(mem.rec.Value, "\"")! I'll defer to your wisdom on how best to solve this, but I'm happy to be a guinea pig to verify that a suggested solution works. Also happy to create a PR if you have an idea of how you'd want this addressed.

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

I think CertMagic is the best thing since sliced bread. Genuinely, it is wonderful software and has improved my life. I hope this bug report is high-quality enough to not waste too much of your time :)

mholt commented 1 year ago

Thanks for the detailed report; however please see https://github.com/libdns/route53/issues/16 where it has already been addressed.

I think CertMagic is the best thing since sliced bread. Genuinely, it is wonderful software and has improved my life.

That is wonderful to hear :grinning:

aidansteele commented 1 year ago

Oh man, my apologies. I could have sworn I had checked that I was using the latest packages and couldn't find this issue. But it's good to know I missed an easy solution: just upgrade 😀

caddyserver / certmagic