Add support for DNS01 challenges

[XaF]

Created another Dockerfile, pretty much a copy of the normal one, but with dns-cloudflare support so the DNS01 challenge type can be used, and support for wildcard certificates is also possible.

[JonasAlfredsson]

Thank you for the contribution, I am a little bit busy at the moment so I apologize in advance for not being able to give this pull request that much of time right now.

This is an interesting addition to the feature set of this image, and I think that if we are to support DNS challenges we should include it in the current Debian and Alpine images, so we can probably begin by adding this change directly to them. There is also some strange indentation, so please make sure that it is consistent across the entire file.

I have not worked with this type of challenge before, so I would like to do some more research about how we best implement this in a sustainable way in case we want to add more DNS providers.

[XaF]

This is an interesting addition to the feature set of this image, and I think that if we are to support DNS challenges we should include it in the current Debian and Alpine images, so we can probably begin by adding this change directly to them.

I hesitated at first, I didn't know if you wanted to keep the images as small as possible (hence avoiding to add the extra package there). I'm all for adding it in the main package.
I guess I can drop the env variable there too, as I was setting it up so that the same script would work for all versions, but if we add support everywhere, we're good :)

There is also some strange indentation, so please make sure that it is consistent across the entire file.

Oops, you're right. I've done that from a fresh installed server before updating my vim config, and it tried putting tabs everywhere even though the indentation clearly wasn't. I'll fix that up.

I have not worked with this type of challenge before, so I would like to do some more research about how we best implement this in a sustainable way in case we want to add more DNS providers.

I don't know many of the plugins for DNS01 challenges. I know that Cloudflare is one, and that you can also connect to your local bind for it to refresh your local DNS server automatically. That latter one might be more complicated to implement with containers. That being said, we could support any plugins by using "dns-cloudflare" (the plugin name) in the cert name, for instance (+ the parameter for default plugin to use could be that too). I'll make those changes and update the PR!

[XaF]

Ok, updated with a more generic approach.

From the certbot documentation, we can find the different authenticators using DNS-01:

  --dns-cloudflare      Obtain certificates using a DNS TXT record (if you are
                        using Cloudflare for DNS). (default: False)
  --dns-cloudxns        Obtain certificates using a DNS TXT record (if you are
                        using CloudXNS for DNS). (default: False)
  --dns-digitalocean    Obtain certificates using a DNS TXT record (if you are
                        using DigitalOcean for DNS). (default: False)
  --dns-dnsimple        Obtain certificates using a DNS TXT record (if you are
                        using DNSimple for DNS). (default: False)
  --dns-dnsmadeeasy     Obtain certificates using a DNS TXT record (if you are
                        using DNS Made Easy for DNS). (default: False)
  --dns-gehirn          Obtain certificates using a DNS TXT record (if you are
                        using Gehirn Infrastructure Service for DNS).
                        (default: False)
  --dns-google          Obtain certificates using a DNS TXT record (if you are
                        using Google Cloud DNS). (default: False)
  --dns-linode          Obtain certificates using a DNS TXT record (if you are
                        using Linode for DNS). (default: False)
  --dns-luadns          Obtain certificates using a DNS TXT record (if you are
                        using LuaDNS for DNS). (default: False)
  --dns-nsone           Obtain certificates using a DNS TXT record (if you are
                        using NS1 for DNS). (default: False)
  --dns-ovh             Obtain certificates using a DNS TXT record (if you are
                        using OVH for DNS). (default: False)
  --dns-rfc2136         Obtain certificates using a DNS TXT record (if you are
                        using BIND for DNS). (default: False)
  --dns-route53         Obtain certificates using a DNS TXT record (if you are
                        using Route53 for DNS). (default: False)
  --dns-sakuracloud     Obtain certificates using a DNS TXT record (if you are
                        using Sakura Cloud for DNS). (default: False)

The question for you @JonasAlfredsson - do we want to add support for all of those ?
While I'm here, I don't mind adding support for all of them (except for rfc2136 since it's not an external provider, but a bind server).

[XaF]

Last commit adds support for all DNS01 providers listed above, except for rfc2136.

[JonasAlfredsson]

This is a really good change, I like it and thank you for taking the time to make it.
However, there are some places where some small changes can really improve the flow and readability of the code :)

There is only one concern that I have, and it is described in the very long comment.
As I have said earlier, I have not worked with DNS challenges before, so is there any benefit of doing it in case you are not interested in a wildcard certificate?

[XaF]

@JonasAlfredsson any other changes needed for this to be mergeable ? :)

[JonasAlfredsson]

This is starting to look really good.
We should expand the documentation to reflect all of these new features, but that can be done in the future.
Just take a look at the last final touches, and then I think this will be possible to merge :)

[JonasAlfredsson]

The docker images are building correctly, and the code looks like it would be backwards compatible without users doing anything which means that we should probably be able to get away with just a bump to the minor version number.
I have not had time to test the actual functionality out myself, so I trust that you have tried it out on your end? :)

[XaF]

Added documentation, and addressed your last comments!

As per your question, I'm currently already using my PR-ed container for my server, until we can merge this and get the new version ready so I can revert to using yours ;) - It works well!

Found mention of authenticator 'dns-cloudflare' in name '<REDACTED>-dns-cloudflare'
Requesting an RSA certificate for '<REDACTED>-dns-cloudflare' (dns-01 through dns-cloudflare)
Requesting a certificate for <REDACTED>

Successfully received certificate.
Certificate is saved at: /etc/letsencrypt/live/<REDACTED>-dns-cloudflare/fullchain.pem
Key is saved at:         /etc/letsencrypt/live/<REDACTED>-dns-cloudflare/privkey.pem
This certificate expires on 2022-02-09.
These files will be updated when the certificate renews.
NEXT STEPS:
- The certificate will need to be renewed before it expires. Certbot can automatically renew the certificate in the background, but you may need to take steps to enable that functionality. See https://certbot.org/renewal-setup for instructions.
Saving debug log to /var/log/letsencrypt/letsencrypt.log

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
If you like Certbot, please consider supporting our work by:
 * Donating to ISRG / Let's Encrypt:   https://letsencrypt.org/donate
 * Donating to EFF:                    https://eff.org/donate-le
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Found mention of authenticator 'dns-cloudflare' in name '<REDACTED>-dns-cloudflare'
Requesting an RSA certificate for '<REDACTED>-dns-cloudflare' (dns-01 through dns-cloudflare)
Certificate not yet due for renewal

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Certificate not yet due for renewal; no action taken.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Saving debug log to /var/log/letsencrypt/letsencrypt.log

[XaF]

As another note: I reverted back to use the local authenticator variable in the get_certificates method, as I needed an intermediate variable to lowercase, while still being able to trim the dns- prefix to get the provider name.

Alternatively, I could be lowercasing when we're calling the get_certificates method, but that would be weird because we're not doing any of the other lowercasing there...

I updated my test containers with this, and things are working on my own nginx container :)

[JonasAlfredsson]

Once again excellent work!
Very nice to hear that you are able to get it to work on your end :)
The only concern I have now is the changed regex, I think it should at least be consistent with how the RSA/ECC matching works.

[JonasAlfredsson]

I don't really have more comments here and now, so unless you want to add something extra I don't see why this couldn't be merged :)

[XaF]

I don't really have more comments here and now, so unless you want to add something extra I don't see why this couldn't be merged :)

I'm fine with that, too!! 🎉
Thanks for the back and forth to make this better, @JonasAlfredsson !

[JonasAlfredsson]

This is now merged, and will be built for the :latest tag.
I will probably try to add some small extra things before I make a new release version though.

[maxkagamine]

Just wanted to say thanks, both of you! This was just what I needed; worked like a charm, wildcard cert too 👍

[JonasAlfredsson]

Oh my, that was a fast adoption of a new feature. Really nice to hear :)
Just because I have not yet had time to work with wildcard domains, would you mind sharing how your Nginx config is set up so I can get a better understanding on how/when this may come to use?

[maxkagamine]

I'm setting up some docker containers on my NAS and needed DNS validation since they'll be mostly internal; my thinking was I'd use a single wildcard cert as they'll all be subdomains. (Your timing here was impeccable btw; when I found the repo looking for a simple container with nginx+letsencrypt, your comment this was ready to be merged was just a few hours ago!)

Looking at the code, though, it doesn't seem there's a way to have sites share a cert. Not a huge deal since with the way your script automatically parses out sites' domain names, I frankly didn't even need a wildcard (I'm also using it for the Unraid web ui with a small script to copy the cert from the container, but in retrospect I could have just explicitly added its domain).

That said it might be a nice improvement, e.g. aggregate cert name→domains before calling get_certificate. (Although thinking about it further I'm not sure what the usefulness of that would be. The auto-domain-parsing is really quite handy.)

[JonasAlfredsson]

Alright, glad to hear that this was such a nice coincidence :)

But yes, I have also been thinking about how to best handle sharing of a single wildcard certificate over multiple configs/sites.
A solution (which starts to make this entrypoint script complicated enough to make me want to move to a another language) would perhaps to create an associative array and store [${cert_name}] = "domain_name,domain_name" and then iterate over that in the end.
I don't really like it, because it looks like the value here would really benefit from being a list and Bash does not support that. However, there are workarounds to that (using cut -d,), so I think we should be fine.

The more difficult part is how to define you want this to be a wildcard certificate. Perhaps we can add something like this:

ssl_certificate_key     /etc/letsencrypt/live/mydomain.org/privkey.pem; #wildcard

where we then pick up the wildcard comment and make this request become [mydomain.org] = "mydomain.org,*.mydomain.org". So in this case the cert_name must be the base domain?

This discussion should probably continue in the two new issues I have created:

• Support sharing certificate over multiple configuration files #91
• Better support for wildcard domains #92