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

Support acme-dns #89

Merged
merged 3 commits into from Mar 24, 2018

Conversation

Projects
None yet
3 participants
@dnet
Contributor

dnet commented Mar 20, 2018

acme-dns is a simplified DNS server with a RESTful HTTP API to provide a simple way to automate ACME DNS challenges. This PR adds the necessary class to use any 3rd party or self-hosted acme-dns instance with sewer. I tested this in the staging environment with my own domain running my own acme-dns instance, and it generated the certificate successfully.

@komuw

This comment has been minimized.

Owner

komuw commented Mar 20, 2018

@dnet hi, thanks. I'll review later.

You also need to add acmedns to

choices=['cloudflare', 'aurora'],

@komuw komuw self-requested a review Mar 20, 2018

@dnet

This comment has been minimized.

Contributor

dnet commented Mar 20, 2018

@komuw you're right, I added that now in d111c8a and modified README.md accordingly as well

resolver.nameservers = ['8.8.8.8']
answer = resolver.query('_acme-challenge.{0}.'.format(domain_name), 'TXT')
subdomain, _ = str(answer.canonical_name).split('.', 1)

This comment has been minimized.

@komuw

komuw Mar 20, 2018

Owner

@dnet Hi, do we need dnspython in order to get the value of subdomain?

I do not really know how acme-dns works, but going through its documentation;
wouldn't it be possible to get the value of subdomain just by making a POST request to the registration endpoint? ie;

url = urllib.parse.urljoin(self.ACME_DNS_API_BASE_URL, 'register')
resp = requests.get(url, headers=headers, timeout=self.HTTP_TIMEOUT)
subdomain = resp.json()['subdomain']

and then use the subdomain in the subsequent request to the update endpoint.

If that is possible, it would help us avoid adding dnspython as a dependency.

This comment has been minimized.

@dnet

dnet Mar 20, 2018

Contributor

I don't like the idea of the dependency either, however the main use-case of acme-dns is pointing a CNAME record to the acme-dns instance from the main domain. So for example, in case of hsbp.org, I've set it up like this: (auth.vsza.hu is my private acme-dns instance)

$ host -a _acme-challenge.hsbp.org.
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 41523
;; flags: qr rd ra; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;_acme-challenge.hsbp.org.      IN      ANY

;; ANSWER SECTION:
_acme-challenge.hsbp.org. 3599  IN      CNAME   78d4dd08-c13d-4700-acdc-2bd2a7b3f2ef.auth.vsza.hu.

You're right, I got 78d4dd08-c13d-4700-acdc-2bd2a7b3f2ef from the response to the register call, however, I did that only once, as the whole point that acme-dns solves is that I don't want to update the _acme-challenge entry, I just want to set it up once to point to _acme-dns. And it seemed to me that the Python standard library doesn't have the bits to perform the necessary DNS queries, since gethostbyname{,_ex} relies on A and AAAA records only.

If you really dislike the dependency, I see several ways, some of them could even be combined:

  • Put it as an extra dependency, since it's only used by the acme-dns module (and let's face it, the zeitgeist is people using cloud magic, self-hosters like me are a minority, even if a vocal one).
  • Fall back to calling host(1), nslookup(1) or dig(1) using subprocess (ugly but it can work).
  • Let the user specify the subdomain (like after 6ead878 but before 85aabfe), although this might cause some problems if there are multiple domains specified using domain_alt_names and not all might point to the same acme-dns instance.

This comment has been minimized.

@komuw

komuw Mar 20, 2018

Owner

I think we can keep the dependency then since the alternatives do not appear to be much better.
At a later date we may refactor and put dns-python as an extra dependency, so that people can pip install sewer[acme-dns] and they would get sewer with dns-python installed.
I would also like to do the same for 'tldextract' and 'apache-libcloud'[1]

  1. sewer/setup.py

    Line 77 in 5072941

    'requests', 'pyopenssl', 'cryptography', 'tldextract', 'apache-libcloud'

But depending on dnspython seems ok now.

@komuw

komuw approved these changes Mar 20, 2018 edited

Looks good.

I'll release a new version of sewer to pypi later on in the week and it will include this work.

Thanks @dnet .

Repository owner deleted a comment from codacy-bot Mar 20, 2018

@codacy-bot

This comment has been minimized.

codacy-bot commented Mar 20, 2018

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- sewer/cli.py  2
         

See the complete overview on Codacy

@komuw komuw merged commit 7886cf2 into komuw:master Mar 24, 2018

1 check passed

Codacy/PR Quality Review Good work! A positive pull request.
Details

@dnet dnet deleted the dnet:acme-dns branch Mar 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment