From 3b73b04bfed721a36e6933423d7b796f14810feb Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 25 Aug 2015 07:01:30 +0000 Subject: [PATCH 01/44] SimpleHTTP manual plugin: v04 provisioned resource contents (fixes #679). --- letsencrypt/plugins/manual.py | 4 ++-- letsencrypt/plugins/manual_test.py | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index d13f35f99..672326c87 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -37,7 +37,7 @@ class ManualAuthenticator(common.Plugin): Make sure your web server displays the following content at {uri} before continuing: -{achall.token} +{validation} Content-Type header MUST be set to {ct}. @@ -158,7 +158,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") raise errors.Error("Couldn't execute manual command") else: self._notify_and_wait(self.MESSAGE_TEMPLATE.format( - achall=achall, response=response, + validation=validation.json_dumps(), response=response, uri=response.uri(achall.domain, achall.challb.chall), ct=response.CONTENT_TYPE, command=command)) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index caf7fb3c4..bb969243b 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -61,7 +61,28 @@ class ManualAuthenticatorTest(unittest.TestCase): self.achalls[0].challb.chall, "foo.com", KEY.public_key(), 4430) message = mock_stdout.write.mock_calls[0][1][0] - self.assertTrue(self.achalls[0].token in message) + self.assertEqual(message, """\ +Make sure your web server displays the following content at +http://foo.com/.well-known/acme-challenge/ZXZhR3hmQURzNnBTUmIyTEF2OUlaZjE3RHQzanV4R0orUEN0OTJ3citvQQ before continuing: + +{"header": {"alg": "RS256", "jwk": {"e": "AQAB", "kty": "RSA", "n": "rHVztFHtH92ucFJD_N_HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2-6QWE30cWgdmJS86ObRz6lUTor4R0T-3C5Q"}}, "payload": "eyJ0bHMiOiBmYWxzZSwgInRva2VuIjogIlpYWmhSM2htUVVSek5uQlRVbUl5VEVGMk9VbGFaakUzUkhRemFuVjRSMG9yVUVOME9USjNjaXR2UVEiLCAidHlwZSI6ICJzaW1wbGVIdHRwIn0", "signature": "jFPJFC-2eRyBw7Sl0wyEBhsdvRZtKk8hc6HykEPAiofZlIwdIu76u2xHqMVZWSZdpxwMNUnnawTEAqgMWFydMA"} + +Content-Type header MUST be set to application/jose+json. + +If you don\'t have HTTP server configured, you can run the following +command on the target server (as root): + +mkdir -p /tmp/letsencrypt/public_html/.well-known/acme-challenge +cd /tmp/letsencrypt/public_html +echo -n \'{"header": {"alg": "RS256", "jwk": {"e": "AQAB", "kty": "RSA", "n": "rHVztFHtH92ucFJD_N_HW9AsdRsUuHUBBBDlHwNlRd3fp580rv2-6QWE30cWgdmJS86ObRz6lUTor4R0T-3C5Q"}}, "payload": "eyJ0bHMiOiBmYWxzZSwgInRva2VuIjogIlpYWmhSM2htUVVSek5uQlRVbUl5VEVGMk9VbGFaakUzUkhRemFuVjRSMG9yVUVOME9USjNjaXR2UVEiLCAidHlwZSI6ICJzaW1wbGVIdHRwIn0", "signature": "jFPJFC-2eRyBw7Sl0wyEBhsdvRZtKk8hc6HykEPAiofZlIwdIu76u2xHqMVZWSZdpxwMNUnnawTEAqgMWFydMA"}\' > .well-known/acme-challenge/ZXZhR3hmQURzNnBTUmIyTEF2OUlaZjE3RHQzanV4R0orUEN0OTJ3citvQQ +# run only once per server: +$(command -v python2 || command -v python2.7 || command -v python2.6) -c \\ +"import BaseHTTPServer, SimpleHTTPServer; \\ +SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {\'\': \'application/jose+json\'}; \\ +s = BaseHTTPServer.HTTPServer((\'\', 4430), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ +s.serve_forever()" +""") + #self.assertTrue(validation in message) mock_verify.return_value = False self.assertEqual([None], self.auth.perform(self.achalls)) From 138f1d1b28cf4f766e1f19312cb5fe058760433b Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 08:21:29 +0000 Subject: [PATCH 02/44] lint: space check for dict-separator --- .pylintrc | 2 +- acme/acme/challenges_test.py | 6 +++--- .../letsencrypt_apache/tests/parser_test.py | 2 +- letsencrypt/auth_handler.py | 14 +++++++------- letsencrypt/tests/auth_handler_test.py | 2 +- letsencrypt/tests/validator_test.py | 10 +++++----- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/.pylintrc b/.pylintrc index d954b2658..4d370eb3c 100644 --- a/.pylintrc +++ b/.pylintrc @@ -218,7 +218,7 @@ ignore-long-lines=^\s*(# )??$ single-line-if-stmt=no # List of optional constructs for which whitespace checking is disabled -no-space-check=trailing-comma,dict-separator +no-space-check=trailing-comma # Maximum number of lines in a module max-module-lines=1250 diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index d123eca20..f7d25a4b4 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -350,9 +350,9 @@ class RecoveryContactTest(unittest.TestCase): contact='c********n@example.com') self.jmsg = { 'type': 'recoveryContact', - 'activationURL' : 'https://example.ca/sendrecovery/a5bd99383fb0', - 'successURL' : 'https://example.ca/confirmrecovery/bb1b9928932', - 'contact' : 'c********n@example.com', + 'activationURL': 'https://example.ca/sendrecovery/a5bd99383fb0', + 'successURL': 'https://example.ca/confirmrecovery/bb1b9928932', + 'contact': 'c********n@example.com', } def test_to_partial_json(self): diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index ce234bff7..d2e4dec14 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -143,7 +143,7 @@ class BasicParserTest(util.ParserTest): 'Group: name="www-data" id=33 not_used\n' ) expected_vars = {"TEST": "", "U_MICH": "", "TLS": "443", - "example_path":"Documents/path"} + "example_path": "Documents/path"} self.parser.update_runtime_variables("ctl") self.assertEqual(self.parser.variables, expected_vars) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 894510191..00c30fe3d 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -493,26 +493,26 @@ _ERROR_HELP_COMMON = ( _ERROR_HELP = { - "connection" : + "connection": _ERROR_HELP_COMMON + " Additionally, please check that your computer " "has publicly routable IP address and no firewalls are preventing the " "server from communicating with the client.", - "dnssec" : + "dnssec": _ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for " "your domain, please ensure the signature is valid.", - "malformed" : + "malformed": "To fix these errors, please make sure that you did not provide any " "invalid information to the client and try running Let's Encrypt " "again.", - "serverInternal" : + "serverInternal": "Unfortunately, an error on the ACME server prevented you from completing " "authorization. Please try again later.", - "tls" : + "tls": _ERROR_HELP_COMMON + " Additionally, please check that you have an up " "to date TLS configuration that allows the server to communicate with " "the Let's Encrypt client.", - "unauthorized" : _ERROR_HELP_COMMON, - "unknownHost" : _ERROR_HELP_COMMON,} + "unauthorized": _ERROR_HELP_COMMON, + "unknownHost": _ERROR_HELP_COMMON,} def _report_failed_challs(failed_achalls): diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 486b55a20..2127c8f5c 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -427,7 +427,7 @@ class ReportFailedChallsTest(unittest.TestCase): from letsencrypt import achallenges kwargs = { - "chall" : acme_util.SIMPLE_HTTP, + "chall": acme_util.SIMPLE_HTTP, "uri": "uri", "status": messages.STATUS_INVALID, "error": messages.Error(typ="tls", detail="detail"), diff --git a/letsencrypt/tests/validator_test.py b/letsencrypt/tests/validator_test.py index c02a7d865..5ce5fa557 100644 --- a/letsencrypt/tests/validator_test.py +++ b/letsencrypt/tests/validator_test.py @@ -38,15 +38,15 @@ class ValidatorTest(unittest.TestCase): @mock.patch("letsencrypt.validator.requests.get") def test_succesful_redirect(self, mock_get_request): mock_get_request.return_value = create_response( - 301, {"location" : "https://test.com"}) + 301, {"location": "https://test.com"}) self.assertTrue(self.validator.redirect("test.com")) @mock.patch("letsencrypt.validator.requests.get") def test_redirect_with_headers(self, mock_get_request): mock_get_request.return_value = create_response( - 301, {"location" : "https://test.com"}) + 301, {"location": "https://test.com"}) self.assertTrue(self.validator.redirect( - "test.com", headers={"Host" : "test.com"})) + "test.com", headers={"Host": "test.com"})) @mock.patch("letsencrypt.validator.requests.get") def test_redirect_missing_location(self, mock_get_request): @@ -56,13 +56,13 @@ class ValidatorTest(unittest.TestCase): @mock.patch("letsencrypt.validator.requests.get") def test_redirect_wrong_status_code(self, mock_get_request): mock_get_request.return_value = create_response( - 201, {"location" : "https://test.com"}) + 201, {"location": "https://test.com"}) self.assertFalse(self.validator.redirect("test.com")) @mock.patch("letsencrypt.validator.requests.get") def test_redirect_wrong_redirect_code(self, mock_get_request): mock_get_request.return_value = create_response( - 303, {"location" : "https://test.com"}) + 303, {"location": "https://test.com"}) self.assertFalse(self.validator.redirect("test.com")) @mock.patch("letsencrypt.validator.requests.get") From 89c99a1f349b0fc41929a22c13f0d717d0c5f7fb Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:19:26 +0000 Subject: [PATCH 03/44] pep8 acme --- acme/acme/challenges_test.py | 4 ++-- acme/acme/client_test.py | 4 ++++ acme/acme/jose/json_util.py | 15 +++++++++++---- acme/acme/jose/json_util_test.py | 5 +++++ acme/acme/jose/jwa.py | 4 ++-- acme/acme/jose/jwk.py | 2 +- acme/acme/jose/jws.py | 9 +++++---- acme/acme/messages.py | 5 +++++ acme/acme/messages_test.py | 2 +- acme/acme/test_util.py | 6 ++++++ 10 files changed, 42 insertions(+), 14 deletions(-) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index f7d25a4b4..4e3bfa8e0 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -136,7 +136,7 @@ class SimpleHTTPResponseTest(unittest.TestCase): jose.JWS.sign(payload=bad_resource.json_dumps().encode('utf-8'), alg=jose.RS256, key=account_key) for bad_resource in (resource.update(tls=True), - resource.update(token=b'x'*20)) + resource.update(token=(b'x' * 20))) ) for validation in validations: self.assertFalse(self.resp_http.check_validation( @@ -320,7 +320,7 @@ class DVSNIResponseTest(unittest.TestCase): def test_simple_verify_wrong_token(self): msg = self.msg.update(validation=jose.JWS.sign( - payload=self.chall.update(token=b'b'*20).json_dumps().encode(), + payload=self.chall.update(token=(b'b' * 20)).json_dumps().encode(), key=self.key, alg=jose.RS256)) self.assertFalse(msg.simple_verify( self.chall, self.domain, self.key.public_key())) diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index dcc0832e3..12589217f 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -379,11 +379,14 @@ class ClientNetworkTest(unittest.TestCase): # pylint: disable=missing-docstring def __init__(self, value): self.value = value + def to_partial_json(self): return {'foo': self.value} + @classmethod def from_json(cls, value): pass # pragma: no cover + # pylint: disable=protected-access jws_dump = self.net._wrap_in_jws( MockJSONDeSerializable('foo'), nonce=b'Tg') @@ -487,6 +490,7 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase): self.all_nonces = [jose.b64encode(b'Nonce'), jose.b64encode(b'Nonce2')] self.available_nonces = self.all_nonces[:] + def send_request(*args, **kwargs): # pylint: disable=unused-argument,missing-docstring if self.available_nonces: diff --git a/acme/acme/jose/json_util.py b/acme/acme/jose/json_util.py index 51d55ebd9..7b95e3fce 100644 --- a/acme/acme/jose/json_util.py +++ b/acme/acme/jose/json_util.py @@ -307,6 +307,7 @@ def encode_b64jose(data): # b64encode produces ASCII characters only return b64.b64encode(data).decode('ascii') + def decode_b64jose(data, size=None, minimum=False): """Decode JOSE Base-64 field. @@ -324,13 +325,14 @@ def decode_b64jose(data, size=None, minimum=False): except error_cls as error: raise errors.DeserializationError(error) - if size is not None and ((not minimum and len(decoded) != size) - or (minimum and len(decoded) < size)): + if size is not None and ((not minimum and len(decoded) != size) or + (minimum and len(decoded) < size)): raise errors.DeserializationError( "Expected at least or exactly {0} bytes".format(size)) return decoded + def encode_hex16(value): """Hexlify. @@ -340,6 +342,7 @@ def encode_hex16(value): """ return binascii.hexlify(value).decode() + def decode_hex16(value, size=None, minimum=False): """Decode hexlified field. @@ -352,8 +355,8 @@ def decode_hex16(value, size=None, minimum=False): """ value = value.encode() - if size is not None and ((not minimum and len(value) != size * 2) - or (minimum and len(value) < size * 2)): + if size is not None and ((not minimum and len(value) != size * 2) or + (minimum and len(value) < size * 2)): raise errors.DeserializationError() error_cls = TypeError if six.PY2 else binascii.Error try: @@ -361,6 +364,7 @@ def decode_hex16(value, size=None, minimum=False): except error_cls as error: raise errors.DeserializationError(error) + def encode_cert(cert): """Encode certificate as JOSE Base-64 DER. @@ -371,6 +375,7 @@ def encode_cert(cert): return encode_b64jose(OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_ASN1, cert)) + def decode_cert(b64der): """Decode JOSE Base-64 DER-encoded certificate. @@ -384,6 +389,7 @@ def decode_cert(b64der): except OpenSSL.crypto.Error as error: raise errors.DeserializationError(error) + def encode_csr(csr): """Encode CSR as JOSE Base-64 DER. @@ -394,6 +400,7 @@ def encode_csr(csr): return encode_b64jose(OpenSSL.crypto.dump_certificate_request( OpenSSL.crypto.FILETYPE_ASN1, csr)) + def decode_csr(b64der): """Decode JOSE Base-64 DER-encoded CSR. diff --git a/acme/acme/jose/json_util_test.py b/acme/acme/jose/json_util_test.py index 313282e67..a055f3bf7 100644 --- a/acme/acme/jose/json_util_test.py +++ b/acme/acme/jose/json_util_test.py @@ -52,6 +52,7 @@ class FieldTest(unittest.TestCase): # pylint: disable=missing-docstring def to_partial_json(self): return 'foo' # pragma: no cover + @classmethod def from_json(cls, jobj): pass # pragma: no cover @@ -93,14 +94,18 @@ class JSONObjectWithFieldsMetaTest(unittest.TestCase): self.field2 = Field('Baz2') # pylint: disable=invalid-name,missing-docstring,too-few-public-methods # pylint: disable=blacklisted-name + @six.add_metaclass(JSONObjectWithFieldsMeta) class A(object): __slots__ = ('bar',) baz = self.field + class B(A): pass + class C(A): baz = self.field2 + self.a_cls = A self.b_cls = B self.c_cls = C diff --git a/acme/acme/jose/jwa.py b/acme/acme/jose/jwa.py index 0c84905df..4ce5ca3f5 100644 --- a/acme/acme/jose/jwa.py +++ b/acme/acme/jose/jwa.py @@ -21,7 +21,7 @@ from acme.jose import jwk logger = logging.getLogger(__name__) -class JWA(interfaces.JSONDeSerializable): # pylint: disable=abstract-method +class JWA(interfaces.JSONDeSerializable): # pylint: disable=abstract-method # pylint: disable=too-few-public-methods # for some reason disable=abstract-method has to be on the line # above... @@ -159,7 +159,7 @@ class _JWAES(JWASignature): # pylint: disable=abstract-class-not-used def sign(self, key, msg): # pragma: no cover raise NotImplementedError() - def verify(self, key, msg, sig): # pragma: no cover + def verify(self, key, msg, sig): # pragma: no cover raise NotImplementedError() diff --git a/acme/acme/jose/jwk.py b/acme/acme/jose/jwk.py index d9b903eb0..7a976f189 100644 --- a/acme/acme/jose/jwk.py +++ b/acme/acme/jose/jwk.py @@ -231,7 +231,7 @@ class JWKRSA(JWK): 'n': numbers.n, 'e': numbers.e, } - else: # rsa.RSAPrivateKey + else: # rsa.RSAPrivateKey private = self.key.private_numbers() public = self.key.public_key().public_numbers() params = { diff --git a/acme/acme/jose/jws.py b/acme/acme/jose/jws.py index 392a2f074..9a9a7621f 100644 --- a/acme/acme/jose/jws.py +++ b/acme/acme/jose/jws.py @@ -294,10 +294,10 @@ class JWS(json_util.JSONObjectWithFields): # ... it must be in protected return ( - b64.b64encode(self.signature.protected.encode('utf-8')) - + b'.' + - b64.b64encode(self.payload) - + b'.' + + b64.b64encode(self.signature.protected.encode('utf-8')) + + b'.' + + b64.b64encode(self.payload) + + b'.' + b64.b64encode(self.signature.signature)) @classmethod @@ -345,6 +345,7 @@ class JWS(json_util.JSONObjectWithFields): signatures=tuple(cls.signature_cls.from_json(sig) for sig in jobj['signatures'])) + class CLI(object): """JWS CLI.""" diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 970cf4e6e..7b9702278 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -216,16 +216,19 @@ class Registration(ResourceBody): """All emails found in the ``contact`` field.""" return self._filter_contact(self.email_prefix) + class NewRegistration(Registration): """New registration.""" resource_type = 'new-reg' resource = fields.Resource(resource_type) + class UpdateRegistration(Registration): """Update registration.""" resource_type = 'reg' resource = fields.Resource(resource_type) + class RegistrationResource(ResourceWithURI): """Registration Resource. @@ -328,11 +331,13 @@ class Authorization(ResourceBody): return tuple(tuple(self.challenges[idx] for idx in combo) for combo in self.combinations) + class NewAuthorization(Authorization): """New authorization.""" resource_type = 'new-authz' resource = fields.Resource(resource_type) + class AuthorizationResource(ResourceWithURI): """Authorization Resource. diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py index 481c2e2a3..fc3e3c97e 100644 --- a/acme/acme/messages_test.py +++ b/acme/acme/messages_test.py @@ -60,6 +60,7 @@ class ConstantTest(unittest.TestCase): def setUp(self): from acme.messages import _Constant + class MockConstant(_Constant): # pylint: disable=missing-docstring POSSIBLE_NAMES = {} @@ -211,7 +212,6 @@ class ChallengeBodyTest(unittest.TestCase): 'detail': 'Unable to communicate with DNS server', } - def test_to_partial_json(self): self.assertEqual(self.jobj_to, self.challb.to_partial_json()) diff --git a/acme/acme/test_util.py b/acme/acme/test_util.py index 8ad118e17..d6fe7d11d 100644 --- a/acme/acme/test_util.py +++ b/acme/acme/test_util.py @@ -20,12 +20,14 @@ def vector_path(*names): return pkg_resources.resource_filename( __name__, os.path.join('testdata', *names)) + def load_vector(*names): """Load contents of a test vector.""" # luckily, resource_string opens file in binary mode return pkg_resources.resource_string( __name__, os.path.join('testdata', *names)) + def _guess_loader(filename, loader_pem, loader_der): _, ext = os.path.splitext(filename) if ext.lower() == '.pem': @@ -35,6 +37,7 @@ def _guess_loader(filename, loader_pem, loader_der): else: # pragma: no cover raise ValueError("Loader could not be recognized based on extension") + def load_cert(*names): """Load certificate.""" loader = _guess_loader( @@ -42,6 +45,7 @@ def load_cert(*names): return jose.ComparableX509(OpenSSL.crypto.load_certificate( loader, load_vector(*names))) + def load_csr(*names): """Load certificate request.""" loader = _guess_loader( @@ -49,6 +53,7 @@ def load_csr(*names): return jose.ComparableX509(OpenSSL.crypto.load_certificate_request( loader, load_vector(*names))) + def load_rsa_private_key(*names): """Load RSA private key.""" loader = _guess_loader(names[-1], serialization.load_pem_private_key, @@ -56,6 +61,7 @@ def load_rsa_private_key(*names): return jose.ComparableRSAKey(loader( load_vector(*names), password=None, backend=default_backend())) + def load_pyopenssl_private_key(*names): """Load pyOpenSSL private key.""" loader = _guess_loader( From 83185e55538cd5c87fa7e63fefec8c6348b235c5 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:20:11 +0000 Subject: [PATCH 04/44] pep8 letsencrypt --- letsencrypt/auth_handler.py | 5 ++-- letsencrypt/cli.py | 10 ++++---- letsencrypt/client.py | 4 ++-- letsencrypt/configuration.py | 2 +- letsencrypt/crypto_util.py | 1 + letsencrypt/display/ops.py | 4 ++-- letsencrypt/display/util.py | 4 ++-- letsencrypt/errors.py | 1 + letsencrypt/interfaces.py | 1 - letsencrypt/le_util.py | 2 ++ letsencrypt/plugins/common.py | 2 ++ letsencrypt/plugins/disco_test.py | 1 + .../standalone/tests/authenticator_test.py | 12 ++++------ letsencrypt/proof_of_possession.py | 4 ++-- letsencrypt/storage.py | 5 ++-- letsencrypt/tests/acme_util.py | 2 +- letsencrypt/tests/auth_handler_test.py | 24 +++++++++++-------- letsencrypt/tests/cli_test.py | 2 +- letsencrypt/tests/client_test.py | 2 +- letsencrypt/tests/continuity_auth_test.py | 2 +- letsencrypt/tests/display/ops_test.py | 1 + letsencrypt/tests/display/util_test.py | 2 +- letsencrypt/tests/notify_test.py | 5 ++-- letsencrypt/tests/proof_of_possession_test.py | 2 +- letsencrypt/tests/renewer_test.py | 4 ++-- letsencrypt/tests/validator_test.py | 3 ++- 26 files changed, 59 insertions(+), 48 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 00c30fe3d..6498a5c19 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -244,7 +244,7 @@ class AuthHandler(object): """ for authzr_challb in authzr.body.challenges: - if type(authzr_challb.chall) is type(achall.challb.chall): + if type(authzr_challb.chall) is type(achall.challb.chall): # noqa return authzr_challb raise errors.AuthorizationError( "Target challenge not found in authorization resource") @@ -512,7 +512,8 @@ _ERROR_HELP = { "to date TLS configuration that allows the server to communicate with " "the Let's Encrypt client.", "unauthorized": _ERROR_HELP_COMMON, - "unknownHost": _ERROR_HELP_COMMON,} + "unknownHost": _ERROR_HELP_COMMON, +} def _report_failed_challs(failed_achalls): diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 066aa388d..7765233e4 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -69,7 +69,7 @@ Choice of server for authentication/installation: More detailed help: - -h, --help [topic] print this message, or detailed help on a topic; + -h, --help [topic] print this message, or detailed help on a topic; the available topics are: all, apache, automation, nginx, paths, security, testing, or any of the @@ -334,6 +334,7 @@ class SilentParser(object): # pylint: disable=too-few-public-methods """ def __init__(self, parser): self.parser = parser + def add_argument(self, *args, **kwargs): """Wrap, but silence help""" kwargs["help"] = argparse.SUPPRESS @@ -362,14 +363,14 @@ class HelpfulArgumentParser(object): default_config_files=flag_default("config_files")) # This is the only way to turn off overly verbose config flag documentation - self.parser._add_config_file_help = False # pylint: disable=protected-access + self.parser._add_config_file_help = False # pylint: disable=protected-access self.silent_parser = SilentParser(self.parser) help1 = self.prescan_for_flag("-h", self.help_topics) help2 = self.prescan_for_flag("--help", self.help_topics) assert max(True, "a") == "a", "Gravity changed direction" help_arg = max(help1, help2) - if help_arg == True: + if help_arg: # just --help with no topic; avoid argparse altogether print USAGE sys.exit(0) @@ -546,6 +547,7 @@ def create_parser(plugins, args): def _create_subparsers(helpful): subparsers = helpful.parser.add_subparsers(metavar="SUBCOMMAND") + def add_subparser(name, func): # pylint: disable=missing-docstring subparser = subparsers.add_parser( name, help=func.__doc__.splitlines()[0], description=func.__doc__) @@ -701,7 +703,7 @@ def _handle_exception(exc_type, exc_value, trace, args): with open(logfile, "w") as logfd: traceback.print_exception( exc_type, exc_value, trace, file=logfd) - except: # pylint: disable=bare-except + except: # pylint: disable=bare-except sys.exit("".join( traceback.format_exception(exc_type, exc_value, trace))) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e8dd08c8e..259f8b922 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -279,8 +279,8 @@ class Client(object): :param .RenewableCert cert: Newly issued certificate """ - if ("autorenew" not in cert.configuration - or cert.configuration.as_bool("autorenew")): + if ("autorenew" not in cert.configuration or + cert.configuration.as_bool("autorenew")): if ("autodeploy" not in cert.configuration or cert.configuration.as_bool("autodeploy")): msg = "Automatic renewal and deployment has " diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index c7c780535..6f3ece9fd 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -45,7 +45,7 @@ class NamespaceConfig(object): return (parsed.netloc + parsed.path).replace('/', os.path.sep) @property - def accounts_dir(self): #pylint: disable=missing-docstring + def accounts_dir(self): # pylint: disable=missing-docstring return os.path.join( self.namespace.config_dir, constants.ACCOUNTS_DIR, self.server_path) diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 279330f0c..1d807fcd9 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -205,6 +205,7 @@ def _pyopenssl_load(data, method, types=( raise errors.Error("Unable to load: {0}".format(",".join( str(error) for error in openssl_errors))) + def pyopenssl_load_certificate(data): """Load PEM/DER certificate. diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index a220d07d9..92d1978f9 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -25,8 +25,8 @@ def choose_plugin(prepared, question): :rtype: `~.PluginEntryPoint` """ - opts = [plugin_ep.description_with_name - + (" [Misconfigured]" if plugin_ep.misconfigured else "") + opts = [plugin_ep.description_with_name + + (" [Misconfigured]" if plugin_ep.misconfigured else "") for plugin_ep in prepared] while True: diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index de3e829fe..0e9c76e38 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -76,7 +76,7 @@ class NcursesDisplay(object): "help_label": help_label, "width": self.width, "height": self.height, - "menu_height": self.height-6, + "menu_height": self.height - 6, } # Can accept either tuples or just the actual choices @@ -315,7 +315,7 @@ class FileDisplay(object): if index < 1 or index > len(tags): return [] # Transform indices to appropriate tags - return [tags[index-1] for index in indices] + return [tags[index - 1] for index in indices] def _print_menu(self, message, choices): """Print a menu on the screen. diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py index b15728c39..ba0601d29 100644 --- a/letsencrypt/errors.py +++ b/letsencrypt/errors.py @@ -73,6 +73,7 @@ class NoInstallationError(PluginError): class MisconfigurationError(PluginError): """Let's Encrypt Misconfiguration error.""" + class NotSupportedError(PluginError): """Let's Encrypt Plugin function not supported error.""" diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index f330e28ce..d57d1a15f 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -440,7 +440,6 @@ class IValidator(zope.interface.Interface): """ - def hsts(name): """Verify HSTS header is enabled diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index f8c911d99..194a80201 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -196,6 +196,8 @@ def safely_remove(path): # start with a period or have two consecutive periods <- this needs to # be done in addition to the regex EMAIL_REGEX = re.compile("[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+$") + + def safe_email(email): """Scrub email address before using it.""" if EMAIL_REGEX.match(email) is not None: diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index bef8b4d81..59598a35e 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -18,6 +18,7 @@ def option_namespace(name): """ArgumentParser options namespace (prefix of all options).""" return name + "-" + def dest_namespace(name): """ArgumentParser dest namespace (prefix of all destinations).""" return name.replace("-", "_") + "_" @@ -86,6 +87,7 @@ class Plugin(object): # other + class Addr(object): r"""Represents an virtual host address. diff --git a/letsencrypt/plugins/disco_test.py b/letsencrypt/plugins/disco_test.py index 56808c7da..41699d1ef 100644 --- a/letsencrypt/plugins/disco_test.py +++ b/letsencrypt/plugins/disco_test.py @@ -101,6 +101,7 @@ class PluginEntryPointTest(unittest.TestCase): with mock.patch("letsencrypt.plugins." "disco.zope.interface") as mock_zope: mock_zope.exceptions = exceptions + def verify_object(iface, obj): # pylint: disable=missing-docstring assert obj is plugin assert iface is iface1 or iface is iface2 or iface is iface3 diff --git a/letsencrypt/plugins/standalone/tests/authenticator_test.py b/letsencrypt/plugins/standalone/tests/authenticator_test.py index bae20ac4d..7ff2c03e1 100644 --- a/letsencrypt/plugins/standalone/tests/authenticator_test.py +++ b/letsencrypt/plugins/standalone/tests/authenticator_test.py @@ -321,10 +321,8 @@ class PerformTest(unittest.TestCase): self.authenticator.already_listening = mock.Mock(return_value=False) result = self.authenticator.perform(self.achalls) self.assertEqual(len(self.authenticator.tasks), 2) - self.assertTrue( - self.authenticator.tasks.has_key(self.achall1.token)) - self.assertTrue( - self.authenticator.tasks.has_key(self.achall2.token)) + self.assertTrue(self.achall1.token in self.authenticator.tasks) + self.assertTrue(self.achall2.token in self.authenticator.tasks) self.assertTrue(isinstance(result, list)) self.assertEqual(len(result), 3) self.assertTrue(isinstance(result[0], challenges.ChallengeResponse)) @@ -340,10 +338,8 @@ class PerformTest(unittest.TestCase): self.authenticator.already_listening = mock.Mock(return_value=False) result = self.authenticator.perform(self.achalls) self.assertEqual(len(self.authenticator.tasks), 2) - self.assertTrue( - self.authenticator.tasks.has_key(self.achall1.token)) - self.assertTrue( - self.authenticator.tasks.has_key(self.achall2.token)) + self.assertTrue(self.achall1.token in self.authenticator.tasks) + self.assertTrue(self.achall2.token in self.authenticator.tasks) self.assertTrue(isinstance(result, list)) self.assertEqual(len(result), 3) self.assertEqual(result, [None, None, False]) diff --git a/letsencrypt/proof_of_possession.py b/letsencrypt/proof_of_possession.py index f13238c85..7928c60e7 100644 --- a/letsencrypt/proof_of_possession.py +++ b/letsencrypt/proof_of_possession.py @@ -17,7 +17,7 @@ from letsencrypt.display import util as display_util logger = logging.getLogger(__name__) -class ProofOfPossession(object): # pylint: disable=too-few-public-methods +class ProofOfPossession(object): # pylint: disable=too-few-public-methods """Proof of Possession Identifier Validation Challenge. Based on draft-barnes-acme, section 6.5. @@ -71,7 +71,7 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods # If we get here, the key wasn't found return False - def _gen_response(self, achall, key_path): # pylint: disable=no-self-use + def _gen_response(self, achall, key_path): # pylint: disable=no-self-use """Create the response to the Proof of Possession Challenge. :param achall: Proof of Possession Challenge diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 431f56aff..2ff11ae49 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -486,8 +486,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :rtype: bool """ - if ("autorenew" not in self.configuration - or self.configuration.as_bool("autorenew")): + if ("autorenew" not in self.configuration or + self.configuration.as_bool("autorenew")): # Consider whether to attempt to autorenew this cert now # Renewals on the basis of revocation @@ -603,7 +603,6 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes new_config.write() return cls(new_config, config, cli_config) - def save_successor(self, prior_version, new_cert, new_privkey, new_chain): """Save new cert and chain as a successor of a prior version. diff --git a/letsencrypt/tests/acme_util.py b/letsencrypt/tests/acme_util.py index 33bf605e0..235810435 100644 --- a/letsencrypt/tests/acme_util.py +++ b/letsencrypt/tests/acme_util.py @@ -30,7 +30,7 @@ POP = challenges.ProofOfPossession( "16d95b7b63f1972b980b14c20291f3c0d1855d95", "48b46570d9fc6358108af43ad1649484def0debf" ), - certs=(), # TODO + certs=(), # TODO subject_key_identifiers=("d0083162dcc4c8a23ecb8aecbd86120e56fd24e5"), serial_numbers=(34234239832, 23993939911, 17), issuers=( diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 2127c8f5c..ed29ead25 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -37,7 +37,7 @@ class ChallengeFactoryTest(unittest.TestCase): self.dom = "test" self.handler.authzr[self.dom] = acme_util.gen_authzr( messages.STATUS_PENDING, self.dom, acme_util.CHALLENGES, - [messages.STATUS_PENDING]*6, False) + [messages.STATUS_PENDING] * 6, False) def test_all(self): cont_c, dv_c = self.handler._challenge_factory( @@ -163,7 +163,7 @@ class GetAuthorizationsTest(unittest.TestCase): messages.STATUS_VALID, dom, [challb.chall for challb in azr.body.challenges], - [messages.STATUS_VALID]*len(azr.body.challenges), + [messages.STATUS_VALID] * len(azr.body.challenges), azr.body.combinations) @@ -183,15 +183,15 @@ class PollChallengesTest(unittest.TestCase): self.doms = ["0", "1", "2"] self.handler.authzr[self.doms[0]] = acme_util.gen_authzr( messages.STATUS_PENDING, self.doms[0], - acme_util.DV_CHALLENGES, [messages.STATUS_PENDING]*3, False) + acme_util.DV_CHALLENGES, [messages.STATUS_PENDING] * 3, False) self.handler.authzr[self.doms[1]] = acme_util.gen_authzr( messages.STATUS_PENDING, self.doms[1], - acme_util.DV_CHALLENGES, [messages.STATUS_PENDING]*3, False) + acme_util.DV_CHALLENGES, [messages.STATUS_PENDING] * 3, False) self.handler.authzr[self.doms[2]] = acme_util.gen_authzr( messages.STATUS_PENDING, self.doms[2], - acme_util.DV_CHALLENGES, [messages.STATUS_PENDING]*3, False) + acme_util.DV_CHALLENGES, [messages.STATUS_PENDING] * 3, False) self.chall_update = {} for dom in self.doms: @@ -282,6 +282,7 @@ class PollChallengesTest(unittest.TestCase): ) return (new_authzr, "response") + class GenChallengePathTest(unittest.TestCase): """Tests for letsencrypt.auth_handler.gen_challenge_path. @@ -321,7 +322,7 @@ class GenChallengePathTest(unittest.TestCase): combos = acme_util.gen_combos(challbs) self.assertEqual(self._call(challbs, prefs, combos), (0, 2)) - # dumb_path() trivial test + # dumb_path() trivial test self.assertTrue(self._call(challbs, prefs, None)) def test_full_cont_server(self): @@ -434,19 +435,22 @@ class ReportFailedChallsTest(unittest.TestCase): } self.simple_http = achallenges.SimpleHTTP( - challb=messages.ChallengeBody(**kwargs),# pylint: disable=star-args + # pylint: disable=star-args + challb=messages.ChallengeBody(**kwargs), domain="example.com", account_key="key") kwargs["chall"] = acme_util.DVSNI self.dvsni_same = achallenges.DVSNI( - challb=messages.ChallengeBody(**kwargs),# pylint: disable=star-args + # pylint: disable=star-args + challb=messages.ChallengeBody(**kwargs), domain="example.com", account_key="key") kwargs["error"] = messages.Error(typ="dnssec", detail="detail") self.dvsni_diff = achallenges.DVSNI( - challb=messages.ChallengeBody(**kwargs),# pylint: disable=star-args + # pylint: disable=star-args + challb=messages.ChallengeBody(**kwargs), domain="foo.bar", account_key="key") @@ -477,7 +481,7 @@ def gen_dom_authzr(domain, unused_new_authzr_uri, challs): """Generates new authzr for domains.""" return acme_util.gen_authzr( messages.STATUS_PENDING, domain, challs, - [messages.STATUS_PENDING]*len(challs)) + [messages.STATUS_PENDING] * len(challs)) if __name__ == "__main__": diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 613c3189b..312137666 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -60,7 +60,7 @@ class CLITest(unittest.TestCase): for args in itertools.chain( *(itertools.combinations(flags, r) for r in xrange(len(flags)))): - self._call(['plugins',] + list(args)) + self._call(['plugins'] + list(args)) @mock.patch("letsencrypt.cli.sys") def test_handle_exception(self, mock_sys): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index b992089cc..1a36f2371 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -166,7 +166,7 @@ class RollbackTest(unittest.TestCase): self.assertEqual(self.m_install().restart.call_count, 1) def test_no_installer(self): - self._call(1, None) # Just make sure no exceptions are raised + self._call(1, None) # Just make sure no exceptions are raised if __name__ == "__main__": diff --git a/letsencrypt/tests/continuity_auth_test.py b/letsencrypt/tests/continuity_auth_test.py index f8238a727..d80a1cfb4 100644 --- a/letsencrypt/tests/continuity_auth_test.py +++ b/letsencrypt/tests/continuity_auth_test.py @@ -35,7 +35,7 @@ class PerformTest(unittest.TestCase): self.assertRaises( errors.ContAuthError, self.auth.perform, [ achallenges.DVSNI( - challb=None, domain="0", account_key="invalid_key"),]) + challb=None, domain="0", account_key="invalid_key")]) def test_chall_pref(self): self.assertEqual( diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index fc4013bed..019139c35 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -250,6 +250,7 @@ class GenSSLLabURLs(unittest.TestCase): self.assertTrue("eff.org" in urls[0]) self.assertTrue("umich.edu" in urls[1]) + class GenHttpsNamesTest(unittest.TestCase): """Test _gen_https_names.""" def setUp(self): diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index 41075c9ce..001a9e578 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -35,7 +35,7 @@ class NcursesDisplayTest(unittest.TestCase): "help_label": "", "width": display_util.WIDTH, "height": display_util.HEIGHT, - "menu_height": display_util.HEIGHT-6, + "menu_height": display_util.HEIGHT - 6, } @mock.patch("letsencrypt.display.util.dialog.Dialog.msgbox") diff --git a/letsencrypt/tests/notify_test.py b/letsencrypt/tests/notify_test.py index 1ccfdbf87..60364fff8 100644 --- a/letsencrypt/tests/notify_test.py +++ b/letsencrypt/tests/notify_test.py @@ -1,9 +1,10 @@ """Tests for letsencrypt.notify.""" - -import mock import socket import unittest +import mock + + class NotifyTests(unittest.TestCase): """Tests for the notifier.""" diff --git a/letsencrypt/tests/proof_of_possession_test.py b/letsencrypt/tests/proof_of_possession_test.py index bfe3478d1..f2e7b2021 100644 --- a/letsencrypt/tests/proof_of_possession_test.py +++ b/letsencrypt/tests/proof_of_possession_test.py @@ -80,4 +80,4 @@ class ProofOfPossessionTest(unittest.TestCase): if __name__ == "__main__": - unittest.main() # pragma: no cover + unittest.main() # pragma: no cover diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 1b58d9e0f..898dd406f 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -24,6 +24,7 @@ def unlink_all(rc_object): for kind in ALL_FOUR: os.unlink(getattr(rc_object, kind)) + def fill_with_sample_data(rc_object): """Put dummy data into all four files of this RenewableCert.""" for kind in ALL_FOUR: @@ -97,7 +98,7 @@ class RenewableCertTests(unittest.TestCase): self.assertRaises( errors.CertStorageError, storage.RenewableCert, config, defaults) - def test_consistent(self): # pylint: disable=too-many-statements + def test_consistent(self): # pylint: disable=too-many-statements oldcert = self.test_rc.cert self.test_rc.cert = "relative/path" # Absolute path for item requirement @@ -608,7 +609,6 @@ class RenewableCertTests(unittest.TestCase): # This should fail because the renewal itself appears to fail self.assertFalse(renewer.renew(self.test_rc, 1)) - @mock.patch("letsencrypt.renewer.notify") @mock.patch("letsencrypt.storage.RenewableCert") @mock.patch("letsencrypt.renewer.renew") diff --git a/letsencrypt/tests/validator_test.py b/letsencrypt/tests/validator_test.py index 5ce5fa557..c7416dc46 100644 --- a/letsencrypt/tests/validator_test.py +++ b/letsencrypt/tests/validator_test.py @@ -106,6 +106,7 @@ class ValidatorTest(unittest.TestCase): self.assertRaises( NotImplementedError, self.validator.ocsp_stapling, "test.com") + def create_response(status_code=200, headers=None): """Creates a requests.Response object for testing""" response = requests.Response() @@ -118,4 +119,4 @@ def create_response(status_code=200, headers=None): if __name__ == '__main__': - unittest.main() # pragma: no cover + unittest.main() # pragma: no cover From 95c8edc66cd4d4a7c9515a702171dd3d20a5a65d Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:20:41 +0000 Subject: [PATCH 05/44] pep8 letsencrypt-apache --- letsencrypt-apache/letsencrypt_apache/configurator.py | 10 +++------- letsencrypt-apache/letsencrypt_apache/obj.py | 4 ++-- letsencrypt-apache/letsencrypt_apache/parser.py | 5 ++--- .../letsencrypt_apache/tests/complex_parsing_test.py | 2 -- .../letsencrypt_apache/tests/configurator_test.py | 1 + letsencrypt-apache/setup.py | 2 +- 6 files changed, 9 insertions(+), 15 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 8403b974c..1ddf913b5 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -84,7 +84,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): description = "Apache Web Server - Alpha" - @classmethod def add_parser_arguments(cls, add): add("ctl", default=constants.CLI_DEFAULTS["ctl"], @@ -283,7 +282,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.assoc[target_name] = vhost return vhost - def _find_best_vhost(self, target_name): """Finds the best vhost for a target_name. @@ -583,7 +581,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): ssl_vhost = self._create_vhost(vh_p) self.vhosts.append(ssl_vhost) - # NOTE: Searches through Augeas seem to ruin changes to directives # The configuration must also be saved before being searched # for the new directives; For these reasons... this is tacked @@ -794,7 +791,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.PluginError( "Let's Encrypt has already enabled redirection") - def _create_redirect_vhost(self, ssl_vhost): """Creates an http_vhost specifically to redirect for the ssl_vhost. @@ -997,9 +993,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ # Support Debian specific setup - if (not os.path.isdir(os.path.join(self.parser.root, "mods-available")) - or not os.path.isdir( - os.path.join(self.parser.root, "mods-enabled"))): + avail_path = os.path.join(self.parser.root, "mods-available") + enabled_path = os.path.join(self.parser.root, "mods-enabled") + if not os.path.isdir(avail_path) or not os.path.isdir(enabled_path): raise errors.NotSupportedError( "Unsupported directory layout. You may try to enable mod %s " "and try again." % mod_name) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 8cd2378a4..58a6c740e 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -14,8 +14,8 @@ class Addr(common.Addr): """ if isinstance(other, self.__class__): return ((self.tup == other.tup) or - (self.tup[0] == other.tup[0] - and self.is_wildcard() and other.is_wildcard())) + (self.tup[0] == other.tup[0] and + self.is_wildcard() and other.is_wildcard())) return False def __ne__(self, other): diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index da3fc97e7..d7dc3c422 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -195,8 +195,7 @@ class ApacheParser(object): self.aug.set(nvh_path + "/arg", args[0]) else: for i, arg in enumerate(args): - self.aug.set("%s/arg[%d]" % (nvh_path, i+1), arg) - + self.aug.set("%s/arg[%d]" % (nvh_path, i + 1), arg) def _get_ifmod(self, aug_conf_path, mod): """Returns the path to and creates one if it doesn't exist. @@ -568,7 +567,7 @@ def case_i(string): :param str string: string to make case i regex """ - return "".join(["["+c.upper()+c.lower()+"]" + return "".join(["[" + c.upper() + c.lower() + "]" if c.isalpha() else c for c in re.escape(string)]) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py index 406b6c39e..e7bd03cc5 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/complex_parsing_test.py @@ -56,7 +56,6 @@ class ComplexParserTest(util.ParserTest): self.assertRaises( errors.PluginError, self.parser.get_arg, matches[0]) - def test_basic_ifdefine(self): self.assertEqual(len(self.parser.find_dir("VAR_DIRECTIVE")), 2) self.assertEqual(len(self.parser.find_dir("INVALID_VAR_DIRECTIVE")), 0) @@ -71,7 +70,6 @@ class ComplexParserTest(util.ParserTest): self.assertEqual( len(self.parser.find_dir("INVALID_NESTED_DIRECTIVE")), 0) - def test_load_modules(self): """If only first is found, there is bad variable parsing.""" self.assertTrue("status_module" in self.parser.modules) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 71599bd1d..026594a8f 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -551,6 +551,7 @@ class TwoVhost80Test(util.ApacheTest): self.assertRaises( errors.PluginError, self.config.enhance, "letsencrypt.demo", "redirect") + def test_unknown_rewrite2(self): # Skip the enable mod self.config.parser.modules.add("rewrite_module") diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index 39f4b68e1..5ecb071c7 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -18,7 +18,7 @@ setup( entry_points={ 'letsencrypt.plugins': [ 'apache = letsencrypt_apache.configurator:ApacheConfigurator', - ], + ], }, include_package_data=True, ) From d8c55f3da317ef2e23be1a57957401272851aaa7 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:21:03 +0000 Subject: [PATCH 06/44] pep8 letsencrypt-nginx --- letsencrypt-nginx/letsencrypt_nginx/nginxparser.py | 9 +++++---- letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py | 1 - letsencrypt-nginx/setup.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py b/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py index 814b5f15e..2926a43d0 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py +++ b/letsencrypt-nginx/letsencrypt_nginx/nginxparser.py @@ -7,6 +7,7 @@ from pyparsing import ( from pyparsing import stringEnd from pyparsing import restOfLine + class RawNginxParser(object): # pylint: disable=expression-not-assigned """A class that parses nginx configuration with pyparsing.""" @@ -32,10 +33,10 @@ class RawNginxParser(object): block = Forward() block << Group( - (Group(key + location_statement) ^ Group(if_statement)) - + left_bracket - + Group(ZeroOrMore(Group(comment | assignment) | block)) - + right_bracket) + (Group(key + location_statement) ^ Group(if_statement)) + + left_bracket + + Group(ZeroOrMore(Group(comment | assignment) | block)) + + right_bracket) script = OneOrMore(Group(comment | assignment) ^ block) + stringEnd diff --git a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py index a164397b6..a09bebba2 100644 --- a/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py +++ b/letsencrypt-nginx/letsencrypt_nginx/tests/dvsni_test.py @@ -41,7 +41,6 @@ class DvsniPerformTest(util.NginxTest): domain="www.example.org", account_key=account_key), ] - def setUp(self): super(DvsniPerformTest, self).setUp() diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index 92b974974..2448da71b 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -17,7 +17,7 @@ setup( entry_points={ 'letsencrypt.plugins': [ 'nginx = letsencrypt_nginx.configurator:NginxConfigurator', - ], + ], }, include_package_data=True, ) From 413bd6f425c0a123d9cce5d23b568b9c2f8d3b74 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:21:28 +0000 Subject: [PATCH 07/44] pep8 letsencrypt-compatibility-test --- .../configurators/apache/apache24.py | 4 ++-- .../letsencrypt_compatibility_test/configurators/common.py | 7 +++---- .../letsencrypt_compatibility_test/test_driver.py | 4 ++-- .../letsencrypt_compatibility_test/util.py | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/apache24.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/apache24.py index 2ffc44976..3cc6fdf8e 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/apache24.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/apache/apache24.py @@ -11,7 +11,7 @@ from letsencrypt_compatibility_test.configurators.apache import common as apache # config uses mod_heartbeat or mod_heartmonitor (which aren't installed and # therefore the config won't be loaded), I believe this isn't a problem # http://httpd.apache.org/docs/2.4/mod/mod_watchdog.html -STATIC_MODULES = {"core", "so", "http", "mpm_event", "watchdog",} +STATIC_MODULES = set(["core", "so", "http", "mpm_event", "watchdog"]) SHARED_MODULES = { @@ -31,7 +31,7 @@ SHARED_MODULES = { "session_cookie", "session_crypto", "session_dbd", "setenvif", "slotmem_shm", "socache_dbm", "socache_memcache", "socache_shmcb", "speling", "ssl", "status", "substitute", "unique_id", "userdir", - "vhost_alias",} + "vhost_alias"} class Proxy(apache_common.Proxy): diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/common.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/common.py index 65f14bbe9..7c5e5dfcb 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/common.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/configurators/common.py @@ -72,11 +72,10 @@ class Proxy(object): logger.debug(line) host_config = docker.utils.create_host_config( - binds={ - self._temp_dir : {"bind" : self._temp_dir, "mode" : "rw"}}, + binds={self._temp_dir: {"bind": self._temp_dir, "mode": "rw"}}, port_bindings={ - 80 : ("127.0.0.1", self.http_port), - 443 : ("127.0.0.1", self.https_port)},) + 80: ("127.0.0.1", self.http_port), + 443: ("127.0.0.1", self.https_port)},) container = self._docker_client.create_container( image_name, command, ports=[80, 443], volumes=self._temp_dir, host_config=host_config) diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py index eac2278bb..b91322c3c 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/test_driver.py @@ -30,7 +30,7 @@ tests that the plugin supports are performed. """ -PLUGINS = {"apache" : apache24.Proxy} +PLUGINS = {"apache": apache24.Proxy} logger = logging.getLogger(__name__) @@ -191,7 +191,7 @@ def test_enhancements(plugin, domains): success = True for domain in domains: verify = functools.partial(validator.Validator().redirect, "localhost", - plugin.http_port, headers={"Host" : domain}) + plugin.http_port, headers={"Host": domain}) if not _try_until_true(verify): logger.error("Improper redirect for domain %s", domain) success = False diff --git a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py index 03b15d217..43070cf03 100644 --- a/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py +++ b/letsencrypt-compatibility-test/letsencrypt_compatibility_test/util.py @@ -34,7 +34,7 @@ def create_le_config(parent_dir): os.mkdir(config["work_dir"]) os.mkdir(config["logs_dir"]) - return argparse.Namespace(**config) # pylint: disable=star-args + return argparse.Namespace(**config) # pylint: disable=star-args def extract_configs(configs, parent_dir): From 79a70cfd61d3418f85318d3cc1eab68bbe7855a1 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:21:57 +0000 Subject: [PATCH 08/44] pep8 letshelp-letsencrypt --- letshelp-letsencrypt/letshelp_letsencrypt/apache.py | 12 ++++++------ .../letshelp_letsencrypt/apache_test.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/apache.py b/letshelp-letsencrypt/letshelp_letsencrypt/apache.py index 3b3ab31e7..ac4e9b831 100755 --- a/letshelp-letsencrypt/letshelp_letsencrypt/apache.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/apache.py @@ -87,7 +87,7 @@ def copy_config(server_root, temp_dir): dir_len = len(os.path.dirname(server_root)) for config_path, config_dirs, config_files in os.walk(server_root): - temp_path = os.path.join(temp_dir, config_path[dir_len+1:]) + temp_path = os.path.join(temp_dir, config_path[dir_len + 1:]) os.mkdir(temp_path) copied_all = True @@ -151,7 +151,7 @@ def safe_config_file(config_file): empty_or_all_comments = False if line.startswith("-----BEGIN"): return False - elif not ":" in line: + elif ":" not in line: possible_password_file = False # If file isn't empty or commented out and could be a password file, # don't include it in selection. It is safe to include the file if @@ -234,9 +234,9 @@ def locate_config(apache_ctl): for line in output.splitlines(): # Relevant output lines are of the form: -D DIRECTIVE="VALUE" if "HTTPD_ROOT" in line: - server_root = line[line.find('"')+1:-1] + server_root = line[line.find('"') + 1:-1] elif "SERVER_CONFIG_FILE" in line: - config_file = line[line.find('"')+1:-1] + config_file = line[line.find('"') + 1:-1] if not (server_root and config_file): sys.exit("Unable to locate Apache configuration. Please run this " @@ -272,7 +272,7 @@ def get_args(): args.config_file = os.path.abspath(args.config_file) if args.config_file.startswith(args.server_root): - args.config_file = args.config_file[len(args.server_root)+1:] + args.config_file = args.config_file[len(args.server_root) + 1:] else: sys.exit("This script expects the Apache configuration file to be " "inside the server root") @@ -300,4 +300,4 @@ def main(): if __name__ == "__main__": - main() # pragma: no cover + main() # pragma: no cover diff --git a/letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py b/letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py index e1012797a..7ed1df760 100644 --- a/letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py +++ b/letshelp-letsencrypt/letshelp_letsencrypt/apache_test.py @@ -141,7 +141,7 @@ class LetsHelpApacheTest(unittest.TestCase): @mock.patch(_MODULE_NAME + ".subprocess.Popen") def test_locate_config(self, mock_popen): mock_popen().communicate.side_effect = [ - OSError, ("bad_output", None), (_COMPILE_SETTINGS, None),] + OSError, ("bad_output", None), (_COMPILE_SETTINGS, None)] self.assertRaises( SystemExit, letshelp_le_apache.locate_config, "ctl") From fe3e8d7302c71db4b7eb092a57be8b95bf9187c7 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:22:34 +0000 Subject: [PATCH 09/44] Travis: add pep8 checks --- .pep8 | 4 ++++ tox.ini | 2 ++ 2 files changed, 6 insertions(+) create mode 100644 .pep8 diff --git a/.pep8 b/.pep8 new file mode 100644 index 000000000..79a82a252 --- /dev/null +++ b/.pep8 @@ -0,0 +1,4 @@ +[pep8] +# E265 block comment should start with '# ' +# E501 line too long (X > 79 characters) +ignore = E265,E501 \ No newline at end of file diff --git a/tox.ini b/tox.ini index ebe9746c9..4caecf681 100644 --- a/tox.ini +++ b/tox.ini @@ -36,6 +36,8 @@ basepython = python2.7 # duplicate code checking; if one of the commands fails, others will # continue, but tox return code will reflect previous error commands = + pip install pep8 + pep8 setup.py acme letsencrypt letsencrypt-apache letsencrypt-nginx letsencrypt-compatibility-test letshelp-letsencrypt pip install -r requirements.txt -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt pylint --rcfile=.pylintrc letsencrypt pylint --rcfile=.pylintrc acme/acme From 6fab6c80b613334a5d1df85c214963a3e2c208f8 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 6 Sep 2015 09:27:39 +0000 Subject: [PATCH 10/44] nit: fix missing EOF newline --- .pep8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pep8 b/.pep8 index 79a82a252..22045d3d3 100644 --- a/.pep8 +++ b/.pep8 @@ -1,4 +1,4 @@ [pep8] # E265 block comment should start with '# ' # E501 line too long (X > 79 characters) -ignore = E265,E501 \ No newline at end of file +ignore = E265,E501 From dbf5d086bda683679a12f70b70d91aa2de04166b Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 22 Aug 2015 15:41:37 +0000 Subject: [PATCH 11/44] v4 DNS challenge --- acme/acme/challenges.py | 94 +++++++++++++++++++++++++++++++++++- acme/acme/challenges_test.py | 77 ++++++++++++++++++++++++++--- acme/acme/client_test.py | 10 ++-- acme/acme/messages_test.py | 8 +-- 4 files changed, 173 insertions(+), 16 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index a2235b61e..13186cc4f 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -514,10 +514,100 @@ class DNS(DVChallenge): """ typ = "dns" - token = jose.Field("token") + + LABEL = "_acme-challenge" + """Label clients prepend to the domain name being validated.""" + + TOKEN_SIZE = 128 / 8 # Based on the entropy value from the spec + """Minimum size of the :attr:`token` in bytes.""" + + token = jose.Field( + "token", encoder=jose.encode_b64jose, decoder=functools.partial( + jose.decode_b64jose, size=TOKEN_SIZE, minimum=True)) + + def gen_validation(self, account_key, alg=jose.RS256, **kwargs): + """Generate validation. + + :param .JWK account_key: Private account key. + :param .JWA alg: + + :returns: This challenge wrapped in `.JWS` + :rtype: .JWS + + """ + return jose.JWS.sign( + payload=self.json_dumps(sort_keys=True).encode('utf-8'), + key=account_key, alg=alg, **kwargs) + + def check_validation(self, validation, account_public_key): + """Check validation. + + :param validation + :type account_public_key: + `~cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey` + or + `~cryptography.hazmat.primitives.asymmetric.dsa.DSAPublicKey` + or + `~cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePublicKey` + wrapped in `.ComparableKey` + + :rtype: bool + + """ + if not validation.verify(key=account_public_key): + return False + try: + return self == self.json_loads( + validation.payload.decode('utf-8')) + except jose.DeserializationError as error: + logger.debug("Checking validation for DNS failed: %s", error) + return False + + def gen_response(self, account_key, **kwargs): + """Generate response. + + :param .JWK account_key: Private account key. + :param .JWA alg: + + :rtype: DNSResponse + + """ + return DNSResponse(validation=self.gen_validation( + self, account_key, **kwargs)) + + def validation_domain_name(self, name): + """Domain name for TXT validation record. + + :param unicode name: Domain name being validated. + + """ + return "{0}.{1}".format(self.LABEL, name) @ChallengeResponse.register class DNSResponse(ChallengeResponse): - """ACME "dns" challenge response.""" + """ACME "dns" challenge response. + + :param JWS validation: + + """ typ = "dns" + + validation = jose.Field("validation", decoder=jose.JWS.from_json) + + def check_validation(self, chall, account_public_key): + """Check validation. + + :param challenges.DNS chall: + :type account_public_key: + `~cryptography.hazmat.primitives.asymmetric.rsa.RSAPublicKey` + or + `~cryptography.hazmat.primitives.asymmetric.dsa.DSAPublicKey` + or + `~cryptography.hazmat.primitives.asymmetric.ec.EllipticCurvePublicKey` + wrapped in `.ComparableKey` + + :rtype: bool + + """ + return chall.check_validation(self.validation, account_public_key) diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index d123eca20..06f5dffe1 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -570,9 +570,15 @@ class ProofOfPossessionResponseTest(unittest.TestCase): class DNSTest(unittest.TestCase): def setUp(self): + self.account_key = jose.JWKRSA.load( + test_util.load_vector('rsa512_key.pem')) from acme.challenges import DNS - self.msg = DNS(token='17817c66b60ce2e4012dfad92657527a') - self.jmsg = {'type': 'dns', 'token': '17817c66b60ce2e4012dfad92657527a'} + self.msg = DNS(token=jose.b64decode( + b'evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA')) + self.jmsg = { + 'type': 'dns', + 'token': 'evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA', + } def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) @@ -585,27 +591,84 @@ class DNSTest(unittest.TestCase): from acme.challenges import DNS hash(DNS.from_json(self.jmsg)) + def test_gen_check_validation(self): + self.assertTrue(self.msg.check_validation( + self.msg.gen_validation(self.account_key), + self.account_key.public_key())) + + def test_gen_check_validation_wrong_key(self): + key2 = jose.JWKRSA.load(test_util.load_vector('rsa1024_key.pem')) + self.assertFalse(self.msg.check_validation( + self.msg.gen_validation(self.account_key), key2.public_key())) + + def test_check_validation_wrong_payload(self): + validations = tuple( + jose.JWS.sign(payload=payload, alg=jose.RS256, key=self.account_key) + for payload in (b'', b'{}') + ) + for validation in validations: + self.assertFalse(self.msg.check_validation( + validation, self.account_key.public_key())) + + 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'), + alg=jose.RS256, key=self.account_key) + self.assertFalse(self.msg.check_validation( + bad_validation, self.account_key.public_key())) + + def test_gen_response(self): + with mock.patch('acme.challenges.DNS.gen_validation') as mock_gen: + mock_gen.return_value = mock.sentinel.validation + response = self.msg.gen_response(self.account_key) + from acme.challenges import DNSResponse + self.assertTrue(isinstance(response, DNSResponse)) + self.assertEqual(response.validation, mock.sentinel.validation) + + def test_validation_domain_name(self): + self.assertEqual( + '_acme-challenge.le.wtf', self.msg.validation_domain_name('le.wtf')) + class DNSResponseTest(unittest.TestCase): def setUp(self): + self.key = jose.JWKRSA(key=KEY) + + from acme.challenges import DNS + self.chall = DNS(token=jose.b64decode( + b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA")) + self.validation = jose.JWS.sign( + payload=self.chall.json_dumps(sort_keys=True).encode(), + key=self.key, alg=jose.RS256) + from acme.challenges import DNSResponse - self.msg = DNSResponse() - self.jmsg = { + self.msg = DNSResponse(validation=self.validation) + self.jmsg_to = { 'resource': 'challenge', 'type': 'dns', + 'validation': self.validation, + } + self.jmsg_from = { + 'resource': 'challenge', + 'type': 'dns', + 'validation': self.validation.to_json(), } def test_to_partial_json(self): - self.assertEqual(self.jmsg, self.msg.to_partial_json()) + self.assertEqual(self.jmsg_to, self.msg.to_partial_json()) def test_from_json(self): from acme.challenges import DNSResponse - self.assertEqual(self.msg, DNSResponse.from_json(self.jmsg)) + self.assertEqual(self.msg, DNSResponse.from_json(self.jmsg_from)) def test_from_json_hashable(self): from acme.challenges import DNSResponse - hash(DNSResponse.from_json(self.jmsg)) + hash(DNSResponse.from_json(self.jmsg_from)) + + def test_check_validation(self): + self.assertTrue( + self.msg.check_validation(self.chall, self.key.public_key())) if __name__ == '__main__': diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index dcc0832e3..ed0c6f65a 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -55,7 +55,8 @@ class ClientTest(unittest.TestCase): authzr_uri = 'https://www.letsencrypt-demo.org/acme/authz/1' challb = messages.ChallengeBody( uri=(authzr_uri + '/1'), status=messages.STATUS_VALID, - chall=challenges.DNS(token='foo')) + chall=challenges.DNS(token=jose.b64decode( + 'evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA'))) self.challr = messages.ChallengeResource( body=challb, authzr_uri=authzr_uri) self.authz = messages.Authorization( @@ -155,7 +156,7 @@ class ClientTest(unittest.TestCase): self.response.links['up'] = {'url': self.challr.authzr_uri} self.response.json.return_value = self.challr.body.to_json() - chall_response = challenges.DNSResponse() + chall_response = challenges.DNSResponse(validation=None) self.client.answer_challenge(self.challr.body, chall_response) @@ -164,8 +165,9 @@ class ClientTest(unittest.TestCase): self.challr.body.update(uri='foo'), chall_response) def test_answer_challenge_missing_next(self): - self.assertRaises(errors.ClientError, self.client.answer_challenge, - self.challr.body, challenges.DNSResponse()) + self.assertRaises( + errors.ClientError, self.client.answer_challenge, + self.challr.body, challenges.DNSResponse(validation=None)) def test_retry_after_date(self): self.response.headers['Retry-After'] = 'Fri, 31 Dec 1999 23:59:59 GMT' diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py index 481c2e2a3..608ada2c2 100644 --- a/acme/acme/messages_test.py +++ b/acme/acme/messages_test.py @@ -185,7 +185,8 @@ class ChallengeBodyTest(unittest.TestCase): """Tests for acme.messages.ChallengeBody.""" def setUp(self): - self.chall = challenges.DNS(token='foo') + self.chall = challenges.DNS(token=jose.b64decode( + 'evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA')) from acme.messages import ChallengeBody from acme.messages import Error @@ -201,7 +202,7 @@ class ChallengeBodyTest(unittest.TestCase): 'uri': 'http://challb', 'status': self.status, 'type': 'dns', - 'token': 'foo', + 'token': 'evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA', 'error': error, } self.jobj_from = self.jobj_to.copy() @@ -224,7 +225,8 @@ class ChallengeBodyTest(unittest.TestCase): hash(ChallengeBody.from_json(self.jobj_from)) def test_proxy(self): - self.assertEqual('foo', self.challb.token) + self.assertEqual(jose.b64decode( + 'evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ-PCt92wr-oA'), self.challb.token) class AuthorizationTest(unittest.TestCase): From 7aa9fe845ae6ba2f55c373d22a99ec9966ce725b Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 8 Sep 2015 01:33:03 -0700 Subject: [PATCH 12/44] Basic fix for #411 --- letsencrypt/cli.py | 111 ++++++++++++++++++++++++-- letsencrypt/client.py | 2 - letsencrypt/display/ops.py | 20 +++++ letsencrypt/storage.py | 19 +++++ letsencrypt/tests/display/ops_test.py | 22 +++++ letsencrypt/tests/renewer_test.py | 20 +++++ 6 files changed, 186 insertions(+), 8 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 066aa388d..4844c4691 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,7 +1,9 @@ """Let's Encrypt CLI.""" # TODO: Sanity check all input. Be sure to avoid shell code etc... + import argparse import atexit +import configobj import functools import logging import logging.handlers @@ -12,6 +14,7 @@ import time import traceback import configargparse +import OpenSSL import zope.component import zope.interface.exceptions import zope.interface.verify @@ -27,6 +30,7 @@ from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import log from letsencrypt import reporter +from letsencrypt import storage from letsencrypt.display import util as display_util from letsencrypt.display import ops as display_ops @@ -69,7 +73,7 @@ Choice of server for authentication/installation: More detailed help: - -h, --help [topic] print this message, or detailed help on a topic; + -h, --help [topic] print this message, or detailed help on a topic; the available topics are: all, apache, automation, nginx, paths, security, testing, or any of the @@ -159,7 +163,8 @@ def _init_le_client(args, config, authenticator, installer): return client.Client(config, acc, authenticator, installer, acme=acme) -def run(args, config, plugins): +def run(args, config, plugins): # pylint: disable=too-many-locals,too-many-branches,too-many-statements + """Obtain a certificate and install.""" if args.configurator is not None and (args.installer is not None or args.authenticator is not None): @@ -181,15 +186,109 @@ def run(args, config, plugins): return "Configurator could not be determined" domains = _find_domains(args, installer) + domains_set = set(domains) + renew_config = configuration.RenewerConfiguration(config) + # I am not sure whether that correctly reads the systemwide + # configuration file. + + configs_dir = renew_config.renewal_configs_dir + identical_names_cert = None + subset_names_cert = None + + for renewal_file in os.listdir(configs_dir): + full_path = os.path.join(configs_dir, renewal_file) + rc_config = configobj.ConfigObj(renew_config.renewer_config_file) + rc_config.merge(configobj.ConfigObj(full_path)) + rc_config.filename = full_path + cli_config = configuration.RenewerConfiguration(config.namespace) + + candidate_lineage = storage.RenewableCert(rc_config, None, cli_config) + # TODO: Handle these differently depending on whether they are + # expired or still valid? + candidate_names = set(candidate_lineage.names()) + if candidate_names == domains_set: + identical_names_cert = (renewal_file, candidate_lineage) + elif candidate_names.issubset(domains_set): + subset_names_cert = (renewal_file, candidate_lineage, + candidate_lineage.names()) + treat_as_renewal = False + question = None + same_cert_question = "You have an existing certificate that contains " + same_cert_question += "exactly the same domains you requested.\n\n{0}" + same_cert_question += "\n\nDo you want to renew and replace this " + same_cert_question += "certificate with a newly-issued one?" + subset_cert_question = "You have an existing certificate that contains " + subset_cert_question += "a portion of the domains you requested (ref: {0})" + subset_cert_question += "\n\nIt contains these names: {1}\n\nYou " + subset_cert_question += "requested these names for the new certificate: " + subset_cert_question += "{2}.\n\nDo you want to replace this existing " + subset_cert_question += "certificate with the new certificate?" + + if identical_names_cert: + question = same_cert_question.format(identical_names_cert[0]) + elif subset_names_cert: + question = subset_cert_question.format(subset_names_cert[0], + ", ".join(subset_names_cert[2]), + ", ".join(domains)) + if question: + if zope.component.getUtility(interfaces.IDisplay).yesno(question, + "Replace", + "Cancel"): + treat_as_renewal = True + else: + # TODO: Once the --duplicate (?) option is implemented, this + # exit will be suppressed and we will not treat this + # as a renewal. This should ideally be done by leaving + # question as None above so that we don't even prompt + # the user with the question. (We might say "if question + # and not duplicate_option:" instead of "if question".) + msg = "To obtain a new certificate that {0} an existing " + msg += "certificate in its domain-name coverage, consult the " + msg += "documentation about the --XXX-TODO option." + what = "duplicates" if identical_names_cert else "overlaps with" + msg = msg.format(what) + reporter_util = zope.component.getUtility(interfaces.IReporter) + reporter_util.add_message(msg, reporter_util.HIGH_PRIORITY, True) + sys.exit(1) + # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) - lineage = le_client.obtain_and_enroll_certificate( - domains, authenticator, installer, plugins) - if not lineage: - return "Certificate could not be obtained" + if treat_as_renewal: + # TREAT AS RENEWAL + if identical_names_cert: + lineage = identical_names_cert[1] + else: + lineage = subset_names_cert[1] + # TODO: Use existing privkey instead of generating a new one + new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) + # TODO: Check whether it worked! + old_version = lineage.latest_common_version() + lineage.save_successor(old_version, + OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, + new_certr.body), + new_key.pem, + OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, + new_chain)) + lineage.update_all_links_to(lineage.latest_common_version()) + # TODO: Check return value of save_successor + # TODO: Also update lineage renewal config with any relevant + # configuration values from this attempt? + else: + # TREAT AS NEW REQUEST + lineage = le_client.obtain_and_enroll_certificate( + domains, authenticator, installer, plugins) + if not lineage: + return "Certificate could not be obtained" + # TODO: This treats the key as changed even when it wasn't le_client.deploy_certificate( domains, lineage.privkey, lineage.cert, lineage.chain) le_client.enhance_config(domains, args.redirect) + if treat_as_renewal: + display_ops.success_renewal(domains) + else: + display_ops.success_installation(domains) def auth(args, config, plugins): diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e8dd08c8e..5dad04c09 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -378,8 +378,6 @@ class Client(object): # sites may have been enabled / final cleanup self.installer.restart() - display_ops.success_installation(domains) - def enhance_config(self, domains, redirect=None): """Enhance the configuration. diff --git a/letsencrypt/display/ops.py b/letsencrypt/display/ops.py index a220d07d9..8370750db 100644 --- a/letsencrypt/display/ops.py +++ b/letsencrypt/display/ops.py @@ -233,6 +233,26 @@ def success_installation(domains): pause=False) +def success_renewal(domains): + """Display a box confirming the renewal of an existing certificate. + + .. todo:: This should be centered on the screen + + :param list domains: domain names which were renewed + + """ + util(interfaces.IDisplay).notification( + "Your existing certificate has been successfully renewed, and the " + "new certificate has been installed.{1}{1}" + "The new certificate covers the following domains: {0}{1}{1}" + "You should test your configuration at:{1}{2}".format( + _gen_https_names(domains), + os.linesep, + os.linesep.join(_gen_ssl_lab_urls(domains))), + height=(14 + len(domains)), + pause=False) + + def _gen_ssl_lab_urls(domains): """Returns a list of urls. diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 431f56aff..2cdc8b765 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -11,6 +11,7 @@ import pytz import pyrfc3339 from letsencrypt import constants +from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util @@ -421,6 +422,24 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ return self._notafterbefore(lambda x509: x509.get_notAfter(), version) + def names(self, version=None): + """What are the subject names of this certificate? + + (If no version is specified, use the current version.) + + :param int version: the desired version number + :returns: the subject names + :rtype: `list` of `str` + + """ + if version is None: + target = self.current_target("cert") + else: + target = self.version("cert", version) + with open(target) as f: + sans = crypto_util.get_sans_from_cert(f.read()) + return sans + def should_autodeploy(self): """Should this lineage now automatically deploy a newer version? diff --git a/letsencrypt/tests/display/ops_test.py b/letsencrypt/tests/display/ops_test.py index fc4013bed..696deb3b0 100644 --- a/letsencrypt/tests/display/ops_test.py +++ b/letsencrypt/tests/display/ops_test.py @@ -383,5 +383,27 @@ class SuccessInstallationTest(unittest.TestCase): self.assertTrue(name in arg) +class SuccessRenewalTest(unittest.TestCase): + # pylint: disable=too-few-public-methods + """Test the success renewal message.""" + @classmethod + def _call(cls, names): + from letsencrypt.display.ops import success_renewal + success_renewal(names) + + @mock.patch("letsencrypt.display.ops.util") + def test_success_renewal(self, mock_util): + mock_util().notification.return_value = None + names = ["example.com", "abc.com"] + + self._call(names) + + self.assertEqual(mock_util().notification.call_count, 1) + arg = mock_util().notification.call_args_list[0][0][0] + + for name in names: + self.assertTrue(name in arg) + + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 1b58d9e0f..0169f1159 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -295,6 +295,26 @@ class RenewableCertTests(unittest.TestCase): else: self.assertFalse(self.test_rc.has_pending_deployment()) + def test_names(self): + # Trying the current version + test_cert = test_util.load_vector("cert-san.pem") + os.symlink(os.path.join("..", "..", "archive", "example.org", + "cert12.pem"), self.test_rc.cert) + with open(self.test_rc.cert, "w") as f: + f.write(test_cert) + self.assertEqual(self.test_rc.names(), + ["example.com", "www.example.com"]) + + # Trying a non-current version + test_cert = test_util.load_vector("cert.pem") + os.unlink(self.test_rc.cert) + os.symlink(os.path.join("..", "..", "archive", "example.org", + "cert15.pem"), self.test_rc.cert) + with open(self.test_rc.cert, "w") as f: + f.write(test_cert) + self.assertEqual(self.test_rc.names(12), + ["example.com", "www.example.com"]) + def _test_notafterbefore(self, function, timestamp): test_cert = test_util.load_vector("cert.pem") os.symlink(os.path.join("..", "..", "archive", "example.org", From 7dda21817a7426dc7a031e4330e30128b4eccc49 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 8 Sep 2015 01:50:29 -0700 Subject: [PATCH 13/44] Add --duplicate command-line option --- letsencrypt/cli.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4844c4691..2e2ae1969 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -230,21 +230,15 @@ def run(args, config, plugins): # pylint: disable=too-many-locals,too-many-bran question = subset_cert_question.format(subset_names_cert[0], ", ".join(subset_names_cert[2]), ", ".join(domains)) - if question: + if question and not config.duplicate: if zope.component.getUtility(interfaces.IDisplay).yesno(question, "Replace", "Cancel"): treat_as_renewal = True else: - # TODO: Once the --duplicate (?) option is implemented, this - # exit will be suppressed and we will not treat this - # as a renewal. This should ideally be done by leaving - # question as None above so that we don't even prompt - # the user with the question. (We might say "if question - # and not duplicate_option:" instead of "if question".) msg = "To obtain a new certificate that {0} an existing " msg += "certificate in its domain-name coverage, consult the " - msg += "documentation about the --XXX-TODO option." + msg += "documentation about the --duplicate option." what = "duplicates" if identical_names_cert else "overlaps with" msg = msg.format(what) reporter_util = zope.component.getUtility(interfaces.IReporter) @@ -582,6 +576,9 @@ def create_parser(plugins, args): #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") helpful.add(None, "-d", "--domains", metavar="DOMAIN", action="append") + helpful.add(None, "-D", "--duplicate", dest="duplicate", + action="store_true", + help="Allow getting a certificate that duplicates an existing one") helpful.add_group( "automation", From b375b9c074af8ff5dc9206cf3f1618a3e2d7bd0e Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 8 Sep 2015 08:48:46 -0700 Subject: [PATCH 14/44] Fix indentation --- letsencrypt/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 2e2ae1969..182e2bbc5 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -576,8 +576,8 @@ def create_parser(plugins, args): #for subparser in parser_run, parser_auth, parser_install: # subparser.add_argument("domains", nargs="*", metavar="domain") helpful.add(None, "-d", "--domains", metavar="DOMAIN", action="append") - helpful.add(None, "-D", "--duplicate", dest="duplicate", - action="store_true", + helpful.add( + None, "-D", "--duplicate", dest="duplicate", action="store_true", help="Allow getting a certificate that duplicates an existing one") helpful.add_group( From df42cca26ea0c98aae2e61709772a1b181efc21e Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 8 Sep 2015 08:58:43 -0700 Subject: [PATCH 15/44] More useful explanation of --duplicate --- letsencrypt/cli.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 182e2bbc5..cb9eec2b1 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -237,8 +237,9 @@ def run(args, config, plugins): # pylint: disable=too-many-locals,too-many-bran treat_as_renewal = True else: msg = "To obtain a new certificate that {0} an existing " - msg += "certificate in its domain-name coverage, consult the " - msg += "documentation about the --duplicate option." + msg += "certificate in its domain-name coverage, you must use " + msg += "the --duplicate option.\n\nFor example:\n\n" + msg += sys.argv[0] + " --duplicate " + " ".join(sys.argv[1:]) what = "duplicates" if identical_names_cert else "overlaps with" msg = msg.format(what) reporter_util = zope.component.getUtility(interfaces.IReporter) From 3cc15c6193401f12ab54fb9f910fee9dedaebedd Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 8 Sep 2015 21:01:53 -0700 Subject: [PATCH 16/44] Cleanup --- letsencrypt/cli.py | 123 +++++++++++++++++++++++---------------------- 1 file changed, 63 insertions(+), 60 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index cb9eec2b1..819c094e3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -163,7 +163,33 @@ def _init_le_client(args, config, authenticator, installer): return client.Client(config, acc, authenticator, installer, acme=acme) -def run(args, config, plugins): # pylint: disable=too-many-locals,too-many-branches,too-many-statements +def _find_duplicative_certs(domains, config, renew_config): + """Find existing certs that duplicate the request.""" + + identical_names_cert, subset_names_cert = None, None + + configs_dir = renew_config.renewal_configs_dir + for renewal_file in os.listdir(configs_dir): + full_path = os.path.join(configs_dir, renewal_file) + rc_config = configobj.ConfigObj(renew_config.renewer_config_file) + rc_config.merge(configobj.ConfigObj(full_path)) + rc_config.filename = full_path + cli_config = configuration.RenewerConfiguration(config.namespace) + + candidate_lineage = storage.RenewableCert(rc_config, None, cli_config) + # TODO: Handle these differently depending on whether they are + # expired or still valid? + candidate_names = set(candidate_lineage.names()) + if candidate_names == set(domains): + identical_names_cert = (renewal_file, candidate_lineage) + elif candidate_names.issubset(set(domains)): + subset_names_cert = (renewal_file, candidate_lineage, + candidate_lineage.names()) + + return identical_names_cert, subset_names_cert + + +def run(args, config, plugins): # pylint: disable=too-many-branches """Obtain a certificate and install.""" if args.configurator is not None and (args.installer is not None or @@ -186,62 +212,40 @@ def run(args, config, plugins): # pylint: disable=too-many-locals,too-many-bran return "Configurator could not be determined" domains = _find_domains(args, installer) - domains_set = set(domains) renew_config = configuration.RenewerConfiguration(config) # I am not sure whether that correctly reads the systemwide # configuration file. - configs_dir = renew_config.renewal_configs_dir - identical_names_cert = None - subset_names_cert = None - - for renewal_file in os.listdir(configs_dir): - full_path = os.path.join(configs_dir, renewal_file) - rc_config = configobj.ConfigObj(renew_config.renewer_config_file) - rc_config.merge(configobj.ConfigObj(full_path)) - rc_config.filename = full_path - cli_config = configuration.RenewerConfiguration(config.namespace) - - candidate_lineage = storage.RenewableCert(rc_config, None, cli_config) - # TODO: Handle these differently depending on whether they are - # expired or still valid? - candidate_names = set(candidate_lineage.names()) - if candidate_names == domains_set: - identical_names_cert = (renewal_file, candidate_lineage) - elif candidate_names.issubset(domains_set): - subset_names_cert = (renewal_file, candidate_lineage, - candidate_lineage.names()) treat_as_renewal = False - question = None - same_cert_question = "You have an existing certificate that contains " - same_cert_question += "exactly the same domains you requested.\n\n{0}" - same_cert_question += "\n\nDo you want to renew and replace this " - same_cert_question += "certificate with a newly-issued one?" - subset_cert_question = "You have an existing certificate that contains " - subset_cert_question += "a portion of the domains you requested (ref: {0})" - subset_cert_question += "\n\nIt contains these names: {1}\n\nYou " - subset_cert_question += "requested these names for the new certificate: " - subset_cert_question += "{2}.\n\nDo you want to replace this existing " - subset_cert_question += "certificate with the new certificate?" - if identical_names_cert: - question = same_cert_question.format(identical_names_cert[0]) - elif subset_names_cert: - question = subset_cert_question.format(subset_names_cert[0], - ", ".join(subset_names_cert[2]), - ", ".join(domains)) - if question and not config.duplicate: - if zope.component.getUtility(interfaces.IDisplay).yesno(question, - "Replace", - "Cancel"): + if not config.duplicate: + identical_names_cert, subset_names_cert = _find_duplicative_certs( + domains, config, renew_config) + if identical_names_cert: + question = ( + "You have an existing certificate that contains exactly the " + "same domains you requested (ref: {0})\n\nDo you want to " + "renew and replace this certificate with a newly-issued one?" + ).format(identical_names_cert[0]) + elif subset_names_cert: + question = ( + "You have an existing certificate that contains a portion of " + "the domains you requested (ref: {0})\n\nIt contains these " + "names: {1}\n\nYou requested these names for the new " + "certificate: {2}.\n\nDo you want to replace this existing " + "certificate with the new certificate?" + ).format(subset_names_cert[0], ", ".join(subset_names_cert[2]), + ", ".join(domains)) + if zope.component.getUtility(interfaces.IDisplay).yesno( + question, "Replace", "Cancel"): treat_as_renewal = True else: msg = "To obtain a new certificate that {0} an existing " msg += "certificate in its domain-name coverage, you must use " msg += "the --duplicate option.\n\nFor example:\n\n" msg += sys.argv[0] + " --duplicate " + " ".join(sys.argv[1:]) - what = "duplicates" if identical_names_cert else "overlaps with" - msg = msg.format(what) + msg = msg.format( + "duplicates" if identical_names_cert else "overlaps with") reporter_util = zope.component.getUtility(interfaces.IReporter) reporter_util.add_message(msg, reporter_util.HIGH_PRIORITY, True) sys.exit(1) @@ -258,31 +262,30 @@ def run(args, config, plugins): # pylint: disable=too-many-locals,too-many-bran new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! old_version = lineage.latest_common_version() - lineage.save_successor(old_version, - OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, - new_certr.body), - new_key.pem, - OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, - new_chain)) + lineage.save_successor( + old_version, OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_certr.body), + new_key.pem, OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_chain)) + lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant # configuration values from this attempt? + le_client.deploy_certificate( + domains, lineage.privkey, lineage.cert, lineage.chain) + display_ops.success_renewal(domains) else: # TREAT AS NEW REQUEST lineage = le_client.obtain_and_enroll_certificate( domains, authenticator, installer, plugins) if not lineage: return "Certificate could not be obtained" - # TODO: This treats the key as changed even when it wasn't - le_client.deploy_certificate( - domains, lineage.privkey, lineage.cert, lineage.chain) - le_client.enhance_config(domains, args.redirect) - if treat_as_renewal: - display_ops.success_renewal(domains) - else: + # TODO: This treats the key as changed even when it wasn't + # TODO: We also need to pass the fullchain (for Nginx) + le_client.deploy_certificate( + domains, lineage.privkey, lineage.cert, lineage.chain) + le_client.enhance_config(domains, args.redirect) display_ops.success_installation(domains) From 244dc30306f1a78c0122987771f99798071ef946 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 9 Sep 2015 00:11:44 -0700 Subject: [PATCH 17/44] Fewer locals (lint would still complain) --- letsencrypt/cli.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a21251172..b53e0a79e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -189,7 +189,7 @@ def _find_duplicative_certs(domains, config, renew_config): return identical_names_cert, subset_names_cert -def run(args, config, plugins): # pylint: disable=too-many-branches +def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals """Obtain a certificate and install.""" if args.configurator is not None and (args.installer is not None or @@ -212,15 +212,16 @@ def run(args, config, plugins): # pylint: disable=too-many-branches return "Configurator could not be determined" domains = _find_domains(args, installer) - renew_config = configuration.RenewerConfiguration(config) - # I am not sure whether that correctly reads the systemwide - # configuration file. treat_as_renewal = False + # Considering the possibility that the requested certificate is + # related to an existing certificate. if not config.duplicate: identical_names_cert, subset_names_cert = _find_duplicative_certs( - domains, config, renew_config) + domains, config, configuration.RenewerConfiguration(config)) + # I am not sure whether that correctly reads the systemwide + # configuration file. if identical_names_cert: question = ( "You have an existing certificate that contains exactly the " @@ -240,20 +241,20 @@ def run(args, config, plugins): # pylint: disable=too-many-branches question, "Replace", "Cancel"): treat_as_renewal = True else: - msg = "To obtain a new certificate that {0} an existing " - msg += "certificate in its domain-name coverage, you must use " - msg += "the --duplicate option.\n\nFor example:\n\n" - msg += sys.argv[0] + " --duplicate " + " ".join(sys.argv[1:]) - msg = msg.format( - "duplicates" if identical_names_cert else "overlaps with") reporter_util = zope.component.getUtility(interfaces.IReporter) - reporter_util.add_message(msg, reporter_util.HIGH_PRIORITY, True) + reporter_util.add_message(( + "To obtain a new certificate that {0} an existing certificate " + "in its domain-name coverage, you must use the --duplicate " + "option.\n\nFor example:\n\n{1} --duplicate {2}").format( + "duplicates" if identical_names_cert else "overlaps with", + sys.argv[0], " ".join(sys.argv[1:])), + reporter_util.HIGH_PRIORITY, True) sys.exit(1) + # Attempting to obtain the certificate # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) if treat_as_renewal: - # TREAT AS RENEWAL if identical_names_cert: lineage = identical_names_cert[1] else: @@ -261,9 +262,8 @@ def run(args, config, plugins): # pylint: disable=too-many-branches # TODO: Use existing privkey instead of generating a new one new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! - old_version = lineage.latest_common_version() lineage.save_successor( - old_version, OpenSSL.crypto.dump_certificate( + lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, new_certr.body), new_key.pem, OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, new_chain)) From bf754b6302e87b09a3b4b3c966226dfb7dce0dc5 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 8 Sep 2015 20:37:09 +0000 Subject: [PATCH 18/44] Add ACME Directory Resource --- acme/acme/client.py | 24 ++++++++--- acme/acme/client_test.py | 19 +++++++-- acme/acme/messages.py | 70 +++++++++++++++++++++++++------- acme/acme/messages_test.py | 46 +++++++++++++++++---- acme/acme/util.py | 7 ++++ acme/acme/util_test.py | 16 ++++++++ letsencrypt/client.py | 2 +- letsencrypt/constants.py | 2 +- letsencrypt/interfaces.py | 3 +- letsencrypt/revoker.py | 2 +- letsencrypt/tests/client_test.py | 2 +- 11 files changed, 155 insertions(+), 38 deletions(-) create mode 100644 acme/acme/util.py create mode 100644 acme/acme/util_test.py diff --git a/acme/acme/client.py b/acme/acme/client.py index ef982b093..9c32a81a4 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -4,6 +4,7 @@ import heapq import logging import time +import six from six.moves import http_client # pylint: disable=import-error import OpenSSL @@ -32,7 +33,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes Clean up raised error types hierarchy, document, and handle (wrap) instances of `.DeserializationError` raised in `from_json()`. - :ivar str new_reg_uri: Location of new-reg + :ivar messages.Directory directory: :ivar key: `.JWK` (private) :ivar alg: `.JWASignature` :ivar bool verify_ssl: Verify SSL certificates? @@ -43,12 +44,23 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ DER_CONTENT_TYPE = 'application/pkix-cert' - def __init__(self, new_reg_uri, key, alg=jose.RS256, - verify_ssl=True, net=None): - self.new_reg_uri = new_reg_uri + def __init__(self, directory, key, alg=jose.RS256, verify_ssl=True, + net=None): + """Initialize. + + :param directory: Directory Resource (`.messages.Directory`) or + URI from which the resource will be downloaded. + + """ self.key = key self.net = ClientNetwork(key, alg, verify_ssl) if net is None else net + if isinstance(directory, six.string_types): + self.directory = messages.Directory.from_json( + self.net.get(directory).json()) + else: + self.directory = directory + @classmethod def _regr_from_response(cls, response, uri=None, new_authzr_uri=None, terms_of_service=None): @@ -82,7 +94,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes new_reg = messages.NewRegistration() if new_reg is None else new_reg assert isinstance(new_reg, messages.NewRegistration) - response = self.net.post(self.new_reg_uri, new_reg) + response = self.net.post(self.directory[new_reg], new_reg) # TODO: handle errors assert response.status_code == http_client.CREATED @@ -441,7 +453,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes :raises .ClientError: If revocation is unsuccessful. """ - response = self.net.post(messages.Revocation.url(self.new_reg_uri), + response = self.net.post(self.directory[messages.Revocation], messages.Revocation(certificate=cert)) if response.status_code != http_client.OK: raise errors.ClientError( diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index dcc0832e3..ce03256c3 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -33,10 +33,14 @@ class ClientTest(unittest.TestCase): self.net.post.return_value = self.response 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', + }) + from acme.client import Client self.client = Client( - new_reg_uri='https://www.letsencrypt-demo.org/acme/new-reg', - key=KEY, alg=jose.RS256, net=self.net) + directory=self.directory, key=KEY, alg=jose.RS256, net=self.net) self.identifier = messages.Identifier( typ=messages.IDENTIFIER_FQDN, value='example.com') @@ -72,6 +76,13 @@ class ClientTest(unittest.TestCase): uri='https://www.letsencrypt-demo.org/acme/cert/1', cert_chain_uri='https://www.letsencrypt-demo.org/ca') + def test_init_downloads_directory(self): + uri = 'http://www.letsencrypt-demo.org/directory' + from acme.client import Client + self.client = Client( + directory=uri, key=KEY, alg=jose.RS256, net=self.net) + self.net.get.assert_called_once_with(uri) + def test_register(self): # "Instance of 'Field' has no to_json/update member" bug: # pylint: disable=no-member @@ -348,8 +359,8 @@ class ClientTest(unittest.TestCase): def test_revoke(self): self.client.revoke(self.certr.body) - self.net.post.assert_called_once_with(messages.Revocation.url( - self.client.new_reg_uri), mock.ANY) + self.net.post.assert_called_once_with( + self.directory[messages.Revocation], mock.ANY) def test_revoke_bad_status_raises_error(self): self.response.status_code = http_client.METHOD_NOT_ALLOWED diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 970cf4e6e..2d82ccb5e 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -1,11 +1,10 @@ """ACME protocol messages.""" import collections -from six.moves.urllib import parse as urllib_parse # pylint: disable=import-error - from acme import challenges from acme import fields from acme import jose +from acme import util class Error(jose.JSONObjectWithFields, Exception): @@ -128,6 +127,56 @@ class Identifier(jose.JSONObjectWithFields): value = jose.Field('value') +class Directory(jose.JSONDeSerializable): + """Directory.""" + + _REGISTERED_TYPES = {} + + @classmethod + def _canon_key(cls, key): + return getattr(key, 'resource_type', key) + + @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 + return resource_body_cls + + def __init__(self, jobj): + canon_jobj = util.map_keys(jobj, self._canon_key) + if not set(canon_jobj).issubset(self._REGISTERED_TYPES): + # TODO: acme-spec is not clear about this: 'It is a JSON + # dictionary, whose keys are the "resource" values listed + # in {{https-requests}}'z + raise ValueError('Wrong directory fields') + # TODO: check that everything is an absolute URL; acme-spec is + # not clear on that + self._jobj = canon_jobj + + def __getattr__(self, name): + try: + return self[name.replace('_', '-')] + except KeyError as error: + raise AttributeError(error.message) + + def __getitem__(self, name): + try: + return self._jobj[self._canon_key(name)] + except KeyError: + raise KeyError('Directory field not found') + + def to_partial_json(self): + return self._jobj + + @classmethod + def from_json(cls, jobj): + try: + return cls(jobj) + except ValueError as error: + raise jose.DeserializationError(str(error)) + + class Resource(jose.JSONObjectWithFields): """ACME Resource. @@ -216,6 +265,7 @@ class Registration(ResourceBody): """All emails found in the ``contact`` field.""" return self._filter_contact(self.email_prefix) +@Directory.register class NewRegistration(Registration): """New registration.""" resource_type = 'new-reg' @@ -328,6 +378,7 @@ class Authorization(ResourceBody): return tuple(tuple(self.challenges[idx] for idx in combo) for combo in self.combinations) +@Directory.register class NewAuthorization(Authorization): """New authorization.""" resource_type = 'new-authz' @@ -344,6 +395,7 @@ class AuthorizationResource(ResourceWithURI): new_cert_uri = jose.Field('new_cert_uri') +@Directory.register class CertificateRequest(jose.JSONObjectWithFields): """ACME new-cert request. @@ -369,6 +421,7 @@ class CertificateResource(ResourceWithURI): authzrs = jose.Field('authzrs') +@Directory.register class Revocation(jose.JSONObjectWithFields): """Revocation message. @@ -380,16 +433,3 @@ class Revocation(jose.JSONObjectWithFields): resource = fields.Resource(resource_type) certificate = jose.Field( 'certificate', decoder=jose.decode_cert, encoder=jose.encode_cert) - - # TODO: acme-spec#138, this allows only one ACME server instance per domain - PATH = '/acme/revoke-cert' - """Path to revocation URL, see `url`""" - - @classmethod - def url(cls, base): - """Get revocation URL. - - :param str base: New Registration Resource or server (root) URL. - - """ - return urllib_parse.urljoin(base, cls.PATH) diff --git a/acme/acme/messages_test.py b/acme/acme/messages_test.py index 481c2e2a3..c0aafe2e1 100644 --- a/acme/acme/messages_test.py +++ b/acme/acme/messages_test.py @@ -92,6 +92,45 @@ class ConstantTest(unittest.TestCase): self.assertFalse(self.const_a != const_a_prime) +class DirectoryTest(unittest.TestCase): + """Tests for acme.messages.Directory.""" + + def setUp(self): + from acme.messages import Directory + self.dir = Directory({ + 'new-reg': 'reg', + mock.MagicMock(resource_type='new-cert'): 'cert', + }) + + def test_init_wrong_key_value_error(self): + from acme.messages import Directory + self.assertRaises(ValueError, Directory, {'foo': 'bar'}) + + def test_getitem(self): + self.assertEqual('reg', self.dir['new-reg']) + from acme.messages import NewRegistration + self.assertEqual('reg', self.dir[NewRegistration]) + self.assertEqual('reg', self.dir[NewRegistration()]) + + def test_getitem_fails_with_key_error(self): + self.assertRaises(KeyError, self.dir.__getitem__, 'foo') + + def test_getattr(self): + self.assertEqual('reg', self.dir.new_reg) + + def test_getattr_fails_with_attribute_error(self): + self.assertRaises(AttributeError, self.dir.__getattr__, 'foo') + + def test_to_partial_json(self): + self.assertEqual( + self.dir.to_partial_json(), {'new-reg': 'reg', 'new-cert': 'cert'}) + + def test_from_json_deserialization_error_on_wrong_key(self): + from acme.messages import Directory + self.assertRaises( + jose.DeserializationError, Directory.from_json, {'foo': 'bar'}) + + class RegistrationTest(unittest.TestCase): """Tests for acme.messages.Registration.""" @@ -320,13 +359,6 @@ class CertificateResourceTest(unittest.TestCase): class RevocationTest(unittest.TestCase): """Tests for acme.messages.RevocationTest.""" - def test_url(self): - from acme.messages import Revocation - url = 'https://letsencrypt-demo.org/acme/revoke-cert' - self.assertEqual(url, Revocation.url('https://letsencrypt-demo.org')) - self.assertEqual( - url, Revocation.url('https://letsencrypt-demo.org/acme/new-reg')) - def setUp(self): from acme.messages import Revocation self.rev = Revocation(certificate=CERT) diff --git a/acme/acme/util.py b/acme/acme/util.py new file mode 100644 index 000000000..1fff89a9e --- /dev/null +++ b/acme/acme/util.py @@ -0,0 +1,7 @@ +"""ACME utilities.""" +import six + + +def map_keys(dikt, func): + """Map dictionary keys.""" + return dict((func(key), value) for key, value in six.iteritems(dikt)) diff --git a/acme/acme/util_test.py b/acme/acme/util_test.py new file mode 100644 index 000000000..00aa8b02d --- /dev/null +++ b/acme/acme/util_test.py @@ -0,0 +1,16 @@ +"""Tests for acme.util.""" +import unittest + + +class MapKeysTest(unittest.TestCase): + """Tests for acme.util.map_keys.""" + + def test_it(self): + from acme.util import map_keys + self.assertEqual({'a': 'b', 'c': 'd'}, + map_keys({'a': 'b', 'c': 'd'}, lambda key: key)) + self.assertEqual({2: 2, 4: 4}, map_keys({1: 2, 3: 4}, lambda x: x + 1)) + + +if __name__ == '__main__': + unittest.main() # pragma: no cover diff --git a/letsencrypt/client.py b/letsencrypt/client.py index e8dd08c8e..e5cdc81c9 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -33,7 +33,7 @@ logger = logging.getLogger(__name__) def _acme_from_config_key(config, key): # TODO: Allow for other alg types besides RS256 - return acme_client.Client(new_reg_uri=config.server, key=key, + return acme_client.Client(directory=config.server, key=key, verify_ssl=(not config.no_verify_ssl)) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 230860762..0d00f2d75 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -16,7 +16,7 @@ CLI_DEFAULTS = dict( "letsencrypt", "cli.ini"), ], verbose_count=-(logging.WARNING / 10), - server="https://acme-staging.api.letsencrypt.org/acme/new-reg", + server="https://acme-staging.api.letsencrypt.org/directory", rsa_key_size=2048, rollback_checkpoints=1, config_dir="/etc/letsencrypt", diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 2271b9050..3dee1b1ea 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -194,8 +194,7 @@ class IConfig(zope.interface.Interface): filtered, stripped or sanitized. """ - server = zope.interface.Attribute( - "ACME new registration URI (including /acme/new-reg).") + server = zope.interface.Attribute("ACME Directory Resource URI.") email = zope.interface.Attribute( "Email used for registration and recovery contact.") rsa_key_size = zope.interface.Attribute("Size of the RSA key.") diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index 160d911a5..e8b154012 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -48,7 +48,7 @@ class Revoker(object): """ def __init__(self, installer, config, no_confirm=False): # XXX - self.acme = acme_client.Client(new_reg_uri=None, key=None, alg=None) + self.acme = acme_client.Client(directory=None, key=None, alg=None) self.installer = installer self.config = config diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index b992089cc..df3a341a2 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -70,7 +70,7 @@ class ClientTest(unittest.TestCase): def test_init_acme_verify_ssl(self): self.acme_client.assert_called_once_with( - new_reg_uri=mock.ANY, key=mock.ANY, verify_ssl=True) + directory=mock.ANY, key=mock.ANY, verify_ssl=True) def _mock_obtain_certificate(self): self.client.auth_handler = mock.MagicMock() From 302e3ceb7dbd9cedb8042ec1708e2f0ac27c40ef Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 9 Sep 2015 20:04:28 +0000 Subject: [PATCH 19/44] Revocation: integration testable --- acme/acme/client.py | 3 ++- acme/acme/client_test.py | 2 +- letsencrypt/cli.py | 34 ++++++++++++++++++++++------------ tests/boulder-integration.sh | 7 +++++++ 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 9c32a81a4..ae9cde33f 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -454,7 +454,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ response = self.net.post(self.directory[messages.Revocation], - messages.Revocation(certificate=cert)) + messages.Revocation(certificate=cert), + content_type=None) if response.status_code != http_client.OK: raise errors.ClientError( 'Successful revocation must return HTTP OK status') diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index ce03256c3..06c0a2313 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -360,7 +360,7 @@ class ClientTest(unittest.TestCase): def test_revoke(self): self.client.revoke(self.certr.body) self.net.post.assert_called_once_with( - self.directory[messages.Revocation], mock.ANY) + self.directory[messages.Revocation], mock.ANY, content_type=None) def test_revoke_bad_status_raises_error(self): self.response.status_code = http_client.METHOD_NOT_ALLOWED diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a70db8dd2..bb04bc3d6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -16,12 +16,16 @@ import zope.component import zope.interface.exceptions import zope.interface.verify +from acme import client as acme_client +from acme import jose + import letsencrypt from letsencrypt import account from letsencrypt import configuration from letsencrypt import constants from letsencrypt import client +from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util @@ -241,16 +245,20 @@ def install(args, config, plugins): le_client.enhance_config(domains, args.redirect) -def revoke(args, unused_config, unused_plugins): +def revoke(args, config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" - if args.cert_path is None and args.key_path is None: - return "At least one of --cert-path or --key-path is required" - - # This depends on the renewal config and cannot be completed yet. - zope.component.getUtility(interfaces.IDisplay).notification( - "Revocation is not available with the new Boulder server yet.") - #client.revoke(args.installer, config, plugins, args.no_confirm, - # args.cert_path, args.key_path) + if args.key_path is not None: # revocation by cert key + logger.debug("Revoking %s using cert key %s", + args.cert_path[0], args.key_path[0]) + acme = acme_client.Client( + config.server, key=jose.JWK.load(args.key_path[1])) + else: # revocation by account key + logger.debug("Revoking %s using Account Key", args.cert_path[0]) + acc, _ = _determine_account(args, config) + # pylint: disable=protected-access + acme = client._acme_from_config_key(config, acc.key) + acme.revoke(jose.ComparableX509(crypto_util.pyopenssl_load_certificate( + args.cert_path[1])[0])) def rollback(args, config, plugins): @@ -576,14 +584,16 @@ def _create_subparsers(helpful): "--cert-path", required=True, help="Path to a certificate that " "is going to be installed.") parser_install.add_argument( - "--key-path", required=True, help="Accompynying private key") + "--key-path", required=True, help="Accompanying private key") parser_install.add_argument( "--chain-path", help="Accompanying path to a certificate chain.") parser_revoke.add_argument( - "--cert-path", type=read_file, help="Revoke a specific certificate.") + "--cert-path", type=read_file, help="Revoke a specific certificate.", + required=True) parser_revoke.add_argument( "--key-path", type=read_file, - help="Revoke all certs generated by the provided authorized key.") + help="Revoke certificate using its accompanying key. Useful if " + "Account Key is lost.") parser_rollback.add_argument( "--checkpoints", type=int, metavar="N", diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 23bfcf3ca..8a4f715ea 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -54,6 +54,13 @@ do [ "${dir}/${latest}" = "$live" ] # renewer fails this test done +# revoke by account key +common revoke --cert-path /etc/conf/live/le.wtf/cert.pem +# revoke renewed +common revoke --cert-path /etc/conf/live/le1.wtf/cert.pem +# revoke by cert key +common revoke --cert-path /etc/conf/live/le2.wtf/cert.pem \ + --key-path /etc/conf/live/le2.wtf/privkey.pem if type nginx; then From 817ab468d1335ac09a56cef75050525739f2e1e5 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 9 Sep 2015 20:21:33 +0000 Subject: [PATCH 20/44] py3 compat: str(exc) instead of exc.message --- acme/acme/messages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 2d82ccb5e..1a7463fba 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -158,7 +158,7 @@ class Directory(jose.JSONDeSerializable): try: return self[name.replace('_', '-')] except KeyError as error: - raise AttributeError(error.message) + raise AttributeError(str(error)) def __getitem__(self, name): try: From c93564b99ec4fdf7cc2016cb169fc94fd41a250a Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 9 Sep 2015 20:21:57 +0000 Subject: [PATCH 21/44] Travis: update --server to point at directory --- tests/integration/_common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 8656b8518..c8b142cf2 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -13,7 +13,7 @@ export root store_flags letsencrypt_test () { letsencrypt \ - --server "${SERVER:-http://localhost:4000/acme/new-reg}" \ + --server "${SERVER:-http://localhost:4000/directory}" \ --no-verify-ssl \ --dvsni-port 5001 \ --simple-http-port 5001 \ From ed051b7c28a67109582caf6bc0ff42a3dc95b4fe Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 9 Sep 2015 20:40:04 +0000 Subject: [PATCH 22/44] Boulder Monolithic does not exist --- tests/boulder-integration.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 23bfcf3ca..786ceb1b2 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -4,9 +4,7 @@ # instance (see ./boulder-start.sh). # # Environment variables: -# SERVER: Passed as "letsencrypt --server" argument. Boulder -# monolithic defaults to :4000, AMQP defaults to :4300. This -# script defaults to monolithic. +# SERVER: Passed as "letsencrypt --server" argument. # # Note: this script is called by Boulder integration test suite! From 67d6b89382753b369f64a9146807a9380eee4279 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 9 Sep 2015 20:54:11 +0000 Subject: [PATCH 23/44] Fix paths in integration testing --- tests/boulder-integration.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 3a1c8748a..67cc4c5e9 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -53,12 +53,12 @@ do done # revoke by account key -common revoke --cert-path /etc/conf/live/le.wtf/cert.pem +common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" # revoke renewed -common revoke --cert-path /etc/conf/live/le1.wtf/cert.pem +common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" # revoke by cert key -common revoke --cert-path /etc/conf/live/le2.wtf/cert.pem \ - --key-path /etc/conf/live/le2.wtf/privkey.pem +common revoke --cert-path "$root/conf/live/le2.wtf/cert.pem" \ + --key-path "$root/conf/live/le2.wtf/privkey.pem" if type nginx; then From 39aff967a52dcad1dc4810f009c4717bb09d2de3 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 10 Sep 2015 20:14:47 +0000 Subject: [PATCH 24/44] Revocation: expect application/json (boulder#771). --- acme/acme/client.py | 3 +-- acme/acme/client_test.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index ae9cde33f..9c32a81a4 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -454,8 +454,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ response = self.net.post(self.directory[messages.Revocation], - messages.Revocation(certificate=cert), - content_type=None) + messages.Revocation(certificate=cert)) if response.status_code != http_client.OK: raise errors.ClientError( 'Successful revocation must return HTTP OK status') diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index 06c0a2313..ce03256c3 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -360,7 +360,7 @@ class ClientTest(unittest.TestCase): def test_revoke(self): self.client.revoke(self.certr.body) self.net.post.assert_called_once_with( - self.directory[messages.Revocation], mock.ANY, content_type=None) + self.directory[messages.Revocation], mock.ANY) def test_revoke_bad_status_raises_error(self): self.response.status_code = http_client.METHOD_NOT_ALLOWED From b3ade6abe4d55010b042f81e828e6a1c891cf870 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 10 Sep 2015 20:43:20 +0000 Subject: [PATCH 25/44] Revert "Revocation: expect application/json (boulder#771)." This reverts commit 39aff967a52dcad1dc4810f009c4717bb09d2de3. --- acme/acme/client.py | 3 ++- acme/acme/client_test.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/acme/acme/client.py b/acme/acme/client.py index 9c32a81a4..ae9cde33f 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -454,7 +454,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ response = self.net.post(self.directory[messages.Revocation], - messages.Revocation(certificate=cert)) + messages.Revocation(certificate=cert), + content_type=None) if response.status_code != http_client.OK: raise errors.ClientError( 'Successful revocation must return HTTP OK status') diff --git a/acme/acme/client_test.py b/acme/acme/client_test.py index ce03256c3..06c0a2313 100644 --- a/acme/acme/client_test.py +++ b/acme/acme/client_test.py @@ -360,7 +360,7 @@ class ClientTest(unittest.TestCase): def test_revoke(self): self.client.revoke(self.certr.body) self.net.post.assert_called_once_with( - self.directory[messages.Revocation], mock.ANY) + self.directory[messages.Revocation], mock.ANY, content_type=None) def test_revoke_bad_status_raises_error(self): self.response.status_code = http_client.METHOD_NOT_ALLOWED From 7c2a87a51dc8e8501e3660ba5744ec03622d864f Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Sep 2015 14:45:30 -0700 Subject: [PATCH 26/44] Remove explicit .namespace for easier testing --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b53e0a79e..194e50c2d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -174,7 +174,7 @@ def _find_duplicative_certs(domains, config, renew_config): rc_config = configobj.ConfigObj(renew_config.renewer_config_file) rc_config.merge(configobj.ConfigObj(full_path)) rc_config.filename = full_path - cli_config = configuration.RenewerConfiguration(config.namespace) + cli_config = configuration.RenewerConfiguration(config) candidate_lineage = storage.RenewableCert(rc_config, None, cli_config) # TODO: Handle these differently depending on whether they are From ff85bd30b7a570bb9e369ad5ea07147f06fc3502 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Sep 2015 14:59:10 -0700 Subject: [PATCH 27/44] Disable duplicate-code pylint check --- .pylintrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pylintrc b/.pylintrc index d954b2658..9e49cb992 100644 --- a/.pylintrc +++ b/.pylintrc @@ -38,7 +38,7 @@ load-plugins=linter_plugin # --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=fixme,locally-disabled,abstract-class-not-used +disable=fixme,locally-disabled,abstract-class-not-used,duplicate-code # abstract-class-not-used cannot be disabled locally (at least in pylint 1.4.1) From 7b5b182f77085df6b31c7a489551dd5ec92da369 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Sep 2015 14:59:29 -0700 Subject: [PATCH 28/44] Test for _find_duplicative_certs --- letsencrypt/tests/cli_test.py | 73 +++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 613c3189b..0d7ec3b85 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,4 +1,5 @@ """Tests for letsencrypt.cli.""" +import configobj import itertools import os import shutil @@ -11,6 +12,10 @@ import mock from letsencrypt import account from letsencrypt import configuration from letsencrypt import errors +from letsencrypt import storage + +from letsencrypt.storage import ALL_FOUR +from letsencrypt.tests import test_util class CLITest(unittest.TestCase): @@ -162,5 +167,73 @@ class DetermineAccountTest(unittest.TestCase): self.assertEqual("other email", self.args.email) +class DuplicativeCertsTest(unittest.TestCase): + + def setUp(self): + # The stuff below is taken from renewer_test.py + self.tempdir = tempfile.mkdtemp() + self.cli_config = configuration.RenewerConfiguration( + namespace=mock.MagicMock(config_dir=self.tempdir)) + os.makedirs(os.path.join(self.tempdir, "live", "example.org")) + os.makedirs(os.path.join(self.tempdir, "archive", "example.org")) + os.makedirs(os.path.join(self.tempdir, "configs")) + config = configobj.ConfigObj() + for kind in ALL_FOUR: + config[kind] = os.path.join(self.tempdir, "live", "example.org", + kind + ".pem") + config.filename = os.path.join(self.tempdir, "configs", + "example.org.conf") + config.write() + self.config = config + self.defaults = configobj.ConfigObj() + self.test_rc = storage.RenewableCert( + self.config, self.defaults, self.cli_config) + for kind in ALL_FOUR: + where = getattr(self.test_rc, kind) + os.symlink(os.path.join("..", "..", "archive", "example.org", + "{0}12.pem".format(kind)), where) + with open(where, "w") as f: + f.write(kind) + os.unlink(where) + os.symlink(os.path.join("..", "..", "archive", "example.org", + "{0}11.pem".format(kind)), where) + with open(where, "w") as f: + f.write(kind) + + # Here we will use test_rc to create duplicative stuff + + def tearDown(self): + shutil.rmtree(self.tempdir) + + def test_find_duplicative_names(self): + from letsencrypt.cli import _find_duplicative_certs + test_cert = test_util.load_vector("cert-san.pem") + with open(self.test_rc.cert, "w") as f: + f.write(test_cert) + + # No overlap at all + result = _find_duplicative_certs(["wow.net", "hooray.org"], + self.config, self.cli_config) + self.assertEqual(result, (None, None)) + + # Totally identical + result = _find_duplicative_certs(["example.com", "www.example.com"], + self.config, self.cli_config) + self.assertEqual(result[0][0], "example.org.conf") + self.assertEqual(result[1], None) + + # Superset + result = _find_duplicative_certs(["example.com", "www.example.com", + "something.new"], self.config, + self.cli_config) + self.assertEqual(result[1][0], "example.org.conf") + self.assertEqual(result[0], None) + + # Partial overlap doesn't count + result = _find_duplicative_certs(["example.com", "something.new"], + self.config, self.cli_config) + self.assertEqual(result, (None, None)) + + if __name__ == '__main__': unittest.main() # pragma: no cover From f09016321bcc9dc54e823f633fd8f62130cae479 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 10 Sep 2015 15:12:36 -0700 Subject: [PATCH 29/44] Fix logic error if requesting nonduplicative cert --- letsencrypt/cli.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 194e50c2d..d7cbbc5b9 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -216,12 +216,15 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo treat_as_renewal = False # Considering the possibility that the requested certificate is - # related to an existing certificate. + # related to an existing certificate. (config.duplicate, which + # is set with --duplicate, skips all of this logic and forces any + # kind of certificate to be obtained with treat_as_renewal = False.) if not config.duplicate: identical_names_cert, subset_names_cert = _find_duplicative_certs( domains, config, configuration.RenewerConfiguration(config)) # I am not sure whether that correctly reads the systemwide # configuration file. + question = None if identical_names_cert: question = ( "You have an existing certificate that contains exactly the " @@ -237,7 +240,11 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo "certificate with the new certificate?" ).format(subset_names_cert[0], ", ".join(subset_names_cert[2]), ", ".join(domains)) - if zope.component.getUtility(interfaces.IDisplay).yesno( + if question is None: + # We aren't in a duplicative-names situation at all, so we don't + # have to tell or ask the user anything about this. + pass + elif zope.component.getUtility(interfaces.IDisplay).yesno( question, "Replace", "Cancel"): treat_as_renewal = True else: From c03f4977274c79df9ba7fe184585ab9abf8a5637 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 15:28:01 -0700 Subject: [PATCH 30/44] Add dependencies for known used modules --- .../letsencrypt_apache/configurator.py | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 8403b974c..b8528c407 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1004,18 +1004,53 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "Unsupported directory layout. You may try to enable mod %s " "and try again." % mod_name) + deps = self._get_mod_deps(mod_name) + + # Enable all dependencies + for dep in deps: + if dep not in self.parser.modules: + self._enable_mod_debian(dep, temp) + self._add_parser_mod(dep) + + note = "Enabled dependency of module %s - %s" % (mod_name, dep) + self.save_notes += note + os.linesep + logger.debug(note) + + # Enable actual module self._enable_mod_debian(mod_name, temp) - self.save_notes += "Enabled %s module in Apache" % mod_name - logger.debug("Enabled Apache %s module", mod_name) + self._add_parser_mod(mod_name) + + self.save_notes += "Enabled %s module in Apache\n" % mod_name + logger.info("Enabled Apache %s module", mod_name) # Modules can enable additional config files. Variables may be defined # within these new configuration sections. # Restart is not necessary as DUMP_RUN_CFG uses latest config. self.parser.update_runtime_variables(self.conf("ctl")) + def _add_parser_mod(self, mod_name): + """Shortcut for updating parser modules.""" self.parser.modules.add(mod_name + "_module") self.parser.modules.add("mod_" + mod_name + ".c") + def _get_mod_deps(self, mod_name): + """Get known module dependencies. + + .. note:: This does not need to be accurate in order for the client to + run. This simply keeps things clean if the user decides to revert + changes. + .. warning:: If all deps are not included, it may cause incorrect parsing + behavior, due to enable_mod's shortcut for updating the parser's + currently defined modules (:method:`._add_parser_mod`) + This would only present a major problem in extremely atypical + configs that use ifmod for the missing deps. + + """ + deps = { + "ssl": ["setenvif", "mime", "socache_shmcb"] + } + return deps.get(mod_name, []) + def _enable_mod_debian(self, mod_name, temp): """Assumes mods-available, mods-enabled layout.""" # Generate reversal command. From 8b093032aefcd345d61decc8c7e4d8f675caf426 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 15:39:13 -0700 Subject: [PATCH 31/44] Change debug/info output --- letsencrypt-apache/letsencrypt_apache/configurator.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index b8528c407..2da21a906 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -492,7 +492,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ if "ssl_module" not in self.parser.modules: - logger.info("Loading mod_ssl into Apache Server") self.enable_mod("ssl", temp=temp) # Check for Listen @@ -1012,7 +1011,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self._enable_mod_debian(dep, temp) self._add_parser_mod(dep) - note = "Enabled dependency of module %s - %s" % (mod_name, dep) + note = "Enabled dependency of %s module - %s" % (mod_name, dep) self.save_notes += note + os.linesep logger.debug(note) From b2ef04178570768cd5a39f8a7092d0fa34b1234a Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 15:57:27 -0700 Subject: [PATCH 32/44] Don't log notes if save is temporary --- letsencrypt-apache/letsencrypt_apache/configurator.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 2da21a906..2e1a7a824 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1012,14 +1012,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self._add_parser_mod(dep) note = "Enabled dependency of %s module - %s" % (mod_name, dep) - self.save_notes += note + os.linesep + if not temp: + self.save_notes += note + os.linesep logger.debug(note) # Enable actual module self._enable_mod_debian(mod_name, temp) self._add_parser_mod(mod_name) - self.save_notes += "Enabled %s module in Apache\n" % mod_name + if not temp: + self.save_notes += "Enabled %s module in Apache\n" % mod_name logger.info("Enabled Apache %s module", mod_name) # Modules can enable additional config files. Variables may be defined From 9c47b1061c7706ce19cb7a651c2a9332c2dd0e89 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 16:34:20 -0700 Subject: [PATCH 33/44] Search for correct module names in dependency list --- 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 2e1a7a824..579668aae 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1007,7 +1007,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Enable all dependencies for dep in deps: - if dep not in self.parser.modules: + if (dep + "_module") not in self.parser.modules: self._enable_mod_debian(dep, temp) self._add_parser_mod(dep) From 7a66bfef28f8594936022d9b376828e02b52d8f4 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 16:49:50 -0700 Subject: [PATCH 34/44] method to func, thanks pylint --- .../letsencrypt_apache/configurator.py | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 579668aae..e1af9c8a5 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1003,7 +1003,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "Unsupported directory layout. You may try to enable mod %s " "and try again." % mod_name) - deps = self._get_mod_deps(mod_name) + deps = _get_mod_deps(mod_name) # Enable all dependencies for dep in deps: @@ -1034,24 +1034,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser.modules.add(mod_name + "_module") self.parser.modules.add("mod_" + mod_name + ".c") - def _get_mod_deps(self, mod_name): - """Get known module dependencies. - - .. note:: This does not need to be accurate in order for the client to - run. This simply keeps things clean if the user decides to revert - changes. - .. warning:: If all deps are not included, it may cause incorrect parsing - behavior, due to enable_mod's shortcut for updating the parser's - currently defined modules (:method:`._add_parser_mod`) - This would only present a major problem in extremely atypical - configs that use ifmod for the missing deps. - - """ - deps = { - "ssl": ["setenvif", "mime", "socache_shmcb"] - } - return deps.get(mod_name, []) - def _enable_mod_debian(self, mod_name, temp): """Assumes mods-available, mods-enabled layout.""" # Generate reversal command. @@ -1176,6 +1158,25 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.parser.init_modules() +def _get_mod_deps(mod_name): + """Get known module dependencies. + + .. note:: This does not need to be accurate in order for the client to + run. This simply keeps things clean if the user decides to revert + changes. + .. warning:: If all deps are not included, it may cause incorrect parsing + behavior, due to enable_mod's shortcut for updating the parser's + currently defined modules (:method:`._add_parser_mod`) + This would only present a major problem in extremely atypical + configs that use ifmod for the missing deps. + + """ + deps = { + "ssl": ["setenvif", "mime", "socache_shmcb"] + } + return deps.get(mod_name, []) + + def apache_restart(apache_init_script): """Restarts the Apache Server. From 4fb27e035059cfead3fcfb5de63e2724cdca02ca Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 18:48:44 -0700 Subject: [PATCH 35/44] fix documentation link --- 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 e1af9c8a5..7e9ab9541 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -1166,7 +1166,7 @@ def _get_mod_deps(mod_name): changes. .. warning:: If all deps are not included, it may cause incorrect parsing behavior, due to enable_mod's shortcut for updating the parser's - currently defined modules (:method:`._add_parser_mod`) + currently defined modules (:method:`.ApacheConfigurator._add_parser_mod`) This would only present a major problem in extremely atypical configs that use ifmod for the missing deps. From 63b0d62f7bc6329bdc5c6c69920e451e309dd2fb Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 23:46:02 -0700 Subject: [PATCH 36/44] fix #765 --- letsencrypt/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 5b1e90edc..2ca423ae0 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -268,7 +268,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes if kind not in ALL_FOUR: raise errors.CertStorageError("unknown kind of item") where = os.path.dirname(self.current_target(kind)) - return os.path.join(where, "{0}{1}.pem".format(kind, version)) + return os.path.abspath( + os.path.join(where, "{0}{1}.pem".format(kind, version))) def available_versions(self, kind): """Which alternative versions of the specified kind of item exist? From 1a5f0c434e2b0956aabe8efc41cad6c0d629af00 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 11 Sep 2015 00:02:09 -0700 Subject: [PATCH 37/44] remove source of abspath problem... not side-effect --- letsencrypt/storage.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 2ca423ae0..e8b36d5cb 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -223,7 +223,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes target = os.readlink(link) if not os.path.isabs(target): target = os.path.join(os.path.dirname(link), target) - return target + return os.path.abspath(target) def current_version(self, kind): """Returns numerical version of the specified item. @@ -268,8 +268,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes if kind not in ALL_FOUR: raise errors.CertStorageError("unknown kind of item") where = os.path.dirname(self.current_target(kind)) - return os.path.abspath( - os.path.join(where, "{0}{1}.pem".format(kind, version))) + return os.path.join(where, "{0}{1}.pem".format(kind, version)) def available_versions(self, kind): """Which alternative versions of the specified kind of item exist? From 809f4966d6629e86f7ef517ef8b5ab9dd1321170 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 11 Sep 2015 07:04:13 +0000 Subject: [PATCH 38/44] Require pep8 in [testing] --- setup.py | 1 + tox.ini | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index f816c6c56..f8a12b29a 100644 --- a/setup.py +++ b/setup.py @@ -67,6 +67,7 @@ testing_extras = [ 'coverage', 'nose', 'nosexcover', + 'pep8', 'tox', ] diff --git a/tox.ini b/tox.ini index 4caecf681..f278c4bb5 100644 --- a/tox.ini +++ b/tox.ini @@ -36,9 +36,8 @@ basepython = python2.7 # duplicate code checking; if one of the commands fails, others will # continue, but tox return code will reflect previous error commands = - pip install pep8 - pep8 setup.py acme letsencrypt letsencrypt-apache letsencrypt-nginx letsencrypt-compatibility-test letshelp-letsencrypt pip install -r requirements.txt -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt + pep8 setup.py acme letsencrypt letsencrypt-apache letsencrypt-nginx letsencrypt-compatibility-test letshelp-letsencrypt pylint --rcfile=.pylintrc letsencrypt pylint --rcfile=.pylintrc acme/acme pylint --rcfile=.pylintrc letsencrypt-apache/letsencrypt_apache From 0ebef628463f30ea2fd74d876e9b4939977828fe Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 11 Sep 2015 07:05:56 +0000 Subject: [PATCH 39/44] Travis: no fail on pep8 --- pep8.travis.sh | 12 ++++++++++++ tox.ini | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100755 pep8.travis.sh diff --git a/pep8.travis.sh b/pep8.travis.sh new file mode 100755 index 000000000..ccac0a435 --- /dev/null +++ b/pep8.travis.sh @@ -0,0 +1,12 @@ +#!/bin/sh +pep8 \ + setup.py \ + acme \ + letsencrypt \ + letsencrypt-apache \ + letsencrypt-nginx \ + letsencrypt-compatibility-test \ + letshelp-letsencrypt \ + || echo "PEP8 checking failed, but it's ignored in Travis" + +# echo exits with 0 diff --git a/tox.ini b/tox.ini index f278c4bb5..2596050bc 100644 --- a/tox.ini +++ b/tox.ini @@ -37,7 +37,7 @@ basepython = python2.7 # continue, but tox return code will reflect previous error commands = pip install -r requirements.txt -e acme -e .[dev] -e letsencrypt-apache -e letsencrypt-nginx -e letsencrypt-compatibility-test -e letshelp-letsencrypt - pep8 setup.py acme letsencrypt letsencrypt-apache letsencrypt-nginx letsencrypt-compatibility-test letshelp-letsencrypt + ./pep8.travis.sh pylint --rcfile=.pylintrc letsencrypt pylint --rcfile=.pylintrc acme/acme pylint --rcfile=.pylintrc letsencrypt-apache/letsencrypt_apache From a38bb418567f2b0710ebd2f0bbc424ed3464a4f5 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 11 Sep 2015 13:49:26 -0700 Subject: [PATCH 40/44] PR cleanup --- letsencrypt/cli.py | 35 ++++++++++++++++------------------- letsencrypt/storage.py | 3 +-- letsencrypt/tests/cli_test.py | 11 +++++------ 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7244ad29e..317e7a541 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,9 +1,7 @@ """Let's Encrypt CLI.""" # TODO: Sanity check all input. Be sure to avoid shell code etc... - import argparse import atexit -import configobj import functools import logging import logging.handlers @@ -14,6 +12,7 @@ import time import traceback import configargparse +import configobj import OpenSSL import zope.component import zope.interface.exceptions @@ -173,22 +172,22 @@ def _find_duplicative_certs(domains, config, renew_config): identical_names_cert, subset_names_cert = None, None configs_dir = renew_config.renewal_configs_dir + cli_config = configuration.RenewerConfiguration(config) for renewal_file in os.listdir(configs_dir): full_path = os.path.join(configs_dir, renewal_file) rc_config = configobj.ConfigObj(renew_config.renewer_config_file) rc_config.merge(configobj.ConfigObj(full_path)) rc_config.filename = full_path - cli_config = configuration.RenewerConfiguration(config) - candidate_lineage = storage.RenewableCert(rc_config, None, cli_config) + candidate_lineage = storage.RenewableCert(rc_config, config_opts=None, + cli_config=cli_config) # TODO: Handle these differently depending on whether they are # expired or still valid? candidate_names = set(candidate_lineage.names()) if candidate_names == set(domains): - identical_names_cert = (renewal_file, candidate_lineage) + identical_names_cert = candidate_lineage elif candidate_names.issubset(set(domains)): - subset_names_cert = (renewal_file, candidate_lineage, - candidate_lineage.names()) + subset_names_cert = candidate_lineage return identical_names_cert, subset_names_cert @@ -229,20 +228,21 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo # I am not sure whether that correctly reads the systemwide # configuration file. question = None - if identical_names_cert: + if identical_names_cert is not None: question = ( "You have an existing certificate that contains exactly the " "same domains you requested (ref: {0})\n\nDo you want to " "renew and replace this certificate with a newly-issued one?" - ).format(identical_names_cert[0]) - elif subset_names_cert: + ).format(identical_names_cert.configfile.filename) + elif subset_names_cert is not None: question = ( "You have an existing certificate that contains a portion of " "the domains you requested (ref: {0})\n\nIt contains these " "names: {1}\n\nYou requested these names for the new " "certificate: {2}.\n\nDo you want to replace this existing " "certificate with the new certificate?" - ).format(subset_names_cert[0], ", ".join(subset_names_cert[2]), + ).format(subset_names_cert.configfile.filename, + ", ".join(subset_names_cert.names()), ", ".join(domains)) if question is None: # We aren't in a duplicative-names situation at all, so we don't @@ -257,19 +257,16 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo "To obtain a new certificate that {0} an existing certificate " "in its domain-name coverage, you must use the --duplicate " "option.\n\nFor example:\n\n{1} --duplicate {2}").format( - "duplicates" if identical_names_cert else "overlaps with", - sys.argv[0], " ".join(sys.argv[1:])), - reporter_util.HIGH_PRIORITY, True) - sys.exit(1) + "duplicates" if identical_names_cert is not None else + "overlaps with", sys.argv[0], " ".join(sys.argv[1:])), + reporter_util.HIGH_PRIORITY) + return 1 # Attempting to obtain the certificate # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) if treat_as_renewal: - if identical_names_cert: - lineage = identical_names_cert[1] - else: - lineage = subset_names_cert[1] + lineage = identical_names_cert if identical_names_cert is not None else subset_names_cert # TODO: Use existing privkey instead of generating a new one new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) # TODO: Check whether it worked! diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index bc9f32af5..504011180 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -437,8 +437,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes else: target = self.version("cert", version) with open(target) as f: - sans = crypto_util.get_sans_from_cert(f.read()) - return sans + return crypto_util.get_sans_from_cert(f.read()) def should_autodeploy(self): """Should this lineage now automatically deploy a newer version? diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 3903c824e..9442a3992 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,5 +1,4 @@ """Tests for letsencrypt.cli.""" -import configobj import itertools import os import shutil @@ -7,6 +6,7 @@ import traceback import tempfile import unittest +import configobj import mock from letsencrypt import account @@ -14,7 +14,6 @@ from letsencrypt import configuration from letsencrypt import errors from letsencrypt import storage -from letsencrypt.storage import ALL_FOUR from letsencrypt.tests import test_util @@ -178,7 +177,7 @@ class DuplicativeCertsTest(unittest.TestCase): os.makedirs(os.path.join(self.tempdir, "archive", "example.org")) os.makedirs(os.path.join(self.tempdir, "configs")) config = configobj.ConfigObj() - for kind in ALL_FOUR: + for kind in storage.ALL_FOUR: config[kind] = os.path.join(self.tempdir, "live", "example.org", kind + ".pem") config.filename = os.path.join(self.tempdir, "configs", @@ -188,7 +187,7 @@ class DuplicativeCertsTest(unittest.TestCase): self.defaults = configobj.ConfigObj() self.test_rc = storage.RenewableCert( self.config, self.defaults, self.cli_config) - for kind in ALL_FOUR: + for kind in storage.ALL_FOUR: where = getattr(self.test_rc, kind) os.symlink(os.path.join("..", "..", "archive", "example.org", "{0}12.pem".format(kind)), where) @@ -219,15 +218,15 @@ class DuplicativeCertsTest(unittest.TestCase): # Totally identical result = _find_duplicative_certs(["example.com", "www.example.com"], self.config, self.cli_config) - self.assertEqual(result[0][0], "example.org.conf") + self.assertTrue(result[0].configfile.filename.endswith("example.org.conf")) self.assertEqual(result[1], None) # Superset result = _find_duplicative_certs(["example.com", "www.example.com", "something.new"], self.config, self.cli_config) - self.assertEqual(result[1][0], "example.org.conf") self.assertEqual(result[0], None) + self.assertTrue(result[1].configfile.filename.endswith("example.org.conf")) # Partial overlap doesn't count result = _find_duplicative_certs(["example.com", "something.new"], From 06d87cb56cd3cbfc64a06b3d1f183dadb29eff24 Mon Sep 17 00:00:00 2001 From: Vladimir Rutsky Date: Sun, 13 Sep 2015 09:47:56 +0300 Subject: [PATCH 41/44] fix typo: "Python'd" -> "Python's" --- docs/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 7ddbdcf24..1398e818c 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -61,7 +61,7 @@ The following tools are there to help you: - For debugging, we recommend ``pip install ipdb`` and putting ``import ipdb; ipdb.set_trace()`` statement inside the source - code. Alternatively, you can use Python'd standard library `pdb`, + code. Alternatively, you can use Python's standard library `pdb`, but you won't get TAB completion... From d3cb4746e92ea2d7980bfba9cc6ebb4409950c46 Mon Sep 17 00:00:00 2001 From: Vladimir Rutsky Date: Sun, 13 Sep 2015 09:53:53 +0300 Subject: [PATCH 42/44] fix path to script with nginx prerequisites The path is copied from `.. include` directive below. --- docs/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 7ddbdcf24..c506bd2a9 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -81,7 +81,7 @@ patient - it will take some time... Once its ready, you will see If you would like to test `letsencrypt_nginx` plugin (highly encouraged) make sure to install prerequisites as listed in -``tests/integration/nginx.sh``: +``letsencrypt-nginx/tests/boulder-integration.sh``: .. include:: ../letsencrypt-nginx/tests/boulder-integration.sh :start-line: 1 From d66e65af1116f3557eeb98c548679e83aaab97ea Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Sep 2015 10:31:43 -0700 Subject: [PATCH 43/44] Remove short -D for --duplicate --- letsencrypt/cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 317e7a541..a9447b2b2 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -193,7 +193,6 @@ def _find_duplicative_certs(domains, config, renew_config): def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals - """Obtain a certificate and install.""" if args.configurator is not None and (args.installer is not None or args.authenticator is not None): @@ -594,7 +593,7 @@ def create_parser(plugins, args): # subparser.add_argument("domains", nargs="*", metavar="domain") helpful.add(None, "-d", "--domains", metavar="DOMAIN", action="append") helpful.add( - None, "-D", "--duplicate", dest="duplicate", action="store_true", + None, "--duplicate", dest="duplicate", action="store_true", help="Allow getting a certificate that duplicates an existing one") helpful.add_group( From f160a51aa708ee53361fb160f41cc64c6c40e400 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 15 Sep 2015 17:27:27 -0700 Subject: [PATCH 44/44] Don't crash if an existing lineage is slightly corrupt --- letsencrypt/cli.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a9447b2b2..c6f90ea70 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -174,13 +174,17 @@ def _find_duplicative_certs(domains, config, renew_config): configs_dir = renew_config.renewal_configs_dir cli_config = configuration.RenewerConfiguration(config) for renewal_file in os.listdir(configs_dir): - full_path = os.path.join(configs_dir, renewal_file) - rc_config = configobj.ConfigObj(renew_config.renewer_config_file) - rc_config.merge(configobj.ConfigObj(full_path)) - rc_config.filename = full_path - - candidate_lineage = storage.RenewableCert(rc_config, config_opts=None, - cli_config=cli_config) + try: + full_path = os.path.join(configs_dir, renewal_file) + rc_config = configobj.ConfigObj(renew_config.renewer_config_file) + rc_config.merge(configobj.ConfigObj(full_path)) + rc_config.filename = full_path + candidate_lineage = storage.RenewableCert( + rc_config, config_opts=None, cli_config=cli_config) + except (configobj.ConfigObjError, errors.CertStorageError, IOError): + logger.warning("Renewal configuration file %s is broken. " + "Skipping.", full_path) + continue # TODO: Handle these differently depending on whether they are # expired or still valid? candidate_names = set(candidate_lineage.names())