First (very incomplete) attempt at http-01 challenge type

[JayH5]

Hi there,

I'm trying to add the http-01 challenge type to txacme. I'd like to have http-01 support as it is much easier to work with than tls-sni-01 when performing ACME challenges from behind an HTTP proxy.

I've seen that you've mentioned that you'd like a file-based http-01 responder in #31. I think a "standalone" setup with a basic HTTP server is a simpler start and is better matched to what I specifically need.

Apologies that this is a very incomplete attempt right now... I'm sort of getting lost in all this Twisted stuff. But I wanted to get some code out there to try get feedback at an early stage. I will hopefully be working with a colleague on this next week. He has much better Twisted skills than I do.

Thanks for txacme :)

---

This change is

[mithrandi]

I'll probably only get to look at this PR over the weekend, but could you elaborate a bit about how your proxy setup is configured? (I want to make sure I understand the use case properly)

The reason I suggested a file-based http-01 responder in #31 is that I expected the need for http-01 to arise where you have another application serving http (eg. nginx, Apache) which you would then need to interact with to complete the challenge, and dropping the files in a directory is the most straightforward way to do this in most cases. However, I do aim to have txacme support (or at least provide the hooks to support) pretty much any possible use case.

So if I understand correctly, the scenario is something like this: You have a reverse proxy / load balancer terminating your TLS, proxying to a Twisted application on the backend that is serving plain HTTP. The reverse proxy / load balancer isn't capable of provisioning the needed certificates itself, however, and you don't want the backend service to terminate the TLS connection, so you want the backend service to obtain the certificates (using the http-01 challenge), and then pass the certificates to the reverse proxy / load balancer on the frontend. Is that explanation about right?

[JayH5]

@mithrandi yeah that explanation is about right, txacme on a backend server, separate from the frontend load-balancers.

There are a couple of reasons for this. We have one of these new-fangled container orchestration systems, and I'm trying to build a service using txacme that will integrate with the platform to automatically set up HTTPS for things running on the platform.

Our load balancers (HAProxy) have dynamically-generated config depending on what's running on the cluster. This txacme service I'm building would essentially be "just another container" running somewhere on the cluster and the load balancers would dynamically route ACME challenges to it. This service would then (through some hoops) update the certs on the load-balancers.

I've seen ways to route the tls-sni-01 challenge (such as this) but so far those solutions are very convoluted; compared to, say, routing all requests with a path starting with /.well-known/acme-challenge/ to the txacme service.

[mithrandi]

Okay, that sounds reasonable. I think the dns-01 challenge might generally be easier to use than http-01 in this scenario, but there's no reason you shouldn't be able to use either, and DNS access may be difficult or unacceptable in some scenarios.

[JayH5]

Cool. Please don't rush to review as this is so incomplete and it's a Friday afternoon. But if I'm doing something very wrong please tell me 😅

[mithrandi]

Aside from my inline comments, there were also some flake8 failures during the build; I wouldn't worry about these for now, we can clean up anything that's still an issue once we're closer to merging the PR.

---

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

src/txacme/endpoint.py, line 133 [r1] (raw file):

@implementer(IStreamServerEndpoint)
@attr.s(cmp=False, hash=False)
class AutoHTTPEndpoint(object):

I think the idea of AutoHTTPEndpoint doesn't make sense. AutoTLSEndpoint is an endpoint, because it integrates at the TLS transport level; you can run any protocol over a "TLS challenge responder enhanced" TLS endpoint (although since it has to be port 443, it just so happens that you'll usually be running HTTP over it).

But for http-01 challenges, the challenge is responded to at the HTTP protocol level, not the transport (TCP) level. So I think the way to integrate this would be to take the resource from the responder (which you already implemented), and then add it to your root resource. For a "one stop shop" solution to http-01 challenges, I think the twistd acme command proposed in #62 is the way to go.

---

src/txacme/challenges/_http.py, line 26 [r1] (raw file):

        Add the child resource.
        """
        self.resource.putChild(challenge.path, _HTTP01Resource(response))

Unfortunately this won't work; challenge.path is a value with multiple path segments separated by '/'. This means that you end up with a resource with an encoded URI like http://example.com/.well-known%2Facme-challenge%2Fblahblah which of course isn't what was intended.

In order to make this actually work, you would have to construct a tree of multiple resources (one child for .well-known, one child of that for acme-challenge, and then one child of that for the challenge response itself). However, I think the resource provided by this responder should correspond to the /.well-known/acme-challenge resource rather than the root resource, as otherwise you would have difficulty integrating this into an existing resource tree, which I think is something we should support.

So in that case, you would use challenge.encode('token') as the child name here, which is only a single segment.

---

src/txacme/challenges/_http.py, line 36 [r1] (raw file):

class _HTTP01Resource(Resource):

This class is probably not necessary; you can use twisted.web.static.Data instead.

---

src/txacme/test/test_challenges.py, line 178 [r1] (raw file):

class HTTPResponderTests(_CommonResponderTests, TestCase):

The tests here are a good start. Unfortunately, they fail to detect the issue with the path that I mentioned above, for reasons of symmetry (the tests are broken in the same way as the actual implementation, so they pass).

I think some HTTP-level tests would be appropriate here as well. Conveniently, challenge.uri('example.com') will give you a full URI to the challenge); this could then be used in conjunction withtreq.testing.RequestTraversalAgent which allows you to issue requests directly against a resource, thus exercising most of the resource traversal machinery without going as far as opening actual sockets.

---

Comments from Reviewable

[mithrandi]

Oh, and finally, if you're having any difficulty figuring out any of my feedback or otherwise how to go about doing things, feel free to ask for help 😀

---

Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[codecov-io]

Current coverage is 99.93% (diff: 100%)

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

@@             master        #65   diff @@
==========================================
  Files            21         22     +1   
  Lines          1561       1606    +45   
  Methods           0          0          
  Messages          0          0          
  Branches        143        145     +2   
==========================================
+ Hits           1560       1605    +45   
  Misses            1          1          
  Partials          0          0          

Powered by Codecov. Last update a12af20...249de66

[JayH5]

src/txacme/challenges/_http.py, line 26 at r1 (raw file):

Previously, mithrandi (Tristan Seligmann) wrote…

Unfortunately this won't work; challenge.path is a value with multiple path segments separated by '/'. This means that you end up with a resource with an encoded URI like http://example.com/.well-known%2Facme-challenge%2Fblahblah which of course isn't what was intended.

In order to make this actually work, you would have to construct a tree of multiple resources (one child for .well-known, one child of that for acme-challenge, and then one child of that for the challenge response itself). However, I think the resource provided by this responder should correspond to the /.well-known/acme-challenge resource rather than the root resource, as otherwise you would have difficulty integrating this into an existing resource tree, which I think is something we should support.

So in that case, you would use challenge.encode('token') as the child name here, which is only a single segment.

I've changed this to serve from `challenge.encode('token')`... There's still the parent (empty) `Resource` which I'm not so sure about, though...

---

Comments from Reviewable

[JayH5]

@mithrandi sorry for the extremely slow response 😢. I've been focussing on other bits of our system to make sure this whole thing is actually feasible (it is). Thanks a lot for your initial feedback 🍰.

I've removed the AutoHTTPEndpoint. I've made some changes to the HTTP responder as you suggested. I'm going to have a look at actually testing the responder properly today.

I'm still not really clear on what the API for this will be, though, without AutoHTTPResponder. I'd like to have txacme manage the directory of certificates and serve challenges via http-01 but want to call it programmatically from my code, not via a twistd command. Any suggestions?

[mithrandi]

To integrate this into an existing web server, there would be two steps:

1. Create an HTTP01Responder, and hook the resource into your twisted.web resource hierarchy at the appropriate place (exactly how you do this is somewhat application-specific, but would usually involve modifying the site root resource).
2. Create an AcmeIssuingService (passing it the responder you created), and start it along with your web server. If you are starting your whole application with twist or twistd, then this may be as simple as attaching the service to the main Application, but if you're not, then you would want to call startService and stopService at the appropriate times (or possibly ignore stopService, as it doesn't do anything that is important in a process that is about to terminate anyway).

Does that sound reasonable for your use case? We should probably have something in the documentation explaining how to do this, once all the details are figured out.

[JayH5]

Yeah I think that sounds reasonable, thanks.

[jerith]

I've figured out the test failures, and (as hypothesised on IRC) it's all bytes vs unicode. As usual, py27's implicit encoding and decoding sweeps this all under the rug.

For starters, all URL components should be bytes rather than unicode:

• All putChild() calls in the tests should be given bytes instead of unicode. (u'.well-known' -> b'.well-known', etc.)
• In the responder resource, the value of challenge.encode('token') is a unicode object which also needs to be encoded in the putChild() and delEntity() calls.

These changes get us to the point where the resource is found and returns a response. However, we're expecting bytes in the response and we're getting unicode. Since the bytes are what we actually care about, we probably want to use .content() instead of .text() to read the response.

[JayH5]

Test passing finally :)
Ready for re-review.

[mithrandi]

Reviewed 1 of 3 files at r2, 2 of 2 files at r6.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

src/txacme/challenges/_http.py, line 26 at r1 (raw file):

Previously, JayH5 (Jamie Hewland) wrote…

I've changed this to serve from challenge.encode('token')... There's still the parent (empty) Resource which I'm not so sure about, though...

FYI We could put some "Hi I am an ACME well-known resource" message in the parent `Resource`, or even a directory index, but I don't think it's too important; this seems fine to me as it is now.

---

src/txacme/test/test_challenges.py, line 223 at r6 (raw file):

                    Equals([b'text/plain']))),
            AfterPreprocessing(methodcaller('content'), succeeded(
                Equals(response.key_authorization.encode())))

Let's use .encode('utf-8') explicitly here rather than relying on the default encoding (which has some weird semantics).

---

Comments from Reviewable

[mithrandi]

Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

a discussion (no related file):
Looks like we're almost done here :) One other thing, could you add a news fragment? Since there's no separate issue, using the PR number is fine, so this would be src/txacme/newsfragments/65.feature, which should have a short sentence like "txacme.challenges.HTTP01Responder, an http-01 challenge responder that can be embedded into an existing twisted.web application". (Feel free to change my wording)

---

Comments from Reviewable

[mithrandi]

Reviewed 2 of 2 files at r7, 1 of 1 files at r8.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable