tls-alpn-01 challenge support #28
Conversation
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
==========================================
+ Coverage 95% 96.69% +1.69%
==========================================
Files 6 7 +1
Lines 400 545 +145
Branches 28 45 +17
==========================================
+ Hits 380 527 +147
+ Misses 12 11 -1
+ Partials 8 7 -1
Continue to review full report at Codecov.
|
|
It might be helpful to think of this code as only doing the same job as http port 80 for the http-01 http://<YOUR_DOMAIN>/.well-known/acme-challenge/ challenge type. Most of the ACME protocol is elsewhere, and this is quite useful without txacme. txacme would build on top of txsni by pausing an incoming request, putting the special acme certificate in SNIMap.acme_mapping, having letsencrypt fetch that certificate, and on success install the new certificate to continue with the original request. Everything letsencrypt-related except the challenge is the same as what txacme does now. I've tried to make the certificate loaders more generic. They probably work with more than just dehydrated. They are like HostDirectoryMap but they load the certificate from two files. Do they need to be underscored if HostDirectoryMap is not? It was not clear quite which code that comment was about. |
|
I've improved the test coverage and lo and behold found bugs, like "empty dict is falsy". It looks like the CI is using a pretty old version of pypy. Tests pass over here on python 2 and 3 versions of pypy 7. |
|
Oops. I tried turning on codacy as an experiment, it's not supposed to be gating PRs like this. |
| certificateOptionsFromPileOfPEM | ||
| ) | ||
|
|
||
| class PerHostnameDirectoryMap(object): |
There was a problem hiding this comment.
Could you submit a separate PR which just adds these different sorts of mappings so we can reduce the line count on this one?
| raise KeyError("no pem files for " + hostname.decode('charmap')) | ||
|
|
||
|
|
||
| class PerHostnameFilesMap(object): |
There was a problem hiding this comment.
Definitely like the idea here; certbot compatibility would be nice!
There was a problem hiding this comment.
Especially after I got some requests to dingoskidneys.com with a hostname I knew nothing about, because there is an old DNS entry for my IP address, I like the "separate client, a bit of configuration, and web server integration" strategy.
There was a problem hiding this comment.
It sounds like what you really want here is better error handling in txsni or txacme?
| IPlugin) | ||
| class AcmeSNIParser(object): | ||
| """ | ||
| Expects a ${BASEDIR} with certs/ and alpn-certs/ subdirectories. |
There was a problem hiding this comment.
This seems like a weird addition just for working with this one specific certificate provisioning flow. I'm not totally opposed to including it but it seems like it should be its own addition.
| """ | ||
| Look for acme in the first packet only. | ||
| """ | ||
| self._acme_tls_1 = _detect_acme(buf) |
There was a problem hiding this comment.
Could you do a slightly more principled parsing of the ClientHello? It would be nice to deliver this to the next layer up as a string rather than a flag. For example, acme-tls/2 might be interesting one day.
There was a problem hiding this comment.
Do you have documentation for the client hello structure? I found and lost it.
There was a problem hiding this comment.
There was a problem hiding this comment.
Sorry, let me use a more durable link: https://github.com/python-tls/tls/blob/9368423e46a8bcb89c82b2b31bba5e900406fdb9/tls/_constructs.py#L195-L206
There was a problem hiding this comment.
For future reference the easy way to get an idea of the ClientHello format is to capture one in WireShark. It cross highlights the hex dump with the named protocol elements.
There was a problem hiding this comment.
Check this out.
https://gist.github.com/dholth/b766f20cdab26cee082f3f58d7c015d7
#!/usr/bin/env python
# Extremely Principled TLS v1.2 ClientHello parser for ALPN extensions
import struct
import binascii
_int16 = struct.Struct(">H")
def int16(b):
"""
Return first two bytes of b as an unsigned integer.
"""
return _int16.unpack(b[:2])[0]
def take(data, count):
prefix = data[:count]
data = data[count:]
return prefix, data
# Handshake, 3, 1 (TLS 1.0)
HEADER = b"\x16\x03\x01"
TYPE_ALPN = b"\x00\x10"
def parseHello(data):
"""
Parse TLS 1.2 ClientHello from data, return the extensions as binary data
or None if not found.
Likely to raise struct.error or IndexError on error.
"""
# if not data.startswith(HEADER):
# return
header, data = take(data, 7)
messageId, major, minor, l1, l2 = struct.unpack(">BBBHH", header)
print(messageId, major, minor, l1, l2)
# if not data.startswith(b"\x01"): # ClientHello
# return
header2, data = take(data, 4)
print(struct.unpack(">HBB", header2)) # inner length; 3; 3
# random
random, data = take(data, 32)
# session identifier
slen, data = take(data, 1)
slen = slen[0]
session, data = take(data, slen)
# ciphers list
clen, data = take(data, 2)
clen = int16(clen)
ciphers, data = take(data, clen)
# compression methods (should always be an array of length 1, with one 0 element)
compression_length, data = take(data, 1)
compression_methods, data = take(data, compression_length[0])
# extensions
extlen, data = take(data, 2)
extensions, data = take(data, int16(extlen))
return extensions
def parseExtensions(data):
"""
Yield (type, body) for TLS extensions in data, as binary data.
"""
while data:
type, data = take(data, 2)
length, data = take(data, 2)
body, data = take(data, int16(length))
yield (type, body)
def parseAlpn(body):
"""
Parse array of Pascal strings, ignore a 16-bit length header.
"""
length, body = take(body, 2)
while body:
protocol, body = take(body[1:], body[0])
yield protocol
if __name__ == "__main__":
sample_hello = b"FgMBAgABAAH8AwN3t6WJKcsKcWo+roqQX7Nuc8SYCUAKTIkINuDoJm4ooiDRiC2236q0JY/NewWV9KcViEzk7S03gwwUSioSOKbOcAAkEwETAxMCwCvAL8ypzKjALMAwwArACcATwBQAMwA5AC8ANQAKAQABjwAAAA4ADAAACWxvY2FsaG9zdAAXAAD/AQABAAAKAA4ADAAdABcAGAAZAQABAQALAAIBAAAjAAAAEAAOAAwCaDIIaHR0cC8xLjEABQAFAQAAAAAAMwBrAGkAHQAgcqzbr+1AYblh6qcR+qvjokWhIpbChkaqpXuDY9uHhVoAFwBBBAq/uAsPt0n3lc9MGArs6RqLoQE+1eWkstNR0zPjxlQcqGSD+1mKyvSCGEwU0DCZAEFEvhnj5YxSyqcAFODwnp4AKwAJCAMEAwMDAgMBAA0AGAAWBAMFAwYDCAQIBQgGBAEFAQYBAgMCAQAtAAIBAQAcAAJAAQAVAJUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=="
import base64
import pprint
data = bytearray(base64.b64decode(sample_hello))
extensions = parseHello(data)
for (type, body) in parseExtensions(extensions):
if type == TYPE_ALPN:
pprint.pprint(list(parseAlpn(body)))There was a problem hiding this comment.
Looking great! Thank you for doing this!
There was a problem hiding this comment.
Thanks for taking a break from being successful to review this.
|
|
||
| def alpn_callback(connection, protocols): | ||
| """wrapped alpn_select_callback""" | ||
| return self._factory.selectAlpn(lambda: cb(connection, protocols), connection, protocols) |
There was a problem hiding this comment.
Is this actually helpful given the problems within OpenSSL?
There was a problem hiding this comment.
Yes. The wrapped function says "yes, I will speak acme-tls/1" when the client has asked for it. We can do that without including acme-tls/1 in the list used by the default callback.
| @@ -28,6 +28,13 @@ | |||
| HTTP2BIN_CERT_PATH = os.path.join(CERT_DIR, 'http2bin.org.pem') | |||
| HTTP2BIN_KEY_PATH = os.path.join(CERT_DIR, 'http2bin.org.key') | |||
|
|
|||
| # acme style (not concatenated) | |||
There was a problem hiding this comment.
It might be nice to have these put together in an object that actually represents the "acme style" in one place that isn't in the tests
|
Thanks for your responses! |
|
What's the status of this branch? |
|
I stopped working on it and explored other acme server implementations.
…On Thu, Feb 27, 2020, at 10:55 AM, Tristan Seligmann wrote:
What's the status of this branch?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#28?email_source=notifications&email_token=AABSZESJ6YFQ3HHPJMFJI6DRE7O55A5CNFSM4HAWH34KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENE3KDI#issuecomment-592033037>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABSZEUOOA73KHC3QGGE7HDRE7O55ANCNFSM4HAWH34A>.
|
This is the simplest possible acme-tls/1 responder for txsni.
To use, get the dehydrated shell script, configure ~/etc/dehydrated/ with config (set BASEDIR) and domains.txt (list of domains), run
authbind twist web --port acmesni:~/etc/dehydrated:tcp6:443, and rundehydrated -c --forcein the ~/etc/dehydrated/ folder. For testing it's a good idea to use a separate-stagingdirectory and config to avoid running against letsencrypt rate limits.It also has a couple of unicode fixes.
Tested in pypy 3.6.1 version 7.0.0-alpha0