upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-06-05 06:42:10 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Merge branch 'cli_authz_failure_reporting' into ux

Browse source
This commit is contained in:
Alex Zorin 2021-05-18 14:57:10 +10:00
commit 1446a666f2
11 changed files with 222 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
certbot-apache/certbot_apache/_internal/configurator.py
View file

@ -2505,6 +2505,11 @@ class ApacheConfigurator(common.Installer):
version=".".join(str(i) for i in self.version))
)
def auth_hint(self, failed_achalls): # pragma: no cover
return ("The Certificate Authority failed to verify the temporary Apache configuration "
"changes made by Certbot. Ensure that the listed domains point to this Apache "
"server and that it is accessible from the internet.")
###########################################################################
# Challenges Section
###########################################################################

7
certbot-nginx/certbot_nginx/_internal/configurator.py
View file

@ -1078,6 +1078,13 @@ class NginxConfigurator(common.Installer):
version=".".join(str(i) for i in self.version))
)
def auth_hint(self, failed_achalls): # pragma: no cover
return (
"The Certificate Authority failed to verify the temporary nginx configuration changes "
"made by Certbot. Ensure the listed domains point to this nginx server and that it is "
"accessible from the internet."
)
###################################################
# Wrapper functions for Reverter class (IInstaller)
###################################################

84
certbot/certbot/_internal/auth_handler.py
View file

@ -15,6 +15,8 @@ from certbot import achallenges
from certbot import errors
from certbot import interfaces
from certbot._internal import error_handler
from certbot.display import util as display_util
from certbot.plugins import common as plugin_common
logger = logging.getLogger(__name__)
@ -173,7 +175,7 @@ class AuthHandler:
# 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.')
@ -264,6 +266,29 @@ class AuthHandler:
return achalls
def _report_failed_authzrs(self, failed_authzrs: List[messages.AuthorizationResource]) -> None:
"""Notifies the user about failed authorizations."""
problems: 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 = [f"\nCertbot failed to authenticate some domains (authenticator: {self.auth.name})."
" The Certificate Authority reported these problems:"]
for _, achalls in sorted(problems.items(), key=lambda item: item[0]):
msg.append(_generate_failed_chall_msg(achalls))
# auth_hint will only be called on authenticators that subclass
# plugin_common.Plugin. Refer to comment on that function.
if failed_achalls and isinstance(self.auth, plugin_common.Plugin):
msg.append(f"\nHint: {self.auth.auth_hint(failed_achalls)}\n")
display_util.notify("".join(msg))
def challb_to_achall(challb, account_key, domain):
"""Converts a ChallengeBody object to an AnnotatedChallenge.
@ -393,60 +418,13 @@ def _report_no_chall_path(challbs):
raise errors.AuthorizationError(msg)
_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 contain(s) the "
"right IP address.")
_ERROR_HELP = {
"connection":
_ERROR_HELP_COMMON + " Additionally, please check that your computer "
"has a publicly routable IP address and that no firewalls are preventing "
"the server from communicating with the client. If you're using the "
"webroot plugin, you should also verify that you are serving files "
"from the webroot path you provided.",
"dnssec":
_ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for "
"your domain, please ensure that 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 Certbot "
"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 Certbot client.",
"unauthorized": _ERROR_HELP_COMMON,
"unknownHost": _ERROR_HELP_COMMON,
}
def _report_failed_authzrs(failed_authzrs, account_key):
"""Notifies the user about failed authorizations."""
problems: 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)
reporter = zope.component.getUtility(interfaces.IReporter)
for achalls in problems.values():
reporter.add_message(_generate_failed_chall_msg(achalls), reporter.MEDIUM_PRIORITY)
def _generate_failed_chall_msg(failed_achalls):
# type: (List[achallenges.AnnotatedChallenge]) -> str
"""Creates a user friendly error message about failed challenges.
:param list failed_achalls: A list of failed
:class:`certbot.achallenges.AnnotatedChallenge` with the same error
type.
:returns: A formatted error message for the client.
:rtype: str
@ -455,14 +433,10 @@ def _generate_failed_chall_msg(failed_achalls):
typ = error.typ
if messages.is_acme_error(error):
typ = error.code
msg = ["The following errors were reported by the server:"]
msg = []
for achall in failed_achalls:
msg.append("\n\nDomain: %s\nType: %s\nDetail: %s" % (
msg.append("\n Domain: %s\n Type: %s\n Detail: %s\n" % (
achall.domain, typ, achall.error.detail))
if typ in _ERROR_HELP:
msg.append("\n\n")
msg.append(_ERROR_HELP[typ])
return "".join(msg)

37
certbot/certbot/_internal/plugins/manual.py
View file

@ -10,6 +10,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
@ -125,6 +126,42 @@ permitted by DNS standards.)
'validation challenges either through shell scripts provided by '
'the user or by performing the setup manually.')
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 {resources} created by the '
'--manual-auth-hook. Ensure that this hook is functioning correctly{dns_hint}. '
'Refer to "{certbot} --help manual" and the Certbot User Guide.'
.format(
certbot=cli_constants.cli_command,
resources=resources,
dns_hint=(
' 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 these in the correct location{dns_hint}.'
.format(
resources=resources,
dns_hint=(
', or try waiting longer for DNS propagation on the next attempt'
) if has_dns else ''
)
)
def get_chall_pref(self, domain):
# pylint: disable=unused-argument,missing-function-docstring
return [challenges.HTTP01, challenges.DNS01]

6
certbot/certbot/_internal/plugins/webroot.py
View file

@ -61,6 +61,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, failed_achalls): # pragma: no cover
return ("The Certificate Authority failed to download the temporary challenge files "
"created by Certbot. Ensure that the listed 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]

27
certbot/certbot/plugins/common.py
View file

@ -97,6 +97,33 @@ class Plugin:
"""Find a configuration value for variable ``var``."""
return getattr(self.config, self.dest(var))
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.
Should describe, in simple language, what the authenticator tried to do, what went
wrong and what the user should try as their "next steps".
TODO: auth_hint belongs in IAuthenticator but can't be added until the next major
version of Certbot. For now, it lives in .Plugin and auth_handler will only call it
on authenticators that subclass .Plugin. For now, inherit from `.Plugin` to implement
and/or override the 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 {challs} challenges. Ensure the plugin is configured "
"correctly and that the changes it makes are accessible from the internet."
.format(name=self.name, challs=challs))
class Installer(Plugin):
"""An installer base class with reverter and ssl_dhparam methods defined.

9
certbot/certbot/plugins/dns_common.py
View file

@ -37,6 +37,15 @@ 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, failed_achalls):
delay = self.conf('propagation-seconds')
return (
'The Certificate Authority failed to verify the DNS TXT records created by --{name}. '
'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
return [challenges.DNS01]

74
certbot/tests/auth_handler_test.py
View file

@ -17,6 +17,7 @@ from certbot import achallenges
from certbot import errors
from certbot import interfaces
from certbot import util
from certbot.plugins import common as plugin_common
from certbot.tests import acme_util
from certbot.tests import util as test_util
@ -327,7 +328,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 +476,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(spec=plugin_common.Plugin, 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",
@ -504,21 +514,57 @@ class ReportFailedAuthzrsTest(unittest.TestCase):
self.authzr2.body.identifier.value = 'foo.bar'
self.authzr2.body.challenges = [http_01_diff]
@test_util.patch_get_utility()
def test_same_error_and_domain(self, mock_zope):
from certbot._internal import auth_handler
@mock.patch('certbot._internal.auth_handler.display_util.notify')
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 (authenticator: buzz). '
'The Certificate Authority reported these problems:\n'
' Domain: example.com\n'
' Type: tls\n'
' Detail: detail\n'
'\n'
' Domain: example.com\n'
' Type: tls\n'
' Detail: detail\n'
'\nHint: the buzz hint\n'
)
auth_handler._report_failed_authzrs([self.authzr1], 'key')
call_list = mock_zope().add_message.call_args_list
self.assertEqual(len(call_list), 1)
self.assertIn("Domain: example.com\nType: tls\nDetail: detail", call_list[0][0][0])
@mock.patch('certbot._internal.auth_handler.display_util.notify')
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 (authenticator: quux). '
'The Certificate Authority reported these problems:\n'
' Domain: foo.bar\n'
' Type: dnssec\n'
' Detail: detail\n'
'\n'
' Domain: example.com\n'
' Type: tls\n'
' Detail: detail\n'
'\n'
' Domain: example.com\n'
' Type: tls\n'
' Detail: detail\n'
'\nHint: quuuuuux\n'
)
@test_util.patch_get_utility()
def test_different_errors_and_domains(self, mock_zope):
from certbot._internal import auth_handler
@mock.patch('certbot._internal.auth_handler.display_util.notify')
def test_non_subclassed_authenticator(self, mock_notify):
"""If authenticator not derived from common.Plugin, we shouldn't call .auth_hint"""
from certbot._internal.auth_handler import AuthHandler
auth_handler._report_failed_authzrs([self.authzr1, self.authzr2], 'key')
self.assertEqual(mock_zope().add_message.call_count, 2)
self.mock_auth = mock.MagicMock(name="quuz")
self.mock_auth.name = "quuz"
self.mock_auth.auth_hint.side_effect = Exception
self.handler = AuthHandler(self.mock_auth, mock.MagicMock(), mock.MagicMock(), [])
self.handler._report_failed_authzrs([self.authzr1])
self.assertEqual(mock_notify.call_count, 1)
def gen_auth_resp(chall_list):

7
certbot/tests/plugins/common_test.py
View file

@ -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."""

6
certbot/tests/plugins/dns_common_test.py
View file

@ -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):

29
certbot/tests/plugins/manual_test.py
View file

@ -120,6 +120,35 @@ class AuthenticatorTest(test_util.TempDirTestCase):
else:
self.assertNotIn('CERTBOT_TOKEN', 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" and the Certbot User Guide.'
)
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" and the Certbot User Guide.'
)
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