From 3b73b04bfed721a36e6933423d7b796f14810feb Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 25 Aug 2015 07:01:30 +0000 Subject: [PATCH 01/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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/28] 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 bf754b6302e87b09a3b4b3c966226dfb7dce0dc5 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 8 Sep 2015 20:37:09 +0000 Subject: [PATCH 11/28] 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 12/28] 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 13/28] 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 14/28] 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 15/28] 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 16/28] 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 17/28] 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 18/28] 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 c03f4977274c79df9ba7fe184585ab9abf8a5637 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 10 Sep 2015 15:28:01 -0700 Subject: [PATCH 19/28] 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 20/28] 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 21/28] 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 22/28] 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 23/28] 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 24/28] 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 25/28] 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 26/28] 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 27/28] 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 28/28] 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