From c235803bd4d9b07b04cc75c946edaeddc116ec9f Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Mon, 1 Feb 2016 12:32:08 +0200 Subject: [PATCH 01/56] Adding --allow-subset-of-names flag --- letsencrypt/cli.py | 4 ++++ letsencrypt/client.py | 2 +- letsencrypt/tests/client_test.py | 6 ++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index d5811538e..734fc9d51 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1603,6 +1603,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") + helpful.add( + "automation", "--allow-subset-of-names", dest="allow_subset_of_names", + action="store_true", default=False, + help="Allow subsets of domain names to fail validation without exiting.") helpful.add_group( "renew", description="The 'renew' subcommand will attempt to renew all" diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 9dfa70e8d..06a09257a 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -222,7 +222,7 @@ class Client(object): logger.debug("CSR: %s, domains: %s", csr, domains) - authzr = self.auth_handler.get_authorizations(domains) + authzr = self.auth_handler.get_authorizations(domains, self.config.allow_subset_of_names) certr = self.acme.request_issuance( jose.ComparableX509( OpenSSL.crypto.load_certificate_request(typ, csr.data)), diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index dbc57565e..7a42cd4a3 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -79,7 +79,7 @@ class ClientTest(unittest.TestCase): def setUp(self): self.config = mock.MagicMock( - no_verify_ssl=False, config_dir="/etc/letsencrypt") + no_verify_ssl=False, config_dir="/etc/letsencrypt", allow_subset_of_names=False) # pylint: disable=star-args self.account = mock.MagicMock(**{"key.pem": KEY}) self.eg_domains = ["example.com", "www.example.com"] @@ -102,7 +102,9 @@ class ClientTest(unittest.TestCase): self.acme.fetch_chain.return_value = mock.sentinel.chain def _check_obtain_certificate(self): - self.client.auth_handler.get_authorizations.assert_called_once_with(self.eg_domains) + self.client.auth_handler.get_authorizations.assert_called_once_with( + self.eg_domains, + self.config.allow_subset_of_names) self.acme.request_issuance.assert_called_once_with( jose.ComparableX509(OpenSSL.crypto.load_certificate_request( OpenSSL.crypto.FILETYPE_ASN1, CSR_SAN)), From b68c5ace0c50a16af482be8e901102c5d7a3bfdc Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Wed, 3 Feb 2016 11:54:39 +0200 Subject: [PATCH 02/56] Creating CSR after auth --- letsencrypt/auth_handler.py | 32 +++++++++++++++++++++++++------- letsencrypt/client.py | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index ffbd70ced..94cf8639c 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -42,6 +42,7 @@ class AuthHandler(object): form of :class:`letsencrypt.achallenges.AnnotatedChallenge` """ + def __init__(self, dv_auth, cont_auth, acme, account): self.dv_auth = dv_auth self.cont_auth = cont_auth @@ -75,19 +76,32 @@ class AuthHandler(object): self._choose_challenges(domains) + failed_domains = set() + # While there are still challenges remaining... while self.dv_c or self.cont_c: cont_resp, dv_resp = self._solve_challenges() logger.info("Waiting for verification...") # Send all Responses - this modifies dv_c and cont_c - self._respond(cont_resp, dv_resp, best_effort) + response = self._respond(cont_resp, dv_resp, best_effort) + + if response: + failed_domains = failed_domains.union(response) + + my_authzr = self.authzr + + returnDomains = set() + #Remove failing domains if best_effort is true + for domain in domains: + if not domain in failed_domains: + returnDomains.add(domain) # Just make sure all decisions are complete. self.verify_authzr_complete() # Only return valid authorizations - return [authzr for authzr in self.authzr.values() - if authzr.body.status == messages.STATUS_VALID] + return ([authzr for authzr in my_authzr.values() + if authzr.body.status == messages.STATUS_VALID], returnDomains) def _choose_challenges(self, domains): """Retrieve necessary challenges to satisfy server.""" @@ -139,11 +153,13 @@ class AuthHandler(object): # Check for updated status... try: - self._poll_challenges(chall_update, best_effort) + result = self._poll_challenges(chall_update, best_effort) finally: # This removes challenges from self.dv_c and self.cont_c self._cleanup_challenges(active_achalls) + return result + def _send_responses(self, achalls, resps, chall_update): """Send responses and make sure errors are handled. @@ -175,6 +191,7 @@ class AuthHandler(object): """Wait for all challenge results to be determined.""" dom_to_check = set(chall_update.keys()) comp_domains = set() + failed_domains = set() rounds = 0 while dom_to_check and rounds < max_rounds: @@ -192,9 +209,8 @@ class AuthHandler(object): chall_update[domain].remove(achall) # We failed some challenges... damage control else: - # Right now... just assume a loss and carry on... if best_effort: - comp_domains.add(domain) + failed_domains.add(domain) else: all_failed_achalls.update( updated for _, updated in failed_achalls) @@ -203,10 +219,12 @@ class AuthHandler(object): _report_failed_challs(all_failed_achalls) raise errors.FailedChallenges(all_failed_achalls) - dom_to_check -= comp_domains + dom_to_check -= comp_domains.union(failed_domains) comp_domains.clear() rounds += 1 + return failed_domains + def _handle_check(self, domain, achalls): """Returns tuple of ('completed', 'failed').""" completed = [] diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 06a09257a..928249caa 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,7 +195,7 @@ class Client(object): else: self.auth_handler = None - def obtain_certificate_from_csr(self, domains, csr, + def _obtain_certificate(self, domains, csr, authzr, typ=OpenSSL.crypto.FILETYPE_ASN1): """Obtain certificate. @@ -222,13 +222,35 @@ class Client(object): logger.debug("CSR: %s, domains: %s", csr, domains) - authzr = self.auth_handler.get_authorizations(domains, self.config.allow_subset_of_names) certr = self.acme.request_issuance( jose.ComparableX509( OpenSSL.crypto.load_certificate_request(typ, csr.data)), authzr) return certr, self.acme.fetch_chain(certr) +<<<<<<< HEAD +======= + def obtain_certificate_from_csr(self, csr): + """Obtain certficiate from CSR. + + :param .le_util.CSR csr: DER-encoded Certificate Signing + Request. + + :returns: `.CertificateResource` and certificate chain (as + returned by `.fetch_chain`). + :rtype: tuple + + """ + domains = crypto_util.get_sans_from_csr( + csr.data, OpenSSL.crypto.FILETYPE_ASN1) + + authzr, domains = self.auth_handler.get_authorizations(domains, + self.config.allow_subset_of_names) + + return self._obtain_certificate( + # TODO: add CN to domains? + domains, csr, authzr) +>>>>>>> Creating CSR after auth def obtain_certificate(self, domains): """Obtains a certificate from the ACME server. @@ -244,12 +266,19 @@ class Client(object): :rtype: tuple """ + authzr, domains = self.auth_handler.get_authorizations(domains, + self.config.allow_subset_of_names) + # Create CSR from names key = crypto_util.init_save_key( self.config.rsa_key_size, self.config.key_dir) csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) +<<<<<<< HEAD return self.obtain_certificate_from_csr(domains, csr) + (key, csr) +======= + return self._obtain_certificate(domains, csr, authzr) + (key, csr) +>>>>>>> Creating CSR after auth def obtain_and_enroll_certificate(self, domains): """Obtain and enroll certificate. From fc2c90726113da922919abc176b35b28d78df516 Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Sat, 6 Feb 2016 15:07:27 +0200 Subject: [PATCH 03/56] Fixing tox cover --- letsencrypt/auth_handler.py | 32 +++++++------------------------- letsencrypt/tests/client_test.py | 12 +++++++++++- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 94cf8639c..ffbd70ced 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -42,7 +42,6 @@ class AuthHandler(object): form of :class:`letsencrypt.achallenges.AnnotatedChallenge` """ - def __init__(self, dv_auth, cont_auth, acme, account): self.dv_auth = dv_auth self.cont_auth = cont_auth @@ -76,32 +75,19 @@ class AuthHandler(object): self._choose_challenges(domains) - failed_domains = set() - # While there are still challenges remaining... while self.dv_c or self.cont_c: cont_resp, dv_resp = self._solve_challenges() logger.info("Waiting for verification...") # Send all Responses - this modifies dv_c and cont_c - response = self._respond(cont_resp, dv_resp, best_effort) - - if response: - failed_domains = failed_domains.union(response) - - my_authzr = self.authzr - - returnDomains = set() - #Remove failing domains if best_effort is true - for domain in domains: - if not domain in failed_domains: - returnDomains.add(domain) + self._respond(cont_resp, dv_resp, best_effort) # Just make sure all decisions are complete. self.verify_authzr_complete() # Only return valid authorizations - return ([authzr for authzr in my_authzr.values() - if authzr.body.status == messages.STATUS_VALID], returnDomains) + return [authzr for authzr in self.authzr.values() + if authzr.body.status == messages.STATUS_VALID] def _choose_challenges(self, domains): """Retrieve necessary challenges to satisfy server.""" @@ -153,13 +139,11 @@ class AuthHandler(object): # Check for updated status... try: - result = self._poll_challenges(chall_update, best_effort) + self._poll_challenges(chall_update, best_effort) finally: # This removes challenges from self.dv_c and self.cont_c self._cleanup_challenges(active_achalls) - return result - def _send_responses(self, achalls, resps, chall_update): """Send responses and make sure errors are handled. @@ -191,7 +175,6 @@ class AuthHandler(object): """Wait for all challenge results to be determined.""" dom_to_check = set(chall_update.keys()) comp_domains = set() - failed_domains = set() rounds = 0 while dom_to_check and rounds < max_rounds: @@ -209,8 +192,9 @@ class AuthHandler(object): chall_update[domain].remove(achall) # We failed some challenges... damage control else: + # Right now... just assume a loss and carry on... if best_effort: - failed_domains.add(domain) + comp_domains.add(domain) else: all_failed_achalls.update( updated for _, updated in failed_achalls) @@ -219,12 +203,10 @@ class AuthHandler(object): _report_failed_challs(all_failed_achalls) raise errors.FailedChallenges(all_failed_achalls) - dom_to_check -= comp_domains.union(failed_domains) + dom_to_check -= comp_domains comp_domains.clear() rounds += 1 - return failed_domains - def _handle_check(self, domain, achalls): """Returns tuple of ('completed', 'failed').""" completed = [] diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 7a42cd4a3..395539659 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -98,6 +98,7 @@ class ClientTest(unittest.TestCase): def _mock_obtain_certificate(self): self.client.auth_handler = mock.MagicMock() + self.client.auth_handler.get_authorizations.return_value = (None, None) self.acme.request_issuance.return_value = mock.sentinel.certr self.acme.fetch_chain.return_value = mock.sentinel.chain @@ -105,10 +106,14 @@ class ClientTest(unittest.TestCase): self.client.auth_handler.get_authorizations.assert_called_once_with( self.eg_domains, self.config.allow_subset_of_names) + + authzr, _ = self.client.auth_handler.get_authorizations() + self.acme.request_issuance.assert_called_once_with( jose.ComparableX509(OpenSSL.crypto.load_certificate_request( OpenSSL.crypto.FILETYPE_ASN1, CSR_SAN)), - self.client.auth_handler.get_authorizations()) + authzr) + self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr) # FIXME move parts of this to test_cli.py... @@ -148,6 +153,11 @@ class ClientTest(unittest.TestCase): mock_crypto_util.init_save_key.return_value = mock.sentinel.key domains = ["example.com", "www.example.com"] + # return_value is essentially set to (None, None) in + # _mock_obtain_certificate(), which breaks this test. + # Thus fixed by the next line. + self.client.auth_handler.get_authorizations.return_value = (None, domains) + self.assertEqual( self.client.obtain_certificate(domains), (mock.sentinel.certr, mock.sentinel.chain, mock.sentinel.key, csr)) From acc4f52745d0f91b51a945dec5fd3444a5bb6b81 Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Tue, 9 Feb 2016 00:49:24 +0200 Subject: [PATCH 04/56] Fixing integration testing --- letsencrypt/auth_handler.py | 31 ++++++++++++++++++++------ letsencrypt/tests/auth_handler_test.py | 4 ++-- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index ffbd70ced..4c5d2b869 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -75,19 +75,32 @@ class AuthHandler(object): self._choose_challenges(domains) + failed_domains = set() + # While there are still challenges remaining... while self.dv_c or self.cont_c: cont_resp, dv_resp = self._solve_challenges() logger.info("Waiting for verification...") # Send all Responses - this modifies dv_c and cont_c - self._respond(cont_resp, dv_resp, best_effort) + response = self._respond(cont_resp, dv_resp, best_effort) + + if response: + failed_domains = failed_domains.union(response) + + my_authzr = self.authzr + + returnDomains = [] + #Remove failing domains if best_effort is true + for domain in domains: + if not domain in failed_domains: + returnDomains.append(domain) # Just make sure all decisions are complete. self.verify_authzr_complete() # Only return valid authorizations - return [authzr for authzr in self.authzr.values() - if authzr.body.status == messages.STATUS_VALID] + return ([authzr for authzr in my_authzr.values() + if authzr.body.status == messages.STATUS_VALID], returnDomains) def _choose_challenges(self, domains): """Retrieve necessary challenges to satisfy server.""" @@ -139,11 +152,13 @@ class AuthHandler(object): # Check for updated status... try: - self._poll_challenges(chall_update, best_effort) + result = self._poll_challenges(chall_update, best_effort) finally: # This removes challenges from self.dv_c and self.cont_c self._cleanup_challenges(active_achalls) + return result + def _send_responses(self, achalls, resps, chall_update): """Send responses and make sure errors are handled. @@ -175,6 +190,7 @@ class AuthHandler(object): """Wait for all challenge results to be determined.""" dom_to_check = set(chall_update.keys()) comp_domains = set() + failed_domains = set() rounds = 0 while dom_to_check and rounds < max_rounds: @@ -192,9 +208,8 @@ class AuthHandler(object): chall_update[domain].remove(achall) # We failed some challenges... damage control else: - # Right now... just assume a loss and carry on... if best_effort: - comp_domains.add(domain) + failed_domains.add(domain) else: all_failed_achalls.update( updated for _, updated in failed_achalls) @@ -203,10 +218,12 @@ class AuthHandler(object): _report_failed_challs(all_failed_achalls) raise errors.FailedChallenges(all_failed_achalls) - dom_to_check -= comp_domains + dom_to_check -= comp_domains.union(failed_domains) comp_domains.clear() rounds += 1 + return failed_domains + def _handle_check(self, domain, achalls): """Returns tuple of ('completed', 'failed').""" completed = [] diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 5a6199ca3..ad658353f 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -96,7 +96,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr = self.handler.get_authorizations(["0"]) + authzr, domains = self.handler.get_authorizations(["0"]) self.assertEqual(self.mock_net.answer_challenge.call_count, 1) @@ -120,7 +120,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr = self.handler.get_authorizations(["0", "1", "2"]) + authzr, domains = self.handler.get_authorizations(["0", "1", "2"]) self.assertEqual(self.mock_net.answer_challenge.call_count, 6) From 4f53476cf012d8588408f1f67bf8c3e769204989 Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Tue, 9 Feb 2016 02:04:02 +0200 Subject: [PATCH 05/56] Fixing lint --- letsencrypt/tests/auth_handler_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index ad658353f..313939a97 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -96,7 +96,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr, domains = self.handler.get_authorizations(["0"]) + authzr, _ = self.handler.get_authorizations(["0"]) self.assertEqual(self.mock_net.answer_challenge.call_count, 1) @@ -120,7 +120,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr, domains = self.handler.get_authorizations(["0", "1", "2"]) + authzr, _ = self.handler.get_authorizations(["0", "1", "2"]) self.assertEqual(self.mock_net.answer_challenge.call_count, 6) From 7b2c2d3a482177ef3560dddb03355a28813d4686 Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Fri, 12 Feb 2016 14:36:12 +0200 Subject: [PATCH 06/56] Fixing more conflicts --- letsencrypt/client.py | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 928249caa..dad5b87ff 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,7 +195,7 @@ class Client(object): else: self.auth_handler = None - def _obtain_certificate(self, domains, csr, authzr, + def obtain_certificate_from_csr(self, domains, csr, authzr, typ=OpenSSL.crypto.FILETYPE_ASN1): """Obtain certificate. @@ -228,30 +228,6 @@ class Client(object): authzr) return certr, self.acme.fetch_chain(certr) -<<<<<<< HEAD -======= - def obtain_certificate_from_csr(self, csr): - """Obtain certficiate from CSR. - - :param .le_util.CSR csr: DER-encoded Certificate Signing - Request. - - :returns: `.CertificateResource` and certificate chain (as - returned by `.fetch_chain`). - :rtype: tuple - - """ - domains = crypto_util.get_sans_from_csr( - csr.data, OpenSSL.crypto.FILETYPE_ASN1) - - authzr, domains = self.auth_handler.get_authorizations(domains, - self.config.allow_subset_of_names) - - return self._obtain_certificate( - # TODO: add CN to domains? - domains, csr, authzr) ->>>>>>> Creating CSR after auth - def obtain_certificate(self, domains): """Obtains a certificate from the ACME server. @@ -274,11 +250,7 @@ class Client(object): self.config.rsa_key_size, self.config.key_dir) csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) -<<<<<<< HEAD - return self.obtain_certificate_from_csr(domains, csr) + (key, csr) -======= - return self._obtain_certificate(domains, csr, authzr) + (key, csr) ->>>>>>> Creating CSR after auth + return self.obtain_certificate_from_csr(domains, csr, authzr) + (key, csr) def obtain_and_enroll_certificate(self, domains): """Obtain and enroll certificate. From d38828751f7eeb24039f4880ef0512e9c68baf4a Mon Sep 17 00:00:00 2001 From: TheNavigat Date: Fri, 12 Feb 2016 14:45:19 +0200 Subject: [PATCH 07/56] Fixing tests --- letsencrypt/auth_handler.py | 2 ++ letsencrypt/cli.py | 2 +- letsencrypt/client.py | 9 +++++++-- letsencrypt/tests/client_test.py | 7 ++++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 4c5d2b869..9afd4f324 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -90,6 +90,8 @@ class AuthHandler(object): my_authzr = self.authzr + logger.debug("authzr: %s", my_authzr) + returnDomains = [] #Remove failing domains if best_effort is true for domain in domains: diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 734fc9d51..49b0e53ff 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -694,7 +694,7 @@ def obtain_cert(config, plugins, lineage=None): if config.csr is not None: assert lineage is None, "Did not expect a CSR with a RenewableCert" csr, typ = config.actual_csr - certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ) + certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, False, typ) if config.dry_run: logger.info( "Dry run: skipping saving certificate to %s", config.cert_path) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index dad5b87ff..c9b094910 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,7 +195,7 @@ class Client(object): else: self.auth_handler = None - def obtain_certificate_from_csr(self, domains, csr, authzr, + def obtain_certificate_from_csr(self, domains, csr, authzr=False, typ=OpenSSL.crypto.FILETYPE_ASN1): """Obtain certificate. @@ -222,10 +222,15 @@ class Client(object): logger.debug("CSR: %s, domains: %s", csr, domains) + if authzr is False: + authzr, _ = self.auth_handler.get_authorizations( + domains, + self.config.allow_subset_of_names) + certr = self.acme.request_issuance( jose.ComparableX509( OpenSSL.crypto.load_certificate_request(typ, csr.data)), - authzr) + authzr) return certr, self.acme.fetch_chain(certr) def obtain_certificate(self, domains): diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 395539659..220f60a38 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -137,9 +137,14 @@ class ClientTest(unittest.TestCase): self.assertRaises(errors.ConfigurationError, cli.HelpfulArgumentParser.handle_csr, mock_parser, mock_parsed_args) + authzr, _ = self.client.auth_handler.get_authorizations(self.eg_domains, False) + self.assertEqual( (mock.sentinel.certr, mock.sentinel.chain), - self.client.obtain_certificate_from_csr(self.eg_domains, test_csr)) + self.client.obtain_certificate_from_csr( + self.eg_domains, + test_csr, + authzr)) # and that the cert was obtained correctly self._check_obtain_certificate() From de31ece45a0d1b9105b85f1871d528c0842949d8 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Sat, 27 Feb 2016 01:03:15 +0200 Subject: [PATCH 08/56] Fixing styling and naming issues --- letsencrypt/auth_handler.py | 19 ++++++------------- letsencrypt/cli.py | 5 +++-- letsencrypt/client.py | 6 +++--- letsencrypt/tests/client_test.py | 2 +- 4 files changed, 13 insertions(+), 19 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 9afd4f324..63426f7bd 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -88,21 +88,14 @@ class AuthHandler(object): if response: failed_domains = failed_domains.union(response) - my_authzr = self.authzr - - logger.debug("authzr: %s", my_authzr) - - returnDomains = [] - #Remove failing domains if best_effort is true - for domain in domains: - if not domain in failed_domains: - returnDomains.append(domain) + returnDomains = [domain for domain in domains + if domain not in failed_domains] # Just make sure all decisions are complete. self.verify_authzr_complete() # Only return valid authorizations - return ([authzr for authzr in my_authzr.values() - if authzr.body.status == messages.STATUS_VALID], returnDomains) + return [authzr for authzr in self.authzr.values() + if authzr.body.status == messages.STATUS_VALID], returnDomains def _choose_challenges(self, domains): """Retrieve necessary challenges to satisfy server.""" @@ -154,12 +147,12 @@ class AuthHandler(object): # Check for updated status... try: - result = self._poll_challenges(chall_update, best_effort) + failed_domains = self._poll_challenges(chall_update, best_effort) finally: # This removes challenges from self.dv_c and self.cont_c self._cleanup_challenges(active_achalls) - return result + return failed_domains def _send_responses(self, achalls, resps, chall_update): """Send responses and make sure errors are handled. diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 49b0e53ff..eba05055d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -694,7 +694,7 @@ def obtain_cert(config, plugins, lineage=None): if config.csr is not None: assert lineage is None, "Did not expect a CSR with a RenewableCert" csr, typ = config.actual_csr - certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, False, typ) + certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ, authzr=False) if config.dry_run: logger.info( "Dry run: skipping saving certificate to %s", config.cert_path) @@ -1606,7 +1606,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): helpful.add( "automation", "--allow-subset-of-names", dest="allow_subset_of_names", action="store_true", default=False, - help="Allow subsets of domain names to fail validation without exiting.") + help="Allow subsets of domain names in a single lineage to fail " + "validation without exiting.") helpful.add_group( "renew", description="The 'renew' subcommand will attempt to renew all" diff --git a/letsencrypt/client.py b/letsencrypt/client.py index c9b094910..d825f6da7 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -195,8 +195,8 @@ class Client(object): else: self.auth_handler = None - def obtain_certificate_from_csr(self, domains, csr, authzr=False, - typ=OpenSSL.crypto.FILETYPE_ASN1): + def obtain_certificate_from_csr(self, domains, csr, + typ=OpenSSL.crypto.FILETYPE_ASN1, authzr=False): """Obtain certificate. Internal function with precondition that `domains` are @@ -255,7 +255,7 @@ class Client(object): self.config.rsa_key_size, self.config.key_dir) csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) - return self.obtain_certificate_from_csr(domains, csr, authzr) + (key, csr) + return self.obtain_certificate_from_csr(domains, csr, authzr=authzr) + (key, csr) def obtain_and_enroll_certificate(self, domains): """Obtain and enroll certificate. diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 220f60a38..07f76b64a 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -144,7 +144,7 @@ class ClientTest(unittest.TestCase): self.client.obtain_certificate_from_csr( self.eg_domains, test_csr, - authzr)) + authzr=authzr)) # and that the cert was obtained correctly self._check_obtain_certificate() From e64fd392dc60fb01a37ccdfd9110d115c2b3f7d5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 28 Feb 2016 23:34:44 -0800 Subject: [PATCH 09/56] Rope refactor: cli -> main --- letsencrypt/cli.py | 712 ++-------------------------------- letsencrypt/main.py | 696 +++++++++++++++++++++++++++++++++ letsencrypt/tests/cli_test.py | 30 +- 3 files changed, 736 insertions(+), 702 deletions(-) create mode 100644 letsencrypt/main.py diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3551d5a10..d308db72e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,9 +1,5 @@ """Let's Encrypt CLI.""" from __future__ import print_function - -# TODO: Sanity check all input. Be sure to avoid shell code etc... -# pylint: disable=too-many-lines -# (TODO: split this file into main.py and cli.py) import argparse import atexit import copy @@ -23,10 +19,6 @@ import zope.component import zope.interface.exceptions import zope.interface.verify -from acme import jose - -import letsencrypt - from letsencrypt import account from letsencrypt import colored_logging from letsencrypt import configuration @@ -37,6 +29,7 @@ from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import log +from letsencrypt import main from letsencrypt import reporter from letsencrypt import storage @@ -44,6 +37,15 @@ from letsencrypt.display import util as display_util from letsencrypt.display import ops as display_ops from letsencrypt.plugins import disco as plugins_disco +# TODO: Sanity check all input. Be sure to avoid shell code etc... +# pylint: disable=too-many-lines +# (TODO: split this file into main.py and cli.py) + + + + + + logger = logging.getLogger(__name__) # Global, to save us from a lot of argument passing within the scope of this module @@ -127,158 +129,7 @@ def usage_strings(plugins): return USAGE % (apache_doc, nginx_doc), SHORT_USAGE -def _find_domains(config, installer): - if not config.domains: - domains = display_ops.choose_names(installer) - # record in config.domains (so that it can be serialised in renewal config files), - # and set webroot_map entries if applicable - for d in domains: - _process_domain(config, d) - else: - domains = config.domains - - if not domains: - raise errors.Error("Please specify --domains, or --installer that " - "will help in domain names autodiscovery") - - return domains - - -def _determine_account(config): - """Determine which account to use. - - In order to make the renewer (configuration de/serialization) happy, - if ``config.account`` is ``None``, it will be updated based on the - user input. Same for ``config.email``. - - :param argparse.Namespace config: CLI arguments - :param letsencrypt.interface.IConfig config: Configuration object - :param .AccountStorage account_storage: Account storage. - - :returns: Account and optionally ACME client API (biproduct of new - registration). - :rtype: `tuple` of `letsencrypt.account.Account` and - `acme.client.Client` - - """ - account_storage = account.AccountFileStorage(config) - acme = None - - if config.account is not None: - acc = account_storage.load(config.account) - else: - accounts = account_storage.find_all() - if len(accounts) > 1: - acc = display_ops.choose_account(accounts) - elif len(accounts) == 1: - acc = accounts[0] - else: # no account registered yet - if config.email is None and not config.register_unsafely_without_email: - config.namespace.email = display_ops.get_email() - - def _tos_cb(regr): - if config.tos: - return True - msg = ("Please read the Terms of Service at {0}. You " - "must agree in order to register with the ACME " - "server at {1}".format( - regr.terms_of_service, config.server)) - obj = zope.component.getUtility(interfaces.IDisplay) - return obj.yesno(msg, "Agree", "Cancel", cli_flag="--agree-tos") - - try: - acc, acme = client.register( - config, account_storage, tos_cb=_tos_cb) - except errors.MissingCommandlineFlag: - raise - except errors.Error as error: - logger.debug(error, exc_info=True) - raise errors.Error( - "Unable to register an account with ACME server") - - config.namespace.account = acc.id - return acc, acme - - -def _init_le_client(config, authenticator, installer): - if authenticator is not None: - # if authenticator was given, then we will need account... - acc, acme = _determine_account(config) - logger.debug("Picked account: %r", acc) - # XXX - #crypto_util.validate_key_csr(acc.key) - else: - acc, acme = None, None - - return client.Client(config, acc, authenticator, installer, acme=acme) - - -def _find_duplicative_certs(config, domains): - """Find existing certs that duplicate the request.""" - - identical_names_cert, subset_names_cert = None, None - - cli_config = configuration.RenewerConfiguration(config) - configs_dir = cli_config.renewal_configs_dir - # Verify the directory is there - le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) - - for renewal_file in _renewal_conf_files(cli_config): - try: - candidate_lineage = storage.RenewableCert(renewal_file, cli_config) - except (errors.CertStorageError, IOError): - logger.warning("Renewal conf file %s is broken. Skipping.", renewal_file) - logger.debug("Traceback was:\n%s", traceback.format_exc()) - continue - # TODO: Handle these differently depending on whether they are - # expired or still valid? - candidate_names = set(candidate_lineage.names()) - if candidate_names == set(domains): - identical_names_cert = candidate_lineage - elif candidate_names.issubset(set(domains)): - # This logic finds and returns the largest subset-names cert - # in the case where there are several available. - if subset_names_cert is None: - subset_names_cert = candidate_lineage - elif len(candidate_names) > len(subset_names_cert.names()): - subset_names_cert = candidate_lineage - - return identical_names_cert, subset_names_cert - - -def _treat_as_renewal(config, domains): - """Determine whether there are duplicated names and how to handle - them (renew, reinstall, newcert, or raising an error to stop - the client run if the user chooses to cancel the operation when - prompted). - - :returns: Two-element tuple containing desired new-certificate behavior as - a string token ("reinstall", "renew", or "newcert"), plus either - a RenewableCert instance or None if renewal shouldn't occur. - - :raises .Error: If the user would like to rerun the client again. - - """ - # Considering the possibility that the requested certificate is - # related to an existing certificate. (config.duplicate, which - # is set with --duplicate, skips all of this logic and forces any - # kind of certificate to be obtained with renewal = False.) - if config.duplicate: - return "newcert", None - # TODO: Also address superset case - ident_names_cert, subset_names_cert = _find_duplicative_certs(config, domains) - # XXX ^ schoen is not sure whether that correctly reads the systemwide - # configuration file. - if ident_names_cert is None and subset_names_cert is None: - return "newcert", None - - if ident_names_cert is not None: - return _handle_identical_cert_request(config, ident_names_cert) - elif subset_names_cert is not None: - return _handle_subset_cert_request(config, domains, subset_names_cert) - - -def _should_renew(config, lineage): +def should_renew(config, lineage): "Return true if any of the circumstances for automatic renewal apply." if config.renew_by_default: logger.info("Auto-renewal forced with --force-renewal...") @@ -293,217 +144,6 @@ def _should_renew(config, lineage): return False -def _handle_identical_cert_request(config, cert): - """Figure out what to do if a cert has the same names as a previously obtained one - - :param storage.RenewableCert cert: - - :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal - :rtype: tuple - - """ - if _should_renew(config, cert): - return "renew", cert - if config.reinstall: - # Set with --reinstall, force an identical certificate to be - # reinstalled without further prompting. - return "reinstall", cert - question = ( - "You have an existing certificate that contains exactly the same " - "domains you requested and isn't close to expiry." - "{br}(ref: {0}){br}{br}What would you like to do?" - ).format(cert.configfile.filename, br=os.linesep) - - if config.verb == "run": - keep_opt = "Attempt to reinstall this existing certificate" - elif config.verb == "certonly": - keep_opt = "Keep the existing certificate for now" - choices = [keep_opt, - "Renew & replace the cert (limit ~5 per 7 days)"] - - display = zope.component.getUtility(interfaces.IDisplay) - response = display.menu(question, choices, "OK", "Cancel", default=0) - if response[0] == display_util.CANCEL: - # TODO: Add notification related to command-line options for - # skipping the menu for this case. - raise errors.Error( - "User chose to cancel the operation and may " - "reinvoke the client.") - elif response[1] == 0: - return "reinstall", cert - elif response[1] == 1: - return "renew", cert - else: - assert False, "This is impossible" - - -def _handle_subset_cert_request(config, domains, cert): - """Figure out what to do if a previous cert had a subset of the names now requested - - :param storage.RenewableCert cert: - - :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal - :rtype: tuple - - """ - existing = ", ".join(cert.names()) - question = ( - "You have an existing certificate that contains a portion of " - "the domains you requested (ref: {0}){br}{br}It contains these " - "names: {1}{br}{br}You requested these names for the new " - "certificate: {2}.{br}{br}Do you want to expand and replace this existing " - "certificate with the new certificate?" - ).format(cert.configfile.filename, - existing, - ", ".join(domains), - br=os.linesep) - if config.expand or config.renew_by_default or zope.component.getUtility( - interfaces.IDisplay).yesno(question, "Expand", "Cancel", - cli_flag="--expand (or in some cases, --duplicate)"): - return "renew", cert - else: - reporter_util = zope.component.getUtility(interfaces.IReporter) - reporter_util.add_message( - "To obtain a new certificate that contains these names without " - "replacing your existing certificate for {0}, you must use the " - "--duplicate option.{br}{br}" - "For example:{br}{br}{1} --duplicate {2}".format( - existing, - sys.argv[0], " ".join(sys.argv[1:]), - br=os.linesep - ), - reporter_util.HIGH_PRIORITY) - raise errors.Error( - "User chose to cancel the operation and may " - "reinvoke the client.") - - -def _report_new_cert(cert_path, fullchain_path): - """Reports the creation of a new certificate to the user. - - :param str cert_path: path to cert - :param str fullchain_path: path to full chain - - """ - expiry = crypto_util.notAfter(cert_path).date() - reporter_util = zope.component.getUtility(interfaces.IReporter) - if fullchain_path: - # Print the path to fullchain.pem because that's what modern webservers - # (Nginx and Apache2.4) will want. - and_chain = "and chain have" - path = fullchain_path - else: - # Unless we're in .csr mode and there really isn't one - and_chain = "has " - path = cert_path - # XXX Perhaps one day we could detect the presence of known old webservers - # and say something more informative here. - msg = ("Congratulations! Your certificate {0} been saved at {1}." - " Your cert will expire on {2}. To obtain a new version of the " - "certificate in the future, simply run Let's Encrypt again." - .format(and_chain, path, expiry)) - reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY) - - -def _suggest_donation_if_appropriate(config, action): - """Potentially suggest a donation to support Let's Encrypt.""" - if config.staging or config.verb == "renew": - # --dry-run implies --staging - return - if action not in ["renew", "newcert"]: - return - reporter_util = zope.component.getUtility(interfaces.IReporter) - msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" - "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" - "Donating to EFF: https://eff.org/donate-le\n\n") - reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) - - -def _report_successful_dry_run(config): - reporter_util = zope.component.getUtility(interfaces.IReporter) - if config.verb != "renew": - reporter_util.add_message("The dry run was successful.", - reporter_util.HIGH_PRIORITY, on_crash=False) - - -def _auth_from_domains(le_client, config, domains, lineage=None): - """Authenticate and enroll certificate.""" - # Note: This can raise errors... caught above us though. This is now - # a three-way case: reinstall (which results in a no-op here because - # although there is a relevant lineage, we don't do anything to it - # inside this function -- we don't obtain a new certificate), renew - # (which results in treating the request as a renewal), or newcert - # (which results in treating the request as a new certificate request). - - # If lineage is specified, use that one instead of looking around for - # a matching one. - if lineage is None: - # This will find a relevant matching lineage that exists - action, lineage = _treat_as_renewal(config, domains) - else: - # Renewal, where we already know the specific lineage we're - # interested in - action = "renew" - - if action == "reinstall": - # The lineage already exists; allow the caller to try installing - # it without getting a new certificate at all. - return lineage, "reinstall" - elif action == "renew": - original_server = lineage.configuration["renewalparams"]["server"] - _avoid_invalidating_lineage(config, lineage, original_server) - # TODO: schoen wishes to reuse key - discussion - # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 - new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) - # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) - if config.dry_run: - logger.info("Dry run: skipping updating lineage at %s", - os.path.dirname(lineage.cert)) - else: - lineage.save_successor( - lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), - new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain), - configuration.RenewerConfiguration(config.namespace)) - lineage.update_all_links_to(lineage.latest_common_version()) - # TODO: Check return value of save_successor - # TODO: Also update lineage renewal config with any relevant - # configuration values from this attempt? <- Absolutely (jdkasten) - elif action == "newcert": - # TREAT AS NEW REQUEST - lineage = le_client.obtain_and_enroll_certificate(domains) - if lineage is False: - raise errors.Error("Certificate could not be obtained") - - if not config.dry_run and not config.verb == "renew": - _report_new_cert(lineage.cert, lineage.fullchain) - - return lineage, action - - -def _avoid_invalidating_lineage(config, lineage, original_server): - "Do not renew a valid cert with one from a staging server!" - def _is_staging(srv): - return srv == constants.STAGING_URI or "staging" in srv - - # Some lineages may have begun with --staging, but then had production certs - # added to them - latest_cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, - open(lineage.cert).read()) - # all our test certs are from happy hacker fake CA, though maybe one day - # we should test more methodically - now_valid = "fake" not in repr(latest_cert.get_issuer()).lower() - - if _is_staging(config.server): - if not _is_staging(original_server) or now_valid: - if not config.break_my_certs: - names = ", ".join(lineage.names()) - raise errors.Error( - "You've asked to renew/replace a seemingly valid certificate with " - "a test certificate (domains: {0}). We will not do that " - "unless you use the --break-my-certs flag!".format(names)) - - def diagnose_configurator_problem(cfg_type, requested, plugins): """ Raise the most helpful error message about a plugin being unavailable @@ -645,103 +285,6 @@ def record_chosen_plugins(config, plugins, auth, inst): cn.installer = plugins.find_init(inst).name if inst else "none" -# TODO: Make run as close to auth + install as possible -# Possible difficulties: config.csr was hacked into auth -def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals - """Obtain a certificate and install.""" - try: - installer, authenticator = choose_configurator_plugins(config, plugins, "run") - except errors.PluginSelectionError as e: - return e.message - - domains = _find_domains(config, installer) - - # TODO: Handle errors from _init_le_client? - le_client = _init_le_client(config, authenticator, installer) - - lineage, action = _auth_from_domains(le_client, config, domains) - - le_client.deploy_certificate( - domains, lineage.privkey, lineage.cert, - lineage.chain, lineage.fullchain) - - le_client.enhance_config(domains, config) - - if len(lineage.available_versions("cert")) == 1: - display_ops.success_installation(domains) - else: - display_ops.success_renewal(domains, action) - - _suggest_donation_if_appropriate(config, action) - - -def obtain_cert(config, plugins, lineage=None): - """Implements "certonly": authenticate & obtain cert, but do not install it.""" - # pylint: disable=too-many-locals - try: - # installers are used in auth mode to determine domain names - installer, authenticator = choose_configurator_plugins(config, plugins, "certonly") - except errors.PluginSelectionError as e: - logger.info("Could not choose appropriate plugin: %s", e) - raise - - # TODO: Handle errors from _init_le_client? - le_client = _init_le_client(config, authenticator, installer) - - action = "newcert" - # This is a special case; cert and chain are simply saved - if config.csr is not None: - assert lineage is None, "Did not expect a CSR with a RenewableCert" - csr, typ = config.actual_csr - certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ) - if config.dry_run: - logger.info( - "Dry run: skipping saving certificate to %s", config.cert_path) - else: - cert_path, _, cert_fullchain = le_client.save_certificate( - certr, chain, config.cert_path, config.chain_path, config.fullchain_path) - _report_new_cert(cert_path, cert_fullchain) - else: - domains = _find_domains(config, installer) - _, action = _auth_from_domains(le_client, config, domains, lineage) - - if config.dry_run: - _report_successful_dry_run(config) - elif config.verb == "renew": - if installer is None: - # Tell the user that the server was not restarted. - print("new certificate deployed without reload, fullchain is", - lineage.fullchain) - else: - # In case of a renewal, reload server to pick up new certificate. - # In principle we could have a configuration option to inhibit this - # from happening. - installer.restart() - print("new certificate deployed with reload of", - config.installer, "server; fullchain is", lineage.fullchain) - _suggest_donation_if_appropriate(config, action) - - -def install(config, plugins): - """Install a previously obtained cert in a server.""" - # XXX: Update for renewer/RenewableCert - # FIXME: be consistent about whether errors are raised or returned from - # this function ... - - try: - installer, _ = choose_configurator_plugins(config, plugins, "install") - except errors.PluginSelectionError as e: - return e.message - - domains = _find_domains(config, installer) - le_client = _init_le_client(config, authenticator=None, installer=installer) - assert config.cert_path is not None # required=True in the subparser - le_client.deploy_certificate( - domains, config.key_path, config.cert_path, config.chain_path, - config.fullchain_path) - le_client.enhance_config(domains, config) - - def _set_by_cli(var): """ Return True if a particular config variable has been set by the user @@ -925,7 +468,7 @@ def _reconstitute(config, full_path): try: for d in renewal_candidate.names(): - _process_domain(config, d) + process_domain(config, d) except errors.ConfigurationError as error: logger.warning("Renewal configuration file %s references a cert " "that contains an invalid domain name. The problem " @@ -1012,9 +555,9 @@ def renew(config, unused_plugins): else: # XXX: ensure that each call here replaces the previous one zope.component.provideUtility(lineage_config) - if _should_renew(lineage_config, renewal_candidate): + if should_renew(lineage_config, renewal_candidate): plugins = plugins_disco.PluginsRegistry.find_all() - obtain_cert(lineage_config, plugins, renewal_candidate) + main.obtain_cert(lineage_config, plugins, renewal_candidate) renew_successes.append(renewal_candidate.fullchain) else: renew_skipped.append(renewal_candidate.fullchain) @@ -1036,63 +579,6 @@ def renew(config, unused_plugins): logger.debug("no renewal failures") -def revoke(config, unused_plugins): # TODO: coop with renewal config - """Revoke a previously obtained certificate.""" - # For user-agent construction - config.namespace.installer = config.namespace.authenticator = "none" - if config.key_path is not None: # revocation by cert key - logger.debug("Revoking %s using cert key %s", - config.cert_path[0], config.key_path[0]) - key = jose.JWK.load(config.key_path[1]) - else: # revocation by account key - logger.debug("Revoking %s using Account Key", config.cert_path[0]) - acc, _ = _determine_account(config) - key = acc.key - acme = client.acme_from_config_key(config, key) - cert = crypto_util.pyopenssl_load_certificate(config.cert_path[1])[0] - acme.revoke(jose.ComparableX509(cert)) - - -def rollback(config, plugins): - """Rollback server configuration changes made during install.""" - client.rollback(config.installer, config.checkpoints, config, plugins) - - -def config_changes(config, unused_plugins): - """Show changes made to server config during installation - - View checkpoints and associated configuration changes. - - """ - client.view_config_changes(config) - - -def plugins_cmd(config, plugins): # TODO: Use IDisplay rather than print - """List server software plugins.""" - logger.debug("Expected interfaces: %s", config.ifaces) - - ifaces = [] if config.ifaces is None else config.ifaces - filtered = plugins.visible().ifaces(ifaces) - logger.debug("Filtered plugins: %r", filtered) - - if not config.init and not config.prepare: - print(str(filtered)) - return - - filtered.init(config) - verified = filtered.verify(ifaces) - logger.debug("Verified plugins: %r", verified) - - if not config.prepare: - print(str(verified)) - return - - verified.prepare() - available = verified.available() - logger.debug("Prepared plugins: %s", available) - print(str(available)) - - def read_file(filename, mode="rb"): """Returns the given file's contents. @@ -1152,10 +638,10 @@ class HelpfulArgumentParser(object): """ # Maps verbs/subcommands to the functions that implement them - VERBS = {"auth": obtain_cert, "certonly": obtain_cert, - "config_changes": config_changes, "everything": run, - "install": install, "plugins": plugins_cmd, "renew": renew, - "revoke": revoke, "rollback": rollback, "run": run} + VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert, + "config_changes": main.config_changes, "everything": main.run, + "install": main.install, "plugins": main.plugins_cmd, "renew": renew, + "revoke": main.revoke, "rollback": main.rollback, "run": main.run} # List of topics for which additional help can be provided HELP_TOPICS = ["all", "security", @@ -1247,7 +733,7 @@ class HelpfulArgumentParser(object): def handle_csr(self, parsed_args): """ Process a --csr flag. This needs to happen early enough that the - webroot plugin can know about the calls to _process_domain + webroot plugin can know about the calls to process_domain """ if parsed_args.verb != "certonly": raise errors.Error("Currently, a CSR file may only be specified " @@ -1270,7 +756,7 @@ class HelpfulArgumentParser(object): logger.debug("PEM CSR parse error %s", traceback.format_exc()) raise errors.Error("Failed to parse CSR file: {0}".format(parsed_args.csr[0])) for d in domains: - _process_domain(parsed_args, d) + process_domain(parsed_args, d) for d in domains: sanitised = le_util.enforce_domain_sanity(d) @@ -1802,7 +1288,7 @@ class WebrootPathProcessor(argparse.Action): # pylint: disable=missing-docstrin args.webroot_path.append(webroot) -def _process_domain(args_or_config, domain_arg, webroot_path=None): +def process_domain(args_or_config, domain_arg, webroot_path=None): """ Process a new -d flag, helping the webroot plugin construct a map of {domain : webrootpath} if -w / --webroot-path is in use @@ -1828,165 +1314,17 @@ class WebrootMapProcessor(argparse.Action): # pylint: disable=missing-docstring def __call__(self, parser, args, webroot_map_arg, option_string=None): webroot_map = json.loads(webroot_map_arg) for domains, webroot_path in webroot_map.iteritems(): - _process_domain(args, domains, [webroot_path]) + process_domain(args, domains, [webroot_path]) class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring def __call__(self, parser, args, domain_arg, option_string=None): - """Just wrap _process_domain in argparseese.""" - _process_domain(args, domain_arg) + """Just wrap process_domain in argparseese.""" + process_domain(args, domain_arg) -def setup_log_file_handler(config, logfile, fmt): - """Setup file debug logging.""" - log_file_path = os.path.join(config.logs_dir, logfile) - handler = logging.handlers.RotatingFileHandler( - log_file_path, maxBytes=2 ** 20, backupCount=10) - # rotate on each invocation, rollover only possible when maxBytes - # is nonzero and backupCount is nonzero, so we set maxBytes as big - # as possible not to overrun in single CLI invocation (1MB). - handler.doRollover() # TODO: creates empty letsencrypt.log.1 file - handler.setLevel(logging.DEBUG) - handler_formatter = logging.Formatter(fmt=fmt) - handler_formatter.converter = time.gmtime # don't use localtime - handler.setFormatter(handler_formatter) - return handler, log_file_path - - -def _cli_log_handler(config, level, fmt): - if config.text_mode or config.noninteractive_mode or config.verb == "renew": - handler = colored_logging.StreamHandler() - handler.setFormatter(logging.Formatter(fmt)) - else: - handler = log.DialogHandler() - # dialog box is small, display as less as possible - handler.setFormatter(logging.Formatter("%(message)s")) - handler.setLevel(level) - return handler - - -def setup_logging(config, cli_handler_factory, logfile): - """Setup logging.""" - fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" - level = -config.verbose_count * 10 - file_handler, log_file_path = setup_log_file_handler( - config, logfile=logfile, fmt=fmt) - cli_handler = cli_handler_factory(config, level, fmt) - - # TODO: use fileConfig? - - root_logger = logging.getLogger() - root_logger.setLevel(logging.DEBUG) # send all records to handlers - root_logger.addHandler(cli_handler) - root_logger.addHandler(file_handler) - - logger.debug("Root logging level set at %d", level) - logger.info("Saving debug log to %s", log_file_path) - - -def _handle_exception(exc_type, exc_value, trace, config): - """Logs exceptions and reports them to the user. - - Config is used to determine how to display exceptions to the user. In - general, if config.debug is True, then the full exception and traceback is - shown to the user, otherwise it is suppressed. If config itself is None, - then the traceback and exception is attempted to be written to a logfile. - If this is successful, the traceback is suppressed, otherwise it is shown - to the user. sys.exit is always called with a nonzero status. - - """ - logger.debug( - "Exiting abnormally:%s%s", - os.linesep, - "".join(traceback.format_exception(exc_type, exc_value, trace))) - - if issubclass(exc_type, Exception) and (config is None or not config.debug): - if config is None: - logfile = "letsencrypt.log" - try: - with open(logfile, "w") as logfd: - traceback.print_exception( - exc_type, exc_value, trace, file=logfd) - except: # pylint: disable=bare-except - sys.exit("".join( - traceback.format_exception(exc_type, exc_value, trace))) - - if issubclass(exc_type, errors.Error): - sys.exit(exc_value) - else: - # Here we're passing a client or ACME error out to the client at the shell - # Tell the user a bit about what happened, without overwhelming - # them with a full traceback - err = traceback.format_exception_only(exc_type, exc_value)[0] - # Typical error from the ACME module: - # acme.messages.Error: urn:acme:error:malformed :: The request message was - # malformed :: Error creating new registration :: Validation of contact - # mailto:none@longrandomstring.biz failed: Server failure at resolver - if (("urn:acme" in err and ":: " in err and - config.verbose_count <= flag_default("verbose_count"))): - # prune ACME error code, we have a human description - _code, _sep, err = err.partition(":: ") - msg = "An unexpected error occurred:\n" + err + "Please see the " - if config is None: - msg += "logfile '{0}' for more details.".format(logfile) - else: - msg += "logfiles in {0} for more details.".format(config.logs_dir) - sys.exit(msg) - else: - sys.exit("".join( - traceback.format_exception(exc_type, exc_value, trace))) - - -def main(cli_args=sys.argv[1:]): - """Command line argument parsing and main script execution.""" - sys.excepthook = functools.partial(_handle_exception, config=None) - plugins = plugins_disco.PluginsRegistry.find_all() - - # note: arg parser internally handles --help (and exits afterwards) - args = prepare_and_parse_args(plugins, cli_args) - config = configuration.NamespaceConfig(args) - zope.component.provideUtility(config) - - # Setup logging ASAP, otherwise "No handlers could be found for - # logger ..." TODO: this should be done before plugins discovery - for directory in config.config_dir, config.work_dir: - le_util.make_or_verify_dir( - directory, constants.CONFIG_DIRS_MODE, os.geteuid(), - "--strict-permissions" in cli_args) - # TODO: logs might contain sensitive data such as contents of the - # private key! #525 - le_util.make_or_verify_dir( - config.logs_dir, 0o700, os.geteuid(), "--strict-permissions" in cli_args) - setup_logging(config, _cli_log_handler, logfile='letsencrypt.log') - - logger.debug("letsencrypt version: %s", letsencrypt.__version__) - # do not log `config`, as it contains sensitive data (e.g. revoke --key)! - logger.debug("Arguments: %r", cli_args) - logger.debug("Discovered plugins: %r", plugins) - - sys.excepthook = functools.partial(_handle_exception, config=config) - - # Displayer - if config.noninteractive_mode: - displayer = display_util.NoninteractiveDisplay(sys.stdout) - elif config.text_mode: - displayer = display_util.FileDisplay(sys.stdout) - elif config.verb == "renew": - config.noninteractive_mode = True - displayer = display_util.NoninteractiveDisplay(sys.stdout) - else: - displayer = display_util.NcursesDisplay() - zope.component.provideUtility(displayer) - - # Reporter - report = reporter.Reporter() - zope.component.provideUtility(report) - atexit.register(report.atexit_print_messages) - - return config.func(config, plugins) - if __name__ == "__main__": - err_string = main() + err_string = main.main() if err_string: logger.warn("Exiting with message %s", err_string) sys.exit(err_string) # pragma: no cover diff --git a/letsencrypt/main.py b/letsencrypt/main.py new file mode 100644 index 000000000..89e7a85a3 --- /dev/null +++ b/letsencrypt/main.py @@ -0,0 +1,696 @@ +from __future__ import print_function +import atexit +import functools +import os +import sys +import zope.component + +from letsencrypt import account +from letsencrypt import client +from letsencrypt import cli +from letsencrypt import crypto_util +from letsencrypt import colored_logging +from letsencrypt import configuration +from letsencrypt import constants +from letsencrypt import errors +from letsencrypt import interfaces +from letsencrypt import le_util +from letsencrypt import log +from letsencrypt import reporter +from letsencrypt import storage + +from letsencrypt.cli import choose_configurator_plugins, _renewal_conf_files, should_renew +from letsencrypt.display import util as display_util, ops as display_ops +from letsencrypt.plugins import disco as plugins_disco + +import traceback +import logging.handlers +import time +from acme import jose +import OpenSSL + +logger = logging.getLogger(__name__) + +def _suggest_donation_if_appropriate(config, action): + """Potentially suggest a donation to support Let's Encrypt.""" + if config.staging or config.verb == "renew": + # --dry-run implies --staging + return + if action not in ["renew", "newcert"]: + return + reporter_util = zope.component.getUtility(interfaces.IReporter) + msg = ("If you like Let's Encrypt, please consider supporting our work by:\n\n" + "Donating to ISRG / Let's Encrypt: https://letsencrypt.org/donate\n" + "Donating to EFF: https://eff.org/donate-le\n\n") + reporter_util.add_message(msg, reporter_util.LOW_PRIORITY) + + +def _avoid_invalidating_lineage(config, lineage, original_server): + "Do not renew a valid cert with one from a staging server!" + def _is_staging(srv): + return srv == constants.STAGING_URI or "staging" in srv + + # Some lineages may have begun with --staging, but then had production certs + # added to them + latest_cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, + open(lineage.cert).read()) + # all our test certs are from happy hacker fake CA, though maybe one day + # we should test more methodically + now_valid = "fake" not in repr(latest_cert.get_issuer()).lower() + + if _is_staging(config.server): + if not _is_staging(original_server) or now_valid: + if not config.break_my_certs: + names = ", ".join(lineage.names()) + raise errors.Error( + "You've asked to renew/replace a seemingly valid certificate with " + "a test certificate (domains: {0}). We will not do that " + "unless you use the --break-my-certs flag!".format(names)) + + +def _report_successful_dry_run(config): + reporter_util = zope.component.getUtility(interfaces.IReporter) + if config.verb != "renew": + reporter_util.add_message("The dry run was successful.", + reporter_util.HIGH_PRIORITY, on_crash=False) + + +def _auth_from_domains(le_client, config, domains, lineage=None): + """Authenticate and enroll certificate.""" + # Note: This can raise errors... caught above us though. This is now + # a three-way case: reinstall (which results in a no-op here because + # although there is a relevant lineage, we don't do anything to it + # inside this function -- we don't obtain a new certificate), renew + # (which results in treating the request as a renewal), or newcert + # (which results in treating the request as a new certificate request). + + # If lineage is specified, use that one instead of looking around for + # a matching one. + if lineage is None: + # This will find a relevant matching lineage that exists + action, lineage = _treat_as_renewal(config, domains) + else: + # Renewal, where we already know the specific lineage we're + # interested in + action = "renew" + + if action == "reinstall": + # The lineage already exists; allow the caller to try installing + # it without getting a new certificate at all. + return lineage, "reinstall" + elif action == "renew": + original_server = lineage.configuration["renewalparams"]["server"] + _avoid_invalidating_lineage(config, lineage, original_server) + # TODO: schoen wishes to reuse key - discussion + # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 + new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) + # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) + if config.dry_run: + logger.info("Dry run: skipping updating lineage at %s", + os.path.dirname(lineage.cert)) + else: + lineage.save_successor( + lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped), + new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain), + configuration.RenewerConfiguration(config.namespace)) + lineage.update_all_links_to(lineage.latest_common_version()) + # TODO: Check return value of save_successor + # TODO: Also update lineage renewal config with any relevant + # configuration values from this attempt? <- Absolutely (jdkasten) + elif action == "newcert": + # TREAT AS NEW REQUEST + lineage = le_client.obtain_and_enroll_certificate(domains) + if lineage is False: + raise errors.Error("Certificate could not be obtained") + + if not config.dry_run and not config.verb == "renew": + _report_new_cert(lineage.cert, lineage.fullchain) + + return lineage, action + + +def _handle_subset_cert_request(config, domains, cert): + """Figure out what to do if a previous cert had a subset of the names now requested + + :param storage.RenewableCert cert: + + :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + :rtype: tuple + + """ + existing = ", ".join(cert.names()) + question = ( + "You have an existing certificate that contains a portion of " + "the domains you requested (ref: {0}){br}{br}It contains these " + "names: {1}{br}{br}You requested these names for the new " + "certificate: {2}.{br}{br}Do you want to expand and replace this existing " + "certificate with the new certificate?" + ).format(cert.configfile.filename, + existing, + ", ".join(domains), + br=os.linesep) + if config.expand or config.renew_by_default or zope.component.getUtility( + interfaces.IDisplay).yesno(question, "Expand", "Cancel", + cli_flag="--expand (or in some cases, --duplicate)"): + return "renew", cert + else: + reporter_util = zope.component.getUtility(interfaces.IReporter) + reporter_util.add_message( + "To obtain a new certificate that contains these names without " + "replacing your existing certificate for {0}, you must use the " + "--duplicate option.{br}{br}" + "For example:{br}{br}{1} --duplicate {2}".format( + existing, + sys.argv[0], " ".join(sys.argv[1:]), + br=os.linesep + ), + reporter_util.HIGH_PRIORITY) + raise errors.Error( + "User chose to cancel the operation and may " + "reinvoke the client.") + + +def _handle_identical_cert_request(config, cert): + """Figure out what to do if a cert has the same names as a previously obtained one + + :param storage.RenewableCert cert: + + :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal + :rtype: tuple + + """ + if should_renew(config, cert): + return "renew", cert + if config.reinstall: + # Set with --reinstall, force an identical certificate to be + # reinstalled without further prompting. + return "reinstall", cert + question = ( + "You have an existing certificate that contains exactly the same " + "domains you requested and isn't close to expiry." + "{br}(ref: {0}){br}{br}What would you like to do?" + ).format(cert.configfile.filename, br=os.linesep) + + if config.verb == "run": + keep_opt = "Attempt to reinstall this existing certificate" + elif config.verb == "certonly": + keep_opt = "Keep the existing certificate for now" + choices = [keep_opt, + "Renew & replace the cert (limit ~5 per 7 days)"] + + display = zope.component.getUtility(interfaces.IDisplay) + response = display.menu(question, choices, "OK", "Cancel", default=0) + if response[0] == display_util.CANCEL: + # TODO: Add notification related to command-line options for + # skipping the menu for this case. + raise errors.Error( + "User chose to cancel the operation and may " + "reinvoke the client.") + elif response[1] == 0: + return "reinstall", cert + elif response[1] == 1: + return "renew", cert + else: + assert False, "This is impossible" + + +def _treat_as_renewal(config, domains): + """Determine whether there are duplicated names and how to handle + them (renew, reinstall, newcert, or raising an error to stop + the client run if the user chooses to cancel the operation when + prompted). + + :returns: Two-element tuple containing desired new-certificate behavior as + a string token ("reinstall", "renew", or "newcert"), plus either + a RenewableCert instance or None if renewal shouldn't occur. + + :raises .Error: If the user would like to rerun the client again. + + """ + # Considering the possibility that the requested certificate is + # related to an existing certificate. (config.duplicate, which + # is set with --duplicate, skips all of this logic and forces any + # kind of certificate to be obtained with renewal = False.) + if config.duplicate: + return "newcert", None + # TODO: Also address superset case + ident_names_cert, subset_names_cert = _find_duplicative_certs(config, domains) + # XXX ^ schoen is not sure whether that correctly reads the systemwide + # configuration file. + if ident_names_cert is None and subset_names_cert is None: + return "newcert", None + + if ident_names_cert is not None: + return _handle_identical_cert_request(config, ident_names_cert) + elif subset_names_cert is not None: + return _handle_subset_cert_request(config, domains, subset_names_cert) + + +def _find_duplicative_certs(config, domains): + """Find existing certs that duplicate the request.""" + + identical_names_cert, subset_names_cert = None, None + + cli_config = configuration.RenewerConfiguration(config) + configs_dir = cli_config.renewal_configs_dir + # Verify the directory is there + le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) + + for renewal_file in _renewal_conf_files(cli_config): + try: + candidate_lineage = storage.RenewableCert(renewal_file, cli_config) + except (errors.CertStorageError, IOError): + logger.warning("Renewal conf file %s is broken. Skipping.", renewal_file) + logger.debug("Traceback was:\n%s", traceback.format_exc()) + continue + # TODO: Handle these differently depending on whether they are + # expired or still valid? + candidate_names = set(candidate_lineage.names()) + if candidate_names == set(domains): + identical_names_cert = candidate_lineage + elif candidate_names.issubset(set(domains)): + # This logic finds and returns the largest subset-names cert + # in the case where there are several available. + if subset_names_cert is None: + subset_names_cert = candidate_lineage + elif len(candidate_names) > len(subset_names_cert.names()): + subset_names_cert = candidate_lineage + + return identical_names_cert, subset_names_cert + + +def _find_domains(config, installer): + if not config.domains: + domains = display_ops.choose_names(installer) + # record in config.domains (so that it can be serialised in renewal config files), + # and set webroot_map entries if applicable + for d in domains: + cli.process_domain(config, d) + else: + domains = config.domains + + if not domains: + raise errors.Error("Please specify --domains, or --installer that " + "will help in domain names autodiscovery") + + return domains + + +def _report_new_cert(cert_path, fullchain_path): + """Reports the creation of a new certificate to the user. + + :param str cert_path: path to cert + :param str fullchain_path: path to full chain + + """ + expiry = crypto_util.notAfter(cert_path).date() + reporter_util = zope.component.getUtility(interfaces.IReporter) + if fullchain_path: + # Print the path to fullchain.pem because that's what modern webservers + # (Nginx and Apache2.4) will want. + and_chain = "and chain have" + path = fullchain_path + else: + # Unless we're in .csr mode and there really isn't one + and_chain = "has " + path = cert_path + # XXX Perhaps one day we could detect the presence of known old webservers + # and say something more informative here. + msg = ("Congratulations! Your certificate {0} been saved at {1}." + " Your cert will expire on {2}. To obtain a new version of the " + "certificate in the future, simply run Let's Encrypt again." + .format(and_chain, path, expiry)) + reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY) + + +def _determine_account(config): + """Determine which account to use. + + In order to make the renewer (configuration de/serialization) happy, + if ``config.account`` is ``None``, it will be updated based on the + user input. Same for ``config.email``. + + :param argparse.Namespace config: CLI arguments + :param letsencrypt.interface.IConfig config: Configuration object + :param .AccountStorage account_storage: Account storage. + + :returns: Account and optionally ACME client API (biproduct of new + registration). + :rtype: `tuple` of `letsencrypt.account.Account` and + `acme.client.Client` + + """ + account_storage = account.AccountFileStorage(config) + acme = None + + if config.account is not None: + acc = account_storage.load(config.account) + else: + accounts = account_storage.find_all() + if len(accounts) > 1: + acc = display_ops.choose_account(accounts) + elif len(accounts) == 1: + acc = accounts[0] + else: # no account registered yet + if config.email is None and not config.register_unsafely_without_email: + config.namespace.email = display_ops.get_email() + + def _tos_cb(regr): + if config.tos: + return True + msg = ("Please read the Terms of Service at {0}. You " + "must agree in order to register with the ACME " + "server at {1}".format( + regr.terms_of_service, config.server)) + obj = zope.component.getUtility(interfaces.IDisplay) + return obj.yesno(msg, "Agree", "Cancel", cli_flag="--agree-tos") + + try: + acc, acme = client.register( + config, account_storage, tos_cb=_tos_cb) + except errors.MissingCommandlineFlag: + raise + except errors.Error as error: + logger.debug(error, exc_info=True) + raise errors.Error( + "Unable to register an account with ACME server") + + config.namespace.account = acc.id + return acc, acme + + +def _init_le_client(config, authenticator, installer): + if authenticator is not None: + # if authenticator was given, then we will need account... + acc, acme = _determine_account(config) + logger.debug("Picked account: %r", acc) + # XXX + #crypto_util.validate_key_csr(acc.key) + else: + acc, acme = None, None + + return client.Client(config, acc, authenticator, installer, acme=acme) + + +def install(config, plugins): + """Install a previously obtained cert in a server.""" + # XXX: Update for renewer/RenewableCert + # FIXME: be consistent about whether errors are raised or returned from + # this function ... + + try: + installer, _ = choose_configurator_plugins(config, plugins, "install") + except errors.PluginSelectionError as e: + return e.message + + domains = _find_domains(config, installer) + le_client = _init_le_client(config, authenticator=None, installer=installer) + assert config.cert_path is not None # required=True in the subparser + le_client.deploy_certificate( + domains, config.key_path, config.cert_path, config.chain_path, + config.fullchain_path) + le_client.enhance_config(domains, config) + + +def plugins_cmd(config, plugins): # TODO: Use IDisplay rather than print + """List server software plugins.""" + logger.debug("Expected interfaces: %s", config.ifaces) + + ifaces = [] if config.ifaces is None else config.ifaces + filtered = plugins.visible().ifaces(ifaces) + logger.debug("Filtered plugins: %r", filtered) + + if not config.init and not config.prepare: + print(str(filtered)) + return + + filtered.init(config) + verified = filtered.verify(ifaces) + logger.debug("Verified plugins: %r", verified) + + if not config.prepare: + print(str(verified)) + return + + verified.prepare() + available = verified.available() + logger.debug("Prepared plugins: %s", available) + print(str(available)) + + +def rollback(config, plugins): + """Rollback server configuration changes made during install.""" + client.rollback(config.installer, config.checkpoints, config, plugins) + + +def config_changes(config, unused_plugins): + """Show changes made to server config during installation + + View checkpoints and associated configuration changes. + + """ + client.view_config_changes(config) + + +def revoke(config, unused_plugins): # TODO: coop with renewal config + """Revoke a previously obtained certificate.""" + # For user-agent construction + config.namespace.installer = config.namespace.authenticator = "none" + if config.key_path is not None: # revocation by cert key + logger.debug("Revoking %s using cert key %s", + config.cert_path[0], config.key_path[0]) + key = jose.JWK.load(config.key_path[1]) + else: # revocation by account key + logger.debug("Revoking %s using Account Key", config.cert_path[0]) + acc, _ = _determine_account(config) + key = acc.key + acme = client.acme_from_config_key(config, key) + cert = crypto_util.pyopenssl_load_certificate(config.cert_path[1])[0] + acme.revoke(jose.ComparableX509(cert)) + + +def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals + """Obtain a certificate and install.""" + # TODO: Make run as close to auth + install as possible + # Possible difficulties: config.csr was hacked into auth + try: + installer, authenticator = choose_configurator_plugins(config, plugins, "run") + except errors.PluginSelectionError as e: + return e.message + + domains = _find_domains(config, installer) + + # TODO: Handle errors from _init_le_client? + le_client = _init_le_client(config, authenticator, installer) + + lineage, action = _auth_from_domains(le_client, config, domains) + + le_client.deploy_certificate( + domains, lineage.privkey, lineage.cert, + lineage.chain, lineage.fullchain) + + le_client.enhance_config(domains, config) + + if len(lineage.available_versions("cert")) == 1: + display_ops.success_installation(domains) + else: + display_ops.success_renewal(domains, action) + + _suggest_donation_if_appropriate(config, action) + + +def obtain_cert(config, plugins, lineage=None): + """Implements "certonly": authenticate & obtain cert, but do not install it.""" + # pylint: disable=too-many-locals + try: + # installers are used in auth mode to determine domain names + installer, authenticator = choose_configurator_plugins(config, plugins, "certonly") + except errors.PluginSelectionError as e: + logger.info("Could not choose appropriate plugin: %s", e) + raise + + # TODO: Handle errors from _init_le_client? + le_client = _init_le_client(config, authenticator, installer) + + action = "newcert" + # This is a special case; cert and chain are simply saved + if config.csr is not None: + assert lineage is None, "Did not expect a CSR with a RenewableCert" + csr, typ = config.actual_csr + certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ) + if config.dry_run: + logger.info( + "Dry run: skipping saving certificate to %s", config.cert_path) + else: + cert_path, _, cert_fullchain = le_client.save_certificate( + certr, chain, config.cert_path, config.chain_path, config.fullchain_path) + _report_new_cert(cert_path, cert_fullchain) + else: + domains = _find_domains(config, installer) + _, action = _auth_from_domains(le_client, config, domains, lineage) + + if config.dry_run: + _report_successful_dry_run(config) + elif config.verb == "renew": + if installer is None: + # Tell the user that the server was not restarted. + print("new certificate deployed without reload, fullchain is", + lineage.fullchain) + else: + # In case of a renewal, reload server to pick up new certificate. + # In principle we could have a configuration option to inhibit this + # from happening. + installer.restart() + print("new certificate deployed with reload of", + config.installer, "server; fullchain is", lineage.fullchain) + _suggest_donation_if_appropriate(config, action) + + +def setup_log_file_handler(config, logfile, fmt): + """Setup file debug logging.""" + log_file_path = os.path.join(config.logs_dir, logfile) + handler = logging.handlers.RotatingFileHandler( + log_file_path, maxBytes=2 ** 20, backupCount=10) + # rotate on each invocation, rollover only possible when maxBytes + # is nonzero and backupCount is nonzero, so we set maxBytes as big + # as possible not to overrun in single CLI invocation (1MB). + handler.doRollover() # TODO: creates empty letsencrypt.log.1 file + handler.setLevel(logging.DEBUG) + handler_formatter = logging.Formatter(fmt=fmt) + handler_formatter.converter = time.gmtime # don't use localtime + handler.setFormatter(handler_formatter) + return handler, log_file_path + + +def _cli_log_handler(config, level, fmt): + if config.text_mode or config.noninteractive_mode or config.verb == "renew": + handler = colored_logging.StreamHandler() + handler.setFormatter(logging.Formatter(fmt)) + else: + handler = log.DialogHandler() + # dialog box is small, display as less as possible + handler.setFormatter(logging.Formatter("%(message)s")) + handler.setLevel(level) + return handler + + +def setup_logging(config, cli_handler_factory, logfile): + """Setup logging.""" + fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" + level = -config.verbose_count * 10 + file_handler, log_file_path = setup_log_file_handler( + config, logfile=logfile, fmt=fmt) + cli_handler = cli_handler_factory(config, level, fmt) + + # TODO: use fileConfig? + + root_logger = logging.getLogger() + root_logger.setLevel(logging.DEBUG) # send all records to handlers + root_logger.addHandler(cli_handler) + root_logger.addHandler(file_handler) + + logger.debug("Root logging level set at %d", level) + logger.info("Saving debug log to %s", log_file_path) + + +def _handle_exception(exc_type, exc_value, trace, config): + """Logs exceptions and reports them to the user. + + Config is used to determine how to display exceptions to the user. In + general, if config.debug is True, then the full exception and traceback is + shown to the user, otherwise it is suppressed. If config itself is None, + then the traceback and exception is attempted to be written to a logfile. + If this is successful, the traceback is suppressed, otherwise it is shown + to the user. sys.exit is always called with a nonzero status. + + """ + logger.debug( + "Exiting abnormally:%s%s", + os.linesep, + "".join(traceback.format_exception(exc_type, exc_value, trace))) + + if issubclass(exc_type, Exception) and (config is None or not config.debug): + if config is None: + logfile = "letsencrypt.log" + try: + with open(logfile, "w") as logfd: + traceback.print_exception( + exc_type, exc_value, trace, file=logfd) + except: # pylint: disable=bare-except + sys.exit("".join( + traceback.format_exception(exc_type, exc_value, trace))) + + if issubclass(exc_type, errors.Error): + sys.exit(exc_value) + else: + # Here we're passing a client or ACME error out to the client at the shell + # Tell the user a bit about what happened, without overwhelming + # them with a full traceback + err = traceback.format_exception_only(exc_type, exc_value)[0] + # Typical error from the ACME module: + # acme.messages.Error: urn:acme:error:malformed :: The request message was + # malformed :: Error creating new registration :: Validation of contact + # mailto:none@longrandomstring.biz failed: Server failure at resolver + if (("urn:acme" in err and ":: " in err and + config.verbose_count <= cli.flag_default("verbose_count"))): + # prune ACME error code, we have a human description + _code, _sep, err = err.partition(":: ") + msg = "An unexpected error occurred:\n" + err + "Please see the " + if config is None: + msg += "logfile '{0}' for more details.".format(logfile) + else: + msg += "logfiles in {0} for more details.".format(config.logs_dir) + sys.exit(msg) + else: + sys.exit("".join( + traceback.format_exception(exc_type, exc_value, trace))) + + +def main(cli_args=sys.argv[1:]): + """Command line argument parsing and main script execution.""" + sys.excepthook = functools.partial(_handle_exception, config=None) + plugins = plugins_disco.PluginsRegistry.find_all() + + # note: arg parser internally handles --help (and exits afterwards) + args = cli.prepare_and_parse_args(plugins, cli_args) + config = configuration.NamespaceConfig(args) + zope.component.provideUtility(config) + + # Setup logging ASAP, otherwise "No handlers could be found for + # logger ..." TODO: this should be done before plugins discovery + for directory in config.config_dir, config.work_dir: + le_util.make_or_verify_dir( + directory, constants.CONFIG_DIRS_MODE, os.geteuid(), + "--strict-permissions" in cli_args) + # TODO: logs might contain sensitive data such as contents of the + # private key! #525 + le_util.make_or_verify_dir( + config.logs_dir, 0o700, os.geteuid(), "--strict-permissions" in cli_args) + setup_logging(config, _cli_log_handler, logfile='letsencrypt.log') + + logger.debug("letsencrypt version: %s", letsencrypt.__version__) + # do not log `config`, as it contains sensitive data (e.g. revoke --key)! + logger.debug("Arguments: %r", cli_args) + logger.debug("Discovered plugins: %r", plugins) + + sys.excepthook = functools.partial(_handle_exception, config=config) + + # Displayer + if config.noninteractive_mode: + displayer = display_util.NoninteractiveDisplay(sys.stdout) + elif config.text_mode: + displayer = display_util.FileDisplay(sys.stdout) + elif config.verb == "renew": + config.noninteractive_mode = True + displayer = display_util.NoninteractiveDisplay(sys.stdout) + else: + displayer = display_util.NcursesDisplay() + zope.component.provideUtility(displayer) + + # Reporter + report = reporter.Reporter() + zope.component.provideUtility(report) + atexit.register(report.atexit_print_messages) + + return config.func(config, plugins) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index aef3447c3..f17f0fb73 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -13,7 +13,7 @@ import mock from acme import jose -from letsencrypt import account +from letsencrypt import account, main from letsencrypt import cli from letsencrypt import configuration from letsencrypt import constants @@ -60,7 +60,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stdout') as stdout: with mock.patch('letsencrypt.cli.sys.stderr') as stderr: - ret = cli.main(args[:]) # NOTE: parser can alter its args! + ret = main.main(args[:]) # NOTE: parser can alter its args! return ret, stdout, stderr def _call_stdout(self, args): @@ -71,7 +71,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stderr') as stderr: with mock.patch('letsencrypt.cli.client') as client: - ret = cli.main(args[:]) # NOTE: parser can alter its args! + ret = main.main(args[:]) # NOTE: parser can alter its args! return ret, None, stderr, client def test_no_flags(self): @@ -136,7 +136,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods exc = None try: with mock.patch('letsencrypt.cli.sys.stderr'): - cli.main(self.standard_args + args[:]) # NOTE: parser can alter its args! + main.main(self.standard_args + args[:]) # NOTE: parser can alter its args! except errors.MissingCommandlineFlag as exc: self.assertTrue(message in str(exc)) self.assertTrue(exc is not None) @@ -459,7 +459,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods if domains_arg: webroot_map_args.extend(["-d", domains_arg]) namespace = parse(webroot_map_args) - domains = cli._find_domains(namespace, mock.MagicMock()) # pylint: disable=protected-access + domains = main._find_domains(namespace, mock.MagicMock()) # pylint: disable=protected-access self.assertEqual(namespace.webroot_map, expected_map) self.assertEqual(set(domains), set(expectect_domains)) @@ -835,7 +835,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.cli.open', mock_open, create=True): exception = Exception('detail') config.verbose_count = 1 - cli._handle_exception( + main._handle_exception( Exception, exc_value=exception, trace=None, config=None) mock_open().write.assert_called_once_with(''.join( traceback.format_exception_only(Exception, exception))) @@ -845,7 +845,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.cli.open', mock_open, create=True): mock_open.side_effect = [KeyboardInterrupt] error = errors.Error('detail') - cli._handle_exception( + main._handle_exception( errors.Error, exc_value=error, trace=None, config=None) # assert_any_call used because sys.exit doesn't exit in cli.py mock_sys.exit.assert_any_call(''.join( @@ -854,7 +854,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods exception = messages.Error(detail='alpha', typ='urn:acme:error:triffid', title='beta') config = mock.MagicMock(debug=False, verbose_count=-3) - cli._handle_exception( + main._handle_exception( messages.Error, exc_value=exception, trace=None, config=config) error_msg = mock_sys.exit.call_args_list[-1][0][0] self.assertTrue('unexpected error' in error_msg) @@ -862,7 +862,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue('alpha' in error_msg) self.assertTrue('beta' in error_msg) config = mock.MagicMock(debug=False, verbose_count=1) - cli._handle_exception( + main._handle_exception( messages.Error, exc_value=exception, trace=None, config=config) error_msg = mock_sys.exit.call_args_list[-1][0][0] self.assertTrue('unexpected error' in error_msg) @@ -870,7 +870,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue('alpha' in error_msg) interrupt = KeyboardInterrupt('detail') - cli._handle_exception( + main._handle_exception( KeyboardInterrupt, exc_value=interrupt, trace=None, config=None) mock_sys.exit.assert_called_with(''.join( traceback.format_exception_only(KeyboardInterrupt, interrupt))) @@ -909,7 +909,7 @@ class DetermineAccountTest(unittest.TestCase): from letsencrypt.cli import _determine_account with mock.patch('letsencrypt.cli.account.AccountFileStorage') as mock_storage: mock_storage.return_value = self.account_storage - return _determine_account(self.config) + return main._determine_account(self.config) def test_args_account_set(self): self.account_storage.save(self.accs[1]) @@ -977,24 +977,24 @@ class DuplicativeCertsTest(storage_test.BaseRenewableCertTest): f.write(test_cert) # No overlap at all - result = _find_duplicative_certs( + result = main._find_duplicative_certs( self.cli_config, ['wow.net', 'hooray.org']) self.assertEqual(result, (None, None)) # Totally identical - result = _find_duplicative_certs( + result = main._find_duplicative_certs( self.cli_config, ['example.com', 'www.example.com']) self.assertTrue(result[0].configfile.filename.endswith('example.org.conf')) self.assertEqual(result[1], None) # Superset - result = _find_duplicative_certs( + result = main._find_duplicative_certs( self.cli_config, ['example.com', 'www.example.com', 'something.new']) self.assertEqual(result[0], None) self.assertTrue(result[1].configfile.filename.endswith('example.org.conf')) # Partial overlap doesn't count - result = _find_duplicative_certs( + result = main._find_duplicative_certs( self.cli_config, ['example.com', 'something.new']) self.assertEqual(result, (None, None)) From 001c1cd8354e6cc8da7cf7cbc205e1021d115e91 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sun, 28 Feb 2016 23:49:11 -0800 Subject: [PATCH 10/56] Refactor cli -> main With some help from rope... --- letsencrypt/cli.py | 28 +++------------------------- letsencrypt/main.py | 9 +++++++++ letsencrypt/tests/cli_test.py | 17 +++++++++-------- 3 files changed, 21 insertions(+), 33 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index d308db72e..df56d74ae 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,16 +1,13 @@ -"""Let's Encrypt CLI.""" +"""Let's Encrypt command CLI argument processing.""" from __future__ import print_function import argparse -import atexit import copy -import functools import glob import json import logging import logging.handlers import os import sys -import time import traceback import configargparse @@ -19,32 +16,20 @@ import zope.component import zope.interface.exceptions import zope.interface.verify -from letsencrypt import account -from letsencrypt import colored_logging +import letsencrypt + 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 -from letsencrypt import log from letsencrypt import main -from letsencrypt import reporter from letsencrypt import storage -from letsencrypt.display import util as display_util from letsencrypt.display import ops as display_ops from letsencrypt.plugins import disco as plugins_disco -# TODO: Sanity check all input. Be sure to avoid shell code etc... -# pylint: disable=too-many-lines -# (TODO: split this file into main.py and cli.py) - - - - - logger = logging.getLogger(__name__) @@ -1321,10 +1306,3 @@ class DomainFlagProcessor(argparse.Action): # pylint: disable=missing-docstring def __call__(self, parser, args, domain_arg, option_string=None): """Just wrap process_domain in argparseese.""" process_domain(args, domain_arg) - - -if __name__ == "__main__": - err_string = main.main() - if err_string: - logger.warn("Exiting with message %s", err_string) - sys.exit(err_string) # pragma: no cover diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 89e7a85a3..516cdf843 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -1,3 +1,4 @@ +"""Let's Encrypt main entry point.""" from __future__ import print_function import atexit import functools @@ -5,6 +6,8 @@ import os import sys import zope.component +import letsencrypt + from letsencrypt import account from letsencrypt import client from letsencrypt import cli @@ -694,3 +697,9 @@ def main(cli_args=sys.argv[1:]): atexit.register(report.atexit_print_messages) return config.func(config, plugins) + +if __name__ == "__main__": + err_string = main() + if err_string: + logger.warn("Exiting with message %s", err_string) + sys.exit(err_string) # pragma: no cover diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index f17f0fb73..c3fd91c11 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -13,13 +13,14 @@ import mock from acme import jose -from letsencrypt import account, main +from letsencrypt import account from letsencrypt import cli from letsencrypt import configuration from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util +from letsencrypt import main from letsencrypt import storage from letsencrypt.plugins import disco @@ -906,10 +907,10 @@ class DetermineAccountTest(unittest.TestCase): def _call(self): # pylint: disable=protected-access - from letsencrypt.cli import _determine_account + from letsencrypt.main import _determine_account with mock.patch('letsencrypt.cli.account.AccountFileStorage') as mock_storage: mock_storage.return_value = self.account_storage - return main._determine_account(self.config) + return _determine_account(self.config) def test_args_account_set(self): self.account_storage.save(self.accs[1]) @@ -971,30 +972,30 @@ class DuplicativeCertsTest(storage_test.BaseRenewableCertTest): @mock.patch('letsencrypt.le_util.make_or_verify_dir') def test_find_duplicative_names(self, unused_makedir): - from letsencrypt.cli import _find_duplicative_certs + from letsencrypt.main import _find_duplicative_certs test_cert = test_util.load_vector('cert-san.pem') with open(self.test_rc.cert, 'w') as f: f.write(test_cert) # No overlap at all - result = main._find_duplicative_certs( + result = _find_duplicative_certs( self.cli_config, ['wow.net', 'hooray.org']) self.assertEqual(result, (None, None)) # Totally identical - result = main._find_duplicative_certs( + result = _find_duplicative_certs( self.cli_config, ['example.com', 'www.example.com']) self.assertTrue(result[0].configfile.filename.endswith('example.org.conf')) self.assertEqual(result[1], None) # Superset - result = main._find_duplicative_certs( + result = _find_duplicative_certs( self.cli_config, ['example.com', 'www.example.com', 'something.new']) self.assertEqual(result[0], None) self.assertTrue(result[1].configfile.filename.endswith('example.org.conf')) # Partial overlap doesn't count - result = main._find_duplicative_certs( + result = _find_duplicative_certs( self.cli_config, ['example.com', 'something.new']) self.assertEqual(result, (None, None)) From 1ae8d344b09938be41b6599b2b0ec69a0f0ff17f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Mar 2016 17:53:57 -0800 Subject: [PATCH 11/56] Endure incredible amounts of mockery to ensure that tests pass --- letsencrypt/cli.py | 22 ++++---- letsencrypt/main.py | 11 ++++ letsencrypt/tests/cli_test.py | 96 +++++++++++++++++------------------ 3 files changed, 68 insertions(+), 61 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index df56d74ae..5ed97d03f 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -24,7 +24,6 @@ from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util -from letsencrypt import main from letsencrypt import storage from letsencrypt.display import ops as display_ops @@ -542,6 +541,7 @@ def renew(config, unused_plugins): zope.component.provideUtility(lineage_config) if should_renew(lineage_config, renewal_candidate): plugins = plugins_disco.PluginsRegistry.find_all() + from letsencrypt import main main.obtain_cert(lineage_config, plugins, renewal_candidate) renew_successes.append(renewal_candidate.fullchain) else: @@ -612,7 +612,6 @@ class SilentParser(object): # pylint: disable=too-few-public-methods kwargs["help"] = argparse.SUPPRESS self.parser.add_argument(*args, **kwargs) - class HelpfulArgumentParser(object): """Argparse Wrapper. @@ -622,19 +621,16 @@ class HelpfulArgumentParser(object): """ - # Maps verbs/subcommands to the functions that implement them - VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert, - "config_changes": main.config_changes, "everything": main.run, - "install": main.install, "plugins": main.plugins_cmd, "renew": renew, - "revoke": main.revoke, "rollback": main.rollback, "run": main.run} - - # List of topics for which additional help can be provided - HELP_TOPICS = ["all", "security", - "paths", "automation", "testing"] + VERBS.keys() - def __init__(self, args, plugins, detect_defaults=False): + from letsencrypt import main + self.VERBS = main.VERBS + + # List of topics for which additional help can be provided + HELP_TOPICS = ["all", "security", + "paths", "automation", "testing"] + main.VERBS.keys() + plugin_names = [name for name, _p in plugins.iteritems()] - self.help_topics = self.HELP_TOPICS + plugin_names + [None] + self.help_topics = HELP_TOPICS + plugin_names + [None] usage, short_usage = usage_strings(plugins) self.parser = configargparse.ArgParser( usage=short_usage, diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 516cdf843..e8b3f7345 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -34,6 +34,7 @@ import OpenSSL logger = logging.getLogger(__name__) + def _suggest_donation_if_appropriate(config, action): """Potentially suggest a donation to support Let's Encrypt.""" if config.staging or config.verb == "renew": @@ -698,6 +699,16 @@ def main(cli_args=sys.argv[1:]): return config.func(config, plugins) + +# Maps verbs/subcommands to the functions that implement them +# In principle this should live in cli.HelpfulArgumentParser, but +# due to issues with import cycles and testing, it lives here +VERBS = {"auth": obtain_cert, "certonly": obtain_cert, + "config_changes": config_changes, "everything": run, + "install": install, "plugins": plugins_cmd, "renew": cli.renew, + "revoke": revoke, "rollback": rollback, "run": run} + + if __name__ == "__main__": err_string = main() if err_string: diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index c3fd91c11..282a0b1d3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -52,15 +52,15 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _call(self, args): "Run the cli with output streams and actual client mocked out" - with mock.patch('letsencrypt.cli.client') as client: + with mock.patch('letsencrypt.main.client') as client: ret, stdout, stderr = self._call_no_clientmock(args) return ret, stdout, stderr, client def _call_no_clientmock(self, args): "Run the client with output streams mocked out" args = self.standard_args + args - with mock.patch('letsencrypt.cli.sys.stdout') as stdout: - with mock.patch('letsencrypt.cli.sys.stderr') as stderr: + with mock.patch('letsencrypt.main.sys.stdout') as stdout: + with mock.patch('letsencrypt.main.sys.stderr') as stderr: ret = main.main(args[:]) # NOTE: parser can alter its args! return ret, stdout, stderr @@ -70,8 +70,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods caller. """ args = self.standard_args + args - with mock.patch('letsencrypt.cli.sys.stderr') as stderr: - with mock.patch('letsencrypt.cli.client') as client: + with mock.patch('letsencrypt.main.sys.stderr') as stderr: + with mock.patch('letsencrypt.main.client') as client: ret = main.main(args[:]) # NOTE: parser can alter its args! return ret, None, stderr, client @@ -83,7 +83,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def _help_output(self, args): "Run a command, and return the ouput string for scrutiny" output = StringIO.StringIO() - with mock.patch('letsencrypt.cli.sys.stdout', new=output): + with mock.patch('letsencrypt.main.sys.stdout', new=output): self.assertRaises(SystemExit, self._call_stdout, args) out = output.getvalue() return out @@ -136,7 +136,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods "Ensure that a particular error raises a missing cli flag error containing message" exc = None try: - with mock.patch('letsencrypt.cli.sys.stderr'): + with mock.patch('letsencrypt.main.sys.stderr'): main.main(self.standard_args + args[:]) # NOTE: parser can alter its args! except errors.MissingCommandlineFlag as exc: self.assertTrue(message in str(exc)) @@ -147,15 +147,15 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._cli_missing_flag(args, "specify a plugin") args.extend(['--standalone', '-d', 'eg.is']) self._cli_missing_flag(args, "register before running") - with mock.patch('letsencrypt.cli._auth_from_domains'): - with mock.patch('letsencrypt.cli.client.acme_from_config_key'): + with mock.patch('letsencrypt.main._auth_from_domains'): + with mock.patch('letsencrypt.main.client.acme_from_config_key'): args.extend(['--email', 'io@io.is']) self._cli_missing_flag(args, "--agree-tos") - @mock.patch('letsencrypt.cli.client.acme_client.Client') - @mock.patch('letsencrypt.cli._determine_account') - @mock.patch('letsencrypt.cli.client.Client.obtain_and_enroll_certificate') - @mock.patch('letsencrypt.cli._auth_from_domains') + @mock.patch('letsencrypt.main.client.acme_client.Client') + @mock.patch('letsencrypt.main._determine_account') + @mock.patch('letsencrypt.main.client.Client.obtain_and_enroll_certificate') + @mock.patch('letsencrypt.main._auth_from_domains') def test_user_agent(self, afd, _obt, det, _client): # Normally the client is totally mocked out, but here we need more # arguments to automate it... @@ -164,7 +164,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods det.return_value = mock.MagicMock(), None afd.return_value = mock.MagicMock(), "newcert" - with mock.patch('letsencrypt.cli.client.acme_client.ClientNetwork') as acme_net: + with mock.patch('letsencrypt.main.client.acme_client.ClientNetwork') as acme_net: self._call_no_clientmock(args) os_ver = " ".join(le_util.get_os_info()) ua = acme_net.call_args[1]["user_agent"] @@ -174,7 +174,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods if "linux" in plat.lower(): self.assertTrue(platform.linux_distribution()[0] in ua) - with mock.patch('letsencrypt.cli.client.acme_client.ClientNetwork') as acme_net: + with mock.patch('letsencrypt.main.client.acme_client.ClientNetwork') as acme_net: ua = "bandersnatch" args += ["--user-agent", ua] self._call_no_clientmock(args) @@ -197,8 +197,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(args.chain_path, os.path.abspath(chain)) self.assertEqual(args.fullchain_path, os.path.abspath(fullchain)) - @mock.patch('letsencrypt.cli.record_chosen_plugins') - @mock.patch('letsencrypt.cli.display_ops') + @mock.patch('letsencrypt.main.cli.record_chosen_plugins') + @mock.patch('letsencrypt.main.cli.display_ops') def test_installer_selection(self, mock_display_ops, _rec): self._call(['install', '--domains', 'foo.bar', '--cert-path', 'cert', '--key-path', 'key', '--chain-path', 'chain']) @@ -237,8 +237,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._cli_missing_flag(["--standalone"], "With the standalone plugin, you probably") - with mock.patch("letsencrypt.cli._init_le_client") as mock_init: - with mock.patch("letsencrypt.cli._auth_from_domains") as mock_afd: + with mock.patch("letsencrypt.main._init_le_client") as mock_init: + with mock.patch("letsencrypt.main._auth_from_domains") as mock_afd: mock_afd.return_value = (mock.MagicMock(), mock.MagicMock()) self._call(["certonly", "--manual", "-d", "foo.bar"]) unused_config, auth, unused_installer = mock_init.call_args[0] @@ -267,8 +267,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods for r in xrange(len(flags)))): self._call(['plugins'] + list(args)) - @mock.patch('letsencrypt.cli.plugins_disco') - @mock.patch('letsencrypt.cli.HelpfulArgumentParser.determine_help_topics') + @mock.patch('letsencrypt.main.plugins_disco') + @mock.patch('letsencrypt.main.cli.HelpfulArgumentParser.determine_help_topics') def test_plugins_no_args(self, _det, mock_disco): ifaces = [] plugins = mock_disco.PluginsRegistry.find_all() @@ -279,8 +279,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods filtered = plugins.visible().ifaces() stdout.write.called_once_with(str(filtered)) - @mock.patch('letsencrypt.cli.plugins_disco') - @mock.patch('letsencrypt.cli.HelpfulArgumentParser.determine_help_topics') + @mock.patch('letsencrypt.main.plugins_disco') + @mock.patch('letsencrypt.main.cli.HelpfulArgumentParser.determine_help_topics') def test_plugins_init(self, _det, mock_disco): ifaces = [] plugins = mock_disco.PluginsRegistry.find_all() @@ -294,8 +294,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods verified = filtered.verify() stdout.write.called_once_with(str(verified)) - @mock.patch('letsencrypt.cli.plugins_disco') - @mock.patch('letsencrypt.cli.HelpfulArgumentParser.determine_help_topics') + @mock.patch('letsencrypt.main.plugins_disco') + @mock.patch('letsencrypt.main.cli.HelpfulArgumentParser.determine_help_topics') def test_plugins_prepare(self, _det, mock_disco): ifaces = [] plugins = mock_disco.PluginsRegistry.find_all() @@ -504,9 +504,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods {"eg.com": "/tmp", "www.eg.com": "/tmp", "eg.is": "/tmp2"}) def _certonly_new_request_common(self, mock_client, args=None): - with mock.patch('letsencrypt.cli._treat_as_renewal') as mock_renewal: + with mock.patch('letsencrypt.main._treat_as_renewal') as mock_renewal: mock_renewal.return_value = ("newcert", None) - with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + with mock.patch('letsencrypt.main._init_le_client') as mock_init: mock_init.return_value = mock_client if args is None: args = [] @@ -563,17 +563,17 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_client.obtain_certificate.return_value = (mock_certr, 'chain', mock_key, 'csr') try: - with mock.patch('letsencrypt.cli._find_duplicative_certs') as mock_fdc: + with mock.patch('letsencrypt.main._find_duplicative_certs') as mock_fdc: mock_fdc.return_value = (mock_lineage, None) - with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + with mock.patch('letsencrypt.main._init_le_client') as mock_init: mock_init.return_value = mock_client - get_utility_path = 'letsencrypt.cli.zope.component.getUtility' + get_utility_path = 'letsencrypt.main.zope.component.getUtility' with mock.patch(get_utility_path) as mock_get_utility: - with mock.patch('letsencrypt.cli.OpenSSL') as mock_ssl: + with mock.patch('letsencrypt.main.OpenSSL') as mock_ssl: mock_latest = mock.MagicMock() mock_latest.get_issuer.return_value = "Fake fake" mock_ssl.crypto.load_certificate.return_value = mock_latest - with mock.patch('letsencrypt.cli.crypto_util'): + with mock.patch('letsencrypt.main.crypto_util'): if not args: args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly'] if extra_args: @@ -689,7 +689,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods if names is not None: mock_lineage.names.return_value = names mock_rc.return_value = mock_lineage - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, error_expected=error_expected, args=['renew'], renew=False) if assert_oc_called is not None: @@ -738,7 +738,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_rc.return_value = mock_lineage mock_lineage.configuration = { 'renewalparams': {'authenticator': 'webroot'}} - with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert: mock_obtain_cert.side_effect = Exception self._test_renewal_common(True, None, error_expected=True, args=['renew'], renew=False) @@ -750,8 +750,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods renew=False, error_expected=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') - @mock.patch('letsencrypt.cli._treat_as_renewal') - @mock.patch('letsencrypt.cli._init_le_client') + @mock.patch('letsencrypt.main._treat_as_renewal') + @mock.patch('letsencrypt.main._init_le_client') def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility): mock_renewal.return_value = ('reinstall', mock.MagicMock()) mock_init.return_value = mock_client = mock.MagicMock() @@ -768,9 +768,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_client.obtain_certificate_from_csr.return_value = (certr, chain) cert_path = '/etc/letsencrypt/live/example.com/cert.pem' mock_client.save_certificate.return_value = cert_path, None, None - with mock.patch('letsencrypt.cli._init_le_client') as mock_init: + with mock.patch('letsencrypt.main._init_le_client') as mock_init: mock_init.return_value = mock_client - get_utility_path = 'letsencrypt.cli.zope.component.getUtility' + get_utility_path = 'letsencrypt.main.zope.component.getUtility' with mock.patch(get_utility_path) as mock_get_utility: chain_path = '/etc/letsencrypt/live/example.com/chain.pem' full_path = '/etc/letsencrypt/live/example.com/fullchain.pem' @@ -779,7 +779,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods CSR, cert_path, chain_path, full_path).split() if extra_args: args += extra_args - with mock.patch('letsencrypt.cli.crypto_util'): + with mock.patch('letsencrypt.main.crypto_util'): self._call(args) if '--dry-run' in args: @@ -803,7 +803,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( 'dry run' in mock_get_utility().add_message.call_args[0][0]) - @mock.patch('letsencrypt.cli.client.acme_client') + @mock.patch('letsencrypt.main.client.acme_client') def test_revoke_with_key(self, mock_acme_client): server = 'foo.bar' self._call_no_clientmock(['--cert-path', CERT, '--key-path', KEY, @@ -816,7 +816,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_revoke = mock_acme_client.Client().revoke mock_revoke.assert_called_once_with(jose.ComparableX509(cert)) - @mock.patch('letsencrypt.cli._determine_account') + @mock.patch('letsencrypt.main._determine_account') def test_revoke_without_key(self, mock_determine_account): mock_determine_account.return_value = (mock.MagicMock(), None) _, _, _, client = self._call(['--cert-path', CERT, 'revoke']) @@ -825,7 +825,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_revoke = client.acme_from_config_key().revoke mock_revoke.assert_called_once_with(jose.ComparableX509(cert)) - @mock.patch('letsencrypt.cli.sys') + @mock.patch('letsencrypt.main.sys') def test_handle_exception(self, mock_sys): # pylint: disable=protected-access from acme import messages @@ -833,7 +833,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods config = mock.MagicMock() mock_open = mock.mock_open() - with mock.patch('letsencrypt.cli.open', mock_open, create=True): + with mock.patch('letsencrypt.main.open', mock_open, create=True): exception = Exception('detail') config.verbose_count = 1 main._handle_exception( @@ -843,7 +843,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods error_msg = mock_sys.exit.call_args_list[0][0][0] self.assertTrue('unexpected error' in error_msg) - with mock.patch('letsencrypt.cli.open', mock_open, create=True): + with mock.patch('letsencrypt.main.open', mock_open, create=True): mock_open.side_effect = [KeyboardInterrupt] error = errors.Error('detail') main._handle_exception( @@ -908,7 +908,7 @@ class DetermineAccountTest(unittest.TestCase): def _call(self): # pylint: disable=protected-access from letsencrypt.main import _determine_account - with mock.patch('letsencrypt.cli.account.AccountFileStorage') as mock_storage: + with mock.patch('letsencrypt.main.account.AccountFileStorage') as mock_storage: mock_storage.return_value = self.account_storage return _determine_account(self.config) @@ -940,7 +940,7 @@ class DetermineAccountTest(unittest.TestCase): def test_no_accounts_no_email(self, mock_get_email): mock_get_email.return_value = 'foo@bar.baz' - with mock.patch('letsencrypt.cli.client') as client: + with mock.patch('letsencrypt.main.client') as client: client.register.return_value = ( self.accs[0], mock.sentinel.acme) self.assertEqual((self.accs[0], mock.sentinel.acme), self._call()) @@ -952,7 +952,7 @@ class DetermineAccountTest(unittest.TestCase): def test_no_accounts_email(self): self.config.email = 'other email' - with mock.patch('letsencrypt.cli.client') as client: + with mock.patch('letsencrypt.main.client') as client: client.register.return_value = (self.accs[1], mock.sentinel.acme) self._call() self.assertEqual(self.accs[1].id, self.config.account) @@ -1014,7 +1014,7 @@ class MockedVerb(object): """ def __init__(self, verb_name): - self.verb_dict = cli.HelpfulArgumentParser.VERBS + self.verb_dict = main.VERBS self.verb_func = None self.verb_name = verb_name From 683bebd56c841bc53bc7d13480ff6440848bf05e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Mar 2016 18:31:29 -0800 Subject: [PATCH 12/56] Lint --- letsencrypt/cli.py | 1 + letsencrypt/main.py | 2 +- letsencrypt/tests/display/util_test.py | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5ed97d03f..508eb48ad 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -29,6 +29,7 @@ from letsencrypt import storage from letsencrypt.display import ops as display_ops from letsencrypt.plugins import disco as plugins_disco +# pylint: disable=too-many-lines logger = logging.getLogger(__name__) diff --git a/letsencrypt/main.py b/letsencrypt/main.py index e8b3f7345..56725d300 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -701,7 +701,7 @@ def main(cli_args=sys.argv[1:]): # Maps verbs/subcommands to the functions that implement them -# In principle this should live in cli.HelpfulArgumentParser, but +# In principle this should live in cli.HelpfulArgumentParser, but # due to issues with import cycles and testing, it lives here VERBS = {"auth": obtain_cert, "certonly": obtain_cert, "config_changes": config_changes, "everything": run, diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index a16eb544e..3f8ee8bb5 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -8,7 +8,6 @@ import letsencrypt.errors as errors from letsencrypt.display import util as display_util - CHOICES = [("First", "Description1"), ("Second", "Description2")] TAGS = ["tag1", "tag2", "tag3"] TAGS_CHOICES = [("1", "tag1"), ("2", "tag2"), ("3", "tag3")] From 3c3c6ce359db78d586a417b3f79d50475d8b41c0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Mar 2016 18:54:03 -0800 Subject: [PATCH 13/56] Fight with cyclic lint --- .pylintrc | 2 +- letsencrypt/cli.py | 4 ++-- letsencrypt/renew.py | 0 3 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 letsencrypt/renew.py diff --git a/.pylintrc b/.pylintrc index 92dde98c0..49d0f29ea 100644 --- a/.pylintrc +++ b/.pylintrc @@ -38,7 +38,7 @@ load-plugins=linter_plugin # --enable=similarities". If you want to run only the classes checker, but have # no Warning level messages displayed, use"--disable=all --enable=classes # --disable=W" -disable=fixme,locally-disabled,abstract-class-not-used,abstract-class-little-used,bad-continuation,too-few-public-methods,no-self-use,invalid-name,too-many-instance-attributes +disable=fixme,locally-disabled,abstract-class-not-used,abstract-class-little-used,bad-continuation,too-few-public-methods,no-self-use,invalid-name,too-many-instance-attributes,cyclic-import # abstract-class-not-used cannot be disabled locally (at least in # pylint 1.4.1), same for abstract-class-little-used diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index dee43eac1..b76311777 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,4 +1,5 @@ """Let's Encrypt command CLI argument processing.""" +# pylint: disable=too-many-lines from __future__ import print_function import argparse import copy @@ -30,7 +31,6 @@ from letsencrypt import storage from letsencrypt.display import ops as display_ops from letsencrypt.plugins import disco as plugins_disco -# pylint: disable=too-many-lines logger = logging.getLogger(__name__) @@ -635,7 +635,7 @@ class HelpfulArgumentParser(object): self.VERBS = main.VERBS # List of topics for which additional help can be provided HELP_TOPICS = ["all", "security", - "paths", "automation", "testing"] + list(six.iterkeys(self.VERBS) + "paths", "automation", "testing"] + list(six.iterkeys(self.VERBS)) plugin_names = list(six.iterkeys(plugins)) self.help_topics = HELP_TOPICS + plugin_names + [None] diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py new file mode 100644 index 000000000..e69de29bb From d468e9926eeae9b3fbc534461dd6dce2a04a41a8 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 10 Mar 2016 18:58:49 -0800 Subject: [PATCH 14/56] Fix stray mockery --- letsencrypt/tests/client_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 7dd513e18..8be2178b9 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -128,7 +128,7 @@ class ClientTest(unittest.TestCase): # FIXME move parts of this to test_cli.py... @mock.patch("letsencrypt.client.logger") - @mock.patch("letsencrypt.cli._process_domain") + @mock.patch("letsencrypt.cli.process_domain") def test_obtain_certificate_from_csr(self, mock_process_domain, mock_logger): self._mock_obtain_certificate() from letsencrypt import cli From 2656d97260c3bd5eb074a70de5b6a06ce57a37d5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Mar 2016 11:53:58 -0800 Subject: [PATCH 15/56] Update entry point --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index b187e6fdb..87cef2cb2 100644 --- a/setup.py +++ b/setup.py @@ -127,7 +127,7 @@ setup( entry_points={ 'console_scripts': [ - 'letsencrypt = letsencrypt.cli:main', + 'letsencrypt = letsencrypt.main:main', ], 'letsencrypt.plugins': [ 'manual = letsencrypt.plugins.manual:Authenticator', From 388baa5a1eb49f22187548a6a7dee5d38dfe98aa Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Mar 2016 12:29:31 -0800 Subject: [PATCH 16/56] Start splitting renew.py out of cli.py --- letsencrypt/cli.py | 306 +--------------------------------- letsencrypt/main.py | 14 +- letsencrypt/plugins/common.py | 2 +- letsencrypt/renew.py | 304 +++++++++++++++++++++++++++++++++ letsencrypt/tests/cli_test.py | 24 +-- 5 files changed, 333 insertions(+), 317 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b76311777..e34bd971b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -2,7 +2,6 @@ # pylint: disable=too-many-lines from __future__ import print_function import argparse -import copy import glob import json import logging @@ -14,19 +13,14 @@ import traceback import configargparse import OpenSSL import six -import zope.component -import zope.interface.exceptions -import zope.interface.verify import letsencrypt -from letsencrypt import configuration from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util -from letsencrypt import storage from letsencrypt.display import ops as display_ops from letsencrypt.plugins import disco as plugins_disco @@ -35,16 +29,7 @@ from letsencrypt.plugins import disco as plugins_disco logger = logging.getLogger(__name__) # Global, to save us from a lot of argument passing within the scope of this module -_parser = None - -# These are the items which get pulled out of a renewal configuration -# file's renewalparams and actually used in the client configuration -# during the renewal process. We have to record their types here because -# the renewal configuration process loses this information. -STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent", - "server", "account", "authenticator", "installer", - "standalone_supported_challenges"] -INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] +helpful_parser = None # For help strings, figure out how the user ran us. # When invoked from letsencrypt-auto, sys.argv[0] is something like: @@ -115,21 +100,6 @@ def usage_strings(plugins): return USAGE % (apache_doc, nginx_doc), SHORT_USAGE -def should_renew(config, lineage): - "Return true if any of the circumstances for automatic renewal apply." - if config.renew_by_default: - logger.info("Auto-renewal forced with --force-renewal...") - return True - if lineage.should_autorenew(interactive=True): - logger.info("Cert is due for renewal, auto-renewing...") - return True - if config.dry_run: - logger.info("Cert not due for renewal, but simulating renewal for dry run") - return True - logger.info("Cert not yet due for renewal") - return False - - def diagnose_configurator_problem(cfg_type, requested, plugins): """ Raise the most helpful error message about a plugin being unavailable @@ -271,20 +241,20 @@ def record_chosen_plugins(config, plugins, auth, inst): cn.installer = plugins.find_init(inst).name if inst else "none" -def _set_by_cli(var): +def set_by_cli(var): """ Return True if a particular config variable has been set by the user (CLI or config file) including if the user explicitly set it to the default. Returns False if the variable was assigned a default value. """ - detector = _set_by_cli.detector + detector = set_by_cli.detector if detector is None: # Setup on first run: `detector` is a weird version of config in which # the default value of every attribute is wrangled to be boolean-false plugins = plugins_disco.PluginsRegistry.find_all() # reconstructed_args == sys.argv[1:], or whatever was passed to main() - reconstructed_args = _parser.args + [_parser.verb] - detector = _set_by_cli.detector = prepare_and_parse_args( + reconstructed_args = helpful_parser.args + [helpful_parser.verb] + detector = set_by_cli.detector = prepare_and_parse_args( plugins, reconstructed_args, detect_defaults=True) # propagate plugin requests: eg --standalone modifies config.authenticator auth, inst = cli_plugin_requests(detector) @@ -312,265 +282,7 @@ def _set_by_cli(var): else: return False # static housekeeping var -_set_by_cli.detector = None - -def _restore_required_config_elements(config, renewalparams): - """Sets non-plugin specific values in config from renewalparams - - :param configuration.NamespaceConfig config: configuration for the - current lineage - :param configobj.Section renewalparams: parameters from the renewal - configuration file that defines this lineage - - """ - # string-valued items to add if they're present - for config_item in STR_CONFIG_ITEMS: - if config_item in renewalparams and not _set_by_cli(config_item): - value = renewalparams[config_item] - # Unfortunately, we've lost type information from ConfigObj, - # so we don't know if the original was NoneType or str! - if value == "None": - value = None - setattr(config.namespace, config_item, value) - # int-valued items to add if they're present - for config_item in INT_CONFIG_ITEMS: - if config_item in renewalparams and not _set_by_cli(config_item): - config_value = renewalparams[config_item] - # the default value for http01_port was None during private beta - if config_item == "http01_port" and config_value == "None": - logger.info("updating legacy http01_port value") - int_value = flag_default("http01_port") - else: - try: - int_value = int(config_value) - except ValueError: - raise errors.Error( - "Expected a numeric value for {0}".format(config_item)) - setattr(config.namespace, config_item, int_value) - - -def _restore_plugin_configs(config, renewalparams): - """Sets plugin specific values in config from renewalparams - - :param configuration.NamespaceConfig config: configuration for the - current lineage - :param configobj.Section renewalparams: Parameters from the renewal - configuration file that defines this lineage - - """ - # Now use parser to get plugin-prefixed items with correct types - # XXX: the current approach of extracting only prefixed items - # related to the actually-used installer and authenticator - # works as long as plugins don't need to read plugin-specific - # variables set by someone else (e.g., assuming Apache - # configurator doesn't need to read webroot_ variables). - # Note: if a parameter that used to be defined in the parser is no - # longer defined, stored copies of that parameter will be - # deserialized as strings by this logic even if they were - # originally meant to be some other type. - if renewalparams["authenticator"] == "webroot": - _restore_webroot_config(config, renewalparams) - plugin_prefixes = [] - else: - plugin_prefixes = [renewalparams["authenticator"]] - - if renewalparams.get("installer", None) is not None: - plugin_prefixes.append(renewalparams["installer"]) - for plugin_prefix in set(plugin_prefixes): - for config_item, config_value in six.iteritems(renewalparams): - if config_item.startswith(plugin_prefix + "_") and not _set_by_cli(config_item): - # Values None, True, and False need to be treated specially, - # As they don't get parsed correctly based on type - if config_value in ("None", "True", "False"): - # bool("False") == True - # pylint: disable=eval-used - setattr(config.namespace, config_item, eval(config_value)) - continue - for action in _parser.parser._actions: # pylint: disable=protected-access - if action.type is not None and action.dest == config_item: - setattr(config.namespace, config_item, - action.type(config_value)) - break - else: - setattr(config.namespace, config_item, str(config_value)) - -def _restore_webroot_config(config, renewalparams): - """ - webroot_map is, uniquely, a dict, and the general-purpose configuration - restoring logic is not able to correctly parse it from the serialized - form. - """ - if "webroot_map" in renewalparams: - # if the user does anything that would create a new webroot map on the - # CLI, don't use the old one - if not (_set_by_cli("webroot_map") or _set_by_cli("webroot_path")): - setattr(config.namespace, "webroot_map", renewalparams["webroot_map"]) - elif "webroot_path" in renewalparams: - logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path") - wp = renewalparams["webroot_path"] - if isinstance(wp, str): # prior to 0.1.0, webroot_path was a string - wp = [wp] - setattr(config.namespace, "webroot_path", wp) - - -def _reconstitute(config, full_path): - """Try to instantiate a RenewableCert, updating config with relevant items. - - This is specifically for use in renewal and enforces several checks - and policies to ensure that we can try to proceed with the renwal - request. The config argument is modified by including relevant options - read from the renewal configuration file. - - :param configuration.NamespaceConfig config: configuration for the - current lineage - :param str full_path: Absolute path to the configuration file that - defines this lineage - - :returns: the RenewableCert object or None if a fatal error occurred - :rtype: `storage.RenewableCert` or NoneType - - """ - try: - renewal_candidate = storage.RenewableCert( - full_path, configuration.RenewerConfiguration(config)) - except (errors.CertStorageError, IOError): - logger.warning("Renewal configuration file %s is broken. Skipping.", full_path) - logger.debug("Traceback was:\n%s", traceback.format_exc()) - return None - if "renewalparams" not in renewal_candidate.configuration: - logger.warning("Renewal configuration file %s lacks " - "renewalparams. Skipping.", full_path) - return None - renewalparams = renewal_candidate.configuration["renewalparams"] - if "authenticator" not in renewalparams: - logger.warning("Renewal configuration file %s does not specify " - "an authenticator. Skipping.", full_path) - return None - # Now restore specific values along with their data types, if - # those elements are present. - try: - _restore_required_config_elements(config, renewalparams) - _restore_plugin_configs(config, renewalparams) - except (ValueError, errors.Error) as error: - logger.warning( - "An error occured while parsing %s. The error was %s. " - "Skipping the file.", full_path, error.message) - logger.debug("Traceback was:\n%s", traceback.format_exc()) - return None - - try: - for d in renewal_candidate.names(): - process_domain(config, d) - except errors.ConfigurationError as error: - logger.warning("Renewal configuration file %s references a cert " - "that contains an invalid domain name. The problem " - "was: %s. Skipping.", full_path, error) - return None - - return renewal_candidate - -def _renewal_conf_files(config): - """Return /path/to/*.conf in the renewal conf directory""" - return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) - - -def _renew_describe_results(config, renew_successes, renew_failures, - renew_skipped, parse_failures): - status = lambda x, msg: " " + "\n ".join(i + " (" + msg +")" for i in x) - if config.dry_run: - print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") - print("** (The test certificates below have not been saved.)") - print() - if renew_skipped: - print("The following certs are not due for renewal yet:") - print(status(renew_skipped, "skipped")) - if not renew_successes and not renew_failures: - print("No renewals were attempted.") - elif renew_successes and not renew_failures: - print("Congratulations, all renewals succeeded. The following certs " - "have been renewed:") - print(status(renew_successes, "success")) - elif renew_failures and not renew_successes: - print("All renewal attempts failed. The following certs could not be " - "renewed:") - print(status(renew_failures, "failure")) - elif renew_failures and renew_successes: - print("The following certs were successfully renewed:") - print(status(renew_successes, "success")) - print("\nThe following certs could not be renewed:") - print(status(renew_failures, "failure")) - - if parse_failures: - print("\nAdditionally, the following renewal configuration files " - "were invalid: ") - print(status(parse_failures, "parsefail")) - - if config.dry_run: - print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") - print("** (The test certificates above have not been saved.)") - - -def renew(config, unused_plugins): - """Renew previously-obtained certificates.""" - - if config.domains != []: - raise errors.Error("Currently, the renew verb is only capable of " - "renewing all installed certificates that are due " - "to be renewed; individual domains cannot be " - "specified with this action. If you would like to " - "renew specific certificates, use the certonly " - "command. The renew verb may provide other options " - "for selecting certificates to renew in the future.") - renewer_config = configuration.RenewerConfiguration(config) - renew_successes = [] - renew_failures = [] - renew_skipped = [] - parse_failures = [] - for renewal_file in _renewal_conf_files(renewer_config): - print("Processing " + renewal_file) - lineage_config = copy.deepcopy(config) - - # Note that this modifies config (to add back the configuration - # elements from within the renewal configuration file). - try: - renewal_candidate = _reconstitute(lineage_config, renewal_file) - except Exception as e: # pylint: disable=broad-except - logger.warning("Renewal configuration file %s produced an " - "unexpected error: %s. Skipping.", renewal_file, e) - logger.debug("Traceback was:\n%s", traceback.format_exc()) - parse_failures.append(renewal_file) - continue - - try: - if renewal_candidate is None: - parse_failures.append(renewal_file) - else: - # XXX: ensure that each call here replaces the previous one - zope.component.provideUtility(lineage_config) - if should_renew(lineage_config, renewal_candidate): - plugins = plugins_disco.PluginsRegistry.find_all() - from letsencrypt import main - main.obtain_cert(lineage_config, plugins, renewal_candidate) - renew_successes.append(renewal_candidate.fullchain) - else: - renew_skipped.append(renewal_candidate.fullchain) - except Exception as e: # pylint: disable=broad-except - # obtain_cert (presumably) encountered an unanticipated problem. - logger.warning("Attempting to renew cert from %s produced an " - "unexpected error: %s. Skipping.", renewal_file, e) - logger.debug("Traceback was:\n%s", traceback.format_exc()) - renew_failures.append(renewal_candidate.fullchain) - - # Describe all the results - _renew_describe_results(config, renew_successes, renew_failures, - renew_skipped, parse_failures) - - if renew_failures or parse_failures: - raise errors.Error("{0} renew failure(s), {1} parse failure(s)".format( - len(renew_failures), len(parse_failures))) - else: - logger.debug("no renewal failures") - +set_by_cli.detector = None def read_file(filename, mode="rb"): """Returns the given file's contents. @@ -839,7 +551,7 @@ class HelpfulArgumentParser(object): def modify_arg_for_default_detection(self, *args, **kwargs): """ Adding an arg, but ensure that it has a default that evaluates to false, - so that _set_by_cli can tell if it was set. Only called if detect_defaults==True. + so that set_by_cli can tell if it was set. Only called if detect_defaults==True. :param list *args: the names of this argument flag :param dict **kwargs: various argparse settings for this argument @@ -1108,8 +820,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): _plugins_parsing(helpful, plugins) if not detect_defaults: - global _parser # pylint: disable=global-statement - _parser = helpful + global helpful_parser # pylint: disable=global-statement + helpful_parser = helpful return helpful.parse_args() diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 56725d300..8d59993df 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -20,9 +20,9 @@ from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import log from letsencrypt import reporter +from letsencrypt import renew from letsencrypt import storage -from letsencrypt.cli import choose_configurator_plugins, _renewal_conf_files, should_renew from letsencrypt.display import util as display_util, ops as display_ops from letsencrypt.plugins import disco as plugins_disco @@ -184,7 +184,7 @@ def _handle_identical_cert_request(config, cert): :rtype: tuple """ - if should_renew(config, cert): + if renew.should_renew(config, cert): return "renew", cert if config.reinstall: # Set with --reinstall, force an identical certificate to be @@ -261,7 +261,7 @@ def _find_duplicative_certs(config, domains): # Verify the directory is there le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) - for renewal_file in _renewal_conf_files(cli_config): + for renewal_file in renew.renewal_conf_files(cli_config): try: candidate_lineage = storage.RenewableCert(renewal_file, cli_config) except (errors.CertStorageError, IOError): @@ -404,7 +404,7 @@ def install(config, plugins): # this function ... try: - installer, _ = choose_configurator_plugins(config, plugins, "install") + installer, _ = cli.choose_configurator_plugins(config, plugins, "install") except errors.PluginSelectionError as e: return e.message @@ -479,7 +479,7 @@ def run(config, plugins): # pylint: disable=too-many-branches,too-many-locals # TODO: Make run as close to auth + install as possible # Possible difficulties: config.csr was hacked into auth try: - installer, authenticator = choose_configurator_plugins(config, plugins, "run") + installer, authenticator = cli.choose_configurator_plugins(config, plugins, "run") except errors.PluginSelectionError as e: return e.message @@ -509,7 +509,7 @@ def obtain_cert(config, plugins, lineage=None): # pylint: disable=too-many-locals try: # installers are used in auth mode to determine domain names - installer, authenticator = choose_configurator_plugins(config, plugins, "certonly") + installer, authenticator = cli.choose_configurator_plugins(config, plugins, "certonly") except errors.PluginSelectionError as e: logger.info("Could not choose appropriate plugin: %s", e) raise @@ -705,7 +705,7 @@ def main(cli_args=sys.argv[1:]): # due to issues with import cycles and testing, it lives here VERBS = {"auth": obtain_cert, "certonly": obtain_cert, "config_changes": config_changes, "everything": run, - "install": install, "plugins": plugins_cmd, "renew": cli.renew, + "install": install, "plugins": plugins_cmd, "renew": renew.renew, "revoke": revoke, "rollback": rollback, "run": run} diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index 319692344..f6a2c3d76 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -52,7 +52,7 @@ class Plugin(object): NOTE: if you add argpase arguments such that users setting them can create a config entry that python's bool() would consider false (ie, the use might set the variable to "", [], 0, etc), please ensure that - cli._set_by_cli() works for your variable. + cli.set_by_cli() works for your variable. """ diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py index e69de29bb..6eb3fa27c 100644 --- a/letsencrypt/renew.py +++ b/letsencrypt/renew.py @@ -0,0 +1,304 @@ +"""Functionality for autorenewal and associated juggling of configurations""" +from __future__ import print_function +import copy +import glob +import logging +import os +import traceback + +import six +import zope.component + +from letsencrypt import configuration +from letsencrypt import cli +from letsencrypt import errors +from letsencrypt import storage +from letsencrypt.plugins import disco as plugins_disco + +logger = logging.getLogger(__name__) + +# These are the items which get pulled out of a renewal configuration +# file's renewalparams and actually used in the client configuration +# during the renewal process. We have to record their types here because +# the renewal configuration process loses this information. +STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent", + "server", "account", "authenticator", "installer", + "standalone_supported_challenges"] +INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] + + +def _renew_describe_results(config, renew_successes, renew_failures, + renew_skipped, parse_failures): + status = lambda x, msg: " " + "\n ".join(i + " (" + msg +")" for i in x) + if config.dry_run: + print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") + print("** (The test certificates below have not been saved.)") + print() + if renew_skipped: + print("The following certs are not due for renewal yet:") + print(status(renew_skipped, "skipped")) + if not renew_successes and not renew_failures: + print("No renewals were attempted.") + elif renew_successes and not renew_failures: + print("Congratulations, all renewals succeeded. The following certs " + "have been renewed:") + print(status(renew_successes, "success")) + elif renew_failures and not renew_successes: + print("All renewal attempts failed. The following certs could not be " + "renewed:") + print(status(renew_failures, "failure")) + elif renew_failures and renew_successes: + print("The following certs were successfully renewed:") + print(status(renew_successes, "success")) + print("\nThe following certs could not be renewed:") + print(status(renew_failures, "failure")) + + if parse_failures: + print("\nAdditionally, the following renewal configuration files " + "were invalid: ") + print(status(parse_failures, "parsefail")) + + if config.dry_run: + print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") + print("** (The test certificates above have not been saved.)") + + +def renewal_conf_files(config): + """Return /path/to/*.conf in the renewal conf directory""" + return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) + + +def _reconstitute(config, full_path): + """Try to instantiate a RenewableCert, updating config with relevant items. + + This is specifically for use in renewal and enforces several checks + and policies to ensure that we can try to proceed with the renwal + request. The config argument is modified by including relevant options + read from the renewal configuration file. + + :param configuration.NamespaceConfig config: configuration for the + current lineage + :param str full_path: Absolute path to the configuration file that + defines this lineage + + :returns: the RenewableCert object or None if a fatal error occurred + :rtype: `storage.RenewableCert` or NoneType + + """ + try: + renewal_candidate = storage.RenewableCert( + full_path, configuration.RenewerConfiguration(config)) + except (errors.CertStorageError, IOError): + logger.warning("Renewal configuration file %s is broken. Skipping.", full_path) + logger.debug("Traceback was:\n%s", traceback.format_exc()) + return None + if "renewalparams" not in renewal_candidate.configuration: + logger.warning("Renewal configuration file %s lacks " + "renewalparams. Skipping.", full_path) + return None + renewalparams = renewal_candidate.configuration["renewalparams"] + if "authenticator" not in renewalparams: + logger.warning("Renewal configuration file %s does not specify " + "an authenticator. Skipping.", full_path) + return None + # Now restore specific values along with their data types, if + # those elements are present. + try: + _restore_required_config_elements(config, renewalparams) + _restore_plugin_configs(config, renewalparams) + except (ValueError, errors.Error) as error: + logger.warning( + "An error occured while parsing %s. The error was %s. " + "Skipping the file.", full_path, error.message) + logger.debug("Traceback was:\n%s", traceback.format_exc()) + return None + + try: + for d in renewal_candidate.names(): + cli.process_domain(config, d) + except errors.ConfigurationError as error: + logger.warning("Renewal configuration file %s references a cert " + "that contains an invalid domain name. The problem " + "was: %s. Skipping.", full_path, error) + return None + + return renewal_candidate + + +def _restore_webroot_config(config, renewalparams): + """ + webroot_map is, uniquely, a dict, and the general-purpose configuration + restoring logic is not able to correctly parse it from the serialized + form. + """ + if "webroot_map" in renewalparams: + # if the user does anything that would create a new webroot map on the + # CLI, don't use the old one + if not (cli.set_by_cli("webroot_map") or cli.set_by_cli("webroot_path")): + setattr(config.namespace, "webroot_map", renewalparams["webroot_map"]) + elif "webroot_path" in renewalparams: + logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path") + wp = renewalparams["webroot_path"] + if isinstance(wp, str): # prior to 0.1.0, webroot_path was a string + wp = [wp] + setattr(config.namespace, "webroot_path", wp) + + +def _restore_plugin_configs(config, renewalparams): + """Sets plugin specific values in config from renewalparams + + :param configuration.NamespaceConfig config: configuration for the + current lineage + :param configobj.Section renewalparams: Parameters from the renewal + configuration file that defines this lineage + + """ + # Now use parser to get plugin-prefixed items with correct types + # XXX: the current approach of extracting only prefixed items + # related to the actually-used installer and authenticator + # works as long as plugins don't need to read plugin-specific + # variables set by someone else (e.g., assuming Apache + # configurator doesn't need to read webroot_ variables). + # Note: if a parameter that used to be defined in the parser is no + # longer defined, stored copies of that parameter will be + # deserialized as strings by this logic even if they were + # originally meant to be some other type. + if renewalparams["authenticator"] == "webroot": + _restore_webroot_config(config, renewalparams) + plugin_prefixes = [] + else: + plugin_prefixes = [renewalparams["authenticator"]] + + if renewalparams.get("installer", None) is not None: + plugin_prefixes.append(renewalparams["installer"]) + for plugin_prefix in set(plugin_prefixes): + for config_item, config_value in six.iteritems(renewalparams): + if config_item.startswith(plugin_prefix + "_") and not cli.set_by_cli(config_item): + # Values None, True, and False need to be treated specially, + # As they don't get parsed correctly based on type + if config_value in ("None", "True", "False"): + # bool("False") == True + # pylint: disable=eval-used + setattr(config.namespace, config_item, eval(config_value)) + continue + # If argparse has a type for this variable, use it: + # pylint: disable=protected-access + for action in cli.helpful_parser.parser._actions: + if action.type is not None and action.dest == config_item: + setattr(config.namespace, config_item, + action.type(config_value)) + break + else: + setattr(config.namespace, config_item, str(config_value)) + + +def _restore_required_config_elements(config, renewalparams): + """Sets non-plugin specific values in config from renewalparams + + :param configuration.NamespaceConfig config: configuration for the + current lineage + :param configobj.Section renewalparams: parameters from the renewal + configuration file that defines this lineage + + """ + # string-valued items to add if they're present + for config_item in STR_CONFIG_ITEMS: + if config_item in renewalparams and not cli.set_by_cli(config_item): + value = renewalparams[config_item] + # Unfortunately, we've lost type information from ConfigObj, + # so we don't know if the original was NoneType or str! + if value == "None": + value = None + setattr(config.namespace, config_item, value) + # int-valued items to add if they're present + for config_item in INT_CONFIG_ITEMS: + if config_item in renewalparams and not cli.set_by_cli(config_item): + config_value = renewalparams[config_item] + # the default value for http01_port was None during private beta + if config_item == "http01_port" and config_value == "None": + logger.info("updating legacy http01_port value") + int_value = cli.flag_default("http01_port") + else: + try: + int_value = int(config_value) + except ValueError: + raise errors.Error( + "Expected a numeric value for {0}".format(config_item)) + setattr(config.namespace, config_item, int_value) + + +def should_renew(config, lineage): + "Return true if any of the circumstances for automatic renewal apply." + if config.renew_by_default: + logger.info("Auto-renewal forced with --force-renewal...") + return True + if lineage.should_autorenew(interactive=True): + logger.info("Cert is due for renewal, auto-renewing...") + return True + if config.dry_run: + logger.info("Cert not due for renewal, but simulating renewal for dry run") + return True + logger.info("Cert not yet due for renewal") + return False + + +def renew(config, unused_plugins): + """Renew previously-obtained certificates.""" + + if config.domains != []: + raise errors.Error("Currently, the renew verb is only capable of " + "renewing all installed certificates that are due " + "to be renewed; individual domains cannot be " + "specified with this action. If you would like to " + "renew specific certificates, use the certonly " + "command. The renew verb may provide other options " + "for selecting certificates to renew in the future.") + renewer_config = configuration.RenewerConfiguration(config) + renew_successes = [] + renew_failures = [] + renew_skipped = [] + parse_failures = [] + for renewal_file in renewal_conf_files(renewer_config): + print("Processing " + renewal_file) + lineage_config = copy.deepcopy(config) + + # Note that this modifies config (to add back the configuration + # elements from within the renewal configuration file). + try: + renewal_candidate = _reconstitute(lineage_config, renewal_file) + except Exception as e: # pylint: disable=broad-except + logger.warning("Renewal configuration file %s produced an " + "unexpected error: %s. Skipping.", renewal_file, e) + logger.debug("Traceback was:\n%s", traceback.format_exc()) + parse_failures.append(renewal_file) + continue + + try: + if renewal_candidate is None: + parse_failures.append(renewal_file) + else: + # XXX: ensure that each call here replaces the previous one + zope.component.provideUtility(lineage_config) + if should_renew(lineage_config, renewal_candidate): + plugins = plugins_disco.PluginsRegistry.find_all() + from letsencrypt import main + main.obtain_cert(lineage_config, plugins, renewal_candidate) + renew_successes.append(renewal_candidate.fullchain) + else: + renew_skipped.append(renewal_candidate.fullchain) + except Exception as e: # pylint: disable=broad-except + # obtain_cert (presumably) encountered an unanticipated problem. + logger.warning("Attempting to renew cert from %s produced an " + "unexpected error: %s. Skipping.", renewal_file, e) + logger.debug("Traceback was:\n%s", traceback.format_exc()) + renew_failures.append(renewal_candidate.fullchain) + + # Describe all the results + _renew_describe_results(config, renew_successes, renew_failures, + renew_skipped, parse_failures) + + if renew_failures or parse_failures: + raise errors.Error("{0} renew failure(s), {1} parse failure(s)".format( + len(renew_failures), len(parse_failures))) + else: + logger.debug("no renewal failures") diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index c5865206d..e79bcb95a 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -1,5 +1,4 @@ """Tests for letsencrypt.cli.""" - from __future__ import print_function import argparse @@ -24,6 +23,7 @@ from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util from letsencrypt import main +from letsencrypt import renew from letsencrypt import storage from letsencrypt.plugins import disco @@ -555,7 +555,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._certonly_new_request_common, mock_client) def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None, - args=None, renew=True, error_expected=False): + args=None, should_renew=True, error_expected=False): # pylint: disable=too-many-locals,too-many-arguments cert_path = 'letsencrypt/tests/testdata/cert.pem' chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem' @@ -594,7 +594,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods "Unexpected renewal error:\n" + traceback.format_exc()) - if renew: + if should_renew: mock_client.obtain_certificate.assert_called_once_with(['isnot.org']) else: self.assertEqual(mock_client.obtain_certificate.call_count, 0) @@ -629,7 +629,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(get_utility().add_message.call_count, 1) _, _ = self._test_renewal_common(False, ['-tvv', '--debug', '--keep'], - log_out="not yet due", renew=False) + log_out="not yet due", should_renew=False) def _dump_log(self): with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf: @@ -652,9 +652,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_renew_verb(self): self._make_test_renewal_conf('sample-renewal.conf') args = ["renew", "--dry-run", "-tvv"] - self._test_renewal_common(True, [], args=args, renew=True) + self._test_renewal_common(True, [], args=args, should_renew=True) - @mock.patch("letsencrypt.cli._set_by_cli") + @mock.patch("letsencrypt.cli.set_by_cli") def test_ancient_webroot_renewal_conf(self, mock_set_by_cli): mock_set_by_cli.return_value = False rc_path = self._make_test_renewal_conf('sample-renewal-ancient.conf') @@ -664,7 +664,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods configuration.RenewerConfiguration(config)) renewalparams = lineage.configuration["renewalparams"] # pylint: disable=protected-access - cli._restore_webroot_config(config, renewalparams) + renew._restore_webroot_config(config, renewalparams) self.assertEqual(config.webroot_path, ["/var/www/"]) def test_renew_verb_empty_config(self): @@ -674,7 +674,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with open(os.path.join(rd, 'empty.conf'), 'w'): pass # leave the file empty args = ["renew", "--dry-run", "-tvv"] - self._test_renewal_common(False, [], args=args, renew=False, error_expected=True) + self._test_renewal_common(False, [], args=args, should_renew=False, error_expected=True) def _make_dummy_renewal_config(self): renewer_configs_dir = os.path.join(self.config_dir, 'renewal') @@ -695,7 +695,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_rc.return_value = mock_lineage with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert: self._test_renewal_common(True, None, error_expected=error_expected, - args=['renew'], renew=False) + args=['renew'], should_renew=False) if assert_oc_called is not None: if assert_oc_called: self.assertTrue(mock_obtain_cert.called) @@ -751,13 +751,13 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert: mock_obtain_cert.side_effect = Exception self._test_renewal_common(True, None, error_expected=True, - args=['renew'], renew=False) + args=['renew'], should_renew=False) def test_renew_with_bad_cli_args(self): self._test_renewal_common(True, None, args='renew -d example.com'.split(), - renew=False, error_expected=True) + should_renew=False, error_expected=True) self._test_renewal_common(True, None, args='renew --csr {0}'.format(CSR).split(), - renew=False, error_expected=True) + should_renew=False, error_expected=True) @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.main._treat_as_renewal') From 4ca25828b25c6f3ee3d5ce0591ccdbb4718f24e1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Mar 2016 13:46:37 -0800 Subject: [PATCH 17/56] Get tests passing --- letsencrypt/cli.py | 1 - letsencrypt/plugins/disco.py | 1 + letsencrypt/tests/cli_test.py | 8 ++++---- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e34bd971b..bea8b198d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,5 +1,4 @@ """Let's Encrypt command CLI argument processing.""" -# pylint: disable=too-many-lines from __future__ import print_function import argparse import glob diff --git a/letsencrypt/plugins/disco.py b/letsencrypt/plugins/disco.py index 9ed6ac596..27d2fb541 100644 --- a/letsencrypt/plugins/disco.py +++ b/letsencrypt/plugins/disco.py @@ -4,6 +4,7 @@ import logging import pkg_resources import zope.interface +import zope.interface.verify from letsencrypt import constants from letsencrypt import errors diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e79bcb95a..f1f539016 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -517,7 +517,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args += '-d foo.bar -a standalone certonly'.split() self._call(args) - @mock.patch('letsencrypt.cli.zope.component.getUtility') + @mock.patch('letsencrypt.main.zope.component.getUtility') def test_certonly_dry_run_new_request_success(self, mock_get_utility): mock_client = mock.MagicMock() mock_client.obtain_and_enroll_certificate.return_value = None @@ -530,7 +530,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(mock_get_utility().add_message.call_count, 1) @mock.patch('letsencrypt.crypto_util.notAfter') - @mock.patch('letsencrypt.cli.zope.component.getUtility') + @mock.patch('letsencrypt.main.zope.component.getUtility') def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter): cert_path = '/etc/letsencrypt/live/foo.bar' date = '1970-01-01' @@ -736,7 +736,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_renew_reconstitute_error(self): # pylint: disable=protected-access - with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: + with mock.patch('letsencrypt.main.renew._reconstitute') as mock_reconstitute: mock_reconstitute.side_effect = Exception self._test_renew_common(assert_oc_called=False, error_expected=True) @@ -759,7 +759,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._test_renewal_common(True, None, args='renew --csr {0}'.format(CSR).split(), should_renew=False, error_expected=True) - @mock.patch('letsencrypt.cli.zope.component.getUtility') + @mock.patch('letsencrypt.main.zope.component.getUtility') @mock.patch('letsencrypt.main._treat_as_renewal') @mock.patch('letsencrypt.main._init_le_client') def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility): From e644560a55087a5c0fbef7aff25d9c5ab7bec134 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Mar 2016 14:00:57 -0800 Subject: [PATCH 18/56] neaten --- letsencrypt/renew.py | 72 ++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py index 6eb3fa27c..bd00543c8 100644 --- a/letsencrypt/renew.py +++ b/letsencrypt/renew.py @@ -27,42 +27,6 @@ STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent", INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] -def _renew_describe_results(config, renew_successes, renew_failures, - renew_skipped, parse_failures): - status = lambda x, msg: " " + "\n ".join(i + " (" + msg +")" for i in x) - if config.dry_run: - print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") - print("** (The test certificates below have not been saved.)") - print() - if renew_skipped: - print("The following certs are not due for renewal yet:") - print(status(renew_skipped, "skipped")) - if not renew_successes and not renew_failures: - print("No renewals were attempted.") - elif renew_successes and not renew_failures: - print("Congratulations, all renewals succeeded. The following certs " - "have been renewed:") - print(status(renew_successes, "success")) - elif renew_failures and not renew_successes: - print("All renewal attempts failed. The following certs could not be " - "renewed:") - print(status(renew_failures, "failure")) - elif renew_failures and renew_successes: - print("The following certs were successfully renewed:") - print(status(renew_successes, "success")) - print("\nThe following certs could not be renewed:") - print(status(renew_failures, "failure")) - - if parse_failures: - print("\nAdditionally, the following renewal configuration files " - "were invalid: ") - print(status(parse_failures, "parsefail")) - - if config.dry_run: - print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") - print("** (The test certificates above have not been saved.)") - - def renewal_conf_files(config): """Return /path/to/*.conf in the renewal conf directory""" return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) @@ -242,6 +206,42 @@ def should_renew(config, lineage): return False +def _renew_describe_results(config, renew_successes, renew_failures, + renew_skipped, parse_failures): + status = lambda ms, category: " " + "\n ".join(m + " (%s)" % category for m in ms) + if config.dry_run: + print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") + print("** (The test certificates below have not been saved.)") + print() + if renew_skipped: + print("The following certs are not due for renewal yet:") + print(status(renew_skipped, "skipped")) + if not renew_successes and not renew_failures: + print("No renewals were attempted.") + elif renew_successes and not renew_failures: + print("Congratulations, all renewals succeeded. The following certs " + "have been renewed:") + print(status(renew_successes, "success")) + elif renew_failures and not renew_successes: + print("All renewal attempts failed. The following certs could not be " + "renewed:") + print(status(renew_failures, "failure")) + elif renew_failures and renew_successes: + print("The following certs were successfully renewed:") + print(status(renew_successes, "success")) + print("\nThe following certs could not be renewed:") + print(status(renew_failures, "failure")) + + if parse_failures: + print("\nAdditionally, the following renewal configuration files " + "were invalid: ") + print(status(parse_failures, "parsefail")) + + if config.dry_run: + print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") + print("** (The test certificates above have not been saved.)") + + def renew(config, unused_plugins): """Renew previously-obtained certificates.""" From 6f7f036e2cac155b3bfddd614fb603bd7ce0ac9d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Mar 2016 14:07:35 -0800 Subject: [PATCH 19/56] Neaten further --- letsencrypt/renew.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py index bd00543c8..367eda5b0 100644 --- a/letsencrypt/renew.py +++ b/letsencrypt/renew.py @@ -208,34 +208,35 @@ def should_renew(config, lineage): def _renew_describe_results(config, renew_successes, renew_failures, renew_skipped, parse_failures): - status = lambda ms, category: " " + "\n ".join(m + " (%s)" % category for m in ms) + def _status(msgs, category): + return " " + "\n ".join("%s (%s)" % (m, category) for m in msgs) if config.dry_run: print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") print("** (The test certificates below have not been saved.)") print() if renew_skipped: print("The following certs are not due for renewal yet:") - print(status(renew_skipped, "skipped")) + print(_status(renew_skipped, "skipped")) if not renew_successes and not renew_failures: print("No renewals were attempted.") elif renew_successes and not renew_failures: print("Congratulations, all renewals succeeded. The following certs " "have been renewed:") - print(status(renew_successes, "success")) + print(_status(renew_successes, "success")) elif renew_failures and not renew_successes: print("All renewal attempts failed. The following certs could not be " "renewed:") - print(status(renew_failures, "failure")) + print(_status(renew_failures, "failure")) elif renew_failures and renew_successes: print("The following certs were successfully renewed:") - print(status(renew_successes, "success")) + print(_status(renew_successes, "success")) print("\nThe following certs could not be renewed:") - print(status(renew_failures, "failure")) + print(_status(renew_failures, "failure")) if parse_failures: print("\nAdditionally, the following renewal configuration files " "were invalid: ") - print(status(parse_failures, "parsefail")) + print(_status(parse_failures, "parsefail")) if config.dry_run: print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry") From 86ab35df4f66dfc15a99cf0c581b9100b77cc643 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 11 Mar 2016 15:07:13 -0800 Subject: [PATCH 20/56] Move argparse type extraction back into cli.py --- letsencrypt/cli.py | 14 ++++++++++++++ letsencrypt/renew.py | 13 +++---------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bea8b198d..017e0a62e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -283,6 +283,16 @@ def set_by_cli(var): # static housekeeping var set_by_cli.detector = None + +def argparse_type(variable): + "Return our argparse type function for a config variable (default: str)" + # pylint: disable=protected-access + for action in helpful_parser.parser._actions: + if action.type is not None and action.dest == variable: + return action.type + return str + + def read_file(filename, mode="rb"): """Returns the given file's contents. @@ -304,6 +314,10 @@ def read_file(filename, mode="rb"): def flag_default(name): """Default value for CLI flag.""" + # XXX: this is an internal housekeeping notion of defaults before + # argparse has been set up; it is not accurate for all flags. Call it + # with caution. Plugin defaults are missing, and some things are using + # defaults defined in this file, not in constants.py :( return constants.CLI_DEFAULTS[name] diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py index 367eda5b0..9dc1ff4df 100644 --- a/letsencrypt/renew.py +++ b/letsencrypt/renew.py @@ -139,21 +139,14 @@ def _restore_plugin_configs(config, renewalparams): for config_item, config_value in six.iteritems(renewalparams): if config_item.startswith(plugin_prefix + "_") and not cli.set_by_cli(config_item): # Values None, True, and False need to be treated specially, - # As they don't get parsed correctly based on type + # As their types aren't handled correctly by configobj if config_value in ("None", "True", "False"): # bool("False") == True # pylint: disable=eval-used setattr(config.namespace, config_item, eval(config_value)) - continue - # If argparse has a type for this variable, use it: - # pylint: disable=protected-access - for action in cli.helpful_parser.parser._actions: - if action.type is not None and action.dest == config_item: - setattr(config.namespace, config_item, - action.type(config_value)) - break else: - setattr(config.namespace, config_item, str(config_value)) + cast = cli.argparse_type(config_item) + setattr(config.namespace, config_item, cast(config_value)) def _restore_required_config_elements(config, renewalparams): From 0aed0e90f1a1a8e0ca67b83e7187ac44966b593e Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Fri, 11 Mar 2016 17:07:13 -0800 Subject: [PATCH 21/56] don't include files that have multiple vhosts --- letsencrypt-apache/letsencrypt_apache/configurator.py | 1 + letsencrypt-apache/letsencrypt_apache/display_ops.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 07c145bfc..3a679fa7e 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -545,6 +545,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): paths = self.aug.match( ("/files%s//*[label()=~regexp('%s')]" % (vhost_path, parser.case_i("VirtualHost")))) + paths = [path for path in paths if os.path.basename(path) == "VirtualHost"] for path in paths: new_vhost = self._create_vhost(path) realpath = os.path.realpath(new_vhost.filep) diff --git a/letsencrypt-apache/letsencrypt_apache/display_ops.py b/letsencrypt-apache/letsencrypt_apache/display_ops.py index 6a2308d73..bd3aa524d 100644 --- a/letsencrypt-apache/letsencrypt_apache/display_ops.py +++ b/letsencrypt-apache/letsencrypt_apache/display_ops.py @@ -83,7 +83,8 @@ def _vhost_menu(domain, vhosts): code, tag = zope.component.getUtility(interfaces.IDisplay).menu( "We were unable to find a vhost with a ServerName " "or Address of {0}.{1}Which virtual host would you " - "like to choose?".format(domain, os.linesep), + "like to choose?\n(note: conf files with multiple " + "vhosts are not currently supported)".format(domain, os.linesep), choices, help_label="More Info", ok_label="Select") except errors.MissingCommandlineFlag as e: msg = ("Failed to run Apache plugin non-interactively{1}{0}{1}" From aefab5cc3267df08d3b0949c0de8436dc2af9aad Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Mon, 14 Mar 2016 15:11:52 +0200 Subject: [PATCH 22/56] Fixing tests --- letsencrypt/tests/auth_handler_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 6398ec7a4..24718fa82 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -126,7 +126,10 @@ class GetAuthorizationsTest(unittest.TestCase): for achall in self.mock_auth.cleanup.call_args[0][0]: self.assertTrue(achall.typ in ["tls-sni-01", "http-01", "dns"]) - self.assertEqual(len(authzr), 1) + # Length of authorizations list + self.assertEqual(len(authzr[0]), 1) + # Length of valid domains list + self.assertEqual(len(authzr[1]), 1) @mock.patch("letsencrypt.auth_handler.AuthHandler._poll_challenges") def test_name3_tls_sni_01_3(self, mock_poll): From 2054150abb430e125b89cde7eaa03a8b34896932 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Mar 2016 15:35:20 -0700 Subject: [PATCH 23/56] Work in progress --- letsencrypt/cli.py | 14 ++++++++ letsencrypt/storage.py | 77 +++++++++++++++++++++++++++++++++--------- 2 files changed, 75 insertions(+), 16 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 024f53d0b..0992f0877 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -537,6 +537,20 @@ def diagnose_configurator_problem(cfg_type, requested, plugins): raise errors.PluginSelectionError(msg) +def _relevant(option): + """ + Is this option one that could be restored and used for future renewal purposes? + :param str option: the name of the option + + :rtype: bool + """ + # The list() here produces a list of the plugin names as strings. + plugins = list(plugins_disco.PluginsRegistry.find_all()) + return (option in STR_CONFIG_ITEMS + or option in INT_CONFIG_ITEMS + or any(option.startswith(x + "_") for x in plugins)) + + def set_configurator(previously, now): """ Setting configurators multiple ways is okay, as long as they all agree diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index cff2d53e1..bea686e19 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -50,13 +50,12 @@ def add_time_interval(base_time, interval, textparser=parsedatetime.Calendar()): return textparser.parseDT(interval, base_time, tzinfo=tzinfo)[0] -def write_renewal_config(filename, target, cli_config): +def write_renewal_config(filename, target, relevant_data): """Writes a renewal config file with the specified name and values. :param str filename: Absolute path to the config file :param dict target: Maps ALL_FOUR to their symlink paths - :param .RenewerConfiguration cli_config: parsed command line - arguments + :param dict relevant_data: Renewal configuration options to save :returns: Configuration object for the new config file :rtype: configobj.ConfigObj @@ -67,18 +66,11 @@ def write_renewal_config(filename, target, cli_config): for kind in ALL_FOUR: config[kind] = target[kind] - # XXX: We clearly need a more general and correct way of getting - # options into the configobj for the RenewableCert instance. - # This is a quick-and-dirty way to do it to allow integration - # testing to start. (Note that the config parameter to new_lineage - # ideally should be a ConfigObj, but in this case a dict will be - # accepted in practice.) - renewalparams = vars(cli_config.namespace) - if renewalparams: - config["renewalparams"] = renewalparams + if relevant_data: + config["renewalparams"] = relevant_data config.comments["renewalparams"] = ["", - "Options and defaults used" - " in the renewal process"] + "Options used in " + "the renewal process"] # TODO: add human-readable comments explaining other available # parameters @@ -106,7 +98,10 @@ def update_configuration(lineagename, target, cli_config): # If an existing tempfile exists, delete it if os.path.exists(temp_filename): os.unlink(temp_filename) - write_renewal_config(temp_filename, target, cli_config) + + # Save only the config items that are relevant to renewal + values = relevant_values(vars(cli_config.namespace)) + write_renewal_config(temp_filename, target, values) os.rename(temp_filename, config_filename) return configobj.ConfigObj(config_filename) @@ -127,6 +122,50 @@ def get_link_target(link): return os.path.abspath(target) +def relevant_values(all_values): + """Return a new dict containing only items relevant for renewal. + + :param dict all_values: The original values. + + :returns: A new dictionary containing items that can be used in renewal. + :rtype dict:""" + + from letsencrypt import cli + + import code + code.interact(local=locals()) + def _is_cli_default(option, value): + # Look through the CLI parser defaults and see if this option is + # both present and equal to the specified value. If not, return + # False. + for x in cli._parser.parser._actions: + if x.dest == option: + if x.default == value: + return True + else: + break + return False + + values = dict() + for option, value in all_values.iteritems(): + # Try to find reasons to store this item in the + # renewal config. It can be stored if it is relevant and + # (it is _set_by_cli() or flag_default() is different + # from the value or flag_default() doesn't exist). + if cli._relevant(option): + if (cli._set_by_cli(option) + or not _is_cli_default(option, value)): +# or option not in constants.CLI_DEFAULTS +# or constants.CLI_DEFAULTS[option] != value): + values[option] = value + print option, value + else: + print "not saving", option, value + else: + print "not relevant", option, value + return values + + class RenewableCert(object): # pylint: disable=too-many-instance-attributes """Renewable certificate. @@ -690,6 +729,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :rtype: :class:`storage.renewableCert` """ + # Examine the configuration and find the new lineage's name for i in (cli_config.renewal_configs_dir, cli_config.archive_dir, cli_config.live_dir): @@ -744,7 +784,12 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Document what we've done in a new renewal config file config_file.close() - new_config = write_renewal_config(config_filename, target, cli_config) + + # Save only the config items that are relevant to renewal + values = relevant_values(vars(cli_config.namespace)) + print (values) + + new_config = write_renewal_config(config_filename, target, values) return cls(new_config.filename, cli_config) def save_successor(self, prior_version, new_cert, From bd1b5229406991a03dcccea01f483607bb26fd10 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Mar 2016 17:11:09 -0700 Subject: [PATCH 24/56] Remove interact() --- letsencrypt/storage.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index bea686e19..672d3ff1f 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -132,8 +132,6 @@ def relevant_values(all_values): from letsencrypt import cli - import code - code.interact(local=locals()) def _is_cli_default(option, value): # Look through the CLI parser defaults and see if this option is # both present and equal to the specified value. If not, return From b31372b2714f00db21cdfb44369b27b64ab4ea4f Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Mar 2016 17:13:52 -0700 Subject: [PATCH 25/56] Remove debug prints --- letsencrypt/storage.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 672d3ff1f..263803d7f 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -157,10 +157,6 @@ def relevant_values(all_values): # or constants.CLI_DEFAULTS[option] != value): values[option] = value print option, value - else: - print "not saving", option, value - else: - print "not relevant", option, value return values From 62679b2b21c4ee49a4be055e68828a00aa1ed115 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Mar 2016 17:28:35 -0700 Subject: [PATCH 26/56] Test coverage for storage.py changes --- letsencrypt/tests/storage_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index 7bc31dab3..0d89156d3 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -557,6 +557,22 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.exists(temp_config_file)) + def test_relevant_values(self): + """Test that relevant_values() can reject an irrelevant value.""" + from letsencrypt import storage + self.assertEqual(storage.relevant_values({"hello": "there"}), {}) + + def test_relevant_values_default(self): + """Test that relevant_values() can reject a default value.""" + from letsencrypt import storage + self.assertEqual(storage.relevant_values({"rsa_key_size": 2048}), {}) + + def test_relevant_values_nondefault(self): + """Test that relevant_values() can retain a non-default value.""" + from letsencrypt import storage + self.assertEqual(storage.relevant_values({"rsa_key_size": 12}), + {"rsa_key_size": 12}) + def test_new_lineage(self): """Test for new_lineage() class method.""" from letsencrypt import storage From 29a4b2a37e557c97e864e2c38abce0de72966978 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Mar 2016 17:40:26 -0700 Subject: [PATCH 27/56] Remove two more debug prints --- letsencrypt/storage.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 263803d7f..5f5064f7d 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -156,7 +156,6 @@ def relevant_values(all_values): # or option not in constants.CLI_DEFAULTS # or constants.CLI_DEFAULTS[option] != value): values[option] = value - print option, value return values @@ -781,7 +780,6 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Save only the config items that are relevant to renewal values = relevant_values(vars(cli_config.namespace)) - print (values) new_config = write_renewal_config(config_filename, target, values) return cls(new_config.filename, cli_config) From b19c74be327c03fdb2810fc492fbbc8a6e7ec71b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 14 Mar 2016 18:44:29 -0700 Subject: [PATCH 28/56] Address review comments --- letsencrypt/cli.py | 7 +++---- letsencrypt/main.py | 11 ++++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b76311777..c4d127718 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1,4 +1,4 @@ -"""Let's Encrypt command CLI argument processing.""" +"""Let's Encrypt command line argument & config processing.""" # pylint: disable=too-many-lines from __future__ import print_function import argparse @@ -634,10 +634,9 @@ class HelpfulArgumentParser(object): from letsencrypt import main self.VERBS = main.VERBS # List of topics for which additional help can be provided - HELP_TOPICS = ["all", "security", - "paths", "automation", "testing"] + list(six.iterkeys(self.VERBS)) + HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + list(self.VERBS) - plugin_names = list(six.iterkeys(plugins)) + plugin_names = list(plugins) self.help_topics = HELP_TOPICS + plugin_names + [None] usage, short_usage = usage_strings(plugins) self.parser = configargparse.ArgParser( diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 56725d300..d2d2c55ac 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -2,8 +2,13 @@ from __future__ import print_function import atexit import functools +import logging.handlers import os import sys +import time +import traceback + +import OpenSSL import zope.component import letsencrypt @@ -22,15 +27,11 @@ from letsencrypt import log from letsencrypt import reporter from letsencrypt import storage +from acme import jose from letsencrypt.cli import choose_configurator_plugins, _renewal_conf_files, should_renew from letsencrypt.display import util as display_util, ops as display_ops from letsencrypt.plugins import disco as plugins_disco -import traceback -import logging.handlers -import time -from acme import jose -import OpenSSL logger = logging.getLogger(__name__) From 55eeb655bbcc55520b5a415d68e812d7d9166a5a Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 14 Mar 2016 20:11:31 -0700 Subject: [PATCH 29/56] Moved VERBS back to cli.py --- letsencrypt/cli.py | 8 +++++-- letsencrypt/main.py | 9 -------- letsencrypt/tests/cli_test.py | 39 +++++------------------------------ 3 files changed, 11 insertions(+), 45 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 85a0d1d8a..662e1a94b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -629,9 +629,13 @@ class HelpfulArgumentParser(object): """ def __init__(self, args, plugins, detect_defaults=False): - from letsencrypt import main - self.VERBS = main.VERBS + self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert, + "config_changes": main.config_changes, "run": main.run, + "install": main.install, "plugins": main.plugins_cmd, + "renew": renew, "revoke": main.revoke, + "rollback": main.rollback, "everything": main.run} + # List of topics for which additional help can be provided HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + list(self.VERBS) diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 19636b93e..264f7625e 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -701,15 +701,6 @@ def main(cli_args=sys.argv[1:]): return config.func(config, plugins) -# Maps verbs/subcommands to the functions that implement them -# In principle this should live in cli.HelpfulArgumentParser, but -# due to issues with import cycles and testing, it lives here -VERBS = {"auth": obtain_cert, "certonly": obtain_cert, - "config_changes": config_changes, "everything": run, - "install": install, "plugins": plugins_cmd, "renew": cli.renew, - "revoke": revoke, "rollback": rollback, "run": run} - - if __name__ == "__main__": err_string = main() if err_string: diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index c5865206d..7b901d410 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -79,7 +79,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods return ret, None, stderr, client def test_no_flags(self): - with MockedVerb("run") as mock_run: + with mock.patch('letsencrypt.main.run') as mock_run: self._call([]) self.assertEqual(1, mock_run.call_count) @@ -190,7 +190,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods chain = 'chain' fullchain = 'fullchain' - with MockedVerb('install') as mock_install: + with mock.patch('letsencrypt.main.install') as mock_install: self._call(['install', '--cert-path', cert, '--key-path', 'key', '--chain-path', 'chain', '--fullchain-path', 'fullchain']) @@ -248,7 +248,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods unused_config, auth, unused_installer = mock_init.call_args[0] self.assertTrue(isinstance(auth, manual.Authenticator)) - with MockedVerb("certonly") as mock_certonly: + with mock.patch('letsencrypt.main.obtain_cert') as mock_certonly: self._call(["auth", "--standalone"]) self.assertEqual(1, mock_certonly.call_count) @@ -321,7 +321,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods chain = 'chain' fullchain = 'fullchain' - with MockedVerb('certonly') as mock_obtaincert: + with mock.patch('letsencrypt.main.obtain_cert') as mock_obtaincert: self._call(['certonly', '--cert-path', cert, '--key-path', 'key', '--chain-path', 'chain', '--fullchain-path', 'fullchain']) @@ -900,7 +900,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(contents, test_contents) def test_agree_dev_preview_config(self): - with MockedVerb('run') as mocked_run: + with mock.patch('letsencrypt.main.run') as mocked_run: self._call(['-c', test_util.vector_path('cli.ini')]) self.assertTrue(mocked_run.called) @@ -1010,34 +1010,5 @@ class DuplicativeCertsTest(storage_test.BaseRenewableCertTest): self.assertEqual(result, (None, None)) -class MockedVerb(object): - """Simple class that can be used for mocking out verbs/subcommands. - - Storing a dictionary of verbs and the functions that implement them - in letsencrypt.cli makes mocking much more complicated. This class - can be used as a simple context manager for mocking out verbs in CLI - tests. For example: - - with MockedVerb("run") as mock_run: - self._call([]) - self.assertEqual(1, mock_run.call_count) - - """ - def __init__(self, verb_name): - self.verb_dict = main.VERBS - self.verb_func = None - self.verb_name = verb_name - - def __enter__(self): - self.verb_func = self.verb_dict[self.verb_name] - mocked_func = mock.MagicMock() - self.verb_dict[self.verb_name] = mocked_func - - return mocked_func - - def __exit__(self, unused_type, unused_value, unused_trace): - self.verb_dict[self.verb_name] = self.verb_func - - if __name__ == '__main__': unittest.main() # pragma: no cover From 1f22b3cbefab8e9100e6d2e6a5f502ea1f4cf6ae Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 14 Mar 2016 20:58:20 -0700 Subject: [PATCH 30/56] Move _relevant from cli.py to storage.py --- letsencrypt/cli.py | 14 -------------- letsencrypt/storage.py | 18 +++++++++++++++++- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 12ed74edc..8e545a5de 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -537,20 +537,6 @@ def diagnose_configurator_problem(cfg_type, requested, plugins): raise errors.PluginSelectionError(msg) -def _relevant(option): - """ - Is this option one that could be restored and used for future renewal purposes? - :param str option: the name of the option - - :rtype: bool - """ - # The list() here produces a list of the plugin names as strings. - plugins = list(plugins_disco.PluginsRegistry.find_all()) - return (option in STR_CONFIG_ITEMS - or option in INT_CONFIG_ITEMS - or any(option.startswith(x + "_") for x in plugins)) - - def set_configurator(previously, now): """ Setting configurators multiple ways is okay, as long as they all agree diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 5f5064f7d..5e5d70518 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -122,6 +122,22 @@ def get_link_target(link): return os.path.abspath(target) +def _relevant(option): + """ + Is this option one that could be restored for future renewal purposes? + :param str option: the name of the option + + :rtype: bool + """ + # The list() here produces a list of the plugin names as strings. + from letsencrypt import cli + from letsencrypt.plugins import disco as plugins_disco + plugins = list(plugins_disco.PluginsRegistry.find_all()) + return (option in cli.STR_CONFIG_ITEMS + or option in cli.INT_CONFIG_ITEMS + or any(option.startswith(x + "_") for x in plugins)) + + def relevant_values(all_values): """Return a new dict containing only items relevant for renewal. @@ -150,7 +166,7 @@ def relevant_values(all_values): # renewal config. It can be stored if it is relevant and # (it is _set_by_cli() or flag_default() is different # from the value or flag_default() doesn't exist). - if cli._relevant(option): + if _relevant(option): if (cli._set_by_cli(option) or not _is_cli_default(option, value)): # or option not in constants.CLI_DEFAULTS From 68ca289e7f84b7579787751a22bc56216b4ec955 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Tue, 15 Mar 2016 12:07:46 -0700 Subject: [PATCH 31/56] change wording --- letsencrypt-apache/letsencrypt_apache/display_ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/display_ops.py b/letsencrypt-apache/letsencrypt_apache/display_ops.py index bd3aa524d..4c01579cc 100644 --- a/letsencrypt-apache/letsencrypt_apache/display_ops.py +++ b/letsencrypt-apache/letsencrypt_apache/display_ops.py @@ -84,7 +84,7 @@ def _vhost_menu(domain, vhosts): "We were unable to find a vhost with a ServerName " "or Address of {0}.{1}Which virtual host would you " "like to choose?\n(note: conf files with multiple " - "vhosts are not currently supported)".format(domain, os.linesep), + "vhosts are not yet supported)".format(domain, os.linesep), choices, help_label="More Info", ok_label="Select") except errors.MissingCommandlineFlag as e: msg = ("Failed to run Apache plugin non-interactively{1}{0}{1}" From 97dba025cfa4d0e275e3af1b2c93cea4fd72ba3d Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Wed, 16 Mar 2016 00:37:39 +0200 Subject: [PATCH 32/56] Logging failed domains --- letsencrypt/auth_handler.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index d63b29156..3a3dc7c58 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -78,6 +78,10 @@ class AuthHandler(object): if response: failed_domains = failed_domains.union(response) + for domain in failed_domains: + logger.warning( + "Challenge failed for domain %s", + domain) returnDomains = [domain for domain in domains if domain not in failed_domains] From f254d6def137eca5ca4a53fb4b45e48380aea39f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 15 Mar 2016 17:27:19 -0700 Subject: [PATCH 33/56] fix spacing --- letsencrypt/tests/cli_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index bfd3818c1..3cc65fd11 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -133,7 +133,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods out = self._help_output(['-h']) self.assertTrue(cli.usage_strings(plugins)[0] in out) - def _cli_missing_flag(self, args, message): "Ensure that a particular error raises a missing cli flag error containing message" exc = None From 9444c9e54b8bcdd65d81245e0b382f40acb71616 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 15 Mar 2016 17:34:15 -0700 Subject: [PATCH 34/56] Make _test_renew_common more useful --- letsencrypt/tests/cli_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 3cc65fd11..9b474522e 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -679,8 +679,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: f.write("My contents don't matter") - def _test_renew_common(self, renewalparams=None, error_expected=False, - names=None, assert_oc_called=None): + def _test_renew_common(self, renewalparams=None, names=None, + assert_oc_called=None, **kwargs): self._make_dummy_renewal_config() with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: mock_lineage = mock.MagicMock() @@ -691,8 +691,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods mock_lineage.names.return_value = names mock_rc.return_value = mock_lineage with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: - self._test_renewal_common(True, None, error_expected=error_expected, - args=['renew'], renew=False) + kwargs.setdefault('args', ['renew']) + self._test_renewal_common(True, None, renew=False, **kwargs) if assert_oc_called is not None: if assert_oc_called: self.assertTrue(mock_obtain_cert.called) From b3fbb05f487a5e15ed7cb5ff9737bf7b7cd7ae4c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 15 Mar 2016 17:42:37 -0700 Subject: [PATCH 35/56] More test cleanup --- letsencrypt/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 9b474522e..66edb3be2 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -715,7 +715,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_renew_with_nonetype_http01(self): renewalparams = {'authenticator': 'webroot', 'http01_port': 'None'} - self._test_renew_common(renewalparams=renewalparams, error_expected=False, + self._test_renew_common(renewalparams=renewalparams, assert_oc_called=True) def test_renew_with_bad_domain(self): From 640582d4b62c0288f287f5218fae407f8d361cca Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 15 Mar 2016 18:29:12 -0700 Subject: [PATCH 36/56] fixes #2668 --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 8e545a5de..e0aea0916 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -543,7 +543,7 @@ def set_configurator(previously, now): :param str previously: previously identified request for the installer/authenticator :param str requested: the request currently being processed """ - if now is None: + if not now: # we're not actually setting anything return previously if previously: From 910a437f5e2f7d7ecf54f2368f8824c6096eecbe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 15 Mar 2016 18:35:34 -0700 Subject: [PATCH 37/56] Add test to prevent regressions --- letsencrypt/tests/cli_test.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 66edb3be2..e4a9dc3de 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -51,6 +51,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def tearDown(self): shutil.rmtree(self.tmp_dir) + # Reset globals in cli + # pylint: disable=protected-access + cli._parser = cli._set_by_cli.detector = None def _call(self, args): "Run the cli with output streams and actual client mocked out" @@ -724,6 +727,12 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._test_renew_common(renewalparams=renewalparams, error_expected=True, names=names, assert_oc_called=False) + def test_renew_with_configurator(self): + renewalparams = {'authenticator': 'webroot'} + self._test_renew_common( + renewalparams=renewalparams, assert_oc_called=True, + args='renew --configurator apache'.split()) + def test_renew_plugin_config_restoration(self): renewalparams = {'authenticator': 'webroot', 'webroot_path': 'None', From 1390c96e65259c216dec172159696d0e80f10d24 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Wed, 16 Mar 2016 14:38:50 +0100 Subject: [PATCH 38/56] handle permission denied during cleanup if the .well-known/acme-challenge was created before and the user has no permissions to delete it, it failed at cleanup. --- letsencrypt/plugins/webroot.py | 3 +++ letsencrypt/plugins/webroot_test.py | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/webroot.py b/letsencrypt/plugins/webroot.py index 0e3ebe1a7..6d2899511 100644 --- a/letsencrypt/plugins/webroot.py +++ b/letsencrypt/plugins/webroot.py @@ -152,5 +152,8 @@ to serve all files under specified web root ({0}).""" if exc.errno == errno.ENOTEMPTY: logger.debug("Challenges cleaned up but %s not empty", root_path) + elif exc.errno == errno.EACCES: + logger.debug("Challenges cleaned up but no permissions for %s", + root_path) else: raise diff --git a/letsencrypt/plugins/webroot_test.py b/letsencrypt/plugins/webroot_test.py index 8c1427340..ed0326555 100644 --- a/letsencrypt/plugins/webroot_test.py +++ b/letsencrypt/plugins/webroot_test.py @@ -158,7 +158,7 @@ class AuthenticatorTest(unittest.TestCase): os.rmdir(leftover_path) @mock.patch('os.rmdir') - def test_cleanup_oserror(self, mock_rmdir): + def test_cleanup_permission_denied(self, mock_rmdir): self.auth.prepare() self.auth.perform([self.achall]) @@ -166,10 +166,22 @@ class AuthenticatorTest(unittest.TestCase): os_error.errno = errno.EACCES mock_rmdir.side_effect = os_error + self.auth.cleanup([self.achall]) + self.assertFalse(os.path.exists(self.validation_path)) + self.assertTrue(os.path.exists(self.root_challenge_path)) + + @mock.patch('os.rmdir') + def test_cleanup_oserror(self, mock_rmdir): + self.auth.prepare() + self.auth.perform([self.achall]) + + os_error = OSError() + os_error.errno = errno.ENOENT + mock_rmdir.side_effect = os_error + self.assertRaises(OSError, self.auth.cleanup, [self.achall]) self.assertFalse(os.path.exists(self.validation_path)) self.assertTrue(os.path.exists(self.root_challenge_path)) - if __name__ == "__main__": unittest.main() # pragma: no cover From 4d6a1ee7ff23b29d4416ee91610a7d8a58173292 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Thu, 17 Mar 2016 08:30:07 +0200 Subject: [PATCH 39/56] Cleaning up code based on bmw's comments --- letsencrypt/auth_handler.py | 40 +++++++++++--------------- letsencrypt/cli.py | 11 +++---- letsencrypt/client.py | 21 ++++++++------ letsencrypt/tests/auth_handler_test.py | 8 ++---- letsencrypt/tests/client_test.py | 16 +++++++++-- 5 files changed, 52 insertions(+), 44 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 3a3dc7c58..1d82705b7 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -66,31 +66,27 @@ class AuthHandler(object): self._choose_challenges(domains) - failed_domains = set() # While there are still challenges remaining... while self.achalls: resp = self._solve_challenges() logger.info("Waiting for verification...") - # Send all Responses - this modifies dv_c and cont_c - response = self._respond(resp, best_effort) - - if response: - failed_domains = failed_domains.union(response) - for domain in failed_domains: - logger.warning( - "Challenge failed for domain %s", - domain) - - returnDomains = [domain for domain in domains - if domain not in failed_domains] + # Send all Responses - this modifies achalls + self._respond(resp, best_effort) # Just make sure all decisions are complete. self.verify_authzr_complete() + # Only return valid authorizations - return [authzr for authzr in self.authzr.values() - if authzr.body.status == messages.STATUS_VALID], returnDomains + retVal = [authzr for authzr in self.authzr.values() + if authzr.body.status == messages.STATUS_VALID] + + if len(retVal) <= 0: + logger.critical("Challenges failed for all domains") + raise + + return retVal def _choose_challenges(self, domains): """Retrieve necessary challenges to satisfy server.""" @@ -134,13 +130,11 @@ class AuthHandler(object): # Check for updated status... try: - failed_domains = self._poll_challenges(chall_update, best_effort) + self._poll_challenges(chall_update, best_effort) finally: # This removes challenges from self.achalls self._cleanup_challenges(active_achalls) - return failed_domains - def _send_responses(self, achalls, resps, chall_update): """Send responses and make sure errors are handled. @@ -172,7 +166,6 @@ class AuthHandler(object): """Wait for all challenge results to be determined.""" dom_to_check = set(chall_update.keys()) comp_domains = set() - failed_domains = set() rounds = 0 while dom_to_check and rounds < max_rounds: @@ -191,7 +184,10 @@ class AuthHandler(object): # We failed some challenges... damage control else: if best_effort: - failed_domains.add(domain) + comp_domains.add(domain) + logger.warning( + "Challenge failed for domain %s", + domain) else: all_failed_achalls.update( updated for _, updated in failed_achalls) @@ -200,12 +196,10 @@ class AuthHandler(object): _report_failed_challs(all_failed_achalls) raise errors.FailedChallenges(all_failed_achalls) - dom_to_check -= comp_domains.union(failed_domains) + dom_to_check -= comp_domains comp_domains.clear() rounds += 1 - return failed_domains - def _handle_check(self, domain, achalls): """Returns tuple of ('completed', 'failed').""" completed = [] diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 4a0c80725..2ccff232b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -694,7 +694,7 @@ def obtain_cert(config, plugins, lineage=None): if config.csr is not None: assert lineage is None, "Did not expect a CSR with a RenewableCert" csr, typ = config.actual_csr - certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ, authzr=False) + certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ) if config.dry_run: logger.info( "Dry run: skipping saving certificate to %s", config.cert_path) @@ -1621,10 +1621,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") helpful.add( - "automation", "--allow-subset-of-names", dest="allow_subset_of_names", - action="store_true", default=False, - help="Allow subsets of domain names in a single lineage to fail " - "validation without exiting.") + "automation", "--allow-subset-of-names", + action="store_true", + help="When performing domain validation, do not consider it a failure " + "if authorizations can not be obtained for a strict subset of " + "the requested domains. This option cannot be used with --csr.") helpful.add_group( "renew", description="The 'renew' subcommand will attempt to renew all" diff --git a/letsencrypt/client.py b/letsencrypt/client.py index f1a36197e..f2c3d1636 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -189,7 +189,7 @@ class Client(object): self.auth_handler = None def obtain_certificate_from_csr(self, domains, csr, - typ=OpenSSL.crypto.FILETYPE_ASN1, authzr=False): + typ=OpenSSL.crypto.FILETYPE_ASN1, authzr=None): """Obtain certificate. Internal function with precondition that `domains` are @@ -199,6 +199,8 @@ class Client(object): :param .le_util.CSR csr: DER-encoded Certificate Signing Request. The key used to generate this CSR can be different than `authkey`. + :param dict authzr: ACME Authorization Resource dict where keys are + domains and values are :class:`acme.messages.AuthorizationResource` :returns: `.CertificateResource` and certificate chain (as returned by `.fetch_chain`). @@ -215,10 +217,8 @@ class Client(object): logger.debug("CSR: %s, domains: %s", csr, domains) - if authzr is False: - authzr, _ = self.auth_handler.get_authorizations( - domains, - self.config.allow_subset_of_names) + if authzr is None: + authzr = self.auth_handler.get_authorizations(domains) certr = self.acme.request_issuance( jose.ComparableX509( @@ -240,15 +240,20 @@ class Client(object): :rtype: tuple """ - authzr, domains = self.auth_handler.get_authorizations(domains, - self.config.allow_subset_of_names) + authzr = self.auth_handler.get_authorizations( + domains, + self.config.allow_subset_of_names) + + domains = [a.body.identifier.value.encode('ascii', 'ignore') + for a in authzr] # Create CSR from names key = crypto_util.init_save_key( self.config.rsa_key_size, self.config.key_dir) csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) - return self.obtain_certificate_from_csr(domains, csr, authzr=authzr) + (key, csr) + return (self.obtain_certificate_from_csr(domains, csr, authzr=authzr) + + (key, csr)) def obtain_and_enroll_certificate(self, domains): """Obtain and enroll certificate. diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 24718fa82..61dd0e21b 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -87,7 +87,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr, _ = self.handler.get_authorizations(["0"]) + authzr = self.handler.get_authorizations(["0"]) self.assertEqual(self.mock_net.answer_challenge.call_count, 1) @@ -127,9 +127,7 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertTrue(achall.typ in ["tls-sni-01", "http-01", "dns"]) # Length of authorizations list - self.assertEqual(len(authzr[0]), 1) - # Length of valid domains list - self.assertEqual(len(authzr[1]), 1) + self.assertEqual(len(authzr), 1) @mock.patch("letsencrypt.auth_handler.AuthHandler._poll_challenges") def test_name3_tls_sni_01_3(self, mock_poll): @@ -138,7 +136,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all - authzr, _ = self.handler.get_authorizations(["0", "1", "2"]) + authzr = self.handler.get_authorizations(["0", "1", "2"]) self.assertEqual(self.mock_net.answer_challenge.call_count, 3) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 780c109c5..1f725a0a1 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -124,7 +124,7 @@ class ClientTest(unittest.TestCase): self.eg_domains, self.config.allow_subset_of_names) - authzr, _ = self.client.auth_handler.get_authorizations() + authzr = self.client.auth_handler.get_authorizations() self.acme.request_issuance.assert_called_once_with( jose.ComparableX509(OpenSSL.crypto.load_certificate_request( @@ -158,7 +158,7 @@ class ClientTest(unittest.TestCase): self.assertRaises(errors.ConfigurationError, cli.HelpfulArgumentParser.handle_csr, mock_parser, mock_parsed_args) - authzr, _ = self.client.auth_handler.get_authorizations(self.eg_domains, False) + authzr = self.client.auth_handler.get_authorizations(self.eg_domains, False) self.assertEqual( (mock.sentinel.certr, mock.sentinel.chain), @@ -190,7 +190,17 @@ class ClientTest(unittest.TestCase): # return_value is essentially set to (None, None) in # _mock_obtain_certificate(), which breaks this test. # Thus fixed by the next line. - self.client.auth_handler.get_authorizations.return_value = (None, domains) + + authzr = [] + + for domain in domains: + authzr.append( + mock.MagicMock( + body=mock.MagicMock( + identifier=mock.MagicMock( + value=domain)))) + + self.client.auth_handler.get_authorizations.return_value = authzr self.assertEqual( self.client.obtain_certificate(domains), From 1fbfbda33c2ac04e848ffcf5611e58ec68cad1cf Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Mar 2016 15:43:40 -0700 Subject: [PATCH 40/56] Address review nits --- letsencrypt/main.py | 3 ++- letsencrypt/tests/display/util_test.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 264f7625e..d82122481 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -11,6 +11,8 @@ import traceback import OpenSSL import zope.component +from acme import jose + import letsencrypt from letsencrypt import account @@ -27,7 +29,6 @@ from letsencrypt import log from letsencrypt import reporter from letsencrypt import storage -from acme import jose from letsencrypt.cli import choose_configurator_plugins, _renewal_conf_files, should_renew from letsencrypt.display import util as display_util, ops as display_ops from letsencrypt.plugins import disco as plugins_disco diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index 3f8ee8bb5..a16eb544e 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -8,6 +8,7 @@ import letsencrypt.errors as errors from letsencrypt.display import util as display_util + CHOICES = [("First", "Description1"), ("Second", "Description2")] TAGS = ["tag1", "tag2", "tag3"] TAGS_CHOICES = [("1", "tag1"), ("2", "tag2"), ("3", "tag3")] From 5292e3963bc6e113c040bce6dae937bda2e5fae9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 17 Mar 2016 16:01:40 -0700 Subject: [PATCH 41/56] Use urn:acme:error:invalidEmail when checking for e-mail errors --- letsencrypt/client.py | 3 +-- letsencrypt/tests/client_test.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 58e9c8e9f..3d611f856 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -146,8 +146,7 @@ def perform_registration(acme, config): try: return acme.register(messages.NewRegistration.from_data(email=config.email)) except messages.Error as e: - err = repr(e) - if "MX record" in err or "Validation of contact mailto" in err: + if e.typ == "urn:acme:error:invalidEmail": config.namespace.email = display_ops.get_email(more=True, invalid=True) return perform_registration(acme, config) else: diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 7dd513e18..1395a4f8f 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -63,8 +63,8 @@ class RegisterTest(unittest.TestCase): @mock.patch("letsencrypt.client.display_ops.get_email") def test_email_retry(self, _rep, mock_get_email): from acme import messages - msg = "Validation of contact mailto:sousaphone@improbablylongggstring.tld failed" - mx_err = messages.Error(detail=msg, typ="malformed", title="title") + msg = "DNS problem: NXDOMAIN looking up MX for example.com" + mx_err = messages.Error(detail=msg, typ="urn:acme:error:invalidEmail") with mock.patch("letsencrypt.client.acme_client.Client") as mock_client: mock_client().register.side_effect = [mx_err, mock.MagicMock()] self._call() From 0847c73fd8c6626e3d64f99930cef934224a1649 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 17 Mar 2016 16:22:29 -0700 Subject: [PATCH 42/56] Fix dependency issue --- letsencrypt/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 9bbbe923a..0ba5d321e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -355,10 +355,11 @@ class HelpfulArgumentParser(object): def __init__(self, args, plugins, detect_defaults=False): from letsencrypt import main + from letsencrypt import renew self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert, "config_changes": main.config_changes, "run": main.run, "install": main.install, "plugins": main.plugins_cmd, - "renew": renew, "revoke": main.revoke, + "renew": renew.renew, "revoke": main.revoke, "rollback": main.rollback, "everything": main.run} # List of topics for which additional help can be provided From bbbfe80b94545bb366fef27eea53edcd665f0c0c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sun, 20 Mar 2016 21:57:52 +0200 Subject: [PATCH 43/56] Changed testdata directory names and TwoVhost80Test to a better fitting one --- .../tests/augeas_configurator_test.py | 2 +- .../letsencrypt_apache/tests/configurator_test.py | 6 +++--- .../letsencrypt_apache/tests/display_ops_test.py | 2 +- .../letsencrypt_apache/tests/parser_test.py | 2 +- .../apache2/apache2.conf | 0 .../apache2/conf-available/bad_conf_file.conf | 0 .../conf-available/other-vhosts-access-log.conf | 0 .../apache2/conf-available/security.conf | 0 .../apache2/conf-available/serve-cgi-bin.conf | 0 .../conf-enabled/other-vhosts-access-log.conf | 0 .../apache2/conf-enabled/security.conf | 0 .../apache2/conf-enabled/serve-cgi-bin.conf | 0 .../apache2/envvars | 0 .../apache2/mods-available/authz_svn.load | 0 .../apache2/mods-available/dav.load | 0 .../apache2/mods-available/dav_svn.conf | 0 .../apache2/mods-available/dav_svn.load | 0 .../apache2/mods-available/rewrite.load | 0 .../apache2/mods-available/ssl.conf | 0 .../apache2/mods-available/ssl.load | 0 .../apache2/mods-enabled/.gitignore | 0 .../apache2/mods-enabled/authz_svn.load | 0 .../apache2/mods-enabled/dav.load | 0 .../apache2/mods-enabled/dav_svn.conf | 0 .../apache2/mods-enabled/dav_svn.load | 0 .../apache2/ports.conf | 0 .../apache2/sites-available/000-default.conf | 0 .../sites-available/default-ssl-port-only.conf | 0 .../apache2/sites-available/default-ssl.conf | 0 .../sites-available/encryption-example.conf | 0 .../apache2/sites-available/letsencrypt.conf | 0 .../apache2/sites-available/mod_macro-example.conf | 0 .../apache2/sites-available/wildcard.conf | 0 .../apache2/sites-enabled/000-default.conf | 0 .../apache2/sites-enabled/encryption-example.conf | 0 .../apache2/sites-enabled/letsencrypt.conf | 0 .../apache2/sites-enabled/mod_macro-example.conf | 0 .../{two_vhost_80 => multiple_vhosts}/sites | 0 .../letsencrypt_apache/tests/util.py | 14 +++++++------- 39 files changed, 13 insertions(+), 13 deletions(-) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/apache2.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-available/bad_conf_file.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-available/other-vhosts-access-log.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-available/security.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-available/serve-cgi-bin.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-enabled/other-vhosts-access-log.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-enabled/security.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/conf-enabled/serve-cgi-bin.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/envvars (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/authz_svn.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/dav.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/dav_svn.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/dav_svn.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/rewrite.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/ssl.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-available/ssl.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-enabled/.gitignore (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-enabled/authz_svn.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-enabled/dav.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-enabled/dav_svn.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/mods-enabled/dav_svn.load (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/ports.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/000-default.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/default-ssl-port-only.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/default-ssl.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/encryption-example.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/letsencrypt.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/mod_macro-example.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-available/wildcard.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-enabled/000-default.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-enabled/encryption-example.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-enabled/letsencrypt.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/apache2/sites-enabled/mod_macro-example.conf (100%) rename letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/{two_vhost_80 => multiple_vhosts}/sites (100%) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py index b70e1c7f1..bf95f72ce 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py @@ -20,7 +20,7 @@ class AugeasConfiguratorTest(util.ApacheTest): self.config_path, self.vhost_path, self.config_dir, self.work_dir) self.vh_truth = util.get_vh_truth( - self.temp_dir, "debian_apache_2_4/two_vhost_80") + self.temp_dir, "debian_apache_2_4/multiple_vhosts") def tearDown(self): shutil.rmtree(self.config_dir) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 81e237be3..927d918ae 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -20,17 +20,17 @@ from letsencrypt_apache import obj from letsencrypt_apache.tests import util -class TwoVhost80Test(util.ApacheTest): +class MultipleVhostsTest(util.ApacheTest): """Test two standard well-configured HTTP vhosts.""" def setUp(self): # pylint: disable=arguments-differ - super(TwoVhost80Test, self).setUp() + super(MultipleVhostsTest, self).setUp() self.config = util.get_apache_configurator( self.config_path, self.vhost_path, self.config_dir, self.work_dir) self.config = self.mock_deploy_cert(self.config) self.vh_truth = util.get_vh_truth( - self.temp_dir, "debian_apache_2_4/two_vhost_80") + self.temp_dir, "debian_apache_2_4/multiple_vhosts") def mock_deploy_cert(self, config): """A test for a mock deploy cert""" diff --git a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py index f7fbac947..124ba2f17 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py @@ -20,7 +20,7 @@ class SelectVhostTest(unittest.TestCase): zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) self.base_dir = "/example_path" self.vhosts = util.get_vh_truth( - self.base_dir, "debian_apache_2_4/two_vhost_80") + self.base_dir, "debian_apache_2_4/multiple_vhosts") @classmethod def _call(cls, vhosts): diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index 2e6481aba..f4d4660c9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -193,7 +193,7 @@ class ParserInitTest(util.ApacheTest): "update_runtime_variables"): path = os.path.join( self.temp_dir, - "debian_apache_2_4/////two_vhost_80/../two_vhost_80/apache2") + "debian_apache_2_4/////multiple_vhosts/../multiple_vhosts/apache2") parser = ApacheParser(self.aug, path, "/dummy/vhostpath") diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/apache2.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/apache2.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/apache2.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/apache2.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/bad_conf_file.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/bad_conf_file.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/bad_conf_file.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/other-vhosts-access-log.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/other-vhosts-access-log.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/other-vhosts-access-log.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/other-vhosts-access-log.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/security.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/security.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/security.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/security.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/serve-cgi-bin.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/serve-cgi-bin.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-available/serve-cgi-bin.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-available/serve-cgi-bin.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-enabled/other-vhosts-access-log.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-enabled/other-vhosts-access-log.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-enabled/other-vhosts-access-log.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-enabled/other-vhosts-access-log.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-enabled/security.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-enabled/security.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-enabled/security.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-enabled/security.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-enabled/serve-cgi-bin.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-enabled/serve-cgi-bin.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/conf-enabled/serve-cgi-bin.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/conf-enabled/serve-cgi-bin.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/envvars b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/envvars similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/envvars rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/envvars diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/authz_svn.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/authz_svn.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/authz_svn.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/authz_svn.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/dav.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/dav.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/dav.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/dav.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/dav_svn.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/dav_svn.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/dav_svn.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/dav_svn.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/dav_svn.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/dav_svn.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/dav_svn.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/dav_svn.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/rewrite.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/rewrite.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/rewrite.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/rewrite.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/ssl.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/ssl.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/ssl.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/ssl.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/ssl.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/ssl.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-available/ssl.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-available/ssl.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/.gitignore b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/.gitignore similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/.gitignore rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/.gitignore diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/authz_svn.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/authz_svn.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/authz_svn.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/authz_svn.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/dav.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/dav.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/dav.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/dav.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/dav_svn.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/dav_svn.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/dav_svn.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/dav_svn.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/dav_svn.load b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/dav_svn.load similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/mods-enabled/dav_svn.load rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/mods-enabled/dav_svn.load diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/ports.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/ports.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/ports.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/ports.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/000-default.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/000-default.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/000-default.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/000-default.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl-port-only.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/default-ssl-port-only.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl-port-only.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/default-ssl-port-only.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/default-ssl.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/default-ssl.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/default-ssl.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/encryption-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/encryption-example.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/encryption-example.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/encryption-example.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/letsencrypt.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/letsencrypt.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/letsencrypt.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/letsencrypt.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/mod_macro-example.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/mod_macro-example.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/wildcard.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/wildcard.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/wildcard.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-available/wildcard.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/000-default.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/000-default.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/000-default.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/000-default.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/encryption-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/encryption-example.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/encryption-example.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/encryption-example.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/letsencrypt.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/letsencrypt.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/letsencrypt.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/letsencrypt.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/mod_macro-example.conf similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/apache2/sites-enabled/mod_macro-example.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/sites b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/sites similarity index 100% rename from letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/sites rename to letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/multiple_vhosts/sites diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index fb1e1442d..928084e3c 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -22,9 +22,9 @@ from letsencrypt_apache import obj class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods - def setUp(self, test_dir="debian_apache_2_4/two_vhost_80", - config_root="debian_apache_2_4/two_vhost_80/apache2", - vhost_root="debian_apache_2_4/two_vhost_80/apache2/sites-available"): + def setUp(self, test_dir="debian_apache_2_4/multiple_vhosts", + config_root="debian_apache_2_4/multiple_vhosts/apache2", + vhost_root="debian_apache_2_4/multiple_vhosts/apache2/sites-available"): # pylint: disable=arguments-differ super(ApacheTest, self).setUp() @@ -59,9 +59,9 @@ class ApacheTest(unittest.TestCase): # pylint: disable=too-few-public-methods class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods - def setUp(self, test_dir="debian_apache_2_4/two_vhost_80", - config_root="debian_apache_2_4/two_vhost_80/apache2", - vhost_root="debian_apache_2_4/two_vhost_80/apache2/sites-available"): + def setUp(self, test_dir="debian_apache_2_4/multiple_vhosts", + config_root="debian_apache_2_4/multiple_vhosts/apache2", + vhost_root="debian_apache_2_4/multiple_vhosts/apache2/sites-available"): super(ParserTest, self).setUp(test_dir, config_root, vhost_root) zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) @@ -116,7 +116,7 @@ def get_apache_configurator( def get_vh_truth(temp_dir, config_name): """Return the ground truth for the specified directory.""" - if config_name == "debian_apache_2_4/two_vhost_80": + if config_name == "debian_apache_2_4/multiple_vhosts": prefix = os.path.join( temp_dir, config_name, "apache2/sites-available") aug_pre = "/files" + prefix From 6f25005559ce104a72ff2016438ae57ab249048a Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Mon, 21 Mar 2016 01:12:59 +0200 Subject: [PATCH 44/56] Adding tests --- letsencrypt/auth_handler.py | 7 +++---- letsencrypt/cli.py | 3 +++ letsencrypt/tests/auth_handler_test.py | 3 +++ letsencrypt/tests/cli_test.py | 4 ++++ 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 1d82705b7..284f9affd 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -66,7 +66,6 @@ class AuthHandler(object): self._choose_challenges(domains) - # While there are still challenges remaining... while self.achalls: resp = self._solve_challenges() @@ -80,11 +79,11 @@ class AuthHandler(object): # Only return valid authorizations retVal = [authzr for authzr in self.authzr.values() - if authzr.body.status == messages.STATUS_VALID] + if authzr.body.status == messages.STATUS_VALID] if len(retVal) <= 0: - logger.critical("Challenges failed for all domains") - raise + raise errors.AuthorizationError( + "Challenges failed for all domains") return retVal diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 2ccff232b..4b3b3860b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -1244,6 +1244,9 @@ class HelpfulArgumentParser(object): parsed_args.register_unsafely_without_email = True if parsed_args.csr: + if parsed_args.allow_subset_of_names: + raise errors.Error("--allow-subset-of-names " + "cannot be used with --csr") self.handle_csr(parsed_args) if self.detect_defaults: # plumbing diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 61dd0e21b..b7ac04984 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -163,6 +163,9 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertRaises( errors.AuthorizationError, self.handler.get_authorizations, ["0"]) + def test_no_domains(self): + self.assertRaises(errors.AuthorizationError, self.handler.get_authorizations, []) + def _validate_all(self, unused_1, unused_2): for dom in self.handler.authzr.keys(): azr = self.handler.authzr[dom] diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index bfd3818c1..feb70c2e9 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -360,6 +360,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call, ['-d', '204.11.231.35']) + def test_csr_with_besteffort(self): + args = ["--csr", CSR, "--allow-subset-of-names"] + self.assertRaises(errors.Error, self._call, args) + def test_run_with_csr(self): # This is an error because you can only use --csr with certonly try: From a8350ce04a2d1e5b86f0a1931bee9cb5b297abe5 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Mon, 21 Mar 2016 23:43:38 +0200 Subject: [PATCH 45/56] Refining code and documentation --- letsencrypt/auth_handler.py | 7 +++---- letsencrypt/client.py | 6 +++--- letsencrypt/tests/client_test.py | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 284f9affd..658315597 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -52,9 +52,8 @@ class AuthHandler(object): :param bool best_effort: Whether or not all authorizations are required (this is useful in renewal) - :returns: tuple of lists of authorization resources. Takes the - form of (`completed`, `failed`) - :rtype: tuple + :returns: List of authorization resources + :rtype: list :raises .AuthorizationError: If unable to retrieve all authorizations @@ -81,7 +80,7 @@ class AuthHandler(object): retVal = [authzr for authzr in self.authzr.values() if authzr.body.status == messages.STATUS_VALID] - if len(retVal) <= 0: + if not retVal: raise errors.AuthorizationError( "Challenges failed for all domains") diff --git a/letsencrypt/client.py b/letsencrypt/client.py index f2c3d1636..30df6147f 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -199,7 +199,7 @@ class Client(object): :param .le_util.CSR csr: DER-encoded Certificate Signing Request. The key used to generate this CSR can be different than `authkey`. - :param dict authzr: ACME Authorization Resource dict where keys are + :param list authzr: ACME Authorization Resource dict where keys are domains and values are :class:`acme.messages.AuthorizationResource` :returns: `.CertificateResource` and certificate chain (as @@ -244,8 +244,8 @@ class Client(object): domains, self.config.allow_subset_of_names) - domains = [a.body.identifier.value.encode('ascii', 'ignore') - for a in authzr] + domains = [a.body.identifier.value.encode('ascii') + for a in authzr] # Create CSR from names key = crypto_util.init_save_key( diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 1f725a0a1..33e0f1a11 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -115,7 +115,7 @@ class ClientTest(unittest.TestCase): def _mock_obtain_certificate(self): self.client.auth_handler = mock.MagicMock() - self.client.auth_handler.get_authorizations.return_value = (None, None) + self.client.auth_handler.get_authorizations.return_value = [None] self.acme.request_issuance.return_value = mock.sentinel.certr self.acme.fetch_chain.return_value = mock.sentinel.chain From 838eb28d0a2f412249df86d2fcf25a72b4a82169 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Tue, 22 Mar 2016 00:00:51 +0200 Subject: [PATCH 46/56] Adding test for obtain_certificate_from_csr with authzr=None --- letsencrypt/tests/client_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 33e0f1a11..f1bc3de25 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -169,6 +169,17 @@ class ClientTest(unittest.TestCase): # and that the cert was obtained correctly self._check_obtain_certificate() + # Test for authzr=None + self.assertEqual( + (mock.sentinel.certr, mock.sentinel.chain), + self.client.obtain_certificate_from_csr( + self.eg_domains, + test_csr, + authzr=None)) + + self.client.auth_handler.get_authorizations.assert_called_with( + self.eg_domains) + # Test for no auth_handler self.client.auth_handler = None self.assertRaises( From dc564efd0cabe6d3e7f4cb5553fb445a9a1213d0 Mon Sep 17 00:00:00 2001 From: Amjad Mashaal Date: Tue, 22 Mar 2016 00:38:59 +0200 Subject: [PATCH 47/56] Refining obtain_certificate_from_csr docstring --- letsencrypt/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 30df6147f..1345a7f23 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -199,8 +199,8 @@ class Client(object): :param .le_util.CSR csr: DER-encoded Certificate Signing Request. The key used to generate this CSR can be different than `authkey`. - :param list authzr: ACME Authorization Resource dict where keys are - domains and values are :class:`acme.messages.AuthorizationResource` + :param list authzr: List of + :class:`acme.messages.AuthorizationResource` :returns: `.CertificateResource` and certificate chain (as returned by `.fetch_chain`). From 2d211c7aef4951e9147d35369e96b2e522c9f233 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 22 Mar 2016 12:44:40 -0700 Subject: [PATCH 48/56] Mention that we're talking about Unix OSes --- README.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 874b0c450..75de094a8 100644 --- a/README.rst +++ b/README.rst @@ -23,11 +23,11 @@ configuring webservers to use them. Installation ------------ -If ``letsencrypt`` is packaged for your OS, you can install it from there, and -run it by typing ``letsencrypt``. Because not all operating systems have -packages yet, we provide a temporary solution via the ``letsencrypt-auto`` -wrapper script, which obtains some dependencies from your OS and puts others -in a python virtual environment:: +If ``letsencrypt`` is packaged for your Unix OS, you can install it from +there, and run it by typing ``letsencrypt``. Because not all operating +systems have packages yet, we provide a temporary solution via the +``letsencrypt-auto`` wrapper script, which obtains some dependencies +from your OS and puts others in a python virtual environment:: user@webserver:~$ git clone https://github.com/letsencrypt/letsencrypt user@webserver:~$ cd letsencrypt From 46ef3e6374358dc65c0ee07fccc82332b7af5a20 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Tue, 22 Mar 2016 12:49:39 -0700 Subject: [PATCH 49/56] More explicit --- README.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 75de094a8..050cde82b 100644 --- a/README.rst +++ b/README.rst @@ -18,7 +18,8 @@ The Let's Encrypt Client is a fully-featured, extensible client for the Let's Encrypt CA (or any other CA that speaks the `ACME `_ protocol) that can automate the tasks of obtaining certificates and -configuring webservers to use them. +configuring webservers to use them. This client runs on Unix-based operating +systems. Installation ------------ From 278b8852fd3844f66af03d3ee6586ac10aee6c32 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Mar 2016 14:26:58 -0700 Subject: [PATCH 50/56] Merge snafu --- letsencrypt/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index adbe72701..55a504f82 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -54,7 +54,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods shutil.rmtree(self.tmp_dir) # Reset globals in cli # pylint: disable=protected-access - cli._parser = cli._set_by_cli.detector = None + cli._parser = cli.set_by_cli.detector = None def _call(self, args): "Run the cli with output streams and actual client mocked out" From 4bc8e9a44ac7b7dcd9a96eee0a34203850d6e1ea Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 23 Mar 2016 14:30:40 -0700 Subject: [PATCH 51/56] Improve mocking --- letsencrypt/tests/storage_test.py | 42 ++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index 0d89156d3..9660e2688 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -493,7 +493,12 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertTrue(self.test_rc.should_autorenew()) mock_ocsp.return_value = False - def test_save_successor(self): + @mock.patch("letsencrypt.storage.relevant_values") + def test_save_successor(self, mock_rv): + # Mock relevant_values() to claim that all values are relevant here + # (to avoid instantiating parser) + mock_rv.side_effect = lambda x: x + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) @@ -557,24 +562,44 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.exists(temp_config_file)) - def test_relevant_values(self): + @mock.patch("letsencrypt.cli._parser") + def test_relevant_values(self, mock_parser): """Test that relevant_values() can reject an irrelevant value.""" from letsencrypt import storage + mock_parser.verb = "certonly" + mock_parser.args = ["--standalone"] + mock_action = mock.Mock(dest="rsa_key_size", default=2048) + mock_parser.parser._actions = [mock_action] self.assertEqual(storage.relevant_values({"hello": "there"}), {}) - def test_relevant_values_default(self): + @mock.patch("letsencrypt.cli._parser") + def test_relevant_values_default(self, mock_parser): """Test that relevant_values() can reject a default value.""" from letsencrypt import storage + mock_parser.verb = "certonly" + mock_parser.args = ["--standalone"] + mock_action = mock.Mock(dest="rsa_key_size", default=2048) + mock_parser.parser._actions = [mock_action] self.assertEqual(storage.relevant_values({"rsa_key_size": 2048}), {}) - def test_relevant_values_nondefault(self): + @mock.patch("letsencrypt.cli._parser") + def test_relevant_values_nondefault(self, mock_parser): """Test that relevant_values() can retain a non-default value.""" from letsencrypt import storage + mock_parser.verb = "certonly" + mock_parser.args = ["--standalone"] + mock_action = mock.Mock(dest="rsa_key_size", default=2048) + mock_parser.parser._actions = [mock_action] self.assertEqual(storage.relevant_values({"rsa_key_size": 12}), {"rsa_key_size": 12}) - def test_new_lineage(self): + @mock.patch("letsencrypt.storage.relevant_values") + def test_new_lineage(self, mock_rv): """Test for new_lineage() class method.""" + # Mock relevant_values to say everything is relevant here (so we + # don't have to mock the parser to help it decide!) + mock_rv.side_effect = lambda x: x + from letsencrypt import storage result = storage.RenewableCert.new_lineage( "the-lineage.com", "cert", "privkey", "chain", self.cli_config) @@ -608,8 +633,13 @@ class RenewableCertTests(BaseRenewableCertTest): # TODO: Conceivably we could test that the renewal parameters actually # got saved - def test_new_lineage_nonexistent_dirs(self): + @mock.patch("letsencrypt.storage.relevant_values") + def test_new_lineage_nonexistent_dirs(self, mock_rv): """Test that directories can be created if they don't exist.""" + # Mock relevant_values to say everything is relevant here (so we + # don't have to mock the parser to help it decide!) + mock_rv.side_effect = lambda x: x + from letsencrypt import storage shutil.rmtree(self.cli_config.renewal_configs_dir) shutil.rmtree(self.cli_config.archive_dir) From 4f7c5b32e87809e650ff05d9a326295283fcad71 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 23 Mar 2016 14:57:01 -0700 Subject: [PATCH 52/56] Cleanup responding to protected-access problems --- letsencrypt/cli.py | 18 +++++++++--------- letsencrypt/plugins/common.py | 2 +- letsencrypt/storage.py | 6 +++--- letsencrypt/tests/cli_test.py | 4 ++-- letsencrypt/tests/storage_test.py | 3 +++ 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index fccebe02f..fa1e0090e 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -271,20 +271,20 @@ def record_chosen_plugins(config, plugins, auth, inst): cn.installer = plugins.find_init(inst).name if inst else "none" -def _set_by_cli(var): +def set_by_cli(var): """ Return True if a particular config variable has been set by the user (CLI or config file) including if the user explicitly set it to the default. Returns False if the variable was assigned a default value. """ - detector = _set_by_cli.detector + detector = set_by_cli.detector if detector is None: # Setup on first run: `detector` is a weird version of config in which # the default value of every attribute is wrangled to be boolean-false plugins = plugins_disco.PluginsRegistry.find_all() # reconstructed_args == sys.argv[1:], or whatever was passed to main() reconstructed_args = _parser.args + [_parser.verb] - detector = _set_by_cli.detector = prepare_and_parse_args( + detector = set_by_cli.detector = prepare_and_parse_args( plugins, reconstructed_args, detect_defaults=True) # propagate plugin requests: eg --standalone modifies config.authenticator auth, inst = cli_plugin_requests(detector) @@ -312,7 +312,7 @@ def _set_by_cli(var): else: return False # static housekeeping var -_set_by_cli.detector = None +set_by_cli.detector = None def _restore_required_config_elements(config, renewalparams): """Sets non-plugin specific values in config from renewalparams @@ -325,7 +325,7 @@ def _restore_required_config_elements(config, renewalparams): """ # string-valued items to add if they're present for config_item in STR_CONFIG_ITEMS: - if config_item in renewalparams and not _set_by_cli(config_item): + if config_item in renewalparams and not set_by_cli(config_item): value = renewalparams[config_item] # Unfortunately, we've lost type information from ConfigObj, # so we don't know if the original was NoneType or str! @@ -334,7 +334,7 @@ def _restore_required_config_elements(config, renewalparams): setattr(config.namespace, config_item, value) # int-valued items to add if they're present for config_item in INT_CONFIG_ITEMS: - if config_item in renewalparams and not _set_by_cli(config_item): + if config_item in renewalparams and not set_by_cli(config_item): config_value = renewalparams[config_item] # the default value for http01_port was None during private beta if config_item == "http01_port" and config_value == "None": @@ -378,7 +378,7 @@ def _restore_plugin_configs(config, renewalparams): plugin_prefixes.append(renewalparams["installer"]) for plugin_prefix in set(plugin_prefixes): for config_item, config_value in six.iteritems(renewalparams): - if config_item.startswith(plugin_prefix + "_") and not _set_by_cli(config_item): + if config_item.startswith(plugin_prefix + "_") and not set_by_cli(config_item): # Values None, True, and False need to be treated specially, # As they don't get parsed correctly based on type if config_value in ("None", "True", "False"): @@ -403,7 +403,7 @@ def _restore_webroot_config(config, renewalparams): if "webroot_map" in renewalparams: # if the user does anything that would create a new webroot map on the # CLI, don't use the old one - if not (_set_by_cli("webroot_map") or _set_by_cli("webroot_path")): + if not (set_by_cli("webroot_map") or set_by_cli("webroot_path")): setattr(config.namespace, "webroot_map", renewalparams["webroot_map"]) elif "webroot_path" in renewalparams: logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path") @@ -844,7 +844,7 @@ class HelpfulArgumentParser(object): def modify_arg_for_default_detection(self, *args, **kwargs): """ Adding an arg, but ensure that it has a default that evaluates to false, - so that _set_by_cli can tell if it was set. Only called if detect_defaults==True. + so that set_by_cli can tell if it was set. Only called if detect_defaults==True. :param list *args: the names of this argument flag :param dict **kwargs: various argparse settings for this argument diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index 319692344..f6a2c3d76 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -52,7 +52,7 @@ class Plugin(object): NOTE: if you add argpase arguments such that users setting them can create a config entry that python's bool() would consider false (ie, the use might set the variable to "", [], 0, etc), please ensure that - cli._set_by_cli() works for your variable. + cli.set_by_cli() works for your variable. """ diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 5e5d70518..3807cb63d 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -152,7 +152,7 @@ def relevant_values(all_values): # Look through the CLI parser defaults and see if this option is # both present and equal to the specified value. If not, return # False. - for x in cli._parser.parser._actions: + for x in cli._parser.parser._actions: # pylint: disable=protected-access if x.dest == option: if x.default == value: return True @@ -164,10 +164,10 @@ def relevant_values(all_values): for option, value in all_values.iteritems(): # Try to find reasons to store this item in the # renewal config. It can be stored if it is relevant and - # (it is _set_by_cli() or flag_default() is different + # (it is set_by_cli() or flag_default() is different # from the value or flag_default() doesn't exist). if _relevant(option): - if (cli._set_by_cli(option) + if (cli.set_by_cli(option) or not _is_cli_default(option, value)): # or option not in constants.CLI_DEFAULTS # or constants.CLI_DEFAULTS[option] != value): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 920bc2d99..74229cb6b 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -54,7 +54,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods shutil.rmtree(self.tmp_dir) # Reset globals in cli # pylint: disable=protected-access - cli._parser = cli._set_by_cli.detector = None + cli._parser = cli.set_by_cli.detector = None def _call(self, args): "Run the cli with output streams and actual client mocked out" @@ -660,7 +660,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) - @mock.patch("letsencrypt.cli._set_by_cli") + @mock.patch("letsencrypt.cli.set_by_cli") def test_ancient_webroot_renewal_conf(self, mock_set_by_cli): mock_set_by_cli.return_value = False rc_path = self._make_test_renewal_conf('sample-renewal-ancient.conf') diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index 9660e2688..0d007a831 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -565,6 +565,7 @@ class RenewableCertTests(BaseRenewableCertTest): @mock.patch("letsencrypt.cli._parser") def test_relevant_values(self, mock_parser): """Test that relevant_values() can reject an irrelevant value.""" + # pylint: disable=protected-access from letsencrypt import storage mock_parser.verb = "certonly" mock_parser.args = ["--standalone"] @@ -575,6 +576,7 @@ class RenewableCertTests(BaseRenewableCertTest): @mock.patch("letsencrypt.cli._parser") def test_relevant_values_default(self, mock_parser): """Test that relevant_values() can reject a default value.""" + # pylint: disable=protected-access from letsencrypt import storage mock_parser.verb = "certonly" mock_parser.args = ["--standalone"] @@ -585,6 +587,7 @@ class RenewableCertTests(BaseRenewableCertTest): @mock.patch("letsencrypt.cli._parser") def test_relevant_values_nondefault(self, mock_parser): """Test that relevant_values() can retain a non-default value.""" + # pylint: disable=protected-access from letsencrypt import storage mock_parser.verb = "certonly" mock_parser.args = ["--standalone"] From a9faa3b4ba9b3353e3eb66dda48e303dc881312c Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 23 Mar 2016 16:14:41 -0700 Subject: [PATCH 53/56] Shim renew() in main.py, keep the work in renewal.py --- letsencrypt/cli.py | 3 +-- letsencrypt/main.py | 11 ++++++++--- letsencrypt/{renew.py => renewal.py} | 4 ++-- letsencrypt/tests/cli_test.py | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-) rename letsencrypt/{renew.py => renewal.py} (99%) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 933b51432..bc39c3a8d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -355,11 +355,10 @@ class HelpfulArgumentParser(object): def __init__(self, args, plugins, detect_defaults=False): from letsencrypt import main - from letsencrypt import renew self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert, "config_changes": main.config_changes, "run": main.run, "install": main.install, "plugins": main.plugins_cmd, - "renew": renew.renew, "revoke": main.revoke, + "renew": main.renew, "revoke": main.revoke, "rollback": main.rollback, "everything": main.run} # List of topics for which additional help can be provided diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 90dfb456a..a3ebde64e 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -27,7 +27,7 @@ from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import log from letsencrypt import reporter -from letsencrypt import renew +from letsencrypt import renewal from letsencrypt import storage from letsencrypt.display import util as display_util, ops as display_ops @@ -186,7 +186,7 @@ def _handle_identical_cert_request(config, cert): :rtype: tuple """ - if renew.should_renew(config, cert): + if renewal.should_renew(config, cert): return "renew", cert if config.reinstall: # Set with --reinstall, force an identical certificate to be @@ -263,7 +263,7 @@ def _find_duplicative_certs(config, domains): # Verify the directory is there le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) - for renewal_file in renew.renewal_conf_files(cli_config): + for renewal_file in renewal.renewal_conf_files(cli_config): try: candidate_lineage = storage.RenewableCert(renewal_file, cli_config) except (errors.CertStorageError, IOError): @@ -552,6 +552,11 @@ def obtain_cert(config, plugins, lineage=None): config.installer, "server; fullchain is", lineage.fullchain) _suggest_donation_if_appropriate(config, action) +def renew(config, unused_plugins): + """Renew previously-obtained certificates.""" + renewal.renew_all_lineages(config) + + def setup_log_file_handler(config, logfile, fmt): """Setup file debug logging.""" diff --git a/letsencrypt/renew.py b/letsencrypt/renewal.py similarity index 99% rename from letsencrypt/renew.py rename to letsencrypt/renewal.py index 9dc1ff4df..27546bec9 100644 --- a/letsencrypt/renew.py +++ b/letsencrypt/renewal.py @@ -236,8 +236,8 @@ def _renew_describe_results(config, renew_successes, renew_failures, print("** (The test certificates above have not been saved.)") -def renew(config, unused_plugins): - """Renew previously-obtained certificates.""" +def renew_all_lineages(config): + """Examine each lineage; renew if due and report results""" if config.domains != []: raise errors.Error("Currently, the renew verb is only capable of " diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 55a504f82..0c2bf5d54 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -23,7 +23,7 @@ from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util from letsencrypt import main -from letsencrypt import renew +from letsencrypt import renewal from letsencrypt import storage from letsencrypt.plugins import disco @@ -666,7 +666,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods configuration.RenewerConfiguration(config)) renewalparams = lineage.configuration["renewalparams"] # pylint: disable=protected-access - renew._restore_webroot_config(config, renewalparams) + renewal._restore_webroot_config(config, renewalparams) self.assertEqual(config.webroot_path, ["/var/www/"]) def test_renew_verb_empty_config(self): @@ -745,7 +745,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods def test_renew_reconstitute_error(self): # pylint: disable=protected-access - with mock.patch('letsencrypt.main.renew._reconstitute') as mock_reconstitute: + with mock.patch('letsencrypt.main.renewal._reconstitute') as mock_reconstitute: mock_reconstitute.side_effect = Exception self._test_renew_common(assert_oc_called=False, error_expected=True) From f3362d99784eb865c9cef7612566db8a73222fb0 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 23 Mar 2016 17:41:56 -0700 Subject: [PATCH 54/56] Disabling a protected access complaint --- letsencrypt/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 838ec868f..da5e5f665 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -148,7 +148,7 @@ def relevant_values(all_values): from letsencrypt import cli - def _is_cli_default(option, value): + def _is_cli_default(option, value): # pylint: disable=protected-access # Look through the CLI parser defaults and see if this option is # both present and equal to the specified value. If not, return # False. From a9e0b0f6ab07e5b5272d16768448e3e390333729 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 24 Mar 2016 12:02:11 -0700 Subject: [PATCH 55/56] Another try on protected access lint error --- letsencrypt/storage.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index da5e5f665..59daa1a0d 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -148,10 +148,11 @@ def relevant_values(all_values): from letsencrypt import cli - def _is_cli_default(option, value): # pylint: disable=protected-access + def _is_cli_default(option, value): # Look through the CLI parser defaults and see if this option is # both present and equal to the specified value. If not, return # False. + # pylint: disable=protected-access for x in cli.helpful_parser.parser._actions: if x.dest == option: if x.default == value: From f3f665bf63f256c66ea1f3b1b03a665be6310d0e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 24 Mar 2016 12:35:52 -0700 Subject: [PATCH 56/56] s/none/None --- letsencrypt/cli.py | 4 ++-- letsencrypt/main.py | 2 +- letsencrypt/tests/testdata/sample-renewal-ancient.conf | 2 +- letsencrypt/tests/testdata/sample-renewal.conf | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e10a531ab..1af4ef898 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -236,8 +236,8 @@ def choose_configurator_plugins(config, plugins, verb): def record_chosen_plugins(config, plugins, auth, inst): "Update the config entries to reflect the plugins we actually selected." cn = config.namespace - cn.authenticator = plugins.find_init(auth).name if auth else "none" - cn.installer = plugins.find_init(inst).name if inst else "none" + cn.authenticator = plugins.find_init(auth).name if auth else "None" + cn.installer = plugins.find_init(inst).name if inst else "None" def set_by_cli(var): diff --git a/letsencrypt/main.py b/letsencrypt/main.py index a3ebde64e..fc6540dcf 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -462,7 +462,7 @@ def config_changes(config, unused_plugins): def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" # For user-agent construction - config.namespace.installer = config.namespace.authenticator = "none" + config.namespace.installer = config.namespace.authenticator = "None" if config.key_path is not None: # revocation by cert key logger.debug("Revoking %s using cert key %s", config.cert_path[0], config.key_path[0]) diff --git a/letsencrypt/tests/testdata/sample-renewal-ancient.conf b/letsencrypt/tests/testdata/sample-renewal-ancient.conf index ff246ba7c..dd3075b8e 100644 --- a/letsencrypt/tests/testdata/sample-renewal-ancient.conf +++ b/letsencrypt/tests/testdata/sample-renewal-ancient.conf @@ -14,7 +14,7 @@ apache_dismod = a2dismod register_unsafely_without_email = False apache_handle_modules = True uir = None -installer = none +installer = None nginx_ctl = nginx config_dir = MAGICDIR text_mode = False diff --git a/letsencrypt/tests/testdata/sample-renewal.conf b/letsencrypt/tests/testdata/sample-renewal.conf index d6ebbd845..08032af86 100644 --- a/letsencrypt/tests/testdata/sample-renewal.conf +++ b/letsencrypt/tests/testdata/sample-renewal.conf @@ -14,7 +14,7 @@ apache_dismod = a2dismod register_unsafely_without_email = False apache_handle_modules = True uir = None -installer = none +installer = None nginx_ctl = nginx config_dir = MAGICDIR text_mode = False