diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index c20a4fdd6..b4e4ccd07 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -2467,6 +2467,11 @@ class ApacheConfigurator(common.Installer): version=".".join(str(i) for i in self.version)) ) + def auth_hint(self, chall_type): + return ("The Certificate Authority failed to verify the temporary Apache configuration " + "changes made by the --apache plugin. Ensure that the above domains point to " + "this Apache server and that it is accessible from the internet.") + ########################################################################### # Challenges Section ########################################################################### diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 87afedd38..5958c1df6 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -1029,6 +1029,13 @@ class NginxConfigurator(common.Installer): version=".".join(str(i) for i in self.version)) ) + def auth_hint(self, chall_type): + return ( + "The Certificate Authority failed to verify the temporary nginx configuration changes " + "made by the --nginx plugin. Ensure the above domains point to this nginx server and " + "that it is accessible from the internet, or try increase --nginx-sleep-seconds." + ) + ################################################### # Wrapper functions for Reverter class (IInstaller) ################################################### diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py index e14412c02..45ebd10d4 100644 --- a/certbot/certbot/_internal/auth_handler.py +++ b/certbot/certbot/_internal/auth_handler.py @@ -176,7 +176,7 @@ class AuthHandler(object): # In case of failed authzrs, create a report to the user. if authzrs_failed_to_report: - _report_failed_authzrs(authzrs_failed_to_report, self.account.key) + self._report_failed_authzrs(authzrs_failed_to_report) if not best_effort: # Without best effort, having failed authzrs is critical and fail the process. raise errors.AuthorizationError('Some challenges have failed.') @@ -267,6 +267,31 @@ class AuthHandler(object): return achalls + def _report_failed_authzrs(self, failed_authzrs): + # type: (List[messages.AuthorizationResource]) -> None + """Notifies the user about failed authorizations.""" + problems = {} # type: Dict[str, List[achallenges.AnnotatedChallenge]] + failed_achalls = [challb_to_achall(challb, self.account.key, authzr.body.identifier.value) + for authzr in failed_authzrs for challb in authzr.body.challenges + if challb.error] + + for achall in failed_achalls: + problems.setdefault(achall.error.typ, []).append(achall) + + msg = ["\nCertbot failed to authenticate some domains (using the {} plugin)." + " The Certificate Authority reported these problems:".format(self.auth.name)] + + for _, achalls in sorted(problems.items(), key=lambda item: item[0]): + msg.append(_generate_failed_chall_msg(achalls)) + + # Show a hint for whatever the first failed challenge was. Every failed challenge will + # almost certainly be the same type, except for very exotic --manual setups. + if failed_achalls: + chall_type = failed_achalls[0].typ + msg.append('\nHint: {}\n'.format(self.auth.auth_hint(chall_type))) + + display_util.notify("".join(msg)) + def challb_to_achall(challb, account_key, domain): """Converts a ChallengeBody object to an AnnotatedChallenge. @@ -396,33 +421,6 @@ def _report_no_chall_path(challbs): raise errors.AuthorizationError(msg) -def _report_failed_authzrs(failed_authzrs, account_key): - # type: (List[messages.AuthorizationResource], JWK) -> None - """Notifies the user about failed authorizations.""" - problems = {} # type: Dict[str, List[achallenges.AnnotatedChallenge]] - failed_achalls = [challb_to_achall(challb, account_key, authzr.body.identifier.value) - for authzr in failed_authzrs for challb in authzr.body.challenges - if challb.error] - - for achall in failed_achalls: - problems.setdefault(achall.error.typ, []).append(achall) - - config = zope.component.getUtility(interfaces.IConfig) - msg = ["\nCertbot failed to authenticate some domains (using the {} plugin)." - " The ACME server reported these problems:".format(config.authenticator)] - - for _, achalls in sorted(problems.items(), key=lambda item: item[0]): - msg.append(_generate_failed_chall_msg(achalls)) - - msg.append( - '\nHint: the Certificate Authority externally verifies the local changes that ' - 'Certbot makes. Ensure the above domains are configured correctly and that changes ' - 'made by the {} plugin are accessible from the internet.\n' - .format(config.authenticator) - ) - display_util.notify("".join(msg)) - - def _generate_failed_chall_msg(failed_achalls): # type: (List[achallenges.AnnotatedChallenge]) -> str """Creates a user friendly error message about failed challenges. diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py index a2e4bb28e..37c16382b 100644 --- a/certbot/certbot/_internal/plugins/manual.py +++ b/certbot/certbot/_internal/plugins/manual.py @@ -108,6 +108,34 @@ permitted by DNS standards.) 'validation challenges either through shell scripts provided by ' 'the user or by performing the setup manually.') + def auth_hint(self, chall_type): + is_dns = chall_type == 'dns-01' + if self.conf('auth-hook'): + return ( + 'The Certificate Authority failed to verify the changes made by the ' + '--manual-auth-hook. Ensure that this hook is functioning correctly{dns_hint}. ' + 'Refer to {certbot} --help manual.' + .format( + certbot='certbot', + dns_hint=( + ' and that it waits a sufficient duration ' + 'of time after creating a DNS TXT record' + ) if is_dns else '' + ) + ) + else: + return ( + 'The Certificate Authority failed to {verify} the manually created {resources}. ' + 'Ensure that you created the {resources} in the correct location{dns_hint}.' + .format( + verify='verify' if is_dns else 'download', + resources='DNS TXT records' if is_dns else 'challenge files', + dns_hint=( + ' or try waiting longer for DNS propagation on the next attempt' + ) if is_dns else '' + ) + ) + def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-function-docstring return [challenges.HTTP01, challenges.DNS01] diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py index 484d209d6..2ad1b9457 100644 --- a/certbot/certbot/_internal/plugins/webroot.py +++ b/certbot/certbot/_internal/plugins/webroot.py @@ -62,6 +62,12 @@ to serve all files under specified web root ({0}).""" "file, it needs to be on a single line, like: webroot-map = " '{"example.com":"/var/www"}.') + def auth_hint(self, chall_type): + return ("The Certificate Authority failed to download the temporary challenge files " + "created by the --webroot plugin. Ensure that the above domains serve their " + "content from the provided --webroot-path/-w and that files created there " + "can be downloaded from the internet.") + def get_chall_pref(self, domain): # pragma: no cover # pylint: disable=unused-argument,missing-function-docstring return [challenges.HTTP01] diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py index d3fb5b7dc..a5d61eda2 100644 --- a/certbot/certbot/plugins/common.py +++ b/certbot/certbot/plugins/common.py @@ -97,6 +97,26 @@ class Plugin(object): """Find a configuration value for variable ``var``.""" return getattr(self.config, self.dest(var)) + def auth_hint(self, chall_type): + # type: (str) -> str + """Human-readable string to help the user troubleshoot the authenticator. + + Shown to the user if one or more of the attempted challenges were not a success. + + Should describe, in simple language, what the authenticator tried to do, what went + wrong and what the user should try as their "next steps". + + :param str chall_type: The challenge type (label of ACME Validation Method) + + :rtype str: + """ + # This is a fallback hint. Authenticators should implement their own auth_hint that + # addresses the specific mechanics of that authenticator. + return ("The Certificate Authority couldn't exterally verify that the {name} plugin " + "completed the required {chall_type} challenges. Ensure the plugin is configured " + "correctly and that the changes it makes are accessible from the internet." + .format(name=self.name, chall_type=chall_type)) + class Installer(Plugin): """An installer base class with reverter and ssl_dhparam methods defined. diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py index 245b7dc05..c6b881816 100644 --- a/certbot/certbot/plugins/dns_common.py +++ b/certbot/certbot/plugins/dns_common.py @@ -37,6 +37,13 @@ class DNSAuthenticator(common.Plugin): help='The number of seconds to wait for DNS to propagate before asking the ACME server ' 'to verify the DNS record.') + def auth_hint(self, chall_type): + return ( + 'The Certificate Authority failed to verify the DNS TXT records created by the ' + '--{name} plugin. Ensure that the above domains are managed by that DNS provider, ' + 'or try increasing --{name}-propagation-seconds.'.format(name=self.name) + ) + def get_chall_pref(self, unused_domain): # pylint: disable=missing-function-docstring return [challenges.DNS01] diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index dd7fd002b..8e927f0ae 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -327,7 +327,8 @@ class HandleAuthorizationsTest(unittest.TestCase): mock_order = mock.MagicMock(authorizations=authzrs) - with mock.patch('certbot._internal.auth_handler._report_failed_authzrs') as mock_report: + with mock.patch('certbot._internal.auth_handler.AuthHandler._report_failed_authzrs') \ + as mock_report: valid_authzr = self.handler.handle_authorizations(mock_order, True) # Because best_effort=True, we did not blow up. Instead ... @@ -474,10 +475,18 @@ class GenChallengePathTest(unittest.TestCase): class ReportFailedAuthzrsTest(unittest.TestCase): - """Tests for certbot._internal.auth_handler._report_failed_authzrs.""" + """Tests for certbot._internal.auth_handler.AuthHandler._report_failed_authzrs.""" # pylint: disable=protected-access + def setUp(self): + from certbot._internal.auth_handler import AuthHandler + + self.mock_auth = mock.MagicMock(name="buzz") + self.mock_auth.name = "buzz" + self.mock_auth.auth_hint.return_value = "the buzz hint" + self.handler = AuthHandler(self.mock_auth, mock.MagicMock(), mock.MagicMock(), []) + kwargs = { "chall": acme_util.HTTP01, "uri": "uri", @@ -505,15 +514,12 @@ class ReportFailedAuthzrsTest(unittest.TestCase): self.authzr2.body.challenges = [http_01_diff] @mock.patch('certbot._internal.auth_handler.display_util.notify') - @test_util.patch_get_utility() - def test_same_error_and_domain(self, mock_util, mock_notify): - from certbot._internal import auth_handler - mock_util().authenticator = "foobaz" - auth_handler._report_failed_authzrs([self.authzr1], 'key') + def test_same_error_and_domain(self, mock_notify): + self.handler._report_failed_authzrs([self.authzr1]) mock_notify.assert_called_with( '\n' - 'Certbot failed to authenticate some domains (using the foobaz plugin). ' - 'The ACME server reported these problems:\n' + 'Certbot failed to authenticate some domains (using the buzz plugin). ' + 'The Certificate Authority reported these problems:\n' ' Domain: example.com\n' ' Type: tls\n' ' Detail: detail\n' @@ -521,21 +527,18 @@ class ReportFailedAuthzrsTest(unittest.TestCase): ' Domain: example.com\n' ' Type: tls\n' ' Detail: detail\n' - '\nHint: the Certificate Authority externally verifies the local changes that ' - 'Certbot makes. Ensure the above domains are configured correctly and that ' - 'changes made by the foobaz plugin are accessible from the internet.\n' + '\nHint: the buzz hint\n' ) @mock.patch('certbot._internal.auth_handler.display_util.notify') - @test_util.patch_get_utility() - def test_different_errors_and_domains(self, mock_util, mock_notify): - from certbot._internal import auth_handler - mock_util().authenticator = "quux" - auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key') + def test_different_errors_and_domains(self, mock_notify): + self.mock_auth.name = "quux" + self.mock_auth.auth_hint.return_value = "quuuuuux" + self.handler._report_failed_authzrs([self.authzr1, self.authzr2]) mock_notify.assert_called_with( '\n' 'Certbot failed to authenticate some domains (using the quux plugin). ' - 'The ACME server reported these problems:\n' + 'The Certificate Authority reported these problems:\n' ' Domain: foo.bar\n' ' Type: dnssec\n' ' Detail: detail\n' @@ -547,9 +550,7 @@ class ReportFailedAuthzrsTest(unittest.TestCase): ' Domain: example.com\n' ' Type: tls\n' ' Detail: detail\n' - '\nHint: the Certificate Authority externally verifies the local changes that ' - 'Certbot makes. Ensure the above domains are configured correctly and that ' - 'changes made by the quux plugin are accessible from the internet.\n' + '\nHint: quuuuuux\n' )