Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for dns challenge #238

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

steelman
Copy link
Contributor

Since there is no single interface for administrating DNS servers a custom
script (specified with --challenge-script) is called by acme_tiny.py. The
script needs to support the following interface:

challenge_script (--add|--remove) --domain DOMAIN TXTRECORD

--add - add a TXT record
--remove - remove a TXT record
--domain DOMAIN - specify a domain name
TXTRECORD - value of a TXT record to be added or removed

@atsampson
Copy link

Somewhat related - I've been using acme-tiny with this patch for several years - it runs a script after creating the HTTP challenge file, which lets me run acme-tiny on a different machine from the webserver (with the script syncing the challenge across). Looking at this pull request, it seems like it wouldn't be very hard to make --challenge-script work for that use case too...

acme_tiny.py Outdated Show resolved Hide resolved
Since there is no single interface for administrating DNS servers a custom
script (specified with --challenge-script) is called by acme_tiny.py. The
script needs to support the following interface:

  challenge_script (--add|--remove) --domain DOMAIN TXTRECORD

  --add - add a TXT record
  --remove - remove a TXT record
  --domain DOMAIN - specify a domain name
  TXTRECORD - value of a TXT record to be added or removed
@allanrbo
Copy link

allanrbo commented Jun 25, 2020

@steelman noticed a few problems while trying to get this to work. It may not make sense to specify the client whether to use dns-01 or http-01. It's the acme server's choice, whether they will give you a dns-01 or http-01 challenge.

Specifically, I ran into this issue when trying to sign a server with cn=example.com, and SAN *.example.com. Because cn=example.com is not a wildcard name, LetsEncrypt only offered the http-01 challenge in its validation of example.com, and the code crashes on this line challenge = [c for c in authorization['challenges'] if c['type'] == "dns-01"][0]. If it hadn't crashed, the for auth_url in order['authorizations']: would have come around again for the *.example.com domain, and this the challenges would contain be a dns-01 challenge. So maybe instead of having a --challenge-type parameter, it could be just a check for first http-01, and if it doesn't exist, then try dns-01 with the given challenge script.

Would you mind if I send a separate PR with these change suggestions? Or I could send them to your fork first, so you can merge there first, and then here afterwards, if you want to keep credit for the original idea?

@steelman
Copy link
Contributor Author

@allanrbo I developed the patch, because I needed a wildcard certificate and the patch works for me. I am glad you have found problems I haven't. Sure, send your PR to my fork I will accept it and update this request.

challenge = [c for c in authorization['challenges'] if c['type'] == "dns-01"][0]
token = re.sub(r"[^A-Za-z0-9_\-]", "_", challenge['token'])
keyauthorization = "{0}.{1}".format(token, thumbprint)
txtrecord = _b64(hashlib.sha256(keyauthorization).digest())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch is very useful; thanks. I had to make the following change to get it working for me:

-            txtrecord = _b64(hashlib.sha256(keyauthorization).digest())
+            txtrecord = _b64(hashlib.sha256(keyauthorization.encode('utf-8')).digest())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have developed and worked on Python 2 which didn't differentiate between unicode strings and byte strings. Fixed.

@cavi21
Copy link

cavi21 commented Jan 7, 2022

Hi @steelman and @allanrbo do you have any update on this PR or event the other PR mentioned in this conversation ?

Are you still using this script?

Thanks for all the work put into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants