Deprecate usage of IConfig as a singleton in Certbot (#8869)

* Deprecate usage of IConfig as a singleton in Certbot

* Fix local oldest requirements

* Add changelog

* Add tests for certbot.crypto_util.init_save_* functions

Co-authored-by: Brad Warren <bmw@users.noreply.github.com>

9 files changed, 182 insertions, 77 deletions

diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index b22576b2f..d23a88c94 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -19,6 +19,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
   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
 
diff --git a/certbot/certbot/_internal/auth_handler.py b/certbot/certbot/_internal/auth_handler.py
index 036cc91e3..41ae492e2 100644
--- a/certbot/certbot/_internal/auth_handler.py
+++ b/certbot/certbot/_internal/auth_handler.py
@@ -44,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
@@ -72,7 +73,6 @@ class AuthHandler:
                 resps = self.auth.perform(achalls)
 
                 # If debug is on, wait for user input before starting the verification process.
-                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. '
diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py
index c9a240de3..5c0bed220 100644
--- a/certbot/certbot/_internal/client.py
+++ b/certbot/certbot/_internal/client.py
@@ -334,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):
@@ -362,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
@@ -420,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):
diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py
index 4c6dfac5c..2100a2943 100644
--- a/certbot/certbot/_internal/main.py
+++ b/certbot/certbot/_internal/main.py
@@ -1525,6 +1525,9 @@ def main(cli_args=None):
     # 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.
diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py
index acb64d7e2..b4fa6aa01 100644
--- a/certbot/certbot/_internal/renewal.py
+++ b/certbot/certbot/_internal/renewal.py
@@ -450,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
diff --git a/certbot/certbot/crypto_util.py b/certbot/certbot/crypto_util.py
index 51ef2c53a..5cb21b510 100644
--- a/certbot/certbot/crypto_util.py
+++ b/certbot/certbot/crypto_util.py
@@ -9,7 +9,7 @@ import logging
 import re
 import warnings
 
-from typing import List
+from typing import List, Set
 # See https://github.com/pyca/cryptography/issues/4275
 from cryptography import x509  # type: ignore
 from cryptography.exceptions import InvalidSignature
@@ -38,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.
@@ -53,6 +54,8 @@ def init_save_key(
     :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`
@@ -69,9 +72,8 @@ def init_save_key(
         logger.error("Encountered error while making key: %s", str(err))
         raise err
 
-    config = zope.component.getUtility(interfaces.IConfig)
     # Save file
-    util.make_or_verify_dir(key_dir, 0o700, config.strict_permissions)
+    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:
@@ -84,27 +86,61 @@ def init_save_key(
     return util.Key(key_path, key_pem)
 
 
-def init_save_csr(privkey, names, path):
+# 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]
+    :param str elliptic_curve: Name of the elliptic curve if key type is ecdsa.
+    :param str keyname: Filename of key
+
+    :returns: Key
+    :rtype: :class:`certbot.util.Key`
+
+    :raises ValueError: If unable to generate the key given key_size.
+
+    """
+    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)
+
+    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`
 
     """
-    config = zope.component.getUtility(interfaces.IConfig)
-
     csr_pem = acme_crypto_util.make_csr(
-        privkey.pem, names, must_staple=config.must_staple)
+        privkey.pem, names, must_staple=must_staple)
 
     # Save CSR
-    util.make_or_verify_dir(path, 0o755, config.strict_permissions)
+    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:
@@ -114,6 +150,33 @@ def init_save_csr(privkey, names, path):
     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`
+
+    :param set names: `str` names to include in the CSR
+
+    :param str path: Certificate save directory.
+
+    :returns: CSR
+    :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)
+
+    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
 # We should either...
 # A. Do more checks to verify that the CSR is trusted/valid
diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py
index 751c445fe..25b19def7 100644
--- a/certbot/tests/auth_handler_test.py
+++ b/certbot/tests/auth_handler_test.py
@@ -69,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")
 
@@ -99,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)
 
@@ -131,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)
 
@@ -152,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)
 
@@ -176,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)
 
@@ -195,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)
@@ -214,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)]
@@ -225,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)]
@@ -242,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(
@@ -260,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)
@@ -273,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(
@@ -293,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")
@@ -305,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(
@@ -330,7 +334,7 @@ class HandleAuthorizationsTest(unittest.TestCase):
 
         with mock.patch('certbot._internal.auth_handler.AuthHandler._report_failed_authzrs') \
             as mock_report:
-            valid_authzr = self.handler.handle_authorizations(mock_order, True)
+            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
@@ -340,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))
@@ -354,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
@@ -364,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
diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py
index 8d9811fa3..51c6767f6 100644
--- a/certbot/tests/client_test.py
+++ b/certbot/tests/client_test.py
@@ -242,6 +242,7 @@ class ClientTest(ClientTestCommon):
 
         self.config.allow_subset_of_names = False
         self.config.dry_run = False
+        self.config.strict_permissions = True
         self.eg_domains = ["example.com", "www.example.com"]
         self.eg_order = mock.MagicMock(
             authorizations=[None],
@@ -263,6 +264,7 @@ class ClientTest(ClientTestCommon):
         if auth_count == 1:
             self.client.auth_handler.handle_authorizations.assert_called_once_with(
                 self.eg_order,
+                self.config,
                 self.config.allow_subset_of_names)
         else:
             self.assertEqual(self.client.auth_handler.handle_authorizations.call_count, auth_count)
@@ -273,16 +275,14 @@ class ClientTest(ClientTestCommon):
 
     @mock.patch("certbot._internal.client.crypto_util")
     @mock.patch("certbot._internal.client.logger")
-    @test_util.patch_get_utility()
-    def test_obtain_certificate_from_csr(self, unused_mock_get_utility,
-                                         mock_logger, mock_crypto_util):
+    def test_obtain_certificate_from_csr(self, mock_logger, mock_crypto_util):
         self._mock_obtain_certificate()
         test_csr = util.CSR(form="pem", file=None, data=CSR_SAN)
         auth_handler = self.client.auth_handler
         self._set_mock_from_fullchain(mock_crypto_util.cert_and_chain_from_fullchain)
 
         orderr = self.acme.new_order(test_csr.data)
-        auth_handler.handle_authorizations(orderr, False)
+        auth_handler.handle_authorizations(orderr, self.config, False)
         self.assertEqual(
             (mock.sentinel.cert, mock.sentinel.chain),
             self.client.obtain_certificate_from_csr(
@@ -310,7 +310,7 @@ class ClientTest(ClientTestCommon):
             self.client.obtain_certificate_from_csr(
                 test_csr,
                 orderr=None))
-        auth_handler.handle_authorizations.assert_called_with(self.eg_order, False)
+        auth_handler.handle_authorizations.assert_called_with(self.eg_order, self.config, False)
 
         # Test for no auth_handler
         self.client.auth_handler = None
@@ -323,20 +323,21 @@ class ClientTest(ClientTestCommon):
     @mock.patch("certbot._internal.client.crypto_util")
     def test_obtain_certificate(self, mock_crypto_util):
         csr = util.CSR(form="pem", file=None, data=CSR_SAN)
-        mock_crypto_util.init_save_csr.return_value = csr
-        mock_crypto_util.init_save_key.return_value = mock.sentinel.key
+        mock_crypto_util.generate_csr.return_value = csr
+        mock_crypto_util.generate_key.return_value = mock.sentinel.key
         self._set_mock_from_fullchain(mock_crypto_util.cert_and_chain_from_fullchain)
 
         self._test_obtain_certificate_common(mock.sentinel.key, csr)
 
-        mock_crypto_util.init_save_key.assert_called_once_with(
+        mock_crypto_util.generate_key.assert_called_once_with(
             key_size=self.config.rsa_key_size,
             key_dir=self.config.key_dir,
             key_type=self.config.key_type,
-            elliptic_curve=None,  # elliptic curve is not set
+            elliptic_curve="secp256r1",
+            strict_permissions=True,
         )
-        mock_crypto_util.init_save_csr.assert_called_once_with(
-            mock.sentinel.key, self.eg_domains, self.config.csr_dir)
+        mock_crypto_util.generate_csr.assert_called_once_with(
+            mock.sentinel.key, self.eg_domains, self.config.csr_dir, False, True)
         mock_crypto_util.cert_and_chain_from_fullchain.assert_called_once_with(
             self.eg_order.fullchain_pem)
 
@@ -345,16 +346,16 @@ class ClientTest(ClientTestCommon):
     def test_obtain_certificate_partial_success(self, mock_remove, mock_crypto_util):
         csr = util.CSR(form="pem", file=mock.sentinel.csr_file, data=CSR_SAN)
         key = util.CSR(form="pem", file=mock.sentinel.key_file, data=CSR_SAN)
-        mock_crypto_util.init_save_csr.return_value = csr
-        mock_crypto_util.init_save_key.return_value = key
+        mock_crypto_util.generate_csr.return_value = csr
+        mock_crypto_util.generate_key.return_value = key
         self._set_mock_from_fullchain(mock_crypto_util.cert_and_chain_from_fullchain)
 
         authzr = self._authzr_from_domains(["example.com"])
         self.config.allow_subset_of_names = True
         self._test_obtain_certificate_common(key, csr, authzr_ret=authzr, auth_count=2)
 
-        self.assertEqual(mock_crypto_util.init_save_key.call_count, 2)
-        self.assertEqual(mock_crypto_util.init_save_csr.call_count, 2)
+        self.assertEqual(mock_crypto_util.generate_key.call_count, 2)
+        self.assertEqual(mock_crypto_util.generate_csr.call_count, 2)
         self.assertEqual(mock_remove.call_count, 2)
         self.assertEqual(mock_crypto_util.cert_and_chain_from_fullchain.call_count, 1)
 
@@ -372,13 +373,13 @@ class ClientTest(ClientTestCommon):
 
         mock_crypto.make_key.assert_called_once_with(
             bits=self.config.rsa_key_size,
-            elliptic_curve=None,  # not making an elliptic private key
+            elliptic_curve="secp256r1",
             key_type=self.config.key_type,
         )
         mock_acme_crypto.make_csr.assert_called_once_with(
             mock.sentinel.key_pem, self.eg_domains, self.config.must_staple)
-        mock_crypto.init_save_key.assert_not_called()
-        mock_crypto.init_save_csr.assert_not_called()
+        mock_crypto.generate_key.assert_not_called()
+        mock_crypto.generate_csr.assert_not_called()
         self.assertEqual(mock_crypto.cert_and_chain_from_fullchain.call_count, 1)
 
     @mock.patch("certbot._internal.client.logger")
diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py
index 34ad4b09a..858db079c 100644
--- a/certbot/tests/crypto_util_test.py
+++ b/certbot/tests/crypto_util_test.py
@@ -2,15 +2,15 @@
 import logging
 import unittest
 
+import certbot.util
+
 try:
     import mock
 except ImportError:  # pragma: no cover
     from unittest import mock
 import OpenSSL
-import zope.component
 
 from certbot import errors
-from certbot import interfaces
 from certbot import util
 from certbot.compat import filesystem
 from certbot.compat import os
@@ -33,8 +33,8 @@ CERT_ISSUER = test_util.load_vector('cert_intermediate_1.pem')
 CERT_ALT_ISSUER = test_util.load_vector('cert_intermediate_2.pem')
 
 
-class InitSaveKeyTest(test_util.TempDirTestCase):
-    """Tests for certbot.crypto_util.init_save_key."""
+class GenerateKeyTest(test_util.TempDirTestCase):
+    """Tests for certbot.crypto_util.generate_key."""
     def setUp(self):
         super().setUp()
 
@@ -42,8 +42,6 @@ class InitSaveKeyTest(test_util.TempDirTestCase):
         filesystem.mkdir(self.workdir, mode=0o700)
 
         logging.disable(logging.CRITICAL)
-        zope.component.provideUtility(
-            mock.Mock(strict_permissions=True), interfaces.IConfig)
 
     def tearDown(self):
         super().tearDown()
@@ -52,8 +50,8 @@ class InitSaveKeyTest(test_util.TempDirTestCase):
 
     @classmethod
     def _call(cls, key_size, key_dir):
-        from certbot.crypto_util import init_save_key
-        return init_save_key(key_size, key_dir, 'key-certbot.pem')
+        from certbot.crypto_util import generate_key
+        return generate_key(key_size, key_dir, 'key-certbot.pem', strict_permissions=True)
 
     @mock.patch('certbot.crypto_util.make_key')
     def test_success(self, mock_make):
@@ -69,29 +67,57 @@ class InitSaveKeyTest(test_util.TempDirTestCase):
         self.assertRaises(ValueError, self._call, 431, self.workdir)
 
 
-class InitSaveCSRTest(test_util.TempDirTestCase):
-    """Tests for certbot.crypto_util.init_save_csr."""
+class InitSaveKey(unittest.TestCase):
+    """Test for certbot.crypto_util.init_save_key."""
+    @mock.patch("certbot.crypto_util.generate_key")
+    @mock.patch("certbot.crypto_util.zope.component")
+    def test_it(self, mock_zope, mock_generate):
+        from certbot.crypto_util import init_save_key
 
-    def setUp(self):
-        super().setUp()
+        mock_zope.getUtility.return_value = mock.MagicMock(strict_permissions=True)
+
+        with self.assertWarns(DeprecationWarning):
+            init_save_key(4096, "/some/path")
+
+        mock_generate.assert_called_with(4096, "/some/path", elliptic_curve="secp256r1",
+                                         key_type="rsa", keyname="key-certbot.pem",
+                                         strict_permissions=True)
 
-        zope.component.provideUtility(
-            mock.Mock(strict_permissions=True), interfaces.IConfig)
 
+class GenerateCSRTest(test_util.TempDirTestCase):
+    """Tests for certbot.crypto_util.generate_csr."""
     @mock.patch('acme.crypto_util.make_csr')
     @mock.patch('certbot.crypto_util.util.make_or_verify_dir')
     def test_it(self, unused_mock_verify, mock_csr):
-        from certbot.crypto_util import init_save_csr
+        from certbot.crypto_util import generate_csr
 
         mock_csr.return_value = b'csr_pem'
 
-        csr = init_save_csr(
-            mock.Mock(pem='dummy_key'), 'example.com', self.tempdir)
+        csr = generate_csr(
+            mock.Mock(pem='dummy_key'), 'example.com', self.tempdir, strict_permissions=True)
 
         self.assertEqual(csr.data, b'csr_pem')
         self.assertIn('csr-certbot.pem', csr.file)
 
 
+class InitSaveCsr(unittest.TestCase):
+    """Tests for certbot.crypto_util.init_save_csr."""
+    @mock.patch("certbot.crypto_util.generate_csr")
+    @mock.patch("certbot.crypto_util.zope.component")
+    def test_it(self, mock_zope, mock_generate):
+        from certbot.crypto_util import init_save_csr
+
+        mock_zope.getUtility.return_value = mock.MagicMock(must_staple=True,
+                                                           strict_permissions=True)
+        key = certbot.util.Key(file=None, pem=None)
+
+        with self.assertWarns(DeprecationWarning):
+            init_save_csr(key, {"dummy"}, "/some/path")
+
+        mock_generate.assert_called_with(key, {"dummy"}, "/some/path",
+                                         must_staple=True, strict_permissions=True)
+
+
 class ValidCSRTest(unittest.TestCase):
     """Tests for certbot.crypto_util.valid_csr."""