Thanks for the PR!

I see this as an important addition to the existing functionality of DNS plugins, and we definitely will need to handle it.

I read through the PR code, and there's a few points I would like to raise for discussion:

• I can see how this could cause confusion in user, and/or plugin behavior, as the CNAME can and often will point to a different DNS server and/or API. eg. using Route 53 plugin as the zone root exists in Route 53, but the CNAME points to a Cloudflare instance. Or to a same API, but with different credentials.
• This is functionality that's tailored towards more advanced configuration, and adding complexity in form of CNAME chain resolution seems like something that we could omit here. The user should know where the CNAME points to, and what to update.
• The PR adds a new dependency (dnspython), and we're trying to trim down dependencies as much as possible, as they cause a lot of extra work for distribution package maintainers.

That said, I really would love to see the functionality in our range of DNS plugins as it is an important and effective way to avoid storing credentials to the actual DNS zone in the boxes themselves.

I would propose following changes to the PR:

• Removing the autodetection code (CNAME chain resolving), and hence avoid adding the new dependency.
• Making the new CLI flag something along the lines of:
  --dns-force-record=where.cname.points.to.tld, which would simply be the actual record that the plugin needs to update instead of the one derived from -d yourdomain.tld

These changes would make it clear from the get go which plugin, and which credentials need to be used in addition of avoiding adding complexity and a new dependency.

Let me know what you think!

Hi @joohoi , thanks for your feedback!

Simply overriding the target name was an option I did consider. I opted for doing CNAME chain resolution instead out of a DRY consideration: requiring the user to provide the target of the CNAME repeats some information we know already exists in the DNS. Also, one thing to keep in mind is that we need to handle the case of multiple subjectaltnames being requested: in that case you have different target names, so the command line option to override the challenge domain needs to be per-subjectaltname (just like -w is for webroot).

On the other hand, I do see the merit of not dragging in an additional dependency.

So, I'll send in a modified version:

• removing the CNAME chain resolution
• adding a new option allowing to set a per-domain override for the challenge domain
• in order to make it more convenient and limit the amount of stuff that needs to be repeated, I'll make it a format string so that you can say:

--dns-challenge-domain "{domain}.mychallenges.example.com" -d domain1.example.com -d domain2.example.com

instead of having to write:

--dns-challenge-domain domain1.example.com.mychallenges.example.com -d domain1.example.com --dns-challenge-domain domain2.example.com.mychallenges.example.com -d domain2.example.com

(However I think the second, more verbose form needs to be supported anyway, so I'll steal some code from webroot -- I'll try to see if some of it can be shared).

I look forward to the changes. The proposed convenience functionality of variable record part seems like a good idea where multiple SAN domains are needed. And there definitely is a need for the webroot style mapping as well, good catch!

@joohoi The new code is here, with CI passing! I had to tweak the testsuite a bit in order to avoid having to repeat configuration options for every DNS plugin. Common options are now cleanly set in a single place.

Thanks! We're currently busy with mitigating the removal of TLS-SNI, but I'll try to get back to review this PR as soon as possible.

Understood, thanks for the heads-up, and good luck with the SNI issue!

I have pushed a new commit that fixes the case of no override specified. I have adjusted the testsuite as mentioned in my earlier comment, so that both this case, and the case of override specifieds, are tested with the actual argument parsing circuitry, instead of a mock configuration. I also had successful certificate issuance dry-runs, both with and without override specified.

Hmmm right indeed. As a matter of fact, the one I was able to extensively test (RFC 2136) did have that issue. I looked for it in other plugins, but missed the fact that the shared Lexicon module had it too. So, let's see:

• Lexicon-based ones are now OK (cloudxns, dnsimple, dnsmadeeasy, luadns, nsone)
• Digitalocean, Google, Cloudflare are now OK
• RFC 2136 was OK with my first commit
• Route53 was already OK

I have a few thoughts regarding the arguments passed to the actual DNS plugin method _perform() from the plugins/dns_common.py, namely domain, validation_domain and validation.

I feel that a solution where the domain value passed to the actual DNS plugin would have the overriding domain would be much cleaner. This would also allow possible third party DNS plugins that are using our DNS plugin API to benefit from this change automatically.

When the validation domain is overridden by the newly added CLI argument, I don't think there would be any reason for the actual executor part (the DNS plugin) to handle the original domain if it's explicitly overridden. I might be missing something, but wasn't able to come up with an situation where this wouldn't be the case.

How would you feel to change the behavior as such?

If you suggestion is to remove the original domain for the arguments of _perform, and keep only the validation (challenge) domain, yes, that makes sense; as a matter of fact that's the direction I've been taking, already removing the original domain parameter from the various {add,del}_txt_record functions. I can go one step further and also remove it from _perform, that will be a good cleanup.

If you suggestion is to remove the original domain for the arguments of _perform, and keep only the validation (challenge) domain, yes, that makes sense; as a matter of fact that's the direction I've been taking, already removing the original domain parameter from the various {add,del}_txt_record functions. I can go one step further and also remove it from _perform, that will be a good cleanup.

I'm afraid we can't do that in order to keep the API intact with the external DNS plugins. But what I think we should do is to override the value with domain from --dns-<plugin>-override-challenge.

I see your point with not changing the API for external plugins. So what you suggest is actually passing the same value (the challenge domain, possibly overridden) for both parameters of _perform, right?

I see your point with not changing the API for external plugins. So what you suggest is actually passing the same value (the challenge domain, possibly overridden) for both parameters of _perform, right?

Correct! If you see some issues in this approach, please let me know and we can figure a better way to handle this. Currently I think that this would be most sane way to handle the issue at hand.

OK, good, that's what I implemented in the last commit (80a6754).

Change rebased to resolve conflicts with latest changes for Google DNS.

I think that leaving the actual plugin (executor) code like it is currently would be the best way forward, as when not overriding the domain, it will save a few queries for finding the domain root. When the domain is overridden, the plugins would then just use the (also overridden) domain value for the SOA resolution.

Hi @joohoi, sorry for the delay in getting back to you, I was on vacation.
There's one thing to be careful of with keeping the old behaviour unchanged when the name is not overridden: it is incorrect anytime _acme-challenge. is itself a delegation apex. Always resolving from that name (and not from its parent by default) ensures that this corner case is handled correctly as well.

Hi @joohoi, where do we stand with the current situation? As I said, restoring the previous behaviour of starting the lookup at the enclosing domain in the case where it's not overridden will save a few queries, but will reintroduce a bug whereby the case where _acme-challenge is itself a delegation apex is not handled correctly.

Yeah! You are absolutely correct, and this was addressed separately in another PR for rfc2136 plugin. The review process is unfortunately taking a while, as we have to go through all the different providers functionality separately. Sorry for the delay, but this is being worked on!

Good catch! Route53 plugin fixed.

@joohoi, can you give an update on the status here? I don't think you need to do the work, but I'd like to know what still needs to be done so someone else can take over.

@joohoi @bmw any chance of this getting merged anytime soon?

I have tested it against all but Google DNS service, and the few that we don't have credentials for. Everything should be in order regarding the functionality. I still have some concerns regarding the CLI help text, and feel that it should be clarified a bit, but am unsure how to actually do this while keeping the help text short enough.

One possible clarification would be to add to the help text:

For the typical use case where you want all of your challenges under a
single dynamic zone "dyn-zone.example.com", you would thus specify
"{domain}.dyn-zone.example.com".

I'll be happy to take care of it if you think this addition would be useful.

@joohoi ping? Shall I update the help text as indicated Is there anything else still blocking merge?

I would definitely appreciate this getting merged! I've got a need for this exact use case in the very near future :)

Following CNAMEs for DNS challenges is what's blocking our IRC network (hackint.org) from migrating to Letsencrypt at this point, so additional priority given to this would be highly appreciated.

Nearly an year later... will this merge ever happen? @joohoi

Still no feedback more than three weeks after my last message here. I do not have the bandwidth to maintain a fork of certbot in the long term, so I will be looking into other ACME clients. I find it quite unfortunate that we were not able to have this useful feature incorporated in the "official" one.

I'm sorry you haven't gotten a response here. Both joohoi and I have been on personal leave. While we often are unable to immediately review PRs (as Certbot has only 1 full time developer right now), we certainly try to avoid people getting no response for weeks at a time.

We appreciate this contribution and plan to look at it in the coming weeks. In the meantime, if another client works better for you, please use it! The important thing is you're able to get a cert for your site.

@joohoi / @bmw - any news on merging this?

Unfortunately no. It's still on our radar, but we haven't had time to get to it yet. As far as I know, the biggest issue here is manually testing the changes here against all affected providers.

@quinot, are you OK with us moving your code here into a branch on our repo and building off of it and testing it ourselves when we finally have cycles to work on this?

@bmw feel free to.

Done. I created #6566 and put your work in the quinot/topic/dns-follow-cnames branch.

Thanks again for your help @quinot.

Is the functionality to use an alias for the dns challenge merged to master? Or do I need to use the branch https://github.com/certbot/certbot/tree/quinot/topic/dns-follow-cnames to have it?