From 74ce332b5a4a0747cad56afc4cbe72fa519ce31f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 19:38:00 +0000 Subject: [PATCH 01/16] Manual SimpleHTTP integration tests. --- letsencrypt/auth_handler.py | 7 +- letsencrypt/plugins/manual.py | 77 ++++++++++++++++--- letsencrypt/plugins/manual_test.py | 3 +- .../plugins/standalone/authenticator.py | 4 + tests/boulder-integration.sh | 2 + tests/integration/_common.sh | 3 +- 6 files changed, 81 insertions(+), 15 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 2929166c2..3d8275901 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -156,7 +156,7 @@ class AuthHandler(object): active_achalls = [] for achall, resp in itertools.izip(achalls, resps): # Don't send challenges for None and False authenticator responses - if resp: + if resp is not None and resp: self.network.answer_challenge(achall.challb, resp) # TODO: answer_challenge returns challr, with URI, # that can be used in _find_updated_challr @@ -166,6 +166,11 @@ class AuthHandler(object): chall_update[achall.domain].append(achall) else: chall_update[achall.domain] = [achall] + else: # resp is None or not resp + # XXX: make sure that achalls corresponding to None or + # False returned from Authenticator are removed from + # the queue and thus avoid infinite loop + active_achalls.append(achall) return active_achalls diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 700759194..771833d6f 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -1,6 +1,12 @@ """Manual plugin.""" import os +import logging +import shutil +import signal +import subprocess import sys +import tempfile +import time import zope.component import zope.interface @@ -8,10 +14,14 @@ import zope.interface from acme import challenges from acme import jose +from letsencrypt import errors from letsencrypt import interfaces from letsencrypt.plugins import common +logger = logging.getLogger(__name__) + + class ManualAuthenticator(common.Plugin): """Manual Authenticator. @@ -43,8 +53,8 @@ command on the target server (as root): # anything recursively under the cwd HTTP_TEMPLATE = """\ -mkdir -p /tmp/letsencrypt/public_html/{response.URI_ROOT_PATH} -cd /tmp/letsencrypt/public_html +mkdir -p {root}/public_html/{response.URI_ROOT_PATH} +cd {root}/public_html echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: python -c "import BaseHTTPServer, SimpleHTTPServer; \\ @@ -55,8 +65,8 @@ s.serve_forever()" """ # https://www.piware.de/2011/01/creating-an-https-server-in-python/ HTTPS_TEMPLATE = """\ -mkdir -p /tmp/letsencrypt/public_html/{response.URI_ROOT_PATH} -cd /tmp/letsencrypt/public_html +mkdir -p {root}/public_html/{response.URI_ROOT_PATH} +cd {root}/public_html echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: openssl req -new -newkey rsa:4096 -subj "/" -days 1 -nodes -x509 -keyout ../key.pem -out ../cert.pem @@ -77,6 +87,14 @@ s.serve_forever()" """ super(ManualAuthenticator, self).__init__(*args, **kwargs) self.template = (self.HTTP_TEMPLATE if self.config.no_simple_http_tls else self.HTTPS_TEMPLATE) + self._root = (tempfile.mkdtemp() if self.conf("test-mode") + else "/tmp/letsencrypt") + + @classmethod + def add_parser_arguments(cls, add): + add("test-mode", action="store_true", + help="Test mode. Executes the manual command in subprocess. " + "Requires openssl to be installed unless --no-simple-http-tls.") def prepare(self): # pylint: disable=missing-docstring,no-self-use pass # pragma: no cover @@ -110,17 +128,44 @@ binary for temporary key/certificate generation.""".replace("\n", "") tls=(not self.config.no_simple_http_tls)) assert response.good_path # is encoded os.urandom(18) good? - self._notify_and_wait(self.MESSAGE_TEMPLATE.format( - achall=achall, response=response, uri=response.uri(achall.domain), - ct=response.CONTENT_TYPE, command=self.template.format( - achall=achall, response=response, ct=response.CONTENT_TYPE, - port=(response.port if self.config.simple_http_port is None - else self.config.simple_http_port)))) + command = self.template.format( + root=self._root, achall=achall, response=response, + ct=response.CONTENT_TYPE, port=( + response.port if self.config.simple_http_port is None + else self.config.simple_http_port)) + if self.conf("test-mode"): + logger.debug("Test mode. Executing the manual command: %s", command) + try: + # pylint: disable=attribute-defined-outside-init + self._httpd = subprocess.Popen( + command, + # don't care about setting stdout and stderr, + # we're in test mode anyway + shell=True, + # "preexec_fn" is UNIX specific, but so is "command" + preexec_fn=os.setsid) + except OSError as error: # ValueError should not happen! + logging.debug( + "Couldn't execute manual command", error, exc_info=True) + return False + logger.debug("Manual command running as PID %s.", self._httpd.pid) + # give it some time to bootstrap, before we try to verify + # (cert generation in case of simpleHttpS might take time) + time.sleep(4) # XXX + if self._httpd.poll(): + raise errors.Error("Couldn't execute manual command") + else: + self._notify_and_wait(self.MESSAGE_TEMPLATE.format( + achall=achall, response=response, + uri=response.uri(achall.domain), ct=response.CONTENT_TYPE, + command=command)) if response.simple_verify( achall.challb, achall.domain, self.config.simple_http_port): return response else: + if self.conf("test-mode") and self._httpd.poll(): + return False return None def _notify_and_wait(self, message): # pylint: disable=no-self-use @@ -130,5 +175,13 @@ binary for temporary key/certificate generation.""".replace("\n", "") sys.stdout.write(message) raw_input("Press ENTER to continue") - def cleanup(self, achalls): # pylint: disable=missing-docstring,no-self-use - pass # pragma: no cover + def cleanup(self, achalls): + # pylint: disable=missing-docstring,no-self-use,unused-argument + if self.conf("test-mode"): + if self._httpd.poll() is None: + logger.debug("Terminating manual command process") + os.killpg(self._httpd.pid, signal.SIGTERM) + else: + logger.debug("Manual command process already terminated " + "with %s code", self._httpd.returncode) + shutil.rmtree(self._root) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index a533bcc75..1360ebf44 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -15,7 +15,8 @@ class ManualAuthenticatorTest(unittest.TestCase): def setUp(self): from letsencrypt.plugins.manual import ManualAuthenticator self.config = mock.MagicMock( - no_simple_http_tls=True, simple_http_port=4430) + no_simple_http_tls=True, simple_http_port=4430, + manual_test_mode=False) self.auth = ManualAuthenticator(config=self.config, name="manual") self.achalls = [achallenges.SimpleHTTP( challb=acme_util.SIMPLE_HTTP, domain="foo.com", key=None)] diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index 971f90266..d55d70aa0 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -197,6 +197,10 @@ class StandaloneAuthenticator(common.Plugin): """ signal.signal(signal.SIGINT, self.subproc_signal_handler) self.sock = socket.socket() + # SO_REUSEADDR flag tells the kernel to reuse a local socket + # in TIME_WAIT state, without waiting for its natural timeout + # to expire. + self.sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) try: self.sock.bind(("0.0.0.0", port)) except socket.error, error: diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 06a1d8aa9..975d030da 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -23,6 +23,8 @@ common() { common --domains le1.wtf auth common --domains le2.wtf run +common -a manual -d le.wtf auth +common -a manual -d le.wtf --no-simple-http-tls auth export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ OPENSSL_CNF=examples/openssl.cnf diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 0f26d3815..de16a939a 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -10,11 +10,12 @@ store_flags="$store_flags --logs-dir $root/logs" export root store_flags letsencrypt_test () { - # first three flags required, rest is handy defaults letsencrypt \ --server "${SERVER:-http://localhost:4000/acme/new-reg}" \ --no-verify-ssl \ --dvsni-port 5001 \ + --simple-http-port 5001 \ + --manual-test-mode \ $store_flags \ --text \ --agree-eula \ From ccc6a3212b29bacfb5355d3cf873a54c7d4acfca Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 19 Jun 2015 14:37:41 +0000 Subject: [PATCH 02/16] Simple DVSNI verification --- acme/acme/challenges.py | 50 +++++++++++++++++++++++++++++++++++ acme/acme/challenges_test.py | 34 ++++++++++++++++++++++++ acme/acme/crypto_util.py | 19 +++++++++++++ acme/acme/crypto_util_test.py | 20 ++++++++++++++ acme/setup.py | 2 +- 5 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 acme/acme/crypto_util.py create mode 100644 acme/acme/crypto_util_test.py diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 8024728fa..c6c8f9bdf 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -4,13 +4,18 @@ import functools import hashlib import logging import os +import socket +import OpenSSL import requests +from acme import crypto_util from acme import interfaces from acme import jose from acme import other +from letsencrypt import crypto_util as le_crypto_util + logger = logging.getLogger(__name__) @@ -192,6 +197,18 @@ class DVSNI(DVChallenge): """Domain name used in SNI.""" return binascii.hexlify(self.nonce) + self.DOMAIN_SUFFIX + def probe_cert(self, domain, **kwargs): + """Probe DVSNI challenge certificate.""" + host = socket.gethostbyname(domain) + logging.debug('%s resolved to %s', domain, host) + + kwargs.setdefault("port", self.PORT) + kwargs.setdefault("host", host) + kwargs["server_hostname"] = self.nonce_domain + # TODO: try different methods? + # pylint: disable=protected-access + return crypto_util._probe_sni(**kwargs) + @ChallengeResponse.register class DVSNIResponse(ChallengeResponse): @@ -231,6 +248,39 @@ class DVSNIResponse(ChallengeResponse): """Domain name for certificate subjectAltName.""" return self.z(chall) + self.DOMAIN_SUFFIX + def simple_verify(self, chall, domain, key, **kwargs): + """Verify DVSNI. + + :param .challenges.DVSNI chall: Corresponding challenge. + :param str domain: Domain name being validated. + :param OpenSSL.crypto.PKey key: Public key for the key pair + being authorized. If ``None`` key verification is not + performed! + + :returns: ``True`` iff client's control of the domain has been + verified, ``False`` otherwise. + :rtype: bool + + """ + cert = chall.probe_cert(domain=domain, **kwargs) + # TODO: check "It is a valid self-signed certificate" and + # return False if not + + # pylint: disable=protected-access + sans = le_crypto_util._pyopenssl_cert_or_req_san(cert) + logging.debug('Certificate %s. SANs: %s', cert.digest('sha1'), sans) + + key_filetype = OpenSSL.crypto.FILETYPE_ASN1 + if key is None: + logging.warn('No key verification is performed') + keys_match = key is None or OpenSSL.crypto.dump_privatekey( + key_filetype, key) == OpenSSL.crypto.dump_privatekey( + key_filetype, cert.get_pubkey()) + + return (keys_match and domain in sans and + self.z_domain(chall) in sans) + + @Challenge.register class RecoveryContact(ContinuityChallenge): """ACME "recoveryContact" challenge.""" diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index a1214c2f9..377584f5a 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -181,6 +181,33 @@ class DVSNITest(unittest.TestCase): self.assertRaises( jose.DeserializationError, DVSNI.from_json, self.jmsg) + @mock.patch('acme.challenges.socket.gethostbyname') + @mock.patch('acme.challenges.crypto_util._probe_sni') + def test_probe_cert(self, mock_probe_sni, mock_gethostbyname): + mock_gethostbyname.return_value = '127.0.0.1' + self.msg.probe_cert('foo.com') + mock_gethostbyname.assert_called_once_with('foo.com') + mock_probe_sni.assert_called_once_with( + host='127.0.0.1', port=self.msg.PORT, + server_hostname='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') + + self.msg.probe_cert('foo.com', host='8.8.8.8') + mock_probe_sni.assert_called_with( + host='8.8.8.8', port=mock.ANY, server_hostname=mock.ANY) + + self.msg.probe_cert('foo.com', port=1234) + mock_probe_sni.assert_called_with( + host=mock.ANY, port=1234, server_hostname=mock.ANY) + + self.msg.probe_cert('foo.com', bar='baz') + mock_probe_sni.assert_called_with( + host=mock.ANY, port=mock.ANY, server_hostname=mock.ANY, bar='baz') + + self.msg.probe_cert('foo.com', server_hostname='xxx') + mock_probe_sni.assert_called_with( + host=mock.ANY, port=mock.ANY, + server_hostname='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') + class DVSNIResponseTest(unittest.TestCase): @@ -218,6 +245,13 @@ class DVSNIResponseTest(unittest.TestCase): from acme.challenges import DVSNIResponse hash(DVSNIResponse.from_json(self.jmsg)) + def test_simple_verify(self): # TODO + chall = mock.MagicMock() + chall.probe_cert.return_value = OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, test_util.load_vector('cert.pem')) + self.assertFalse(self.msg.simple_verify(chall, "example.com", key=None)) + # TODO: key not None + class RecoveryContactTest(unittest.TestCase): diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py new file mode 100644 index 000000000..6049df26e --- /dev/null +++ b/acme/acme/crypto_util.py @@ -0,0 +1,19 @@ +"""Crypto utilities.""" +import socket + +import OpenSSL + + +def _probe_sni(server_hostname, host, port, timeout=10, + method=OpenSSL.SSL.SSLv23_METHOD): + sock = socket.create_connection((host, port), source_address=('0', 0)) + context = OpenSSL.SSL.Context(method) + context.set_timeout(timeout) + connection = OpenSSL.SSL.Connection(context, sock) + connection.set_tlsext_host_name(server_hostname) # pyOpenSSL>=0.13 + connection.set_connect_state() + connection.do_handshake() + cert = connection.get_peer_certificate() + sock.close() + # TODO: shutdown() + return cert diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py new file mode 100644 index 000000000..f3f237cf1 --- /dev/null +++ b/acme/acme/crypto_util_test.py @@ -0,0 +1,20 @@ +"""Tests for acme.crypto_util.""" +import socket +import unittest + +import OpenSSL + + +class ProbeSNITest(unittest.TestCase): + """Tests for acme.crypto_util._probe_sni.""" + + def test_it(self): + from acme.crypto_util import _probe_sni + # TODO: mock this out + cert = _probe_sni( + "google.com", socket.gethostbyname("google.com"), port=443) + self.assertTrue(isinstance(cert, OpenSSL.crypto.X509)) + + +if __name__ == "__main__": + unittest.main() # pragma: no cover diff --git a/acme/setup.py b/acme/setup.py index d83131d2a..6426d6db2 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -11,7 +11,7 @@ install_requires = [ 'pyrfc3339', 'ndg-httpsclient', # urllib3 InsecurePlatformWarning (#304) 'pyasn1', # urllib3 InsecurePlatformWarning (#304) - 'PyOpenSSL', + 'PyOpenSSL>=0.13', # Connection.set_tlsext_host_name 'pytz', 'requests', 'werkzeug', From c2a8195f197e027a2594a790be5a18d6d0334b6c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 19 Jun 2015 15:10:23 +0000 Subject: [PATCH 03/16] Move _pyopenssl_cert_or_req_san to acme. --- acme/acme/challenges.py | 4 +-- acme/acme/crypto_util.py | 42 ++++++++++++++++++++++++++ acme/acme/crypto_util_test.py | 41 +++++++++++++++++++++++++ acme/acme/testdata/csr-6sans.pem | 12 ++++++++ acme/acme/testdata/csr-nosans.pem | 8 +++++ acme/setup.py | 3 +- letsencrypt/crypto_util.py | 43 ++------------------------- letsencrypt/revoker.py | 3 +- letsencrypt/tests/achallenges_test.py | 3 +- setup.py | 3 +- 10 files changed, 114 insertions(+), 48 deletions(-) create mode 100644 acme/acme/testdata/csr-6sans.pem create mode 100644 acme/acme/testdata/csr-nosans.pem diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index c6c8f9bdf..a619e9a30 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -14,8 +14,6 @@ from acme import interfaces from acme import jose from acme import other -from letsencrypt import crypto_util as le_crypto_util - logger = logging.getLogger(__name__) @@ -267,7 +265,7 @@ class DVSNIResponse(ChallengeResponse): # return False if not # pylint: disable=protected-access - sans = le_crypto_util._pyopenssl_cert_or_req_san(cert) + sans = crypto_util._pyopenssl_cert_or_req_san(cert) logging.debug('Certificate %s. SANs: %s', cert.digest('sha1'), sans) key_filetype = OpenSSL.crypto.FILETYPE_ASN1 diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 6049df26e..053089cb9 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -17,3 +17,45 @@ def _probe_sni(server_hostname, host, port, timeout=10, sock.close() # TODO: shutdown() return cert + + +def _pyopenssl_cert_or_req_san(cert_or_req): + """Get Subject Alternative Names from certificate or CSR using pyOpenSSL. + + .. todo:: Implement directly in PyOpenSSL! + + .. note:: Although this is `acme` internal API, it is used by + `letsencrypt`. + + :param cert_or_req: Certificate or CSR. + :type cert_or_req: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`. + + :returns: A list of Subject Alternative Names. + :rtype: list + + """ + # constants based on implementation of + # OpenSSL.crypto.X509Error._subjectAltNameString + parts_separator = ", " + part_separator = ":" + extension_short_name = "subjectAltName" + + if hasattr(cert_or_req, 'get_extensions'): # X509Req + extensions = cert_or_req.get_extensions() + else: # X509 + extensions = [cert_or_req.get_extension(i) + for i in xrange(cert_or_req.get_extension_count())] + + # pylint: disable=protected-access,no-member + label = OpenSSL.crypto.X509Extension._prefixes[OpenSSL.crypto._lib.GEN_DNS] + assert parts_separator not in label + prefix = label + part_separator + + san_extensions = [ + ext._subjectAltNameString().split(parts_separator) + for ext in extensions if ext.get_short_name() == extension_short_name] + # WARNING: this function assumes that no SAN can include + # parts_separator, hence the split! + + return [part.split(part_separator)[1] for parts in san_extensions + for part in parts if part.startswith(prefix)] diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index f3f237cf1..2e0c71b2b 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -4,6 +4,8 @@ import unittest import OpenSSL +from acme import test_util + class ProbeSNITest(unittest.TestCase): """Tests for acme.crypto_util._probe_sni.""" @@ -16,5 +18,44 @@ class ProbeSNITest(unittest.TestCase): self.assertTrue(isinstance(cert, OpenSSL.crypto.X509)) +class PyOpenSSLCertOrReqSANTest(unittest.TestCase): + """Test for acme.crypto_util._pyopenssl_cert_or_req_san.""" + + @classmethod + def _call(cls, loader, name): + # pylint: disable=protected-access + from acme.crypto_util import _pyopenssl_cert_or_req_san + return _pyopenssl_cert_or_req_san(loader(name)) + + def _call_cert(self, name): + return self._call(test_util.load_cert, name) + + def _call_csr(self, name): + return self._call(test_util.load_csr, name) + + def test_cert_no_sans(self): + self.assertEqual(self._call_cert('cert.pem'), []) + + def test_cert_two_sans(self): + self.assertEqual(self._call_cert('cert-san.pem'), + ['example.com', 'www.example.com']) + + def test_csr_no_sans(self): + self.assertEqual(self._call_csr('csr-nosans.pem'), []) + + def test_csr_one_san(self): + self.assertEqual(self._call_csr('csr.pem'), ['example.com']) + + def test_csr_two_sans(self): + self.assertEqual(self._call_csr('csr-san.pem'), + ['example.com', 'www.example.com']) + + def test_csr_six_sans(self): + self.assertEqual(self._call_csr('csr-6sans.pem'), + ["example.com", "example.org", "example.net", + "example.info", "subdomain.example.com", + "other.subdomain.example.com"]) + + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/acme/acme/testdata/csr-6sans.pem b/acme/acme/testdata/csr-6sans.pem new file mode 100644 index 000000000..8f6b52bd7 --- /dev/null +++ b/acme/acme/testdata/csr-6sans.pem @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIBuzCCAWUCAQAweTELMAkGA1UEBhMCVVMxETAPBgNVBAgTCE1pY2hpZ2FuMRIw +EAYDVQQHEwlBbm4gQXJib3IxDDAKBgNVBAoTA0VGRjEfMB0GA1UECxMWVW5pdmVy +c2l0eSBvZiBNaWNoaWdhbjEUMBIGA1UEAxMLZXhhbXBsZS5jb20wXDANBgkqhkiG +9w0BAQEFAANLADBIAkEA9LYRcVE3Nr+qleecEcX8JwVDnjeG1X7ucsCasuuZM0e0 +9cmYuUzxIkMjO/9x4AVcvXXRXPEV+LzWWkfkTlzRMwIDAQABoIGGMIGDBgkqhkiG +9w0BCQ4xdjB0MHIGA1UdEQRrMGmCC2V4YW1wbGUuY29tggtleGFtcGxlLm9yZ4IL +ZXhhbXBsZS5uZXSCDGV4YW1wbGUuaW5mb4IVc3ViZG9tYWluLmV4YW1wbGUuY29t +ghtvdGhlci5zdWJkb21haW4uZXhhbXBsZS5jb20wDQYJKoZIhvcNAQELBQADQQBd +k4BE5qvEvkYoZM/2++Xd9RrQ6wsdj0QiJQCozfsI4lQx6ZJnbtNc7HpDrX4W6XIv +IvzVBz/nD11drfz/RNuX +-----END CERTIFICATE REQUEST----- diff --git a/acme/acme/testdata/csr-nosans.pem b/acme/acme/testdata/csr-nosans.pem new file mode 100644 index 000000000..813db67b0 --- /dev/null +++ b/acme/acme/testdata/csr-nosans.pem @@ -0,0 +1,8 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIBFTCBwAIBADBbMQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEh +MB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMRQwEgYDVQQDDAtleGFt +cGxlLm9yZzBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQD0thFxUTc2v6qV55wRxfwn +BUOeN4bVfu5ywJqy65kzR7T1yZi5TPEiQyM7/3HgBVy9ddFc8RX4vNZaR+ROXNEz +AgMBAAGgADANBgkqhkiG9w0BAQsFAANBAMikGL8Ch7hQCStXH7chhDp6+pt2+VSo +wgsrPQ2Bw4veDMlSemUrH+4e0TwbbntHfvXTDHWs9P3BiIDJLxFrjuA= +-----END CERTIFICATE REQUEST----- diff --git a/acme/setup.py b/acme/setup.py index 6426d6db2..a5acb8d3e 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -11,7 +11,8 @@ install_requires = [ 'pyrfc3339', 'ndg-httpsclient', # urllib3 InsecurePlatformWarning (#304) 'pyasn1', # urllib3 InsecurePlatformWarning (#304) - 'PyOpenSSL>=0.13', # Connection.set_tlsext_host_name + # Connection.set_tlsext_host_name (>=0.13), X509Req.get_extensions (>=0.15) + 'PyOpenSSL>=0.15', 'pytz', 'requests', 'werkzeug', diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index edfd2eccf..e06fb334c 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -11,6 +11,7 @@ import os from cryptography.hazmat.primitives import serialization import OpenSSL +from acme import crypto_util as acme_crypto_util from acme import jose from letsencrypt import errors @@ -260,45 +261,6 @@ def make_ss_cert(key, domains, not_before=None, return OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, cert) -def _pyopenssl_cert_or_req_san(cert_or_req): - """Get Subject Alternative Names from certificate or CSR using pyOpenSSL. - - .. todo:: Implement directly in PyOpenSSL! - - :param cert_or_req: Certificate or CSR. - :type cert_or_req: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`. - - :returns: A list of Subject Alternative Names. - :rtype: list - - """ - # constants based on implementation of - # OpenSSL.crypto.X509Error._subjectAltNameString - parts_separator = ", " - part_separator = ":" - extension_short_name = "subjectAltName" - - if hasattr(cert_or_req, 'get_extensions'): # X509Req - extensions = cert_or_req.get_extensions() - else: # X509 - extensions = [cert_or_req.get_extension(i) - for i in xrange(cert_or_req.get_extension_count())] - - # pylint: disable=protected-access,no-member - label = OpenSSL.crypto.X509Extension._prefixes[OpenSSL.crypto._lib.GEN_DNS] - assert parts_separator not in label - prefix = label + part_separator - - san_extensions = [ - ext._subjectAltNameString().split(parts_separator) - for ext in extensions if ext.get_short_name() == extension_short_name] - # WARNING: this function assumes that no SAN can include - # parts_separator, hence the split! - - return [part.split(part_separator)[1] for parts in san_extensions - for part in parts if part.startswith(prefix)] - - def _get_sans_from_cert_or_req( cert_or_req_str, load_func, typ=OpenSSL.crypto.FILETYPE_PEM): try: @@ -306,7 +268,8 @@ def _get_sans_from_cert_or_req( except OpenSSL.crypto.Error as error: logger.exception(error) raise - return _pyopenssl_cert_or_req_san(cert_or_req) + # pylint: disable=protected-access + return acme_crypto_util._pyopenssl_cert_or_req_san(cert_or_req) def get_sans_from_cert(cert, typ=OpenSSL.crypto.FILETYPE_PEM): diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index 426237661..160d911a5 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -16,6 +16,7 @@ import tempfile import OpenSSL from acme import client as acme_client +from acme import crypto_util as acme_crypto_util from acme.jose import util as jose_util from letsencrypt import crypto_util @@ -520,7 +521,7 @@ class Cert(object): def get_san(self): """Get subject alternative name if available.""" # pylint: disable=protected-access - return ", ".join(crypto_util._pyopenssl_cert_or_req_san(self._cert)) + return ", ".join(acme_crypto_util._pyopenssl_cert_or_req_san(self._cert)) def __str__(self): text = [ diff --git a/letsencrypt/tests/achallenges_test.py b/letsencrypt/tests/achallenges_test.py index 253cc20d7..0ad47cbc9 100644 --- a/letsencrypt/tests/achallenges_test.py +++ b/letsencrypt/tests/achallenges_test.py @@ -4,6 +4,7 @@ import unittest import OpenSSL from acme import challenges +from acme import crypto_util as acme_crypto_util from acme import jose from letsencrypt import crypto_util @@ -35,7 +36,7 @@ class DVSNITest(unittest.TestCase): OpenSSL.crypto.FILETYPE_PEM, cert_pem) self.assertEqual(cert.get_subject().CN, "example.com") # pylint: disable=protected-access - self.assertEqual(crypto_util._pyopenssl_cert_or_req_san(cert), [ + self.assertEqual(acme_crypto_util._pyopenssl_cert_or_req_san(cert), [ "example.com", self.chall.nonce_domain, self.response.z_domain(self.chall)]) diff --git a/setup.py b/setup.py index 1e0d58a70..78207e230 100644 --- a/setup.py +++ b/setup.py @@ -37,8 +37,7 @@ install_requires = [ 'mock<1.1.0', # py26 'parsedatetime', 'psutil>=2.1.0', # net_connections introduced in 2.1.0 - # https://pyopenssl.readthedocs.org/en/latest/api/crypto.html#OpenSSL.crypto.X509Req.get_extensions - 'PyOpenSSL>=0.15', + 'PyOpenSSL', 'pyrfc3339', 'python2-pythondialog>=3.2.2rc1', # Debian squeeze support, cf. #280 'pytz', From 9f319769283c89118685dac27f0fcddfaf7667bf Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 13 Jul 2015 10:59:32 +0000 Subject: [PATCH 04/16] Add acme.crypto_util._serve_sni and ServeProbeSNITest. --- acme/acme/challenges.py | 4 +- acme/acme/challenges_test.py | 12 ++-- acme/acme/crypto_util.py | 105 ++++++++++++++++++++++++++++++---- acme/acme/crypto_util_test.py | 57 +++++++++++++++--- 4 files changed, 152 insertions(+), 26 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index a619e9a30..e60655ea1 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -200,9 +200,9 @@ class DVSNI(DVChallenge): host = socket.gethostbyname(domain) logging.debug('%s resolved to %s', domain, host) - kwargs.setdefault("port", self.PORT) kwargs.setdefault("host", host) - kwargs["server_hostname"] = self.nonce_domain + kwargs.setdefault("port", self.PORT) + kwargs["name"] = self.nonce_domain # TODO: try different methods? # pylint: disable=protected-access return crypto_util._probe_sni(**kwargs) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 377584f5a..f76bdf05a 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -189,24 +189,24 @@ class DVSNITest(unittest.TestCase): mock_gethostbyname.assert_called_once_with('foo.com') mock_probe_sni.assert_called_once_with( host='127.0.0.1', port=self.msg.PORT, - server_hostname='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') + name='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') self.msg.probe_cert('foo.com', host='8.8.8.8') mock_probe_sni.assert_called_with( - host='8.8.8.8', port=mock.ANY, server_hostname=mock.ANY) + host='8.8.8.8', port=mock.ANY, name=mock.ANY) self.msg.probe_cert('foo.com', port=1234) mock_probe_sni.assert_called_with( - host=mock.ANY, port=1234, server_hostname=mock.ANY) + host=mock.ANY, port=1234, name=mock.ANY) self.msg.probe_cert('foo.com', bar='baz') mock_probe_sni.assert_called_with( - host=mock.ANY, port=mock.ANY, server_hostname=mock.ANY, bar='baz') + host=mock.ANY, port=mock.ANY, name=mock.ANY, bar='baz') - self.msg.probe_cert('foo.com', server_hostname='xxx') + self.msg.probe_cert('foo.com', name='xxx') mock_probe_sni.assert_called_with( host=mock.ANY, port=mock.ANY, - server_hostname='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') + name='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') class DVSNIResponseTest(unittest.TestCase): diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 053089cb9..4268ede07 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -1,22 +1,105 @@ """Crypto utilities.""" +import contextlib +import logging import socket +import sys import OpenSSL +from acme import errors -def _probe_sni(server_hostname, host, port, timeout=10, - method=OpenSSL.SSL.SSLv23_METHOD): - sock = socket.create_connection((host, port), source_address=('0', 0)) + +logger = logging.getLogger(__name__) + + +_DEFAULT_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD + + +def _serve_sni(certs, sock, reuseaddr=True, method=_DEFAULT_SSL_METHOD, + accept=None): + """Start SNI-enabled server, that drops connection after handshake. + + :param certs: Mapping from SNI name to ``(key, cert)`` `tuple`. + :param sock: Already bound socket. + :param bool reuseaddr: Should `socket.SO_REUSEADDR` be set? + :param method: See `OpenSSL.SSL.Context` for allowed values. + :param accept: Callable that doesn't take any arguments and + returns ``True`` if more connections should be served. + + """ + def _pick_certificate(connection): + try: + key, cert = certs[connection.get_servername()] + except KeyError: + return + new_context = OpenSSL.SSL.Context(method) + new_context.use_privatekey(key) + new_context.use_certificate(cert) + connection.set_context(new_context) + + if reuseaddr: + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + sock.listen(1) # TODO: add func arg? + + while accept is None or accept(): + server, addr = sock.accept() + logger.debug('Received connection from %s', addr) + + with contextlib.closing(server): + context = OpenSSL.SSL.Context(method) + context.set_tlsext_servername_callback(_pick_certificate) + + server_ssl = OpenSSL.SSL.Connection(context, server) + server_ssl.set_accept_state() + try: + server_ssl.do_handshake() + server_ssl.shutdown() + except OpenSSL.SSL.Error as error: + raise errors.Error(error) + + +def _probe_sni(name, host, port=443, timeout=300, + method=_DEFAULT_SSL_METHOD, source_address=('0', 0)): + """Probe SNI server for SSL certificate. + + :param bytes name: Byte string to send as the server name in the + client hello message. + :param bytes host: Host to connect to. + :param int port: Port to connect to. + :param int timeout: Timeout in seconds. + :param method: See `OpenSSL.SSL.Context` for allowed values. + :param tuple source_address: Enables multi-path probing (selection + of source interface). See `socket.creation_connection` for more + info. Available only in Python 2.7+. + + :raises acme.errors.Error: In case of any problems. + + :returns: SSL certificate presented by the server. + :rtype: OpenSSL.crypto.X509 + + """ context = OpenSSL.SSL.Context(method) context.set_timeout(timeout) - connection = OpenSSL.SSL.Connection(context, sock) - connection.set_tlsext_host_name(server_hostname) # pyOpenSSL>=0.13 - connection.set_connect_state() - connection.do_handshake() - cert = connection.get_peer_certificate() - sock.close() - # TODO: shutdown() - return cert + + socket_kwargs = {} if sys.version < (2, 7) else { + 'source_address': source_address} + + try: + # pylint: disable=star-args + sock = socket.create_connection((host, port), **socket_kwargs) + except socket.error as error: + raise errors.Error(error) + + with contextlib.closing(sock) as client: + client_ssl = OpenSSL.SSL.Connection(context, client) + client_ssl.set_connect_state() + client_ssl.set_tlsext_host_name(name) # pyOpenSSL>=0.13 + try: + client_ssl.do_handshake() + client_ssl.shutdown() + except OpenSSL.SSL.Error as error: + raise errors.Error(error) + return client_ssl.get_peer_certificate() def _pyopenssl_cert_or_req_san(cert_or_req): diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 2e0c71b2b..5aa9d2d5b 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -1,21 +1,64 @@ """Tests for acme.crypto_util.""" import socket +import threading +import time import unittest +import mock import OpenSSL +from acme import errors +from acme import jose from acme import test_util -class ProbeSNITest(unittest.TestCase): - """Tests for acme.crypto_util._probe_sni.""" +class ServeProbeSNITest(unittest.TestCase): + """Tests for acme.crypto_util._serve_sni/_probe_sni.""" - def test_it(self): + def setUp(self): + self.cert = test_util.load_cert('cert.pem') + key = OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, + test_util.load_vector('rsa512_key.pem')) + # pylint: disable=protected-access + certs = {'foo': (key, self.cert._wrapped)} + + sock = socket.socket() + sock.bind(('', 0)) # pick random port + self.port = sock.getsockname()[1] + + self.server = threading.Thread(target=self._run_server, args=(certs, sock)) + self.server.start() + time.sleep(1) # TODO: avoid race conditions in other way + + @classmethod + def _run_server(cls, certs, sock): + from acme.crypto_util import _serve_sni + # TODO: improve testing of server errors and their conditions + try: + return _serve_sni( + certs, sock, accept=mock.Mock(side_effect=[True, False])) + except errors.Error: + pass + + def tearDown(self): + self.server.join() + + def _probe(self, name): from acme.crypto_util import _probe_sni - # TODO: mock this out - cert = _probe_sni( - "google.com", socket.gethostbyname("google.com"), port=443) - self.assertTrue(isinstance(cert, OpenSSL.crypto.X509)) + return jose.ComparableX509(_probe_sni( + name, host='127.0.0.1', port=self.port)) + + def test_probe_ok(self): + self.assertEqual(self.cert, self._probe('foo')) + + def test_probe_not_recognized_name(self): + self.assertRaises(errors.Error, self._probe, 'bar') + + def test_probe_connection_error(self): + self._probe('foo') + time.sleep(1) # TODO: avoid race conditions in other way + self.assertRaises(errors.Error, self._probe, 'bar') class PyOpenSSLCertOrReqSANTest(unittest.TestCase): From c546dddd7f6896d52b32eb1bce19c39615abc88d Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 13 Jul 2015 15:11:11 +0000 Subject: [PATCH 05/16] Add DVSNIResponse.verify_cert. --- acme/acme/challenges.py | 23 ++++++++++++++++++----- acme/acme/challenges_test.py | 33 ++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index e60655ea1..f2fc5ed07 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -9,6 +9,7 @@ import socket import OpenSSL import requests +from acme import errors from acme import crypto_util from acme import interfaces from acme import jose @@ -247,7 +248,23 @@ class DVSNIResponse(ChallengeResponse): return self.z(chall) + self.DOMAIN_SUFFIX def simple_verify(self, chall, domain, key, **kwargs): - """Verify DVSNI. + """Simple verify. + + Probes DVSNI certificate and checks it using `verify_cert`; + hence all arguments documented in `verify_cert`. + + """ + try: + cert = chall.probe_cert(domain=domain, **kwargs) + except errors.Error as error: + logger.debug(error, exc_info=True) + return False + # TODO: check "It is a valid self-signed certificate" and + # return False if not + return self.verify_cert(chall, domain, key, cert) + + def verify_cert(self, chall, domain, key, cert): + """Verify DVSNI certificate. :param .challenges.DVSNI chall: Corresponding challenge. :param str domain: Domain name being validated. @@ -260,10 +277,6 @@ class DVSNIResponse(ChallengeResponse): :rtype: bool """ - cert = chall.probe_cert(domain=domain, **kwargs) - # TODO: check "It is a valid self-signed certificate" and - # return False if not - # pylint: disable=protected-access sans = crypto_util._pyopenssl_cert_or_req_san(cert) logging.debug('Certificate %s. SANs: %s', cert.digest('sha1'), sans) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index f76bdf05a..a68a62ab4 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -6,6 +6,7 @@ import OpenSSL import requests import urlparse +from acme import errors from acme import jose from acme import other from acme import test_util @@ -245,12 +246,34 @@ class DVSNIResponseTest(unittest.TestCase): from acme.challenges import DVSNIResponse hash(DVSNIResponse.from_json(self.jmsg)) - def test_simple_verify(self): # TODO + @mock.patch('acme.challenges.DVSNIResponse.verify_cert') + def test_simple_verify(self, mock_verify_cert): + chall = mock.Mock() + chall.probe_cert.return_value = mock.sentinel.cert + mock_verify_cert.return_value = 'x' + self.assertEqual('x', self.msg.simple_verify( + chall, mock.sentinel.domain, mock.sentinel.key)) + chall.probe_cert.assert_called_once_with(domain=mock.sentinel.domain) + self.msg.verify_cert.assert_called_once_with( + chall, mock.sentinel.domain, mock.sentinel.key, + mock.sentinel.cert) + + def test_simple_verify_false_on_probe_error(self): + chall = mock.Mock() + chall.probe_cert.side_effect = errors.Error + self.assertFalse(self.msg.simple_verify( + chall=chall, domain=None, key=None)) + + def test_verify_cert_postive(self): + pass # XXX + + def test_verify_cert_negative(self): chall = mock.MagicMock() - chall.probe_cert.return_value = OpenSSL.crypto.load_certificate( - OpenSSL.crypto.FILETYPE_PEM, test_util.load_vector('cert.pem')) - self.assertFalse(self.msg.simple_verify(chall, "example.com", key=None)) - # TODO: key not None + self.assertFalse(self.msg.verify_cert( + chall, domain="example.com", key=None, + cert=OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, + test_util.load_vector('cert.pem')))) class RecoveryContactTest(unittest.TestCase): From bfe6adf215dc56a654c719cf8f34923bf1de2cc3 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 13 Jul 2015 19:53:23 +0000 Subject: [PATCH 06/16] py3 compat --- acme/acme/challenges.py | 2 +- acme/acme/challenges_test.py | 6 +++--- acme/acme/crypto_util.py | 8 +++++--- acme/acme/crypto_util_test.py | 10 +++++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 07872bed1..369558ae0 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -299,7 +299,7 @@ class DVSNIResponse(ChallengeResponse): key_filetype, cert.get_pubkey()) return (keys_match and domain in sans and - self.z_domain(chall) in sans) + self.z_domain(chall).decode() in sans) @Challenge.register diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 087d93bc8..e1cd1c541 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -191,7 +191,7 @@ class DVSNITest(unittest.TestCase): mock_gethostbyname.assert_called_once_with('foo.com') mock_probe_sni.assert_called_once_with( host='127.0.0.1', port=self.msg.PORT, - name='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') + name=b'a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') self.msg.probe_cert('foo.com', host='8.8.8.8') mock_probe_sni.assert_called_with( @@ -205,10 +205,10 @@ class DVSNITest(unittest.TestCase): mock_probe_sni.assert_called_with( host=mock.ANY, port=mock.ANY, name=mock.ANY, bar='baz') - self.msg.probe_cert('foo.com', name='xxx') + self.msg.probe_cert('foo.com', name=b'xxx') mock_probe_sni.assert_called_with( host=mock.ANY, port=mock.ANY, - name='a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') + name=b'a82d5ff8ef740d12881f6d3c2277ab2e.acme.invalid') class DVSNIResponseTest(unittest.TestCase): diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 4268ede07..8893dd53f 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -4,6 +4,8 @@ import logging import socket import sys +from six.moves import range # pylint: disable=import-error + import OpenSSL from acme import errors @@ -81,7 +83,7 @@ def _probe_sni(name, host, port=443, timeout=300, context = OpenSSL.SSL.Context(method) context.set_timeout(timeout) - socket_kwargs = {} if sys.version < (2, 7) else { + socket_kwargs = {} if sys.version_info < (2, 7) else { 'source_address': source_address} try: @@ -121,13 +123,13 @@ def _pyopenssl_cert_or_req_san(cert_or_req): # OpenSSL.crypto.X509Error._subjectAltNameString parts_separator = ", " part_separator = ":" - extension_short_name = "subjectAltName" + extension_short_name = b"subjectAltName" if hasattr(cert_or_req, 'get_extensions'): # X509Req extensions = cert_or_req.get_extensions() else: # X509 extensions = [cert_or_req.get_extension(i) - for i in xrange(cert_or_req.get_extension_count())] + for i in range(cert_or_req.get_extension_count())] # pylint: disable=protected-access,no-member label = OpenSSL.crypto.X509Extension._prefixes[OpenSSL.crypto._lib.GEN_DNS] diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 5aa9d2d5b..10d62fbf5 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -21,7 +21,7 @@ class ServeProbeSNITest(unittest.TestCase): OpenSSL.crypto.FILETYPE_PEM, test_util.load_vector('rsa512_key.pem')) # pylint: disable=protected-access - certs = {'foo': (key, self.cert._wrapped)} + certs = {b'foo': (key, self.cert._wrapped)} sock = socket.socket() sock.bind(('', 0)) # pick random port @@ -50,15 +50,15 @@ class ServeProbeSNITest(unittest.TestCase): name, host='127.0.0.1', port=self.port)) def test_probe_ok(self): - self.assertEqual(self.cert, self._probe('foo')) + self.assertEqual(self.cert, self._probe(b'foo')) def test_probe_not_recognized_name(self): - self.assertRaises(errors.Error, self._probe, 'bar') + self.assertRaises(errors.Error, self._probe, b'bar') def test_probe_connection_error(self): - self._probe('foo') + self._probe(b'foo') time.sleep(1) # TODO: avoid race conditions in other way - self.assertRaises(errors.Error, self._probe, 'bar') + self.assertRaises(errors.Error, self._probe, b'bar') class PyOpenSSLCertOrReqSANTest(unittest.TestCase): From d7d98d79ce45c7ea3793227835b2b7efaa313ee0 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 13 Jul 2015 20:24:16 +0000 Subject: [PATCH 07/16] please pylint --- acme/acme/crypto_util.py | 2 +- letsencrypt/tests/achallenges_test.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 8893dd53f..58e2aecf3 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -4,7 +4,7 @@ import logging import socket import sys -from six.moves import range # pylint: disable=import-error +from six.moves import range # pylint: disable=import-error,redefined-builtin import OpenSSL diff --git a/letsencrypt/tests/achallenges_test.py b/letsencrypt/tests/achallenges_test.py index 0ad47cbc9..3d46c781a 100644 --- a/letsencrypt/tests/achallenges_test.py +++ b/letsencrypt/tests/achallenges_test.py @@ -7,8 +7,6 @@ from acme import challenges from acme import crypto_util as acme_crypto_util from acme import jose -from letsencrypt import crypto_util - from letsencrypt.tests import acme_util from letsencrypt.tests import test_util From f3538cd114b9d1822ddeae22e28e019f8ad7da36 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 18 Jul 2015 07:33:46 +0000 Subject: [PATCH 08/16] Add comment about _DEFAULT_DVSNI_SSL_METHOD. --- acme/acme/crypto_util.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 58e2aecf3..5633c077d 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -13,11 +13,20 @@ from acme import errors logger = logging.getLogger(__name__) - -_DEFAULT_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD +# DVSNI certificate serving and probing is not affected by SSL +# vulnerabilities: prober needs to check certificate for expected +# contents anyway. Working SNI is the only thing that's necessary for +# the challenge and thus scoping down SSL/TLS method (version) would +# cause interoperability issues: TLSv1_METHOD is only compatible with +# TLSv1_METHOD, while SSLv23_METHOD is compatible with all other +# methods, including TLSv2_METHOD (read more at +# https://www.openssl.org/docs/ssl/SSLv23_method.html). _serve_sni +# should be changed to use "set_options" to disable SSLv2 and SSLv3, +# in case it's used for things other than probing/serving! +_DEFAULT_DVSNI_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD -def _serve_sni(certs, sock, reuseaddr=True, method=_DEFAULT_SSL_METHOD, +def _serve_sni(certs, sock, reuseaddr=True, method=_DEFAULT_DVSNI_SSL_METHOD, accept=None): """Start SNI-enabled server, that drops connection after handshake. @@ -61,7 +70,7 @@ def _serve_sni(certs, sock, reuseaddr=True, method=_DEFAULT_SSL_METHOD, def _probe_sni(name, host, port=443, timeout=300, - method=_DEFAULT_SSL_METHOD, source_address=('0', 0)): + method=_DEFAULT_DVSNI_SSL_METHOD, source_address=('0', 0)): """Probe SNI server for SSL certificate. :param bytes name: Byte string to send as the server name in the From 61e19c98822cc34ba730822dc938ecab5f47582a Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 18 Jul 2015 10:53:43 +0000 Subject: [PATCH 09/16] DVSNIResponse.gen_cert, fix verify_cert, add tests. --- acme/acme/challenges.py | 57 +++++++++++++----- acme/acme/challenges_test.py | 59 ++++++++++++------- acme/acme/crypto_util.py | 42 ++++++++++++- acme/acme/jose/__init__.py | 1 + acme/acme/jose/util.py | 42 ++++++++----- acme/acme/test_util.py | 6 ++ .../letsencrypt_nginx/configurator.py | 17 ++++-- letsencrypt/achallenges.py | 11 +++- letsencrypt/crypto_util.py | 51 +++------------- letsencrypt/tests/crypto_util_test.py | 9 --- 10 files changed, 179 insertions(+), 116 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index a2fc5db3c..89bc68966 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -6,6 +6,8 @@ import logging import os import socket +from cryptography.hazmat.backends import default_backend +from cryptography import x509 import OpenSSL import requests @@ -245,10 +247,25 @@ class DVSNIResponse(ChallengeResponse): return z.hexdigest().encode() def z_domain(self, chall): - """Domain name for certificate subjectAltName.""" + """Domain name for certificate subjectAltName. + + :rtype bytes: + + """ return self.z(chall) + self.DOMAIN_SUFFIX - def simple_verify(self, chall, domain, key, **kwargs): + def gen_cert(self, chall, domain, key): + """Generate DVSNI certificate. + + :param .DVSNI chall: Corresponding challenge. + :param unicode domain: + :param OpenSSL.crypto.PKey + + """ + return crypto_util.gen_ss_cert(key, [ + domain, chall.nonce_domain.decode(), self.z_domain(chall).decode()]) + + def simple_verify(self, chall, domain, public_key, **kwargs): """Simple verify. Probes DVSNI certificate and checks it using `verify_cert`; @@ -260,37 +277,47 @@ class DVSNIResponse(ChallengeResponse): except errors.Error as error: logger.debug(error, exc_info=True) return False - # TODO: check "It is a valid self-signed certificate" and - # return False if not - return self.verify_cert(chall, domain, key, cert) + return self.verify_cert(chall, domain, public_key, cert) - def verify_cert(self, chall, domain, key, cert): + def verify_cert(self, chall, domain, public_key, cert): """Verify DVSNI certificate. :param .challenges.DVSNI chall: Corresponding challenge. :param str domain: Domain name being validated. - :param OpenSSL.crypto.PKey key: Public key for the key pair + :param public_key: Public key for the key pair being authorized. If ``None`` key verification is not performed! + :type public_key: + `~cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey` + or + `~cryptography.hazmat.primitives.asymmetric.dsa.DSAPublicKey` + or + `~cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePublicKey` + wrapped in `.ComparableKey + :param OpenSSL.crypto.X509 cert: :returns: ``True`` iff client's control of the domain has been verified, ``False`` otherwise. :rtype: bool """ + # TODO: check "It is a valid self-signed certificate" and + # return False if not + # pylint: disable=protected-access sans = crypto_util._pyopenssl_cert_or_req_san(cert) logging.debug('Certificate %s. SANs: %s', cert.digest('sha1'), sans) - key_filetype = OpenSSL.crypto.FILETYPE_ASN1 - if key is None: - logging.warn('No key verification is performed') - keys_match = key is None or OpenSSL.crypto.dump_privatekey( - key_filetype, key) == OpenSSL.crypto.dump_privatekey( - key_filetype, cert.get_pubkey()) + cert = x509.load_der_x509_certificate( + OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_ASN1, cert), + default_backend()) - return (keys_match and domain in sans and - self.z_domain(chall).decode() in sans) + if public_key is None: + logging.warn('No key verification is performed') + elif public_key != jose.ComparableKey(cert.public_key()): + return False + + return domain in sans and self.z_domain(chall).decode() in sans @Challenge.register diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index efadf077f..e4ec37362 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -210,26 +210,31 @@ class DVSNIResponseTest(unittest.TestCase): def setUp(self): from acme.challenges import DVSNIResponse - self.msg = DVSNIResponse( - s=b'\xf5\xd6\xe3\xb2]\xe0L\x0bN\x9cKJ\x14I\xa1K\xa3#\xf9\xa8' - b'\xcd\x8c7\x0e\x99\x19)\xdc\xb7\xf3\x9bw') + # pylint: disable=invalid-name + s = '9dbjsl3gTAtOnEtKFEmhS6Mj-ajNjDcOmRkp3Lfzm3c' + self.msg = DVSNIResponse(s=jose.decode_b64jose(s)) self.jmsg = { 'resource': 'challenge', 'type': 'dvsni', - 's': '9dbjsl3gTAtOnEtKFEmhS6Mj-ajNjDcOmRkp3Lfzm3c', + 's': s, } - def test_z_and_domain(self): from acme.challenges import DVSNI - challenge = DVSNI( - r=b"O*\xb4-\xad\xec\x95>\xed\xa9\r0\x94\xe8\x97\x9c&6" - b"\xbf'\xb3\xed\x9a9nX\x0f'\\m\xe7\x12", - nonce=int('439736375371401115242521957580409149254868992063' - '44333654741504362774620418661')) + self.chall = DVSNI( + r=jose.decode_b64jose('Tyq0La3slT7tqQ0wlOiXnCY2vyez7Zo5blgPJ1xt5xI'), + nonce=jose.decode_b64jose('a82d5ff8ef740d12881f6d3c2277ab2e')) + self.z = (b'38e612b0397cc2624a07d351d7ef50e4' + b'6134c0213d9ed52f7d7c611acaeed41b') + self.domain = 'foo.com' + self.key = test_util.load_pyopenssl_private_key('rsa512_key.pem') + self.public_key = test_util.load_rsa_private_key( + 'rsa512_key.pem').public_key() + + def test_z_and_domain(self): # pylint: disable=invalid-name - z = b'38e612b0397cc2624a07d351d7ef50e46134c0213d9ed52f7d7c611acaeed41b' - self.assertEqual(z, self.msg.z(challenge)) - self.assertEqual(z + b'.acme.invalid', self.msg.z_domain(challenge)) + self.assertEqual(self.z, self.msg.z(self.chall)) + self.assertEqual( + self.z + b'.acme.invalid', self.msg.z_domain(self.chall)) def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) @@ -258,18 +263,28 @@ class DVSNIResponseTest(unittest.TestCase): chall = mock.Mock() chall.probe_cert.side_effect = errors.Error self.assertFalse(self.msg.simple_verify( - chall=chall, domain=None, key=None)) + chall=chall, domain=None, public_key=None)) - def test_verify_cert_postive(self): - pass # XXX + def test_gen_verify_cert_postive_no_key(self): + cert = self.msg.gen_cert(self.chall, self.domain, self.key) + self.assertTrue(self.msg.verify_cert( + self.chall, self.domain, public_key=None, cert=cert)) - def test_verify_cert_negative(self): - chall = mock.MagicMock() + def test_gen_verify_cert_postive_with_key(self): + cert = self.msg.gen_cert(self.chall, self.domain, self.key) + self.assertTrue(self.msg.verify_cert( + self.chall, self.domain, public_key=self.public_key, cert=cert)) + + def test_gen_verify_cert_negative_with_wrong_key(self): + cert = self.msg.gen_cert(self.chall, self.domain, self.key) + key = test_util.load_rsa_private_key('rsa256_key.pem').public_key() self.assertFalse(self.msg.verify_cert( - chall, domain="example.com", key=None, - cert=OpenSSL.crypto.load_certificate( - OpenSSL.crypto.FILETYPE_PEM, - test_util.load_vector('cert.pem')))) + self.chall, self.domain, public_key=key, cert=cert)) + + def test_gen_verify_cert_negative(self): + cert = self.msg.gen_cert(self.chall, self.domain + 'x', self.key) + self.assertFalse(self.msg.verify_cert( + self.chall, self.domain, public_key=None, cert=cert)) class RecoveryContactTest(unittest.TestCase): diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 5633c077d..cb796cb88 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -125,7 +125,7 @@ def _pyopenssl_cert_or_req_san(cert_or_req): :type cert_or_req: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`. :returns: A list of Subject Alternative Names. - :rtype: list + :rtype: `list` of `unicode` """ # constants based on implementation of @@ -153,3 +153,43 @@ def _pyopenssl_cert_or_req_san(cert_or_req): return [part.split(part_separator)[1] for parts in san_extensions for part in parts if part.startswith(prefix)] + + +def gen_ss_cert(key, domains, not_before=None, validity=(7 * 24 * 60 * 60)): + """Generate new self-signed certificate. + + :type domains: `list` of `unicode` + :param OpenSSL.crypto.PKey key: + + Uses key and contains all domains. + + """ + assert domains, "Must provide one or more hostnames for the cert." + cert = OpenSSL.crypto.X509() + cert.set_serial_number(1337) + cert.set_version(2) + + extensions = [ + OpenSSL.crypto.X509Extension( + b"basicConstraints", True, b"CA:TRUE, pathlen:0"), + ] + + cert.get_subject().CN = domains[0] + # TODO: what to put into cert.get_subject()? + cert.set_issuer(cert.get_subject()) + + if len(domains) > 1: + extensions.append(OpenSSL.crypto.X509Extension( + b"subjectAltName", + critical=False, + value=b", ".join(b"DNS:" + d.encode() for d in domains) + )) + + cert.add_extensions(extensions) + + cert.gmtime_adj_notBefore(0 if not_before is None else not_before) + cert.gmtime_adj_notAfter(validity) + + cert.set_pubkey(key) + cert.sign(key, "sha256") + return cert diff --git a/acme/acme/jose/__init__.py b/acme/acme/jose/__init__.py index 76969139b..f39c3beab 100644 --- a/acme/acme/jose/__init__.py +++ b/acme/acme/jose/__init__.py @@ -76,6 +76,7 @@ from acme.jose.jws import ( from acme.jose.util import ( ComparableX509, + ComparableKey, ComparableRSAKey, ImmutableMap, ) diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py index fd58a9e97..704476795 100644 --- a/acme/acme/jose/util.py +++ b/acme/acme/jose/util.py @@ -66,14 +66,14 @@ class ComparableX509(object): # pylint: disable=too-few-public-methods return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) -class ComparableRSAKey(object): # pylint: disable=too-few-public-methods - """Wrapper for `cryptography` RSA keys. +class ComparableKey(object): # pylint: disable=too-few-public-methods + """Comparable wrapper for `cryptography` keys. - Wraps around: - - `cryptography.hazmat.primitives.assymetric.RSAPrivateKey` - - `cryptography.hazmat.primitives.assymetric.RSAPublicKey` + See https://github.com/pyca/cryptography/issues/2122. """ + __hash__ = NotImplemented + def __init__(self, wrapped): self._wrapped = wrapped @@ -85,19 +85,36 @@ class ComparableRSAKey(object): # pylint: disable=too-few-public-methods if (not isinstance(other, self.__class__) or self._wrapped.__class__ is not other._wrapped.__class__): return NotImplemented - # RSA*KeyWithSerialization requires cryptography>=0.8 - if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization): + elif hasattr(self._wrapped, 'private_numbers'): return self.private_numbers() == other.private_numbers() - elif isinstance(self._wrapped, rsa.RSAPublicKeyWithSerialization): + elif hasattr(self._wrapped, 'public_numbers'): return self.public_numbers() == other.public_numbers() else: - return False # we shouldn't reach here... + return NotImplemented def __ne__(self, other): return not self == other + def __repr__(self): + return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) + + def public_key(self): + """Get wrapped public key.""" + return self.__class__(self._wrapped.public_key()) + + +class ComparableRSAKey(ComparableKey): # pylint: disable=too-few-public-methods + """Wrapper for `cryptography` RSA keys. + + Wraps around: + - `cryptography.hazmat.primitives.assymetric.RSAPrivateKey` + - `cryptography.hazmat.primitives.assymetric.RSAPublicKey` + + """ + def __hash__(self): # public_numbers() hasn't got stable hash! + # https://github.com/pyca/cryptography/issues/2143 if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization): priv = self.private_numbers() pub = priv.public_numbers @@ -107,13 +124,6 @@ class ComparableRSAKey(object): # pylint: disable=too-few-public-methods pub = self.public_numbers() return hash((self.__class__, pub.n, pub.e)) - def __repr__(self): - return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) - - def public_key(self): - """Get wrapped public key.""" - return self.__class__(self._wrapped.public_key()) - class ImmutableMap(collections.Mapping, collections.Hashable): # pylint: disable=too-few-public-methods diff --git a/acme/acme/test_util.py b/acme/acme/test_util.py index 85289a978..8ad118e17 100644 --- a/acme/acme/test_util.py +++ b/acme/acme/test_util.py @@ -55,3 +55,9 @@ def load_rsa_private_key(*names): serialization.load_der_private_key) return jose.ComparableRSAKey(loader( load_vector(*names), password=None, backend=default_backend())) + +def load_pyopenssl_private_key(*names): + """Load pyOpenSSL private key.""" + loader = _guess_loader( + names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1) + return OpenSSL.crypto.load_privatekey(loader, load_vector(*names)) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 6a492428d..18b5d719a 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -7,9 +7,11 @@ import socket import subprocess import sys +import OpenSSL import zope.interface from acme import challenges +from acme import crypto_util as acme_crypto_util from letsencrypt import achallenges from letsencrypt import constants as core_constants @@ -271,14 +273,17 @@ class NginxConfigurator(common.Plugin): def _get_snakeoil_paths(self): # TODO: generate only once tmp_dir = os.path.join(self.config.work_dir, "snakeoil") - key = crypto_util.init_save_key( + le_key = crypto_util.init_save_key( key_size=1024, key_dir=tmp_dir, keyname="key.pem") - cert_pem = crypto_util.make_ss_cert( - key.pem, domains=[socket.gethostname()]) - cert = os.path.join(tmp_dir, "cert.pem") - with open(cert, 'w') as cert_file: + key = OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, le_key.pem) + cert = acme_crypto_util.gen_ss_cert(key, domains=[socket.gethostname()]) + cert_path = os.path.join(tmp_dir, "cert.pem") + cert_pem = OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, cert) + with open(cert_path, 'w') as cert_file: cert_file.write(cert_pem) - return cert, key.file + return cert_path, le_key.file def _make_server_ssl(self, vhost): """Make a server SSL. diff --git a/letsencrypt/achallenges.py b/letsencrypt/achallenges.py index a81ae05a2..df7f96dd6 100644 --- a/letsencrypt/achallenges.py +++ b/letsencrypt/achallenges.py @@ -17,6 +17,8 @@ Note, that all annotated challenges act as a proxy objects:: achall.token == challb.token """ +import OpenSSL + from acme import challenges from acme.jose import util as jose_util @@ -48,7 +50,7 @@ class DVSNI(AnnotatedChallenge): acme_type = challenges.DVSNI def gen_cert_and_response(self, s=None): # pylint: disable=invalid-name - """Generate a DVSNI cert and save it to filepath. + """Generate a DVSNI cert and response. :returns: ``(cert_pem, response)`` tuple, where ``cert_pem`` is the PEM encoded certificate and ``response`` is an instance @@ -56,9 +58,12 @@ class DVSNI(AnnotatedChallenge): :rtype: tuple """ + key = crypto_util.private_jwk_to_pyopenssl(self.key) response = challenges.DVSNIResponse(s=s) - cert_pem = crypto_util.make_ss_cert(self.key, [ - self.domain, self.nonce_domain, response.z_domain(self.challb)]) + cert = response.gen_cert(self.challb.chall, self.domain, key) + cert_pem = OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, cert) + return cert_pem, response diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index e06fb334c..12c449261 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -12,7 +12,6 @@ from cryptography.hazmat.primitives import serialization import OpenSSL from acme import crypto_util as acme_crypto_util -from acme import jose from letsencrypt import errors from letsencrypt import le_util @@ -216,49 +215,13 @@ def pyopenssl_load_certificate(data): return _pyopenssl_load(data, OpenSSL.crypto.load_certificate) -def make_ss_cert(key, domains, not_before=None, - validity=(7 * 24 * 60 * 60)): - """Returns new self-signed cert in PEM form. - - Uses key and contains all domains. - - """ - if isinstance(key, jose.JWK): - key = key.key.private_bytes( - encoding=serialization.Encoding.PEM, - format=serialization.PrivateFormat.TraditionalOpenSSL, - encryption_algorithm=serialization.NoEncryption()) - - assert domains, "Must provide one or more hostnames for the cert." - pkey = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, key) - cert = OpenSSL.crypto.X509() - cert.set_serial_number(1337) - cert.set_version(2) - - extensions = [ - OpenSSL.crypto.X509Extension( - "basicConstraints", True, 'CA:TRUE, pathlen:0'), - ] - - cert.get_subject().CN = domains[0] - # TODO: what to put into cert.get_subject()? - cert.set_issuer(cert.get_subject()) - - if len(domains) > 1: - extensions.append(OpenSSL.crypto.X509Extension( - "subjectAltName", - critical=False, - value=", ".join("DNS:%s" % d for d in domains) - )) - - cert.add_extensions(extensions) - - cert.gmtime_adj_notBefore(0 if not_before is None else not_before) - cert.gmtime_adj_notAfter(validity) - - cert.set_pubkey(pkey) - cert.sign(pkey, "sha256") - return OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, cert) +def private_jwk_to_pyopenssl(jwk): + """Convert private JWK to pyOpenSSL key.""" + key_pem = jwk.key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=serialization.NoEncryption()) + return OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, key_pem) def _get_sans_from_cert_or_req( diff --git a/letsencrypt/tests/crypto_util_test.py b/letsencrypt/tests/crypto_util_test.py index fd3b60c60..b248ffd8a 100644 --- a/letsencrypt/tests/crypto_util_test.py +++ b/letsencrypt/tests/crypto_util_test.py @@ -160,15 +160,6 @@ class ValidPrivkeyTest(unittest.TestCase): self.assertFalse(self._call('foo bar')) -class MakeSSCertTest(unittest.TestCase): - # pylint: disable=too-few-public-methods - """Tests for letsencrypt.crypto_util.make_ss_cert.""" - - def test_it(self): # pylint: disable=no-self-use - from letsencrypt.crypto_util import make_ss_cert - make_ss_cert(RSA512_KEY, ['example.com', 'www.example.com']) - - class GetSANsFromCertTest(unittest.TestCase): """Tests for letsencrypt.crypto_util.get_sans_from_cert.""" From 7a79915f0c789c465b41d2e6819d3aa160afed1c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 18 Jul 2015 12:59:32 +0000 Subject: [PATCH 10/16] Common plugin: export key to PKCS8, not OpenSSL. --- letsencrypt/plugins/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index 104e8d9c4..90daa685f 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -182,7 +182,7 @@ class Dvsni(object): # Write out challenge key key_pem = achall.key.key.private_bytes( encoding=serialization.Encoding.PEM, - format=serialization.PrivateFormat.TraditionalOpenSSL, + format=serialization.PrivateFormat.PKCS8, encryption_algorithm=serialization.NoEncryption()) with le_util.safe_open(key_path, 'wb', chmod=0o400) as key_file: key_file.write(key_pem) From fe3c3be675f175fbb9dfed9b97f1d556253d0014 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 18 Jul 2015 19:25:30 +0000 Subject: [PATCH 11/16] Travis: run Boulder in AMQP mode (fixes #620). --- .travis.yml | 5 ++++- tests/boulder-start.sh | 9 +++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7cccd20c9..52ad49506 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,8 @@ language: python +services: + - rabbitmq + # http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS before_install: - travis_retry sudo ./bootstrap/ubuntu.sh @@ -20,7 +23,7 @@ env: - TOXENV=cover install: "travis_retry pip install tox coveralls" -before_script: '[ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/boulder-start.sh' +before_script: '[ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/boulder-start.sh amqp' script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || (source .tox/$TOXENV/bin/activate && ./tests/boulder-integration.sh))' after_success: '[ "$TOXENV" == "cover" ] && coveralls' diff --git a/tests/boulder-start.sh b/tests/boulder-start.sh index 49139ff3c..20f64bcce 100755 --- a/tests/boulder-start.sh +++ b/tests/boulder-start.sh @@ -11,5 +11,10 @@ export GOPATH="${GOPATH:-/tmp/go}" go get -d github.com/letsencrypt/boulder/cmd/boulder cd $GOPATH/src/github.com/letsencrypt/boulder make -j4 # Travis has 2 cores per build instance. -./start.sh & -# Hopefully start.sh bootstraps before integration test is started... +if [ "$1" = "amqp" ]; +then + ./start.py & +else + ./start.sh & +fi +# Hopefully start.py/start.sh bootstraps before integration test is started... From 9e2682a025f32dabe8fe7256141785a698d0c257 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 19 Jul 2015 11:02:17 +0000 Subject: [PATCH 12/16] 100% coverage for manual test mode and related code. --- letsencrypt/auth_handler.py | 12 ++++---- letsencrypt/plugins/manual.py | 11 ++++--- letsencrypt/plugins/manual_test.py | 48 ++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index e1a964f77..5969dc36f 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -153,22 +153,22 @@ class AuthHandler(object): """ active_achalls = [] for achall, resp in itertools.izip(achalls, resps): + # XXX: make sure that all achalls, including those + # corresponding to None or False returned from + # Authenticator are removed from the queue and thus avoid + # infinite loop + active_achalls.append(achall) + # Don't send challenges for None and False authenticator responses if resp is not None and resp: self.acme.answer_challenge(achall.challb, resp) # TODO: answer_challenge returns challr, with URI, # that can be used in _find_updated_challr # comparisons... - active_achalls.append(achall) if achall.domain in chall_update: chall_update[achall.domain].append(achall) else: chall_update[achall.domain] = [achall] - else: # resp is None or not resp - # XXX: make sure that achalls corresponding to None or - # False returned from Authenticator are removed from - # the queue and thus avoid infinite loop - active_achalls.append(achall) return active_achalls diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 771833d6f..b5ddd2140 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -89,6 +89,7 @@ s.serve_forever()" """ else self.HTTPS_TEMPLATE) self._root = (tempfile.mkdtemp() if self.conf("test-mode") else "/tmp/letsencrypt") + self._httpd = None @classmethod def add_parser_arguments(cls, add): @@ -136,7 +137,6 @@ binary for temporary key/certificate generation.""".replace("\n", "") if self.conf("test-mode"): logger.debug("Test mode. Executing the manual command: %s", command) try: - # pylint: disable=attribute-defined-outside-init self._httpd = subprocess.Popen( command, # don't care about setting stdout and stderr, @@ -146,13 +146,13 @@ binary for temporary key/certificate generation.""".replace("\n", "") preexec_fn=os.setsid) except OSError as error: # ValueError should not happen! logging.debug( - "Couldn't execute manual command", error, exc_info=True) + "Couldn't execute manual command: %s", error, exc_info=True) return False logger.debug("Manual command running as PID %s.", self._httpd.pid) # give it some time to bootstrap, before we try to verify # (cert generation in case of simpleHttpS might take time) time.sleep(4) # XXX - if self._httpd.poll(): + if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: self._notify_and_wait(self.MESSAGE_TEMPLATE.format( @@ -164,7 +164,8 @@ binary for temporary key/certificate generation.""".replace("\n", "") achall.challb, achall.domain, self.config.simple_http_port): return response else: - if self.conf("test-mode") and self._httpd.poll(): + if self.conf("test-mode") and self._httpd.poll() is not None: + # simply verify cause command failure... return False return None @@ -178,6 +179,8 @@ binary for temporary key/certificate generation.""".replace("\n", "") def cleanup(self, achalls): # pylint: disable=missing-docstring,no-self-use,unused-argument if self.conf("test-mode"): + assert self._httpd is not None, ( + "cleanup() must be called after perform()") if self._httpd.poll() is None: logger.debug("Terminating manual command process") os.killpg(self._httpd.pid, signal.SIGTERM) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index e57864b9d..fe95a00f0 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -1,4 +1,5 @@ """Tests for letsencrypt.plugins.manual.""" +import signal import unittest import mock @@ -6,6 +7,8 @@ import mock from acme import challenges from letsencrypt import achallenges +from letsencrypt import errors + from letsencrypt.tests import acme_util @@ -21,6 +24,12 @@ class ManualAuthenticatorTest(unittest.TestCase): self.achalls = [achallenges.SimpleHTTP( challb=acme_util.SIMPLE_HTTP, domain="foo.com", key=None)] + config_test_mode = mock.MagicMock( + no_simple_http_tls=True, simple_http_port=4430, + manual_test_mode=True) + self.auth_test_mode = ManualAuthenticator( + config=config_test_mode, name="manual") + def test_more_info(self): self.assertTrue(isinstance(self.auth.more_info(), str)) @@ -52,6 +61,45 @@ class ManualAuthenticatorTest(unittest.TestCase): mock_verify.return_value = False self.assertEqual([None], self.auth.perform(self.achalls)) + @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) + def test_perform_test_command_oserror(self, mock_popen): + mock_popen.side_effect = OSError + self.assertEqual([False], self.auth_test_mode.perform(self.achalls)) + + @mock.patch("letsencrypt.plugins.manual.time.sleep", autospec=True) + @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) + def test_perform_test_command_run_failure( + self, mock_popen, unused_mock_sleep): + mock_popen.poll.return_value = 10 + mock_popen.return_value.pid = 1234 + self.assertRaises( + errors.Error, self.auth_test_mode.perform, self.achalls) + + @mock.patch("letsencrypt.plugins.manual.time.sleep", autospec=True) + @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify", + autospec=True) + @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) + def test_perform_test_mode(self, mock_popen, mock_verify, mock_sleep): + mock_popen.return_value.poll.side_effect = [None, 10] + mock_popen.return_value.pid = 1234 + mock_verify.return_value = False + self.assertEqual([False], self.auth_test_mode.perform(self.achalls)) + self.assertEqual(1, mock_sleep.call_count) + + def test_cleanup_test_mode_already_terminated(self): + # pylint: disable=protected-access + self.auth_test_mode._httpd = httpd = mock.Mock() + httpd.poll.return_value = 0 + self.auth_test_mode.cleanup(self.achalls) + + @mock.patch("letsencrypt.plugins.manual.os.killpg", autospec=True) + def test_cleanup_test_mode_kills_still_running(self, mock_killpg): + # pylint: disable=protected-access + self.auth_test_mode._httpd = httpd = mock.Mock(pid=1234) + httpd.poll.return_value = None + self.auth_test_mode.cleanup(self.achalls) + mock_killpg.assert_called_once_with(1234, signal.SIGTERM) + if __name__ == "__main__": unittest.main() # pragma: no cover From 82147f1f5e3373ed9e623cbee618339ba8a3f2a6 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 19 Jul 2015 11:22:57 +0000 Subject: [PATCH 13/16] Travis: add le.wtf to /etc/hosts. --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index 7cccd20c9..587ac6ccc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,6 +19,11 @@ env: - TOXENV=lint - TOXENV=cover +# make sure simplehttp simple verification works (custom /etc/hosts) +addons: + hosts: + - le.wtf + install: "travis_retry pip install tox coveralls" before_script: '[ "xxx$BOULDER_INTEGRATION" = "xxx" ] || ./tests/boulder-start.sh' script: 'travis_retry tox && ([ "xxx$BOULDER_INTEGRATION" = "xxx" ] || (source .tox/$TOXENV/bin/activate && ./tests/boulder-integration.sh))' From 7908ea0b868f159aace798c67f6e5987dfef1e78 Mon Sep 17 00:00:00 2001 From: Peter Mosmans Date: Mon, 20 Jul 2015 10:17:58 +0200 Subject: [PATCH 14/16] Fixed typo Changed config-changes in the short help (wrong) to config_changes (right) --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 0797f23b4..6692d9c99 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -57,7 +57,7 @@ USAGE = SHORT_USAGE + """Major SUBCOMMANDS are: install Install a previously obtained cert in a server revoke Revoke a previously obtained certificate rollback Rollback server configuration changes made during install - config-changes Show changes made to server config during installation + config_changes Show changes made to server config during installation Choice of server for authentication/installation: From 8e0b271ccd52a7abfb016e64b799a6e7ba46c78c Mon Sep 17 00:00:00 2001 From: Bigfish Date: Thu, 23 Jul 2015 15:47:11 +0800 Subject: [PATCH 15/16] remove sudo before brew (OS X) brew will refuse sudo ref: https://github.com/Homebrew/homebrew/issues/9953 --- docs/using.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/using.rst b/docs/using.rst index fa0f7b8a9..d22f22076 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -83,7 +83,7 @@ Mac OSX .. code-block:: shell - sudo ./bootstrap/mac.sh + ./bootstrap/mac.sh Fedora From ab097d128b502eaa44a8d77d2ff9d5a59dcb5126 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 24 Jul 2015 06:41:56 +0000 Subject: [PATCH 16/16] Fix logging -> logger typo. --- letsencrypt/plugins/manual.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index b5ddd2140..83f2c0f70 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -145,7 +145,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") # "preexec_fn" is UNIX specific, but so is "command" preexec_fn=os.setsid) except OSError as error: # ValueError should not happen! - logging.debug( + logger.debug( "Couldn't execute manual command: %s", error, exc_info=True) return False logger.debug("Manual command running as PID %s.", self._httpd.pid)