From 512e02c837347f7ef821925ee61452645c97adaf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Jun 2015 11:46:39 -0700 Subject: [PATCH 1/4] Added reporter messages for failed challenges. --- letsencrypt/auth_handler.py | 87 +++++++++++++++++++++++++++++++++++++ letsencrypt/reporter.py | 23 +++++++--- 2 files changed, 103 insertions(+), 7 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index d7d590878..502b0b76d 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -3,12 +3,15 @@ import itertools import logging import time +import zope.component + from acme import challenges from acme import messages from letsencrypt import achallenges from letsencrypt import constants from letsencrypt import errors +from letsencrypt import interfaces class AuthHandler(object): @@ -193,6 +196,7 @@ class AuthHandler(object): updated for _, updated in failed_achalls) if all_failed_achalls: + _report_failed_challs(all_failed_achalls) raise errors.FailedChallenges(all_failed_achalls) dom_to_check -= comp_domains @@ -480,3 +484,86 @@ def is_preferred(offered_challb, satisfied, different=True): return False return True + + +_ERROR_HELP_COMMON = ( + 'To fix these errors, please make sure that your domain name was entered ' + 'correctly and the DNS A/AAAA record(s) for that domain contains the ' + 'right IP address.' +) + + +_ERROR_HELP = { + 'connection' : + _ERROR_HELP_COMMON + ' Additionally, please check that your computer ' + 'has publicly routable IP address and no firewalls are preventing the ' + 'server from communicating with the client.', + 'dnssec' : + _ERROR_HELP_COMMON + ' Additionally, if you have DNSSEC enabled for ' + 'your domain, please ensure the signature is valid.', + 'malformed' : + 'To fix these errors, please make sure that you did not provide any ' + 'invalid information to the client and try running Let\'s Encrypt ' + 'again.', + 'serverInternal' : + 'Unfortunately, an error on the ACME server prevented you from completing ' + 'authorization. Please try again later.', + 'tls' : + _ERROR_HELP_COMMON + ' Additionally, please check that you have an up ' + 'to date TLS configuration that allows the server to communicate with ' + 'the Let\'s Encrypt client.', + 'unauthorized' : _ERROR_HELP_COMMON, + 'unknownHost' : + 'To fix these errors, please make sure that your domain name was ' + 'entered correctly.', +} + + +def _report_failed_challs(failed_achalls): + """Notifies the user about failed challenges. + + :param set failed_achalls: A set of failed + :class:`letsencrypt.achallenges.AnnotatedChallenge`. + + """ + problems = dict() + for achall in failed_achalls: + if achall.error: + problems.setdefault(achall.error.typ, []).append(achall) + + reporter = zope.component.getUtility(interfaces.IReporter) + for achalls in problems.itervalues(): + reporter.add_message(_generate_failed_chall_msg(achalls), 1, True) + + +def _generate_failed_chall_msg(failed_achalls): + """Creates a user friendly error message about failed challenges. + + :param list failed_achalls: A list of failed + :class:`letsencrypt.achallenges.AnnotatedChallenge` with the same error + type. + + :returns: A formatted error message for the client. + :rtype: str + + """ + typ = failed_achalls[0].error.typ + msg = [ + 'The following \'{0}\' errors were reported by the server:'.format(typ) + ] + + problems = dict() + for achall in failed_achalls: + problems.setdefault(achall.error.description, []).append(achall.domain) + for problem in problems: + domains = problems[problem] + domains.sort() + msg.append('\n\nDomains: ') + msg.append(', '.join(domains)) + msg.append('\nError: {0}'.format(problem)) + + if typ in _ERROR_HELP: + msg.append('\n\n') + msg.append(_ERROR_HELP[typ]) + + return "".join(msg) diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 045c1befa..3045a7e19 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -46,9 +46,10 @@ class Reporter(object): printed if the program exits abnormally. """ - assert self.HIGH_PRIORITY <= priority <= self.LOW_PRIORITY - self.messages.put(self._msg_type(priority, msg, on_crash)) - logging.info("Reporting to user: %s", msg) + if msg: + assert self.HIGH_PRIORITY <= priority <= self.LOW_PRIORITY + self.messages.put(self._msg_type(priority, msg, on_crash)) + logging.info("Reporting to user: %s", msg) def atexit_print_messages(self, pid=os.getpid()): """Function to be registered with atexit to print messages. @@ -66,7 +67,8 @@ class Reporter(object): If there is an unhandled exception, only messages for which ``on_crash`` is ``True`` are printed. -""" + + """ bold_on = False if not self.messages.empty(): no_exception = sys.exc_info()[0] is None @@ -74,14 +76,21 @@ class Reporter(object): if bold_on: print self._BOLD print 'IMPORTANT NOTES:' - wrapper = textwrap.TextWrapper(initial_indent=' - ', - subsequent_indent=(' ' * 3)) + first_wrapper = textwrap.TextWrapper( + initial_indent=' - ', subsequent_indent=(' ' * 3)) + next_wrapper = textwrap.TextWrapper( + initial_indent=first_wrapper.subsequent_indent, + subsequent_indent=first_wrapper.subsequent_indent) while not self.messages.empty(): msg = self.messages.get() if no_exception or msg.on_crash: if bold_on and msg.priority > self.HIGH_PRIORITY: sys.stdout.write(self._RESET) bold_on = False - print wrapper.fill(msg.text) + lines = msg.text.splitlines() + print first_wrapper.fill(lines[0]) + if len(lines) > 1: + print "\n".join( + next_wrapper.fill(line) for line in lines[1:]) if bold_on: sys.stdout.write(self._RESET) From 8f760cf828c0b1038313ecdc3483014f2a0107a9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Jun 2015 11:51:50 -0700 Subject: [PATCH 2/4] Cleaned up multiline statements --- letsencrypt/auth_handler.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 502b0b76d..4a685439f 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -489,8 +489,7 @@ def is_preferred(offered_challb, satisfied, _ERROR_HELP_COMMON = ( 'To fix these errors, please make sure that your domain name was entered ' 'correctly and the DNS A/AAAA record(s) for that domain contains the ' - 'right IP address.' -) + 'right IP address.') _ERROR_HELP = { @@ -515,8 +514,7 @@ _ERROR_HELP = { 'unauthorized' : _ERROR_HELP_COMMON, 'unknownHost' : 'To fix these errors, please make sure that your domain name was ' - 'entered correctly.', -} + 'entered correctly.',} def _report_failed_challs(failed_achalls): @@ -533,7 +531,8 @@ def _report_failed_challs(failed_achalls): reporter = zope.component.getUtility(interfaces.IReporter) for achalls in problems.itervalues(): - reporter.add_message(_generate_failed_chall_msg(achalls), 1, True) + reporter.add_message( + _generate_failed_chall_msg(achalls), reporter.MEDIUM_PRIORITY, True) def _generate_failed_chall_msg(failed_achalls): @@ -549,8 +548,7 @@ def _generate_failed_chall_msg(failed_achalls): """ typ = failed_achalls[0].error.typ msg = [ - 'The following \'{0}\' errors were reported by the server:'.format(typ) - ] + "The following '{0}' errors were reported by the server:".format(typ)] problems = dict() for achall in failed_achalls: From 9637142c4c5a19dad1722aa0274100a150a9d8a3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Jun 2015 15:27:34 -0700 Subject: [PATCH 3/4] Added auth_handler tests --- letsencrypt/auth_handler.py | 12 ++---- letsencrypt/tests/auth_handler_test.py | 51 +++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 4a685439f..019cb07dc 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -512,9 +512,7 @@ _ERROR_HELP = { 'to date TLS configuration that allows the server to communicate with ' 'the Let\'s Encrypt client.', 'unauthorized' : _ERROR_HELP_COMMON, - 'unknownHost' : - 'To fix these errors, please make sure that your domain name was ' - 'entered correctly.',} + 'unknownHost' : _ERROR_HELP_COMMON,} def _report_failed_challs(failed_achalls): @@ -552,16 +550,14 @@ def _generate_failed_chall_msg(failed_achalls): problems = dict() for achall in failed_achalls: - problems.setdefault(achall.error.description, []).append(achall.domain) + problems.setdefault(achall.error.description, set()).add(achall.domain) for problem in problems: - domains = problems[problem] - domains.sort() msg.append('\n\nDomains: ') - msg.append(', '.join(domains)) + msg.append(', '.join(sorted(problems[problem]))) msg.append('\nError: {0}'.format(problem)) if typ in _ERROR_HELP: msg.append('\n\n') msg.append(_ERROR_HELP[typ]) - return "".join(msg) + return ''.join(msg) diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 24bceb5f8..15ce6a490 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -216,7 +216,8 @@ class PollChallengesTest(unittest.TestCase): self.assertEqual(authzr.body.status, messages.STATUS_PENDING) @mock.patch("letsencrypt.auth_handler.time") - def test_poll_challenges_failure(self, unused_mock_time): + @mock.patch("letsencrypt.auth_handler.zope.component.getUtility") + def test_poll_challenges_failure(self, unused_mock_time, unused_mock_zope): self.mock_net.poll.side_effect = self._mock_poll_solve_one_invalid self.assertRaises( errors.AuthorizationError, self.handler._poll_challenges, @@ -420,6 +421,54 @@ class IsPreferredTest(unittest.TestCase): self._call(acme_util.DVSNI_P, frozenset([acme_util.DVSNI_P]))) +class ReportFailedChallsTest(unittest.TestCase): + """Tests for letsencrypt.auth_handler._report_failed_challs.""" + # pylint: disable=protected-access + + def setUp(self): + from letsencrypt import achallenges + + kwargs = { + "chall" : acme_util.SIMPLE_HTTP, + "uri": "uri", + "status": messages.STATUS_INVALID, + "error": messages.Error(typ="tls", detail="detail"), + } + + self.simple_http = achallenges.SimpleHTTP( + challb=messages.ChallengeBody(**kwargs),# pylint: disable=star-args + domain="example.com", + key=acme_util.KEY) + + kwargs["chall"] = acme_util.DVSNI + self.dvsni_same = achallenges.DVSNI( + challb=messages.ChallengeBody(**kwargs),# pylint: disable=star-args + domain="example.com", + key=acme_util.KEY) + + kwargs["error"] = messages.Error(typ="dnssec", detail="detail") + self.dvsni_diff = achallenges.DVSNI( + challb=messages.ChallengeBody(**kwargs),# pylint: disable=star-args + domain="foo.bar", + key=acme_util.KEY) + + @mock.patch("letsencrypt.auth_handler.zope.component.getUtility") + def test_same_error_and_domain(self, mock_zope): + from letsencrypt import auth_handler + + auth_handler._report_failed_challs([self.simple_http, self.dvsni_same]) + call_list = mock_zope().add_message.call_args_list + self.assertTrue(len(call_list) == 1) + self.assertIn("Domains: example.com\n", call_list[0][0][0]) + + @mock.patch("letsencrypt.auth_handler.zope.component.getUtility") + def test_different_errors_and_domains(self, mock_zope): + from letsencrypt import auth_handler + + auth_handler._report_failed_challs([self.simple_http, self.dvsni_diff]) + self.assertTrue(mock_zope().add_message.call_count == 2) + + def gen_auth_resp(chall_list): """Generate a dummy authorization response.""" return ["%s%s" % (chall.__class__.__name__, chall.domain) From d15a386f921bc4a75681c54c3a32c5b4cd20fad6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Jun 2015 18:24:54 -0700 Subject: [PATCH 4/4] Incorporated jdkasten's feedback --- letsencrypt/auth_handler.py | 56 +++++++++++++------------- letsencrypt/reporter.py | 7 ++-- letsencrypt/tests/auth_handler_test.py | 2 +- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 019cb07dc..43f7b9fd2 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -487,32 +487,32 @@ def is_preferred(offered_challb, satisfied, _ERROR_HELP_COMMON = ( - 'To fix these errors, please make sure that your domain name was entered ' - 'correctly and the DNS A/AAAA record(s) for that domain contains the ' - 'right IP address.') + "To fix these errors, please make sure that your domain name was entered " + "correctly and the DNS A/AAAA record(s) for that domain contains the " + "right IP address.") _ERROR_HELP = { - 'connection' : - _ERROR_HELP_COMMON + ' Additionally, please check that your computer ' - 'has publicly routable IP address and no firewalls are preventing the ' - 'server from communicating with the client.', - 'dnssec' : - _ERROR_HELP_COMMON + ' Additionally, if you have DNSSEC enabled for ' - 'your domain, please ensure the signature is valid.', - 'malformed' : - 'To fix these errors, please make sure that you did not provide any ' - 'invalid information to the client and try running Let\'s Encrypt ' - 'again.', - 'serverInternal' : - 'Unfortunately, an error on the ACME server prevented you from completing ' - 'authorization. Please try again later.', - 'tls' : - _ERROR_HELP_COMMON + ' Additionally, please check that you have an up ' - 'to date TLS configuration that allows the server to communicate with ' - 'the Let\'s Encrypt client.', - 'unauthorized' : _ERROR_HELP_COMMON, - 'unknownHost' : _ERROR_HELP_COMMON,} + "connection" : + _ERROR_HELP_COMMON + " Additionally, please check that your computer " + "has publicly routable IP address and no firewalls are preventing the " + "server from communicating with the client.", + "dnssec" : + _ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for " + "your domain, please ensure the signature is valid.", + "malformed" : + "To fix these errors, please make sure that you did not provide any " + "invalid information to the client and try running Let's Encrypt " + "again.", + "serverInternal" : + "Unfortunately, an error on the ACME server prevented you from completing " + "authorization. Please try again later.", + "tls" : + _ERROR_HELP_COMMON + " Additionally, please check that you have an up " + "to date TLS configuration that allows the server to communicate with " + "the Let's Encrypt client.", + "unauthorized" : _ERROR_HELP_COMMON, + "unknownHost" : _ERROR_HELP_COMMON,} def _report_failed_challs(failed_achalls): @@ -552,12 +552,12 @@ def _generate_failed_chall_msg(failed_achalls): for achall in failed_achalls: problems.setdefault(achall.error.description, set()).add(achall.domain) for problem in problems: - msg.append('\n\nDomains: ') - msg.append(', '.join(sorted(problems[problem]))) - msg.append('\nError: {0}'.format(problem)) + msg.append("\n\nDomains: ") + msg.append(", ".join(sorted(problems[problem]))) + msg.append("\nError: {0}".format(problem)) if typ in _ERROR_HELP: - msg.append('\n\n') + msg.append("\n\n") msg.append(_ERROR_HELP[typ]) - return ''.join(msg) + return "".join(msg) diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 3045a7e19..dc3859535 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -46,10 +46,9 @@ class Reporter(object): printed if the program exits abnormally. """ - if msg: - assert self.HIGH_PRIORITY <= priority <= self.LOW_PRIORITY - self.messages.put(self._msg_type(priority, msg, on_crash)) - logging.info("Reporting to user: %s", msg) + assert self.HIGH_PRIORITY <= priority <= self.LOW_PRIORITY + self.messages.put(self._msg_type(priority, msg, on_crash)) + logging.info("Reporting to user: %s", msg) def atexit_print_messages(self, pid=os.getpid()): """Function to be registered with atexit to print messages. diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 15ce6a490..6a94baea7 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -459,7 +459,7 @@ class ReportFailedChallsTest(unittest.TestCase): auth_handler._report_failed_challs([self.simple_http, self.dvsni_same]) call_list = mock_zope().add_message.call_args_list self.assertTrue(len(call_list) == 1) - self.assertIn("Domains: example.com\n", call_list[0][0][0]) + self.assertTrue("Domains: example.com\n" in call_list[0][0][0]) @mock.patch("letsencrypt.auth_handler.zope.component.getUtility") def test_different_errors_and_domains(self, mock_zope):