From db712534e545fa05211568b738474b74b0c92217 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 16 Dec 2015 16:53:12 -0800 Subject: [PATCH 001/100] Make dump() public --- acme/acme/jose/util.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py index ab3606efc..1d98aad4e 100644 --- a/acme/acme/jose/util.py +++ b/acme/acme/jose/util.py @@ -43,8 +43,17 @@ class ComparableX509(object): # pylint: disable=too-few-public-methods def __getattr__(self, name): return getattr(self._wrapped, name) - def _dump(self, filetype=OpenSSL.crypto.FILETYPE_ASN1): - # pylint: disable=missing-docstring,protected-access + def dump(self, filetype=OpenSSL.crypto.FILETYPE_ASN1): + """Dumps the object into a buffer with the specified encoding. + + :param int filetype: The desired encoding. Should be one of + OpenSSL.crypto.FILETYPE_ASN1, OpenSSL.crypto.FILETYPE_PEM, + or OpenSSL.crypto.FILETYPE_TEXT. + + :returns: Encoded X509 object. + :rtype: str + + """ if isinstance(self._wrapped, OpenSSL.crypto.X509): func = OpenSSL.crypto.dump_certificate else: # assert in __init__ makes sure this is X509Req @@ -54,10 +63,10 @@ class ComparableX509(object): # pylint: disable=too-few-public-methods def __eq__(self, other): if not isinstance(other, self.__class__): return NotImplemented - return self._dump() == other._dump() # pylint: disable=protected-access + return self.dump() == other.dump() def __hash__(self): - return hash((self.__class__, self._dump())) + return hash((self.__class__, self.dump())) def __ne__(self, other): return not self == other From d21ca90560ef590526971eb3c753eb96a34cc041 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 16 Dec 2015 17:33:08 -0800 Subject: [PATCH 002/100] Use dump on ComparableX509 --- acme/acme/challenges_test.py | 3 +-- acme/acme/jose/json_util.py | 6 ++---- acme/acme/jose/jws.py | 3 +-- acme/acme/jose/jws_test.py | 8 ++------ letsencrypt/cli.py | 6 +++--- letsencrypt/client.py | 6 ++---- letsencrypt/renewer.py | 3 +-- letsencrypt/tests/cli_test.py | 4 ++-- 8 files changed, 14 insertions(+), 25 deletions(-) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index a4e78ebe9..c01511171 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -420,8 +420,7 @@ class ProofOfPossessionHintsTest(unittest.TestCase): self.jmsg_to = { 'jwk': jwk, 'certFingerprints': cert_fingerprints, - 'certs': (jose.encode_b64jose(OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_ASN1, CERT)),), + 'certs': (jose.encode_b64jose(CERT.dump()),), 'subjectKeyIdentifiers': subject_key_identifiers, 'serialNumbers': serial_numbers, 'issuers': issuers, diff --git a/acme/acme/jose/json_util.py b/acme/acme/jose/json_util.py index 7b95e3fce..66776172b 100644 --- a/acme/acme/jose/json_util.py +++ b/acme/acme/jose/json_util.py @@ -372,8 +372,7 @@ def encode_cert(cert): :rtype: unicode """ - return encode_b64jose(OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_ASN1, cert)) + return encode_b64jose(cert.dump()) def decode_cert(b64der): @@ -397,8 +396,7 @@ def encode_csr(csr): :rtype: unicode """ - return encode_b64jose(OpenSSL.crypto.dump_certificate_request( - OpenSSL.crypto.FILETYPE_ASN1, csr)) + return encode_b64jose(csr.dump()) def decode_csr(b64der): diff --git a/acme/acme/jose/jws.py b/acme/acme/jose/jws.py index 1a073e17d..939932d36 100644 --- a/acme/acme/jose/jws.py +++ b/acme/acme/jose/jws.py @@ -123,8 +123,7 @@ class Header(json_util.JSONObjectWithFields): @x5c.encoder def x5c(value): # pylint: disable=missing-docstring,no-self-argument - return [base64.b64encode(OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_ASN1, cert)) for cert in value] + return [base64.b64encode(cert.dump()) for cert in value] @x5c.decoder def x5c(value): # pylint: disable=missing-docstring,no-self-argument diff --git a/acme/acme/jose/jws_test.py b/acme/acme/jose/jws_test.py index 69341f228..065243774 100644 --- a/acme/acme/jose/jws_test.py +++ b/acme/acme/jose/jws_test.py @@ -3,7 +3,6 @@ import base64 import unittest import mock -import OpenSSL from acme import test_util @@ -68,13 +67,10 @@ class HeaderTest(unittest.TestCase): from acme.jose.jws import Header header = Header(x5c=(CERT, CERT)) jobj = header.to_partial_json() - cert_b64 = base64.b64encode(OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_ASN1, CERT)) + cert_b64 = base64.b64encode(CERT.dump()) self.assertEqual(jobj, {'x5c': [cert_b64, cert_b64]}) self.assertEqual(header, Header.from_json(jobj)) - jobj['x5c'][0] = base64.b64encode( - b'xxx' + OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_ASN1, CERT)) + jobj['x5c'][0] = base64.b64encode(b'xxx' + CERT.dump()) self.assertRaises(errors.DeserializationError, Header.from_json, jobj) def test_find_key(self): diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 29519d430..1a141f556 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -391,9 +391,9 @@ def _auth_from_domains(le_client, config, domains): new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) lineage.save_successor( - lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_certr.body), - new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) + lineage.latest_common_version(), + new_certr.body.dump(OpenSSL.crypto.FILETYPE_PEM), new_key.pem, + crypto_util.dump_pyopenssl_chain(new_chain)) lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 080ee7991..59ac11a72 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -299,8 +299,7 @@ class Client(object): "by your operating system package manager") lineage = storage.RenewableCert.new_lineage( - domains[0], OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, certr.body), + domains[0], certr.body.dump(OpenSSL.crypto.FILETYPE_PEM), key.pem, crypto_util.dump_pyopenssl_chain(chain), params, config, cli_config) return lineage @@ -329,8 +328,7 @@ class Client(object): os.path.dirname(path), 0o755, os.geteuid(), self.config.strict_permissions) - cert_pem = OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, certr.body) + cert_pem = certr.body.dump(OpenSSL.crypto.FILETYPE_PEM) cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) try: cert_file.write(cert_pem) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 3996cfe67..6e2366d82 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -102,8 +102,7 @@ def renew(cert, old_version): # new_key if the old key is to be used (since save_successor # already understands this distinction!) return cert.save_successor( - old_version, OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_certr.body), + old_version, new_certr.body.dump(OpenSSL.crypto.FILETYPE_PEM), new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) # TODO: Notify results else: diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ccf16f5b5..39c09dede 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -417,11 +417,11 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path) - mock_cert = mock.MagicMock(body='body') + mock_certr = mock.MagicMock() mock_key = mock.MagicMock(pem='pem_key') mock_renewal.return_value = ("renew", mock_lineage) mock_client = mock.MagicMock() - mock_client.obtain_certificate.return_value = (mock_cert, 'chain', + mock_client.obtain_certificate.return_value = (mock_certr, 'chain', mock_key, 'csr') mock_init.return_value = mock_client with mock.patch('letsencrypt.cli.OpenSSL'): From 20b3188c6527588f1f6ab552104bf3af39978e2d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 16 Dec 2015 19:46:56 -0800 Subject: [PATCH 003/100] No kwargs plz --- letsencrypt/plugins/standalone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/standalone.py b/letsencrypt/plugins/standalone.py index 4319e51f9..cde7041d8 100644 --- a/letsencrypt/plugins/standalone.py +++ b/letsencrypt/plugins/standalone.py @@ -150,7 +150,7 @@ class Authenticator(common.Plugin): # one self-signed key for all tls-sni-01 certificates self.key = OpenSSL.crypto.PKey() - self.key.generate_key(OpenSSL.crypto.TYPE_RSA, bits=2048) + self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048) self.served = collections.defaultdict(set) From 7efdac6c66720715e6aeb595aa5d6149967c89f2 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 17 Dec 2015 17:28:36 -0800 Subject: [PATCH 004/100] Fixed SANs problem --- acme/acme/challenges_test.py | 4 +++- acme/acme/crypto_util.py | 30 ++++++++++-------------------- acme/acme/crypto_util_test.py | 16 ++++++++++------ acme/acme/jose/util_test.py | 3 +++ 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index c01511171..5c2b842ec 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -264,7 +264,9 @@ class TLSSNI01ResponseTest(unittest.TestCase): def test_verify_bad_cert(self): self.assertFalse(self.response.verify_cert( - test_util.load_cert('cert.pem'))) + OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, + test_util.load_vector('cert.pem')))) def test_simple_verify_bad_key_authorization(self): key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem')) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 72a93141a..cde8b2a9a 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -4,11 +4,10 @@ import logging import socket import sys -from six.moves import range # pylint: disable=import-error,redefined-builtin - import OpenSSL from acme import errors +from acme import jose logger = logging.getLogger(__name__) @@ -161,31 +160,22 @@ def _pyopenssl_cert_or_req_san(cert_or_req): :rtype: `list` of `unicode` """ - # constants based on implementation of - # OpenSSL.crypto.X509Error._subjectAltNameString + # constants based on PyOpenSSL certificate/CSR text dump + label = "DNS" parts_separator = ", " part_separator = ":" - 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 range(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 + title = "X509v3 Subject Alternative Name:" - san_extensions = [ - ext._subjectAltNameString().split(parts_separator) - for ext in extensions if ext.get_short_name() == extension_short_name] + text = jose.ComparableX509(cert_or_req).dump(OpenSSL.crypto.FILETYPE_TEXT) + lines = iter(text.decode("utf-8").splitlines()) + sans = [next(lines).split(parts_separator) + for line in lines if title in line] # 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)] + return [part.split(part_separator)[1] for parts in sans + for part in parts if part.lstrip().startswith(prefix)] def gen_ss_cert(key, domains, not_before=None, diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index bfd16388c..9e3062774 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -6,6 +6,8 @@ import unittest from six.moves import socketserver # pylint: disable=import-error +import OpenSSL + from acme import errors from acme import jose from acme import test_util @@ -64,16 +66,18 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): """Test for acme.crypto_util._pyopenssl_cert_or_req_san.""" @classmethod - def _call(cls, loader, name): + def _call(cls, cert_or_req): # pylint: disable=protected-access from acme.crypto_util import _pyopenssl_cert_or_req_san - return _pyopenssl_cert_or_req_san(loader(name)) + return _pyopenssl_cert_or_req_san(cert_or_req) - def _call_cert(self, name): - return self._call(test_util.load_cert, name) + def _call_cert(self, name, filetype=OpenSSL.crypto.FILETYPE_PEM): + return self._call(OpenSSL.crypto.load_certificate( + filetype, test_util.load_vector(name))) - def _call_csr(self, name): - return self._call(test_util.load_csr, name) + def _call_csr(self, name, filetype=OpenSSL.crypto.FILETYPE_PEM): + return self._call(OpenSSL.crypto.load_certificate_request( + filetype, test_util.load_vector(name))) def test_cert_no_sans(self): self.assertEqual(self._call_cert('cert.pem'), []) diff --git a/acme/acme/jose/util_test.py b/acme/acme/jose/util_test.py index 4cdd9127f..5920ce11f 100644 --- a/acme/acme/jose/util_test.py +++ b/acme/acme/jose/util_test.py @@ -20,6 +20,9 @@ class ComparableX509Test(unittest.TestCase): self.cert2 = test_util.load_cert('cert.pem') self.cert_other = test_util.load_cert('cert-san.pem') + def test_getattr_proxy(self): + self.assertTrue(self.cert1.has_expired()) + def test_eq(self): self.assertEqual(self.req1, self.req2) self.assertEqual(self.cert1, self.cert2) From a28f8fe4427c12c2523b16903325d0362b53123e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 17 Dec 2015 17:47:15 -0800 Subject: [PATCH 005/100] Drop version dependency --- acme/setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index e35b40d6e..5fc1bd8d0 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -12,8 +12,8 @@ install_requires = [ 'cryptography>=0.8', 'ndg-httpsclient', # urllib3 InsecurePlatformWarning (#304) 'pyasn1', # urllib3 InsecurePlatformWarning (#304) - # Connection.set_tlsext_host_name (>=0.13), X509Req.get_extensions (>=0.15) - 'PyOpenSSL>=0.15', + # Connection.set_tlsext_host_name (>=0.13) + 'PyOpenSSL>=0.13', 'pyrfc3339', 'pytz', 'requests', From 66a861ead1cc9188bc93133558567eaf41aa72b9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 23 Dec 2015 13:48:52 -0500 Subject: [PATCH 006/100] Add test_comparable_{cert,csr} --- acme/acme/challenges_test.py | 2 +- acme/acme/crypto_util_test.py | 2 +- acme/acme/jose/json_util_test.py | 4 ++-- acme/acme/jose/jws_test.py | 2 +- acme/acme/jose/util_test.py | 14 +++++++------- acme/acme/messages_test.py | 4 ++-- acme/acme/standalone_test.py | 4 ++-- acme/acme/test_util.py | 16 ++++++++++++---- letsencrypt/tests/client_test.py | 2 +- letsencrypt/tests/renewer_test.py | 7 +++++-- letsencrypt/tests/test_util.py | 16 ++++++++++++---- 11 files changed, 46 insertions(+), 27 deletions(-) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 5c2b842ec..0b02102b2 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -13,7 +13,7 @@ from acme import other from acme import test_util -CERT = test_util.load_cert('cert.pem') +CERT = test_util.load_comparable_cert('cert.pem') KEY = jose.JWKRSA(key=test_util.load_rsa_private_key('rsa512_key.pem')) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 9e3062774..72530ec9d 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -17,7 +17,7 @@ class SSLSocketAndProbeSNITest(unittest.TestCase): """Tests for acme.crypto_util.SSLSocket/probe_sni.""" def setUp(self): - self.cert = test_util.load_cert('cert.pem') + self.cert = test_util.load_comparable_cert('cert.pem') key = test_util.load_pyopenssl_private_key('rsa512_key.pem') # pylint: disable=protected-access certs = {b'foo': (key, self.cert._wrapped)} diff --git a/acme/acme/jose/json_util_test.py b/acme/acme/jose/json_util_test.py index a055f3bf7..25e36211e 100644 --- a/acme/acme/jose/json_util_test.py +++ b/acme/acme/jose/json_util_test.py @@ -12,8 +12,8 @@ from acme.jose import interfaces from acme.jose import util -CERT = test_util.load_cert('cert.pem') -CSR = test_util.load_csr('csr.pem') +CERT = test_util.load_comparable_cert('cert.pem') +CSR = test_util.load_comparable_csr('csr.pem') class FieldTest(unittest.TestCase): diff --git a/acme/acme/jose/jws_test.py b/acme/acme/jose/jws_test.py index 065243774..3fd0fbb89 100644 --- a/acme/acme/jose/jws_test.py +++ b/acme/acme/jose/jws_test.py @@ -12,7 +12,7 @@ from acme.jose import jwa from acme.jose import jwk -CERT = test_util.load_cert('cert.pem') +CERT = test_util.load_comparable_cert('cert.pem') KEY = jwk.JWKRSA.load(test_util.load_vector('rsa512_key.pem')) diff --git a/acme/acme/jose/util_test.py b/acme/acme/jose/util_test.py index 5920ce11f..392f81777 100644 --- a/acme/acme/jose/util_test.py +++ b/acme/acme/jose/util_test.py @@ -11,14 +11,14 @@ class ComparableX509Test(unittest.TestCase): """Tests for acme.jose.util.ComparableX509.""" def setUp(self): - # test_util.load_{csr,cert} return ComparableX509 - self.req1 = test_util.load_csr('csr.pem') - self.req2 = test_util.load_csr('csr.pem') - self.req_other = test_util.load_csr('csr-san.pem') + # test_util.load_comparable_{csr,cert} return ComparableX509 + self.req1 = test_util.load_comparable_csr('csr.pem') + self.req2 = test_util.load_comparable_csr('csr.pem') + self.req_other = test_util.load_comparable_csr('csr-san.pem') - self.cert1 = test_util.load_cert('cert.pem') - self.cert2 = test_util.load_cert('cert.pem') - self.cert_other = test_util.load_cert('cert-san.pem') + self.cert1 = test_util.load_comparable_cert('cert.pem') + self.cert2 = test_util.load_comparable_cert('cert.pem') + self.cert_other = test_util.load_comparable_cert('cert-san.pem') def test_getattr_proxy(self): self.assertTrue(self.cert1.has_expired()) diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py index 5a7a71299..8e74826bf 100644 --- a/acme/acme/messages_test.py +++ b/acme/acme/messages_test.py @@ -8,8 +8,8 @@ from acme import jose from acme import test_util -CERT = test_util.load_cert('cert.der') -CSR = test_util.load_csr('csr.der') +CERT = test_util.load_comparable_cert('cert.der') +CSR = test_util.load_comparable_csr('csr.der') KEY = test_util.load_rsa_private_key('rsa512_key.pem') diff --git a/acme/acme/standalone_test.py b/acme/acme/standalone_test.py index 02b1f69d3..2778635f5 100644 --- a/acme/acme/standalone_test.py +++ b/acme/acme/standalone_test.py @@ -35,7 +35,7 @@ class TLSSNI01ServerTest(unittest.TestCase): self.certs = { b'localhost': (test_util.load_pyopenssl_private_key('rsa512_key.pem'), # pylint: disable=protected-access - test_util.load_cert('cert.pem')._wrapped), + test_util.load_cert('cert.pem')), } from acme.standalone import TLSSNI01Server self.server = TLSSNI01Server(("", 0), certs=self.certs) @@ -146,7 +146,7 @@ class TestSimpleTLSSNI01Server(unittest.TestCase): time.sleep(1) # wait until thread starts else: self.assertEqual(jose.ComparableX509(cert), - test_util.load_cert('cert.pem')) + test_util.load_comparable_cert('cert.pem')) break diff --git a/acme/acme/test_util.py b/acme/acme/test_util.py index 2b4c6e00c..24eceff5a 100644 --- a/acme/acme/test_util.py +++ b/acme/acme/test_util.py @@ -40,16 +40,24 @@ def load_cert(*names): """Load certificate.""" loader = _guess_loader( names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1) - return jose.ComparableX509(OpenSSL.crypto.load_certificate( - loader, load_vector(*names))) + return OpenSSL.crypto.load_certificate(loader, load_vector(*names)) + + +def load_comparable_cert(*names): + """Load ComparableX509 cert.""" + return jose.ComparableX509(load_cert(*names)) def load_csr(*names): """Load certificate request.""" loader = _guess_loader( names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1) - return jose.ComparableX509(OpenSSL.crypto.load_certificate_request( - loader, load_vector(*names))) + return OpenSSL.crypto.load_certificate_request(loader, load_vector(*names)) + + +def load_comparable_csr(*names): + """Load ComparableX509 certificate request.""" + return jose.ComparableX509(load_csr(*names)) def load_rsa_private_key(*names): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 6b76f70c9..cf1b3b39f 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -141,7 +141,7 @@ class ClientTest(unittest.TestCase): tmp_path = tempfile.mkdtemp() os.chmod(tmp_path, 0o755) # TODO: really?? - certr = mock.MagicMock(body=test_util.load_cert(certs[0])) + certr = mock.MagicMock(body=test_util.load_comparable_cert(certs[0])) chain_cert = [test_util.load_cert(certs[1]), test_util.load_cert(certs[2])] candidate_cert_path = os.path.join(tmp_path, "certs", "cert.pem") diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index daec9678f..269a9193d 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -9,6 +9,8 @@ import unittest import configobj import mock +from acme import jose + from letsencrypt import configuration from letsencrypt import errors from letsencrypt.storage import ALL_FOUR @@ -702,9 +704,10 @@ class RenewableCertTests(BaseRenewableCertTest): self.test_rc.configfile["renewalparams"]["authenticator"] = "apache" mock_client = mock.MagicMock() # pylint: disable=star-args + comparable_cert = jose.ComparableX509(CERT) mock_client.obtain_certificate.return_value = ( - mock.MagicMock(body=CERT), [CERT], mock.Mock(pem="key"), - mock.sentinel.csr) + mock.MagicMock(body=comparable_cert), [comparable_cert], + mock.Mock(pem="key"), mock.sentinel.csr) mock_c.return_value = mock_client self.assertEqual(2, renewer.renew(self.test_rc, 1)) # TODO: We could also make several assertions about calls that should diff --git a/letsencrypt/tests/test_util.py b/letsencrypt/tests/test_util.py index 2b4c6e00c..24eceff5a 100644 --- a/letsencrypt/tests/test_util.py +++ b/letsencrypt/tests/test_util.py @@ -40,16 +40,24 @@ def load_cert(*names): """Load certificate.""" loader = _guess_loader( names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1) - return jose.ComparableX509(OpenSSL.crypto.load_certificate( - loader, load_vector(*names))) + return OpenSSL.crypto.load_certificate(loader, load_vector(*names)) + + +def load_comparable_cert(*names): + """Load ComparableX509 cert.""" + return jose.ComparableX509(load_cert(*names)) def load_csr(*names): """Load certificate request.""" loader = _guess_loader( names[-1], OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1) - return jose.ComparableX509(OpenSSL.crypto.load_certificate_request( - loader, load_vector(*names))) + return OpenSSL.crypto.load_certificate_request(loader, load_vector(*names)) + + +def load_comparable_csr(*names): + """Load ComparableX509 certificate request.""" + return jose.ComparableX509(load_csr(*names)) def load_rsa_private_key(*names): From 980637a936d24199061f4e4b1182971812062ddf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 23 Dec 2015 17:12:33 -0500 Subject: [PATCH 007/100] Audit calls to test_util.load_cert --- acme/acme/challenges.py | 9 ++++++++- acme/acme/crypto_util_test.py | 16 ++++++---------- letsencrypt/tests/client_test.py | 4 ++-- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 1e456d325..a121b4639 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -391,7 +391,14 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse): return crypto_util.probe_sni(**kwargs) def verify_cert(self, cert): - """Verify tls-sni-01 challenge certificate.""" + """Verify tls-sni-01 challenge certificate. + + :param OpensSSL.crypto.X509 cert: Challenge certificate. + + :returns: Whether the certificate was successfully verified. + :rtype: bool + + """ # 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/crypto_util_test.py b/acme/acme/crypto_util_test.py index 72530ec9d..25b0d2f67 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -6,8 +6,6 @@ import unittest from six.moves import socketserver # pylint: disable=import-error -import OpenSSL - from acme import errors from acme import jose from acme import test_util @@ -66,18 +64,16 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): """Test for acme.crypto_util._pyopenssl_cert_or_req_san.""" @classmethod - def _call(cls, cert_or_req): + 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(cert_or_req) + return _pyopenssl_cert_or_req_san(loader(name)) - def _call_cert(self, name, filetype=OpenSSL.crypto.FILETYPE_PEM): - return self._call(OpenSSL.crypto.load_certificate( - filetype, test_util.load_vector(name))) + def _call_cert(self, name): + return self._call(test_util.load_cert, name) - def _call_csr(self, name, filetype=OpenSSL.crypto.FILETYPE_PEM): - return self._call(OpenSSL.crypto.load_certificate_request( - filetype, test_util.load_vector(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'), []) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index cf1b3b39f..2f117f80c 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -142,8 +142,8 @@ class ClientTest(unittest.TestCase): os.chmod(tmp_path, 0o755) # TODO: really?? certr = mock.MagicMock(body=test_util.load_comparable_cert(certs[0])) - chain_cert = [test_util.load_cert(certs[1]), - test_util.load_cert(certs[2])] + chain_cert = [test_util.load_comparable_cert(certs[1]), + test_util.load_comparable_cert(certs[2])] candidate_cert_path = os.path.join(tmp_path, "certs", "cert.pem") candidate_chain_path = os.path.join(tmp_path, "chains", "chain.pem") candidate_fullchain_path = os.path.join(tmp_path, "chains", "fullchain.pem") From 75b551762b461de933d83de231735380751c5044 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 23 Dec 2015 19:09:05 -0500 Subject: [PATCH 008/100] Expose wrapped, not dump --- acme/acme/challenges_test.py | 3 ++- acme/acme/crypto_util.py | 8 ++++++-- acme/acme/crypto_util_test.py | 2 +- acme/acme/jose/json_util.py | 6 ++++-- acme/acme/jose/jws.py | 3 ++- acme/acme/jose/jws_test.py | 7 +++++-- acme/acme/jose/util.py | 28 ++++++++++++++-------------- letsencrypt/cli.py | 6 +++--- letsencrypt/client.py | 6 ++++-- letsencrypt/crypto_util.py | 2 +- letsencrypt/renewer.py | 3 ++- 11 files changed, 44 insertions(+), 30 deletions(-) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 0b02102b2..6b277ac27 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -422,7 +422,8 @@ class ProofOfPossessionHintsTest(unittest.TestCase): self.jmsg_to = { 'jwk': jwk, 'certFingerprints': cert_fingerprints, - 'certs': (jose.encode_b64jose(CERT.dump()),), + 'certs': (jose.encode_b64jose(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, CERT.wrapped)),), 'subjectKeyIdentifiers': subject_key_identifiers, 'serialNumbers': serial_numbers, 'issuers': issuers, diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index cde8b2a9a..15890175f 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -7,7 +7,6 @@ import sys import OpenSSL from acme import errors -from acme import jose logger = logging.getLogger(__name__) @@ -167,7 +166,12 @@ def _pyopenssl_cert_or_req_san(cert_or_req): prefix = label + part_separator title = "X509v3 Subject Alternative Name:" - text = jose.ComparableX509(cert_or_req).dump(OpenSSL.crypto.FILETYPE_TEXT) + if isinstance(cert_or_req, OpenSSL.crypto.X509): + func = OpenSSL.crypto.dump_certificate + else: + func = OpenSSL.crypto.dump_certificate_request + text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req) + lines = iter(text.decode("utf-8").splitlines()) sans = [next(lines).split(parts_separator) for line in lines if title in line] diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 25b0d2f67..b3c39c388 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -18,7 +18,7 @@ class SSLSocketAndProbeSNITest(unittest.TestCase): self.cert = test_util.load_comparable_cert('cert.pem') key = test_util.load_pyopenssl_private_key('rsa512_key.pem') # pylint: disable=protected-access - certs = {b'foo': (key, self.cert._wrapped)} + certs = {b'foo': (key, self.cert.wrapped)} from acme.crypto_util import SSLSocket diff --git a/acme/acme/jose/json_util.py b/acme/acme/jose/json_util.py index 66776172b..42fb389e6 100644 --- a/acme/acme/jose/json_util.py +++ b/acme/acme/jose/json_util.py @@ -372,7 +372,8 @@ def encode_cert(cert): :rtype: unicode """ - return encode_b64jose(cert.dump()) + return encode_b64jose(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, cert.wrapped)) def decode_cert(b64der): @@ -396,7 +397,8 @@ def encode_csr(csr): :rtype: unicode """ - return encode_b64jose(csr.dump()) + return encode_b64jose(OpenSSL.crypto.dump_certificate_request( + OpenSSL.crypto.FILETYPE_ASN1, csr.wrapped)) def decode_csr(b64der): diff --git a/acme/acme/jose/jws.py b/acme/acme/jose/jws.py index 939932d36..9c14cf729 100644 --- a/acme/acme/jose/jws.py +++ b/acme/acme/jose/jws.py @@ -123,7 +123,8 @@ class Header(json_util.JSONObjectWithFields): @x5c.encoder def x5c(value): # pylint: disable=missing-docstring,no-self-argument - return [base64.b64encode(cert.dump()) for cert in value] + return [base64.b64encode(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, cert.wrapped)) for cert in value] @x5c.decoder def x5c(value): # pylint: disable=missing-docstring,no-self-argument diff --git a/acme/acme/jose/jws_test.py b/acme/acme/jose/jws_test.py index 3fd0fbb89..ec91f6a1b 100644 --- a/acme/acme/jose/jws_test.py +++ b/acme/acme/jose/jws_test.py @@ -3,6 +3,7 @@ import base64 import unittest import mock +import OpenSSL from acme import test_util @@ -67,10 +68,12 @@ class HeaderTest(unittest.TestCase): from acme.jose.jws import Header header = Header(x5c=(CERT, CERT)) jobj = header.to_partial_json() - cert_b64 = base64.b64encode(CERT.dump()) + cert_asn1 = OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, CERT.wrapped) + cert_b64 = base64.b64encode(cert_asn1) self.assertEqual(jobj, {'x5c': [cert_b64, cert_b64]}) self.assertEqual(header, Header.from_json(jobj)) - jobj['x5c'][0] = base64.b64encode(b'xxx' + CERT.dump()) + jobj['x5c'][0] = base64.b64encode(b'xxx' + cert_asn1) self.assertRaises(errors.DeserializationError, Header.from_json, jobj) def test_find_key(self): diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py index 1d98aad4e..a7a129800 100644 --- a/acme/acme/jose/util.py +++ b/acme/acme/jose/util.py @@ -29,50 +29,50 @@ class abstractclassmethod(classmethod): class ComparableX509(object): # pylint: disable=too-few-public-methods """Wrapper for OpenSSL.crypto.X509** objects that supports __eq__. - Wraps around: - - - :class:`OpenSSL.crypto.X509` - - :class:`OpenSSL.crypto.X509Req` + :ivar wrapped: Wrapped certificate or certificate request. + :type wrapped: `OpenSSL.crypto.X509` or `OpenSSL.crypto.X509Req`. """ def __init__(self, wrapped): assert isinstance(wrapped, OpenSSL.crypto.X509) or isinstance( wrapped, OpenSSL.crypto.X509Req) - self._wrapped = wrapped + self.wrapped = wrapped def __getattr__(self, name): - return getattr(self._wrapped, name) + return getattr(self.wrapped, name) - def dump(self, filetype=OpenSSL.crypto.FILETYPE_ASN1): + def _dump(self, filetype=OpenSSL.crypto.FILETYPE_ASN1): """Dumps the object into a buffer with the specified encoding. :param int filetype: The desired encoding. Should be one of - OpenSSL.crypto.FILETYPE_ASN1, OpenSSL.crypto.FILETYPE_PEM, - or OpenSSL.crypto.FILETYPE_TEXT. + `OpenSSL.crypto.FILETYPE_ASN1`, + `OpenSSL.crypto.FILETYPE_PEM`, or + `OpenSSL.crypto.FILETYPE_TEXT`. :returns: Encoded X509 object. :rtype: str """ - if isinstance(self._wrapped, OpenSSL.crypto.X509): + if isinstance(self.wrapped, OpenSSL.crypto.X509): func = OpenSSL.crypto.dump_certificate else: # assert in __init__ makes sure this is X509Req func = OpenSSL.crypto.dump_certificate_request - return func(filetype, self._wrapped) + return func(filetype, self.wrapped) def __eq__(self, other): if not isinstance(other, self.__class__): return NotImplemented - return self.dump() == other.dump() + # pylint: disable=protected-access + return self._dump() == other._dump() def __hash__(self): - return hash((self.__class__, self.dump())) + return hash((self.__class__, self._dump())) def __ne__(self, other): return not self == other def __repr__(self): - return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) + return '<{0}({1!r})>'.format(self.__class__.__name__, self.wrapped) class ComparableKey(object): # pylint: disable=too-few-public-methods diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1a141f556..dfebde13c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -391,9 +391,9 @@ def _auth_from_domains(le_client, config, domains): new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) lineage.save_successor( - lineage.latest_common_version(), - new_certr.body.dump(OpenSSL.crypto.FILETYPE_PEM), new_key.pem, - crypto_util.dump_pyopenssl_chain(new_chain)) + lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), + new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 59ac11a72..c2dfca1bf 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -299,7 +299,8 @@ class Client(object): "by your operating system package manager") lineage = storage.RenewableCert.new_lineage( - domains[0], certr.body.dump(OpenSSL.crypto.FILETYPE_PEM), + domains[0], OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped), key.pem, crypto_util.dump_pyopenssl_chain(chain), params, config, cli_config) return lineage @@ -328,7 +329,8 @@ class Client(object): os.path.dirname(path), 0o755, os.geteuid(), self.config.strict_permissions) - cert_pem = certr.body.dump(OpenSSL.crypto.FILETYPE_PEM) + cert_pem = OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) try: cert_file.write(cert_pem) diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index f897ec852..730c32398 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -271,7 +271,7 @@ def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM): def _dump_cert(cert): if isinstance(cert, jose.ComparableX509): # pylint: disable=protected-access - cert = cert._wrapped + cert = cert.wrapped return OpenSSL.crypto.dump_certificate(filetype, cert) # assumes that OpenSSL.crypto.dump_certificate includes ending diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 6e2366d82..ed4910ef9 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -102,7 +102,8 @@ def renew(cert, old_version): # new_key if the old key is to be used (since save_successor # already understands this distinction!) return cert.save_successor( - old_version, new_certr.body.dump(OpenSSL.crypto.FILETYPE_PEM), + old_version, OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain)) # TODO: Notify results else: From a718cfede0f44cbc0c40ae396911fa3f510c3c7e Mon Sep 17 00:00:00 2001 From: sagi Date: Sun, 3 Jan 2016 22:03:47 +0000 Subject: [PATCH 009/100] Copy only relevant lines from http vhost to ssl vhost skeleton --- .../letsencrypt_apache/configurator.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 836d77135..0d6749638 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -712,8 +712,19 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): with open(avail_fp, "r") as orig_file: with open(ssl_fp, "w") as new_file: new_file.write("\n") + + # In some cases we wouldn't want to copy the exact + # directives used in an http vhost to a ssl vhost. + # An example: + # If there's a redirect rewrite rule directive installed in + # the http vhost - copying it to the ssl vhost would cause + # a redirection loop. + blacklist_set = set(['RewriteRule', 'RewriteEngine']) + for line in orig_file: - new_file.write(line) + line_set = set(line.split()) + if not line_set & blacklist_set: # & -> Intersection + new_file.write(line) new_file.write("\n") except IOError: logger.fatal("Error writing/reading to file in make_vhost_ssl") From e1b4797cbfae87b59d1b3282992a6b605e578a7f Mon Sep 17 00:00:00 2001 From: Wilfried Teiken Date: Tue, 5 Jan 2016 01:12:21 -0500 Subject: [PATCH 010/100] Change the semantics of query_registration and update_registration to set new_authzr_uri from the server if available --- acme/acme/client.py | 12 +++++------- acme/acme/client_test.py | 7 +++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index c3e28ef47..3d84cb3bf 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -66,15 +66,13 @@ class Client(object): # pylint: disable=too-many-instance-attributes @classmethod def _regr_from_response(cls, response, uri=None, new_authzr_uri=None, terms_of_service=None): - terms_of_service = ( - response.links['terms-of-service']['url'] - if 'terms-of-service' in response.links else terms_of_service) + if 'terms-of-service' in response.links: + terms_of_service = response.links['terms-of-service']['url'] + if 'next' in response.links: + new_authzr_uri = response.links['next']['url'] if new_authzr_uri is None: - try: - new_authzr_uri = response.links['next']['url'] - except KeyError: - raise errors.ClientError('"next" link missing') + raise errors.ClientError(response, 'missing "new_authrz_uri"') return messages.RegistrationResource( body=messages.Registration.from_json(response.json()), diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 58f55b293..9e3ea3d88 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -127,6 +127,13 @@ class ClientTest(unittest.TestCase): self.response.json.return_value = self.regr.body.to_json() self.assertEqual(self.regr, self.client.query_registration(self.regr)) + def test_query_registration_updates_new_authzr_uri(self): + self.response.json.return_value = self.regr.body.to_json() + self.response.links = {'next': {'url': 'UPDATED'}} + self.assertEqual( + 'UPDATED', + self.client.query_registration(self.regr).new_authzr_uri) + def test_agree_to_tos(self): self.client.update_registration = mock.Mock() self.client.agree_to_tos(self.regr) From 10a49532ae459ee1b1cc45b3b72072d8bdd37310 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 5 Jan 2016 11:25:21 +0200 Subject: [PATCH 011/100] Add / replace functionality to augeas transform paths to overwrite old narrower scope if needed --- .../letsencrypt_apache/parser.py | 64 ++++++++++++++++--- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 593c807cc..04b61b2d4 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -35,6 +35,7 @@ class ApacheParser(object): # https://httpd.apache.org/docs/2.4/mod/core.html#define # https://httpd.apache.org/docs/2.4/mod/core.html#ifdefine # This only handles invocation parameters and Define directives! + self.parser_paths = {} self.variables = {} if version >= (2, 4): self.update_runtime_variables() @@ -471,16 +472,61 @@ class ApacheParser(object): :param str filepath: Apache config file path """ + use_new, remove_old = self._check_path_actions(filepath) # Test if augeas included file for Httpd.lens # Note: This works for augeas globs, ie. *.conf - inc_test = self.aug.match( - "/augeas/load/Httpd/incl [. ='%s']" % filepath) - if not inc_test: - # Load up files - # This doesn't seem to work on TravisCI - # self.aug.add_transform("Httpd.lns", [filepath]) - self._add_httpd_transform(filepath) - self.aug.load() + if use_new: + inc_test = self.aug.match( + "/augeas/load/Httpd/incl [. ='%s']" % filepath) + if not inc_test: + # Load up files + # This doesn't seem to work on TravisCI + # self.aug.add_transform("Httpd.lns", [filepath]) + if remove_old: + self._remove_httpd_transform(filepath) + self._add_httpd_transform(filepath) + self.aug.load() + + def _check_path_actions(self, filepath): + """Determine actions to take with a new augeas path + + This helper function will return a tuple that defines + if we should try to append the new filepath to augeas + parser paths, and / or remove the old one with more + narrow matching. + + :param str filepath: filepath to check the actions for + + """ + + try: + use_new = False + remove_old = False + new_file_match = os.path.basename(filepath) + existing_match = self.parser_paths[os.path.dirname(filepath)] + if existing_match == new_file_match: + # True here to let augeas verify that the path is parsed + use_new = True + elif new_file_match == "*": + use_new = True + remove_old = True + except KeyError: + use_new = True + return use_new, remove_old + + def _remove_httpd_transform(self, filepath): + """Remove path from Augeas transform + + :param str filepath: filepath to remove + """ + + remove_basename = self.parser_paths[os.path.dirname(filepath)] + remove_dirname = os.path.dirname(filepath) + remove_path = remove_dirname + "/" + remove_basename + remove_inc = self.aug.match( + "/augeas/load/Httpd/incl [. ='%s']" % remove_path) + self.aug.remove(remove_inc[0]) + self.parser_paths.pop(remove_dirname) def _add_httpd_transform(self, incl): """Add a transform to Augeas. @@ -502,6 +548,8 @@ class ApacheParser(object): # Augeas uses base 1 indexing... insert at beginning... self.aug.set("/augeas/load/Httpd/lens", "Httpd.lns") self.aug.set("/augeas/load/Httpd/incl", incl) + # Add included path to paths dictionary + self.parser_paths[os.path.dirname(incl)] = os.path.basename(incl) def standardize_excl(self): """Standardize the excl arguments for the Httpd lens in Augeas. From 63f311eea492433b5a12ac685907b5d49105c44f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 5 Jan 2016 13:16:49 +0200 Subject: [PATCH 012/100] Refactored the checking method to be easier to read --- letsencrypt-apache/letsencrypt_apache/parser.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 04b61b2d4..23ca05ae0 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -500,18 +500,19 @@ class ApacheParser(object): """ try: - use_new = False - remove_old = False new_file_match = os.path.basename(filepath) existing_match = self.parser_paths[os.path.dirname(filepath)] - if existing_match == new_file_match: - # True here to let augeas verify that the path is parsed - use_new = True - elif new_file_match == "*": + if existing_match == "*": + use_new = False + else: use_new = True + if new_file_match == "*": remove_old = True + else: + remove_old = False except KeyError: use_new = True + remove_old = False return use_new, remove_old def _remove_httpd_transform(self, filepath): From 32d5375b9bb101f7e9239627c2ccb17a728046da Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 5 Jan 2016 13:21:18 +0200 Subject: [PATCH 013/100] Change test to check out adding a file not already in the augeas paths --- letsencrypt-apache/letsencrypt_apache/tests/parser_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index b871f89b7..9b78bf6d6 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -36,7 +36,7 @@ class BasicParserTest(util.ParserTest): """ file_path = os.path.join( - self.config_path, "sites-available", "letsencrypt.conf") + self.config_path, "not-parsed-by-default", "letsencrypt.conf") self.parser._parse_file(file_path) # pylint: disable=protected-access From ffeef67e542dfb170d051d60b3e519fa689bbe03 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 5 Jan 2016 14:36:42 +0200 Subject: [PATCH 014/100] Use lists to handle multiple different matching wildcards in same directory --- .../letsencrypt_apache/parser.py | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 23ca05ae0..82effad2b 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -501,8 +501,8 @@ class ApacheParser(object): try: new_file_match = os.path.basename(filepath) - existing_match = self.parser_paths[os.path.dirname(filepath)] - if existing_match == "*": + existing_matches = self.parser_paths[os.path.dirname(filepath)] + if "*" in existing_matches: use_new = False else: use_new = True @@ -521,12 +521,13 @@ class ApacheParser(object): :param str filepath: filepath to remove """ - remove_basename = self.parser_paths[os.path.dirname(filepath)] + remove_basenames = self.parser_paths[os.path.dirname(filepath)] remove_dirname = os.path.dirname(filepath) - remove_path = remove_dirname + "/" + remove_basename - remove_inc = self.aug.match( - "/augeas/load/Httpd/incl [. ='%s']" % remove_path) - self.aug.remove(remove_inc[0]) + for name in remove_basenames: + remove_path = remove_dirname + "/" + name + remove_inc = self.aug.match( + "/augeas/load/Httpd/incl [. ='%s']" % remove_path) + self.aug.remove(remove_inc[0]) self.parser_paths.pop(remove_dirname) def _add_httpd_transform(self, incl): @@ -550,7 +551,12 @@ class ApacheParser(object): self.aug.set("/augeas/load/Httpd/lens", "Httpd.lns") self.aug.set("/augeas/load/Httpd/incl", incl) # Add included path to paths dictionary - self.parser_paths[os.path.dirname(incl)] = os.path.basename(incl) + try: + self.parser_paths[os.path.dirname(incl)].append( + os.path.basename(incl)) + except KeyError: + self.parser_paths[os.path.dirname(incl)] = [ + os.path.basename(incl)] def standardize_excl(self): """Standardize the excl arguments for the Httpd lens in Augeas. From 74237d101041897231881a5ba55142e7fb255a4d Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 12:00:20 -0500 Subject: [PATCH 015/100] Requires chain_path for nginx versions supporting OCSP stapling --chain-path config is not mandatory, so we require this property if nginx supports OCSP stapling. Alternatively, we could disable OCSP stapling on supported nginx versions if --chain-path is missing. --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 11 ++++++++++- .../letsencrypt_nginx/tests/configurator_test.py | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index aaaf43c5f..99a864141 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -122,7 +122,7 @@ class NginxConfigurator(common.Plugin): # Entry point in main.py for installing cert def deploy_cert(self, domain, cert_path, key_path, - chain_path, fullchain_path): + chain_path=None, fullchain_path=None): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. @@ -136,6 +136,9 @@ class NginxConfigurator(common.Plugin): .. note:: This doesn't save the config files! + :raises errors.PluginError: When unable to deploy certificate due to + a lack of directives or configuration + """ vhost = self.choose_vhost(domain) cert_directives = [['ssl_certificate', fullchain_path], @@ -150,6 +153,12 @@ class NginxConfigurator(common.Plugin): ['ssl_stapling', 'on'], ['ssl_stapling_verify', 'on']] + if len(stapling_directives) != 0 and not chain_path: + raise errors.PluginError( + "--chain-path is required to enable " + "Online Certificate Status Protocol (OCSP) stapling " + "on nginx >= 1.3.7.") + try: self.parser.add_server_directives(vhost.filep, vhost.names, cert_directives, replace=True) diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 56ad5110c..35a55befd 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -125,6 +125,15 @@ class NginxConfiguratorTest(util.NginxTest): self.assertTrue(util.contains_at_depth(generated_conf, ['ssl_trusted_certificate', 'example/chain.pem'], 2)) + def test_deploy_cert_stapling_requires_chain_path(self): + self.config.version = (1, 3, 7) + self.assertRaises(errors.PluginError, self.config.deploy_cert, + "www.example.com", + "example/cert.pem", + "example/key.pem", + None, + "example/fullchain.pem") + def test_deploy_cert(self): server_conf = self.config.parser.abs_path('server.conf') nginx_conf = self.config.parser.abs_path('nginx.conf') From e8fc2eca0139df1d8cde1e23c12eaca184371100 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 15:02:15 -0500 Subject: [PATCH 016/100] nginx plugin requires fullchain_path --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 4 ++++ .../letsencrypt_nginx/tests/configurator_test.py | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 99a864141..59a977f09 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -140,6 +140,10 @@ class NginxConfigurator(common.Plugin): a lack of directives or configuration """ + if not fullchain_path: + raise errors.PluginError( + "--fullchain-path is required for nginx plugin.") + vhost = self.choose_vhost(domain) cert_directives = [['ssl_certificate', fullchain_path], ['ssl_certificate_key', key_path]] diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py index 35a55befd..bab107582 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/configurator_test.py @@ -134,6 +134,15 @@ class NginxConfiguratorTest(util.NginxTest): None, "example/fullchain.pem") + def test_deploy_cert_requires_fullchain_path(self): + self.config.version = (1, 3, 1) + self.assertRaises(errors.PluginError, self.config.deploy_cert, + "www.example.com", + "example/cert.pem", + "example/key.pem", + "example/chain.pem", + None) + def test_deploy_cert(self): server_conf = self.config.parser.abs_path('server.conf') nginx_conf = self.config.parser.abs_path('nginx.conf') From 5b5051b6ced180c10311cea89b37900d147929c1 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Mon, 4 Jan 2016 15:16:30 -0500 Subject: [PATCH 017/100] The notes should display the fullchain_path See d01b17f1 and dd8c6d65 --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 59a977f09..963ae8a42 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -181,7 +181,7 @@ class NginxConfigurator(common.Plugin): self.save_notes += ("Changed vhost at %s with addresses of %s\n" % (vhost.filep, ", ".join(str(addr) for addr in vhost.addrs))) - self.save_notes += "\tssl_certificate %s\n" % cert_path + self.save_notes += "\tssl_certificate %s\n" % fullchain_path self.save_notes += "\tssl_certificate_key %s\n" % key_path ####################### From 0b9f505ed5beb7a861c9cb027b5e4349a67eb60d Mon Sep 17 00:00:00 2001 From: Fan Jiang Date: Mon, 4 Jan 2016 15:29:26 -0500 Subject: [PATCH 018/100] update document for --chain-path when required by Nginx >= 1.3.7 --- docs/using.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/using.rst b/docs/using.rst index 5da13f02c..bd40dec02 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -237,7 +237,9 @@ The following files are available: server certificate, i.e. root and intermediate certificates only. This is what Apache < 2.4.8 needs for `SSLCertificateChainFile - `_. + `_, + and what nginx >= 1.3.7 needs for `ssl_trusted_certificate + `_. ``fullchain.pem`` All certificates, **including** server certificate. This is From 7bd7e7ca23420439215e9fe07c9b9ec65f773c54 Mon Sep 17 00:00:00 2001 From: wteiken Date: Tue, 5 Jan 2016 19:51:45 -0500 Subject: [PATCH 019/100] Remove response argument from exception and fix eror messages. --- acme/acme/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 3d84cb3bf..f3c2c6053 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -72,7 +72,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes new_authzr_uri = response.links['next']['url'] if new_authzr_uri is None: - raise errors.ClientError(response, 'missing "new_authrz_uri"') + raise errors.ClientError('"next" link missing') return messages.RegistrationResource( body=messages.Registration.from_json(response.json()), From 6719d0d3804987914e6b53b6227d3ec0c7d4a010 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 12:40:44 -0500 Subject: [PATCH 020/100] Rewrote _pyopenssl_cert_or_req_san --- acme/acme/crypto_util.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 15890175f..ecec351c2 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -1,6 +1,7 @@ """Crypto utilities.""" import contextlib import logging +import re import socket import sys @@ -160,26 +161,22 @@ def _pyopenssl_cert_or_req_san(cert_or_req): """ # constants based on PyOpenSSL certificate/CSR text dump - label = "DNS" - parts_separator = ", " part_separator = ":" - prefix = label + part_separator - title = "X509v3 Subject Alternative Name:" + parts_separator = ", " + prefix = "DNS" + part_separator if isinstance(cert_or_req, OpenSSL.crypto.X509): func = OpenSSL.crypto.dump_certificate else: func = OpenSSL.crypto.dump_certificate_request - text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req) - - lines = iter(text.decode("utf-8").splitlines()) - sans = [next(lines).split(parts_separator) - for line in lines if title in line] + text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8") + match = re.search(r"X509v3 Subject Alternative Name:\s*(.*)", text) + sans_parts = [] if match is None else match.group(1).split(parts_separator) # WARNING: this function assumes that no SAN can include # parts_separator, hence the split! - return [part.split(part_separator)[1] for parts in sans - for part in parts if part.lstrip().startswith(prefix)] + return [part.split(part_separator)[1] + for part in sans_parts if part.startswith(prefix)] def gen_ss_cert(key, domains, not_before=None, From 858dadd85b2b387f8e5c0485e370efa57bb7fe18 Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Wed, 6 Jan 2016 13:36:52 -0500 Subject: [PATCH 021/100] Update error message This is supposed to not happen once #1391 is fixed. --- letsencrypt-nginx/letsencrypt_nginx/configurator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/configurator.py b/letsencrypt-nginx/letsencrypt_nginx/configurator.py index 963ae8a42..43ad36c7f 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/configurator.py +++ b/letsencrypt-nginx/letsencrypt_nginx/configurator.py @@ -142,7 +142,8 @@ class NginxConfigurator(common.Plugin): """ if not fullchain_path: raise errors.PluginError( - "--fullchain-path is required for nginx plugin.") + "The nginx plugin currently requires --fullchain-path to " + "install a cert.") vhost = self.choose_vhost(domain) cert_directives = [['ssl_certificate', fullchain_path], From 1af997158dbc6a3f2233e3c0a6ea7e7b9bc3fc7a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 13:39:14 -0500 Subject: [PATCH 022/100] Fix repr differences between PyOpenSSL versions --- acme/acme/jose/util_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/acme/jose/util_test.py b/acme/acme/jose/util_test.py index 392f81777..0038a6cc1 100644 --- a/acme/acme/jose/util_test.py +++ b/acme/acme/jose/util_test.py @@ -44,8 +44,8 @@ class ComparableX509Test(unittest.TestCase): def test_repr(self): for x509 in self.req1, self.cert1: - self.assertTrue(repr(x509).startswith( - ''.format(x509.wrapped)) class ComparableRSAKeyTest(unittest.TestCase): From 32650a6d08803d6f07b4880a7a037e25bba1de96 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 14:10:57 -0500 Subject: [PATCH 023/100] Added 100 SANs cert and csr --- acme/acme/testdata/cert-100sans.pem | 44 +++++++++++++++++++++++++++++ acme/acme/testdata/csr-100sans.pem | 41 +++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 acme/acme/testdata/cert-100sans.pem create mode 100644 acme/acme/testdata/csr-100sans.pem diff --git a/acme/acme/testdata/cert-100sans.pem b/acme/acme/testdata/cert-100sans.pem new file mode 100644 index 000000000..3fdc9404f --- /dev/null +++ b/acme/acme/testdata/cert-100sans.pem @@ -0,0 +1,44 @@ +-----BEGIN CERTIFICATE----- +MIIHxDCCB26gAwIBAgIJAOGrG1Un9lHiMA0GCSqGSIb3DQEBCwUAMGQxCzAJBgNV +BAgMAkNBMRYwFAYDVQQHDA1TYW4gRnJhbmNpc2NvMScwJQYDVQQLDB5FbGVjdHJv +bmljIEZyb250aWVyIEZvdW5kYXRpb24xFDASBgNVBAMMC2V4YW1wbGUuY29tMB4X +DTE2MDEwNjE5MDkzN1oXDTE2MDEwNzE5MDkzN1owZDELMAkGA1UECAwCQ0ExFjAU +BgNVBAcMDVNhbiBGcmFuY2lzY28xJzAlBgNVBAsMHkVsZWN0cm9uaWMgRnJvbnRp +ZXIgRm91bmRhdGlvbjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wXDANBgkqhkiG9w0B +AQEFAANLADBIAkEArHVztFHtH92ucFJD/N/HW9AsdRsUuHUBBBDlHwNlRd3fp580 +rv2+6QWE30cWgdmJS86ObRz6lUTor4R0T+3C5QIDAQABo4IGATCCBf0wCQYDVR0T +BAIwADALBgNVHQ8EBAMCBeAwggXhBgNVHREEggXYMIIF1IIMZXhhbXBsZTEuY29t +ggxleGFtcGxlMi5jb22CDGV4YW1wbGUzLmNvbYIMZXhhbXBsZTQuY29tggxleGFt +cGxlNS5jb22CDGV4YW1wbGU2LmNvbYIMZXhhbXBsZTcuY29tggxleGFtcGxlOC5j +b22CDGV4YW1wbGU5LmNvbYINZXhhbXBsZTEwLmNvbYINZXhhbXBsZTExLmNvbYIN +ZXhhbXBsZTEyLmNvbYINZXhhbXBsZTEzLmNvbYINZXhhbXBsZTE0LmNvbYINZXhh +bXBsZTE1LmNvbYINZXhhbXBsZTE2LmNvbYINZXhhbXBsZTE3LmNvbYINZXhhbXBs +ZTE4LmNvbYINZXhhbXBsZTE5LmNvbYINZXhhbXBsZTIwLmNvbYINZXhhbXBsZTIx +LmNvbYINZXhhbXBsZTIyLmNvbYINZXhhbXBsZTIzLmNvbYINZXhhbXBsZTI0LmNv +bYINZXhhbXBsZTI1LmNvbYINZXhhbXBsZTI2LmNvbYINZXhhbXBsZTI3LmNvbYIN +ZXhhbXBsZTI4LmNvbYINZXhhbXBsZTI5LmNvbYINZXhhbXBsZTMwLmNvbYINZXhh +bXBsZTMxLmNvbYINZXhhbXBsZTMyLmNvbYINZXhhbXBsZTMzLmNvbYINZXhhbXBs +ZTM0LmNvbYINZXhhbXBsZTM1LmNvbYINZXhhbXBsZTM2LmNvbYINZXhhbXBsZTM3 +LmNvbYINZXhhbXBsZTM4LmNvbYINZXhhbXBsZTM5LmNvbYINZXhhbXBsZTQwLmNv +bYINZXhhbXBsZTQxLmNvbYINZXhhbXBsZTQyLmNvbYINZXhhbXBsZTQzLmNvbYIN +ZXhhbXBsZTQ0LmNvbYINZXhhbXBsZTQ1LmNvbYINZXhhbXBsZTQ2LmNvbYINZXhh +bXBsZTQ3LmNvbYINZXhhbXBsZTQ4LmNvbYINZXhhbXBsZTQ5LmNvbYINZXhhbXBs +ZTUwLmNvbYINZXhhbXBsZTUxLmNvbYINZXhhbXBsZTUyLmNvbYINZXhhbXBsZTUz +LmNvbYINZXhhbXBsZTU0LmNvbYINZXhhbXBsZTU1LmNvbYINZXhhbXBsZTU2LmNv +bYINZXhhbXBsZTU3LmNvbYINZXhhbXBsZTU4LmNvbYINZXhhbXBsZTU5LmNvbYIN +ZXhhbXBsZTYwLmNvbYINZXhhbXBsZTYxLmNvbYINZXhhbXBsZTYyLmNvbYINZXhh +bXBsZTYzLmNvbYINZXhhbXBsZTY0LmNvbYINZXhhbXBsZTY1LmNvbYINZXhhbXBs +ZTY2LmNvbYINZXhhbXBsZTY3LmNvbYINZXhhbXBsZTY4LmNvbYINZXhhbXBsZTY5 +LmNvbYINZXhhbXBsZTcwLmNvbYINZXhhbXBsZTcxLmNvbYINZXhhbXBsZTcyLmNv +bYINZXhhbXBsZTczLmNvbYINZXhhbXBsZTc0LmNvbYINZXhhbXBsZTc1LmNvbYIN +ZXhhbXBsZTc2LmNvbYINZXhhbXBsZTc3LmNvbYINZXhhbXBsZTc4LmNvbYINZXhh +bXBsZTc5LmNvbYINZXhhbXBsZTgwLmNvbYINZXhhbXBsZTgxLmNvbYINZXhhbXBs +ZTgyLmNvbYINZXhhbXBsZTgzLmNvbYINZXhhbXBsZTg0LmNvbYINZXhhbXBsZTg1 +LmNvbYINZXhhbXBsZTg2LmNvbYINZXhhbXBsZTg3LmNvbYINZXhhbXBsZTg4LmNv +bYINZXhhbXBsZTg5LmNvbYINZXhhbXBsZTkwLmNvbYINZXhhbXBsZTkxLmNvbYIN +ZXhhbXBsZTkyLmNvbYINZXhhbXBsZTkzLmNvbYINZXhhbXBsZTk0LmNvbYINZXhh +bXBsZTk1LmNvbYINZXhhbXBsZTk2LmNvbYINZXhhbXBsZTk3LmNvbYINZXhhbXBs +ZTk4LmNvbYINZXhhbXBsZTk5LmNvbYIOZXhhbXBsZTEwMC5jb20wDQYJKoZIhvcN +AQELBQADQQBEunJbKUXcyNKTSfA0pKRyWNiKmkoBqYgfZS6eHNrNH/hjFzHtzyDQ +XYHHK6kgEWBvHfRXGmqhFvht+b1tQKkG +-----END CERTIFICATE----- diff --git a/acme/acme/testdata/csr-100sans.pem b/acme/acme/testdata/csr-100sans.pem new file mode 100644 index 000000000..199814126 --- /dev/null +++ b/acme/acme/testdata/csr-100sans.pem @@ -0,0 +1,41 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIHNTCCBt8CAQAwZDELMAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lz +Y28xJzAlBgNVBAsMHkVsZWN0cm9uaWMgRnJvbnRpZXIgRm91bmRhdGlvbjEUMBIG +A1UEAwwLZXhhbXBsZS5jb20wXDANBgkqhkiG9w0BAQEFAANLADBIAkEArHVztFHt +H92ucFJD/N/HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2+6QWE30cWgdmJS86ObRz6 +lUTor4R0T+3C5QIDAQABoIIGFDCCBhAGCSqGSIb3DQEJDjGCBgEwggX9MAkGA1Ud +EwQCMAAwCwYDVR0PBAQDAgXgMIIF4QYDVR0RBIIF2DCCBdSCDGV4YW1wbGUxLmNv +bYIMZXhhbXBsZTIuY29tggxleGFtcGxlMy5jb22CDGV4YW1wbGU0LmNvbYIMZXhh +bXBsZTUuY29tggxleGFtcGxlNi5jb22CDGV4YW1wbGU3LmNvbYIMZXhhbXBsZTgu +Y29tggxleGFtcGxlOS5jb22CDWV4YW1wbGUxMC5jb22CDWV4YW1wbGUxMS5jb22C +DWV4YW1wbGUxMi5jb22CDWV4YW1wbGUxMy5jb22CDWV4YW1wbGUxNC5jb22CDWV4 +YW1wbGUxNS5jb22CDWV4YW1wbGUxNi5jb22CDWV4YW1wbGUxNy5jb22CDWV4YW1w +bGUxOC5jb22CDWV4YW1wbGUxOS5jb22CDWV4YW1wbGUyMC5jb22CDWV4YW1wbGUy +MS5jb22CDWV4YW1wbGUyMi5jb22CDWV4YW1wbGUyMy5jb22CDWV4YW1wbGUyNC5j +b22CDWV4YW1wbGUyNS5jb22CDWV4YW1wbGUyNi5jb22CDWV4YW1wbGUyNy5jb22C +DWV4YW1wbGUyOC5jb22CDWV4YW1wbGUyOS5jb22CDWV4YW1wbGUzMC5jb22CDWV4 +YW1wbGUzMS5jb22CDWV4YW1wbGUzMi5jb22CDWV4YW1wbGUzMy5jb22CDWV4YW1w +bGUzNC5jb22CDWV4YW1wbGUzNS5jb22CDWV4YW1wbGUzNi5jb22CDWV4YW1wbGUz +Ny5jb22CDWV4YW1wbGUzOC5jb22CDWV4YW1wbGUzOS5jb22CDWV4YW1wbGU0MC5j +b22CDWV4YW1wbGU0MS5jb22CDWV4YW1wbGU0Mi5jb22CDWV4YW1wbGU0My5jb22C +DWV4YW1wbGU0NC5jb22CDWV4YW1wbGU0NS5jb22CDWV4YW1wbGU0Ni5jb22CDWV4 +YW1wbGU0Ny5jb22CDWV4YW1wbGU0OC5jb22CDWV4YW1wbGU0OS5jb22CDWV4YW1w +bGU1MC5jb22CDWV4YW1wbGU1MS5jb22CDWV4YW1wbGU1Mi5jb22CDWV4YW1wbGU1 +My5jb22CDWV4YW1wbGU1NC5jb22CDWV4YW1wbGU1NS5jb22CDWV4YW1wbGU1Ni5j +b22CDWV4YW1wbGU1Ny5jb22CDWV4YW1wbGU1OC5jb22CDWV4YW1wbGU1OS5jb22C +DWV4YW1wbGU2MC5jb22CDWV4YW1wbGU2MS5jb22CDWV4YW1wbGU2Mi5jb22CDWV4 +YW1wbGU2My5jb22CDWV4YW1wbGU2NC5jb22CDWV4YW1wbGU2NS5jb22CDWV4YW1w +bGU2Ni5jb22CDWV4YW1wbGU2Ny5jb22CDWV4YW1wbGU2OC5jb22CDWV4YW1wbGU2 +OS5jb22CDWV4YW1wbGU3MC5jb22CDWV4YW1wbGU3MS5jb22CDWV4YW1wbGU3Mi5j +b22CDWV4YW1wbGU3My5jb22CDWV4YW1wbGU3NC5jb22CDWV4YW1wbGU3NS5jb22C +DWV4YW1wbGU3Ni5jb22CDWV4YW1wbGU3Ny5jb22CDWV4YW1wbGU3OC5jb22CDWV4 +YW1wbGU3OS5jb22CDWV4YW1wbGU4MC5jb22CDWV4YW1wbGU4MS5jb22CDWV4YW1w +bGU4Mi5jb22CDWV4YW1wbGU4My5jb22CDWV4YW1wbGU4NC5jb22CDWV4YW1wbGU4 +NS5jb22CDWV4YW1wbGU4Ni5jb22CDWV4YW1wbGU4Ny5jb22CDWV4YW1wbGU4OC5j +b22CDWV4YW1wbGU4OS5jb22CDWV4YW1wbGU5MC5jb22CDWV4YW1wbGU5MS5jb22C +DWV4YW1wbGU5Mi5jb22CDWV4YW1wbGU5My5jb22CDWV4YW1wbGU5NC5jb22CDWV4 +YW1wbGU5NS5jb22CDWV4YW1wbGU5Ni5jb22CDWV4YW1wbGU5Ny5jb22CDWV4YW1w +bGU5OC5jb22CDWV4YW1wbGU5OS5jb22CDmV4YW1wbGUxMDAuY29tMA0GCSqGSIb3 +DQEBCwUAA0EAW05UMFavHn2rkzMyUfzsOvWzVNlm43eO2yHu5h5TzDb23gkDnNEo +duUAbQ+CLJHYd+MvRCmPQ+3ZnaPy7l/0Hg== +-----END CERTIFICATE REQUEST----- From ba93c576977eafa754c41bb7c4410cc876143b4a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 14:22:13 -0500 Subject: [PATCH 024/100] Added large sans cert and csr test --- acme/acme/crypto_util_test.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index b3c39c388..e926fc317 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -82,6 +82,10 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): self.assertEqual(self._call_cert('cert-san.pem'), ['example.com', 'www.example.com']) + def test_cert_hundred_sans(self): + self.assertEqual(self._call_cert('cert-100sans.pem'), + ['example{0}.com'.format(i) for i in range(1, 101)]) + def test_csr_no_sans(self): self.assertEqual(self._call_csr('csr-nosans.pem'), []) @@ -98,6 +102,10 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): "example.info", "subdomain.example.com", "other.subdomain.example.com"]) + def test_csr_hundred_sans(self): + self.assertEqual(self._call_csr('csr-100sans.pem'), + ['example{0}.com'.format(i) for i in range(1, 101)]) + if __name__ == "__main__": unittest.main() # pragma: no cover From 96114ba84e2a6177cca9533e5d5fb9fafa7fa4b1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 15:10:08 -0500 Subject: [PATCH 025/100] Add IDN SANs CSR and cert --- acme/acme/testdata/cert-idnsans.pem | 30 +++++++++++++++++++++++++++++ acme/acme/testdata/csr-idnsans.pem | 27 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 acme/acme/testdata/cert-idnsans.pem create mode 100644 acme/acme/testdata/csr-idnsans.pem diff --git a/acme/acme/testdata/cert-idnsans.pem b/acme/acme/testdata/cert-idnsans.pem new file mode 100644 index 000000000..932649692 --- /dev/null +++ b/acme/acme/testdata/cert-idnsans.pem @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFNjCCBOCgAwIBAgIJAP4rNqqOKifCMA0GCSqGSIb3DQEBCwUAMGQxCzAJBgNV +BAgMAkNBMRYwFAYDVQQHDA1TYW4gRnJhbmNpc2NvMScwJQYDVQQLDB5FbGVjdHJv +bmljIEZyb250aWVyIEZvdW5kYXRpb24xFDASBgNVBAMMC2V4YW1wbGUuY29tMB4X +DTE2MDEwNjIwMDg1OFoXDTE2MDEwNzIwMDg1OFowZDELMAkGA1UECAwCQ0ExFjAU +BgNVBAcMDVNhbiBGcmFuY2lzY28xJzAlBgNVBAsMHkVsZWN0cm9uaWMgRnJvbnRp +ZXIgRm91bmRhdGlvbjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wXDANBgkqhkiG9w0B +AQEFAANLADBIAkEArHVztFHtH92ucFJD/N/HW9AsdRsUuHUBBBDlHwNlRd3fp580 +rv2+6QWE30cWgdmJS86ObRz6lUTor4R0T+3C5QIDAQABo4IDczCCA28wCQYDVR0T +BAIwADALBgNVHQ8EBAMCBeAwggNTBgNVHREEggNKMIIDRoJiz4PPhM+Fz4bPh8+I +z4nPis+Lz4zPjc+Oz4/PkM+Rz5LPk8+Uz5XPls+Xz5jPmc+az5vPnM+dz57Pn8+g +z6HPos+jz6TPpc+mz6fPqM+pz6rPq8+sz63Prs+vLmludmFsaWSCYs+wz7HPss+z +z7TPtc+2z7fPuM+5z7rPu8+8z73Pvs+/2YHZgtmD2YTZhdmG2YfZiNmJ2YrZi9mM +2Y3ZjtmP2ZDZkdmS2ZPZlNmV2ZbZl9mY2ZnZmtmb2ZzZnS5pbnZhbGlkgmLZntmf +2aDZodmi2aPZpNml2abZp9mo2anZqtmr2azZrdmu2a/ZsNmx2bLZs9m02bXZttm3 +2bjZudm62bvZvNm92b7Zv9qA2oHagtqD2oTahdqG2ofaiNqJ2oouaW52YWxpZIJi +2ovajNqN2o7aj9qQ2pHaktqT2pTaldqW2pfamNqZ2pram9qc2p3antqf2qDaodqi +2qPapNql2qbap9qo2qnaqtqr2qzardqu2q/asNqx2rLas9q02rXattq3LmludmFs +aWSCYtq42rnautq72rzavdq+2r/bgNuB24Lbg9uE24XbhtuH24jbiduK24vbjNuN +247bj9uQ25HbktuT25TblduW25fbmNuZ25rbm9uc253bntuf26Dbodui26PbpC5p +bnZhbGlkgnjbpdum26fbqNup26rbq9us263brtuv27Dbsduy27PbtNu127bbt9u4 +27nbutu74aCg4aCh4aCi4aCj4aCk4aCl4aCm4aCn4aCo4aCp4aCq4aCr4aCs4aCt +4aCu4aCv4aCw4aCx4aCy4aCz4aC04aC1LmludmFsaWSCgY/hoLbhoLfhoLjhoLnh +oLrhoLvhoLzhoL3hoL7hoL/hoYDhoYHhoYLhoYPhoYThoYXhoYbhoYfhoYjhoYnh +oYrhoYvhoYzhoY3hoY7hoY/hoZDhoZHhoZLhoZPhoZThoZXhoZbhoZfhoZjhoZnh +oZrhoZvhoZzhoZ3hoZ7hoZ/hoaDhoaHhoaIuaW52YWxpZIJE4aGj4aGk4aGl4aGm +4aGn4aGo4aGp4aGq4aGr4aGs4aGt4aGu4aGv4aGw4aGx4aGy4aGz4aG04aG14aG2 +LmludmFsaWQwDQYJKoZIhvcNAQELBQADQQAzOQL/54yXxln87/YvEQbBm9ik9zoT +TxEkvnZ4kmTRhDsUPtRjMXhY2FH7LOtXKnJQ7POUB7AsJ2Z6uq2w623G +-----END CERTIFICATE----- diff --git a/acme/acme/testdata/csr-idnsans.pem b/acme/acme/testdata/csr-idnsans.pem new file mode 100644 index 000000000..d6e91a420 --- /dev/null +++ b/acme/acme/testdata/csr-idnsans.pem @@ -0,0 +1,27 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIEpzCCBFECAQAwZDELMAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lz +Y28xJzAlBgNVBAsMHkVsZWN0cm9uaWMgRnJvbnRpZXIgRm91bmRhdGlvbjEUMBIG +A1UEAwwLZXhhbXBsZS5jb20wXDANBgkqhkiG9w0BAQEFAANLADBIAkEArHVztFHt +H92ucFJD/N/HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2+6QWE30cWgdmJS86ObRz6 +lUTor4R0T+3C5QIDAQABoIIDhjCCA4IGCSqGSIb3DQEJDjGCA3MwggNvMAkGA1Ud +EwQCMAAwCwYDVR0PBAQDAgXgMIIDUwYDVR0RBIIDSjCCA0aCYs+Dz4TPhc+Gz4fP +iM+Jz4rPi8+Mz43Pjs+Pz5DPkc+Sz5PPlM+Vz5bPl8+Yz5nPms+bz5zPnc+ez5/P +oM+hz6LPo8+kz6XPps+nz6jPqc+qz6vPrM+tz67Pry5pbnZhbGlkgmLPsM+xz7LP +s8+0z7XPts+3z7jPuc+6z7vPvM+9z77Pv9mB2YLZg9mE2YXZhtmH2YjZidmK2YvZ +jNmN2Y7Zj9mQ2ZHZktmT2ZTZldmW2ZfZmNmZ2ZrZm9mc2Z0uaW52YWxpZIJi2Z7Z +n9mg2aHZotmj2aTZpdmm2afZqNmp2arZq9ms2a3Zrtmv2bDZsdmy2bPZtNm12bbZ +t9m42bnZutm72bzZvdm+2b/agNqB2oLag9qE2oXahtqH2ojaidqKLmludmFsaWSC +YtqL2ozajdqO2o/akNqR2pLak9qU2pXaltqX2pjamdqa2pvanNqd2p7an9qg2qHa +otqj2qTapdqm2qfaqNqp2qraq9qs2q3artqv2rDasdqy2rPatNq12rbaty5pbnZh +bGlkgmLauNq52rrau9q82r3avtq/24DbgduC24PbhNuF24bbh9uI24nbituL24zb +jduO24/bkNuR25Lbk9uU25XbltuX25jbmdua25vbnNud257bn9ug26Hbotuj26Qu +aW52YWxpZIJ426Xbptun26jbqduq26vbrNut267br9uw27Hbstuz27Tbtdu227fb +uNu527rbu+GgoOGgoeGgouGgo+GgpOGgpeGgpuGgp+GgqOGgqeGgquGgq+GgrOGg +reGgruGgr+GgsOGgseGgsuGgs+GgtOGgtS5pbnZhbGlkgoGP4aC24aC34aC44aC5 +4aC64aC74aC84aC94aC+4aC/4aGA4aGB4aGC4aGD4aGE4aGF4aGG4aGH4aGI4aGJ +4aGK4aGL4aGM4aGN4aGO4aGP4aGQ4aGR4aGS4aGT4aGU4aGV4aGW4aGX4aGY4aGZ +4aGa4aGb4aGc4aGd4aGe4aGf4aGg4aGh4aGiLmludmFsaWSCROGho+GhpOGhpeGh +puGhp+GhqOGhqeGhquGhq+GhrOGhreGhruGhr+GhsOGhseGhsuGhs+GhtOGhteGh +ti5pbnZhbGlkMA0GCSqGSIb3DQEBCwUAA0EAeNkY0M0+kMnjRo6dEUoGE4dX9fEr +dfGrpPUBcwG0P5QBdZJWvZxTfRl14yuPYHbGHULXeGqRdkU6HK5pOlzpng== +-----END CERTIFICATE REQUEST----- From 9dc4af5cee5445e1a8bdfab97d56ecbceea6d659 Mon Sep 17 00:00:00 2001 From: Ben Ubois Date: Wed, 6 Jan 2016 12:10:21 -0800 Subject: [PATCH 026/100] Document webroot request path. It's handy to know the implementation details of the webroot plugin so that a server can be configured to properly the ACME challenge files. --- docs/using.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/using.rst b/docs/using.rst index 5da13f02c..f7a59cf7c 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -139,9 +139,20 @@ Would obtain a single certificate for all of those names, using the ``/var/www/example`` webroot directory for the first two, and ``/var/www/eg`` for the second two. +The webroot plugin works by creating a temporary file for each of your requested +domains in ``${webroot-path}/.well-known/acme-challenge``. Then the Let's +Encrypt validation server makes HTTP requests to validate that the DNS for each +requested domain resolves to the server running letsencrypt. An example request +made to your web server would look like: + +:: + + 66.133.109.36 - - [05/Jan/2016:20:11:24 -0500] "GET /.well-known/acme-challenge/HGr8U1IeTW4kY_Z6UIyaakzOkyQgPr_7ArlLgtZE8SX HTTP/1.1" 200 87 "-" "Mozilla/5.0 (compatible; Let's Encrypt validation server; +https://www.letsencrypt.org)" + Note that to use the webroot plugin, your server must be configured to serve files from hidden directories. + Manual ------ From 1cdff156c90e075c888829f928fab644d46b98f9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 15:33:36 -0500 Subject: [PATCH 027/100] Add IDN test --- acme/acme/crypto_util_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index e926fc317..0f3c9225b 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -1,9 +1,11 @@ """Tests for acme.crypto_util.""" +import itertools import socket import threading import time import unittest +import six from six.moves import socketserver # pylint: disable=import-error from acme import errors @@ -69,6 +71,14 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): from acme.crypto_util import _pyopenssl_cert_or_req_san return _pyopenssl_cert_or_req_san(loader(name)) + @classmethod + def _get_idn_names(cls): + chars = [six.unichr(i) for i in itertools.chain(range(0x3c3, 0x400), + range(0x641, 0x6fc), + range(0x1820, 0x1877))] + return [''.join(chars[i: i + 45]) + '.invalid' + for i in range(0, len(chars), 45)] + def _call_cert(self, name): return self._call(test_util.load_cert, name) @@ -86,6 +96,10 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): self.assertEqual(self._call_cert('cert-100sans.pem'), ['example{0}.com'.format(i) for i in range(1, 101)]) + def test_cert_idn_sans(self): + self.assertEqual(self._call_cert('cert-idnsans.pem'), + self._get_idn_names()) + def test_csr_no_sans(self): self.assertEqual(self._call_csr('csr-nosans.pem'), []) @@ -106,6 +120,10 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): self.assertEqual(self._call_csr('csr-100sans.pem'), ['example{0}.com'.format(i) for i in range(1, 101)]) + def test_csr(self): + self.assertEqual(self._call_csr('csr-idnsans.pem'), + self._get_idn_names()) + if __name__ == "__main__": unittest.main() # pragma: no cover From 51bc1311a219b81feba302dc8a7e1a9756201bf8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 15:34:42 -0500 Subject: [PATCH 028/100] Fixed rogue quotes --- acme/acme/crypto_util_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 0f3c9225b..446c19eec 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -112,9 +112,9 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): 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"]) + ['example.com', 'example.org', 'example.net', + 'example.info', 'subdomain.example.com', + 'other.subdomain.example.com']) def test_csr_hundred_sans(self): self.assertEqual(self._call_csr('csr-100sans.pem'), @@ -125,5 +125,5 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): self._get_idn_names()) -if __name__ == "__main__": +if __name__ == '__main__': unittest.main() # pragma: no cover From 2f569f77836d1628a1cf04afe41fc688c0aae516 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 21:40:26 -0500 Subject: [PATCH 029/100] Tox fanciness --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index dbd6d51fa..c1d23b69f 100644 --- a/tox.ini +++ b/tox.ini @@ -6,7 +6,7 @@ # acme and letsencrypt are not yet on pypi, so when Tox invokes # "install *.zip", it will not find deps skipsdist = true -envlist = py26,py27,py33,py34,py35,cover,lint +envlist = py{26,27,33,34,35},cover,lint # nosetest -v => more verbose output, allows to detect busy waiting # loops, especially on Travis From 90f0b15c9d8f4b34d6fcbaa599446fffd0f53dba Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 21:51:42 -0500 Subject: [PATCH 030/100] Add old dependency test --- tox.ini | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index c1d23b69f..e2d7900e0 100644 --- a/tox.ini +++ b/tox.ini @@ -6,7 +6,7 @@ # acme and letsencrypt are not yet on pypi, so when Tox invokes # "install *.zip", it will not find deps skipsdist = true -envlist = py{26,27,33,34,35},cover,lint +envlist = py{26,27,33,34,35},py{26,27}-oldest,cover,lint # nosetest -v => more verbose output, allows to detect busy waiting # loops, especially on Travis @@ -31,6 +31,15 @@ setenv = PYTHONHASHSEED = 0 # https://testrun.org/tox/latest/example/basic.html#special-handling-of-pythonhas + +deps = + py{26,27}-oldest: cryptography==0.8 + py{26,27}-oldest: configargparse==0.10.0 + py{26,27}-oldest: psutil==2.1.0 + py{26,27}-oldest: PyOpenSSL==0.13 + py{26,27}-oldest: pyparsing==1.5.5 + py{26,27}-oldest: python2-pythondialog==3.2.2rc1 + [testenv:py33] commands = pip install -e acme[testing] From 07b3a9fd955121d1ea976bab03d2d6fd7f7ad42d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 21:52:14 -0500 Subject: [PATCH 031/100] Add old dependency tests to Travis --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index a5d6d8a85..2532216f1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,8 @@ env: matrix: - TOXENV=py26 BOULDER_INTEGRATION=1 - TOXENV=py27 BOULDER_INTEGRATION=1 + - TOXENV=py26-oldest + - TOXENV=py27-oldest - TOXENV=lint - TOXENV=cover # Disabled for now due to requiring sudo -> causing more boulder integration From 8a1931e23c015f40cd21c5b003ed56d5c1550bec Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 21:56:52 -0500 Subject: [PATCH 032/100] Run integration tests with old deps --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2532216f1..c3e2e92aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,8 +22,8 @@ env: matrix: - TOXENV=py26 BOULDER_INTEGRATION=1 - TOXENV=py27 BOULDER_INTEGRATION=1 - - TOXENV=py26-oldest - - TOXENV=py27-oldest + - TOXENV=py26-oldest BOULDER_INTEGRATION=1 + - TOXENV=py27-oldest BOULDER_INTEGRATION=1 - TOXENV=lint - TOXENV=cover # Disabled for now due to requiring sudo -> causing more boulder integration From 94508b00dfbcd1265967ee23c13f0dda944b9d30 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 21:57:15 -0500 Subject: [PATCH 033/100] Don't pin pyparsing version --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index e2d7900e0..f91285bec 100644 --- a/tox.ini +++ b/tox.ini @@ -37,7 +37,6 @@ deps = py{26,27}-oldest: configargparse==0.10.0 py{26,27}-oldest: psutil==2.1.0 py{26,27}-oldest: PyOpenSSL==0.13 - py{26,27}-oldest: pyparsing==1.5.5 py{26,27}-oldest: python2-pythondialog==3.2.2rc1 [testenv:py33] From 0b1e1d0937da0087f252807e72fba27848ed1cda Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 6 Jan 2016 22:07:45 -0500 Subject: [PATCH 034/100] Use test_util.load_cert --- acme/acme/challenges_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 6b277ac27..4f2d06167 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -264,9 +264,7 @@ class TLSSNI01ResponseTest(unittest.TestCase): def test_verify_bad_cert(self): self.assertFalse(self.response.verify_cert( - OpenSSL.crypto.load_certificate( - OpenSSL.crypto.FILETYPE_PEM, - test_util.load_vector('cert.pem')))) + test_util.load_cert('cert.pem'))) def test_simple_verify_bad_key_authorization(self): key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem')) From a815ddbafd2856700b9f30ea71b594ef8961da9e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 10:05:33 -0500 Subject: [PATCH 035/100] Remove excessive newline --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index f91285bec..45c7c43d2 100644 --- a/tox.ini +++ b/tox.ini @@ -31,7 +31,6 @@ setenv = PYTHONHASHSEED = 0 # https://testrun.org/tox/latest/example/basic.html#special-handling-of-pythonhas - deps = py{26,27}-oldest: cryptography==0.8 py{26,27}-oldest: configargparse==0.10.0 From e7da21eec0cadfe80f8836d44f1aa64cd89130de Mon Sep 17 00:00:00 2001 From: Reinaldo de Souza Jr Date: Thu, 7 Jan 2016 11:27:40 -0500 Subject: [PATCH 036/100] Makes NginxParser aware of directive Fixes #2059 --- letsencrypt-nginx/letsencrypt_nginx/parser.py | 6 ++++-- .../letsencrypt_nginx/tests/parser_test.py | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/parser.py b/letsencrypt-nginx/letsencrypt_nginx/parser.py index c60d0102a..3b1dd049e 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/parser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/parser.py @@ -113,7 +113,7 @@ class NginxParser(object): for filename in servers: for server in servers[filename]: # Parse the server block into a VirtualHost object - parsed_server = _parse_server(server) + parsed_server = parse_server(server) vhost = obj.VirtualHost(filename, parsed_server['addrs'], parsed_server['ssl'], @@ -451,7 +451,7 @@ def _get_servernames(names): return names.split(' ') -def _parse_server(server): +def parse_server(server): """Parses a list of server directives. :param list server: list of directives in a server block @@ -471,6 +471,8 @@ def _parse_server(server): elif directive[0] == 'server_name': parsed_server['names'].update( _get_servernames(directive[1])) + elif directive[0] == 'ssl' and directive[1] == 'on': + parsed_server['ssl'] = True return parsed_server diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py index 6559a5df6..b64f1dee3 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/parser_test.py @@ -228,6 +228,26 @@ class NginxParserTest(util.NginxTest): c_k = nparser.get_all_certs_keys() self.assertEqual(set([('foo.pem', 'bar.key', filep)]), c_k) + def test_parse_server_ssl(self): + server = parser.parse_server([ + ['listen', '443'] + ]) + self.assertFalse(server['ssl']) + + server = parser.parse_server([ + ['listen', '443 ssl'] + ]) + self.assertTrue(server['ssl']) + + server = parser.parse_server([ + ['listen', '443'], ['ssl', 'off'] + ]) + self.assertFalse(server['ssl']) + + server = parser.parse_server([ + ['listen', '443'], ['ssl', 'on'] + ]) + self.assertTrue(server['ssl']) if __name__ == "__main__": unittest.main() # pragma: no cover From bbf25a2c5edf66acc8df06708cab3181a7928b4f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 08:57:13 -0800 Subject: [PATCH 037/100] Setenvif may be required for conf tests on ubuntu 12.04? --- .../letsencrypt_apache/tests/apache-conf-files/apache-conf-test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test index 4e0443bb7..0afddbb33 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test @@ -49,7 +49,7 @@ if [ "$1" = --debian-modules ] ; then sudo apt-get install -y libapache2-mod-wsgi sudo apt-get install -y libapache2-mod-macro - for mod in ssl rewrite macro wsgi deflate userdir version mime ; do + for mod in ssl rewrite macro wsgi deflate userdir version mime setenvif ; do sudo a2enmod $mod done fi From 4bdd96a29e98cd0aa5b29c1565c986d7b1a8f8e3 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 08:59:53 -0800 Subject: [PATCH 038/100] Verbosity --- .../letsencrypt_apache/tests/apache-conf-files/apache-conf-test | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test index 0afddbb33..7b3f83d13 100755 --- a/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test +++ b/letsencrypt-apache/letsencrypt_apache/tests/apache-conf-files/apache-conf-test @@ -50,6 +50,7 @@ if [ "$1" = --debian-modules ] ; then sudo apt-get install -y libapache2-mod-macro for mod in ssl rewrite macro wsgi deflate userdir version mime setenvif ; do + echo -n enabling $mod sudo a2enmod $mod done fi From dc7f479fe3ac34388da208f97abfff4af98eaef0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 09:44:13 -0800 Subject: [PATCH 039/100] [deb bootstrap] Add precise-backports for libaugeas0 --- bootstrap/_deb_common.sh | 44 ++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 8b96fe6f1..180caf1a1 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -35,25 +35,39 @@ fi augeas_pkg=libaugeas0 AUGVERSION=`apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2` +AddBackportRepo() { + # ARGS: + BACKPORT_NAME="$0" + BACKPORT_SOURCELINE="$1" + if ! grep -v -e ' *#' /etc/apt/sources.list | grep -q "$BACKPORT_NAME" ; then + # This can theoretically error if sources.list.d is empty, but in that case we don't care. + if ! grep -v -e ' *#' /etc/apt/sources.list.d/* 2>/dev/null | grep -q "$BACKPORT_NAME"; then + /bin/echo -n "Installing augeas from wheezy-backports in 3 seconds..." + sleep 1s + /bin/echo -ne "\e[0K\rInstalling augeas from wheezy-backports in 2 seconds..." + sleep 1s + /bin/echo -e "\e[0K\rInstalling augeas from wheezy-backports in 1 second ..." + sleep 1s + if echo $BACKPORT_NAME | grep -q wheezy ; then + /bin/echo '(Backports are only installed if explicitly requested via "apt-get install -t wheezy-backports")' + fi + + echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/"$BACKPORT_NAME".list + apt-get update + fi + fi + +} + + if dpkg --compare-versions 1.0 gt "$AUGVERSION" ; then if lsb_release -a | grep -q wheezy ; then - if ! grep -v -e ' *#' /etc/apt/sources.list | grep -q wheezy-backports ; then - # This can theoretically error if sources.list.d is empty, but in that case we don't care. - if ! grep -v -e ' *#' /etc/apt/sources.list.d/* 2>/dev/null | grep -q wheezy-backports ; then - /bin/echo -n "Installing augeas from wheezy-backports in 3 seconds..." - sleep 1s - /bin/echo -ne "\e[0K\rInstalling augeas from wheezy-backports in 2 seconds..." - sleep 1s - /bin/echo -e "\e[0K\rInstalling augeas from wheezy-backports in 1 second ..." - sleep 1s - /bin/echo '(Backports are only installed if explicitly requested via "apt-get install -t wheezy-backports")' - - echo deb http://http.debian.net/debian wheezy-backports main >> /etc/apt/sources.list.d/wheezy-backports.list - apt-get update - fi - fi + AddBackportRepo wheezy-backports "deb http://http.debian.net/debian wheezy-backports main" apt-get install -y --no-install-recommends -t wheezy-backports libaugeas0 augeas_pkg= + elif lsb_release -a | grep -q precise ; then + # XXX add ARM case + AddBackportRepo precise-backports "deb http://archive.ubuntu.com/ubuntu trusty-backports main restricted universe multiverse" else echo "No libaugeas0 version is available that's new enough to run the" echo "Let's Encrypt apache plugin..." From 491a08e4d47201b8b30891aa9da636c403664776 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 09:46:45 -0800 Subject: [PATCH 040/100] fixen --- bootstrap/_deb_common.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 180caf1a1..410f4e10e 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -37,16 +37,16 @@ AUGVERSION=`apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut - AddBackportRepo() { # ARGS: - BACKPORT_NAME="$0" - BACKPORT_SOURCELINE="$1" + BACKPORT_NAME="$1" + BACKPORT_SOURCELINE="$2" if ! grep -v -e ' *#' /etc/apt/sources.list | grep -q "$BACKPORT_NAME" ; then # This can theoretically error if sources.list.d is empty, but in that case we don't care. if ! grep -v -e ' *#' /etc/apt/sources.list.d/* 2>/dev/null | grep -q "$BACKPORT_NAME"; then - /bin/echo -n "Installing augeas from wheezy-backports in 3 seconds..." + /bin/echo -n "Installing augeas from $BACKPORT_NAME in 3 seconds..." sleep 1s - /bin/echo -ne "\e[0K\rInstalling augeas from wheezy-backports in 2 seconds..." + /bin/echo -ne "\e[0K\rInstalling augeas from $BACKPORT_NAME in 2 seconds..." sleep 1s - /bin/echo -e "\e[0K\rInstalling augeas from wheezy-backports in 1 second ..." + /bin/echo -e "\e[0K\rInstalling augeas from $BACKPORT_NAME in 1 second ..." sleep 1s if echo $BACKPORT_NAME | grep -q wheezy ; then /bin/echo '(Backports are only installed if explicitly requested via "apt-get install -t wheezy-backports")' From 154cd47c83cf5753354563ceb0c92febb2c868ce Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 09:52:10 -0800 Subject: [PATCH 041/100] precision --- bootstrap/_deb_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 410f4e10e..72f8e85c8 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -67,7 +67,7 @@ if dpkg --compare-versions 1.0 gt "$AUGVERSION" ; then augeas_pkg= elif lsb_release -a | grep -q precise ; then # XXX add ARM case - AddBackportRepo precise-backports "deb http://archive.ubuntu.com/ubuntu trusty-backports main restricted universe multiverse" + AddBackportRepo precise-backports "deb http://archive.ubuntu.com/ubuntu precise-backports main restricted universe multiverse" else echo "No libaugeas0 version is available that's new enough to run the" echo "Let's Encrypt apache plugin..." From caf9b1f26111d0a51212aa3b5b70d13d7347b547 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 13:00:29 -0500 Subject: [PATCH 042/100] Clarify _get_idn_sans method --- acme/acme/crypto_util_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 446c19eec..8b510e43e 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -73,6 +73,7 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): @classmethod def _get_idn_names(cls): + """Returns expected names from '{cert,csr}-idnsans.pem'.""" chars = [six.unichr(i) for i in itertools.chain(range(0x3c3, 0x400), range(0x641, 0x6fc), range(0x1820, 0x1877))] From 0f239e0029e77f302401c3c6089e67f45c19d51c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 13:04:29 -0500 Subject: [PATCH 043/100] Add comment about dependency version --- acme/setup.py | 1 + letsencrypt-apache/setup.py | 1 + letsencrypt-nginx/setup.py | 1 + setup.py | 1 + 4 files changed, 4 insertions(+) diff --git a/acme/setup.py b/acme/setup.py index 3b1a42327..4993d7584 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -6,6 +6,7 @@ from setuptools import find_packages version = '0.2.0.dev0' +# Please update tox.ini when modifying dependency version requirements install_requires = [ # load_pem_private/public_key (>=0.6) # rsa_recover_prime_factors (>=0.8) diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index 58008e1e4..a5c5e8a7a 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -6,6 +6,7 @@ from setuptools import find_packages version = '0.2.0.dev0' +# Please update tox.ini when modifying dependency version requirements install_requires = [ 'acme=={0}'.format(version), 'letsencrypt=={0}'.format(version), diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index 1d42fe488..bfb3c3758 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -6,6 +6,7 @@ from setuptools import find_packages version = '0.2.0.dev0' +# Please update tox.ini when modifying dependency version requirements install_requires = [ 'acme=={0}'.format(version), 'letsencrypt=={0}'.format(version), diff --git a/setup.py b/setup.py index f95f672ff..ad7fb6909 100644 --- a/setup.py +++ b/setup.py @@ -30,6 +30,7 @@ readme = read_file(os.path.join(here, 'README.rst')) changes = read_file(os.path.join(here, 'CHANGES.rst')) version = meta['version'] +# Please update tox.ini when modifying dependency version requirements install_requires = [ 'acme=={0}'.format(version), 'configobj', From 99616864766e7cf8acbf8cda7625153159c56738 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 10:09:20 -0800 Subject: [PATCH 044/100] We might need -t afterall --- bootstrap/_deb_common.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 72f8e85c8..2b6ee11be 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -54,6 +54,8 @@ AddBackportRepo() { echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/"$BACKPORT_NAME".list apt-get update + apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" libaugeas0 + augeas_pkg= fi fi @@ -63,8 +65,6 @@ AddBackportRepo() { if dpkg --compare-versions 1.0 gt "$AUGVERSION" ; then if lsb_release -a | grep -q wheezy ; then AddBackportRepo wheezy-backports "deb http://http.debian.net/debian wheezy-backports main" - apt-get install -y --no-install-recommends -t wheezy-backports libaugeas0 - augeas_pkg= elif lsb_release -a | grep -q precise ; then # XXX add ARM case AddBackportRepo precise-backports "deb http://archive.ubuntu.com/ubuntu precise-backports main restricted universe multiverse" From 710eb59f41e9ba0f258bc9b6d74761a6158513ac Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 16:19:21 -0500 Subject: [PATCH 045/100] Fix IDN CSR test name --- acme/acme/crypto_util_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index 8b510e43e..147cd5a2a 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -121,7 +121,7 @@ class PyOpenSSLCertOrReqSANTest(unittest.TestCase): self.assertEqual(self._call_csr('csr-100sans.pem'), ['example{0}.com'.format(i) for i in range(1, 101)]) - def test_csr(self): + def test_csr_idn_sans(self): self.assertEqual(self._call_csr('csr-idnsans.pem'), self._get_idn_names()) From 32957cc5eccc9dc6d61f3c02327edabc860f5d31 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 16:25:23 -0500 Subject: [PATCH 046/100] Comment _pyopenssl_cert_or_req_san method --- acme/acme/crypto_util.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index ecec351c2..0d0e78df6 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -169,11 +169,14 @@ def _pyopenssl_cert_or_req_san(cert_or_req): func = OpenSSL.crypto.dump_certificate else: func = OpenSSL.crypto.dump_certificate_request + + # This method of finding SANs is used to support PyOpenSSL version 0.13. text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8") match = re.search(r"X509v3 Subject Alternative Name:\s*(.*)", text) - sans_parts = [] if match is None else match.group(1).split(parts_separator) + # WARNING: this function assumes that no SAN can include # parts_separator, hence the split! + sans_parts = [] if match is None else match.group(1).split(parts_separator) return [part.split(part_separator)[1] for part in sans_parts if part.startswith(prefix)] From 946f4474da7be9c784b732224f395ab2c22ed621 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 16:45:46 -0500 Subject: [PATCH 047/100] Add warning about multiple SANs extensions --- acme/acme/crypto_util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 0d0e78df6..a25819cf5 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -169,11 +169,11 @@ def _pyopenssl_cert_or_req_san(cert_or_req): func = OpenSSL.crypto.dump_certificate else: func = OpenSSL.crypto.dump_certificate_request - # This method of finding SANs is used to support PyOpenSSL version 0.13. text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8") + # WARNING: this function does not support multiple SANs extensions. + # Multiple X509v3 extensions of the same type is disallowed by RFC 5280. match = re.search(r"X509v3 Subject Alternative Name:\s*(.*)", text) - # WARNING: this function assumes that no SAN can include # parts_separator, hence the split! sans_parts = [] if match is None else match.group(1).split(parts_separator) From cc168c8ef13bb2d7a53c3ce488806632d4f2ca31 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 7 Jan 2016 22:17:40 +0000 Subject: [PATCH 048/100] Generate fresh pylintrc pylint 1.4.2, -generate-rcfile --- acme/.pylintrc | 380 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 380 insertions(+) create mode 100644 acme/.pylintrc diff --git a/acme/.pylintrc b/acme/.pylintrc new file mode 100644 index 000000000..a31ace48d --- /dev/null +++ b/acme/.pylintrc @@ -0,0 +1,380 @@ +[MASTER] + +# Specify a configuration file. +#rcfile= + +# Python code to execute, usually for sys.path manipulation such as +# pygtk.require(). +#init-hook= + +# Profiled execution. +profile=no + +# Add files or directories to the blacklist. They should be base names, not +# paths. +ignore=CVS + +# Pickle collected data for later comparisons. +persistent=yes + +# List of plugins (as comma separated values of python modules names) to load, +# usually to register additional checkers. +load-plugins= + +# DEPRECATED +include-ids=no + +# DEPRECATED +symbols=no + +# Use multiple processes to speed up Pylint. +jobs=1 + +# Allow loading of arbitrary C extensions. Extensions are imported into the +# active Python interpreter and may run arbitrary code. +unsafe-load-any-extension=no + +# A comma-separated list of package or module names from where C extensions may +# be loaded. Extensions are loading into the active Python interpreter and may +# run arbitrary code +extension-pkg-whitelist= + + +[MESSAGES CONTROL] + +# Only show warnings with the listed confidence levels. Leave empty to show +# all. Valid levels: HIGH, INFERENCE, INFERENCE_FAILURE, UNDEFINED +confidence= + +# Enable the message, report, category or checker with the given id(s). You can +# either give multiple identifier separated by comma (,) or put this option +# multiple time. See also the "--disable" option for examples. +#enable= + +# Disable the message, report, category or checker with the given id(s). You +# can either give multiple identifiers separated by comma (,) or put this +# option multiple times (only on the command line, not in the configuration +# file where it should appear only once).You can also use "--disable=all" to +# disable everything first and then reenable specific checks. For example, if +# you want to run only the similarities checker, you can use "--disable=all +# --enable=similarities". If you want to run only the classes checker, but have +# no Warning level messages displayed, use"--disable=all --enable=classes +# --disable=W" +disable=E1608,W1627,E1601,E1603,E1602,E1605,E1604,E1607,E1606,W1621,W1620,W1623,W1622,W1625,W1624,W1609,W1608,W1607,W1606,W1605,W1604,W1603,W1602,W1601,W1639,W1640,I0021,W1638,I0020,W1618,W1619,W1630,W1626,W1637,W1634,W1635,W1610,W1611,W1612,W1613,W1614,W1615,W1616,W1617,W1632,W1633,W0704,W1628,W1629,W1636 + + +[REPORTS] + +# Set the output format. Available formats are text, parseable, colorized, msvs +# (visual studio) and html. You can also give a reporter class, eg +# mypackage.mymodule.MyReporterClass. +output-format=text + +# Put messages in a separate file for each module / package specified on the +# command line instead of printing them on stdout. Reports (if any) will be +# written in a file name "pylint_global.[txt|html]". +files-output=no + +# Tells whether to display a full report or only the messages +reports=yes + +# Python expression which should return a note less than 10 (10 is the highest +# note). You have access to the variables errors warning, statement which +# respectively contain the number of errors / warnings messages and the total +# number of statements analyzed. This is used by the global evaluation report +# (RP0004). +evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10) + +# Add a comment according to your evaluation note. This is used by the global +# evaluation report (RP0004). +comment=no + +# Template used to display messages. This is a python new-style format string +# used to format the message information. See doc for all details +#msg-template= + + +[FORMAT] + +# Maximum number of characters on a single line. +max-line-length=100 + +# Regexp for a line that is allowed to be longer than the limit. +ignore-long-lines=^\s*(# )??$ + +# Allow the body of an if to be on the same line as the test if there is no +# else. +single-line-if-stmt=no + +# List of optional constructs for which whitespace checking is disabled +no-space-check=trailing-comma,dict-separator + +# Maximum number of lines in a module +max-module-lines=1000 + +# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 +# tab). +indent-string=' ' + +# Number of spaces of indent required inside a hanging or continued line. +indent-after-paren=4 + +# Expected format of line ending, e.g. empty (any line ending), LF or CRLF. +expected-line-ending-format= + + +[MISCELLANEOUS] + +# List of note tags to take in consideration, separated by a comma. +notes=FIXME,XXX,TODO + + +[LOGGING] + +# Logging modules to check that the string format arguments are in logging +# function parameter format +logging-modules=logging + + +[SPELLING] + +# Spelling dictionary name. Available dictionaries: none. To make it working +# install python-enchant package. +spelling-dict= + +# List of comma separated words that should not be checked. +spelling-ignore-words= + +# A path to a file that contains private dictionary; one word per line. +spelling-private-dict-file= + +# Tells whether to store unknown words to indicated private dictionary in +# --spelling-private-dict-file option instead of raising a message. +spelling-store-unknown-words=no + + +[TYPECHECK] + +# Tells whether missing members accessed in mixin class should be ignored. A +# mixin class is detected if its name ends with "mixin" (case insensitive). +ignore-mixin-members=yes + +# List of module names for which member attributes should not be checked +# (useful for modules/projects where namespaces are manipulated during runtime +# and thus existing member attributes cannot be deduced by static analysis +ignored-modules= + +# List of classes names for which member attributes should not be checked +# (useful for classes with attributes dynamically set). +ignored-classes=SQLObject + +# When zope mode is activated, add a predefined set of Zope acquired attributes +# to generated-members. +zope=no + +# List of members which are set dynamically and missed by pylint inference +# system, and so shouldn't trigger E0201 when accessed. Python regular +# expressions are accepted. +generated-members=REQUEST,acl_users,aq_parent + + +[SIMILARITIES] + +# Minimum lines number of a similarity. +min-similarity-lines=4 + +# Ignore comments when computing similarities. +ignore-comments=yes + +# Ignore docstrings when computing similarities. +ignore-docstrings=yes + +# Ignore imports when computing similarities. +ignore-imports=no + + +[VARIABLES] + +# Tells whether we should check for unused import in __init__ files. +init-import=no + +# A regular expression matching the name of dummy variables (i.e. expectedly +# not used). +dummy-variables-rgx=_$|dummy + +# List of additional names supposed to be defined in builtins. Remember that +# you should avoid to define new builtins when possible. +additional-builtins= + +# List of strings which can identify a callback function by name. A callback +# name must start or end with one of those strings. +callbacks=cb_,_cb + + +[BASIC] + +# Required attributes for module, separated by a comma +required-attributes= + +# List of builtins function names that should not be used, separated by a comma +bad-functions=map,filter,input + +# Good variable names which should always be accepted, separated by a comma +good-names=i,j,k,ex,Run,_ + +# Bad variable names which should always be refused, separated by a comma +bad-names=foo,bar,baz,toto,tutu,tata + +# Colon-delimited sets of names that determine each other's naming style when +# the name regexes allow several styles. +name-group= + +# Include a hint for the correct naming format with invalid-name +include-naming-hint=no + +# Regular expression matching correct function names +function-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for function names +function-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct variable names +variable-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for variable names +variable-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct constant names +const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Naming hint for constant names +const-name-hint=(([A-Z_][A-Z0-9_]*)|(__.*__))$ + +# Regular expression matching correct attribute names +attr-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for attribute names +attr-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct argument names +argument-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for argument names +argument-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression matching correct class attribute names +class-attribute-rgx=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Naming hint for class attribute names +class-attribute-name-hint=([A-Za-z_][A-Za-z0-9_]{2,30}|(__.*__))$ + +# Regular expression matching correct inline iteration names +inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$ + +# Naming hint for inline iteration names +inlinevar-name-hint=[A-Za-z_][A-Za-z0-9_]*$ + +# Regular expression matching correct class names +class-rgx=[A-Z_][a-zA-Z0-9]+$ + +# Naming hint for class names +class-name-hint=[A-Z_][a-zA-Z0-9]+$ + +# Regular expression matching correct module names +module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Naming hint for module names +module-name-hint=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ + +# Regular expression matching correct method names +method-rgx=[a-z_][a-z0-9_]{2,30}$ + +# Naming hint for method names +method-name-hint=[a-z_][a-z0-9_]{2,30}$ + +# Regular expression which should only match function or class names that do +# not require a docstring. +no-docstring-rgx=__.*__ + +# Minimum line length for functions/classes that require docstrings, shorter +# ones are exempt. +docstring-min-length=-1 + + +[CLASSES] + +# List of interface methods to ignore, separated by a comma. This is used for +# instance to not check methods defines in Zope's Interface base class. +ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by + +# List of method names used to declare (i.e. assign) instance attributes. +defining-attr-methods=__init__,__new__,setUp + +# List of valid names for the first argument in a class method. +valid-classmethod-first-arg=cls + +# List of valid names for the first argument in a metaclass class method. +valid-metaclass-classmethod-first-arg=mcs + +# List of member names, which should be excluded from the protected access +# warning. +exclude-protected=_asdict,_fields,_replace,_source,_make + + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=5 + +# Argument names that match this expression will be ignored. Default to name +# with leading underscore +ignored-argument-names=_.* + +# Maximum number of locals for function / method body +max-locals=15 + +# Maximum number of return / yield for function / method body +max-returns=6 + +# Maximum number of branch for function / method body +max-branches=12 + +# Maximum number of statements in function / method body +max-statements=50 + +# Maximum number of parents for a class (see R0901). +max-parents=7 + +# Maximum number of attributes for a class (see R0902). +max-attributes=7 + +# Minimum number of public methods for a class (see R0903). +min-public-methods=2 + +# Maximum number of public methods for a class (see R0904). +max-public-methods=20 + + +[IMPORTS] + +# Deprecated modules which should not be used, separated by a comma +deprecated-modules=regsub,TERMIOS,Bastion,rexec + +# Create a graph of every (i.e. internal and external) dependencies in the +# given file (report RP0402 must not be disabled) +import-graph= + +# Create a graph of external dependencies in the given file (report RP0402 must +# not be disabled) +ext-import-graph= + +# Create a graph of internal dependencies in the given file (report RP0402 must +# not be disabled) +int-import-graph= + + +[EXCEPTIONS] + +# Exceptions that will emit a warning when being caught. Defaults to +# "Exception" +overgeneral-exceptions=Exception From dba69d079f734f13437f1845f0008d1f1e81f6fa Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 7 Jan 2016 22:18:21 +0000 Subject: [PATCH 049/100] Separate pylintrc for acme --- acme/.pylintrc | 23 +++++++++++++---------- acme/acme/challenges.py | 2 +- acme/acme/crypto_util.py | 2 +- acme/acme/messages.py | 4 ++-- tox.ini | 2 +- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/acme/.pylintrc b/acme/.pylintrc index a31ace48d..33650310d 100644 --- a/acme/.pylintrc +++ b/acme/.pylintrc @@ -19,7 +19,7 @@ persistent=yes # List of plugins (as comma separated values of python modules names) to load, # usually to register additional checkers. -load-plugins= +load-plugins=linter_plugin # DEPRECATED include-ids=no @@ -60,7 +60,10 @@ confidence= # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable=E1608,W1627,E1601,E1603,E1602,E1605,E1604,E1607,E1606,W1621,W1620,W1623,W1622,W1625,W1624,W1609,W1608,W1607,W1606,W1605,W1604,W1603,W1602,W1601,W1639,W1640,I0021,W1638,I0020,W1618,W1619,W1630,W1626,W1637,W1634,W1635,W1610,W1611,W1612,W1613,W1614,W1615,W1616,W1617,W1632,W1633,W0704,W1628,W1629,W1636 +disable=fixme,locally-disabled,abstract-class-not-used +# bstract-class-not-used cannot be disabled locally (at least in +# pylint 1.4.1/2) + [REPORTS] @@ -133,7 +136,7 @@ notes=FIXME,XXX,TODO # Logging modules to check that the string format arguments are in logging # function parameter format -logging-modules=logging +logging-modules=logging,logger [SPELLING] @@ -200,7 +203,7 @@ init-import=no # A regular expression matching the name of dummy variables (i.e. expectedly # not used). -dummy-variables-rgx=_$|dummy +dummy-variables-rgx=_$|dummy|unused # List of additional names supposed to be defined in builtins. Remember that # you should avoid to define new builtins when possible. @@ -220,7 +223,7 @@ required-attributes= bad-functions=map,filter,input # Good variable names which should always be accepted, separated by a comma -good-names=i,j,k,ex,Run,_ +good-names=i,j,k,ex,Run,_,logger # Bad variable names which should always be refused, separated by a comma bad-names=foo,bar,baz,toto,tutu,tata @@ -233,7 +236,7 @@ name-group= include-naming-hint=no # Regular expression matching correct function names -function-rgx=[a-z_][a-z0-9_]{2,30}$ +function-rgx=[a-z_][a-z0-9_]{2,40}$ # Naming hint for function names function-name-hint=[a-z_][a-z0-9_]{2,30}$ @@ -287,14 +290,14 @@ module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ module-name-hint=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$ # Regular expression matching correct method names -method-rgx=[a-z_][a-z0-9_]{2,30}$ +method-rgx=[a-z_][a-z0-9_]{2,49}$ # Naming hint for method names method-name-hint=[a-z_][a-z0-9_]{2,30}$ # Regular expression which should only match function or class names that do # not require a docstring. -no-docstring-rgx=__.*__ +no-docstring-rgx=__.*__|test_[A-Za-z0-9_]*|_.*|.*Test # Minimum line length for functions/classes that require docstrings, shorter # ones are exempt. @@ -324,7 +327,7 @@ exclude-protected=_asdict,_fields,_replace,_source,_make [DESIGN] # Maximum number of arguments for function / method -max-args=5 +max-args=6 # Argument names that match this expression will be ignored. Default to name # with leading underscore @@ -343,7 +346,7 @@ max-branches=12 max-statements=50 # Maximum number of parents for a class (see R0901). -max-parents=7 +max-parents=12 # Maximum number of attributes for a class (see R0902). max-attributes=7 diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 1e456d325..fc414d93b 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -336,7 +336,7 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse): """ @property - def z(self): + def z(self): # pylint: disable=invalid-name """``z`` value used for verification. :rtype bytes: diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 72a93141a..757f7b5a1 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -70,7 +70,7 @@ class SSLSocket(object): # pylint: disable=too-few-public-methods class FakeConnection(object): """Fake OpenSSL.SSL.Connection.""" - # pylint: disable=missing-docstring + # pylint: disable=too-few-public-methods,missing-docstring def __init__(self, connection): self._wrapped = connection diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 0b73864ec..6ef00399d 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -23,13 +23,13 @@ class Error(jose.JSONObjectWithFields, errors.Error): ('badCSR', 'The CSR is unacceptable (e.g., due to a short key)'), ('badNonce', 'The client sent an unacceptable anti-replay nonce'), ('connection', 'The server could not connect to the client to ' - 'verify the domain'), + 'verify the domain'), ('dnssec', 'The server could not validate a DNSSEC signed domain'), ('malformed', 'The request message was malformed'), ('rateLimited', 'There were too many requests of a given type'), ('serverInternal', 'The server experienced an internal error'), ('tls', 'The server experienced a TLS error during domain ' - 'verification'), + 'verification'), ('unauthorized', 'The client lacks sufficient authorization'), ('unknownHost', 'The server could not resolve a domain name'), ) diff --git a/tox.ini b/tox.ini index dbd6d51fa..36e3632c0 100644 --- a/tox.ini +++ b/tox.ini @@ -62,7 +62,7 @@ commands = pip install -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt ./pep8.travis.sh pylint --rcfile=.pylintrc letsencrypt - pylint --rcfile=.pylintrc acme/acme + pylint --rcfile=acme/.pylintrc acme/acme pylint --rcfile=.pylintrc letsencrypt-apache/letsencrypt_apache pylint --rcfile=.pylintrc letsencrypt-nginx/letsencrypt_nginx pylint --rcfile=.pylintrc letsencrypt-compatibility-test/letsencrypt_compatibility_test From 639cbeb7d02801b79429f17f9cd55dad09e962f6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 7 Jan 2016 21:11:09 -0500 Subject: [PATCH 050/100] sans_text_dump_comment += 1 --- acme/acme/crypto_util.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index a25819cf5..a1a1be73b 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -160,6 +160,12 @@ def _pyopenssl_cert_or_req_san(cert_or_req): :rtype: `list` of `unicode` """ + # This function finds SANs by dumping the certificate/CSR to text and + # searching for "X509v3 Subject Alternative Name" in the text. This method + # is used to support PyOpenSSL version 0.13 where the + # `_subjectAltNameString` and `get_extensions` methods are not available + # for CSRs. + # constants based on PyOpenSSL certificate/CSR text dump part_separator = ":" parts_separator = ", " @@ -169,7 +175,6 @@ def _pyopenssl_cert_or_req_san(cert_or_req): func = OpenSSL.crypto.dump_certificate else: func = OpenSSL.crypto.dump_certificate_request - # This method of finding SANs is used to support PyOpenSSL version 0.13. text = func(OpenSSL.crypto.FILETYPE_TEXT, cert_or_req).decode("utf-8") # WARNING: this function does not support multiple SANs extensions. # Multiple X509v3 extensions of the same type is disallowed by RFC 5280. From fc3acc69b69da9875c054c2e1ac602462a2ab533 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jan 2016 19:12:19 -0800 Subject: [PATCH 051/100] Try updating augeas-lenses as well --- bootstrap/_deb_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 2b6ee11be..84ab9e35a 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -54,7 +54,7 @@ AddBackportRepo() { echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/"$BACKPORT_NAME".list apt-get update - apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" libaugeas0 + apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" libaugeas0 augeas-lenses augeas_pkg= fi fi From b1e67f241e64137077215f4f432995ae10439006 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 8 Jan 2016 14:31:30 -0500 Subject: [PATCH 052/100] Fix merge conflicts properly --- acme/setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/acme/setup.py b/acme/setup.py index 82a08b6ef..c10c95546 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -10,7 +10,6 @@ version = '0.2.0.dev0' install_requires = [ # load_pem_private/public_key (>=0.6) # rsa_recover_prime_factors (>=0.8) -<<<<<<< HEAD 'cryptography>=0.8,<1.2', # Connection.set_tlsext_host_name (>=0.13) 'PyOpenSSL>=0.13', From 96b55c8f349ac29e314e5468c406d0e07f4b4917 Mon Sep 17 00:00:00 2001 From: bmw Date: Fri, 8 Jan 2016 17:02:35 -0500 Subject: [PATCH 053/100] Revert "Don't use cryptography version 1.2" --- acme/setup.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 54c4d82d9..7314152cd 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -9,7 +9,7 @@ version = '0.2.0.dev0' install_requires = [ # load_pem_private/public_key (>=0.6) # rsa_recover_prime_factors (>=0.8) - 'cryptography>=0.8,<1.2', + 'cryptography>=0.8', # Connection.set_tlsext_host_name (>=0.13), X509Req.get_extensions (>=0.15) 'PyOpenSSL>=0.15', 'pyrfc3339', diff --git a/setup.py b/setup.py index 4005b0973..f95f672ff 100644 --- a/setup.py +++ b/setup.py @@ -33,7 +33,7 @@ version = meta['version'] install_requires = [ 'acme=={0}'.format(version), 'configobj', - 'cryptography>=0.7,<1.2', # load_pem_x509_certificate + 'cryptography>=0.7', # load_pem_x509_certificate 'parsedatetime', 'psutil>=2.1.0', # net_connections introduced in 2.1.0 'PyOpenSSL', From 705032bc67afe3705af7916d767487f4eeb38bc1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 8 Jan 2016 19:12:30 -0800 Subject: [PATCH 054/100] [Always] Install augeas-lenses - But do we need the augeas-lenses package? - Install augeas from backports even if the backports were already available (this is the third time fixing that bug!) --- bootstrap/_deb_common.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 84ab9e35a..58ea67a45 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -32,7 +32,7 @@ if apt-cache show python-virtualenv > /dev/null 2>&1; then virtualenv="$virtualenv python-virtualenv" fi -augeas_pkg=libaugeas0 +augeas_pkg="libaugeas0 augeas-lenses" AUGVERSION=`apt-cache show --no-all-versions libaugeas0 | grep ^Version: | cut -d" " -f2` AddBackportRepo() { @@ -54,7 +54,7 @@ AddBackportRepo() { echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/"$BACKPORT_NAME".list apt-get update - apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" libaugeas0 augeas-lenses + apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" $augeas_pkg augeas_pkg= fi fi From 7728f4d28a14724783bd8ee42d54ab047d645500 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 Jan 2016 13:24:39 +0000 Subject: [PATCH 055/100] Python 3 Travis testing for acme. Despite its description, https://github.com/letsencrypt/letsencrypt/pull/630, removed not only Python 2.6 support, but also Travis tests against Python 3. ACME library supports Python 3 and Travis should tests it. This must be merged before any pending PRs agains acme library. --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index a5d6d8a85..6cadbd36e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,9 @@ env: matrix: - TOXENV=py26 BOULDER_INTEGRATION=1 - TOXENV=py27 BOULDER_INTEGRATION=1 + - TOXENV=py33 + - TOXENV=py34 + - TOXENV=py35 - TOXENV=lint - TOXENV=cover # Disabled for now due to requiring sudo -> causing more boulder integration From 34010a0168d0df97a08bdd5b985e7694a37123f2 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 Jan 2016 13:31:50 +0000 Subject: [PATCH 056/100] Python 3.5 needs explicit Travis setting --- .travis.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6cadbd36e..680dfeb8a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -24,12 +24,15 @@ env: - TOXENV=py27 BOULDER_INTEGRATION=1 - TOXENV=py33 - TOXENV=py34 - - TOXENV=py35 - TOXENV=lint - TOXENV=cover # Disabled for now due to requiring sudo -> causing more boulder integration # DNS timeouts :( # - TOXENV=apacheconftest +matrix: + include: + - env: TOXENV=py35 + python: 3.5 # Only build pushes to the master branch, PRs, and branches beginning with From b26dda3afe859ca6586d6d89d5b500ee416d2841 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 9 Jan 2016 13:38:12 +0000 Subject: [PATCH 057/100] Add Python 3.5 trove classifier to acme --- acme/setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/acme/setup.py b/acme/setup.py index 7314152cd..f585b3cdd 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -66,6 +66,7 @@ setup( 'Programming Language :: Python :: 3', 'Programming Language :: Python :: 3.3', 'Programming Language :: Python :: 3.4', + 'Programming Language :: Python :: 3.5', 'Topic :: Internet :: WWW/HTTP', 'Topic :: Security', ], From 5b3bd890b70f831c492c904d05e26fec9c5b9201 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 9 Jan 2016 15:15:05 -0800 Subject: [PATCH 058/100] Default: renew 30 days before expiry, rather than 10 - gives more time for various fallback strategies if renewal doesn't work the first time --- letsencrypt/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index ac71bd9fe..fc58ffb8d 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -565,7 +565,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return True # Renewals on the basis of expiry time - interval = self.configuration.get("renew_before_expiry", "10 days") + interval = self.configuration.get("renew_before_expiry", "30 days") expiry = crypto_util.notAfter(self.version( "cert", self.latest_common_version())) now = pytz.UTC.fromutc(datetime.datetime.utcnow()) From 3a7565afe5633abb5329b06c954c72f225e462c0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 10 Jan 2016 01:03:50 -0800 Subject: [PATCH 059/100] trigger travis rerun --- letsencrypt/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index fc58ffb8d..f7e5c3ad3 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -564,7 +564,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes logger.debug("Should renew, certificate is revoked.") return True - # Renewals on the basis of expiry time + # Renews some period before expiry time interval = self.configuration.get("renew_before_expiry", "30 days") expiry = crypto_util.notAfter(self.version( "cert", self.latest_common_version())) From c10bfd6efc6ff27efe94e8a397b809c4faf55986 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 10 Jan 2016 14:01:34 +0000 Subject: [PATCH 060/100] Fix wrong doc comment: account_public_key is None --- acme/acme/challenges.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 68bf3fce4..13d19d3c4 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -236,10 +236,8 @@ class HTTP01Response(KeyAuthorizationChallengeResponse): :param challenges.SimpleHTTP chall: Corresponding challenge. :param unicode domain: Domain name being verified. - :param account_public_key: Public key for the key pair - being authorized. If ``None`` key verification is not - performed! - :param JWK account_public_key: + :param JWK account_public_key: Public key for the key pair + being authorized. :param int port: Port used in the validation. :returns: ``True`` iff validation is successful, ``False`` From f5862a7a4f1313c9b1bf63dc8acedb2b07dac62f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 10 Jan 2016 18:38:53 +0200 Subject: [PATCH 061/100] Parse all included paths in apache root configuration --- .../letsencrypt_apache/configurator.py | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 2a9fb0250..1d4618825 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -488,15 +488,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: list """ - # Search vhost-root, httpd.conf for possible virtual hosts - paths = self.aug.match( - ("/files%s//*[label()=~regexp('%s')]" % - (self.conf("vhost-root"), parser.case_i("VirtualHost")))) - + # Search base config, and all included paths for VirtualHosts vhs = [] + vhost_paths = {} + for vhost_path in self.parser.parser_paths.keys(): + paths = self.aug.match( + ("/files%s//*[label()=~regexp('%s')]" % + (vhost_path, parser.case_i("VirtualHost")))) + for path in paths: + new_vhost = self._create_vhost(path) + realpath = os.path.realpath(new_vhost.filep) + if realpath not in vhost_paths.keys(): + vhs.append(new_vhost) + vhost_paths[realpath] = new_vhost.filep + elif realpath == new_vhost.filep: + # Prefer "real" vhost paths instead of symlinked ones + # ex: sites-enabled/vh.conf -> sites-available/vh.conf - for path in paths: - vhs.append(self._create_vhost(path)) + # remove old (most likely) symlinked one + vhs = [v for v in vhs if v.filep != vhost_paths[realpath]] + vhs.append(new_vhost) + vhost_paths[realpath] = realpath return vhs From 4c02902762dc09b2c2ece0f23ba5da60102f8dad Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 10 Jan 2016 09:01:34 -0800 Subject: [PATCH 062/100] [bootstrap/_deb_common] Re-fix the always-install-backports * This bug seems to come back every time it's fixed :( --- bootstrap/_deb_common.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index 58ea67a45..c2f58db75 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -54,10 +54,10 @@ AddBackportRepo() { echo $BACKPORT_SOURCELINE >> /etc/apt/sources.list.d/"$BACKPORT_NAME".list apt-get update - apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" $augeas_pkg - augeas_pkg= fi fi + apt-get install -y --no-install-recommends -t "$BACKPORT_NAME" $augeas_pkg + augeas_pkg= } From 39e4053b82b4649b6413606c4896c4b2c5cc987a Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 10 Jan 2016 19:15:09 +0200 Subject: [PATCH 063/100] Removed some now obsolete mock code from tests --- .../letsencrypt_apache/tests/configurator_test.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 9838b4f52..212f128f2 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -128,20 +128,10 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(found, 6) # Handle case of non-debian layout get_virtual_hosts - orig_conf = self.config.conf with mock.patch( "letsencrypt_apache.configurator.ApacheConfigurator.conf" - ) as mock_conf: - def conf_sideeffect(key): - """Handle calls to configurator.conf() - :param key: configuration key - :return: configuration value - """ - if key == "handle-sites": - return False - else: - return orig_conf(key) - mock_conf.side_effect = conf_sideeffect + ) as mock_conf: + mock_conf.return_value = False vhs = self.config.get_virtual_hosts() self.assertEqual(len(vhs), 6) From 0a536d50bea24df181788693ff2751411ff6ed4f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 10 Jan 2016 17:31:50 +0000 Subject: [PATCH 064/100] Remove dead code (error in except) --- acme/acme/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index c3e28ef47..7ff740354 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -539,7 +539,7 @@ class ClientNetwork(object): # TODO: response.json() is called twice, once here, and # once in _get and _post clients jobj = response.json() - except ValueError as error: + except ValueError: jobj = None if not response.ok: From fac2ed41d8eaf689e0a565f89a3b5993705165d6 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 10 Jan 2016 18:17:35 +0000 Subject: [PATCH 065/100] ACME: pylint to 80 chars --- acme/.pylintrc | 2 +- acme/acme/challenges_test.py | 16 ++++++++++------ acme/acme/client_test.py | 9 ++++++--- acme/acme/messages.py | 5 +++-- acme/acme/standalone_test.py | 15 ++++++++------- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/acme/.pylintrc b/acme/.pylintrc index 33650310d..d0d150631 100644 --- a/acme/.pylintrc +++ b/acme/.pylintrc @@ -100,7 +100,7 @@ comment=no [FORMAT] # Maximum number of characters on a single line. -max-line-length=100 +max-line-length=80 # Regexp for a line that is allowed to be longer than the limit. ignore-long-lines=^\s*(# )??$ diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 4f2d06167..ef78e1eba 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -73,7 +73,8 @@ class KeyAuthorizationChallengeResponseTest(unittest.TestCase): def test_verify_wrong_form(self): from acme.challenges import KeyAuthorizationChallengeResponse response = KeyAuthorizationChallengeResponse( - key_authorization='.foo.oKGqedy-b-acd5eoybm2f-NVFxvyOoET5CNy3xnv8WY') + key_authorization='.foo.oKGqedy-b-acd5eoybm2f-' + 'NVFxvyOoET5CNy3xnv8WY') self.assertFalse(response.verify(self.chall, KEY.public_key())) @@ -273,10 +274,12 @@ class TLSSNI01ResponseTest(unittest.TestCase): @mock.patch('acme.challenges.TLSSNI01Response.verify_cert', autospec=True) def test_simple_verify(self, mock_verify_cert): mock_verify_cert.return_value = mock.sentinel.verification - self.assertEqual(mock.sentinel.verification, self.response.simple_verify( - self.chall, self.domain, KEY.public_key(), - cert=mock.sentinel.cert)) - mock_verify_cert.assert_called_once_with(self.response, mock.sentinel.cert) + self.assertEqual( + mock.sentinel.verification, self.response.simple_verify( + self.chall, self.domain, KEY.public_key(), + cert=mock.sentinel.cert)) + mock_verify_cert.assert_called_once_with( + self.response, mock.sentinel.cert) @mock.patch('acme.challenges.TLSSNI01Response.probe_cert') def test_simple_verify_false_on_probe_error(self, mock_probe_cert): @@ -590,7 +593,8 @@ class DNSTest(unittest.TestCase): def test_check_validation_wrong_fields(self): bad_validation = jose.JWS.sign( - payload=self.msg.update(token=b'x' * 20).json_dumps().encode('utf-8'), + payload=self.msg.update( + token=b'x' * 20).json_dumps().encode('utf-8'), alg=jose.RS256, key=KEY) self.assertFalse(self.msg.check_validation( bad_validation, KEY.public_key())) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 58f55b293..863fe350f 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -34,8 +34,10 @@ class ClientTest(unittest.TestCase): self.net.get.return_value = self.response self.directory = messages.Directory({ - messages.NewRegistration: 'https://www.letsencrypt-demo.org/acme/new-reg', - messages.Revocation: 'https://www.letsencrypt-demo.org/acme/revoke-cert', + messages.NewRegistration: + 'https://www.letsencrypt-demo.org/acme/new-reg', + messages.Revocation: + 'https://www.letsencrypt-demo.org/acme/revoke-cert', }) from acme.client import Client @@ -331,7 +333,8 @@ class ClientTest(unittest.TestCase): self.assertEqual(clock.dt, datetime.datetime(2015, 3, 27, 0, 1, 7)) # CA sets invalid | TODO: move to a separate test - invalid_authzr = mock.MagicMock(times=[], retries=[messages.STATUS_INVALID]) + invalid_authzr = mock.MagicMock( + times=[], retries=[messages.STATUS_INVALID]) self.assertRaises( errors.PollError, self.client.poll_and_request_issuance, csr, authzrs=(invalid_authzr,), mintime=mintime) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 9c6a5f7b9..06b4492d6 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -130,8 +130,9 @@ class Directory(jose.JSONDeSerializable): @classmethod def register(cls, resource_body_cls): """Register resource.""" - assert resource_body_cls.resource_type not in cls._REGISTERED_TYPES - cls._REGISTERED_TYPES[resource_body_cls.resource_type] = resource_body_cls + resource_type = resource_body_cls.resource_type + assert resource_type not in cls._REGISTERED_TYPES + cls._REGISTERED_TYPES[resource_type] = resource_body_cls return resource_body_cls def __init__(self, jobj): diff --git a/acme/acme/standalone_test.py b/acme/acme/standalone_test.py index 2778635f5..85cd9d11d 100644 --- a/acme/acme/standalone_test.py +++ b/acme/acme/standalone_test.py @@ -32,11 +32,10 @@ class TLSSNI01ServerTest(unittest.TestCase): """Test for acme.standalone.TLSSNI01Server.""" def setUp(self): - self.certs = { - b'localhost': (test_util.load_pyopenssl_private_key('rsa512_key.pem'), - # pylint: disable=protected-access - test_util.load_cert('cert.pem')), - } + self.certs = {b'localhost': ( + test_util.load_pyopenssl_private_key('rsa512_key.pem'), + test_util.load_cert('cert.pem'), + )} from acme.standalone import TLSSNI01Server self.server = TLSSNI01Server(("", 0), certs=self.certs) # pylint: disable=no-member @@ -49,7 +48,8 @@ class TLSSNI01ServerTest(unittest.TestCase): def test_it(self): host, port = self.server.socket.getsockname()[:2] - cert = crypto_util.probe_sni(b'localhost', host=host, port=port, timeout=1) + cert = crypto_util.probe_sni( + b'localhost', host=host, port=port, timeout=1) self.assertEqual(jose.ComparableX509(cert), jose.ComparableX509(self.certs[b'localhost'][1])) @@ -140,7 +140,8 @@ class TestSimpleTLSSNI01Server(unittest.TestCase): while max_attempts: max_attempts -= 1 try: - cert = crypto_util.probe_sni(b'localhost', b'0.0.0.0', self.port) + cert = crypto_util.probe_sni( + b'localhost', b'0.0.0.0', self.port) except errors.Error: self.assertTrue(max_attempts > 0, "Timeout!") time.sleep(1) # wait until thread starts From bdd9fa44854b2e25211fd35425337fdc1d73b6cd Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 10 Jan 2016 18:47:04 +0000 Subject: [PATCH 066/100] Quickfix too-many-instance-attributes. https://github.com/letsencrypt/letsencrypt/pull/2135#issuecomment-170381179 --- acme/acme/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 7ff740354..b65577c53 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -483,7 +483,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes 'Successful revocation must return HTTP OK status') -class ClientNetwork(object): +class ClientNetwork(object): # pylint: disable=too-many-instance-attributes """Client network.""" JSON_CONTENT_TYPE = 'application/json' JSON_ERROR_CONTENT_TYPE = 'application/problem+json' From bf74b2cc644c0ffa1e29f0694b25af214b09bf18 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 19:12:30 +0000 Subject: [PATCH 067/100] Change test RewriteRule so that it conforms with Apaches spec. --- .../letsencrypt_apache/tests/configurator_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 9838b4f52..599334a29 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -857,7 +857,8 @@ class TwoVhost80Test(util.ApacheTest): # Create a preexisting rewrite rule self.config.parser.add_dir( - self.vh_truth[3].path, "RewriteRule", ["Unknown"]) + self.vh_truth[3].path, "RewriteRule", ["UnknownPattern", + "UnknownTarget"]) self.config.save() # This will create an ssl vhost for letsencrypt.demo @@ -872,7 +873,7 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(rw_engine), 1) # three args to rw_rule + 1 arg for the pre existing rewrite - self.assertEqual(len(rw_rule), 4) + self.assertEqual(len(rw_rule), 5) self.assertTrue(rw_engine[0].startswith(self.vh_truth[3].path)) self.assertTrue(rw_rule[0].startswith(self.vh_truth[3].path)) From 6c18a7d318c477ed0ffae1aa3e57f5bd197fa1aa Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 19:15:23 +0000 Subject: [PATCH 068/100] Revise RewriteRule sifting algorithm --- .../letsencrypt_apache/configurator.py | 63 ++++++++++++++++--- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 948514f9b..288ea9dc8 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -696,6 +696,42 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return non_ssl_vh_fp[:-(len(".conf"))] + self.conf("le_vhost_ext") else: return non_ssl_vh_fp + self.conf("le_vhost_ext") + + def _sift_line(self, line): + """ Decides whether a line shouldn't be copied from a http vhost to a + SSL vhost. + + A canonical example of when sifting a line is required: + When the http vhost contains a RewriteRule that unconditionally redirects + any request to the https version of the same site. + e.g: RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent] + Copying the above line to the ssl vhost would cause a redirection loop. + + :param str line: a line extracted from the http vhost config file. + + :returns: True - don't copy line from http vhost to SSL vhost. + :rtype: (bool) + """ + + rewrite_rule = "RewriteRule" + if line.lstrip()[:len(rewrite_rule)] != rewrite_rule: + return False + # line starts with RewriteRule. + + # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html + # The syntax of a RewriteRule is: + # RewriteRule pattern target [Flag1,Flag2,Flag3] + # i.e. target is required, so it must exist. + target = line.split()[2].strip() + + https_prefix = "https://" + if len(target) skeleton. @@ -714,20 +750,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): with open(avail_fp, "r") as orig_file: with open(ssl_fp, "w") as new_file: new_file.write("\n") - - # In some cases we wouldn't want to copy the exact - # directives used in an http vhost to a ssl vhost. - # An example: - # If there's a redirect rewrite rule directive installed in - # the http vhost - copying it to the ssl vhost would cause - # a redirection loop. - blacklist_set = set(['RewriteRule', 'RewriteEngine']) + sift = False for line in orig_file: - line_set = set(line.split()) - if not line_set & blacklist_set: # & -> Intersection + if self._sift_line(line): + if not sift: + new_file.write("# The following rewrite rules" + "were *not* enabled on your HTTPS site, " + "because they have the potential to create " + "redirection loops:") + sift = True + new_file.write("# " + line) + else: new_file.write(line) + new_file.write("\n") + + if sift: + logger.warn("Some rewrite rules were *not* enabled on " + "your HTTPS site, because they have the " + "potential to create redirection loops.") + except IOError: logger.fatal("Error writing/reading to file in make_vhost_ssl") raise errors.PluginError("Unable to write/read in make_vhost_ssl") From ae572fe0840bfc6052c3c86acfe8df7f530a26e7 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 19:20:29 +0000 Subject: [PATCH 069/100] Make lint happy --- .../letsencrypt_apache/configurator.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 288ea9dc8..13c7c91a8 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -696,11 +696,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return non_ssl_vh_fp[:-(len(".conf"))] + self.conf("le_vhost_ext") else: return non_ssl_vh_fp + self.conf("le_vhost_ext") - + def _sift_line(self, line): - """ Decides whether a line shouldn't be copied from a http vhost to a + """ Decides whether a line shouldn't be copied from a http vhost to a SSL vhost. - + A canonical example of when sifting a line is required: When the http vhost contains a RewriteRule that unconditionally redirects any request to the https version of the same site. @@ -709,7 +709,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param str line: a line extracted from the http vhost config file. - :returns: True - don't copy line from http vhost to SSL vhost. + :returns: True - don't copy line from http vhost to SSL vhost. :rtype: (bool) """ @@ -718,14 +718,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return False # line starts with RewriteRule. - # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html + # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html # The syntax of a RewriteRule is: # RewriteRule pattern target [Flag1,Flag2,Flag3] - # i.e. target is required, so it must exist. + # i.e. target is required, so it must exist. target = line.split()[2].strip() https_prefix = "https://" - if len(target) Date: Mon, 11 Jan 2016 19:48:17 +0000 Subject: [PATCH 070/100] Dequote possible quoted target --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 13c7c91a8..383db3d98 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -716,6 +716,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): rewrite_rule = "RewriteRule" if line.lstrip()[:len(rewrite_rule)] != rewrite_rule: return False + # line starts with RewriteRule. # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html @@ -723,6 +724,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # RewriteRule pattern target [Flag1,Flag2,Flag3] # i.e. target is required, so it must exist. target = line.split()[2].strip() + + # target may be surrounded with double quotes + if len(target) > 0 and target[0]==target[-1]=="\"" + target = target[1:-1] https_prefix = "https://" if len(target) < len(https_prefix): From a43e7b11f1d926c4bfe89a402b8597a1fde8fc4c Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 19:55:15 +0000 Subject: [PATCH 071/100] Add colon --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 383db3d98..af00e7527 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -726,7 +726,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): target = line.split()[2].strip() # target may be surrounded with double quotes - if len(target) > 0 and target[0]==target[-1]=="\"" + if len(target) > 0 and target[0]==target[-1]=="\"": target = target[1:-1] https_prefix = "https://" From 9c2a0362a763a0aa804748de389650051d351a6c Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 19:55:55 +0000 Subject: [PATCH 072/100] Add rewrite tests: normal, small, quoted, etc. --- .../letsencrypt_apache/tests/configurator_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 599334a29..78714b881 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -922,6 +922,16 @@ class TwoVhost80Test(util.ApacheTest): self.config._enable_redirect(self.vh_truth[1], "") # pylint: disable=protected-access self.assertEqual(len(self.config.vhosts), 7) + def test_sift_line(self): + # pylint: disable=protected-access + small_quoted_target = "RewriteRule ^ \"http://\"" + self.assertFalse(self.config._sift_line(small_quoted_target)) + + https_target = "RewriteRule ^ https://satoshi" + self.assertTrue(self.config._sift_line(https_target)) + + normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]" + self.assertFalse(self.config._sift_line(normal_target)) def get_achalls(self): """Return testing achallenges.""" From c89dcad313e933630e1b8e7b81b4878ebc4c5534 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 11 Jan 2016 12:22:22 -0800 Subject: [PATCH 073/100] This default shouldn't be a magic string --- letsencrypt/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index f7e5c3ad3..08b48ff5e 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -565,7 +565,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return True # Renews some period before expiry time - interval = self.configuration.get("renew_before_expiry", "30 days") + default_interval = constants.RENEWER_DEFAULTS["renew_before_expiry"] + interval = self.configuration.get("renew_before_expiry", default_interval) expiry = crypto_util.notAfter(self.version( "cert", self.latest_common_version())) now = pytz.UTC.fromutc(datetime.datetime.utcnow()) From 4645bf8329f24b59c7be742c5ad4befed5dd3037 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 20:58:52 +0000 Subject: [PATCH 074/100] Make lint happy --- .../letsencrypt_apache/configurator.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index af00e7527..16f264747 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -724,9 +724,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # RewriteRule pattern target [Flag1,Flag2,Flag3] # i.e. target is required, so it must exist. target = line.split()[2].strip() - + # target may be surrounded with double quotes - if len(target) > 0 and target[0]==target[-1]=="\"": + if len(target) > 0 and target[0] == target[-1] == "\"": target = target[1:-1] https_prefix = "https://" @@ -760,10 +760,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for line in orig_file: if self._sift_line(line): if not sift: - new_file.write("# The following rewrite rules" - "were *not* enabled on your HTTPS site, " - "because they have the potential to create " - "redirection loops:") + new_file.write("# The following rewrite rules " + "were *not* enabled on your HTTPS site,\n" + "# because they have the potential to create " + "redirection loops:\n") sift = True new_file.write("# " + line) else: From b28b5b08d7f325f7bd089b47250450788240e670 Mon Sep 17 00:00:00 2001 From: sagi Date: Mon, 11 Jan 2016 20:59:19 +0000 Subject: [PATCH 075/100] More tests; Make Nose happy --- .../tests/configurator_test.py | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 78714b881..954560aa6 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -930,9 +930,35 @@ class TwoVhost80Test(util.ApacheTest): https_target = "RewriteRule ^ https://satoshi" self.assertTrue(self.config._sift_line(https_target)) - normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]" + normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]" self.assertFalse(self.config._sift_line(normal_target)) + def test_make_vhost_ssl_with_http_vhost_redirect_rewrite_rule(self): + self.config.parser.modules.add("rewrite_module") + + http_vhost = self.vh_truth[0] + + self.config.parser.add_dir( + http_vhost.path, "RewriteEngine", "on") + + self.config.parser.add_dir( + http_vhost.path, "RewriteRule", + ["^", + "https://%{SERVER_NAME}%{REQUEST_URI}", + "[L,QSA,R=permanent]"]) + self.config.save() + + ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) + + #import ipdb; ipdb.set_trace() + self.assertTrue(self.config.parser.find_dir( + "RewriteEngine", "on", ssl_vhost.path, False)) + + conf_text = open(ssl_vhost.filep).read() + commented_rewrite_rule = \ + "# RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent]" + self.assertTrue(commented_rewrite_rule in conf_text) + def get_achalls(self): """Return testing achallenges.""" account_key = self.rsa512jwk From 10df56bab6b5d7a6fb4ee791b1239b149f67ee6a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 11 Jan 2016 18:21:33 -0800 Subject: [PATCH 076/100] Added revisions --- .../letsencrypt_apache/configurator.py | 63 +++++++++---------- .../tests/configurator_test.py | 6 +- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 16f264747..de179966a 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -8,6 +8,7 @@ import shutil import socket import time +import zope.component import zope.interface from acme import challenges @@ -698,42 +699,36 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return non_ssl_vh_fp + self.conf("le_vhost_ext") def _sift_line(self, line): - """ Decides whether a line shouldn't be copied from a http vhost to a - SSL vhost. + """Decides whether a line should be copied to a SSL vhost. A canonical example of when sifting a line is required: - When the http vhost contains a RewriteRule that unconditionally redirects - any request to the https version of the same site. - e.g: RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent] - Copying the above line to the ssl vhost would cause a redirection loop. + When the http vhost contains a RewriteRule that unconditionally + redirects any request to the https version of the same site. + e.g: + RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent] + Copying the above line to the ssl vhost would cause a + redirection loop. - :param str line: a line extracted from the http vhost config file. + :param str line: a line extracted from the http vhost. :returns: True - don't copy line from http vhost to SSL vhost. - :rtype: (bool) + :rtype: bool + """ - - rewrite_rule = "RewriteRule" - if line.lstrip()[:len(rewrite_rule)] != rewrite_rule: + if not line.lstrip().startswith("RewriteRule"): return False - # line starts with RewriteRule. - # According to: http://httpd.apache.org/docs/2.4/rewrite/flags.html # The syntax of a RewriteRule is: # RewriteRule pattern target [Flag1,Flag2,Flag3] # i.e. target is required, so it must exist. target = line.split()[2].strip() - # target may be surrounded with double quotes - if len(target) > 0 and target[0] == target[-1] == "\"": + # target may be surrounded with quotes + if target[0] in ("'", '"') and target[0] == target[-1]: target = target[1:-1] - https_prefix = "https://" - if len(target) < len(https_prefix): - return False - - if target[:len(https_prefix)] == https_prefix: + if target.startswith("https://"): return True return False @@ -750,36 +745,38 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # First register the creation so that it is properly removed if # configuration is rolled back self.reverter.register_file_creation(False, ssl_fp) + sift = False try: with open(avail_fp, "r") as orig_file: with open(ssl_fp, "w") as new_file: new_file.write("\n") - sift = False - for line in orig_file: if self._sift_line(line): if not sift: - new_file.write("# The following rewrite rules " - "were *not* enabled on your HTTPS site,\n" - "# because they have the potential to create " - "redirection loops:\n") + new_file.write( + "# Some rewrite rules in this file were " + "were disabled on your HTTPS site,\n" + "# because they have the potential to " + "create redirection loops.\n") sift = True new_file.write("# " + line) else: new_file.write(line) - new_file.write("\n") - - if sift: - logger.warn("Some rewrite rules were *not* enabled on " - "your HTTPS site, because they have the " - "potential to create redirection loops.") - except IOError: logger.fatal("Error writing/reading to file in make_vhost_ssl") raise errors.PluginError("Unable to write/read in make_vhost_ssl") + if sift: + reporter = zope.component.getUtility(interfaces.IReporter) + reporter.add_message( + "Some rewrite rules copied from {0} were disabled in the " + "vhost for your HTTPS site located at {1} because they have " + "the potential to create redirection loops.".format(avail_fp, + ssl_fp), + reporter.MEDIUM_PRIORITY) + def _update_ssl_vhosts_addrs(self, vh_path): ssl_addrs = set() ssl_addr_p = self.aug.match(vh_path + "/arg") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 954560aa6..5905e281b 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -933,7 +933,8 @@ class TwoVhost80Test(util.ApacheTest): normal_target = "RewriteRule ^/(.*) http://www.a.com:1234/$1 [L,R]" self.assertFalse(self.config._sift_line(normal_target)) - def test_make_vhost_ssl_with_http_vhost_redirect_rewrite_rule(self): + @mock.patch("letsencrypt_apache.configurator.zope.component.getUtility") + def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility): self.config.parser.modules.add("rewrite_module") http_vhost = self.vh_truth[0] @@ -950,7 +951,6 @@ class TwoVhost80Test(util.ApacheTest): ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) - #import ipdb; ipdb.set_trace() self.assertTrue(self.config.parser.find_dir( "RewriteEngine", "on", ssl_vhost.path, False)) @@ -958,6 +958,8 @@ class TwoVhost80Test(util.ApacheTest): commented_rewrite_rule = \ "# RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent]" self.assertTrue(commented_rewrite_rule in conf_text) + mock_get_utility().add_message.assert_called_once_with(mock.ANY, + mock.ANY) def get_achalls(self): """Return testing achallenges.""" From 4cdf63c55e971929f3c14b2c940c6cfc3c9c19f1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 11 Jan 2016 18:27:01 -0800 Subject: [PATCH 077/100] Fix a couple nits --- letsencrypt-apache/letsencrypt_apache/configurator.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 9b1b3a961..4066d6264 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -740,10 +740,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if target[0] in ("'", '"') and target[0] == target[-1]: target = target[1:-1] - if target.startswith("https://"): - return True - - return False + # Sift line if it redirects the request to a HTTPS site + return target.startswith("https://") def _copy_create_ssl_vhost_skeleton(self, avail_fp, ssl_fp): """Copies over existing Vhost with IfModule mod_ssl.c> skeleton. @@ -771,7 +769,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "were disabled on your HTTPS site,\n" "# because they have the potential to " "create redirection loops.\n") - sift = True + sift = True new_file.write("# " + line) else: new_file.write(line) From 99c575f04366d76cb195d7cd912714f3a0be7ede Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 13 Jan 2016 23:56:22 +0200 Subject: [PATCH 078/100] Check augeas version, and raise error if not recent enough --- .../letsencrypt_apache/configurator.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 4066d6264..00b93bb6e 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -158,6 +158,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.NotSupportedError( "Apache Version %s not supported.", str(self.version)) + if not self._check_aug_version(): + raise errors.NotSupportedError( + "Your libaugeas0 is outdated, upgrade it from backports " + " or re-bootstrap letsencrypt") + self.parser = parser.ApacheParser( self.aug, self.conf("server-root"), self.conf("vhost-root"), self.version) @@ -169,6 +174,17 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): install_ssl_options_conf(self.mod_ssl_conf) + def _check_aug_version(self): + """ Checks that we have recent enough version of libaugeas. + If augeas version is recent enough, it will support case insensitive + regexp matching""" + + self.aug.set("/test/path/testing/arg", "aRgUMeNT") + matches = self.aug.match( + "/test//*[self::arg=~regexp('argument', 'i')]") + self.aug.remove("/test/path") + return matches + def deploy_cert(self, domain, cert_path, key_path, chain_path=None, fullchain_path=None): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. From 30ad7dce9fb74f1fd8a401d16641fe8b49b1e06d Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 00:06:52 +0200 Subject: [PATCH 079/100] Pick up the augeas RuntimeError and pass the correct one --- letsencrypt-apache/letsencrypt_apache/configurator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 00b93bb6e..0c4d65c28 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -180,8 +180,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): regexp matching""" self.aug.set("/test/path/testing/arg", "aRgUMeNT") - matches = self.aug.match( - "/test//*[self::arg=~regexp('argument', 'i')]") + try: + matches = self.aug.match( + "/test//*[self::arg=~regexp('argument', 'i')]") + except RuntimeError: + return None self.aug.remove("/test/path") return matches From 7d51480c4dd8610c40d8e4aa4e78024e12e46e63 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 00:23:45 +0200 Subject: [PATCH 080/100] Remove the test path from augeas even if failing --- letsencrypt-apache/letsencrypt_apache/configurator.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 0c4d65c28..72db9c853 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -184,6 +184,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): matches = self.aug.match( "/test//*[self::arg=~regexp('argument', 'i')]") except RuntimeError: + self.aug.remove("/test/path") return None self.aug.remove("/test/path") return matches From ddbfb440412187447d4d51f0f7ac03025954aebf Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 00:50:34 +0200 Subject: [PATCH 081/100] Add tests --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- .../letsencrypt_apache/tests/configurator_test.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 72db9c853..ad407a3bc 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -185,7 +185,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "/test//*[self::arg=~regexp('argument', 'i')]") except RuntimeError: self.aug.remove("/test/path") - return None + return False self.aug.remove("/test/path") return matches diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 113b3b2b2..ce3d20297 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -978,6 +978,13 @@ class TwoVhost80Test(util.ApacheTest): self.assertTrue(self.config.parser.find_dir( "NameVirtualHost", "*:443", exclude=False)) + def test_aug_version(self): + mock_match = mock.Mock(return_value=["something"]) + self.config.aug.match = mock_match + self.assertEquals(self.config._check_aug_version(), ["something"]) + self.config.aug.match.side_effect = RuntimeError + self.assertFalse(self.config._check_aug_version()) + if __name__ == "__main__": unittest.main() # pragma: no cover From d0832f741462cfa94046d7ed7a81835a8836e1e8 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 01:09:28 +0200 Subject: [PATCH 082/100] Added the missing test --- .../letsencrypt_apache/tests/configurator_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index ce3d20297..fe7071f14 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -65,6 +65,15 @@ class TwoVhost80Test(util.ApacheTest): self.assertRaises( errors.NotSupportedError, self.config.prepare) + @mock.patch("letsencrypt_apache.parser.ApacheParser") + @mock.patch("letsencrypt_apache.configurator.le_util.exe_exists") + def test_prepare_old_aug(self, mock_exe_exists, _): + mock_exe_exists.return_value = True + self.config._check_aug_version = mock.Mock(return_value=False) + self.assertRaises( + errors.NotSupportedError, self.config.prepare) + + def test_add_parser_arguments(self): # pylint: disable=no-self-use from letsencrypt_apache.configurator import ApacheConfigurator # Weak test.. From 8f6ef8db5301ce4087a1de86eeaaaeda805f93f9 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 01:10:50 +0200 Subject: [PATCH 083/100] Modified error message --- letsencrypt-apache/letsencrypt_apache/configurator.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index ad407a3bc..47d238140 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -160,8 +160,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if not self._check_aug_version(): raise errors.NotSupportedError( - "Your libaugeas0 is outdated, upgrade it from backports " - " or re-bootstrap letsencrypt") + "Apache plugin support requires libaugeas0 and augeas-lenses " + "version 1.2.0 or higher, please make sure you have you have " + "those installed.") self.parser = parser.ApacheParser( self.aug, self.conf("server-root"), self.conf("vhost-root"), From 8c110e31d75a746ec6bb284bba61179140d445a9 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 01:30:34 +0200 Subject: [PATCH 084/100] Fixed tests --- .../letsencrypt_apache/tests/configurator_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index fe7071f14..5052da893 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -69,11 +69,11 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt_apache.configurator.le_util.exe_exists") def test_prepare_old_aug(self, mock_exe_exists, _): mock_exe_exists.return_value = True + self.config.config_test = mock.Mock() self.config._check_aug_version = mock.Mock(return_value=False) self.assertRaises( errors.NotSupportedError, self.config.prepare) - def test_add_parser_arguments(self): # pylint: disable=no-self-use from letsencrypt_apache.configurator import ApacheConfigurator # Weak test.. From fe8a0dcef26c15480ce0ae86f8f24450836e14a6 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 01:52:35 +0200 Subject: [PATCH 085/100] Make linter happy --- .../letsencrypt_apache/tests/configurator_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 5052da893..2a32b04be 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -70,7 +70,7 @@ class TwoVhost80Test(util.ApacheTest): def test_prepare_old_aug(self, mock_exe_exists, _): mock_exe_exists.return_value = True self.config.config_test = mock.Mock() - self.config._check_aug_version = mock.Mock(return_value=False) + self.config._check_aug_version = mock.Mock(return_value=False) # pylint: disable=protected-access self.assertRaises( errors.NotSupportedError, self.config.prepare) @@ -990,9 +990,9 @@ class TwoVhost80Test(util.ApacheTest): def test_aug_version(self): mock_match = mock.Mock(return_value=["something"]) self.config.aug.match = mock_match - self.assertEquals(self.config._check_aug_version(), ["something"]) + self.assertEquals(self.config._check_aug_version(), ["something"]) # pylint: disable=protected-access self.config.aug.match.side_effect = RuntimeError - self.assertFalse(self.config._check_aug_version()) + self.assertFalse(self.config._check_aug_version()) # pylint: disable=protected-access if __name__ == "__main__": From 8989dfc1ff7533fb961c237eadbee76cae70c049 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 13 Jan 2016 16:17:26 -0800 Subject: [PATCH 086/100] Disable Apache 2.2 support --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 4066d6264..2ce9d008b 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -154,7 +154,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Set Version if self.version is None: self.version = self.get_version() - if self.version < (2, 2): + if self.version < (2, 4): raise errors.NotSupportedError( "Apache Version %s not supported.", str(self.version)) From a7b878b825e73b0d05abd0495d40eded7ae2d608 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 13 Jan 2016 16:51:13 -0800 Subject: [PATCH 087/100] Ensure that all pip upload version #s are reflect in git --- tools/release.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/release.sh b/tools/release.sh index 172f6fea1..a945e3970 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -85,9 +85,12 @@ SetVersion() { sed -i "s/^version.*/version = '$ver'/" $pkg_dir/setup.py done sed -i "s/^__version.*/__version__ = '$ver'/" letsencrypt/__init__.py + + # interactive user input + git add -p letsencrypt $SUBPKGS letsencrypt-compatibility-test - git add -p letsencrypt $SUBPKGS # interactive user input } + SetVersion "$version" git commit --gpg-sign="$RELEASE_GPG_KEY" -m "Release $version" git tag --local-user "$RELEASE_GPG_KEY" \ From 4762ede4ea901abcc6ff0240fc84d7ba80763987 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 13 Jan 2016 17:09:45 -0800 Subject: [PATCH 088/100] Also *set* the letsencrypt-compatibility-test version number --- tools/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/release.sh b/tools/release.sh index a945e3970..43c7556de 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -80,7 +80,7 @@ git checkout "$RELEASE_BRANCH" SetVersion() { ver="$1" - for pkg_dir in $SUBPKGS + for pkg_dir in $SUBPKGS letsencrypt-compatibility-test do sed -i "s/^version.*/version = '$ver'/" $pkg_dir/setup.py done From be1d1321b65527841eb85353aeb350a07101c369 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 13:25:15 +0200 Subject: [PATCH 089/100] Pep8 love --- .../letsencrypt_apache/augeas_configurator.py | 3 +- .../letsencrypt_apache/configurator.py | 97 +++++++----- .../letsencrypt_apache/constants.py | 9 +- .../letsencrypt_apache/display_ops.py | 5 +- .../letsencrypt_apache/parser.py | 18 +-- .../tests/complex_parsing_test.py | 6 +- .../tests/configurator_test.py | 142 ++++++++++-------- .../letsencrypt_apache/tests/obj_test.py | 3 +- .../letsencrypt_apache/tests/parser_test.py | 3 +- .../tests/tls_sni_01_test.py | 13 +- .../letsencrypt_apache/tests/util.py | 9 +- .../letsencrypt_apache/tls_sni_01.py | 4 +- 12 files changed, 182 insertions(+), 130 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py index 9e0948f12..9b51c32a9 100644 --- a/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/augeas_configurator.py @@ -120,7 +120,8 @@ class AugeasConfigurator(common.Plugin): self.reverter.add_to_temp_checkpoint( save_files, self.save_notes) else: - self.reverter.add_to_checkpoint(save_files, self.save_notes) + self.reverter.add_to_checkpoint(save_files, + self.save_notes) except errors.ReverterError as err: raise errors.PluginError(str(err)) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index bfc6a566c..2d822b3a1 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -133,7 +133,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): @property def mod_ssl_conf(self): """Full absolute path to SSL configuration file.""" - return os.path.join(self.config.config_dir, constants.MOD_SSL_CONF_DEST) + return os.path.join(self.config.config_dir, + constants.MOD_SSL_CONF_DEST) def prepare(self): """Prepare the authenticator/installer. @@ -191,15 +192,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return matches def deploy_cert(self, domain, cert_path, key_path, - chain_path=None, fullchain_path=None): # pylint: disable=unused-argument + chain_path=None, fullchain_path=None): """Deploys certificate to specified virtual host. Currently tries to find the last directives to deploy the cert in the VHost associated with the given domain. If it can't find the - directives, it searches the "included" confs. The function verifies that - it has located the three directives and finally modifies them to point - to the correct destination. After the certificate is installed, the - VirtualHost is enabled if it isn't already. + directives, it searches the "included" confs. The function verifies + that it has located the three directives and finally modifies them + to point to the correct destination. After the certificate is + installed, the VirtualHost is enabled if it isn't already. .. todo:: Might be nice to remove chain directive if none exists This shouldn't happen within letsencrypt though @@ -215,8 +216,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # cert_key... can all be parsed appropriately self.prepare_server_https("443") - path = {"cert_path": self.parser.find_dir("SSLCertificateFile", None, vhost.path), - "cert_key": self.parser.find_dir("SSLCertificateKeyFile", None, vhost.path)} + path = {"cert_path": self.parser.find_dir("SSLCertificateFile", + None, vhost.path), + "cert_key": self.parser.find_dir("SSLCertificateKeyFile", + None, vhost.path)} # Only include if a certificate chain is specified if chain_path is not None: @@ -246,7 +249,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser.add_dir(vhost.path, "SSLCertificateChainFile", chain_path) else: - raise errors.PluginError("--chain-path is required for your version of Apache") + raise errors.PluginError("--chain-path is required for your " + "version of Apache") else: if not fullchain_path: raise errors.PluginError("Please provide the --fullchain-path\ @@ -320,7 +324,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): elif not vhost.ssl: addrs = self._get_proposed_addrs(vhost, "443") # TODO: Conflicts is too conservative - if not any(vhost.enabled and vhost.conflicts(addrs) for vhost in self.vhosts): + if not any(vhost.enabled and vhost.conflicts(addrs) for + vhost in self.vhosts): vhost = self.make_vhost_ssl(vhost) else: logger.error( @@ -588,15 +593,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.prepare_https_modules(temp) # Check for Listen # Note: This could be made to also look for ip:443 combo - listens = [self.parser.get_arg(x).split()[0] for x in self.parser.find_dir("Listen")] + listens = [self.parser.get_arg(x).split()[0] for + x in self.parser.find_dir("Listen")] # In case no Listens are set (which really is a broken apache config) if not listens: listens = ["80"] if port in listens: return for listen in listens: - # For any listen statement, check if the machine also listens on Port 443. - # If not, add such a listen statement. + # For any listen statement, check if the machine also listens on + # Port 443. If not, add such a listen statement. if len(listen.split(":")) == 1: # Its listening to all interfaces if port not in listens: @@ -624,8 +630,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser.add_dir_to_ifmodssl( parser.get_aug_path( self.parser.loc["listen"]), "Listen", args) - self.save_notes += "Added Listen %s:%s directive to %s\n" % ( - ip, port, self.parser.loc["listen"]) + self.save_notes += ("Added Listen %s:%s directive to " + "%s\n") % (ip, port, + self.parser.loc["listen"]) listens.append("%s:%s" % (ip, port)) def prepare_https_modules(self, temp): @@ -824,20 +831,25 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def _clean_vhost(self, vhost): # remove duplicated or conflicting ssl directives self._deduplicate_directives(vhost.path, - ["SSLCertificateFile", "SSLCertificateKeyFile"]) + ["SSLCertificateFile", + "SSLCertificateKeyFile"]) # remove all problematic directives self._remove_directives(vhost.path, ["SSLCertificateChainFile"]) def _deduplicate_directives(self, vh_path, directives): for directive in directives: - while len(self.parser.find_dir(directive, None, vh_path, False)) > 1: - directive_path = self.parser.find_dir(directive, None, vh_path, False) + while len(self.parser.find_dir(directive, None, + vh_path, False)) > 1: + directive_path = self.parser.find_dir(directive, None, + vh_path, False) self.aug.remove(re.sub(r"/\w*$", "", directive_path[0])) def _remove_directives(self, vh_path, directives): for directive in directives: - while len(self.parser.find_dir(directive, None, vh_path, False)) > 0: - directive_path = self.parser.find_dir(directive, None, vh_path, False) + while len(self.parser.find_dir(directive, None, + vh_path, False)) > 0: + directive_path = self.parser.find_dir(directive, None, + vh_path, False) self.aug.remove(re.sub(r"/\w*$", "", directive_path[0])) def _add_dummy_ssl_directives(self, vh_path): @@ -864,7 +876,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): for addr in vhost.addrs: for test_vh in self.vhosts: if (vhost.filep != test_vh.filep and - any(test_addr == addr for test_addr in test_vh.addrs) and + any(test_addr == addr for + test_addr in test_vh.addrs) and not self.is_name_vhost(addr)): self.add_name_vhost(addr) logger.info("Enabling NameVirtualHosts on %s", addr) @@ -873,9 +886,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if need_to_save: self.save() - ############################################################################ + ###################################################################### # Enhancements - ############################################################################ + ###################################################################### def supported_enhancements(self): # pylint: disable=no-self-use """Returns currently supported enhancements.""" return ["redirect", "ensure-http-header"] @@ -936,14 +949,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Add directives to server self.parser.add_dir(ssl_vhost.path, "Header", - constants.HEADER_ARGS[header_substring]) + constants.HEADER_ARGS[header_substring]) self.save_notes += ("Adding %s header to ssl vhost in %s\n" % - (header_substring, ssl_vhost.filep)) + (header_substring, ssl_vhost.filep)) self.save() logger.info("Adding %s header to ssl vhost in %s", header_substring, - ssl_vhost.filep) + ssl_vhost.filep) def _verify_no_matching_http_header(self, ssl_vhost, header_substring): """Checks to see if an there is an existing Header directive that @@ -963,14 +976,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): header_substring exists """ - header_path = self.parser.find_dir("Header", None, start=ssl_vhost.path) + header_path = self.parser.find_dir("Header", None, + start=ssl_vhost.path) if header_path: # "Existing Header directive for virtualhost" pat = '(?:[ "]|^)(%s)(?:[ "]|$)' % (header_substring.lower()) for match in header_path: if re.search(pat, self.aug.get(match).lower()): raise errors.PluginEnhancementAlreadyPresent( - "Existing %s header" % (header_substring)) + "Existing %s header" % (header_substring)) def _enable_redirect(self, ssl_vhost, unused_options): """Redirect all equivalent HTTP traffic to ssl_vhost. @@ -1019,7 +1033,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Check if LetsEncrypt redirection already exists self._verify_no_letsencrypt_redirect(general_vh) - # Note: if code flow gets here it means we didn't find the exact # letsencrypt RewriteRule config for redirection. Finding # another RewriteRule is likely to be fine in most or all cases, @@ -1038,10 +1051,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if self.get_version() >= (2, 3, 9): self.parser.add_dir(general_vh.path, "RewriteRule", - constants.REWRITE_HTTPS_ARGS_WITH_END) + constants.REWRITE_HTTPS_ARGS_WITH_END) else: self.parser.add_dir(general_vh.path, "RewriteRule", - constants.REWRITE_HTTPS_ARGS) + constants.REWRITE_HTTPS_ARGS) self.save_notes += ("Redirecting host in %s to ssl vhost in %s\n" % (general_vh.filep, ssl_vhost.filep)) @@ -1063,7 +1076,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): letsencrypt redirection WriteRule exists in virtual host. """ rewrite_path = self.parser.find_dir( - "RewriteRule", None, start=vhost.path) + "RewriteRule", None, start=vhost.path) # There can be other RewriteRule directive lines in vhost config. # rewrite_args_dict keys are directive ids and the corresponding value @@ -1078,12 +1091,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if rewrite_args_dict: redirect_args = [constants.REWRITE_HTTPS_ARGS, - constants.REWRITE_HTTPS_ARGS_WITH_END] + constants.REWRITE_HTTPS_ARGS_WITH_END] for matches in rewrite_args_dict.values(): if [self.aug.get(x) for x in matches] in redirect_args: raise errors.PluginEnhancementAlreadyPresent( - "Let's Encrypt has already enabled redirection") + "Let's Encrypt has already enabled redirection") def _is_rewrite_exists(self, vhost): """Checks if there exists a RewriteRule directive in vhost @@ -1096,7 +1109,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ rewrite_path = self.parser.find_dir( - "RewriteRule", None, start=vhost.path) + "RewriteRule", None, start=vhost.path) return bool(rewrite_path) def _is_rewrite_engine_on(self, vhost): @@ -1107,7 +1120,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ rewrite_engine_path = self.parser.find_dir("RewriteEngine", "on", - start=vhost.path) + start=vhost.path) if rewrite_engine_path: return self.parser.get_arg(rewrite_engine_path[0]) return False @@ -1153,7 +1166,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): else: rewrite_rule_args = constants.REWRITE_HTTPS_ARGS - return ("\n" "%s \n" "%s \n" @@ -1165,7 +1177,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "ErrorLog /var/log/apache2/redirect.error.log\n" "LogLevel warn\n" "\n" - % (" ".join(str(addr) for addr in self._get_proposed_addrs(ssl_vhost)), + % (" ".join(str(addr) for + addr in self._get_proposed_addrs(ssl_vhost)), servername, serveralias, " ".join(rewrite_rule_args))) @@ -1179,7 +1192,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if len(ssl_vhost.name) < (255 - (len(redirect_filename) + 1)): redirect_filename = "le-redirect-%s.conf" % ssl_vhost.name - redirect_filepath = os.path.join(self.conf("vhost-root"), redirect_filename) + redirect_filepath = os.path.join(self.conf("vhost-root"), + redirect_filename) # Register the new file that will be created # Note: always register the creation before writing to ensure file will @@ -1207,7 +1221,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return None - def _get_proposed_addrs(self, vhost, port="80"): # pylint: disable=no-self-use + def _get_proposed_addrs(self, vhost, port="80"): """Return all addrs of vhost with the port replaced with the specified. :param obj.VirtualHost ssl_vhost: Original Vhost @@ -1287,7 +1301,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): .. note:: Does not make sure that the site correctly works or that all modules are enabled appropriately. - .. todo:: This function should number subdomains before the domain vhost + .. todo:: This function should number subdomains before the domain + vhost .. todo:: Make sure link is not broken... diff --git a/letsencrypt-apache/letsencrypt_apache/constants.py b/letsencrypt-apache/letsencrypt_apache/constants.py index 8ac88b197..fe5ef3335 100644 --- a/letsencrypt-apache/letsencrypt_apache/constants.py +++ b/letsencrypt-apache/letsencrypt_apache/constants.py @@ -73,7 +73,8 @@ AUGEAS_LENS_DIR = pkg_resources.resource_filename( REWRITE_HTTPS_ARGS = [ "^", "https://%{SERVER_NAME}%{REQUEST_URI}", "[L,QSA,R=permanent]"] -"""Apache version<2.3.9 rewrite rule arguments used for redirections to https vhost""" +"""Apache version<2.3.9 rewrite rule arguments used for redirections to +https vhost""" REWRITE_HTTPS_ARGS_WITH_END = [ "^", "https://%{SERVER_NAME}%{REQUEST_URI}", "[END,QSA,R=permanent]"] @@ -81,14 +82,14 @@ REWRITE_HTTPS_ARGS_WITH_END = [ https vhost""" HSTS_ARGS = ["always", "set", "Strict-Transport-Security", - "\"max-age=31536000\""] + "\"max-age=31536000\""] """Apache header arguments for HSTS""" UIR_ARGS = ["always", "set", "Content-Security-Policy", - "upgrade-insecure-requests"] + "upgrade-insecure-requests"] HEADER_ARGS = {"Strict-Transport-Security": HSTS_ARGS, - "Upgrade-Insecure-Requests": UIR_ARGS} + "Upgrade-Insecure-Requests": UIR_ARGS} def os_constant(key): diff --git a/letsencrypt-apache/letsencrypt_apache/display_ops.py b/letsencrypt-apache/letsencrypt_apache/display_ops.py index 45c55f49a..d05a995ba 100644 --- a/letsencrypt-apache/letsencrypt_apache/display_ops.py +++ b/letsencrypt-apache/letsencrypt_apache/display_ops.py @@ -79,8 +79,9 @@ def _vhost_menu(domain, vhosts): ) code, tag = zope.component.getUtility(interfaces.IDisplay).menu( - "We were unable to find a vhost with a ServerName or Address of {0}.{1}" - "Which virtual host would you like to choose?".format( + "We were unable to find a vhost with a ServerName " + "or Address of {0}.{1}Which virtual host would you " + "like to choose?".format( domain, os.linesep), choices, help_label="More Info", ok_label="Select") diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 82effad2b..cc7f2ec42 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -96,11 +96,12 @@ class ApacheParser(object): def update_runtime_variables(self): """" - .. note:: Compile time variables (apache2ctl -V) are not used within the - dynamic configuration files. These should not be parsed or + .. note:: Compile time variables (apache2ctl -V) are not used within + the dynamic configuration files. These should not be parsed or interpreted. - .. todo:: Create separate compile time variables... simply for arg_get() + .. todo:: Create separate compile time variables... + simply for arg_get() """ stdout = self._get_runtime_cfg() @@ -177,7 +178,8 @@ class ApacheParser(object): # Make sure we don't cause an IndexError (end of list) # Check to make sure arg + 1 doesn't exist if (i == (len(matches) - 1) or - not matches[i + 1].endswith("/arg[%d]" % (args + 1))): + not matches[i + 1].endswith("/arg[%d]" % + (args + 1))): filtered.append(matches[i][:-len("/arg[%d]" % args)]) return filtered @@ -311,8 +313,6 @@ class ApacheParser(object): for match in matches: dir_ = self.aug.get(match).lower() if dir_ == "include" or dir_ == "includeoptional": - # start[6:] to strip off /files - #print self._get_include_path(self.get_arg(match +"/arg")), directive, arg ordered_matches.extend(self.find_dir( directive, arg, self._get_include_path(self.get_arg(match + "/arg")), @@ -331,8 +331,8 @@ class ApacheParser(object): """ value = self.aug.get(match) - # No need to strip quotes for variables, as apache2ctl already does this - # but we do need to strip quotes for all normal arguments. + # No need to strip quotes for variables, as apache2ctl already does + # this, but we do need to strip quotes for all normal arguments. # Note: normal argument may be a quoted variable # e.g. strip now, not later @@ -454,7 +454,7 @@ class ApacheParser(object): https://apr.apache.org/docs/apr/2.0/apr__fnmatch_8h_source.html http://apache2.sourcearchive.com/documentation/2.2.16-6/apr__fnmatch_8h_source.html - :param str clean_fn_match: Apache style filename match, similar to globs + :param str clean_fn_match: Apache style filename match, like globs :returns: regex suitable for augeas :rtype: str diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 7099c388f..1fc5281c1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -96,7 +96,8 @@ class ComplexParserTest(util.ParserTest): else: self.assertFalse(self.parser.find_dir("FNMATCH_DIRECTIVE")) - # NOTE: Only run one test per function otherwise you will have inf recursion + # NOTE: Only run one test per function otherwise you will have + # inf recursion def test_include(self): self.verify_fnmatch("test_fnmatch.?onf") @@ -104,7 +105,8 @@ class ComplexParserTest(util.ParserTest): self.verify_fnmatch("../complex_parsing/[te][te]st_*.?onf") def test_include_fullpath(self): - self.verify_fnmatch(os.path.join(self.config_path, "test_fnmatch.conf")) + self.verify_fnmatch(os.path.join(self.config_path, + "test_fnmatch.conf")) def test_include_fullpath_trailing_slash(self): self.verify_fnmatch(self.config_path + "//") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 2a32b04be..00a98e33a 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -35,10 +35,10 @@ class TwoVhost80Test(util.ApacheTest): def mock_deploy_cert(self, config): """A test for a mock deploy cert""" self.config.real_deploy_cert = self.config.deploy_cert + def mocked_deploy_cert(*args, **kwargs): """a helper to mock a deployed cert""" - with mock.patch( - "letsencrypt_apache.configurator.ApacheConfigurator.enable_mod"): + with mock.patch("letsencrypt_apache.configurator.ApacheConfigurator.enable_mod"): config.real_deploy_cert(*args, **kwargs) self.config.deploy_cert = mocked_deploy_cert return self.config @@ -70,7 +70,8 @@ class TwoVhost80Test(util.ApacheTest): def test_prepare_old_aug(self, mock_exe_exists, _): mock_exe_exists.return_value = True self.config.config_test = mock.Mock() - self.config._check_aug_version = mock.Mock(return_value=False) # pylint: disable=protected-access + # pylint: disable=protected-access + self.config._check_aug_version = mock.Mock(return_value=False) self.assertRaises( errors.NotSupportedError, self.config.prepare) @@ -110,8 +111,8 @@ class TwoVhost80Test(util.ApacheTest): def test_add_servernames_alias(self): self.config.parser.add_dir( self.vh_truth[2].path, "ServerAlias", ["*.le.co"]) - self.config._add_servernames(self.vh_truth[2]) # pylint: disable=protected-access - + # pylint: disable=protected-access + self.config._add_servernames(self.vh_truth[2]) self.assertEqual( self.vh_truth[2].get_names(), set(["*.le.co", "ip-172-30-0-17"])) @@ -177,7 +178,8 @@ class TwoVhost80Test(util.ApacheTest): def test_choose_vhost_select_vhost_conflicting_non_ssl(self, mock_select): mock_select.return_value = self.vh_truth[3] conflicting_vhost = obj.VirtualHost( - "path", "aug_path", set([obj.Addr.fromstring("*:443")]), True, True) + "path", "aug_path", set([obj.Addr.fromstring("*:443")]), + True, True) self.config.vhosts.append(conflicting_vhost) self.assertRaises( @@ -196,7 +198,8 @@ class TwoVhost80Test(util.ApacheTest): def test_find_best_vhost_variety(self): # pylint: disable=protected-access ssl_vh = obj.VirtualHost( - "fp", "ap", set([obj.Addr(("*", "443")), obj.Addr(("zombo.com",))]), + "fp", "ap", set([obj.Addr(("*", "443")), + obj.Addr(("zombo.com",))]), True, False) self.config.vhosts.append(ssl_vh) self.assertEqual(self.config._find_best_vhost("zombo.com"), ssl_vh) @@ -277,7 +280,8 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_newssl(self): self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + self.config_path, self.vhost_path, self.config_dir, + self.work_dir, version=(2, 4, 16)) self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") @@ -295,7 +299,8 @@ class TwoVhost80Test(util.ApacheTest): self.assertTrue("ssl_module" in self.config.parser.modules) loc_cert = self.config.parser.find_dir( - "sslcertificatefile", "example/fullchain.pem", self.vh_truth[1].path) + "sslcertificatefile", "example/fullchain.pem", + self.vh_truth[1].path) loc_key = self.config.parser.find_dir( "sslcertificateKeyfile", "example/key.pem", self.vh_truth[1].path) @@ -310,7 +315,8 @@ class TwoVhost80Test(util.ApacheTest): def test_deploy_cert_newssl_no_fullchain(self): self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 16)) + self.config_path, self.vhost_path, self.config_dir, + self.work_dir, version=(2, 4, 16)) self.config = self.mock_deploy_cert(self.config) self.config.parser.modules.add("ssl_module") @@ -320,11 +326,13 @@ class TwoVhost80Test(util.ApacheTest): self.config.assoc["random.demo"] = self.vh_truth[1] self.assertRaises(errors.PluginError, lambda: self.config.deploy_cert( - "random.demo", "example/cert.pem", "example/key.pem")) + "random.demo", "example/cert.pem", + "example/key.pem")) def test_deploy_cert_old_apache_no_chain(self): self.config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, version=(2, 4, 7)) + self.config_path, self.vhost_path, self.config_dir, + self.work_dir, version=(2, 4, 7)) self.config = self.mock_deploy_cert(self.config) self.config.parser.modules.add("ssl_module") @@ -334,7 +342,8 @@ class TwoVhost80Test(util.ApacheTest): self.config.assoc["random.demo"] = self.vh_truth[1] self.assertRaises(errors.PluginError, lambda: self.config.deploy_cert( - "random.demo", "example/cert.pem", "example/key.pem")) + "random.demo", "example/cert.pem", + "example/key.pem")) def test_deploy_cert(self): self.config.parser.modules.add("ssl_module") @@ -442,7 +451,8 @@ class TwoVhost80Test(util.ApacheTest): # Test Listen statements with specific ip listeed self.config.prepare_server_https("443") - # Should only be 2 here, as the third interface already listens to the correct port + # Should only be 2 here, as the third interface + # already listens to the correct port self.assertEqual(mock_add_dir.call_count, 2) # Check argument to new Listen statements @@ -456,9 +466,12 @@ class TwoVhost80Test(util.ApacheTest): # Test self.config.prepare_server_https("8080", temp=True) self.assertEqual(mock_add_dir.call_count, 3) - self.assertEqual(mock_add_dir.call_args_list[0][0][2], ["1.2.3.4:8080", "https"]) - self.assertEqual(mock_add_dir.call_args_list[1][0][2], ["[::1]:8080", "https"]) - self.assertEqual(mock_add_dir.call_args_list[2][0][2], ["1.1.1.1:8080", "https"]) + self.assertEqual(mock_add_dir.call_args_list[0][0][2], + ["1.2.3.4:8080", "https"]) + self.assertEqual(mock_add_dir.call_args_list[1][0][2], + ["[::1]:8080", "https"]) + self.assertEqual(mock_add_dir.call_args_list[2][0][2], + ["1.1.1.1:8080", "https"]) def test_prepare_server_https_mixed_listen(self): @@ -476,7 +489,8 @@ class TwoVhost80Test(util.ApacheTest): # Test Listen statements with specific ip listeed self.config.prepare_server_https("443") - # Should only be 2 here, as the third interface already listens to the correct port + # Should only be 2 here, as the third interface + # already listens to the correct port self.assertEqual(mock_add_dir.call_count, 0) def test_make_vhost_ssl(self): @@ -510,7 +524,8 @@ class TwoVhost80Test(util.ApacheTest): for directive in ["SSLCertificateFile", "SSLCertificateKeyFile", "SSLCertificateChainFile", "SSLCACertificatePath"]: for _ in range(10): - self.config.parser.add_dir(self.vh_truth[1].path, directive, ["bogus"]) + self.config.parser.add_dir(self.vh_truth[1].path, + directive, ["bogus"]) self.config.save() self.config._clean_vhost(self.vh_truth[1]) @@ -536,23 +551,24 @@ class TwoVhost80Test(util.ApacheTest): # pylint: disable=protected-access DIRECTIVE = "Foo" for _ in range(10): - self.config.parser.add_dir(self.vh_truth[1].path, DIRECTIVE, ["bar"]) + self.config.parser.add_dir(self.vh_truth[1].path, + DIRECTIVE, ["bar"]) self.config.save() self.config._deduplicate_directives(self.vh_truth[1].path, [DIRECTIVE]) self.config.save() self.assertEqual( - len(self.config.parser.find_dir( - DIRECTIVE, None, self.vh_truth[1].path, False)), - 1) + len(self.config.parser.find_dir( + DIRECTIVE, None, self.vh_truth[1].path, False)), 1) def test_remove_directives(self): # pylint: disable=protected-access DIRECTIVES = ["Foo", "Bar"] for directive in DIRECTIVES: for _ in range(10): - self.config.parser.add_dir(self.vh_truth[1].path, directive, ["baz"]) + self.config.parser.add_dir(self.vh_truth[1].path, + directive, ["baz"]) self.config.save() self.config._remove_directives(self.vh_truth[1].path, DIRECTIVES) @@ -560,9 +576,8 @@ class TwoVhost80Test(util.ApacheTest): for directive in DIRECTIVES: self.assertEqual( - len(self.config.parser.find_dir( - directive, None, self.vh_truth[1].path, False)), - 0) + len(self.config.parser.find_dir( + directive, None, self.vh_truth[1].path, False)), 0) def test_make_vhost_ssl_extra_vhs(self): self.config.aug.match = mock.Mock(return_value=["p1", "p2"]) @@ -651,7 +666,8 @@ class TwoVhost80Test(util.ApacheTest): self.assertRaises(errors.PluginError, self.config.get_version) mock_script.return_value = ( - "Server Version: Apache/2.3{0} Apache/2.4.7".format(os.linesep), "") + "Server Version: Apache/2.3{0} Apache/2.4.7".format( + os.linesep), "") self.assertRaises(errors.PluginError, self.config.get_version) mock_script.side_effect = errors.SubprocessError("Can't find program") @@ -675,7 +691,8 @@ class TwoVhost80Test(util.ApacheTest): def test_config_test_bad_process(self, mock_run_script): mock_run_script.side_effect = errors.SubprocessError - self.assertRaises(errors.MisconfigurationError, self.config.config_test) + self.assertRaises(errors.MisconfigurationError, + self.config.config_test) def test_get_all_certs_keys(self): c_k = self.config.get_all_certs_keys() @@ -687,7 +704,8 @@ class TwoVhost80Test(util.ApacheTest): self.assertTrue("default-ssl" in path) def test_get_all_certs_keys_malformed_conf(self): - self.config.parser.find_dir = mock.Mock(side_effect=[["path"], [], ["path"], []]) + self.config.parser.find_dir = mock.Mock( + side_effect=[["path"], [], ["path"], []]) c_k = self.config.get_all_certs_keys() self.assertFalse(c_k) @@ -708,13 +726,13 @@ class TwoVhost80Test(util.ApacheTest): def test_supported_enhancements(self): self.assertTrue(isinstance(self.config.supported_enhancements(), list)) - @mock.patch("letsencrypt.le_util.exe_exists") def test_enhance_unknown_vhost(self, mock_exe): self.config.parser.modules.add("rewrite_module") mock_exe.return_value = True ssl_vh = obj.VirtualHost( - "fp", "ap", set([obj.Addr(("*", "443")), obj.Addr(("satoshi.com",))]), + "fp", "ap", set([obj.Addr(("*", "443")), + obj.Addr(("satoshi.com",))]), True, False) self.config.vhosts.append(ssl_vh) self.assertRaises( @@ -735,7 +753,7 @@ class TwoVhost80Test(util.ApacheTest): # This will create an ssl vhost for letsencrypt.demo self.config.enhance("letsencrypt.demo", "ensure-http-header", - "Strict-Transport-Security") + "Strict-Transport-Security") self.assertTrue("headers_module" in self.config.parser.modules) @@ -745,7 +763,7 @@ class TwoVhost80Test(util.ApacheTest): # These are not immediately available in find_dir even with save() and # load(). They must be found in sites-available hsts_header = self.config.parser.find_dir( - "Header", None, ssl_vhost.path) + "Header", None, ssl_vhost.path) # four args to HSTS header self.assertEqual(len(hsts_header), 4) @@ -757,12 +775,12 @@ class TwoVhost80Test(util.ApacheTest): # This will create an ssl vhost for letsencrypt.demo self.config.enhance("encryption-example.demo", "ensure-http-header", - "Strict-Transport-Security") + "Strict-Transport-Security") self.assertRaises( errors.PluginEnhancementAlreadyPresent, - self.config.enhance, "encryption-example.demo", "ensure-http-header", - "Strict-Transport-Security") + self.config.enhance, "encryption-example.demo", + "ensure-http-header", "Strict-Transport-Security") @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") @@ -773,7 +791,7 @@ class TwoVhost80Test(util.ApacheTest): # This will create an ssl vhost for letsencrypt.demo self.config.enhance("letsencrypt.demo", "ensure-http-header", - "Upgrade-Insecure-Requests") + "Upgrade-Insecure-Requests") self.assertTrue("headers_module" in self.config.parser.modules) @@ -783,7 +801,7 @@ class TwoVhost80Test(util.ApacheTest): # These are not immediately available in find_dir even with save() and # load(). They must be found in sites-available uir_header = self.config.parser.find_dir( - "Header", None, ssl_vhost.path) + "Header", None, ssl_vhost.path) # four args to HSTS header self.assertEqual(len(uir_header), 4) @@ -795,14 +813,12 @@ class TwoVhost80Test(util.ApacheTest): # This will create an ssl vhost for letsencrypt.demo self.config.enhance("encryption-example.demo", "ensure-http-header", - "Upgrade-Insecure-Requests") + "Upgrade-Insecure-Requests") self.assertRaises( errors.PluginEnhancementAlreadyPresent, - self.config.enhance, "encryption-example.demo", "ensure-http-header", - "Upgrade-Insecure-Requests") - - + self.config.enhance, "encryption-example.demo", + "ensure-http-header", "Upgrade-Insecure-Requests") @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") @@ -836,7 +852,8 @@ class TwoVhost80Test(util.ApacheTest): self.config.get_version = mock.Mock(return_value=(2, 3, 9)) self.config.parser.add_dir( self.vh_truth[3].path, "RewriteRule", ["Unknown"]) - self.assertTrue(self.config._is_rewrite_exists(self.vh_truth[3])) # pylint: disable=protected-access + # pylint: disable=protected-access + self.assertTrue(self.config._is_rewrite_exists(self.vh_truth[3])) def test_rewrite_engine_exists(self): # Skip the enable mod @@ -844,8 +861,8 @@ class TwoVhost80Test(util.ApacheTest): self.config.get_version = mock.Mock(return_value=(2, 3, 9)) self.config.parser.add_dir( self.vh_truth[3].path, "RewriteEngine", "on") - self.assertTrue(self.config._is_rewrite_engine_on(self.vh_truth[3])) # pylint: disable=protected-access - + # pylint: disable=protected-access + self.assertTrue(self.config._is_rewrite_engine_on(self.vh_truth[3])) @mock.patch("letsencrypt.le_util.run_script") @mock.patch("letsencrypt.le_util.exe_exists") @@ -857,7 +874,7 @@ class TwoVhost80Test(util.ApacheTest): # Create a preexisting rewrite rule self.config.parser.add_dir( self.vh_truth[3].path, "RewriteRule", ["UnknownPattern", - "UnknownTarget"]) + "UnknownTarget"]) self.config.save() # This will create an ssl vhost for letsencrypt.demo @@ -879,11 +896,11 @@ class TwoVhost80Test(util.ApacheTest): self.assertTrue("rewrite_module" in self.config.parser.modules) - def test_redirect_with_conflict(self): self.config.parser.modules.add("rewrite_module") ssl_vh = obj.VirtualHost( - "fp", "ap", set([obj.Addr(("*", "443")), obj.Addr(("zombo.com",))]), + "fp", "ap", set([obj.Addr(("*", "443")), + obj.Addr(("zombo.com",))]), True, False) # No names ^ this guy should conflict. @@ -908,7 +925,8 @@ class TwoVhost80Test(util.ApacheTest): self.vh_truth[1].name = "default.com" self.vh_truth[1].aliases = set(["yes.default.com"]) - self.config._enable_redirect(self.vh_truth[1], "") # pylint: disable=protected-access + # pylint: disable=protected-access + self.config._enable_redirect(self.vh_truth[1], "") self.assertEqual(len(self.config.vhosts), 7) def test_create_own_redirect_for_old_apache_version(self): @@ -918,7 +936,8 @@ class TwoVhost80Test(util.ApacheTest): self.vh_truth[1].name = "default.com" self.vh_truth[1].aliases = set(["yes.default.com"]) - self.config._enable_redirect(self.vh_truth[1], "") # pylint: disable=protected-access + # pylint: disable=protected-access + self.config._enable_redirect(self.vh_truth[1], "") self.assertEqual(len(self.config.vhosts), 7) def test_sift_line(self): @@ -942,10 +961,10 @@ class TwoVhost80Test(util.ApacheTest): http_vhost.path, "RewriteEngine", "on") self.config.parser.add_dir( - http_vhost.path, "RewriteRule", - ["^", - "https://%{SERVER_NAME}%{REQUEST_URI}", - "[L,QSA,R=permanent]"]) + http_vhost.path, "RewriteRule", + ["^", + "https://%{SERVER_NAME}%{REQUEST_URI}", + "[L,QSA,R=permanent]"]) self.config.save() ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0]) @@ -954,8 +973,9 @@ class TwoVhost80Test(util.ApacheTest): "RewriteEngine", "on", ssl_vhost.path, False)) conf_text = open(ssl_vhost.filep).read() - commented_rewrite_rule = \ - "# RewriteRule ^ https://%{SERVER_NAME}%{REQUEST_URI} [L,QSA,R=permanent]" + commented_rewrite_rule = ("# RewriteRule ^ " + "https://%{SERVER_NAME}%{REQUEST_URI} " + "[L,QSA,R=permanent]") self.assertTrue(commented_rewrite_rule in conf_text) mock_get_utility().add_message.assert_called_once_with(mock.ANY, mock.ANY) @@ -990,9 +1010,11 @@ class TwoVhost80Test(util.ApacheTest): def test_aug_version(self): mock_match = mock.Mock(return_value=["something"]) self.config.aug.match = mock_match - self.assertEquals(self.config._check_aug_version(), ["something"]) # pylint: disable=protected-access + # pylint: disable=protected-access + self.assertEquals(self.config._check_aug_version(), + ["something"]) self.config.aug.match.side_effect = RuntimeError - self.assertFalse(self.config._check_aug_version()) # pylint: disable=protected-access + self.assertFalse(self.config._check_aug_version()) if __name__ == "__main__": diff --git a/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py b/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py index 13eddaddf..a469702f1 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py @@ -47,7 +47,8 @@ class VirtualHostTest(unittest.TestCase): self.assertTrue(self.vhost1.conflicts([self.addr2])) self.assertFalse(self.vhost1.conflicts([self.addr_default])) - self.assertFalse(self.vhost2.conflicts([self.addr1, self.addr_default])) + self.assertFalse(self.vhost2.conflicts([self.addr1, + self.addr_default])) def test_same_server(self): from letsencrypt_apache.obj import VirtualHost diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index 9b78bf6d6..e976bc9f6 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -118,7 +118,8 @@ class BasicParserTest(util.ParserTest): # pylint: disable=protected-access path = os.path.join(self.parser.root, "httpd.conf") open(path, 'w').close() - self.parser.add_dir(self.parser.loc["default"], "Include", "httpd.conf") + self.parser.add_dir(self.parser.loc["default"], "Include", + "httpd.conf") self.assertEqual( path, self.parser._set_user_config_file()) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py index b69406932..a2d3cacc4 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py @@ -9,6 +9,8 @@ from letsencrypt.plugins import common_test from letsencrypt_apache import obj from letsencrypt_apache.tests import util +from six.moves import xrange # pylint: disable=redefined-builtin + class TlsSniPerformTest(util.ApacheTest): """Test the ApacheTlsSni01 challenge.""" @@ -58,7 +60,7 @@ class TlsSniPerformTest(util.ApacheTest): mock_setup_cert.assert_called_once_with(achall) - # Check to make sure challenge config path is included in apache config. + # Check to make sure challenge config path is included in apache config self.assertEqual( len(self.sni.configurator.parser.find_dir( "Include", self.sni.challenge_conf)), 1) @@ -78,8 +80,7 @@ class TlsSniPerformTest(util.ApacheTest): # pylint: disable=protected-access self.sni._setup_challenge_cert = mock_setup_cert - with mock.patch( - "letsencrypt_apache.configurator.ApacheConfigurator.enable_mod"): + with mock.patch("letsencrypt_apache.configurator.ApacheConfigurator.enable_mod"): sni_responses = self.sni.perform() self.assertEqual(mock_setup_cert.call_count, 2) @@ -126,13 +127,15 @@ class TlsSniPerformTest(util.ApacheTest): def test_get_addrs_default(self): self.sni.configurator.choose_vhost = mock.Mock( return_value=obj.VirtualHost( - "path", "aug_path", set([obj.Addr.fromstring("_default_:443")]), + "path", "aug_path", + set([obj.Addr.fromstring("_default_:443")]), False, False) ) + # pylint: disable=protected-access self.assertEqual( set([obj.Addr.fromstring("*:443")]), - self.sni._get_addrs(self.achalls[0])) # pylint: disable=protected-access + self.sni._get_addrs(self.achalls[0])) if __name__ == "__main__": diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 798d4814b..569f9e150 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -62,7 +62,8 @@ class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods def get_apache_configurator( - config_path, vhost_path, config_dir, work_dir, version=(2, 4, 7), conf=None): + config_path, vhost_path, + config_dir, work_dir, version=(2, 4, 7), conf=None): """Create an Apache Configurator with the specified options. :param conf: Function that returns binary paths. self.conf in Configurator @@ -129,10 +130,12 @@ def get_vh_truth(temp_dir, config_name): os.path.join(prefix, "mod_macro-example.conf"), os.path.join(aug_pre, "mod_macro-example.conf/Macro/VirtualHost"), - set([obj.Addr.fromstring("*:80")]), False, True, modmacro=True), + set([obj.Addr.fromstring("*:80")]), False, True, + modmacro=True), obj.VirtualHost( os.path.join(prefix, "default-ssl-port-only.conf"), - os.path.join(aug_pre, "default-ssl-port-only.conf/IfModule/VirtualHost"), + os.path.join(aug_pre, ("default-ssl-port-only.conf/" + "IfModule/VirtualHost")), set([obj.Addr.fromstring("_default_:443")]), True, False), ] return vh_truth diff --git a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py index ca7985f35..971072311 100644 --- a/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py +++ b/letsencrypt-apache/letsencrypt_apache/tls_sni_01.py @@ -10,6 +10,7 @@ from letsencrypt_apache import parser logger = logging.getLogger(__name__) + class ApacheTlsSni01(common.TLSSNI01): """Class that performs TLS-SNI-01 challenges within the Apache configurator @@ -125,7 +126,8 @@ class ApacheTlsSni01(common.TLSSNI01): addrs.add(default_addr) else: addrs.add( - addr.get_sni_addr(self.configurator.config.tls_sni_01_port)) + addr.get_sni_addr( + self.configurator.config.tls_sni_01_port)) return addrs From 5357a556eb8a2ae5308c175b944c9797549464b1 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 14 Jan 2016 14:18:36 +0200 Subject: [PATCH 090/100] PyLint doesn't play well with six --- letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py index a2d3cacc4..9681bf9fc 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/tls_sni_01_test.py @@ -9,7 +9,7 @@ from letsencrypt.plugins import common_test from letsencrypt_apache import obj from letsencrypt_apache.tests import util -from six.moves import xrange # pylint: disable=redefined-builtin +from six.moves import xrange # pylint: disable=redefined-builtin, import-error class TlsSniPerformTest(util.ApacheTest): From f909d22a3dd906b4230930440ec6ee0feea6bff1 Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Thu, 14 Jan 2016 22:10:00 +0700 Subject: [PATCH 091/100] Test with missing certificate when calling RenewableCert.names() --- letsencrypt/tests/renewer_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 61a9a6e75..a103f5dbf 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -381,6 +381,10 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.names(12), ["example.com", "www.example.com"]) + # Trying missing cert + os.unlink(self.test_rc.cert) + self.assertRaises(errors.CertStorageError, self.test_rc.names) + @mock.patch("letsencrypt.storage.datetime") def test_time_interval_judgments(self, mock_datetime): """Test should_autodeploy() and should_autorenew() on the basis From 54207f9ef3458bb1b22c4481e699476211cbb7c6 Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Thu, 14 Jan 2016 23:15:22 +0700 Subject: [PATCH 092/100] Test view_config_changes with for_logging=True --- letsencrypt/tests/reverter_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt/tests/reverter_test.py b/letsencrypt/tests/reverter_test.py index d31b6f2cc..7699c96f5 100644 --- a/letsencrypt/tests/reverter_test.py +++ b/letsencrypt/tests/reverter_test.py @@ -385,6 +385,15 @@ class TestFullCheckpointsReverter(unittest.TestCase): self.assertRaises( errors.ReverterError, self.reverter.view_config_changes) + def test_view_config_changes_for_logging(self): + self._setup_three_checkpoints() + + config_changes = self.reverter.view_config_changes(for_logging=True) + + self.assertIn("First Checkpoint", config_changes) + self.assertIn("Second Checkpoint", config_changes) + self.assertIn("Third Checkpoint", config_changes) + def _setup_three_checkpoints(self): """Generate some finalized checkpoints.""" # Checkpoint1 - config1 From 84a0fba774ce8f968d3f090362e3e745d67e250d Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Thu, 14 Jan 2016 23:16:26 +0700 Subject: [PATCH 093/100] Properly patch display_util --- letsencrypt/tests/display/ops_test.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index 31db47cce..d98afe180 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -407,10 +407,11 @@ class ChooseNamesTest(unittest.TestCase): "uniçodé.com") self.assertEqual(_choose_names_manually(), []) # IDN exception with previous mocks - with mock.patch("letsencrypt.display.util") as mock_sl: - uerror = UnicodeEncodeError('mock', u'', - 0, 1, 'mock') - mock_sl.separate_list_input.side_effect = uerror + with mock.patch( + "letsencrypt.display.ops.display_util.separate_list_input" + ) as mock_sli: + unicode_error = UnicodeEncodeError('mock', u'', 0, 1, 'mock') + mock_sli.side_effect = unicode_error self.assertEqual(_choose_names_manually(), []) # Punycode and no retry mock_util().input.return_value = (display_util.OK, From 2e034e6c6c6992eea55e74fa6bb5cbd66347ee3c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 14 Jan 2016 11:42:10 -0800 Subject: [PATCH 094/100] Revert changes to acme's setup.py --- acme/setup.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 372c05b13..3d7b882d5 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -11,6 +11,8 @@ install_requires = [ # load_pem_private/public_key (>=0.6) # rsa_recover_prime_factors (>=0.8) 'cryptography>=0.8', + 'ndg-httpsclient', # urllib3 InsecurePlatformWarning (#304) + 'pyasn1', # urllib3 InsecurePlatformWarning (#304) # Connection.set_tlsext_host_name (>=0.13) 'PyOpenSSL>=0.13', 'pyrfc3339', @@ -31,11 +33,6 @@ if sys.version_info < (2, 7): else: install_requires.append('mock') -if sys.version_info < (2, 7, 9): - # For secure SSL connection with Python 2.7 (InsecurePlatformWarning) - install_requires.append('ndg-httpsclient') - install_requires.append('pyasn1') - docs_extras = [ 'Sphinx>=1.0', # autodoc_member_order = 'bysource', autodoc_default_flags 'sphinx_rtd_theme', From 5d93678303cf3d2cdc69aaa16d3297739a72d3e6 Mon Sep 17 00:00:00 2001 From: Erik Rose Date: Thu, 14 Jan 2016 16:40:47 -0500 Subject: [PATCH 095/100] Make ConfigArgParse dependencies unconditional as well. None of this is ideal, since we're making the dependencies tighter than they theoretically need to be, but the behavior of the old le-auto makes this necessary to make it succeed in practice (when using LE wheels). Once we move to the new le-auto (which pins everything and makes setup.py dependencies irrelevant for auto installs), we should redo this using env markers as in https://github.com/letsencrypt/letsencrypt/pull/2177. We're too afraid to do it now. Similarly, we're too afraid to change how we handle argparse right now, despite that it should be required directly by us under 2.6. In practice, ConfigArgParse pulls it in for us, so we're okay as long as it continues to do that. --- setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/setup.py b/setup.py index ad7fb6909..4d9cc7850 100644 --- a/setup.py +++ b/setup.py @@ -33,6 +33,7 @@ version = meta['version'] # Please update tox.ini when modifying dependency version requirements install_requires = [ 'acme=={0}'.format(version), + 'ConfigArgParse>=0.10.0', # python2.6 support, upstream #17 'configobj', 'cryptography>=0.7', # load_pem_x509_certificate 'parsedatetime', @@ -52,12 +53,10 @@ if sys.version_info < (2, 7): install_requires.extend([ # only some distros recognize stdlib argparse as already satisfying 'argparse', - 'ConfigArgParse>=0.10.0', # python2.6 support, upstream #17 'mock<1.1.0', ]) else: install_requires.extend([ - 'ConfigArgParse', 'mock', ]) From b7b3f24da085765f8963c95826a6b54b401c58cd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 14 Jan 2016 16:35:20 -0800 Subject: [PATCH 096/100] Convert sites-enabled files to symlinks --- .../letsencrypt_apache/tests/util.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 798d4814b..12237b209 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -42,6 +42,20 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods self.rsa512jwk = jose.JWKRSA.load(test_util.load_vector( "rsa512_key.pem")) + # Make sure all vhosts in sites-enabled are symlinks (Python packaging + # does not preserve symlinks) + sites_enabled = os.path.join(self.config_path, "sites-enabled") + if not os.path.exists(sites_enabled): + return + + for vhost_basename in os.listdir(sites_enabled): + vhost = os.path.join(sites_enabled, vhost_basename) + if not os.path.islink(vhost): + os.remove(vhost) + target = os.path.join( + os.path.pardir, "sites-available", vhost_basename) + os.symlink(target, vhost) + class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods From 2939b62f242f05e24c5ede23c8dfeed2f1a0535c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 14 Jan 2016 16:59:29 -0800 Subject: [PATCH 097/100] Stop cover from failing --- letsencrypt-apache/letsencrypt_apache/tests/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 12237b209..782ed6f44 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -50,7 +50,7 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods for vhost_basename in os.listdir(sites_enabled): vhost = os.path.join(sites_enabled, vhost_basename) - if not os.path.islink(vhost): + if not os.path.islink(vhost): # pragma: no cover os.remove(vhost) target = os.path.join( os.path.pardir, "sites-available", vhost_basename) From 2a6d3bedb61800cadca3e7264bce73b11b3261e3 Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Fri, 15 Jan 2016 11:37:20 +0700 Subject: [PATCH 098/100] Fix missing --webroot-path handling in webroot plugin --- letsencrypt/plugins/webroot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 0679bc349..f8176417c 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -98,8 +98,8 @@ to serve all files under specified web root ({0}).""" def _path_for_achall(self, achall): try: path = self.full_roots[achall.domain] - except IndexError: - raise errors.PluginError("Missing --webroot-path for domain: {1}" + except KeyError: + raise errors.PluginError("Missing --webroot-path for domain: {0}" .format(achall.domain)) if not os.path.exists(path): raise errors.PluginError("Mysteriously missing path {0} for domain: {1}" From b89d383ff49b39425fd70d708b17a6bb5e251fff Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Fri, 15 Jan 2016 11:38:21 +0700 Subject: [PATCH 099/100] Add tests for missing paths in webroot plugin --- letsencrypt/plugins/webroot_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index defe9396b..e3f926c7f 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -111,6 +111,18 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(os.stat(self.validation_path).st_gid, parent_gid) self.assertEqual(os.stat(self.validation_path).st_uid, parent_uid) + def test_perform_missing_path(self): + self.auth.prepare() + + missing_achall = achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.HTTP01_P, domain="thing2.com", account_key=KEY) + self.assertRaises( + errors.PluginError, self.auth.perform, [missing_achall]) + + self.auth.full_roots[self.achall.domain] = 'null' + self.assertRaises( + errors.PluginError, self.auth.perform, [self.achall]) + def test_perform_cleanup(self): self.auth.prepare() responses = self.auth.perform([self.achall]) From 4366a8988345f4a0fe37afc858112468b384dc0d Mon Sep 17 00:00:00 2001 From: Filip Ochnik Date: Fri, 15 Jan 2016 12:55:24 +0700 Subject: [PATCH 100/100] Don't use assertIn --- letsencrypt/tests/reverter_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/tests/reverter_test.py b/letsencrypt/tests/reverter_test.py index 7699c96f5..aafd3b041 100644 --- a/letsencrypt/tests/reverter_test.py +++ b/letsencrypt/tests/reverter_test.py @@ -390,9 +390,9 @@ class TestFullCheckpointsReverter(unittest.TestCase): config_changes = self.reverter.view_config_changes(for_logging=True) - self.assertIn("First Checkpoint", config_changes) - self.assertIn("Second Checkpoint", config_changes) - self.assertIn("Third Checkpoint", config_changes) + self.assertTrue("First Checkpoint" in config_changes) + self.assertTrue("Second Checkpoint" in config_changes) + self.assertTrue("Third Checkpoint" in config_changes) def _setup_three_checkpoints(self): """Generate some finalized checkpoints."""