go-acme / lego

Let's Encrypt/ACME client and library written in Go
https://go-acme.github.io/lego/
MIT License
7.95k stars 1.02k forks source link

Route53 DNS Verification should prefer the public zone #375

Open ekristen opened 7 years ago

ekristen commented 7 years ago

Route53 allows for both public and private zones for the same domain. The primary use of the private zone is for internal VPC dns resolution.

When doing DNS validation for LE certificates, lego should try and prefer public zones over private zones, as of right now, it appears to pick the first zone to match, which can be the private zone.

mengesb commented 5 years ago

This may be the root of my issue: terraform-providers/terraform-provider-acme/#11

Is there any kind of work-around for this?

mengesb commented 5 years ago

@ldez Mind if we could get some TLC on this issue? I'm running into issues on both submission and renewal of terraform resources crafted using this repo as it's client mechanism.

From what I have observed, I can see the TXT record on dns_challenge get created in the right zone ; however the verification appears to be using the DNS servers from the private zone ... Could you provide some insight on this? Does perhaps the acme provider need to source a newer release of lego?

mengesb commented 5 years ago

Perhaps even a simple exclusion of any private DNS zone for the dns_challenge might be wise --- since it is by definition unverifiable!

ldez commented 5 years ago

@mengesb If you are able to do some tests for me, it will be great. If you can, I will try to find a solution. I will be very busy during the next week (Kubecon). If you are ready to help, it will really help me to help you :wink:.

mengesb commented 5 years ago

Very willing to run tests. Was hoping to find a way to return the list of hosted zones that are public using your client.

Let me know how to do the tests and I'll collect any logs necessary

mengesb commented 5 years ago

Looks like the root issue is here: https://github.com/xenolf/lego/blob/master/acme/dns_challenge.go

Before ACME is notified that the record has propagated, there's a precheck done. It seems that it is harvesting the internal dns servers for this precheck, and I'm guessing this might be the source of my problem. I do see some defaults in there in the event that it cannot source /etc/resolv.conf for DNS nameservers. Perhaps the best thing here would be to supply DNS servers as part of the CONFIG block or some option that can be passed through; maybe something to the effect of LEGO_PRECHECKDNS when the user can declare the DNS to be used to ensure pubic DNS is used... or actually perform a recursive NS lookup on the zone itself so that you're guaranteed to get the public DNS address to ask.

mengesb commented 5 years ago

So the issue is definitely getNameservers! https://github.com/xenolf/lego/blob/master/acme/dns_challenge.go#L52

I inserted above L58 return defaults , and my issue goes away because the test uses google's public DNS servers to test propagation instead of the system's DNS servers from resolv.conf.

I think the best solution here is to add a config var for this that allows the user to specify DNS servers to query; maybe with a key word default specifying to use the default servers (google in this case). Another option is to specify a boolean for default servers or system servers.

If we want a very targeted fix, you can use the AWS SDK to harvest the target zone id's NS records. This might be the best default solution for the route53 provider, but it means some structural changes to the code for the dns_challenge.go because you don't allow supplied DNS servers.

In any case -- this process should be documented a bit more clearly as I was confused for a while as to how the internal NS servers were being selected for testing.

--

If you would make the changes I can start working on tests. Once your PR is merged it will take me some time to get this dependency updated in the upstream terraform-provisioner-acme repo as you've added a lot of functionality that needs a whole lot of documentation. I'm very amateur to golang so i'm not quite sure how I'd configure something like a config var to help out with dns_challenge.go .. maybe I'll take a look soon but you're likely a lot faster.

ldez commented 5 years ago

So it's the issue is solve by using default nameservers, you can use --dns-resolvers value.

You can take a look to my PR #700. (--dns-disable-cp)

I can add an option to skip resolv.conf :thinking:

mengesb commented 5 years ago

So terraform-provider-acme needs some env var or a way to pass config to lego so that it can skip resolv or use that command line option. Also currently terraform-provider-acme uses lego v1.0.1 I believe; I'm guessing I'd have to update the dep anyway

vancluever commented 5 years ago

@mengesb actually, I think we can implement what's here as a resource-level option and keep it out of the DNS provider configuration path. So in other words we should have everything we need here to add the appropriate support without anything else needed from lego. 🙂