Repository: certbot/certbot
Pull request: Deprecate {csr, keys} dirs & automatically truncate lineages
Based on my design [here](https://docs.google.com/document/d/1jGh_bZPnrhi96KzuIcyCJfnudl4m3pRPGkiK4fTo8e4/edit?usp=sharing). 

Fixes https://github.com/certbot/certbot/issues/4634 and https://github.com/certbot/certbot/issues/4635.

- [x] Deprecate `NamespaceConfig.csr_dir`,`NamespaceConfig.key_dir`, ~~`constants.CSR_DIR` and `constants.KEY_DIR`~~. (`constants` is `_internal` so we can just delete it eventually).
- [x] Update `certbot.crypto_util.generate_csr` and `.generate_key` to make `csr_dir` and `key_dir` optional, respectively.
- [x] Change `certbot._internal.client.Client.obtain_certificate` to no longer include `csr_dir` and `key_dir` to the `.generate_csr` and `.generate_key` calls, respectively.
- Automatically delete unwanted lineage items:
  - [x] In `certbot._internal.storage.RenewableCert`, add a function to truncate the lineage history according to the criteria (keep the current and the 5 prior certificates). 
      - [x] Add a test suite for `truncate` 
  - [x] In `certbot._internal.renewal.renew_cert`, call the lineage truncation function after the symlinks have been updated for the renewal.




Problem statement:
Deprecate /etc/letsencrypt/{keys,csr}
Although these were added for super-plausible reasons about going back and finding stuff that people might need, I'm not sure if I've ever come across any case where someone successfully used their contents correctly (with the possible exception of people who naively deleted all of `/etc/letsencrypt/archive`), and I've come across many people who were confused by their presence, some of whom even thought that the CSR was the cert and that they were supposed to point Apache to the most recent CSR in `/etc/letsencrypt/csr` and the most recent key in `/etc/letsencrypt/keys`.

I think it is possible that almost no users manage to use these backups correctly and that they ought to be deprecated and removed.

Hints from issue discussion:
While I understand the argument for deleting private keys (for the future secrecy (FS)), I might be missing the argument(s) for deleting the past CSRs (except for people being confused).
I don't think CSRs affect FS, do they?

I've just actually looked at CSRs to analyze certain aspects of it... While it is not a common use (or, rather, it might be very uncommon), I used them nevertheless. 
@St-Ranger, could you describe how you found the CSRs useful?

We just had a forum thread where someone said that they took up a lot of disk space and so I'm interested in pursuing the question of having Certbot save fewer archival files that it doesn't refer back to itself.
FWIW, you can extract the CSR from `letsencrypt.log`. It would be a hassle to do routinely, but still useful for a debugging emergency.
I personally think we can just make these changes. We never heard back on the use case for `/etc/letsencrypt/csr`, we didn't consider either directory something that should be considered stable as part of 1.0 (see https://certbot.eff.org/docs/compatibility.html), and I can't imagine us no longer using these directories would affect (m)any users.

No longer using `/etc/letsencrypt/csr` should be easy, however, one nice thing about `/etc/letsencrypt/keys` is I believe that is the only place the key is written to disk before we obtain a certificate. If Certbot crashes after the CA issues the cert but before we could write it and the key to disk, it's currently the only way to recover and use that certificate (which can be obtained from public logs).

While I do think this is useful in theory, in practice probably everyone just reissues their cert. If we did want to preserve this behavior though, I think we'd need to do something like either refactoring things to write the key to `/etc/letsencrypt/archive` before the certificate is obtained or delete the copy of the key written in `/etc/letsencrypt/keys` after it was written to the archive.
@bmw any update on this issue?
@bmw One idea that wouldn't require making code changes in Certbot itself would be to have some of the installation methods, at least on Linux, add timers or cronjobs that occasionally run something like

`find /etc/letsencrypt/keys -type f -mtime +90 -delete`
`find /etc/letsencrypt/csr -type f -mtime +90 -delete`

Or even some other number instead of 90. (GNU find counts mtime in terms of days.)

This would stop tons of older files from accumulating in these directories, in advance of any behavior change inside Certbot's code.

I can double-check whether this syntax works in other versions of find or whether anything is a GNU-specific extension.
Hey Seth. @alexzorin has actually been working on this issue and https://github.com/certbot/certbot/issues/4635 a bit lately. I'll let him decide whether he wants to share his current thinking more widely right now, but regardless, stay tuned. I think people should expect to see something here in the coming months.

Commit message: docs: remove reference to /archive and /keys

Patch to review:
diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
index 65eca976d58..356eaa77406 100644
--- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
+++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
@@ -329,7 +329,6 @@ def test_renew_with_changed_private_key_complexity(context: IntegrationTestsCont
 
     context.certbot(['renew', '--rsa-key-size', '2048'])
 
-    assert_cert_count_for_lineage(context.config_dir, certname, 3)
     key3 = join(context.config_dir, 'archive', certname, 'privkey3.pem')
     assert_rsa_key(key3, 2048)
 
@@ -437,38 +436,37 @@ def test_reuse_key(context: IntegrationTestsContext) -> None:
 
     with open(join(context.config_dir, 'archive/{0}/privkey1.pem').format(certname), 'r') as file:
         privkey1 = file.read()
+    with open(join(context.config_dir, 'archive/{0}/cert1.pem').format(certname), 'r') as file:
+        cert1 = file.read()
     with open(join(context.config_dir, 'archive/{0}/privkey2.pem').format(certname), 'r') as file:
         privkey2 = file.read()
+    with open(join(context.config_dir, 'archive/{0}/cert2.pem').format(certname), 'r') as file:
+        cert2 = file.read()
     assert privkey1 == privkey2
 
     context.certbot(['--cert-name', certname, '--domains', certname, '--force-renewal'])
 
     with open(join(context.config_dir, 'archive/{0}/privkey3.pem').format(certname), 'r') as file:
         privkey3 = file.read()
+    with open(join(context.config_dir, 'archive/{0}/cert3.pem').format(certname), 'r') as file:
+        cert3 = file.read()
     assert privkey2 != privkey3
 
     context.certbot(['--cert-name', certname, '--domains', certname,
                      '--reuse-key','--force-renewal'])
-    context.certbot(['renew', '--cert-name', certname, '--no-reuse-key', '--force-renewal'])
-    context.certbot(['renew', '--cert-name', certname, '--force-renewal'])
-
     with open(join(context.config_dir, 'archive/{0}/privkey4.pem').format(certname), 'r') as file:
         privkey4 = file.read()
+    context.certbot(['renew', '--cert-name', certname, '--no-reuse-key', '--force-renewal'])
     with open(join(context.config_dir, 'archive/{0}/privkey5.pem').format(certname), 'r') as file:
         privkey5 = file.read()
+    context.certbot(['renew', '--cert-name', certname, '--force-renewal'])
     with open(join(context.config_dir, 'archive/{0}/privkey6.pem').format(certname), 'r') as file:
         privkey6 = file.read()
+
     assert privkey3 == privkey4
     assert privkey4 != privkey5
     assert privkey5 != privkey6
 
-    with open(join(context.config_dir, 'archive/{0}/cert1.pem').format(certname), 'r') as file:
-        cert1 = file.read()
-    with open(join(context.config_dir, 'archive/{0}/cert2.pem').format(certname), 'r') as file:
-        cert2 = file.read()
-    with open(join(context.config_dir, 'archive/{0}/cert3.pem').format(certname), 'r') as file:
-        cert3 = file.read()
-
     assert len({cert1, cert2, cert3}) == 3
 
 
@@ -615,7 +613,6 @@ def test_renew_with_ec_keys(context: IntegrationTestsContext) -> None:
     # to the lineage key type, Certbot should keep the lineage key type. The curve will still
     # change to the default value, in order to stay consistent with the behavior of certonly.
     context.certbot(['certonly', '--force-renewal', '-d', certname])
-    assert_cert_count_for_lineage(context.config_dir, certname, 3)
     key3 = join(context.config_dir, 'archive', certname, 'privkey3.pem')
     assert 200 < os.stat(key3).st_size < 250  # ec keys of 256 bits are ~225 bytes
     assert_elliptic_key(key3, SECP256R1)
@@ -629,14 +626,12 @@ def test_renew_with_ec_keys(context: IntegrationTestsContext) -> None:
 
     context.certbot(['certonly', '--force-renewal', '-d', certname,
                      '--key-type', 'rsa', '--cert-name', certname])
-    assert_cert_count_for_lineage(context.config_dir, certname, 4)
     key4 = join(context.config_dir, 'archive', certname, 'privkey4.pem')
     assert_rsa_key(key4)
 
     # We expect that the previous behavior of requiring both --cert-name and
     # --key-type to be set to not apply to the renew subcommand.
     context.certbot(['renew', '--force-renewal', '--key-type', 'ecdsa'])
-    assert_cert_count_for_lineage(context.config_dir, certname, 5)
     key5 = join(context.config_dir, 'archive', certname, 'privkey5.pem')
     assert 200 < os.stat(key5).st_size < 250  # ec keys of 256 bits are ~225 bytes
     assert_elliptic_key(key5, SECP256R1)
diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py
index b91b11a53cc..07243df2bdf 100644
--- a/certbot-nginx/certbot_nginx/_internal/configurator.py
+++ b/certbot-nginx/certbot_nginx/_internal/configurator.py
@@ -693,6 +693,7 @@ def _get_snakeoil_paths(self) -> Tuple[str, str]:
         le_key = crypto_util.generate_key(
             key_size=1024, key_dir=tmp_dir, keyname="key.pem",
             strict_permissions=self.config.strict_permissions)
+        assert le_key.file is not None
         key = OpenSSL.crypto.load_privatekey(
             OpenSSL.crypto.FILETYPE_PEM, le_key.pem)
         cert = acme_crypto_util.gen_ss_cert(key, domains=[socket.gethostname()])
diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md
index 25eaff6f2e2..387373fff07 100644
--- a/certbot/CHANGELOG.md
+++ b/certbot/CHANGELOG.md
@@ -10,7 +10,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* Certbot will no longer save previous CSRs and certificate private keys to `/etc/letsencrypt/csr` and `/etc/letsencrypt/keys`, respectively. These directories may be safely deleted.
+* Certbot will now only keep the current and 5 previous certificates in the `/etc/letsencrypt/archive` directory for each certificate lineage. Any prior certificates will be automatically deleted upon renewal. This number may be further lowered in future releases.
+  * As always, users should only reference the certificate files within `/etc/letsencrypt/live` and never use `/etc/letsencrypt/archive` directly. See [Where are my certificates?](https://eff-certbot.readthedocs.io/en/stable/using.html#where-are-my-certificates) in the Certbot User Guide.
+* `certbot.configuration.NamespaceConfig.key_dir` and `.csr_dir` are now deprecated.
 
 ### Fixed
 
diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py
index 89c0e498a8f..1fe1df5ec69 100644
--- a/certbot/certbot/_internal/client.py
+++ b/certbot/certbot/_internal/client.py
@@ -416,13 +416,13 @@ def obtain_certificate(self, domains: List[str], old_keypath: Optional[str] = No
         else:
             key = key or crypto_util.generate_key(
                 key_size=key_size,
-                key_dir=self.config.key_dir,
+                key_dir=None,
                 key_type=self.config.key_type,
                 elliptic_curve=elliptic_curve,
                 strict_permissions=self.config.strict_permissions,
             )
-            csr = crypto_util.generate_csr(key, domains, self.config.csr_dir,
-                                           self.config.must_staple, self.config.strict_permissions)
+            csr = crypto_util.generate_csr(
+                key, domains, None, self.config.must_staple, self.config.strict_permissions)
 
         try:
             orderr = self._get_order_and_authorizations(csr.data, self.config.allow_subset_of_names)
@@ -433,7 +433,7 @@ def obtain_certificate(self, domains: List[str], old_keypath: Optional[str] = No
             if self.config.allow_subset_of_names:
                 successful_domains = self._successful_domains_from_error(error, domains)
                 if successful_domains != domains and len(successful_domains) != 0:
-                    return self._retry_obtain_certificate(key, csr, domains, successful_domains)
+                    return self._retry_obtain_certificate(domains, successful_domains)
             raise
         authzr = orderr.authorizations
         auth_domains = {a.body.identifier.value for a in authzr}
@@ -445,7 +445,7 @@ def obtain_certificate(self, domains: List[str], old_keypath: Optional[str] = No
         # domains contains a wildcard because the ACME spec forbids identifiers
         # in authzs from containing a wildcard character.
         if self.config.allow_subset_of_names and successful_domains != domains:
-            return self._retry_obtain_certificate(key, csr, domains, successful_domains)
+            return self._retry_obtain_certificate(domains, successful_domains)
         else:
             try:
                 cert, chain = self.obtain_certificate_from_csr(csr, orderr)
@@ -457,7 +457,7 @@ def obtain_certificate(self, domains: List[str], old_keypath: Optional[str] = No
                 if self.config.allow_subset_of_names:
                     successful_domains = self._successful_domains_from_error(error, domains)
                     if successful_domains != domains and len(successful_domains) != 0:
-                        return self._retry_obtain_certificate(key, csr, domains, successful_domains)
+                        return self._retry_obtain_certificate(domains, successful_domains)
                 raise
 
     def _get_order_and_authorizations(self, csr_pem: bytes,
@@ -540,16 +540,12 @@ def _successful_domains_from_error(self, error: messages.Error, domains: List[st
             return successful_domains
         return []
 
-    def _retry_obtain_certificate(self, key: util.Key,
-                                csr: util.CSR, domains: List[str], successful_domains: List[str]
+    def _retry_obtain_certificate(self, domains: List[str], successful_domains: List[str]
                                 ) -> Tuple[bytes, bytes, util.Key, util.CSR]:
         failed_domains = [d for d in domains if d not in successful_domains]
         domains_list = ", ".join(failed_domains)
         display_util.notify("Unable to obtain a certificate with every requested "
             f"domain. Retrying without: {domains_list}")
-        if not self.config.dry_run:
-            os.remove(key.file)
-            os.remove(csr.file)
         return self.obtain_certificate(successful_domains)
 
     def _choose_lineagename(self, domains: List[str], certname: Optional[str]) -> str:
diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py
index df5168ea240..ea5e0435f34 100644
--- a/certbot/certbot/_internal/renewal.py
+++ b/certbot/certbot/_internal/renewal.py
@@ -392,6 +392,7 @@ def renew_cert(config: configuration.NamespaceConfig, domains: Optional[List[str
         # TODO: Check return value of save_successor
         lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, config)
         lineage.update_all_links_to(lineage.latest_common_version())
+        lineage.truncate()
 
     hooks.renew_hook(config, domains, lineage.live_dir)
 
diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py
index 978295ccec5..e0fe7926944 100644
--- a/certbot/certbot/_internal/storage.py
+++ b/certbot/certbot/_internal/storage.py
@@ -1,4 +1,5 @@
 """Renewable certificates storage."""
+# pylint: disable=too-many-lines
 import datetime
 import glob
 import logging
@@ -1243,3 +1244,32 @@ def save_successor(self, prior_version: int, new_cert: bytes, new_privkey: bytes
         self.configuration = config_with_defaults(self.configfile)
 
         return target_version
+
+    def truncate(self, num_prior_certs_to_keep: int = 5) -> None:
+        """Delete unused historical certificate, chain and key items from the lineage.
+
+        A certificate version will be deleted if it is:
+          1. not the current target, and
+          2. not a previous version within num_prior_certs_to_keep.
+
+        :param num_prior_certs_to_keep: How many prior certificate versions to keep.
+
+        """
+        # Do not want to delete the current target or current-Nth certificate.
+        current_version = self.latest_common_version()
+        versions_to_delete = set(self.available_versions("cert"))
+        versions_to_delete -= set(range(current_version,
+                                        current_version - 1 - num_prior_certs_to_keep, -1))
+        archive = self.archive_dir
+
+        # Delete the remaining lineage items kinds for those certificate versions.
+        for ver in versions_to_delete:
+            logger.debug("Deleting %s/cert%d.pem and related items during clean up",
+                         archive, ver)
+            for kind in ALL_FOUR:
+                item_path = os.path.join(archive, f"{kind}{ver}.pem")
+                try:
+                    if os.path.exists(item_path):
+                        os.unlink(item_path)
+                except OSError:
+                    logger.debug("Failed to clean up %s", item_path, exc_info=True)
diff --git a/certbot/certbot/configuration.py b/certbot/certbot/configuration.py
index dd40a096fb5..9e0d3f9deeb 100644
--- a/certbot/certbot/configuration.py
+++ b/certbot/certbot/configuration.py
@@ -5,6 +5,7 @@
 from typing import List
 from typing import Optional
 from urllib import parse
+import warnings
 
 from certbot import errors
 from certbot import util
@@ -150,6 +151,8 @@ def backup_dir(self) -> str:
     @property
     def csr_dir(self) -> str:
         """Directory where new Certificate Signing Requests (CSRs) are saved."""
+        warnings.warn("NamespaceConfig.csr_dir is deprecated and will be removed in an upcoming "
+                      "release of Certbot", DeprecationWarning)
         return os.path.join(self.namespace.config_dir, constants.CSR_DIR)
 
     @property
@@ -160,6 +163,8 @@ def in_progress_dir(self) -> str:
     @property
     def key_dir(self) -> str:
         """Keys storage."""
+        warnings.warn("NamespaceConfig.key_dir is deprecated and will be removed in an upcoming "
+                      "release of Certbot", DeprecationWarning)
         return os.path.join(self.namespace.config_dir, constants.KEY_DIR)
 
     @property
diff --git a/certbot/certbot/crypto_util.py b/certbot/certbot/crypto_util.py
index a9a8269fec5..4e95fd11bec 100644
--- a/certbot/certbot/crypto_util.py
+++ b/certbot/certbot/crypto_util.py
@@ -51,7 +51,7 @@
 
 # High level functions
 
-def generate_key(key_size: int, key_dir: str, key_type: str = "rsa",
+def generate_key(key_size: int, key_dir: Optional[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.
@@ -62,7 +62,7 @@ def generate_key(key_size: int, key_dir: str, key_type: str = "rsa",
         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_dir: Optional 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
@@ -85,27 +85,29 @@ def generate_key(key_size: int, key_dir: str, key_type: str = "rsa",
         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)
+    key_path = None
+    if key_dir:
+        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)
 
 
-def generate_csr(privkey: util.Key, names: Union[List[str], Set[str]], path: str,
+def generate_csr(privkey: util.Key, names: Union[List[str], Set[str]], path: Optional[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 str path: Optional 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.
@@ -117,13 +119,15 @@ def generate_csr(privkey: util.Key, names: Union[List[str], Set[str]], path: str
     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)
+    # Save CSR, if requested
+    csr_filename = None
+    if path:
+        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")
 
@@ -254,10 +258,10 @@ def make_key(bits: int = 1024, key_type: str = "rsa",
     return crypto.dump_privatekey(crypto.FILETYPE_PEM, key)
 
 
-def valid_privkey(privkey: str) -> bool:
+def valid_privkey(privkey: Union[str, bytes]) -> bool:
     """Is valid RSA private key?
 
-    :param str privkey: Private key file contents in PEM
+    :param privkey: Private key file contents in PEM
 
     :returns: Validity of private key.
     :rtype: bool
diff --git a/certbot/certbot/util.py b/certbot/certbot/util.py
index 12507ef3625..12427aa92db 100644
--- a/certbot/certbot/util.py
+++ b/certbot/certbot/util.py
@@ -1,7 +1,6 @@
 """Utilities for all Certbot."""
 import argparse
 import atexit
-import collections
 import errno
 import logging
 import platform
@@ -14,6 +13,7 @@
 from typing import Dict
 from typing import IO
 from typing import List
+from typing import NamedTuple
 from typing import Optional
 from typing import Set
 from typing import Tuple
@@ -35,9 +35,18 @@
 logger = logging.getLogger(__name__)
 
 
-Key = collections.namedtuple("Key", "file pem")
-# Note: form is the type of data, "pem" or "der"
-CSR = collections.namedtuple("CSR", "file data form")
+class Key(NamedTuple):
+    """Container for an optional file path and contents for a PEM-formated private key."""
+    file: Optional[str]
+    pem: bytes
+
+
+class CSR(NamedTuple):
+    """Container for an optional file path and contents for a PEM or DER-formatted CSR."""
+    file: Optional[str]
+    data: bytes
+    # Note: form is the type of data, "pem" or "der"
+    form: str
 
 
 # ANSI SGR escape codes
diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst
index add692e73be..359119f0ce9 100644
--- a/certbot/docs/using.rst
+++ b/certbot/docs/using.rst
@@ -879,10 +879,6 @@ private key file, you will also need to use ``chgrp`` and ``chmod
 0640`` to allow the server to read
 ``/etc/letsencrypt/live/$domain/privkey.pem``.
 
-.. note:: ``/etc/letsencrypt/archive`` and ``/etc/letsencrypt/keys``
-   contain all previous keys and certificates, while
-   ``/etc/letsencrypt/live`` symlinks to the latest versions.
-
 The following files are available:
 
 ``privkey.pem``
diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py
index 6b430831f89..cb16890d3c9 100644
--- a/certbot/tests/client_test.py
+++ b/certbot/tests/client_test.py
@@ -373,19 +373,18 @@ def test_obtain_certificate(self, mock_crypto_util):
 
         mock_crypto_util.generate_key.assert_called_once_with(
             key_size=self.config.rsa_key_size,
-            key_dir=self.config.key_dir,
+            key_dir=None,
             key_type=self.config.key_type,
             elliptic_curve="secp256r1",
             strict_permissions=True,
         )
         mock_crypto_util.generate_csr.assert_called_once_with(
-            mock.sentinel.key, self.eg_domains, self.config.csr_dir, False, True)
+            mock.sentinel.key, self.eg_domains, None, False, True)
         mock_crypto_util.cert_and_chain_from_fullchain.assert_called_once_with(
             self.eg_order.fullchain_pem)
 
     @mock.patch("certbot._internal.client.crypto_util")
-    @mock.patch("certbot.compat.os.remove")
-    def test_obtain_certificate_partial_success(self, mock_remove, mock_crypto_util):
+    def test_obtain_certificate_partial_success(self, 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.generate_csr.return_value = csr
@@ -398,12 +397,10 @@ def test_obtain_certificate_partial_success(self, mock_remove, mock_crypto_util)
 
         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)
 
     @mock.patch("certbot._internal.client.crypto_util")
-    @mock.patch("certbot.compat.os.remove")
-    def test_obtain_certificate_finalize_order_partial_success(self, mock_remove, mock_crypto_util):
+    def test_obtain_certificate_finalize_order_partial_success(self, mock_crypto_util):
         from acme import messages
         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)
@@ -435,9 +432,8 @@ def test_obtain_certificate_finalize_order_partial_success(self, mock_remove, mo
         successful_domains = [d for d in self.eg_domains if d != 'example.com']
         self.assertEqual(mock_crypto_util.generate_key.call_count, 2)
         mock_crypto_util.generate_csr.assert_has_calls([
-            mock.call(key, self.eg_domains, self.config.csr_dir, self.config.must_staple, self.config.strict_permissions),
-            mock.call(key, successful_domains, self.config.csr_dir, self.config.must_staple, self.config.strict_permissions)])
-        self.assertEqual(mock_remove.call_count, 2)
+            mock.call(key, self.eg_domains, None, self.config.must_staple, self.config.strict_permissions),
+            mock.call(key, successful_domains, None, self.config.must_staple, self.config.strict_permissions)])
         self.assertEqual(mock_crypto_util.cert_and_chain_from_fullchain.call_count, 1)
 
     @mock.patch("certbot._internal.client.crypto_util")
@@ -496,8 +492,7 @@ def test_obtain_certificate_finalize_order_rejected_identifier_no_subproblems(se
         self.assertEqual(mock_crypto_util.cert_and_chain_from_fullchain.call_count, 0)
 
     @mock.patch("certbot._internal.client.crypto_util")
-    @mock.patch("certbot.compat.os.remove")
-    def test_obtain_certificate_get_order_partial_success(self, mock_remove, mock_crypto_util):
+    def test_obtain_certificate_get_order_partial_success(self, mock_crypto_util):
         from acme import messages
         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)
@@ -529,9 +524,8 @@ def test_obtain_certificate_get_order_partial_success(self, mock_remove, mock_cr
         successful_domains = [d for d in self.eg_domains if d != 'example.com']
         self.assertEqual(mock_crypto_util.generate_key.call_count, 2)
         mock_crypto_util.generate_csr.assert_has_calls([
-            mock.call(key, self.eg_domains, self.config.csr_dir, self.config.must_staple, self.config.strict_permissions),
-            mock.call(key, successful_domains, self.config.csr_dir, self.config.must_staple, self.config.strict_permissions)])
-        self.assertEqual(mock_remove.call_count, 2)
+            mock.call(key, self.eg_domains, None, self.config.must_staple, self.config.strict_permissions),
+            mock.call(key, successful_domains, None, self.config.must_staple, self.config.strict_permissions)])
         self.assertEqual(mock_crypto_util.cert_and_chain_from_fullchain.call_count, 1)
 
     @mock.patch("certbot._internal.client.crypto_util")
diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py
index 3a1f2b7b421..13c2be0a516 100644
--- a/certbot/tests/storage_test.py
+++ b/certbot/tests/storage_test.py
@@ -839,6 +839,23 @@ def test_update_symlinks(self):
         storage.RenewableCert(self.config_file.filename, self.config,
             update_symlinks=True)
 
+    def test_truncate(self):
+        # It should not do anything when there's less than 5 cert history
+        for kind in ALL_FOUR:
+            self._write_out_kind(kind, 1)
+        with mock.patch('certbot.compat.os.unlink') as mock_unlink:
+            self.test_rc.truncate()
+            mock_unlink.assert_not_called()
+
+        # It should truncate the excess when there's more than 5 cert history
+        for kind in ALL_FOUR:
+            for i in range(2, 8):
+                self._write_out_kind(kind, i)
+        with mock.patch('certbot.compat.os.unlink') as mock_unlink:
+            self.test_rc.truncate()
+            self.assertEqual(mock_unlink.call_count, 1 * len(ALL_FOUR))
+            self.assertIn("1.pem", mock_unlink.call_args_list[0][0][0])
+
 class DeleteFilesTest(BaseRenewableCertTest):
     """Tests for certbot._internal.storage.delete_files"""
     def setUp(self):
diff --git a/pytest.ini b/pytest.ini
index e498766ec2d..596cf174d92 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -15,7 +15,9 @@
 #    certbot-dns-rfc2136.
 # 2) pytest-cov uses deprecated functionality in pytest-xdist, to be resolved by
 #    https://github.com/pytest-dev/pytest-cov/issues/557.
+# 3) csr_dir and key_dir are deprecated and should be removed in Certbot 3.0.
 filterwarnings =
     error
     ignore:decodestring\(\) is a deprecated alias:DeprecationWarning:dns
     ignore:.*rsyncdir:DeprecationWarning
+    ignore:.*_dir is deprecated and will be removed:DeprecationWarning


Code review comments:
diff_hunk: @@ -15,7 +15,9 @@
 #    certbot-dns-rfc2136.
 # 2) pytest-cov uses deprecated functionality in pytest-xdist, to be resolved by
 #    https://github.com/pytest-dev/pytest-cov/issues/557.
+# 3) csr_dir and key_dir are deprecated and should be removed in Certbot 3.0.
 filterwarnings =
     error
     ignore:decodestring\(\) is a deprecated alias:DeprecationWarning:dns
     ignore:.*rsyncdir:DeprecationWarning
+    ignore:.*_dir is deprecated and will be removed:DeprecationWarning
path: pytest.ini
text: @user1:
It's likely not even used in the plugins, but in the spirit of the comment under the `[pytest]` line above, can we easily change things so we don't raise this warning? We could [catch the warnings](https://docs.python.org/3/library/warnings.html#warnings.catch_warnings) in the relevant tests or possibly even just delete them.