diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index b4e4ccd07..fc90caecc 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -2467,7 +2467,7 @@ class ApacheConfigurator(common.Installer): version=".".join(str(i) for i in self.version)) ) - def auth_hint(self, chall_type): + def auth_hint(self, failed_achalls): 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.") diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 5958c1df6..be465fbae 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -1029,7 +1029,7 @@ class NginxConfigurator(common.Installer): version=".".join(str(i) for i in self.version)) ) - def auth_hint(self, chall_type): + def auth_hint(self, failed_achalls): 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 " diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py index 45ebd10d4..8eae98752 100644 --- a/certbot/certbot/_internal/auth_handler.py +++ b/certbot/certbot/_internal/auth_handler.py @@ -284,11 +284,8 @@ class AuthHandler(object): 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))) + msg.append('\nHint: {}\n'.format(self.auth.auth_hint(failed_achalls))) display_util.notify("".join(msg)) diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py index 37c16382b..2c632e8b1 100644 --- a/certbot/certbot/_internal/plugins/manual.py +++ b/certbot/certbot/_internal/plugins/manual.py @@ -9,6 +9,7 @@ from certbot import errors from certbot import interfaces from certbot import reverter from certbot import util +from certbot._internal.cli import cli_constants from certbot._internal import hooks from certbot.compat import misc from certbot.compat import os @@ -108,31 +109,39 @@ 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' + def auth_hint(self, failed_achalls): + has_chall = lambda cls: any(isinstance(achall.chall, cls) for achall in failed_achalls) + + has_dns = has_chall(challenges.DNS01) + resource_names = { + challenges.DNS01: 'DNS TXT records', + challenges.HTTP01: 'challenge files', + challenges.TLSALPN01: 'TLS-ALPN certificates' + } + resources = ' and '.join(sorted([v for k, v in resource_names.items() if has_chall(k)])) + if self.conf('auth-hook'): return ( - 'The Certificate Authority failed to verify the changes made by the ' + 'The Certificate Authority failed to verify the {resources} created by the ' '--manual-auth-hook. Ensure that this hook is functioning correctly{dns_hint}. ' - 'Refer to {certbot} --help manual.' + 'Refer to "{certbot} --help manual".' .format( - certbot='certbot', + certbot=cli_constants.cli_command, + resources=resources, dns_hint=( - ' and that it waits a sufficient duration ' - 'of time after creating a DNS TXT record' - ) if is_dns else '' + ' and that it waits a sufficient duration of time for DNS propagation' + ) if has_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}.' + 'The Certificate Authority failed to verify the manually created {resources}. ' + 'Ensure that you created these in the correct location{dns_hint}.' .format( - verify='verify' if is_dns else 'download', - resources='DNS TXT records' if is_dns else 'challenge files', + resources=resources, dns_hint=( - ' or try waiting longer for DNS propagation on the next attempt' - ) if is_dns else '' + ', or try waiting longer for DNS propagation on the next attempt' + ) if has_dns else '' ) ) diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py index 2ad1b9457..51c551821 100644 --- a/certbot/certbot/_internal/plugins/webroot.py +++ b/certbot/certbot/_internal/plugins/webroot.py @@ -62,7 +62,7 @@ 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): + def auth_hint(self, failed_achalls): 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 " diff --git a/certbot/certbot/plugins/common.py b/certbot/certbot/plugins/common.py index a5d61eda2..eeefcf47f 100644 --- a/certbot/certbot/plugins/common.py +++ b/certbot/certbot/plugins/common.py @@ -9,7 +9,7 @@ import pkg_resources import zope.interface from acme.magic_typing import List -from certbot import achallenges # pylint: disable=unused-import +from certbot import achallenges # pylint: disable=unused-import from certbot import crypto_util from certbot import errors from certbot import interfaces @@ -97,8 +97,8 @@ 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 + def auth_hint(self, failed_achalls): + # type: (List[achallenges.AnnotatedChallenge]) -> 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. @@ -106,16 +106,18 @@ class Plugin(object): 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) + :param list failed_achalls: List of one or more failed challenges + (:class:`achallenges.AnnotatedChallenge` subclasses). :rtype str: """ # This is a fallback hint. Authenticators should implement their own auth_hint that # addresses the specific mechanics of that authenticator. + challs = " and ".join(sorted({achall.typ for achall in failed_achalls})) return ("The Certificate Authority couldn't exterally verify that the {name} plugin " - "completed the required {chall_type} challenges. Ensure the plugin is configured " + "completed the required {challs} 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)) + .format(name=self.name, challs=challs)) class Installer(Plugin): diff --git a/certbot/certbot/plugins/dns_common.py b/certbot/certbot/plugins/dns_common.py index c6b881816..03f43f1cc 100644 --- a/certbot/certbot/plugins/dns_common.py +++ b/certbot/certbot/plugins/dns_common.py @@ -37,11 +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): + def auth_hint(self, failed_achalls): + delay = self.conf('propagation-seconds') 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) + '--{name} plugin. Ensure the above domains are hosted by this DNS provider, ' + 'or try increasing --{name}-propagation-seconds (currently {secs} second{suffix}).' + .format(name=self.name, secs=delay, suffix='s' if delay != 1 else '') ) def get_chall_pref(self, unused_domain): # pylint: disable=missing-function-docstring diff --git a/certbot/tests/plugins/common_test.py b/certbot/tests/plugins/common_test.py index bcd190e9b..65067ae5e 100644 --- a/certbot/tests/plugins/common_test.py +++ b/certbot/tests/plugins/common_test.py @@ -83,6 +83,13 @@ class PluginTest(unittest.TestCase): parser.add_argument.assert_called_once_with( "--mock-foo-bar", dest="different_to_foo_bar", x=1, y=None) + def test_fallback_auth_hint(self): + self.assertIn("the mock plugin completed the required dns-01 challenges", + self.plugin.auth_hint([acme_util.DNS01_A, acme_util.DNS01_A])) + self.assertIn("the mock plugin completed the required dns-01 and http-01 challenges", + self.plugin.auth_hint([acme_util.DNS01_A, acme_util.HTTP01_A, + acme_util.DNS01_A])) + class InstallerTest(test_util.ConfigTestCase): """Tests for certbot.plugins.common.Installer.""" diff --git a/certbot/tests/plugins/dns_common_test.py b/certbot/tests/plugins/dns_common_test.py index 993f3b461..4b898bfe1 100644 --- a/certbot/tests/plugins/dns_common_test.py +++ b/certbot/tests/plugins/dns_common_test.py @@ -119,6 +119,12 @@ class DNSAuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthen credentials = self.auth._configure_credentials("credentials", "", {"test": ""}) self.assertEqual(credentials.conf("test"), "value") + def test_auth_hint(self): + self.assertIn( + 'try increasing --fake-propagation-seconds (currently 0 seconds).', + self.auth.auth_hint([mock.MagicMock()]) + ) + class CredentialsConfigurationTest(test_util.TempDirTestCase): class _MockLoggingHandler(logging.Handler): diff --git a/certbot/tests/plugins/manual_test.py b/certbot/tests/plugins/manual_test.py index 7318783fd..d1b7b768c 100644 --- a/certbot/tests/plugins/manual_test.py +++ b/certbot/tests/plugins/manual_test.py @@ -122,6 +122,35 @@ class AuthenticatorTest(test_util.TempDirTestCase): else: self.assertFalse('CERTBOT_TOKEN' in os.environ) + def test_auth_hint_hook(self): + self.config.manual_auth_hook = '/bin/true' + self.assertEqual( + self.auth.auth_hint([acme_util.DNS01_A, acme_util.HTTP01_A]), + 'The Certificate Authority failed to verify the DNS TXT records and challenge ' + 'files created by the --manual-auth-hook. Ensure that this hook is functioning ' + 'correctly and that it waits a sufficient duration of time for DNS propagation. ' + 'Refer to "certbot --help manual".' + ) + self.assertEqual( + self.auth.auth_hint([acme_util.HTTP01_A]), + 'The Certificate Authority failed to verify the challenge files created by the ' + '--manual-auth-hook. Ensure that this hook is functioning correctly. Refer to ' + '"certbot --help manual".' + ) + + def test_auth_hint_no_hook(self): + self.assertEqual( + self.auth.auth_hint([acme_util.DNS01_A, acme_util.HTTP01_A]), + 'The Certificate Authority failed to verify the manually created DNS TXT records ' + 'and challenge files. Ensure that you created these in the correct location, or ' + 'try waiting longer for DNS propagation on the next attempt.' + ) + self.assertEqual( + self.auth.auth_hint([acme_util.HTTP01_A, acme_util.HTTP01_A, acme_util.HTTP01_A]), + 'The Certificate Authority failed to verify the manually created challenge files. ' + 'Ensure that you created these in the correct location.' + ) + if __name__ == '__main__': unittest.main() # pragma: no cover