Implement libcloud-based dns-01 challenge responder. #63

mithrandi · 2016-08-14T17:51:42Z

Fixes #59.

Putting this up for review so people can take a look, but the following things still need to happen before this is merged:

Wait for a new acme release, and remove the git stuff from tox.ini etc.
Write some prose documentation about DNS challenges.
Document that most libcloud providers are not tested, and some are likely broken (eg. Route 53 is broken)
If no zone is configured, then try to enumerate zones to figure it out.

This change is

codecov-io · 2016-08-14T18:00:51Z

Current coverage is 99.94% (diff: 100%)

Merging #63 into master will increase coverage by <.01%

@@             master        #63   diff @@
==========================================
  Files            22         25     +3   
  Lines          1606       1808   +202   
  Methods           0          0          
  Messages          0          0          
  Branches        145        163    +18   
==========================================
+ Hits           1605       1807   +202   
  Misses            1          1          
  Partials          0          0

Powered by Codecov. Last update e29c1aa...c99da60

mithrandi · 2016-08-15T08:40:52Z

@glyph I guess this is relevant to your interests ;) I'd appreciate if you could cast your reviewer's gaze over at least the libcloud/threading junk (either on Reviewable or here, although Reviewable would be preferred).

glyph · 2016-08-15T08:51:15Z

Thanks for doing this. I'll try to look at this when I can :)

glyph · 2016-08-15T08:52:01Z

src/txacme/newsfragments/59.feature

@@ -0,0 +1 @@
+txacme.challenges.LibcloudDNSResponder implements a dns-01 challenge responder using libcloud. Installing txacme[libcloud] is necessary to pull in the dependencies for this.


txacme[dns] maybe?

I'm planning to have other backends in future (that are not libcloud), and I think having a single extra that pulls in everything needed for all of the backends wouldn't be great?

glyph · 2016-08-25T01:05:10Z

src/integration/test_client.py

+    the challenge with.
+    """
+    HOST = _getenv(u'ACME_HOST')
+    PROVIDER = _getenv(u'LIBCLOUD_PROVIDER')


There's a potential namespacing issue here. In my opinion, all variables that influence the operation of txacme should be prefixed TXACME_. This sort of cross-library bleed-over is how we got httpoxy, after all.

Oh wait a second. I misunderstood. This is just the tests confirming that the library will be configured, but these values are in fact read by libcloud; carry on.

Yeah, these only influence the operation of the txacme integration tests.

glyph · 2016-08-25T02:09:51Z

@mithrandi - since you asked specifically about the threadpool… the Twisted side of it looks right, but I have no idea if the driver objects in libcloud are threadsafe. you might want to do an initial release with a threadpool max of 1, just to be on the safe side initially, and then come up with some way to load-test it for a follow-up change.

Unless of course you happen to know that it is totally thread-safe, in which case 👍 .

mithrandi · 2016-08-25T11:52:15Z

In fact, the libcloud docs specifically say that it is not threadsafe, so that's a good catch; I'll adjust the max to 1. To support multiple threads, we would probably need separate driver objects; I don't think supporting concurrent requests here is super important, though, so I'll leave it for now. (We're still creating some objects in the reactor thread instead of a threadpool thread which isn't strictly correct, but I think as long as there's no concurrent access, we should be okay?)

mithrandi · 2016-08-25T20:42:49Z

Review status: 0 of 8 files reviewed at latest revision, 6 unresolved discussions.

src/integration/test_client.py, line 37 [r2] (raw file):

Previously, mithrandi (Tristan Seligmann) wrote…

I like ImmediateClock, I'll use that.

Errr... apparently I didn't end up using this class anywhere, so I'm actually going to delete it instead.

Comments from Reviewable

glyph · 2016-08-26T00:13:18Z

twisted._threads has a better API for doing stuff like this (a resource with a dedicated, associated thread), but no good high-level way to initialize it.

Since ACME_HOST is shared between this and the libcloud tests, this makes it easier to run only the one.

mithrandi · 2016-08-26T16:53:23Z

Reviewed 4 of 7 files at r1, 1 of 3 files at r2, 1 of 2 files at r3, 2 of 2 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

Comments from Reviewable

glyph · 2016-08-30T07:26:55Z

OK so, I tried to provision a bunch of certs with the code in this branch, and my main feedback so far: dang this needs to log a lot more stuff. I just get:

2016-08-30T00:23:37-0700 [txacme.service#info] Requesting a certificate for 'vimes.lan.glyph.im'.
2016-08-30T00:23:37-0700 [txacme.service#info] Requesting a certificate for 'milly.lan.glyph.im'.
2016-08-30T00:23:37-0700 [txacme.service#info] Requesting a certificate for 'rem.lan.glyph.im'.

and it's not clear to me if the provisioning is just taking a really long time, or of it's hung, or if something is going wrong.

Given that ^C doesn't interrupt the process, "something is going wrong" seems like a distinct possibility.

glyph · 2016-08-30T07:28:06Z

Okay! A little bit later:

Traceback (most recent call last):
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/twisted/internet/base.py", line 1204, in mainLoop
    self.runUntilCurrent()
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/twisted/internet/base.py", line 798, in runUntilCurrent
    f(*a, **kw)
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/twisted/internet/defer.py", line 393, in callback
    self._startRunCallbacks(result)
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/twisted/internet/defer.py", line 501, in _startRunCallbacks
    self._runCallbacks()
--- <exception caught here> ---
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/twisted/internet/defer.py", line 587, in _runCallbacks
    current.result = callback(current.result, *args, **kw)
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/txacme/service.py", line 191, in <lambda>
    csr=csr_for_names([server_name], key))))
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/txacme/util.py", line 152, in csr_for_names
    x509.NameAttribute(NameOID.COMMON_NAME, common_name)]))
  File "/Users/glyph/.virtualenvs/getmesomecerts/lib/python2.7/site-packages/cryptography/x509/name.py", line 22, in __init__
    "value argument must be a text type."
exceptions.TypeError: value argument must be a text type.
2016-08-30T00:27:04-0700 [txacme.service#critical] PANIC! Unable to renew certificate for: 'milly.lan.glyph.im'

which I think means... I passed the wrong type to a constructor somewhere?

glyph · 2016-08-30T07:33:12Z

Aah. Looks like there is an undocumented requirement to call .asTextMode() in some places...

glyph · 2016-08-30T07:33:22Z

Wow this really takes a minute doesn't it.

glyph · 2016-08-30T07:35:51Z

2016-08-30T00:35:27-0700 [txacme.service#info] Received certificate for u'rem.lan.glyph.im'.

WOOOOOOOOOOOOOOO

glyph · 2016-08-30T08:22:13Z

OK. I just provisioned some real live certs from real live LE for a couple of internal on-my-LAN machines. This is pretty dang amazing. Feel free to pester me to share the script I used, I do need to clean it up and open source it. Perhaps I should just contribute it here.

mithrandi · 2016-08-30T13:06:35Z

Wow this really takes a minute doesn't it.

There is a literal time.sleep(60) in there, so, yes :) (You can reduce it, but the actual delay I was seeing when testing with Rackspace DNS was around 30-40 seconds, so you probably can't reduce it much without starting to see failures)

glyph · 2016-08-30T17:06:15Z

@mithrandi - what do you think about inferring "zone" from the domain name passed in, rather than asking the user to name it?

mithrandi · 2016-08-30T17:16:43Z

@glyph: Do you mean as a default? In the general case, I think this is impossible (eg. when I was testing this, I used the hostname txt.rackspace.mithrandi.net, with a zone of rackspace.mithrandi.net; there's no way to determine that statically from the hostname).

You could enumerate the available zones based on the credentials provided, but you might be using a set of credentials that doesn't allow this, or a set of credentials that doesn't have access to every zone in the account.

The zone is what your credentials are tied to; handling multiple responders for different zones isn't solved yet, but #64 covers this case; so that's also something to keep in mind.

glyph · 2016-08-31T08:16:00Z

You could enumerate the available zones based on the credentials provided, but you might be using a set of credentials that doesn't allow this, or a set of credentials that doesn't have access to every zone in the account.

Hrm. I didn't realize "enumerating zones" was a thing that could be restricted. In any case, you could still make a pretty clear "best effort", and just give up if the necessary things aren't available to perform the heuristic.

mithrandi · 2016-10-05T02:11:02Z

As it turns out, the API of libcloud doesn't allow for not being able to enumerate zones, so this point is moot. I implemented auto-detection in the case of zone_name = None which should be suitable for almost all use cases.

glyph · 2016-10-05T19:48:33Z

👏 👏 👏

(I can't add emoji reactions to individual commits, apparently?)

… process.

mithrandi · 2016-10-05T23:16:00Z

Reviewed 2 of 4 files at r5, 2 of 2 files at r6, 5 of 5 files at r7.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

mithrandi added 3 commits August 13, 2016 21:36

Add news fragment.

2affbc4

Expand on news fragment a bit.

fb25960

Implement libcloud-based dns-01 challenge responder.

b2e028f

glyph reviewed Aug 15, 2016
View reviewed changes

mithrandi added 2 commits August 15, 2016 12:10

Normalize and test exception handling.

080d334

Fix whitespace.

3939938

glyph reviewed Aug 25, 2016
View reviewed changes

mithrandi added 2 commits August 25, 2016 22:25

Remove unused BrokenClock.

5a45d5a

Pull token example out into a constant.

3616e30

mithrandi added 2 commits August 26, 2016 02:14

Require ACME_ENDPOINT for tls-sni-01 integration tests.

f55ee97

Since ACME_HOST is shared between this and the libcloud tests, this makes it easier to run only the one.

Max threads of 1, since libcloud isn't threadsafe.

871c4b4

mithrandi added 3 commits October 5, 2016 02:14

Warn about broken libcloud backends.

59f09fe

Implement zone autodetection.

d660693

Merge branch 'master' into 59-libcloud-dns

ec4aae7

Acme 0.9.0 is out!

d2d59da

mithrandi added 2 commits October 5, 2016 22:37

Port to twisted._threads, hopefully gaining full test coverage in the…

78ead8c

… process.

Keep trying, you are nearly there.

c99da60

mithrandi merged commit b9fd0ee into master Oct 5, 2016

mithrandi deleted the 59-libcloud-dns branch October 5, 2016 23:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement libcloud-based dns-01 challenge responder. #63

Implement libcloud-based dns-01 challenge responder. #63

mithrandi commented Aug 14, 2016 •

edited

Loading

codecov-io commented Aug 14, 2016 •

edited

Loading

mithrandi commented Aug 15, 2016

glyph commented Aug 15, 2016

glyph Aug 15, 2016

mithrandi Aug 15, 2016

glyph Aug 25, 2016

glyph Aug 25, 2016

mithrandi Aug 25, 2016

glyph commented Aug 25, 2016

mithrandi commented Aug 25, 2016

mithrandi commented Aug 25, 2016

glyph commented Aug 26, 2016

mithrandi commented Aug 26, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

mithrandi commented Aug 30, 2016 •

edited

Loading

glyph commented Aug 30, 2016

mithrandi commented Aug 30, 2016 •

edited

Loading

glyph commented Aug 31, 2016

mithrandi commented Oct 5, 2016

glyph commented Oct 5, 2016

mithrandi commented Oct 5, 2016

		@@ -0,0 +1 @@
		txacme.challenges.LibcloudDNSResponder implements a dns-01 challenge responder using libcloud. Installing txacme[libcloud] is necessary to pull in the dependencies for this.

Implement libcloud-based dns-01 challenge responder. #63

Implement libcloud-based dns-01 challenge responder. #63

Conversation

mithrandi commented Aug 14, 2016 • edited Loading

codecov-io commented Aug 14, 2016 • edited Loading

Current coverage is 99.94% (diff: 100%)

mithrandi commented Aug 15, 2016

glyph commented Aug 15, 2016

glyph Aug 15, 2016

Choose a reason for hiding this comment

mithrandi Aug 15, 2016

Choose a reason for hiding this comment

glyph Aug 25, 2016

Choose a reason for hiding this comment

glyph Aug 25, 2016

Choose a reason for hiding this comment

mithrandi Aug 25, 2016

Choose a reason for hiding this comment

glyph commented Aug 25, 2016

mithrandi commented Aug 25, 2016

mithrandi commented Aug 25, 2016

glyph commented Aug 26, 2016

mithrandi commented Aug 26, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

glyph commented Aug 30, 2016

mithrandi commented Aug 30, 2016 • edited Loading

glyph commented Aug 30, 2016

mithrandi commented Aug 30, 2016 • edited Loading

glyph commented Aug 31, 2016

mithrandi commented Oct 5, 2016

glyph commented Oct 5, 2016

mithrandi commented Oct 5, 2016

mithrandi commented Aug 14, 2016 •

edited

Loading

codecov-io commented Aug 14, 2016 •

edited

Loading

mithrandi commented Aug 30, 2016 •

edited

Loading

mithrandi commented Aug 30, 2016 •

edited

Loading