Use verbose=3 in sample dev cli.ini now that count is allowed in config files

Fixed typo:

exterally -> externally

.azure-pipelines/templates/steps/sphinx-steps.yml

@@ -2,14 +2,12 @@ steps:
   - bash: |
       FINAL_STATUS=0
       declare -a FAILED_BUILDS
-      python3 -m venv .venv
-      source .venv/bin/activate
-      python tools/pipstrap.py
+      tools/venv.py
+      source venv/bin/activate
       for doc_path in */docs
       do
         echo ""
         echo "##[group]Building $doc_path"
-        tools/pip_install_editable.py $doc_path/..[docs]
         if ! sphinx-build -W --keep-going -b html $doc_path $doc_path/_build/html; then
           FINAL_STATUS=1
           FAILED_BUILDS[${#FAILED_BUILDS[@]}]="${doc_path%/docs}"

.github/FUNDING.yml

@@ -0,0 +1 @@
+custom: https://supporters.eff.org/donate/support-work-on-certbot

.pylintrc

@@ -56,7 +56,18 @@ extension-pkg-whitelist=pywintypes,win32api,win32file,win32security
 #    See https://github.com/PyCQA/pylint/issues/1498.
 # 3) Same as point 2 for no-value-for-parameter.
 #    See https://github.com/PyCQA/pylint/issues/2820.
-disable=fixme,locally-disabled,locally-enabled,bad-continuation,no-self-use,invalid-name,cyclic-import,duplicate-code,design,import-outside-toplevel,useless-object-inheritance,unsubscriptable-object,no-value-for-parameter,no-else-return,no-else-raise,no-else-break,no-else-continue
+# 4) raise-missing-from makes it an error to raise an exception from except
+#    block without using explicit exception chaining. While explicit exception
+#    chaining results in a slightly more informative traceback, I don't think
+#    it's beneficial enough for us to change all of our current instances and
+#    give Certbot developers errors about this when they're working on new code
+#    in the future. You can read more about exception chaining and this pylint
+#    check at
+#    https://blog.ram.rachum.com/post/621791438475296768/improving-python-exception-chaining-with.
+# 5) wrong-import-order generates false positives and a pylint developer
+#    suggests that people using isort should disable this check at
+#    https://github.com/PyCQA/pylint/issues/3817#issuecomment-687892090.
+disable=fixme,locally-disabled,locally-enabled,bad-continuation,no-self-use,invalid-name,cyclic-import,duplicate-code,design,import-outside-toplevel,useless-object-inheritance,unsubscriptable-object,no-value-for-parameter,no-else-return,no-else-raise,no-else-break,no-else-continue,raise-missing-from,wrong-import-order
 
 [REPORTS]
 

acme/setup.py

@@ -3,9 +3,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Please update tox.ini when modifying dependency version requirements
 install_requires = [
     'cryptography>=2.1.4',
     # formerly known as acme.jose:

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -220,13 +220,14 @@ def _get_runtime_cfg(command):
 
     """
     try:
-        proc = subprocess.Popen(
+        proc = subprocess.run(
             command,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             universal_newlines=True,
+            check=False,
             env=util.env_no_snap_for_external_calls())
-        stdout, stderr = proc.communicate()
+        stdout, stderr = proc.stdout, proc.stderr
 
     except (OSError, ValueError):
         logger.error(

certbot-apache/certbot_apache/_internal/assertions.py

@@ -136,6 +136,6 @@ def assertEqualPathsList(first, second):  # pragma: no cover
     if any(isPass(path) for path in second):
         return
     for fpath in first:
-        assert any([fnmatch.fnmatch(fpath, spath) for spath in second])
+        assert any(fnmatch.fnmatch(fpath, spath) for spath in second)
     for spath in second:
-        assert any([fnmatch.fnmatch(fpath, spath) for fpath in first])
+        assert any(fnmatch.fnmatch(fpath, spath) for fpath in first)

certbot-apache/certbot_apache/_internal/configurator.py

@@ -25,6 +25,7 @@ from certbot import util
 from certbot.achallenges import KeyAuthorizationAnnotatedChallenge  # pylint: disable=unused-import
 from certbot.compat import filesystem
 from certbot.compat import os
+from certbot.display import util as display_util
 from certbot.plugins import common
 from certbot.plugins.enhancements import AutoHSTSEnhancement
 from certbot.plugins.util import path_surgery
@@ -515,6 +516,8 @@ class ApacheConfigurator(common.Installer):
         vhosts = self.choose_vhosts(domain)
         for vhost in vhosts:
             self._deploy_cert(vhost, cert_path, key_path, chain_path, fullchain_path)
+            display_util.notify("Successfully deployed certificate for {} to {}"
+                                .format(domain, vhost.filep))
 
     def choose_vhosts(self, domain, create_if_no_ssl=True):
         """
@@ -553,6 +556,19 @@ class ApacheConfigurator(common.Installer):
 
         return list(matched)
 
+    def _raise_no_suitable_vhost_error(self, target_name: str):
+        """
+        Notifies the user that Certbot could not find a vhost to secure
+        and raises an error.
+        :param str target_name: The server name that could not be mapped
+        :raises errors.PluginError: Raised unconditionally
+        """
+        raise errors.PluginError(
+            "Certbot could not find a VirtualHost for {0} in the Apache "
+            "configuration. Please create a VirtualHost with a ServerName "
+            "matching {0} and try again.".format(target_name)
+        )
+
     def _in_wildcard_scope(self, name, domain):
         """
         Helper method for _vhosts_for_wildcard() that makes sure that the domain
@@ -590,12 +606,7 @@ class ApacheConfigurator(common.Installer):
         dialog_output = display_ops.select_vhost_multiple(list(dialog_input))
 
         if not dialog_output:
-            logger.error(
-                "No vhost exists with servername or alias for domain %s. "
-                "No vhost was selected. Please specify ServerName or ServerAlias "
-                "in the Apache config.",
-                domain)
-            raise errors.PluginError("No vhost selected")
+            self._raise_no_suitable_vhost_error(domain)
 
         # Make sure we create SSL vhosts for the ones that are HTTP only
         # if requested.
@@ -719,12 +730,7 @@ class ApacheConfigurator(common.Installer):
         # Select a vhost from a list
         vhost = display_ops.select_vhost(target_name, self.vhosts)
         if vhost is None:
-            logger.error(
-                "No vhost exists with servername or alias of %s. "
-                "No vhost was selected. Please specify ServerName or ServerAlias "
-                "in the Apache config.",
-                target_name)
-            raise errors.PluginError("No vhost selected")
+            self._raise_no_suitable_vhost_error(target_name)
         if temp:
             return vhost
         if not vhost.ssl:
@@ -1532,12 +1538,11 @@ class ApacheConfigurator(common.Installer):
             raise errors.PluginError("Unable to write/read in make_vhost_ssl")
 
         if sift:
-            reporter = zope.component.getUtility(interfaces.IReporter)
-            reporter.add_message(
-                "Some rewrite rules copied from {0} were disabled in the "
-                "vhost for your HTTPS site located at {1} because they have "
-                "the potential to create redirection loops.".format(
-                    vhost.filep, ssl_fp), reporter.MEDIUM_PRIORITY)
+            display_util.notify(
+                f"Some rewrite rules copied from {vhost.filep} were disabled in the "
+                f"vhost for your HTTPS site located at {ssl_fp} because they have "
+                "the potential to create redirection loops."
+            )
         self.parser.aug.set("/augeas/files%s/mtime" % (self._escape(ssl_fp)), "0")
         self.parser.aug.set("/augeas/files%s/mtime" % (self._escape(vhost.filep)), "0")
 
@@ -1866,13 +1871,13 @@ class ApacheConfigurator(common.Installer):
             if options:
                 msg_enhancement += ": " + options
             msg = msg_tmpl.format(domain, msg_enhancement)
-            logger.warning(msg)
+            logger.error(msg)
             raise errors.PluginError(msg)
         try:
             for vhost in vhosts:
                 func(vhost, options)
         except errors.PluginError:
-            logger.warning("Failed %s for %s", enhancement, domain)
+            logger.error("Failed %s for %s", enhancement, domain)
             raise
 
     def _autohsts_increase(self, vhost, id_str, nextstep):
@@ -2396,7 +2401,7 @@ class ApacheConfigurator(common.Installer):
             vhost.enabled = True
         return
 
-    def enable_mod(self, mod_name, temp=False):
+    def enable_mod(self, mod_name, temp=False):  # pylint: disable=unused-argument
         """Enables module in Apache.
 
         Both enables and reloads Apache so module is active.
@@ -2436,7 +2441,7 @@ class ApacheConfigurator(common.Installer):
         try:
             util.run_script(self.options.restart_cmd)
         except errors.SubprocessError as err:
-            logger.info("Unable to restart apache using %s",
+            logger.warning("Unable to restart apache using %s",
                         self.options.restart_cmd)
             alt_restart = self.options.restart_cmd_alt
             if alt_restart:
@@ -2500,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
     ###########################################################################
@@ -2593,7 +2603,7 @@ class ApacheConfigurator(common.Installer):
                 msg_tmpl = ("Certbot was not able to find SSL VirtualHost for a "
                             "domain {0} for enabling AutoHSTS enhancement.")
                 msg = msg_tmpl.format(d)
-                logger.warning(msg)
+                logger.error(msg)
                 raise errors.PluginError(msg)
             for vh in vhosts:
                 try:
@@ -2679,7 +2689,7 @@ class ApacheConfigurator(common.Installer):
                 except errors.PluginError:
                     msg = ("Could not find VirtualHost with ID {0}, disabling "
                            "AutoHSTS for this VirtualHost").format(id_str)
-                    logger.warning(msg)
+                    logger.error(msg)
                     # Remove the orphaned AutoHSTS entry from pluginstorage
                     self._autohsts.pop(id_str)
                     continue
@@ -2719,7 +2729,7 @@ class ApacheConfigurator(common.Installer):
                 except errors.PluginError:
                     msg = ("VirtualHost with id {} was not found, unable to "
                            "make HSTS max-age permanent.").format(id_str)
-                    logger.warning(msg)
+                    logger.error(msg)
                     self._autohsts.pop(id_str)
                     continue
                 if self._autohsts_vhost_in_lineage(vhost, lineage):

certbot-apache/certbot_apache/_internal/display_ops.py

@@ -119,7 +119,7 @@ def _vhost_menu(domain, vhosts):
             "guidance in non-interactive mode. Certbot may need "
             "vhosts to be explicitly labelled with ServerName or "
             "ServerAlias directives.".format(domain))
-        logger.warning(msg)
+        logger.error(msg)
         raise errors.MissingCommandlineFlag(msg)
 
     return code, tag

certbot-apache/certbot_apache/_internal/override_debian.py

@@ -58,7 +58,7 @@ class DebianConfigurator(configurator.ApacheConfigurator):
                 # Already in shape
                 vhost.enabled = True
                 return None
-            logger.warning(
+            logger.error(
                 "Could not symlink %s to %s, got error: %s", enabled_path,
                 vhost.filep, err.strerror)
             errstring = ("Encountered error while trying to enable a " +

certbot-apache/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.6.0

certbot-apache/setup.py

@@ -1,13 +1,14 @@
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'acme>=0.29.0',
-    'certbot>=1.6.0',
+    # We specify the minimum acme and certbot version as the current plugin
+    # version for simplicity. See
+    # https://github.com/certbot/certbot/issues/8761 for more info.
+    f'acme>={version}',
+    f'certbot>={version}',
     'python-augeas',
     'setuptools>=39.0.1',
     'zope.component',

certbot-apache/tests/autohsts_test.py

@@ -146,7 +146,7 @@ class AutoHSTSTest(util.ApacheTest):
     @mock.patch("certbot_apache._internal.display_ops.select_vhost")
     def test_autohsts_no_ssl_vhost(self, mock_select):
         mock_select.return_value = self.vh_truth[0]
-        with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
+        with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log:
             self.assertRaises(errors.PluginError,
                               self.config.enable_autohsts,
                               mock.MagicMock(), "invalid.example.com")
@@ -179,7 +179,7 @@ class AutoHSTSTest(util.ApacheTest):
         self.config._autohsts_fetch_state()
         self.config._autohsts["orphan_id"] = {"laststep": 999, "timestamp": 0}
         self.config._autohsts_save_state()
-        with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
+        with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log:
             self.config.deploy_autohsts(mock.MagicMock())
             self.assertTrue(mock_log.called)
             self.assertTrue(

certbot-apache/tests/centos6_test.py

@@ -1,5 +1,6 @@
 """Test for certbot_apache._internal.configurator for CentOS 6 overrides"""
 import unittest
+from unittest import mock
 
 from certbot.compat import os
 from certbot.errors import MisconfigurationError
@@ -65,7 +66,8 @@ class CentOS6Tests(util.ApacheTest):
                 raise Exception("Missed: %s" % vhost)  # pragma: no cover
         self.assertEqual(found, 2)
 
-    def test_loadmod_default(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_default(self, unused_mock_notify):
         ssl_loadmods = self.config.parser.find_dir(
             "LoadModule", "ssl_module", exclude=False)
         self.assertEqual(len(ssl_loadmods), 1)
@@ -95,7 +97,8 @@ class CentOS6Tests(util.ApacheTest):
             ifmod_args = self.config.parser.get_all_args(lm[:-17])
             self.assertTrue("!mod_ssl.c" in ifmod_args)
 
-    def test_loadmod_multiple(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_multiple(self, unused_mock_notify):
         sslmod_args = ["ssl_module", "modules/mod_ssl.so"]
         # Adds another LoadModule to main httpd.conf in addtition to ssl.conf
         self.config.parser.add_dir(self.config.parser.loc["default"], "LoadModule",
@@ -115,7 +118,8 @@ class CentOS6Tests(util.ApacheTest):
         for mod in post_loadmods:
             self.assertTrue(self.config.parser.not_modssl_ifmodule(mod))  #pylint: disable=no-member
 
-    def test_loadmod_rootconf_exists(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_rootconf_exists(self, unused_mock_notify):
         sslmod_args = ["ssl_module", "modules/mod_ssl.so"]
         rootconf_ifmod = self.config.parser.get_ifmod(
             parser.get_aug_path(self.config.parser.loc["default"]),
@@ -142,7 +146,8 @@ class CentOS6Tests(util.ApacheTest):
             self.config.parser.get_all_args(mods[0][:-7]),
             sslmod_args)
 
-    def test_neg_loadmod_already_on_path(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_neg_loadmod_already_on_path(self, unused_mock_notify):
         loadmod_args = ["ssl_module", "modules/mod_ssl.so"]
         ifmod = self.config.parser.get_ifmod(
             self.vh_truth[1].path, "!mod_ssl.c", beginning=True)
@@ -185,7 +190,8 @@ class CentOS6Tests(util.ApacheTest):
         # Make sure that none was changed
         self.assertEqual(pre_matches, post_matches)
 
-    def test_loadmod_not_found(self):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_loadmod_not_found(self, unused_mock_notify):
         # Remove all existing LoadModule ssl_module... directives
         orig_loadmods = self.config.parser.find_dir("LoadModule",
                                                     "ssl_module",

certbot-apache/tests/configurator_test.py

@@ -337,7 +337,8 @@ class MultipleVhostsTest(util.ApacheTest):
         vhosts = self.config._non_default_vhosts(self.config.vhosts)
         self.assertEqual(len(vhosts), 10)
 
-    def test_deploy_cert_enable_new_vhost(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert_enable_new_vhost(self, unused_mock_notify):
         # Create
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
         self.config.parser.modules["ssl_module"] = None
@@ -375,7 +376,8 @@ class MultipleVhostsTest(util.ApacheTest):
                 self.fail("Include shouldn't be added, as patched find_dir 'finds' existing one") \
                     # pragma: no cover
 
-    def test_deploy_cert(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert(self, unused_mock_notify):
         self.config.parser.modules["ssl_module"] = None
         self.config.parser.modules["mod_ssl.c"] = None
         self.config.parser.modules["socache_shmcb_module"] = None
@@ -891,7 +893,7 @@ class MultipleVhostsTest(util.ApacheTest):
             self.config.enhance, "certbot.demo", "unknown_enhancement")
 
     def test_enhance_no_ssl_vhost(self):
-        with mock.patch("certbot_apache._internal.configurator.logger.warning") as mock_log:
+        with mock.patch("certbot_apache._internal.configurator.logger.error") as mock_log:
             self.assertRaises(errors.PluginError, self.config.enhance,
                               "certbot.demo", "redirect")
             # Check that correct logger.warning was printed
@@ -1290,7 +1292,8 @@ class MultipleVhostsTest(util.ApacheTest):
             os.path.basename(inc_path) in self.config.parser.existing_paths[
                 os.path.dirname(inc_path)])
 
-    def test_deploy_cert_not_parsed_path(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert_not_parsed_path(self, unused_mock_notify):
         # Make sure that we add include to root config for vhosts when
         # handle-sites is false
         self.config.parser.modules["ssl_module"] = None
@@ -1386,7 +1389,8 @@ class MultipleVhostsTest(util.ApacheTest):
             self.assertEqual(vhs[0], self.vh_truth[7])
 
 
-    def test_deploy_cert_wildcard(self):
+    @mock.patch('certbot_apache._internal.configurator.display_util.notify')
+    def test_deploy_cert_wildcard(self, unused_mock_notify):
         # pylint: disable=protected-access
         mock_choose_vhosts = mock.MagicMock()
         mock_choose_vhosts.return_value = [self.vh_truth[7]]
@@ -1606,8 +1610,8 @@ class MultiVhostsTest(util.ApacheTest):
         self.assertEqual(self.config._get_new_vh_path(without_index, both),
                          with_index_2[0])
 
-    @certbot_util.patch_get_utility()
-    def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_get_utility):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_make_vhost_ssl_with_existing_rewrite_rule(self, mock_notify):
         self.config.parser.modules["rewrite_module"] = None
 
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[4])
@@ -1623,11 +1627,11 @@ class MultiVhostsTest(util.ApacheTest):
                                     "\"http://new.example.com/docs/$1\"  [R,L]")
         self.assertTrue(commented_rewrite_rule in conf_text)
         self.assertTrue(uncommented_rewrite_rule in conf_text)
-        mock_get_utility().add_message.assert_called_once_with(mock.ANY,
-                                                               mock.ANY)
+        self.assertEqual(mock_notify.call_count, 1)
+        self.assertIn("Some rewrite rules", mock_notify.call_args[0][0])
 
-    @certbot_util.patch_get_utility()
-    def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_get_utility):
+    @mock.patch("certbot_apache._internal.configurator.display_util.notify")
+    def test_make_vhost_ssl_with_existing_rewrite_conds(self, mock_notify):
         self.config.parser.modules["rewrite_module"] = None
 
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[3])
@@ -1652,8 +1656,8 @@ class MultiVhostsTest(util.ApacheTest):
         self.assertTrue(commented_cond1 in conf_line_set)
         self.assertTrue(commented_cond2 in conf_line_set)
         self.assertTrue(commented_rewrite_rule in conf_line_set)
-        mock_get_utility().add_message.assert_called_once_with(mock.ANY,
-                                                               mock.ANY)
+        self.assertEqual(mock_notify.call_count, 1)
+        self.assertIn("Some rewrite rules", mock_notify.call_args[0][0])
 
 
 class InstallSslOptionsConfTest(util.ApacheTest):

certbot-apache/tests/debian_test.py

@@ -49,10 +49,11 @@ class MultipleVhostsTestDebian(util.ApacheTest):
 
     @mock.patch("certbot.util.run_script")
     @mock.patch("certbot.util.exe_exists")
-    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
-    def test_enable_mod(self, mock_popen, mock_exe_exists, mock_run_script):
-        mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "")
-        mock_popen().returncode = 0
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
+    def test_enable_mod(self, mock_run, mock_exe_exists, mock_run_script):
+        mock_run.return_value.stdout = "Define: DUMP_RUN_CFG"
+        mock_run.return_value.stderr = ""
+        mock_run.return_value.returncode = 0
         mock_exe_exists.return_value = True
 
         self.config.enable_mod("ssl")

certbot-apache/tests/parser_test.py

@@ -305,17 +305,19 @@ class BasicParserTest(util.ParserTest):
         self.assertRaises(
             errors.PluginError, self.parser.update_runtime_variables)
 
-    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
-    def test_update_runtime_vars_bad_ctl(self, mock_popen):
-        mock_popen.side_effect = OSError
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
+    def test_update_runtime_vars_bad_ctl(self, mock_run):
+        mock_run.side_effect = OSError
         self.assertRaises(
             errors.MisconfigurationError,
             self.parser.update_runtime_variables)
 
-    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
-    def test_update_runtime_vars_bad_exit(self, mock_popen):
-        mock_popen().communicate.return_value = ("", "")
-        mock_popen.returncode = -1
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.run")
+    def test_update_runtime_vars_bad_exit(self, mock_run):
+        mock_proc = mock_run.return_value
+        mock_proc.stdout = ""
+        mock_proc.stderr = ""
+        mock_proc.returncode = -1
         self.assertRaises(
             errors.MisconfigurationError,
             self.parser.update_runtime_variables)

certbot-ci/certbot_integration_tests/certbot_tests/context.py

@@ -61,7 +61,7 @@ class IntegrationTestsContext:
         Execute certbot with given args, not renewing certificates by default.
         :param args: args to pass to certbot
         :param force_renew: set to False to not renew by default
-        :return: output of certbot execution
+        :return: stdout and stderr from certbot execution
         """
         command = ['--authenticator', 'standalone', '--installer', 'null']
         command.extend(args)

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -78,9 +78,9 @@ def test_registration_override(context):
 
 def test_prepare_plugins(context):
     """Test that plugins are correctly instantiated and displayed."""
-    output = context.certbot(['plugins', '--init', '--prepare'])
+    stdout, _ = context.certbot(['plugins', '--init', '--prepare'])
 
-    assert 'webroot' in output
+    assert 'webroot' in stdout
 
 
 def test_http_01(context):
@@ -407,9 +407,9 @@ def test_invalid_domain_with_dns_challenge(context):
         '--manual-cleanup-hook', context.manual_dns_cleanup_hook
     ])
 
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert context.get_domain('fail-dns1') not in output
+    assert context.get_domain('fail-dns1') not in stdout
 
 
 def test_reuse_key(context):
@@ -614,11 +614,11 @@ def test_revoke_and_unregister(context):
 
     context.certbot(['unregister'])
 
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert cert1 not in output
-    assert cert2 not in output
-    assert cert3 in output
+    assert cert1 not in stdout
+    assert cert2 not in stdout
+    assert cert3 in stdout
 
 
 def test_revoke_mutual_exclusive_flags(context):
@@ -630,7 +630,7 @@ def test_revoke_mutual_exclusive_flags(context):
             'revoke', '--cert-name', cert,
             '--cert-path', join(context.config_dir, 'live', cert, 'fullchain.pem')
         ])
-        assert 'Exactly one of --cert-path or --cert-name must be specified' in error.out
+    assert 'Exactly one of --cert-path or --cert-name must be specified' in error.value.stderr
 
 
 def test_revoke_multiple_lineages(context):
@@ -685,12 +685,12 @@ def test_wildcard_certificates(context):
 def test_ocsp_status_stale(context):
     """Test retrieval of OCSP statuses for staled config"""
     sample_data_path = misc.load_sample_data_path(context.workspace)
-    output = context.certbot(['certificates', '--config-dir', sample_data_path])
+    stdout, _ = context.certbot(['certificates', '--config-dir', sample_data_path])
 
-    assert output.count('TEST_CERT') == 2, ('Did not find two test certs as expected ({0})'
-                                            .format(output.count('TEST_CERT')))
-    assert output.count('EXPIRED') == 2, ('Did not find two expired certs as expected ({0})'
-                                          .format(output.count('EXPIRED')))
+    assert stdout.count('TEST_CERT') == 2, ('Did not find two test certs as expected ({0})'
+                                            .format(stdout.count('TEST_CERT')))
+    assert stdout.count('EXPIRED') == 2, ('Did not find two expired certs as expected ({0})'
+                                          .format(stdout.count('EXPIRED')))
 
 
 def test_ocsp_status_live(context):
@@ -699,20 +699,20 @@ def test_ocsp_status_live(context):
 
     # OSCP 1: Check live certificate OCSP status (VALID)
     context.certbot(['--domains', cert])
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert output.count('VALID') == 1, 'Expected {0} to be VALID'.format(cert)
-    assert output.count('EXPIRED') == 0, 'Did not expect {0} to be EXPIRED'.format(cert)
+    assert stdout.count('VALID') == 1, 'Expected {0} to be VALID'.format(cert)
+    assert stdout.count('EXPIRED') == 0, 'Did not expect {0} to be EXPIRED'.format(cert)
 
     # OSCP 2: Check live certificate OCSP status (REVOKED)
     context.certbot(['revoke', '--cert-name', cert, '--no-delete-after-revoke'])
     # Sometimes in oldest tests (using openssl binary and not cryptography), the OCSP status is
     # not seen immediately by Certbot as invalid. Waiting few seconds solves this transient issue.
     time.sleep(5)
-    output = context.certbot(['certificates'])
+    stdout, _ = context.certbot(['certificates'])
 
-    assert output.count('INVALID') == 1, 'Expected {0} to be INVALID'.format(cert)
-    assert output.count('REVOKED') == 1, 'Expected {0} to be REVOKED'.format(cert)
+    assert stdout.count('INVALID') == 1, 'Expected {0} to be INVALID'.format(cert)
+    assert stdout.count('REVOKED') == 1, 'Expected {0} to be REVOKED'.format(cert)
 
 
 def test_ocsp_renew(context):

certbot-ci/certbot_integration_tests/nginx_tests/context.py

@@ -51,6 +51,7 @@ class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
         with open(self.nginx_config_path, 'w') as file:
             file.write(self.nginx_config)
 
+        # pylint: disable=consider-using-with
         process = subprocess.Popen(['nginx', '-c', self.nginx_config_path, '-g', 'daemon off;'])
 
         assert process.poll() is None

certbot-ci/certbot_integration_tests/utils/acme_server.py

@@ -240,6 +240,7 @@ class ACMEServer:
         if not env:
             env = os.environ
         stdout = sys.stderr if force_stderr else self._stdout
+        # pylint: disable=consider-using-with
         process = subprocess.Popen(
             command, stdout=stdout, stderr=subprocess.STDOUT, cwd=cwd, env=env
         )

certbot-ci/certbot_integration_tests/utils/certbot_call.py

@@ -17,7 +17,7 @@ def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
     Invoke the certbot executable available in PATH in a test context for the given args.
     The test context consists in running certbot in debug mode, with various flags suitable
     for tests (eg. no ssl check, customizable ACME challenge ports and config directory ...).
-    This command captures stdout and returns it to the caller.
+    This command captures both stdout and stderr and returns it to the caller.
     :param list certbot_args: the arguments to pass to the certbot executable
     :param str directory_url: URL of the ACME directory server to use
     :param int http_01_port: port for the HTTP-01 challenges
@@ -25,13 +25,19 @@ def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
     :param str config_dir: certbot configuration directory to use
     :param str workspace: certbot current directory to use
     :param bool force_renew: set False to not force renew existing certificates (default: True)
-    :return: stdout as string
-    :rtype: str
+    :return: stdout and stderr as strings
+    :rtype: `tuple` of `str`
     """
     command, env = _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
                                      config_dir, workspace, force_renew)
 
-    return subprocess.check_output(command, universal_newlines=True, cwd=workspace, env=env)
+    proc = subprocess.run(command, stdout=subprocess.PIPE,
+                          stderr=subprocess.PIPE, check=False, universal_newlines=True,
+                          cwd=workspace, env=env)
+    print('--> Certbot log output was:')
+    print(proc.stderr)
+    proc.check_returncode()
+    return proc.stdout, proc.stderr
 
 
 def _prepare_environ(workspace):

certbot-ci/certbot_integration_tests/utils/dns_server.py

@@ -83,6 +83,7 @@ class DNSServer:
     def _start_bind(self):
         """Launch the BIND9 server as a Docker container"""
         addr_str = "{}:{}".format(BIND_BIND_ADDRESS[0], BIND_BIND_ADDRESS[1])
+        # pylint: disable=consider-using-with
         self.process = subprocess.Popen(
             [
                 "docker",

certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py

@@ -48,7 +48,6 @@ class Proxy(configurators_common.Proxy):
             setattr(self.le_config, "nginx_" + k, constants.os_constant(k))
 
         conf = configuration.NamespaceConfig(self.le_config)
-        zope.component.provideUtility(conf)
         self._configurator = configurator.NginxConfigurator(
             config=conf, name="nginx")
         self._configurator.prepare()

certbot-compatibility-test/certbot_compatibility_test/test_driver.py

@@ -10,6 +10,7 @@ import tempfile
 import time
 from typing import List
 from typing import Tuple
+import zope.component
 
 import OpenSSL
 from urllib3.util import connection
@@ -19,6 +20,7 @@ from acme import crypto_util
 from acme import messages
 from certbot import achallenges
 from certbot import errors as le_errors
+from certbot.display import util as display_util
 from certbot.tests import acme_util
 from certbot_compatibility_test import errors
 from certbot_compatibility_test import util
@@ -327,10 +329,17 @@ def setup_logging(args):
     root_logger.addHandler(handler)
 
 
+def setup_display():
+    """"Prepares IDisplay for the Certbot plugins """
+    displayer = display_util.NoninteractiveDisplay(sys.stdout)
+    zope.component.provideUtility(displayer)
+
+
 def main():
     """Main test script execution."""
     args = get_args()
     setup_logging(args)
+    setup_display()
 
     if args.plugin not in PLUGINS:
         raise errors.Error("Unknown plugin {0}".format(args.plugin))

certbot-compatibility-test/setup.py

@@ -3,7 +3,7 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
 install_requires = [
     'certbot',

certbot-dns-cloudflare/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.1.0

certbot-dns-cloudflare/setup.py

@@ -4,10 +4,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
     'cloudflare>=1.5.1',
     'setuptools>=39.0.1',
@@ -16,8 +14,11 @@ install_requires = [
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.29.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-cloudflare/tests/dns_cloudflare_test.py

@@ -41,7 +41,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         # _get_cloudflare_client | pylint: disable=protected-access
         self.auth._get_cloudflare_client = mock.MagicMock(return_value=self.mock_client)
 
-    def test_perform(self):
+    @test_util.patch_get_utility()
+    def test_perform(self, unused_mock_get_utility):
         self.auth.perform([self.achall])
 
         expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)]
@@ -55,7 +56,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         expected = [mock.call.del_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY)]
         self.assertEqual(expected, self.mock_client.mock_calls)
 
-    def test_api_token(self):
+    @test_util.patch_get_utility()
+    def test_api_token(self, unused_mock_get_utility):
         dns_test_common.write({"cloudflare_api_token": API_TOKEN},
                               self.config.cloudflare_credentials)
         self.auth.perform([self.achall])

certbot-dns-cloudxns/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-cloudxns/setup.py

@@ -4,20 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-digitalocean/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.1.0

certbot-dns-digitalocean/setup.py

@@ -4,10 +4,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
     'python-digitalocean>=1.11', # 1.15.0 or newer is recommended for TTL support
     'setuptools>=39.0.1',
@@ -16,8 +14,11 @@ install_requires = [
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.29.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-digitalocean/tests/dns_digitalocean_test.py

@@ -37,7 +37,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         # _get_digitalocean_client | pylint: disable=protected-access
         self.auth._get_digitalocean_client = mock.MagicMock(return_value=self.mock_client)
 
-    def test_perform(self):
+    @test_util.patch_get_utility()
+    def test_perform(self, unused_mock_get_utility):
         self.auth.perform([self.achall])
 
         expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, 30)]

certbot-dns-dnsimple/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-dnsimple/setup.py

@@ -4,10 +4,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
     'setuptools>=39.0.1',
     'zope.interface',
@@ -15,8 +13,11 @@ install_requires = [
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '
@@ -32,7 +33,7 @@ if os.environ.get('SNAP_BUILD'):
 # which allows us to potentially upgrade our packages in these distros
 # as necessary.
 if os.environ.get('CERTBOT_OLDEST') == '1':
-    install_requires.append('dns-lexicon>=2.2.1')
+    install_requires.append('dns-lexicon>=3.1.0')  # Changed parameter name
 else:
     install_requires.append('dns-lexicon>=3.2.1')
 

certbot-dns-dnsmadeeasy/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-dnsmadeeasy/setup.py

@@ -4,20 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-gehirn/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-gehirn/setup.py

@@ -4,19 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    'dns-lexicon>=2.1.22',
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-google/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.1.0

certbot-dns-google/setup.py

@@ -4,10 +4,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
     'google-api-python-client>=1.5.5',
     'oauth2client>=4.0',
@@ -19,8 +17,11 @@ install_requires = [
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.29.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-google/tests/dns_google_test.py

@@ -43,7 +43,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         # _get_google_client | pylint: disable=protected-access
         self.auth._get_google_client = mock.MagicMock(return_value=self.mock_client)
 
-    def test_perform(self):
+    @test_util.patch_get_utility()
+    def test_perform(self, unused_mock_get_utility):
         self.auth.perform([self.achall])
 
         expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)]
@@ -58,7 +59,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         self.assertEqual(expected, self.mock_client.mock_calls)
 
     @mock.patch('httplib2.Http.request', side_effect=ServerNotFoundError)
-    def test_without_auth(self, unused_mock):
+    @test_util.patch_get_utility()
+    def test_without_auth(self, unused_mock_get_utility, unused_mock):
         self.config.google_credentials = None
         self.assertRaises(PluginError, self.auth.perform, [self.achall])
 

certbot-dns-linode/local-oldest-requirements.txt

@@ -1,4 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0
-dns-lexicon==2.2.3

certbot-dns-linode/setup.py

@@ -4,19 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    'dns-lexicon>=2.2.3',
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-luadns/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-luadns/setup.py

@@ -4,20 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-nsone/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-nsone/setup.py

@@ -4,20 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'dns-lexicon>=2.2.1',  # Support for >1 TXT record per name
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-ovh/local-oldest-requirements.txt

@@ -1,4 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0
-dns-lexicon==2.7.14

certbot-dns-ovh/setup.py

@@ -4,20 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'dns-lexicon>=2.7.14',  # Correct proxy use on OVH provider
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-rfc2136/certbot_dns_rfc2136/_internal/dns_rfc2136.py

@@ -27,7 +27,7 @@ DEFAULT_NETWORK_TIMEOUT = 45
 class Authenticator(dns_common.DNSAuthenticator):
     """DNS Authenticator using RFC 2136 Dynamic Updates
 
-    This Authenticator uses RFC 2136 Dynamic Updates to fulfull a dns-01 challenge.
+    This Authenticator uses RFC 2136 Dynamic Updates to fulfill a dns-01 challenge.
     """
 
     ALGORITHMS = {

certbot-dns-rfc2136/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.1.0

certbot-dns-rfc2136/setup.py

@@ -4,10 +4,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
     'dnspython',
     'setuptools>=39.0.1',
@@ -16,8 +14,11 @@ install_requires = [
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.29.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-rfc2136/tests/dns_rfc2136_test.py

@@ -42,7 +42,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         # _get_rfc2136_client | pylint: disable=protected-access
         self.auth._get_rfc2136_client = mock.MagicMock(return_value=self.mock_client)
 
-    def test_perform(self):
+    @test_util.patch_get_utility()
+    def test_perform(self, unused_mock_get_utility):
         self.auth.perform([self.achall])
 
         expected = [mock.call.add_txt_record('_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)]
@@ -65,7 +66,8 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
                           self.auth.perform,
                           [self.achall])
 
-    def test_valid_algorithm_passes(self):
+    @test_util.patch_get_utility()
+    def test_valid_algorithm_passes(self, unused_mock_get_utility):
         config = VALID_CONFIG.copy()
         config["rfc2136_algorithm"] = "HMAC-sha512"
         dns_test_common.write(config, self.config.rfc2136_credentials)

certbot-dns-route53/.gitignore

@@ -1,62 +0,0 @@
-# Byte-compiled / optimized / DLL files
-__pycache__/
-*.py[cod]
-*$py.class
-
-# C extensions
-*.so
-
-# Distribution / packaging
-.Python
-env/
-build/
-develop-eggs/
-dist/
-downloads/
-eggs/
-.eggs/
-lib/
-lib64/
-parts/
-sdist/
-var/
-*.egg-info/
-.installed.cfg
-*.egg
-
-# PyInstaller
-#  Usually these files are written by a python script from a template
-#  before PyInstaller builds the exe, so as to inject date/other infos into it.
-*.manifest
-*.spec
-
-# Installer logs
-pip-log.txt
-pip-delete-this-directory.txt
-
-# Unit test / coverage reports
-htmlcov/
-.tox/
-.coverage
-.coverage.*
-.cache
-nosetests.xml
-coverage.xml
-*,cover
-.hypothesis/
-
-# Translations
-*.mo
-*.pot
-
-# Django stuff:
-*.log
-
-# Sphinx documentation
-docs/_build/
-
-# PyBuilder
-target/
-
-#Ipython Notebook
-.ipynb_checkpoints

certbot-dns-route53/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.29.0
-certbot[dev]==1.1.0

certbot-dns-route53/setup.py

@@ -4,10 +4,8 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
     'boto3',
     'setuptools>=39.0.1',
@@ -16,8 +14,11 @@ install_requires = [
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.29.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-dns-sakuracloud/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==0.31.0
-certbot[dev]==1.1.0

certbot-dns-sakuracloud/setup.py

@@ -4,19 +4,21 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Please update tox.ini when modifying dependency version requirements
 install_requires = [
-    'dns-lexicon>=2.1.23',
+    'dns-lexicon>=3.1.0',  # Changed `rtype` parameter name
     'setuptools>=39.0.1',
     'zope.interface',
 ]
 
 if not os.environ.get('SNAP_BUILD'):
     install_requires.extend([
-        'acme>=0.31.0',
-        'certbot>=1.1.0',
+        # We specify the minimum acme and certbot version as the current plugin
+        # version for simplicity. See
+        # https://github.com/certbot/certbot/issues/8761 for more info.
+        f'acme>={version}',
+        f'certbot>={version}',
     ])
 elif 'bdist_wheel' in sys.argv[1:]:
     raise RuntimeError('Unset SNAP_BUILD when building wheels '

certbot-nginx/certbot_nginx/_internal/configurator.py

@@ -24,6 +24,7 @@ from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
 from certbot import util
+from certbot.display import util as display_util
 from certbot.compat import os
 from certbot.plugins import common
 from certbot_nginx._internal import constants
@@ -234,6 +235,8 @@ class NginxConfigurator(common.Installer):
         vhosts = self.choose_vhosts(domain, create_if_no_match=True)
         for vhost in vhosts:
             self._deploy_cert(vhost, cert_path, key_path, chain_path, fullchain_path)
+            display_util.notify("Successfully deployed certificate for {} to {}"
+                                .format(domain, vhost.filep))
 
     def _deploy_cert(self, vhost, cert_path, key_path, chain_path, fullchain_path):  # pylint: disable=unused-argument
         """
@@ -675,8 +678,9 @@ class NginxConfigurator(common.Installer):
         """Generate invalid certs that let us create ssl directives for Nginx"""
         # TODO: generate only once
         tmp_dir = os.path.join(self.config.work_dir, "snakeoil")
-        le_key = crypto_util.init_save_key(
-            key_size=1024, key_dir=tmp_dir, keyname="key.pem")
+        le_key = crypto_util.generate_key(
+            key_size=1024, key_dir=tmp_dir, keyname="key.pem",
+            strict_permissions=self.config.strict_permissions)
         key = OpenSSL.crypto.load_privatekey(
             OpenSSL.crypto.FILETYPE_PEM, le_key.pem)
         cert = acme_crypto_util.gen_ss_cert(key, domains=[socket.gethostname()])
@@ -766,7 +770,7 @@ class NginxConfigurator(common.Installer):
             raise errors.PluginError(
                 "Unsupported enhancement: {0}".format(enhancement))
         except errors.PluginError:
-            logger.warning("Failed %s for %s", enhancement, domain)
+            logger.error("Failed %s for %s", enhancement, domain)
             raise
 
     def _has_certbot_redirect(self, vhost, domain):
@@ -985,13 +989,14 @@ class NginxConfigurator(common.Installer):
             Unable to run Nginx version command
         """
         try:
-            proc = subprocess.Popen(
+            proc = subprocess.run(
                 [self.conf('ctl'), "-c", self.nginx_conf, "-V"],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
                 universal_newlines=True,
+                check=False,
                 env=util.env_no_snap_for_external_calls())
-            text = proc.communicate()[1]  # nginx prints output to stderr
+            text = proc.stderr  # nginx prints output to stderr
         except (OSError, ValueError) as error:
             logger.debug(str(error), exc_info=True)
             raise errors.PluginError(
@@ -1075,6 +1080,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)
     ###################################################
@@ -1224,10 +1236,9 @@ def nginx_restart(nginx_ctl, nginx_conf, sleep_duration):
     try:
         reload_output: Text = u""
         with tempfile.TemporaryFile() as out:
-            proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf, "-s", "reload"],
-                                    env=util.env_no_snap_for_external_calls(),
-                                    stdout=out, stderr=out)
-            proc.communicate()
+            proc = subprocess.run([nginx_ctl, "-c", nginx_conf, "-s", "reload"],
+                                  env=util.env_no_snap_for_external_calls(),
+                                  stdout=out, stderr=out, check=False)
             out.seek(0)
             reload_output = out.read().decode("utf-8")
 
@@ -1237,9 +1248,8 @@ def nginx_restart(nginx_ctl, nginx_conf, sleep_duration):
             # Write to temporary files instead of piping because of communication issues on Arch
             # https://github.com/certbot/certbot/issues/4324
             with tempfile.TemporaryFile() as out:
-                nginx_proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf],
-                    stdout=out, stderr=out, env=util.env_no_snap_for_external_calls())
-                nginx_proc.communicate()
+                nginx_proc = subprocess.run([nginx_ctl, "-c", nginx_conf],
+                    stdout=out, stderr=out, env=util.env_no_snap_for_external_calls(), check=False)
                 if nginx_proc.returncode != 0:
                     out.seek(0)
                     # Enter recovery routine...

certbot-nginx/certbot_nginx/_internal/http_01.py

@@ -128,12 +128,12 @@ class NginxHttp01(common.ChallengePerformer):
                 ipv6_addr = ipv6_addr + " ipv6only=on"
             addresses = [obj.Addr.fromstring(default_addr),
                          obj.Addr.fromstring(ipv6_addr)]
-            logger.info(("Using default addresses %s and %s for authentication."),
+            logger.debug(("Using default addresses %s and %s for authentication."),
                         default_addr,
                         ipv6_addr)
         else:
             addresses = [obj.Addr.fromstring(default_addr)]
-            logger.info("Using default address %s for authentication.",
+            logger.debug("Using default address %s for authentication.",
                         default_addr)
         return addresses
 

certbot-nginx/certbot_nginx/_internal/parser.py

@@ -217,7 +217,7 @@ class NginxParser:
                                "character. Only UTF-8 encoding is "
                                "supported.", item)
             except pyparsing.ParseException as err:
-                logger.debug("Could not parse file: %s due to %s", item, err)
+                logger.warning("Could not parse file: %s due to %s", item, err)
         return trees
 
     def _find_config_root(self):
@@ -430,7 +430,7 @@ def _parse_ssl_options(ssl_options):
             logger.warning("Could not read file: %s due to invalid character. "
                            "Only UTF-8 encoding is supported.", ssl_options)
         except pyparsing.ParseBaseException as err:
-            logger.debug("Could not parse file: %s due to %s", ssl_options, err)
+            logger.warning("Could not parse file: %s due to %s", ssl_options, err)
     return []
 
 def _do_for_subarray(entry, condition, func, path=None):

certbot-nginx/local-oldest-requirements.txt

@@ -1,3 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==1.6.0
-certbot[dev]==1.6.0

certbot-nginx/setup.py

@@ -1,13 +1,14 @@
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.16.0.dev0'
+version = '1.17.0.dev0'
 
-# Remember to update local-oldest-requirements.txt when changing the minimum
-# acme/certbot version.
 install_requires = [
-    'acme>=1.4.0',
-    'certbot>=1.6.0',
+    # We specify the minimum acme and certbot version as the current plugin
+    # version for simplicity. See
+    # https://github.com/certbot/certbot/issues/8761 for more info.
+    f'acme>={version}',
+    f'certbot>={version}',
     'PyOpenSSL>=17.3.0',
     'pyparsing>=2.2.0',
     'setuptools>=39.0.1',

certbot-nginx/tests/configurator_test.py

@@ -31,6 +31,10 @@ class NginxConfiguratorTest(util.NginxTest):
         self.config = self.get_nginx_configurator(
             self.config_path, self.config_dir, self.work_dir, self.logs_dir)
 
+        patch = mock.patch('certbot_nginx._internal.configurator.display_util.notify')
+        self.mock_notify = patch.start()
+        self.addCleanup(patch.stop)
+
     @mock.patch("certbot_nginx._internal.configurator.util.exe_exists")
     def test_prepare_no_install(self, mock_exe_exists):
         mock_exe_exists.return_value = False
@@ -42,15 +46,16 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertEqual(13, len(self.config.parser.parsed))
 
     @mock.patch("certbot_nginx._internal.configurator.util.exe_exists")
-    @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
-    def test_prepare_initializes_version(self, mock_popen, mock_exe_exists):
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["nginx version: nginx/1.6.2",
+    @mock.patch("certbot_nginx._internal.configurator.subprocess.run")
+    def test_prepare_initializes_version(self, mock_run, mock_exe_exists):
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["nginx version: nginx/1.6.2",
                            "built by clang 6.0 (clang-600.0.56)"
                            " (based on LLVM 3.5svn)",
                            "TLS SNI support enabled",
                            "configure arguments: --prefix=/usr/local/Cellar/"
-                           "nginx/1.6.2 --with-http_ssl_module"]))
+                           "nginx/1.6.2 --with-http_ssl_module"])
 
         mock_exe_exists.return_value = True
 
@@ -347,141 +352,149 @@ class NginxConfiguratorTest(util.NginxTest):
         self.assertEqual(mock_revert.call_count, 1)
         self.assertEqual(mock_restart.call_count, 2)
 
-    @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
-    def test_get_version(self, mock_popen):
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["nginx version: nginx/1.4.2",
+    @mock.patch("certbot_nginx._internal.configurator.subprocess.run")
+    def test_get_version(self, mock_run):
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["nginx version: nginx/1.4.2",
                            "built by clang 6.0 (clang-600.0.56)"
                            " (based on LLVM 3.5svn)",
                            "TLS SNI support enabled",
                            "configure arguments: --prefix=/usr/local/Cellar/"
-                           "nginx/1.6.2 --with-http_ssl_module"]))
+                           "nginx/1.6.2 --with-http_ssl_module"])
         self.assertEqual(self.config.get_version(), (1, 4, 2))
 
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["nginx version: nginx/0.9",
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["nginx version: nginx/0.9",
                            "built by clang 6.0 (clang-600.0.56)"
                            " (based on LLVM 3.5svn)",
                            "TLS SNI support enabled",
-                           "configure arguments: --with-http_ssl_module"]))
+                           "configure arguments: --with-http_ssl_module"])
         self.assertEqual(self.config.get_version(), (0, 9))
 
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["blah 0.0.1",
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["blah 0.0.1",
                            "built by clang 6.0 (clang-600.0.56)"
                            " (based on LLVM 3.5svn)",
                            "TLS SNI support enabled",
-                           "configure arguments: --with-http_ssl_module"]))
+                           "configure arguments: --with-http_ssl_module"])
         self.assertRaises(errors.PluginError, self.config.get_version)
 
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["nginx version: nginx/1.4.2",
-                           "TLS SNI support enabled"]))
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["nginx version: nginx/1.4.2",
+                           "TLS SNI support enabled"])
         self.assertRaises(errors.PluginError, self.config.get_version)
 
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["nginx version: nginx/1.4.2",
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["nginx version: nginx/1.4.2",
                            "built by clang 6.0 (clang-600.0.56)"
                            " (based on LLVM 3.5svn)",
-                           "configure arguments: --with-http_ssl_module"]))
+                           "configure arguments: --with-http_ssl_module"])
         self.assertRaises(errors.PluginError, self.config.get_version)
 
-        mock_popen().communicate.return_value = (
-            "", "\n".join(["nginx version: nginx/0.8.1",
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = "\n".join(
+                          ["nginx version: nginx/0.8.1",
                            "built by clang 6.0 (clang-600.0.56)"
                            " (based on LLVM 3.5svn)",
                            "TLS SNI support enabled",
-                           "configure arguments: --with-http_ssl_module"]))
+                           "configure arguments: --with-http_ssl_module"])
         self.assertRaises(errors.NotSupportedError, self.config.get_version)
 
-        mock_popen.side_effect = OSError("Can't find program")
+        mock_run.side_effect = OSError("Can't find program")
         self.assertRaises(errors.PluginError, self.config.get_version)
 
-    @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
-    def test_get_openssl_version(self, mock_popen):
+    @mock.patch("certbot_nginx._internal.configurator.subprocess.run")
+    def test_get_openssl_version(self, mock_run):
         # pylint: disable=protected-access
-        mock_popen().communicate.return_value = (
-            "", """
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = """
                 nginx version: nginx/1.15.5
                 built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)
                 built with OpenSSL 1.0.2g  1 Mar 2016
                 TLS SNI support enabled
                 configure arguments:
-            """)
+            """
         self.assertEqual(self.config._get_openssl_version(), "1.0.2g")
 
-        mock_popen().communicate.return_value = (
-            "", """
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = """
                 nginx version: nginx/1.15.5
                 built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)
                 built with OpenSSL 1.0.2-beta1  1 Mar 2016
                 TLS SNI support enabled
                 configure arguments:
-            """)
+            """
         self.assertEqual(self.config._get_openssl_version(), "1.0.2-beta1")
 
-        mock_popen().communicate.return_value = (
-            "", """
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = """
                 nginx version: nginx/1.15.5
                 built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)
                 built with OpenSSL 1.0.2  1 Mar 2016
                 TLS SNI support enabled
                 configure arguments:
-            """)
+            """
         self.assertEqual(self.config._get_openssl_version(), "1.0.2")
 
-        mock_popen().communicate.return_value = (
-            "", """
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = """
                 nginx version: nginx/1.15.5
                 built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)
                 built with OpenSSL 1.0.2g  1 Mar 2016 (running with OpenSSL 1.0.2a  1 Mar 2016)
                 TLS SNI support enabled
                 configure arguments:
-            """)
+            """
         self.assertEqual(self.config._get_openssl_version(), "1.0.2a")
 
-        mock_popen().communicate.return_value = (
-            "", """
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = """
                 nginx version: nginx/1.15.5
                 built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)
                 built with LibreSSL 2.2.2
                 TLS SNI support enabled
                 configure arguments:
-            """)
+            """
         self.assertEqual(self.config._get_openssl_version(), "")
 
-        mock_popen().communicate.return_value = (
-            "", """
+        mock_run.return_value.stdout = ""
+        mock_run.return_value.stderr = """
                 nginx version: nginx/1.15.5
                 built by gcc 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9)
                 TLS SNI support enabled
                 configure arguments:
-            """)
+            """
         self.assertEqual(self.config._get_openssl_version(), "")
 
-    @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
+    @mock.patch("certbot_nginx._internal.configurator.subprocess.run")
     @mock.patch("certbot_nginx._internal.configurator.time")
-    def test_nginx_restart(self, mock_time, mock_popen):
-        mocked = mock_popen()
-        mocked.communicate.return_value = ('', '')
+    def test_nginx_restart(self, mock_time, mock_run):
+        mocked = mock_run.return_value
+        mocked.stdout = ''
+        mocked.stderr = ''
         mocked.returncode = 0
         self.config.restart()
-        self.assertEqual(mocked.communicate.call_count, 1)
+        self.assertEqual(mock_run.call_count, 1)
         mock_time.sleep.assert_called_once_with(0.1234)
 
-    @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
+    @mock.patch("certbot_nginx._internal.configurator.subprocess.run")
     @mock.patch("certbot_nginx._internal.configurator.logger.debug")
-    def test_nginx_restart_fail(self, mock_log_debug, mock_popen):
-        mocked = mock_popen()
-        mocked.communicate.return_value = ('', '')
+    def test_nginx_restart_fail(self, mock_log_debug, mock_run):
+        mocked = mock_run.return_value
+        mocked.stdout = ''
+        mocked.stderr = ''
         mocked.returncode = 1
         self.assertRaises(errors.MisconfigurationError, self.config.restart)
-        self.assertEqual(mocked.communicate.call_count, 2)
+        self.assertEqual(mock_run.call_count, 2)
         mock_log_debug.assert_called_once_with("nginx reload failed:\n%s", "")
 
-    @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
-    def test_no_nginx_start(self, mock_popen):
-        mock_popen.side_effect = OSError("Can't find program")
+    @mock.patch("certbot_nginx._internal.configurator.subprocess.run")
+    def test_no_nginx_start(self, mock_run):
+        mock_run.side_effect = OSError("Can't find program")
         self.assertRaises(errors.MisconfigurationError, self.config.restart)
 
     @mock.patch("certbot.util.run_script")

certbot-nginx/tests/test_util.py

@@ -9,7 +9,6 @@ try:
 except ImportError: # pragma: no cover
     from unittest import mock # type: ignore
 import pkg_resources
-import zope.component
 
 from certbot import util
 from certbot.compat import os
@@ -79,9 +78,6 @@ class NginxTest(test_util.ConfigTestCase):
                     openssl_version=openssl_version)
                 config.prepare()
 
-        # Provide general config utility.
-        zope.component.provideUtility(self.configuration)
-
         return config
 
 

certbot/.gitignore

@@ -1 +0,0 @@
-*.crt

certbot/CHANGELOG.md

@@ -2,7 +2,7 @@
 
 Certbot adheres to [Semantic Versioning](https://semver.org/).
 
-## 1.16.0 - master
+## 1.17.0 - master
 
 ### Added
 
@@ -10,7 +10,12 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* We changed how dependencies are specified between Certbot packages. For this
+  and future releases, higher level Certbot components will require that lower
+  level components are the same version or newer. More specifically, version X
+  of the Certbot package will now always require acme>=X and version Y of a
+  plugin package will always require acme>=Y and certbot=>Y. Specifying
+  dependencies in this way simplifies testing and development.
 
 ### Fixed
 
@@ -18,6 +23,36 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 More details about these changes can be found on our GitHub repo.
 
+## 1.16.0 - 2021-06-01
+
+### Added
+
+*
+
+### Changed
+
+* DNS plugins based on lexicon now require dns-lexicon >= v3.1.0
+* Use UTF-8 encoding for renewal configuration files
+* Windows installer now cleans up old Certbot dependency packages
+  before installing the new ones to avoid version conflicts.
+* This release contains a substantial command-line UX overhaul,
+  based on previous user research. The main goal was to streamline
+  and clarify output. If you would like to see more verbose output, use
+  the -v or -vv flags. UX improvements are an iterative process and
+  the Certbot team welcomes constructive feedback.
+* Functions `certbot.crypto_util.init_save_key` and `certbot.crypto_util.init_save_csr`,
+  whose behaviors rely on the global Certbot `config` singleton, are deprecated and will
+  be removed in a future release. Please use `certbot.crypto_util.generate_key` and
+  `certbot.crypto_util.generate_csr` instead.
+
+### Fixed
+
+* Fix TypeError due to incompatibility with lexicon >= v3.6.0
+* Installers (e.g. nginx, Apache) were being restarted unnecessarily after dry-run renewals.
+* Colors and bold text should properly render in all supported versions of Windows.
+
+More details about these changes can be found on our GitHub repo.
+
 ## 1.15.0 - 2021-05-04
 
 ### Added

certbot/certbot/__init__.py

@@ -1,3 +1,3 @@
 """Certbot client."""
 # version number like 1.2.3a0, must have at least 2 parts, like 1.2
-__version__ = '1.16.0.dev0'
+__version__ = '1.17.0.dev0'

certbot/certbot/_internal/auth_handler.py

@@ -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__)
 
@@ -42,11 +44,12 @@ class AuthHandler:
         self.account = account
         self.pref_challs = pref_challs
 
-    def handle_authorizations(self, orderr, best_effort=False, max_retries=30):
+    def handle_authorizations(self, orderr, config, best_effort=False, max_retries=30):
         """
         Retrieve all authorizations, perform all challenges required to validate
         these authorizations, then poll and wait for the authorization to be checked.
         :param acme.messages.OrderResource orderr: must have authorizations filled in
+        :param interfaces.IConfig config: current Certbot configuration
         :param bool best_effort: if True, not all authorizations need to be validated (eg. renew)
         :param int max_retries: maximum number of retries to poll authorizations
         :returns: list of all validated authorizations
@@ -70,8 +73,6 @@ class AuthHandler:
                 resps = self.auth.perform(achalls)
 
                 # If debug is on, wait for user input before starting the verification process.
-                logger.info('Waiting for verification...')
-                config = zope.component.getUtility(interfaces.IConfig)
                 if config.debug_challenges:
                     notify = zope.component.getUtility(interfaces.IDisplay).notification
                     notify('Challenges loaded. Press continue to submit to CA. '
@@ -88,6 +89,7 @@ class AuthHandler:
                 self.acme.answer_challenge(achall.challb, resp)
 
             # Wait for authorizations to be checked.
+            logger.info('Waiting for verification...')
             self._poll_authorizations(authzrs, max_retries, best_effort)
 
             # Keep validated authorizations only. If there is none, no certificate can be issued.
@@ -148,7 +150,7 @@ class AuthHandler:
             authzrs_failed = [authzr for authzr, _ in authzrs_to_check.values()
                               if authzr.body.status == messages.STATUS_INVALID]
             for authzr_failed in authzrs_failed:
-                logger.warning('Challenge failed for domain %s',
+                logger.info('Challenge failed for domain %s',
                                authzr_failed.body.identifier.value)
             # Accumulating all failed authzrs to build a consolidated report
             # on them at the end of the polling.
@@ -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)

certbot/certbot/_internal/client.py

@@ -9,7 +9,6 @@ from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives.asymmetric.rsa import generate_private_key  # type: ignore
 import josepy as jose
 import OpenSSL
-import zope.component
 
 from acme import client as acme_client
 from acme import crypto_util as acme_crypto_util
@@ -18,7 +17,6 @@ from acme import messages
 import certbot
 from certbot import crypto_util
 from certbot import errors
-from certbot import interfaces
 from certbot import util
 from certbot._internal import account
 from certbot._internal import auth_handler
@@ -30,6 +28,7 @@ from certbot._internal import storage
 from certbot._internal.plugins import selection as plugin_selection
 from certbot.compat import os
 from certbot.display import ops as display_ops
+from certbot.display import util as display_util
 
 logger = logging.getLogger(__name__)
 
@@ -154,7 +153,7 @@ def register(config, account_storage, tos_cb=None):
         if not config.register_unsafely_without_email:
             msg = ("No email was provided and "
                    "--register-unsafely-without-email was not present.")
-            logger.warning(msg)
+            logger.error(msg)
             raise errors.Error(msg)
         if not config.dry_run:
             logger.debug("Registering without email!")
@@ -276,7 +275,7 @@ class Client:
         if self.auth_handler is None:
             msg = ("Unable to obtain certificate because authenticator is "
                    "not set.")
-            logger.warning(msg)
+            logger.error(msg)
             raise errors.Error(msg)
         if self.account.regr is None:
             raise errors.Error("Please register with the ACME server first.")
@@ -335,7 +334,7 @@ class Client:
             key = None
 
         key_size = self.config.rsa_key_size
-        elliptic_curve = None
+        elliptic_curve = "secp256r1"
 
         # key-type defaults to a list, but we are only handling 1 currently
         if isinstance(self.config.key_type, list):
@@ -363,13 +362,15 @@ class Client:
                            data=acme_crypto_util.make_csr(
                                key.pem, domains, self.config.must_staple))
         else:
-            key = key or crypto_util.init_save_key(
+            key = key or crypto_util.generate_key(
                 key_size=key_size,
                 key_dir=self.config.key_dir,
                 key_type=self.config.key_type,
                 elliptic_curve=elliptic_curve,
+                strict_permissions=self.config.strict_permissions,
             )
-            csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir)
+            csr = crypto_util.generate_csr(key, domains, self.config.csr_dir,
+                                           self.config.must_staple, self.config.strict_permissions)
 
         orderr = self._get_order_and_authorizations(csr.data, self.config.allow_subset_of_names)
         authzr = orderr.authorizations
@@ -421,7 +422,7 @@ class Client:
                 logger.warning("Certbot was unable to obtain fresh authorizations for every domain"
                                ". The dry run will continue, but results may not be accurate.")
 
-        authzr = self.auth_handler.handle_authorizations(orderr, best_effort)
+        authzr = self.auth_handler.handle_authorizations(orderr, self.config, best_effort)
         return orderr.update(authorizations=authzr)
 
     def obtain_and_enroll_certificate(self, domains, certname):
@@ -506,8 +507,6 @@ class Client:
             cert_file.write(cert_pem)
         finally:
             cert_file.close()
-        logger.info("Server issued certificate; certificate written to %s",
-                    abs_cert_path)
 
         chain_file, abs_chain_path =\
                 _open_pem_file('chain_path', chain_path)
@@ -519,8 +518,7 @@ class Client:
 
         return abs_cert_path, abs_chain_path, abs_fullchain_path
 
-    def deploy_certificate(self, domains, privkey_path,
-                           cert_path, chain_path, fullchain_path):
+    def deploy_certificate(self, domains, privkey_path, cert_path, chain_path, fullchain_path):
         """Install certificate
 
         :param list domains: list of domains to install the certificate
@@ -530,13 +528,15 @@ class Client:
 
         """
         if self.installer is None:
-            logger.warning("No installer specified, client is unable to deploy"
+            logger.error("No installer specified, client is unable to deploy"
                            "the certificate")
             raise errors.Error("No installer available")
 
         chain_path = None if chain_path is None else os.path.abspath(chain_path)
 
-        msg = ("Unable to install the certificate")
+        display_util.notify("Deploying certificate")
+
+        msg = "Could not install certificate"
         with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg):
             for dom in domains:
                 self.installer.deploy_cert(
@@ -568,7 +568,7 @@ class Client:
 
         """
         if self.installer is None:
-            logger.warning("No installer is specified, there isn't any "
+            logger.error("No installer is specified, there isn't any "
                            "configuration to enhance.")
             raise errors.Error("No installer available")
 
@@ -589,7 +589,7 @@ class Client:
                     self.apply_enhancement(domains, enhancement_name, option)
                     enhanced = True
             elif config_value:
-                logger.warning(
+                logger.error(
                     "Option %s is not supported by the selected installer. "
                     "Skipping enhancement.", config_name)
 
@@ -612,22 +612,20 @@ class Client:
 
 
         """
-        msg = ("We were unable to set up enhancement %s for your server, "
-               "however, we successfully installed your certificate."
-               % (enhancement))
+        msg = f"Could not set up {enhancement} enhancement"
         with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg):
             for dom in domains:
                 try:
                     self.installer.enhance(dom, enhancement, options)
                 except errors.PluginEnhancementAlreadyPresent:
                     if enhancement == "ensure-http-header":
-                        logger.warning("Enhancement %s was already set.",
+                        logger.info("Enhancement %s was already set.",
                                 options)
                     else:
-                        logger.warning("Enhancement %s was already set.",
+                        logger.info("Enhancement %s was already set.",
                                 enhancement)
                 except errors.PluginError:
-                    logger.warning("Unable to set enhancement %s for %s",
+                    logger.error("Unable to set enhancement %s for %s",
                             enhancement, dom)
                     raise
 
@@ -640,8 +638,7 @@ class Client:
 
         """
         self.installer.recovery_routine()
-        reporter = zope.component.getUtility(interfaces.IReporter)
-        reporter.add_message(success_msg, reporter.HIGH_PRIORITY)
+        display_util.notify(success_msg)
 
     def _rollback_and_restart(self, success_msg):
         """Rollback the most recent checkpoint and restart the webserver
@@ -649,20 +646,19 @@ class Client:
         :param str success_msg: message to show on successful rollback
 
         """
-        logger.critical("Rolling back to previous server configuration...")
-        reporter = zope.component.getUtility(interfaces.IReporter)
+        logger.info("Rolling back to previous server configuration...")
         try:
             self.installer.rollback_checkpoints()
             self.installer.restart()
         except:
-            reporter.add_message(
+            logger.error(
                 "An error occurred and we failed to restore your config and "
                 "restart your server. Please post to "
                 "https://community.letsencrypt.org/c/help "
-                "with details about your configuration and this error you received.",
-                reporter.HIGH_PRIORITY)
+                "with details about your configuration and this error you received."
+            )
             raise
-        reporter.add_message(success_msg, reporter.HIGH_PRIORITY)
+        display_util.notify(success_msg)
 
 
 def validate_key_csr(privkey, csr=None):
@@ -761,5 +757,3 @@ def _save_chain(chain_pem, chain_file):
         chain_file.write(chain_pem)
     finally:
         chain_file.close()
-
-    logger.info("Cert chain written to %s", chain_file.name)

certbot/certbot/_internal/constants.py

@@ -22,7 +22,7 @@ CLI_DEFAULTS = dict(
     ],
 
     # Main parser
-    verbose_count=-int(logging.INFO / 10),
+    verbose_count=-int(logging.WARNING / 10),
     text_mode=False,
     max_log_backups=1000,
     preconfigured_renewal=False,
@@ -139,7 +139,7 @@ REVOCATION_REASONS = {
 
 """Defaults for CLI flags and `.IConfig` attributes."""
 
-QUIET_LOGGING_LEVEL = logging.WARNING
+QUIET_LOGGING_LEVEL = logging.ERROR
 """Logging level to use in quiet mode."""
 
 RENEWER_DEFAULTS = dict(

certbot/certbot/_internal/hooks.py

@@ -9,6 +9,7 @@ from certbot import util
 from certbot.compat import filesystem
 from certbot.compat import misc
 from certbot.compat import os
+from certbot.display import ops as display_ops
 from certbot.plugins import util as plug_util
 
 logger = logging.getLogger(__name__)
@@ -210,7 +211,7 @@ def _run_deploy_hook(command, domains, lineage_path, dry_run):
 
     """
     if dry_run:
-        logger.warning("Dry run: skipping deploy hook command: %s",
+        logger.info("Dry run: skipping deploy hook command: %s",
                        command)
         return
 
@@ -227,7 +228,9 @@ def _run_hook(cmd_name, shell_cmd):
     :type shell_cmd: `list` of `str` or `str`
 
     :returns: stderr if there was any"""
-    err, _ = misc.execute_command(cmd_name, shell_cmd, env=util.env_no_snap_for_external_calls())
+    returncode, err, out = misc.execute_command_status(
+        cmd_name, shell_cmd, env=util.env_no_snap_for_external_calls())
+    display_ops.report_executed_command(f"Hook '{cmd_name}'", returncode, out, err)
     return err
 
 

certbot/certbot/_internal/log.py

@@ -13,7 +13,7 @@ and properly flushed before program exit.
 
 The `logging` module is useful for recording messages about about what
 Certbot is doing under the hood, but do not necessarily need to be shown
-to the user on the terminal. The default verbosity is INFO.
+to the user on the terminal. The default verbosity is WARNING.
 
 The preferred method to display important information to the user is to
 use `certbot.display.util` and `certbot.display.ops`.
@@ -28,6 +28,7 @@ import shutil
 import sys
 import tempfile
 import traceback
+from types import TracebackType
 
 from acme import messages
 from certbot import errors
@@ -78,7 +79,9 @@ def pre_arg_parse_setup():
     util.atexit_register(logging.shutdown)
     sys.excepthook = functools.partial(
         pre_arg_parse_except_hook, memory_handler,
-        debug='--debug' in sys.argv, log_path=temp_handler.path)
+        debug='--debug' in sys.argv,
+        quiet='--quiet' in sys.argv or '-q' in sys.argv,
+        log_path=temp_handler.path)
 
 
 def post_arg_parse_setup(config):
@@ -95,7 +98,6 @@ def post_arg_parse_setup(config):
     """
     file_handler, file_path = setup_log_file_handler(
         config, 'letsencrypt.log', FILE_FMT)
-    logs_dir = os.path.dirname(file_path)
 
     root_logger = logging.getLogger()
     memory_handler = stderr_handler = None
@@ -122,10 +124,13 @@ def post_arg_parse_setup(config):
         level = -config.verbose_count * 10
     stderr_handler.setLevel(level)
     logger.debug('Root logging level set at %d', level)
-    logger.info('Saving debug log to %s', file_path)
+
+    if not config.quiet:
+        print(f'Saving debug log to {file_path}', file=sys.stderr)
 
     sys.excepthook = functools.partial(
-        post_arg_parse_except_hook, debug=config.debug, log_path=logs_dir)
+        post_arg_parse_except_hook,
+        debug=config.debug, quiet=config.quiet, log_path=file_path)
 
 
 def setup_log_file_handler(config, logfile, fmt):
@@ -307,7 +312,8 @@ def pre_arg_parse_except_hook(memory_handler, *args, **kwargs):
         memory_handler.flush(force=True)
 
 
-def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path):
+def post_arg_parse_except_hook(exc_type: type, exc_value: BaseException, trace: TracebackType,
+                               debug: bool, quiet: bool, log_path: str):
     """Logs fatal exceptions and reports them to the user.
 
     If debug is True, the full exception and traceback is shown to the
@@ -318,10 +324,13 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path):
     :param BaseException exc_value: raised exception
     :param traceback trace: traceback of where the exception was raised
     :param bool debug: True if the traceback should be shown to the user
+    :param bool quiet: True if Certbot is running in quiet mode
     :param str log_path: path to file or directory containing the log
 
     """
     exc_info = (exc_type, exc_value, trace)
+    # Only print human advice if not running under --quiet
+    exit_func = lambda: sys.exit(1) if quiet else exit_with_advice(log_path)
     # constants.QUIET_LOGGING_LEVEL or higher should be used to
     # display message the user, otherwise, a lower level like
     # logger.DEBUG should be used
@@ -337,7 +346,7 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path):
         # our logger printing warnings and errors in red text.
         if issubclass(exc_type, errors.Error):
             logger.error(str(exc_value))
-            sys.exit(1)
+            exit_func()
         logger.error('An unexpected error occurred:')
         if messages.is_acme_error(exc_value):
             # Remove the ACME error prefix from the exception
@@ -350,11 +359,11 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path):
             # and remove the final newline before passing it to
             # logger.error.
             logger.error(''.join(output).rstrip())
-    exit_with_log_path(log_path)
+    exit_func()
 
 
-def exit_with_log_path(log_path):
-    """Print a message about the log location and exit.
+def exit_with_advice(log_path: str):
+    """Print a link to the community forums, the debug log path, and exit
 
     The message is printed to stderr and the program will exit with a
     nonzero status.
@@ -362,10 +371,11 @@ def exit_with_log_path(log_path):
     :param str log_path: path to file or directory containing the log
 
     """
-    msg = 'Please see the '
+    msg = ("Ask for help or search for solutions at https://community.letsencrypt.org. "
+           "See the ")
     if os.path.isdir(log_path):
-        msg += 'logfiles in {0} '.format(log_path)
+        msg += f'logfiles in {log_path} '
     else:
-        msg += "logfile '{0}' ".format(log_path)
-    msg += 'for more details.'
+        msg += f"logfile {log_path} "
+    msg += 'or re-run Certbot with -v for more details.'
     sys.exit(msg)

certbot/certbot/_internal/main.py

@@ -67,26 +67,14 @@ def _suggest_donation_if_appropriate(config):
     if config.staging:
         # --dry-run implies --staging
         return
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
-    msg = ("If you like Certbot, 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):
-    """Reports on successful dry run
-
-    :param config: Configuration object
-    :type config: interfaces.IConfig
-
-    :returns: `None`
-    :rtype: None
-
-    """
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
-    assert config.verb != "renew"
-    reporter_util.add_message("The dry run was successful.",
-                              reporter_util.HIGH_PRIORITY, on_crash=False)
+    disp = zope.component.getUtility(interfaces.IDisplay)
+    util.atexit_register(
+        disp.notification,
+        "If you like Certbot, please consider supporting our work by:\n"
+        " * Donating to ISRG / Let's Encrypt:   https://letsencrypt.org/donate\n"
+        " * Donating to EFF:                    https://eff.org/donate-le",
+        pause=False
+    )
 
 
 def _get_and_save_cert(le_client, config, domains=None, certname=None, lineage=None):
@@ -214,7 +202,7 @@ def _handle_subset_cert_request(config: configuration.NamespaceConfig,
         "--duplicate option.{br}{br}"
         "For example:{br}{br}{1} --duplicate {2}".format(
             existing,
-            sys.argv[0], " ".join(sys.argv[1:]),
+            cli.cli_command, " ".join(sys.argv[1:]),
             br=os.linesep
         ))
     raise errors.Error(USER_CANCELLED)
@@ -481,9 +469,74 @@ def _find_domains_or_certname(config, installer, question=None):
     return domains, certname
 
 
+def _report_next_steps(config: interfaces.IConfig, installer_err: Optional[errors.Error],
+                       lineage: Optional[storage.RenewableCert],
+                       new_or_renewed_cert: bool = True) -> None:
+    """Displays post-run/certonly advice to the user about renewal and installation.
+
+    The output varies by runtime configuration and any errors encountered during installation.
+
+    :param config: Configuration object
+    :type config: interfaces.IConfig
+
+    :param installer_err: The installer/enhancement error encountered, if any.
+    :type error: Optional[errors.Error]
+
+    :param lineage: The resulting certificate lineage from the issuance, if any.
+    :type lineage: Optional[storage.RenewableCert]
+
+    :param bool new_or_renewed_cert: Whether the verb execution resulted in a certificate
+                                     being saved (created or renewed).
+
+    """
+    steps: List[str] = []
+
+    # If the installation or enhancement raised an error, show advice on trying again
+    if installer_err:
+        steps.append(
+            "The certificate was saved, but could not be installed (installer: "
+            f"{config.installer}). After fixing the error shown below, try installing it again "
+            f"by running:\n  {cli.cli_command} install --cert-name "
+            f"{_cert_name_from_config_or_lineage(config, lineage)}"
+        )
+
+    # If a certificate was obtained or renewed, show applicable renewal advice
+    if new_or_renewed_cert:
+        if config.csr:
+            steps.append(
+                "Certificates created using --csr will not be renewed automatically by Certbot. "
+                "You will need to renew the certificate before it expires, by running the same "
+                "Certbot command again.")
+        elif not config.preconfigured_renewal:
+            steps.append(
+                "The certificate will need to be renewed before it expires. Certbot can "
+                "automatically renew the certificate in the background, but you may need "
+                "to take steps to enable that functionality. "
+                "See https://certbot.org/renewal-setup for instructions.")
+
+    if not steps:
+        return
+
+    # TODO: refactor ANSI escapes during https://github.com/certbot/certbot/issues/8848
+    (bold_on, bold_off) = [c if sys.stdout.isatty() and not config.quiet else '' \
+                           for c in (util.ANSI_SGR_BOLD, util.ANSI_SGR_RESET)]
+
+    print(bold_on, '\n', 'NEXT STEPS:', bold_off, sep='')
+    for step in steps:
+        display_util.notify(f"- {step}")
+
+    # If there was an installer error, segregate the error output with a trailing newline
+    if installer_err:
+        print()
+
+
 def _report_new_cert(config, cert_path, fullchain_path, key_path=None):
+    # type: (interfaces.IConfig, Optional[str], Optional[str], Optional[str]) -> None
     """Reports the creation of a new certificate to the user.
 
+    :param config: Configuration object
+    :type config: interfaces.IConfig
+
     :param cert_path: path to certificate
     :type cert_path: str
 
@@ -498,29 +551,62 @@ def _report_new_cert(config, cert_path, fullchain_path, key_path=None):
 
     """
     if config.dry_run:
-        _report_successful_dry_run(config)
+        display_util.notify("The dry run was successful.")
+        return
+
+    assert cert_path and fullchain_path, "No certificates saved to report."
+
+    display_util.notify(
+        ("\nSuccessfully received certificate.\n"
+        "Certificate is saved at: {cert_path}\n{key_msg}"
+        "This certificate expires on {expiry}.\n"
+        "These files will be updated when the certificate renews.{renewal_msg}{nl}").format(
+            cert_path=fullchain_path,
+            expiry=crypto_util.notAfter(cert_path).date(),
+            key_msg="Key is saved at:         {}\n".format(key_path) if key_path else "",
+            renewal_msg="\nCertbot has set up a scheduled task to automatically renew this "
+                        "certificate in the background." if config.preconfigured_renewal else "",
+            nl="\n" if config.verb == "run" else "" # Normalize spacing across verbs
+        )
+    )
+
+
+def _csr_report_new_cert(config: interfaces.IConfig, cert_path: Optional[str],
+                         chain_path: Optional[str], fullchain_path: Optional[str]):
+    """ --csr variant of _report_new_cert.
+
+    Until --csr is overhauled (#8332) this is transitional function to report the creation
+    of a new certificate using --csr.
+    TODO: remove this function and just call _report_new_cert when --csr is overhauled.
+
+    :param config: Configuration object
+    :type config: interfaces.IConfig
+
+    :param str cert_path: path to cert.pem
+
+    :param str chain_path: path to chain.pem
+
+    :param str fullchain_path: path to fullchain.pem
+
+    """
+    if config.dry_run:
+        display_util.notify("The dry run was successful.")
         return
 
     assert cert_path and fullchain_path, "No certificates saved to report."
 
     expiry = crypto_util.notAfter(cert_path).date()
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
-    # Print the path to fullchain.pem because that's what modern webservers
-    # (Nginx and Apache2.4) will want.
 
-    verbswitch = ' with the "certonly" option' if config.verb == "run" else ""
-    privkey_statement = 'Your key file has been saved at:{br}{0}{br}'.format(
-            key_path, br=os.linesep) if key_path else ""
-    # XXX Perhaps one day we could detect the presence of known old webservers
-    # and say something more informative here.
-    msg = ('Congratulations! Your certificate and chain have been saved at:{br}'
-           '{0}{br}{1}'
-           'Your certificate will expire on {2}. To obtain a new or tweaked version of this '
-           'certificate in the future, simply run {3} again{4}. '
-           'To non-interactively renew *all* of your certificates, run "{3} renew"'
-           .format(fullchain_path, privkey_statement, expiry, cli.cli_command, verbswitch,
-               br=os.linesep))
-    reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY)
+    display_util.notify(
+        ("\nSuccessfully received certificate.\n"
+        "Certificate is saved at:            {cert_path}\n"
+        "Intermediate CA chain is saved at:  {chain_path}\n"
+        "Full certificate chain is saved at: {fullchain_path}\n"
+        "This certificate expires on {expiry}.").format(
+            cert_path=cert_path, chain_path=chain_path,
+            fullchain_path=fullchain_path, expiry=expiry,
+        )
+    )
 
 
 def _determine_account(config):
@@ -616,7 +702,9 @@ def _delete_if_appropriate(config):
 
     # don't delete if the archive_dir is used by some other lineage
     archive_dir = storage.full_archive_path(
-            configobj.ConfigObj(storage.renewal_file_for_certname(config, config.certname)),
+            configobj.ConfigObj(
+                storage.renewal_file_for_certname(config, config.certname),
+                encoding='utf-8', default_encoding='utf-8'),
             config, config.certname)
     try:
         cert_manager.match_and_check_overlaps(config, [lambda x: archive_dir],
@@ -781,6 +869,21 @@ def update_account(config, unused_plugins):
     return None
 
 
+def _cert_name_from_config_or_lineage(config: interfaces.IConfig,
+                                      lineage: Optional[storage.RenewableCert]) -> Optional[str]:
+    if lineage:
+        return lineage.lineagename
+    elif config.certname:
+        return config.certname
+    try:
+        cert_name = cert_manager.cert_path_to_lineage(config)
+        return cert_name
+    except errors.Error:
+        pass
+
+    return None
+
+
 def _install_cert(config, le_client, domains, lineage=None):
     """Install a cert
 
@@ -803,8 +906,8 @@ def _install_cert(config, le_client, domains, lineage=None):
     path_provider = lineage if lineage else config
     assert path_provider.cert_path is not None
 
-    le_client.deploy_certificate(domains, path_provider.key_path,
-        path_provider.cert_path, path_provider.chain_path, path_provider.fullchain_path)
+    le_client.deploy_certificate(domains, path_provider.key_path, path_provider.cert_path,
+                                 path_provider.chain_path, path_provider.fullchain_path)
     le_client.enhance_config(domains, path_provider.chain_path)
 
 
@@ -949,7 +1052,7 @@ def enhance(config, plugins):
     if not enhancements.are_requested(config) and not oldstyle_enh:
         msg = ("Please specify one or more enhancement types to configure. To list "
                "the available enhancement types, run:\n\n%s --help enhance\n")
-        logger.warning(msg, sys.argv[0])
+        logger.error(msg, cli.cli_command)
         raise errors.MisconfigurationError("No enhancements requested, exiting.")
 
     try:
@@ -1172,15 +1275,27 @@ def run(config, plugins):
     if should_get_cert:
         _report_new_cert(config, cert_path, fullchain_path, key_path)
 
-    _install_cert(config, le_client, domains, new_lineage)
+    # The installer error, if any, is being stored as a value here, in order to first print
+    # relevant advice in a nice way, before re-raising the error for normal processing.
+    installer_err: Optional[errors.Error] = None
+    try:
+        _install_cert(config, le_client, domains, new_lineage)
 
-    if enhancements.are_requested(config) and new_lineage:
-        enhancements.enable(new_lineage, domains, installer, config)
+        if enhancements.are_requested(config) and new_lineage:
+            enhancements.enable(new_lineage, domains, installer, config)
 
-    if lineage is None or not should_get_cert:
-        display_ops.success_installation(domains)
-    else:
-        display_ops.success_renewal(domains)
+        if lineage is None or not should_get_cert:
+            display_ops.success_installation(domains)
+        else:
+            display_ops.success_renewal(domains)
+    except errors.Error as e:
+        installer_err = e
+    finally:
+        _report_next_steps(config, installer_err, new_lineage,
+                           new_or_renewed_cert=should_get_cert)
+        # If the installer did fail, re-raise the error to bail out
+        if installer_err:
+            raise installer_err
 
     _suggest_donation_if_appropriate(config)
     eff.handle_subscription(config, le_client.account)
@@ -1188,6 +1303,7 @@ def run(config, plugins):
 
 
 def _csr_get_and_save_cert(config, le_client):
+    # type: (interfaces.IConfig, client.Client) -> Tuple[Optional[str], Optional[str], Optional[str]] # pylint: disable=line-too-long
     """Obtain a cert using a user-supplied CSR
 
     This works differently in the CSR case (for now) because we don't
@@ -1200,20 +1316,29 @@ def _csr_get_and_save_cert(config, le_client):
     :param client: Client object
     :type client: client.Client
 
-    :returns: `cert_path` and `fullchain_path` as absolute paths to the actual files
+    :returns: `cert_path`, `chain_path` and `fullchain_path` as absolute
+              paths to the actual files, or None for each if it's a dry-run.
     :rtype: `tuple` of `str`
 
     """
     csr, _ = config.actual_csr
+    csr_names = crypto_util.get_names_from_req(csr.data)
+    display_util.notify(
+        "{action} for {domains}".format(
+            action="Simulating a certificate request" if config.dry_run else
+                    "Requesting a certificate",
+            domains=display_util.summarize_domain_list(csr_names)
+        )
+    )
     cert, chain = le_client.obtain_certificate_from_csr(csr)
     if config.dry_run:
         logger.debug(
             "Dry run: skipping saving certificate to %s", config.cert_path)
-        return None, None
-    cert_path, _, fullchain_path = le_client.save_certificate(
+        return None, None, None
+    cert_path, chain_path, fullchain_path = le_client.save_certificate(
         cert, chain, os.path.normpath(config.cert_path),
         os.path.normpath(config.chain_path), os.path.normpath(config.fullchain_path))
-    return cert_path, fullchain_path
+    return cert_path, chain_path, fullchain_path
 
 
 def renew_cert(config, plugins, lineage):
@@ -1234,29 +1359,17 @@ def renew_cert(config, plugins, lineage):
     :raises errors.PluginSelectionError: MissingCommandlineFlag if supplied parameters do not pass
 
     """
-    try:
-        # installers are used in auth mode to determine domain names
-        installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
-    except errors.PluginSelectionError as e:
-        logger.info("Could not choose appropriate plugin: %s", e)
-        raise
+    # installers are used in auth mode to determine domain names
+    installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
     le_client = _init_le_client(config, auth, installer)
 
     renewed_lineage = _get_and_save_cert(le_client, config, lineage=lineage)
 
-    notify = zope.component.getUtility(interfaces.IDisplay).notification
-    if installer is None:
-        notify("new certificate deployed without reload, fullchain is {0}".format(
-               lineage.fullchain), pause=False)
-    else:
+    if installer and not config.dry_run:
         # 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.
-        # Run deployer
         updater.run_renewal_deployer(config, renewed_lineage, installer)
-        installer.restart()
-        notify("new certificate deployed with reload of {0} server; fullchain is {1}".format(
-               config.installer, lineage.fullchain), pause=False)
+        display_util.notify(f"Reloading {config.installer} server after certificate renewal")
+        installer.restart() # type: ignore
 
 
 def certonly(config, plugins):
@@ -1277,18 +1390,15 @@ def certonly(config, plugins):
 
     """
     # SETUP: Select plugins and construct a client instance
-    try:
-        # installers are used in auth mode to determine domain names
-        installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
-    except errors.PluginSelectionError as e:
-        logger.info("Could not choose appropriate plugin: %s", e)
-        raise
+    # installers are used in auth mode to determine domain names
+    installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
 
     le_client = _init_le_client(config, auth, installer)
 
     if config.csr:
-        cert_path, fullchain_path = _csr_get_and_save_cert(config, le_client)
-        _report_new_cert(config, cert_path, fullchain_path)
+        cert_path, chain_path, fullchain_path = _csr_get_and_save_cert(config, le_client)
+        _csr_report_new_cert(config, cert_path, chain_path, fullchain_path)
+        _report_next_steps(config, None, None)
         _suggest_donation_if_appropriate(config)
         eff.handle_subscription(config, le_client.account)
         return
@@ -1307,6 +1417,7 @@ def certonly(config, plugins):
     fullchain_path = lineage.fullchain_path if lineage else None
     key_path = lineage.key_path if lineage else None
     _report_new_cert(config, cert_path, fullchain_path, key_path)
+    _report_next_steps(config, None, lineage, new_or_renewed_cert=should_get_cert)
     _suggest_donation_if_appropriate(config)
     eff.handle_subscription(config, le_client.account)
 
@@ -1367,7 +1478,7 @@ def make_displayer(config: configuration.NamespaceConfig
 
     if config.quiet:
         config.noninteractive_mode = True
-        devnull = open(os.devnull, "w")
+        devnull = open(os.devnull, "w")  # pylint: disable=consider-using-with
         displayer = display_util.NoninteractiveDisplay(devnull)
     elif config.noninteractive_mode:
         displayer = display_util.NoninteractiveDisplay(sys.stdout)
@@ -1407,9 +1518,15 @@ def main(cli_args=None):
     logger.debug("Arguments: %r", cli_args)
     logger.debug("Discovered plugins: %r", plugins)
 
+    # Some releases of Windows require escape sequences to be enable explicitly
+    misc.prepare_virtual_console()
+
     # note: arg parser internally handles --help (and exits afterwards)
     args = cli.prepare_and_parse_args(plugins, cli_args)
     config = configuration.NamespaceConfig(args)
+
+    # This call is done only for retro-compatibility purposes.
+    # TODO: Remove this call once zope dependencies are removed from Certbot.
     zope.component.provideUtility(config)
 
     # On windows, shell without administrative right cannot create symlinks required by certbot.

certbot/certbot/_internal/plugins/manual.py

@@ -1,4 +1,5 @@
 """Manual authenticator plugin"""
+import logging
 from typing import Dict
 
 import zope.component
@@ -10,11 +11,14 @@ 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
+from certbot.display import ops as display_ops
 from certbot.plugins import common
 
+logger = logging.getLogger(__name__)
 
 @zope.interface.implementer(interfaces.IAuthenticator)
 @zope.interface.provider(interfaces.IPluginFactory)
@@ -60,7 +64,7 @@ with the following value:
 {validation}
 """
     _DNS_VERIFY_INSTRUCTIONS = """
-Before continuing, verify the TXT record has been deployed. Depending on the DNS 
+Before continuing, verify the TXT record has been deployed. Depending on the DNS
 provider, this may take some time, from a few seconds to multiple minutes. You can
 check if it has finished deploying with aid of online tools, such as the Google
 Admin Toolbox: https://toolbox.googleapps.com/apps/dig/#TXT/{domain}.
@@ -125,6 +129,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]
@@ -153,7 +193,7 @@ permitted by DNS standards.)
         else:
             os.environ.pop('CERTBOT_TOKEN', None)
         os.environ.update(env)
-        _, out = self._execute_hook('auth-hook')
+        _, out = self._execute_hook('auth-hook', achall.domain)
         env['CERTBOT_AUTH_OUTPUT'] = out.strip()
         self.env[achall] = env
 
@@ -196,9 +236,16 @@ permitted by DNS standards.)
                 if 'CERTBOT_TOKEN' not in env:
                     os.environ.pop('CERTBOT_TOKEN', None)
                 os.environ.update(env)
-                self._execute_hook('cleanup-hook')
+                self._execute_hook('cleanup-hook', achall.domain)
         self.reverter.recovery_routine()
 
-    def _execute_hook(self, hook_name):
-        return misc.execute_command(self.option_name(hook_name), self.conf(hook_name),
-                                    env=util.env_no_snap_for_external_calls())
+    def _execute_hook(self, hook_name, achall_domain):
+        returncode, err, out = misc.execute_command_status(
+            self.option_name(hook_name), self.conf(hook_name),
+            env=util.env_no_snap_for_external_calls()
+        )
+
+        display_ops.report_executed_command(
+            f"Hook '--manual-{hook_name}' for {achall_domain}", returncode, out, err)
+
+        return err, out

certbot/certbot/_internal/plugins/selection.py

@@ -2,10 +2,12 @@
 
 import logging
 
+from typing import Optional, Tuple
 import zope.component
 
 from certbot import errors
 from certbot import interfaces
+from certbot._internal.plugins import disco
 from certbot.compat import os
 from certbot.display import util as display_util
 
@@ -164,7 +166,9 @@ def record_chosen_plugins(config, plugins, auth, inst):
          config.authenticator, config.installer)
 
 
-def choose_configurator_plugins(config, plugins, verb):
+def choose_configurator_plugins(config: interfaces.IConfig, plugins: disco.PluginsRegistry,
+                                verb: str) -> Tuple[Optional[interfaces.IInstaller],
+                                                    Optional[interfaces.IAuthenticator]]:
     """
     Figure out which configurator we're going to use, modifies
     config.authenticator and config.installer strings to reflect that choice if
@@ -172,7 +176,7 @@ def choose_configurator_plugins(config, plugins, verb):
 
     :raises errors.PluginSelectionError if there was a problem
 
-    :returns: (an `IAuthenticator` or None, an `IInstaller` or None)
+    :returns: tuple of (`IInstaller` or None, `IAuthenticator` or None)
     :rtype: tuple
     """
 

certbot/certbot/_internal/plugins/webroot.py

@@ -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]
@@ -134,7 +140,7 @@ to serve all files under specified web root ({0})."""
                     "webroot when using the webroot plugin.")
             return None if index == 0 else known_webroots[index - 1]  # code == display_util.OK
 
-    def _prompt_for_new_webroot(self, domain, allowraise=False):  # pylint: no-self-use
+    def _prompt_for_new_webroot(self, domain, allowraise=False):
         code, webroot = ops.validated_directory(
             _validate_webroot,
             "Input the webroot for {0}:".format(domain),
@@ -183,7 +189,7 @@ to serve all files under specified web root ({0})."""
                             filesystem.copy_ownership_and_apply_mode(
                                 path, prefix, 0o755, copy_user=True, copy_group=True)
                         except (OSError, AttributeError) as exception:
-                            logger.info("Unable to change owner and uid of webroot directory")
+                            logger.warning("Unable to change owner and uid of webroot directory")
                             logger.debug("Error was: %s", exception)
                     except OSError as exception:
                         raise errors.PluginError(
@@ -192,7 +198,7 @@ to serve all files under specified web root ({0})."""
             finally:
                 filesystem.umask(old_umask)
 
-    def _get_validation_path(self, root_path, achall):  # pylint: no-self-use
+    def _get_validation_path(self, root_path, achall):
         return os.path.join(root_path, achall.chall.encode("token"))
 
     def _perform_single(self, achall):

certbot/certbot/_internal/renewal.py

@@ -14,7 +14,6 @@ from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives.asymmetric import ec
 from cryptography.hazmat.primitives.asymmetric import rsa
 from cryptography.hazmat.primitives.serialization import load_pem_private_key
-import OpenSSL
 import zope.component
 
 from certbot import crypto_util
@@ -68,18 +67,18 @@ def _reconstitute(config, full_path):
     """
     try:
         renewal_candidate = storage.RenewableCert(full_path, config)
-    except (errors.CertStorageError, IOError):
-        logger.warning("", exc_info=True)
-        logger.warning("Renewal configuration file %s is broken. Skipping.", full_path)
+    except (errors.CertStorageError, IOError) as error:
+        logger.error("Renewal configuration file %s is broken.", full_path)
+        logger.error("The error was: %s\nSkipping.", str(error))
         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 "
+        logger.error("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 "
+        logger.error("Renewal configuration file %s does not specify "
                        "an authenticator. Skipping.", full_path)
         return None
     # Now restore specific values along with their data types, if
@@ -89,7 +88,7 @@ def _reconstitute(config, full_path):
         restore_required_config_elements(config, renewalparams)
         _restore_plugin_configs(config, renewalparams)
     except (ValueError, errors.Error) as error:
-        logger.warning(
+        logger.error(
             "An error occurred while parsing %s. The error was %s. "
             "Skipping the file.", full_path, str(error))
         logger.debug("Traceback was:\n%s", traceback.format_exc())
@@ -99,7 +98,7 @@ def _reconstitute(config, full_path):
         config.domains = [util.enforce_domain_sanity(d)
                           for d in renewal_candidate.names()]
     except errors.ConfigurationError as error:
-        logger.warning("Renewal configuration file %s references a certificate "
+        logger.error("Renewal configuration file %s references a certificate "
                        "that contains an invalid domain name. The problem "
                        "was: %s. Skipping.", full_path, error)
         return None
@@ -295,24 +294,17 @@ def should_renew(config, lineage):
         logger.debug("Auto-renewal forced with --force-renewal...")
         return True
     if lineage.should_autorenew():
-        logger.info("Cert is due for renewal, auto-renewing...")
+        logger.info("Certificate 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")
+        logger.info("Certificate not due for renewal, but simulating renewal for dry run")
         return True
-    logger.info("Cert not yet due for renewal")
+    display_util.notify("Certificate not yet due for renewal")
     return False
 
 
 def _avoid_invalidating_lineage(config, lineage, original_server):
     """Do not renew a valid cert with one from a staging server!"""
-    # Some lineages may have begun with --staging, but then had production
-    # certificates added to them
-    with open(lineage.cert) as the_file:
-        contents = the_file.read()
-    latest_cert = OpenSSL.crypto.load_certificate(
-        OpenSSL.crypto.FILETYPE_PEM, contents)
-
     if util.is_staging(config.server):
         if not util.is_staging(original_server):
             if not config.break_my_certs:
@@ -447,7 +439,7 @@ def handle_renewal_request(config):
         try:
             renewal_candidate = _reconstitute(lineage_config, renewal_file)
         except Exception as e:  # pylint: disable=broad-except
-            logger.warning("Renewal configuration file %s (cert: %s) "
+            logger.error("Renewal configuration file %s (cert: %s) "
                            "produced an unexpected error: %s. Skipping.",
                            renewal_file, lineagename, e)
             logger.debug("Traceback was:\n%s", traceback.format_exc())
@@ -458,7 +450,8 @@ def handle_renewal_request(config):
             if renewal_candidate is None:
                 parse_failures.append(renewal_file)
             else:
-                # XXX: ensure that each call here replaces the previous one
+                # This call is done only for retro-compatibility purposes.
+                # TODO: Remove this call once zope dependencies are removed from Certbot.
                 zope.component.provideUtility(lineage_config)
                 renewal_candidate.ensure_deployed()
                 from certbot._internal import main

certbot/certbot/_internal/storage.py

@@ -67,13 +67,16 @@ def cert_path_for_cert_name(config: interfaces.IConfig, cert_name: str) -> str:
 
     """
     cert_name_implied_conf = renewal_file_for_certname(config, cert_name)
-    return configobj.ConfigObj(cert_name_implied_conf)["fullchain"]
+    return configobj.ConfigObj(
+        cert_name_implied_conf, encoding='utf-8', default_encoding='utf-8')["fullchain"]
 
 
 def config_with_defaults(config=None):
     """Merge supplied config, if provided, on top of builtin defaults."""
-    defaults_copy = configobj.ConfigObj(constants.RENEWER_DEFAULTS)
-    defaults_copy.merge(config if config is not None else configobj.ConfigObj())
+    defaults_copy = configobj.ConfigObj(
+        constants.RENEWER_DEFAULTS, encoding='utf-8', default_encoding='utf-8')
+    defaults_copy.merge(config if config is not None else configobj.ConfigObj(
+        encoding='utf-8', default_encoding='utf-8'))
     return defaults_copy
 
 
@@ -114,7 +117,7 @@ def write_renewal_config(o_filename, n_filename, archive_dir, target, relevant_d
     :rtype: configobj.ConfigObj
 
     """
-    config = configobj.ConfigObj(o_filename)
+    config = configobj.ConfigObj(o_filename, encoding='utf-8', default_encoding='utf-8')
     config["version"] = certbot.__version__
     config["archive_dir"] = archive_dir
     for kind in ALL_FOUR:
@@ -196,7 +199,7 @@ def update_configuration(lineagename, archive_dir, target, cli_config):
     write_renewal_config(config_filename, temp_filename, archive_dir, target, values)
     filesystem.replace(temp_filename, config_filename)
 
-    return configobj.ConfigObj(config_filename)
+    return configobj.ConfigObj(config_filename, encoding='utf-8', default_encoding='utf-8')
 
 
 def get_link_target(link):
@@ -324,10 +327,11 @@ def delete_files(config, certname):
     full_default_archive_dir = full_archive_path(None, config, certname)
     full_default_live_dir = _full_live_path(config, certname)
     try:
-        renewal_config = configobj.ConfigObj(renewal_filename)
+        renewal_config = configobj.ConfigObj(
+            renewal_filename, encoding='utf-8', default_encoding='utf-8')
     except configobj.ConfigObjError:
         # config is corrupted
-        logger.warning("Could not parse %s. You may wish to manually "
+        logger.error("Could not parse %s. You may wish to manually "
             "delete the contents of %s and %s.", renewal_filename,
             full_default_live_dir, full_default_archive_dir)
         raise errors.CertStorageError(
@@ -336,7 +340,7 @@ def delete_files(config, certname):
         # we couldn't read it, but let's at least delete it
         # if this was going to fail, it already would have.
         os.remove(renewal_filename)
-        logger.debug("Removed %s", renewal_filename)
+        logger.info("Removed %s", renewal_filename)
 
     # cert files and (hopefully) live directory
     # it's not guaranteed that the files are in our default storage
@@ -434,7 +438,8 @@ class RenewableCert(interfaces.RenewableCert):
         # systemwide renewal configuration; self.configfile should be
         # used to make and save changes.
         try:
-            self.configfile = configobj.ConfigObj(config_filename)
+            self.configfile = configobj.ConfigObj(
+                config_filename, encoding='utf-8', default_encoding='utf-8')
         except configobj.ConfigObjError:
             raise errors.CertStorageError(
                 "error parsing {0}".format(config_filename))

certbot/certbot/_internal/updater.py

@@ -29,7 +29,7 @@ def run_generic_updaters(config, lineage, plugins):
     try:
         installer = plug_sel.get_unprepared_installer(config, plugins)
     except errors.Error as e:
-        logger.warning("Could not choose appropriate plugin for updaters: %s", e)
+        logger.error("Could not choose appropriate plugin for updaters: %s", e)
         return
     if installer:
         _run_updaters(lineage, installer, config)

certbot/certbot/compat/misc.py

@@ -8,6 +8,7 @@ import logging
 import select
 import subprocess
 import sys
+import warnings
 from typing import Optional
 from typing import Tuple
 
@@ -16,6 +17,8 @@ from certbot.compat import os
 
 try:
     from win32com.shell import shell as shellwin32
+    from win32console import GetStdHandle, STD_OUTPUT_HANDLE
+    from pywintypes import error as pywinerror
     POSIX_MODE = False
 except ImportError:  # pragma: no cover
     POSIX_MODE = True
@@ -38,6 +41,26 @@ def raise_for_non_administrative_windows_rights() -> None:
         raise errors.Error('Error, certbot must be run on a shell with administrative rights.')
 
 
+def prepare_virtual_console() -> None:
+    """
+    On Windows, ensure that Console Virtual Terminal Sequences are enabled.
+
+    """
+    if POSIX_MODE:
+        return
+
+    # https://docs.microsoft.com/en-us/windows/console/setconsolemode
+    ENABLE_VIRTUAL_TERMINAL_PROCESSING = 0x0004
+
+    # stdout/stderr will be the same console screen buffer, but this could return None or raise
+    try:
+        h = GetStdHandle(STD_OUTPUT_HANDLE)
+        if h:
+            h.SetConsoleMode(h.GetConsoleMode() | ENABLE_VIRTUAL_TERMINAL_PROCESSING)
+    except pywinerror:
+        logger.debug("Failed to set console mode", exc_info=True)
+
+
 def readline_with_timeout(timeout: float, prompt: str) -> str:
     """
     Read user input to return the first line entered, or raise after specified timeout.
@@ -112,38 +135,68 @@ def underscores_for_unsupported_characters_in_path(path: str) -> str:
     return drive + tail.replace(':', '_')
 
 
-def execute_command(cmd_name: str, shell_cmd: str, env: Optional[dict] = None) -> Tuple[str, str]:
+def execute_command_status(cmd_name: str, shell_cmd: str,
+                           env: Optional[dict] = None) -> Tuple[int, str, str]:
     """
     Run a command:
-        - on Linux command will be run by the standard shell selected with Popen(shell=True)
+        - on Linux command will be run by the standard shell selected with
+          subprocess.run(shell=True)
         - on Windows command will be run in a Powershell shell
 
+    This differs from execute_command: it returns the exit code, and does not log the result
+    and output of the command.
+
     :param str cmd_name: the user facing name of the hook being run
     :param str shell_cmd: shell command to execute
-    :param dict env: environ to pass into Popen
+    :param dict env: environ to pass into subprocess.run
 
-    :returns: `tuple` (`str` stderr, `str` stdout)
+    :returns: `tuple` (`int` returncode, `str` stderr, `str` stdout)
     """
     logger.info("Running %s command: %s", cmd_name, shell_cmd)
 
     if POSIX_MODE:
-        cmd = subprocess.Popen(shell_cmd, shell=True, stdout=subprocess.PIPE,
-                               stderr=subprocess.PIPE, universal_newlines=True,
-                               env=env)
+        proc = subprocess.run(shell_cmd, shell=True, stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE, universal_newlines=True,
+                              check=False, env=env)
     else:
         line = ['powershell.exe', '-Command', shell_cmd]
-        cmd = subprocess.Popen(line, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                               universal_newlines=True, env=env)
+        proc = subprocess.run(line, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+                              universal_newlines=True, check=False, env=env)
 
-    # universal_newlines causes Popen.communicate()
-    # to return str objects instead of bytes in Python 3
-    out, err = cmd.communicate()
+    # universal_newlines causes stdout and stderr to be str objects instead of
+    # bytes in Python 3
+    out, err = proc.stdout, proc.stderr
+    return proc.returncode, err, out
+
+
+def execute_command(cmd_name: str, shell_cmd: str, env: Optional[dict] = None) -> Tuple[str, str]:
+    """
+    Run a command:
+        - on Linux command will be run by the standard shell selected with
+          subprocess.run(shell=True)
+        - on Windows command will be run in a Powershell shell
+
+    This differs from execute_command: it returns the exit code, and does not log the result
+    and output of the command.
+
+    :param str cmd_name: the user facing name of the hook being run
+    :param str shell_cmd: shell command to execute
+    :param dict env: environ to pass into subprocess.run
+
+    :returns: `tuple` (`str` stderr, `str` stdout)
+    """
+    # Deprecation per https://github.com/certbot/certbot/issues/8854
+    warnings.warn(
+        "execute_command will be deprecated in the future, use execute_command_status instead",
+        PendingDeprecationWarning
+    )
+    returncode, err, out = execute_command_status(cmd_name, shell_cmd, env)
     base_cmd = os.path.basename(shell_cmd.split(None, 1)[0])
     if out:
         logger.info('Output from %s command %s:\n%s', cmd_name, base_cmd, out)
-    if cmd.returncode != 0:
+    if returncode != 0:
         logger.error('%s command "%s" returned error code %d',
-                     cmd_name, shell_cmd, cmd.returncode)
+                     cmd_name, shell_cmd, returncode)
     if err:
         logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err)
     return err, out

certbot/certbot/crypto_util.py

@@ -9,6 +9,7 @@ import logging
 import re
 import warnings
 
+from typing import List, Set
 # See https://github.com/pyca/cryptography/issues/4275
 from cryptography import x509  # type: ignore
 from cryptography.exceptions import InvalidSignature
@@ -37,9 +38,10 @@ logger = logging.getLogger(__name__)
 
 
 # High level functions
-def init_save_key(
-        key_size, key_dir, key_type="rsa", elliptic_curve="secp256r1", keyname="key-certbot.pem"
-):
+
+def generate_key(key_size: int, key_dir: str, key_type: str = "rsa",
+                 elliptic_curve: str = "secp256r1", keyname: str = "key-certbot.pem",
+                 strict_permissions: bool = True) -> util.Key:
     """Initializes and saves a privkey.
 
     Inits key and saves it in PEM format on the filesystem.
@@ -47,6 +49,56 @@ def init_save_key(
     .. note:: keyname is the attempted filename, it may be different if a file
         already exists at the path.
 
+    :param int key_size: key size in bits if key size is rsa.
+    :param str key_dir: Key save directory.
+    :param str key_type: Key Type [rsa, ecdsa]
+    :param str elliptic_curve: Name of the elliptic curve if key type is ecdsa.
+    :param str keyname: Filename of key
+    :param bool strict_permissions: If true and key_dir exists, an exception is raised if
+        the directory doesn't have 0700 permissions or isn't owned by the current user.
+
+    :returns: Key
+    :rtype: :class:`certbot.util.Key`
+
+    :raises ValueError: If unable to generate the key given key_size.
+
+    """
+    try:
+        key_pem = make_key(
+            bits=key_size, elliptic_curve=elliptic_curve or "secp256r1", key_type=key_type,
+        )
+    except ValueError as err:
+        logger.debug("", exc_info=True)
+        logger.error("Encountered error while making key: %s", str(err))
+        raise err
+
+    # Save file
+    util.make_or_verify_dir(key_dir, 0o700, strict_permissions)
+    key_f, key_path = util.unique_file(
+        os.path.join(key_dir, keyname), 0o600, "wb")
+    with key_f:
+        key_f.write(key_pem)
+    if key_type == 'rsa':
+        logger.debug("Generating RSA key (%d bits): %s", key_size, key_path)
+    else:
+        logger.debug("Generating ECDSA key (%d bits): %s", key_size, key_path)
+
+    return util.Key(key_path, key_pem)
+
+
+# TODO: Remove this call once zope dependencies are removed from Certbot.
+def init_save_key(key_size, key_dir, key_type="rsa", elliptic_curve="secp256r1",
+                  keyname="key-certbot.pem"):
+    """Initializes and saves a privkey.
+
+    Inits key and saves it in PEM format on the filesystem.
+
+    .. note:: keyname is the attempted filename, it may be different if a file
+        already exists at the path.
+
+    .. deprecated:: 1.16.0
+       Use :func:`generate_key` instead.
+
     :param int key_size: key size in bits if key size is rsa.
     :param str key_dir: Key save directory.
     :param str key_type: Key Type [rsa, ecdsa]
@@ -59,32 +111,52 @@ def init_save_key(
     :raises ValueError: If unable to generate the key given key_size.
 
     """
-    try:
-        key_pem = make_key(
-            bits=key_size, elliptic_curve=elliptic_curve or "secp256r1", key_type=key_type,
-        )
-    except ValueError as err:
-        logger.error("", exc_info=True)
-        raise err
+    warnings.warn("certbot.crypto_util.init_save_key is deprecated, please use "
+                  "certbot.crypto_util.generate_key instead.", DeprecationWarning)
 
     config = zope.component.getUtility(interfaces.IConfig)
-    # Save file
-    util.make_or_verify_dir(key_dir, 0o700, config.strict_permissions)
-    key_f, key_path = util.unique_file(
-        os.path.join(key_dir, keyname), 0o600, "wb")
-    with key_f:
-        key_f.write(key_pem)
-    if key_type == 'rsa':
-        logger.debug("Generating RSA key (%d bits): %s", key_size, key_path)
-    else:
-        logger.debug("Generating ECDSA key (%d bits): %s", key_size, key_path)
 
-    return util.Key(key_path, key_pem)
+    return generate_key(key_size, key_dir, key_type=key_type, elliptic_curve=elliptic_curve,
+                        keyname=keyname, strict_permissions=config.strict_permissions)
 
 
+def generate_csr(privkey: util.Key, names: Set[str], path: str,
+                 must_staple: bool = False, strict_permissions: bool = True) -> util.CSR:
+    """Initialize a CSR with the given private key.
+
+    :param privkey: Key to include in the CSR
+    :type privkey: :class:`certbot.util.Key`
+    :param set names: `str` names to include in the CSR
+    :param str path: Certificate save directory.
+    :param bool must_staple: If true, include the TLS Feature extension "OCSP Must Staple"
+    :param bool strict_permissions: If true and path exists, an exception is raised if
+        the directory doesn't have 0755 permissions or isn't owned by the current user.
+
+    :returns: CSR
+    :rtype: :class:`certbot.util.CSR`
+
+    """
+    csr_pem = acme_crypto_util.make_csr(
+        privkey.pem, names, must_staple=must_staple)
+
+    # Save CSR
+    util.make_or_verify_dir(path, 0o755, strict_permissions)
+    csr_f, csr_filename = util.unique_file(
+        os.path.join(path, "csr-certbot.pem"), 0o644, "wb")
+    with csr_f:
+        csr_f.write(csr_pem)
+    logger.debug("Creating CSR: %s", csr_filename)
+
+    return util.CSR(csr_filename, csr_pem, "pem")
+
+
+# TODO: Remove this call once zope dependencies are removed from Certbot.
 def init_save_csr(privkey, names, path):
     """Initialize a CSR with the given private key.
 
+    .. deprecated:: 1.16.0
+       Use :func:`generate_csr` instead.
+
     :param privkey: Key to include in the CSR
     :type privkey: :class:`certbot.util.Key`
 
@@ -96,20 +168,13 @@ def init_save_csr(privkey, names, path):
     :rtype: :class:`certbot.util.CSR`
 
     """
+    warnings.warn("certbot.crypto_util.init_save_csr is deprecated, please use "
+                  "certbot.crypto_util.generate_csr instead.", DeprecationWarning)
+
     config = zope.component.getUtility(interfaces.IConfig)
 
-    csr_pem = acme_crypto_util.make_csr(
-        privkey.pem, names, must_staple=config.must_staple)
-
-    # Save CSR
-    util.make_or_verify_dir(path, 0o755, config.strict_permissions)
-    csr_f, csr_filename = util.unique_file(
-        os.path.join(path, "csr-certbot.pem"), 0o644, "wb")
-    with csr_f:
-        csr_f.write(csr_pem)
-    logger.debug("Creating CSR: %s", csr_filename)
-
-    return util.CSR(csr_filename, csr_pem, "pem")
+    return generate_csr(privkey, names, path, must_staple=config.must_staple,
+                        strict_permissions=config.strict_permissions)
 
 
 # WARNING: the csr and private key file are possible attack vectors for TOCTOU
@@ -387,8 +452,9 @@ def _load_cert_or_req(cert_or_req_str, load_func,
                       typ=crypto.FILETYPE_PEM):
     try:
         return load_func(typ, cert_or_req_str)
-    except crypto.Error:
-        logger.error("", exc_info=True)
+    except crypto.Error as err:
+        logger.debug("", exc_info=True)
+        logger.error("Encountered error while loading certificate or csr: %s", str(err))
         raise
 
 
@@ -437,6 +503,18 @@ def get_names_from_cert(csr, typ=crypto.FILETYPE_PEM):
         csr, crypto.load_certificate, typ)
 
 
+def get_names_from_req(csr: str, typ: int = crypto.FILETYPE_PEM) -> List[str]:
+    """Get a list of domains from a CSR, including the CN if it is set.
+
+    :param str cert: CSR (encoded).
+    :param typ: `crypto.FILETYPE_PEM` or `crypto.FILETYPE_ASN1`
+    :returns: A list of domain names.
+    :rtype: list
+
+    """
+    return _get_names_from_cert_or_req(csr, crypto.load_certificate_request, typ)
+
+
 def dump_pyopenssl_chain(chain, filetype=crypto.FILETYPE_PEM):
     """Dump certificate chain into a bundle.
 
@@ -589,7 +667,7 @@ def find_chain_with_issuer(fullchains, issuer_cn, warn_on_no_match=False):
 
     # Nothing matched, return whatever was first in the list.
     if warn_on_no_match:
-        logger.info("Certbot has been configured to prefer certificate chains with "
+        logger.warning("Certbot has been configured to prefer certificate chains with "
                     "issuer '%s', but no chain from the CA matched this issuer. Using "
                     "the default certificate chain instead.", issuer_cn)
     return fullchains[0]

certbot/certbot/display/ops.py

@@ -1,5 +1,6 @@
 """Contains UI methods for LE user operations."""
 import logging
+from textwrap import indent
 
 import zope.component
 
@@ -122,8 +123,7 @@ def choose_names(installer, question=None):
     names = get_valid_domains(domains)
 
     if not names:
-        return _choose_names_manually(
-            "No names were found in your configuration files. ")
+        return _choose_names_manually()
 
     code, names = _filter_names(names, question)
     if code == display_util.OK and names:
@@ -191,7 +191,8 @@ def _choose_names_manually(prompt_prefix=""):
     """
     code, input_ = z_util(interfaces.IDisplay).input(
         prompt_prefix +
-        "Please enter in your domain name(s) (comma and/or space separated) ",
+        "Please enter the domain name(s) you would like on your certificate "
+        "(comma and/or space separated)",
         cli_flag="--domains", force_interactive=True)
 
     if code == display_util.OK:
@@ -240,25 +241,22 @@ def success_installation(domains):
     :param list domains: domain names which were enabled
 
     """
-    z_util(interfaces.IDisplay).notification(
-        "Congratulations! You have successfully enabled {0}".format(
-            _gen_https_names(domains)),
-        pause=False)
+    display_util.notify(
+        "Congratulations! You have successfully enabled HTTPS on {0}"
+        .format(_gen_https_names(domains))
+    )
 
 
-def success_renewal(domains):
+def success_renewal(unused_domains):
     """Display a box confirming the renewal of an existing certificate.
 
     :param list domains: domain names which were renewed
 
     """
-    z_util(interfaces.IDisplay).notification(
+    display_util.notify(
         "Your existing certificate has been successfully renewed, and the "
-        "new certificate has been installed.{1}{1}"
-        "The new certificate covers the following domains: {0}".format(
-            _gen_https_names(domains),
-            os.linesep),
-        pause=False)
+        "new certificate has been installed."
+    )
 
 
 def success_revocation(cert_path):
@@ -273,6 +271,24 @@ def success_revocation(cert_path):
     )
 
 
+def report_executed_command(command_name: str, returncode: int, stdout: str, stderr: str) -> None:
+    """Display a message describing the success or failure of an executed process (e.g. hook).
+
+    :param str command_name: Human-readable description of the executed command
+    :param int returncode: The exit code of the executed command
+    :param str stdout: The stdout output of the executed command
+    :param str stderr: The stderr output of the executed command
+
+    """
+    out_s, err_s = stdout.strip(), stderr.strip()
+    if returncode != 0:
+        logger.warning("%s reported error code %d", command_name, returncode)
+    if out_s:
+        display_util.notify(f"{command_name} ran with output:\n{indent(out_s, ' ')}")
+    if err_s:
+        logger.warning("%s ran with error output:\n%s", command_name, indent(err_s, ' '))
+
+
 def _gen_https_names(domains):
     """Returns a string of the https domains.
 

certbot/certbot/ocsp.py

@@ -3,8 +3,8 @@ from datetime import datetime
 from datetime import timedelta
 import logging
 import re
+import subprocess
 from subprocess import PIPE
-from subprocess import Popen
 from typing import Optional
 from typing import Tuple
 
@@ -50,11 +50,10 @@ class RevocationChecker:
                 return
 
             # New versions of openssl want -header var=val, old ones want -header var val
-            test_host_format = Popen(["openssl", "ocsp", "-header", "var", "val"],
+            test_host_format = subprocess.run(["openssl", "ocsp", "-header", "var", "val"],
                                      stdout=PIPE, stderr=PIPE, universal_newlines=True,
-                                     env=util.env_no_snap_for_external_calls())
-            _out, err = test_host_format.communicate()
-            if "Missing =" in err:
+                                     check=False, env=util.env_no_snap_for_external_calls())
+            if "Missing =" in test_host_format.stderr:
                 self.host_args = lambda host: ["Host=" + host]
             else:
                 self.host_args = lambda host: ["Host", host]
@@ -193,7 +192,7 @@ def _check_ocsp_cryptography(cert_path: str, chain_path: str, url: str, timeout:
 
     # Check OCSP response validity
     if response_ocsp.response_status != ocsp.OCSPResponseStatus.SUCCESSFUL:
-        logger.error("Invalid OCSP response status for %s: %s",
+        logger.warning("Invalid OCSP response status for %s: %s",
                      cert_path, response_ocsp.response_status)
         return False
 
@@ -201,13 +200,13 @@ def _check_ocsp_cryptography(cert_path: str, chain_path: str, url: str, timeout:
     try:
         _check_ocsp_response(response_ocsp, request, issuer, cert_path)
     except UnsupportedAlgorithm as e:
-        logger.error(str(e))
+        logger.warning(str(e))
     except errors.Error as e:
-        logger.error(str(e))
+        logger.warning(str(e))
     except InvalidSignature:
-        logger.error('Invalid signature on OCSP response for %s', cert_path)
+        logger.warning('Invalid signature on OCSP response for %s', cert_path)
     except AssertionError as error:
-        logger.error('Invalid OCSP response for %s: %s.', cert_path, str(error))
+        logger.warning('Invalid OCSP response for %s: %s.', cert_path, str(error))
     else:
         # Check OCSP certificate status
         logger.debug("OCSP certificate status for %s is: %s",

certbot/certbot/plugins/common.py

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

certbot/certbot/plugins/dns_common.py

@@ -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]
 
@@ -63,7 +72,7 @@ class DNSAuthenticator(common.Plugin):
         # DNS updates take time to propagate and checking to see if the update has occurred is not
         # reliable (the machine this code is running on might be able to see an update before
         # the ACME server). So: we sleep for a short amount of time we believe to be long enough.
-        logger.info("Waiting %d seconds for DNS changes to propagate",
+        display_util.notify("Waiting %d seconds for DNS changes to propagate" %
                     self.conf('propagation-seconds'))
         sleep(self.conf('propagation-seconds'))
 

certbot/certbot/plugins/dns_common_lexicon.py

@@ -45,7 +45,7 @@ class LexiconClient:
         self._find_domain_id(domain)
 
         try:
-            self.provider.create_record(type='TXT', name=record_name, content=record_content)
+            self.provider.create_record(rtype='TXT', name=record_name, content=record_content)
         except RequestException as e:
             logger.debug('Encountered error adding TXT record: %s', e, exc_info=True)
             raise errors.PluginError('Error adding TXT record: {0}'.format(e))
@@ -67,7 +67,7 @@ class LexiconClient:
             return
 
         try:
-            self.provider.delete_record(type='TXT', name=record_name, content=record_content)
+            self.provider.delete_record(rtype='TXT', name=record_name, content=record_content)
         except RequestException as e:
             logger.debug('Encountered error deleting TXT record: %s', e, exc_info=True)
 

certbot/certbot/plugins/dns_test_common_lexicon.py

@@ -67,7 +67,8 @@ class _LexiconAwareTestCase(Protocol):
 
 class BaseLexiconAuthenticatorTest(dns_test_common.BaseAuthenticatorTest):
 
-    def test_perform(self: _AuthenticatorCallableLexiconTestCase):
+    @test_util.patch_get_utility()
+    def test_perform(self: _AuthenticatorCallableLexiconTestCase, unused_mock_get_utility):
         self.auth.perform([self.achall])
 
         expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY)]
@@ -94,7 +95,7 @@ class BaseLexiconClientTest:
     def test_add_txt_record(self: _LexiconAwareTestCase):
         self.client.add_txt_record(DOMAIN, self.record_name, self.record_content)
 
-        self.provider_mock.create_record.assert_called_with(type='TXT',
+        self.provider_mock.create_record.assert_called_with(rtype='TXT',
                                                             name=self.record_name,
                                                             content=self.record_content)
 
@@ -103,7 +104,7 @@ class BaseLexiconClientTest:
 
         self.client.add_txt_record(DOMAIN, self.record_name, self.record_content)
 
-        self.provider_mock.create_record.assert_called_with(type='TXT',
+        self.provider_mock.create_record.assert_called_with(rtype='TXT',
                                                             name=self.record_name,
                                                             content=self.record_content)
 
@@ -147,7 +148,7 @@ class BaseLexiconClientTest:
     def test_del_txt_record(self: _LexiconAwareTestCase):
         self.client.del_txt_record(DOMAIN, self.record_name, self.record_content)
 
-        self.provider_mock.delete_record.assert_called_with(type='TXT',
+        self.provider_mock.delete_record.assert_called_with(rtype='TXT',
                                                             name=self.record_name,
                                                             content=self.record_content)
 

certbot/certbot/reverter.py

@@ -198,6 +198,7 @@ class Reverter:
         Read the file returning the lines, and a pointer to the end of the file.
 
         """
+        # pylint: disable=consider-using-with
         # Open up filepath differently depending on if it already exists
         if os.path.isfile(filepath):
             op_fd = open(filepath, "r+")
@@ -348,26 +349,18 @@ class Reverter:
 
         """
         commands_fp = os.path.join(self._get_cp_dir(temporary), "COMMANDS")
-        command_file = None
         # It is strongly advised to set newline = '' on Python 3 with CSV,
         # and it fixes problems on Windows.
         kwargs = {'newline': ''}
         try:
-            if os.path.isfile(commands_fp):
-                command_file = open(commands_fp, "a", **kwargs)  # type: ignore
-            else:
-                command_file = open(commands_fp, "w", **kwargs)  # type: ignore
-
-            csvwriter = csv.writer(command_file)
-            csvwriter.writerow(command)
-
+            mode = "a" if os.path.isfile(commands_fp) else "w"
+            with open(commands_fp, mode, **kwargs) as f:  # type: ignore
+                csvwriter = csv.writer(f)
+                csvwriter.writerow(command)
         except (IOError, OSError):
             logger.error("Unable to register undo command")
             raise errors.ReverterError(
                 "Unable to register undo command.")
-        finally:
-            if command_file is not None:
-                command_file.close()
 
     def _get_cp_dir(self, temporary):
         """Return the proper reverter directory."""
@@ -521,7 +514,7 @@ class Reverter:
                 filesystem.replace(self.config.in_progress_dir, final_dir)
                 return
             except OSError:
-                logger.warning("Extreme, unexpected race condition, retrying (%s)", timestamp)
+                logger.warning("Unexpected race condition, retrying (%s)", timestamp)
 
         # After 10 attempts... something is probably wrong here...
         logger.error(

certbot/certbot/tests/util.py

@@ -259,7 +259,7 @@ class FreezableMock:
 def _create_get_utility_mock():
     display = FreezableMock()
     # Use pylint code for disable to keep on single line under line length limit
-    for name in interfaces.IDisplay.names():  # pylint: E1120
+    for name in interfaces.IDisplay.names():
         if name != 'notification':
             frozen_mock = FreezableMock(frozen=True, func=_assert_valid_call)
             setattr(display, name, frozen_mock)
@@ -284,7 +284,7 @@ def _create_get_utility_mock_with_stdout(stdout):
 
     display = FreezableMock()
     # Use pylint code for disable to keep on single line under line length limit
-    for name in interfaces.IDisplay.names():  # pylint: E1120
+    for name in interfaces.IDisplay.names():
         if name == 'notification':
             frozen_mock = FreezableMock(frozen=True,
                                         func=_write_msg)

certbot/certbot/util.py

@@ -16,6 +16,7 @@ from typing import Dict
 from typing import Text
 from typing import Tuple
 from typing import Union
+import warnings
 
 import configargparse
 
@@ -89,32 +90,31 @@ def env_no_snap_for_external_calls():
 def run_script(params, log=logger.error):
     """Run the script with the given params.
 
-    :param list params: List of parameters to pass to Popen
+    :param list params: List of parameters to pass to subprocess.run
     :param callable log: Logger method to use for errors
 
     """
     try:
-        proc = subprocess.Popen(params,
-                                stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE,
-                                universal_newlines=True,
-                                env=env_no_snap_for_external_calls())
+        proc = subprocess.run(params,
+                              check=False,
+                              stdout=subprocess.PIPE,
+                              stderr=subprocess.PIPE,
+                              universal_newlines=True,
+                              env=env_no_snap_for_external_calls())
 
     except (OSError, ValueError):
         msg = "Unable to run the command: %s" % " ".join(params)
         log(msg)
         raise errors.SubprocessError(msg)
 
-    stdout, stderr = proc.communicate()
-
     if proc.returncode != 0:
         msg = "Error while running %s.\n%s\n%s" % (
-            " ".join(params), stdout, stderr)
+            " ".join(params), proc.stdout, proc.stderr)
         # Enter recovery routine...
         log(msg)
         raise errors.SubprocessError(msg)
 
-    return stdout, stderr
+    return proc.stdout, proc.stderr
 
 
 def exe_exists(exe):
@@ -399,20 +399,20 @@ def get_python_os_info(pretty=False):
             os_ver = info[1]
     elif os_type.startswith('darwin'):
         try:
-            proc = subprocess.Popen(
+            proc = subprocess.run(
                 ["/usr/bin/sw_vers", "-productVersion"],
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                universal_newlines=True,
+                check=False, universal_newlines=True,
                 env=env_no_snap_for_external_calls(),
             )
         except OSError:
-            proc = subprocess.Popen(
+            proc = subprocess.run(
                 ["sw_vers", "-productVersion"],
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                universal_newlines=True,
+                check=False, universal_newlines=True,
                 env=env_no_snap_for_external_calls(),
             )
-        os_ver = proc.communicate()[0].rstrip('\n')
+        os_ver = proc.stdout.rstrip('\n')
     elif os_type.startswith('freebsd'):
         # eg "9.3-RC3-p1"
         os_ver = os_ver.partition("-")[0]
@@ -434,14 +434,14 @@ def safe_email(email):
     """Scrub email address before using it."""
     if EMAIL_REGEX.match(email) is not None:
         return not email.startswith(".") and ".." not in email
-    logger.warning("Invalid email address: %s.", email)
+    logger.error("Invalid email address: %s.", email)
     return False
 
 
 class DeprecatedArgumentAction(argparse.Action):
     """Action to log a warning when an argument is used."""
     def __call__(self, unused1, unused2, unused3, option_string=None):
-        logger.warning("Use of %s is deprecated.", option_string)
+        warnings.warn("Use of %s is deprecated." % option_string, DeprecationWarning)
 
 
 def add_deprecated_argument(add_argument, argument_name, nargs):

certbot/docs/cli-help.txt

@@ -41,7 +41,7 @@ optional arguments:
                         and ~/.config/letsencrypt/cli.ini)
   -v, --verbose         This flag can be used multiple times to incrementally
                         increase the verbosity of output, e.g. -vvv. (default:
-                        -2)
+                        -3)
   --max-log-backups MAX_LOG_BACKUPS
                         Specifies the maximum number of backup logs that
                         should be kept by Certbot's built in log rotation.
@@ -118,7 +118,7 @@ optional arguments:
                         case, and to know when to deprecate support for past
                         Python versions and flags. If you wish to hide this
                         information from the Let's Encrypt server, set this to
-                        "". (default: CertbotACMEClient/1.15.0 (certbot;
+                        "". (default: CertbotACMEClient/1.16.0 (certbot;
                         OS_NAME OS_VERSION) Authenticator/XXX Installer/YYY
                         (SUBCOMMAND; flags: FLAGS) Py/major.minor.patchlevel).
                         The flags encoded in the user agent are: --duplicate,

certbot/docs/using.rst

@@ -84,7 +84,7 @@ Apache
 
 The Apache plugin currently `supports
 <https://github.com/certbot/certbot/blob/master/certbot-apache/certbot_apache/_internal/entrypoint.py>`_
-modern OSes based on Debian, Fedora, SUSE, Gentoo and Darwin.
+modern OSes based on Debian, Fedora, SUSE, Gentoo, CentOS and Darwin.
 This automates both obtaining *and* installing certificates on an Apache
 webserver. To specify this plugin on the command line, simply include
 ``--apache``.
@@ -285,6 +285,7 @@ dns-clouddns_      Y    N    DNS Authentication using CloudDNS API
 dns-lightsail_     Y    N    DNS Authentication using Amazon Lightsail DNS API
 dns-inwx_          Y    Y    DNS Authentication for INWX through the XML API
 dns-azure_         Y    N    DNS Authentication using Azure DNS
+dns-godaddy_       Y    N    DNS Authentication using Godaddy DNS
 ================== ==== ==== ===============================================================
 
 .. _haproxy: https://github.com/greenhost/certbot-haproxy
@@ -300,6 +301,7 @@ dns-azure_         Y    N    DNS Authentication using Azure DNS
 .. _dns-lightsail: https://github.com/noi/certbot-dns-lightsail
 .. _dns-inwx: https://github.com/oGGy990/certbot-dns-inwx/
 .. _dns-azure: https://github.com/binkhq/certbot-dns-azure
+.. _dns-godaddy: https://github.com/miigotu/certbot-dns-godaddy
 
 If you're interested, you can also :ref:`write your own plugin <dev-plugin>`.
 
@@ -693,10 +695,50 @@ is done by means of a scheduled task which runs ``certbot renew`` periodically.
 
 If you are unsure whether you need to configure automated renewal:
 
-1. Review the instructions for your system at https://certbot.eff.org/instructions.
-   They will describe how to set up a scheduled task, if necessary.
-2. (Linux/BSD): Check your system's crontab (typically `/etc/crontab` and
-   `/etc/cron.*/*`) and systemd timers (``systemctl list-timers``).
+1. Review the instructions for your system and installation method at
+   https://certbot.eff.org/instructions. They will describe how to set up a scheduled task,
+   if necessary. If no step is listed, your system comes with automated renewal pre-installed,
+   and you should not need to take any additional actions.
+2. On Linux and BSD, you can check to see if your installation method has pre-installed a timer
+   for you. To do so, look for the ``certbot renew`` command in either your system's crontab
+   (typically `/etc/crontab` or `/etc/cron.*/*`) or systemd timers (``systemctl list-timers``).
+3. If you're still not sure, you can configure automated renewal manually by following the steps
+   in the next section. Certbot has been carefully engineered to handle the case where both manual
+   automated renewal and pre-installed automated renewal are set up.
+
+Setting up automated renewal
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If you think you may need to set up automated renewal, follow these instructions to set up a
+scheduled task to automatically renew your certificates in the background. If you are unsure 
+whether your system has a pre-installed scheduled task for Certbot, it is safe to follow these
+instructions to create one.
+
+If you're using Windows, these instructions are not neccessary as Certbot on Windows comes with
+a scheduled task for automated renewal pre-installed.
+
+Run the following line, which will add a cron job to `/etc/crontab`:
+
+.. code-block:: shell
+
+  SLEEPTIME=$(awk 'BEGIN{srand(); print int(rand()*(3600+1))}'); echo "0 0,12 * * * root sleep $SLEEPTIME && certbot renew -q" | sudo tee -a /etc/crontab > /dev/null
+
+If you needed to stop your webserver to run Certbot, you'll want to
+add ``pre`` and ``post`` hooks to stop and start your webserver automatically.
+For example, if your webserver is HAProxy, run the following commands to create the hook files
+in the appropriate directory:
+
+.. code-block:: shell
+
+  sudo sh -c 'printf "#!/bin/sh\nservice haproxy stop\n" > /etc/letsencrypt/renewal-hooks/pre/haproxy.sh'
+  sudo sh -c 'printf "#!/bin/sh\nservice haproxy start\n" > /etc/letsencrypt/renewal-hooks/post/haproxy.sh'
+  sudo chmod 755 /etc/letsencrypt/renewal-hooks/pre/haproxy.sh
+  sudo chmod 755 /etc/letsencrypt/renewal-hooks/post/haproxy.sh
+
+Congratulations, Certbot will now automatically renew your certificates in the background.
+
+If you are interested in learning more about how Certbot renews your certificates, see the
+`Renewing certificates`_ section above.
 
 .. _where-certs:
 

certbot/examples/dev-cli.ini

@@ -13,8 +13,6 @@ domains = example.com
 text = True
 agree-tos = True
 debug = True
-# Unfortunately, it's not possible to specify "verbose" multiple times
-# (correspondingly to -vvvvvv)
-verbose = True
+verbose = 3
 
 authenticator = standalone

certbot/local-oldest-requirements.txt

@@ -1,2 +0,0 @@
-# Remember to update setup.py to match the package versions below.
-acme[dev]==1.8.0

certbot/setup.py

@@ -41,7 +41,10 @@ version = meta['version']
 # here to avoid masking the more specific request requirements in acme. See
 # https://github.com/pypa/pip/issues/988 for more info.
 install_requires = [
-    'acme>=1.8.0',
+    # We specify the minimum acme version as the current Certbot version for
+    # simplicity. See https://github.com/certbot/certbot/issues/8761 for more
+    # info.
+    f'acme>={version}',
     # We technically need ConfigArgParse 0.10.0 for Python 2.6 support, but
     # saying so here causes a runtime error against our temporary fork of 0.9.3
     # in which we added 2.6 support (see #2243), so we relax the requirement.

certbot/tests/auth_handler_test.py

@@ -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
 
@@ -68,10 +69,9 @@ class HandleAuthorizationsTest(unittest.TestCase):
         from certbot._internal.auth_handler import AuthHandler
 
         self.mock_display = mock.Mock()
+        self.mock_config = mock.Mock(debug_challenges=False)
         zope.component.provideUtility(
             self.mock_display, interfaces.IDisplay)
-        zope.component.provideUtility(
-            mock.Mock(debug_challenges=False), interfaces.IConfig)
 
         self.mock_auth = mock.MagicMock(name="ApacheConfigurator")
 
@@ -98,7 +98,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         self.mock_net.poll.side_effect = _gen_mock_on_poll(retry=1, wait_value=30)
         with mock.patch('certbot._internal.auth_handler.time') as mock_time:
-            authzr = self.handler.handle_authorizations(mock_order)
+            authzr = self.handler.handle_authorizations(mock_order, self.mock_config)
 
             self.assertEqual(self.mock_net.answer_challenge.call_count, 1)
 
@@ -130,7 +130,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=False)
         mock_order = mock.MagicMock(authorizations=[authzr])
-        authzr = self.handler.handle_authorizations(mock_order)
+        authzr = self.handler.handle_authorizations(mock_order, self.mock_config)
 
         self.assertEqual(self.mock_net.answer_challenge.call_count, 2)
 
@@ -151,7 +151,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=False)
         mock_order = mock.MagicMock(authorizations=[authzr])
-        authzr = self.handler.handle_authorizations(mock_order)
+        authzr = self.handler.handle_authorizations(mock_order, self.mock_config)
 
         self.assertEqual(self.mock_net.answer_challenge.call_count, 1)
 
@@ -175,7 +175,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
         mock_order = mock.MagicMock(authorizations=authzrs)
 
         self.mock_net.poll.side_effect = _gen_mock_on_poll()
-        authzr = self.handler.handle_authorizations(mock_order)
+        authzr = self.handler.handle_authorizations(mock_order, self.mock_config)
 
         self.assertEqual(self.mock_net.answer_challenge.call_count, 3)
 
@@ -194,14 +194,13 @@ class HandleAuthorizationsTest(unittest.TestCase):
         self._test_name3_http_01_3_common(combos=False)
 
     def test_debug_challenges(self):
-        zope.component.provideUtility(
-            mock.Mock(debug_challenges=True), interfaces.IConfig)
+        config = mock.Mock(debug_challenges=True)
         authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)]
         mock_order = mock.MagicMock(authorizations=authzrs)
 
         self.mock_net.poll.side_effect = _gen_mock_on_poll()
 
-        self.handler.handle_authorizations(mock_order)
+        self.handler.handle_authorizations(mock_order, config)
 
         self.assertEqual(self.mock_net.answer_challenge.call_count, 1)
         self.assertEqual(self.mock_display.notification.call_count, 1)
@@ -213,7 +212,8 @@ class HandleAuthorizationsTest(unittest.TestCase):
         self.mock_auth.perform.side_effect = errors.AuthorizationError
 
         self.assertRaises(
-            errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+            errors.AuthorizationError, self.handler.handle_authorizations,
+            mock_order, self.mock_config)
 
     def test_max_retries_exceeded(self):
         authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES)]
@@ -224,12 +224,13 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         with self.assertRaises(errors.AuthorizationError) as error:
             # We retry only once, so retries will be exhausted before STATUS_VALID is returned.
-            self.handler.handle_authorizations(mock_order, False, 1)
+            self.handler.handle_authorizations(mock_order, self.mock_config, False, 1)
         self.assertIn('All authorizations were not finalized by the CA.', str(error.exception))
 
     def test_no_domains(self):
         mock_order = mock.MagicMock(authorizations=[])
-        self.assertRaises(errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+        self.assertRaises(errors.AuthorizationError, self.handler.handle_authorizations,
+                          mock_order, self.mock_config)
 
     def _test_preferred_challenge_choice_common(self, combos):
         authzrs = [gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=combos)]
@@ -241,7 +242,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
                                          challenges.DNS01.typ,))
 
         self.mock_net.poll.side_effect = _gen_mock_on_poll()
-        self.handler.handle_authorizations(mock_order)
+        self.handler.handle_authorizations(mock_order, self.mock_config)
 
         self.assertEqual(self.mock_auth.cleanup.call_count, 1)
         self.assertEqual(
@@ -259,7 +260,8 @@ class HandleAuthorizationsTest(unittest.TestCase):
         mock_order = mock.MagicMock(authorizations=authzrs)
         self.handler.pref_challs.append(challenges.DNS01.typ)
         self.assertRaises(
-            errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+            errors.AuthorizationError, self.handler.handle_authorizations,
+            mock_order, self.mock_config)
 
     def test_preferred_challenges_not_supported_acme_1(self):
         self._test_preferred_challenges_not_supported_common(combos=True)
@@ -272,14 +274,16 @@ class HandleAuthorizationsTest(unittest.TestCase):
         authzrs = [gen_dom_authzr(domain="0", challs=[acme_util.DNS01])]
         mock_order = mock.MagicMock(authorizations=authzrs)
         self.assertRaises(
-            errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+            errors.AuthorizationError, self.handler.handle_authorizations,
+            mock_order, self.mock_config)
 
     def test_perform_error(self):
         self.mock_auth.perform.side_effect = errors.AuthorizationError
 
         authzr = gen_dom_authzr(domain="0", challs=acme_util.CHALLENGES, combos=True)
         mock_order = mock.MagicMock(authorizations=[authzr])
-        self.assertRaises(errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+        self.assertRaises(errors.AuthorizationError, self.handler.handle_authorizations,
+                          mock_order, self.mock_config)
 
         self.assertEqual(self.mock_auth.cleanup.call_count, 1)
         self.assertEqual(
@@ -292,7 +296,8 @@ class HandleAuthorizationsTest(unittest.TestCase):
         mock_order = mock.MagicMock(authorizations=authzrs)
 
         self.assertRaises(
-            errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+            errors.AuthorizationError, self.handler.handle_authorizations,
+            mock_order, self.mock_config)
         self.assertEqual(self.mock_auth.cleanup.call_count, 1)
         self.assertEqual(
             self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01")
@@ -304,7 +309,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         with test_util.patch_get_utility():
             with self.assertRaises(errors.AuthorizationError) as error:
-                self.handler.handle_authorizations(mock_order, False)
+                self.handler.handle_authorizations(mock_order, self.mock_config, False)
         self.assertIn('Some challenges have failed.', str(error.exception))
         self.assertEqual(self.mock_auth.cleanup.call_count, 1)
         self.assertEqual(
@@ -327,8 +332,9 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         mock_order = mock.MagicMock(authorizations=authzrs)
 
-        with mock.patch('certbot._internal.auth_handler._report_failed_authzrs') as mock_report:
-            valid_authzr = self.handler.handle_authorizations(mock_order, True)
+        with mock.patch('certbot._internal.auth_handler.AuthHandler._report_failed_authzrs') \
+            as mock_report:
+            valid_authzr = self.handler.handle_authorizations(mock_order, self.mock_config, True)
 
         # Because best_effort=True, we did not blow up. Instead ...
         self.assertEqual(len(valid_authzr), 1)  # ... the valid authzr has been processed
@@ -338,7 +344,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         with test_util.patch_get_utility():
             with self.assertRaises(errors.AuthorizationError) as error:
-                self.handler.handle_authorizations(mock_order, True)
+                self.handler.handle_authorizations(mock_order, self.mock_config, True)
 
         # Despite best_effort=True, process will fail because no authzr is valid.
         self.assertIn('All challenges have failed.', str(error.exception))
@@ -352,7 +358,8 @@ class HandleAuthorizationsTest(unittest.TestCase):
                 [messages.STATUS_PENDING], False)
         mock_order = mock.MagicMock(authorizations=[authzr])
         self.assertRaises(
-            errors.AuthorizationError, self.handler.handle_authorizations, mock_order)
+            errors.AuthorizationError, self.handler.handle_authorizations,
+            mock_order, self.mock_config)
 
         # With a validated challenge that is not supported by the plugin, we
         # expect the challenge to not be solved again and
@@ -362,7 +369,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
                 [acme_util.DNS01],
                 [messages.STATUS_VALID], False)
         mock_order = mock.MagicMock(authorizations=[authzr])
-        self.handler.handle_authorizations(mock_order)
+        self.handler.handle_authorizations(mock_order, self.mock_config)
 
     def test_valid_authzrs_deactivated(self):
         """When we deactivate valid authzrs in an orderr, we expect them to become deactivated
@@ -474,10 +481,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 +519,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):