Add --reuse-key feature (#5901)

* Initial work on new version of --reuse-key

* Test for reuse_key

* Make lint happier

* Also test a non-dry-run reuse_key renewal

* Test --reuse-key in boulder integration test

* Better reuse-key integration testing

* Log fact that key was reused

* Test that the certificates themselves are different

* Change "oldkeypath" to "old_keypath"

* Simply appearance of new-key generation logic

* Reorganize new-key logic

* Move awk logic into TotalAndDistinctLines function

* After refactor, there's now explicit None rather than missing param

* Indicate for MyPy that key can be None

* Actually import the Optional type

* magic_typing is too magical for pylint

* Remove --no-reuse-key option

* Correct pylint test disable

certbot/cli.py

@@ -1017,6 +1017,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
              "certificate already exists for the requested certificate name "
              "but does not match the requested domains, renew it now, "
              "regardless of whether it is near expiry.")
+    helpful.add(
+        "automation", "--reuse-key", dest="reuse_key",
+        action="store_true", default=flag_default("reuse_key"),
+        help="When renewing, use the same private key as the existing "
+             "certificate.")
+
     helpful.add(
         ["automation", "renew", "certonly"],
         "--allow-subset-of-names", action="store_true",

certbot/client.py

@@ -4,6 +4,7 @@ import logging
 import os
 import platform
 
+
 from cryptography.hazmat.backends import default_backend
 # https://github.com/python/typeshed/blob/master/third_party/
 # 2/cryptography/hazmat/primitives/asymmetric/rsa.pyi
@@ -16,6 +17,7 @@ from acme import client as acme_client
 from acme import crypto_util as acme_crypto_util
 from acme import errors as acme_errors
 from acme import messages
+from acme.magic_typing import Optional  # pylint: disable=unused-import,no-name-in-module
 
 import certbot
 
@@ -273,7 +275,7 @@ class Client(object):
         cert, chain = crypto_util.cert_and_chain_from_fullchain(orderr.fullchain_pem)
         return cert.encode(), chain.encode()
 
-    def obtain_certificate(self, domains):
+    def obtain_certificate(self, domains, old_keypath=None):
         """Obtains a certificate from the ACME server.
 
         `.register` must be called before `.obtain_certificate`
@@ -286,16 +288,36 @@ class Client(object):
         :rtype: tuple
 
         """
+
+        # We need to determine the key path, key PEM data, CSR path,
+        # and CSR PEM data.  For a dry run, the paths are None because
+        # they aren't permanently saved to disk.  For a lineage with
+        # --reuse-key, the key path and PEM data are derived from an
+        # existing file.
+
+        if old_keypath is not None:
+            # We've been asked to reuse a specific existing private key.
+            # Therefore, we'll read it now and not generate a new one in
+            # either case below.
+            with open(old_keypath, "r") as f:
+                keypath = old_keypath
+                keypem = f.read()
+            key = util.Key(file=keypath, pem=keypem) # type: Optional[util.Key]
+            logger.info("Reusing existing private key from %s.", old_keypath)
+        else:
+            # The key is set to None here but will be created below.
+            key = None
+
         # Create CSR from names
         if self.config.dry_run:
-            key = util.Key(file=None,
-                           pem=crypto_util.make_key(self.config.rsa_key_size))
+            key = key or util.Key(file=None,
+                                  pem=crypto_util.make_key(self.config.rsa_key_size))
             csr = util.CSR(file=None, form="pem",
                            data=acme_crypto_util.make_csr(
                                key.pem, domains, self.config.must_staple))
         else:
-            key = crypto_util.init_save_key(
-                self.config.rsa_key_size, self.config.key_dir)
+            key = key or crypto_util.init_save_key(self.config.rsa_key_size,
+                                                   self.config.key_dir)
             csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir)
 
         orderr = self._get_order_and_authorizations(csr.data, self.config.allow_subset_of_names)

certbot/constants.py

@@ -64,6 +64,7 @@ CLI_DEFAULTS = dict(
     pref_challs=[],
     validate_hooks=True,
     directory_hooks=True,
+    reuse_key=False,
     disable_renew_updates=False,
 
     # Subparsers

certbot/renewal.py

@@ -36,7 +36,7 @@ STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "pre_hook", "post_hook", "tls_sni_01_address",
                     "http01_address"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
-BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names"]
+BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names", "reuse_key"]
 
 CONFIG_ITEMS = set(itertools.chain(
     BOOL_CONFIG_ITEMS, INT_CONFIG_ITEMS, STR_CONFIG_ITEMS, ('pref_challs',)))
@@ -298,7 +298,10 @@ def renew_cert(config, domains, le_client, lineage):
     _avoid_invalidating_lineage(config, lineage, original_server)
     if not domains:
         domains = lineage.names()
-    new_cert, new_chain, new_key, _ = le_client.obtain_certificate(domains)
+    # The private key is the existing lineage private key if reuse_key is set.
+    # Otherwise, generate a fresh private key by passing None.
+    new_key = lineage.privkey if config.reuse_key else None
+    new_cert, new_chain, new_key, _ = le_client.obtain_certificate(domains, new_key)
     if config.dry_run:
         logger.debug("Dry run: skipping updating lineage at %s",
                     os.path.dirname(lineage.cert))

certbot/tests/main_test.py

@@ -1026,8 +1026,9 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
 
     def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None,
                              args=None, should_renew=True, error_expected=False,
-                                 quiet_mode=False, expiry_date=datetime.datetime.now()):
-        # pylint: disable=too-many-locals,too-many-arguments
+                             quiet_mode=False, expiry_date=datetime.datetime.now(),
+                             reuse_key=False):
+        # pylint: disable=too-many-locals,too-many-arguments,too-many-branches
         cert_path = test_util.vector_path('cert_512.pem')
         chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem'
         mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path,
@@ -1077,7 +1078,13 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                                             traceback.format_exc())
 
             if should_renew:
-                mock_client.obtain_certificate.assert_called_once_with(['isnot.org'])
+                if reuse_key:
+                    # The location of the previous live privkey.pem is passed
+                    # to obtain_certificate
+                    mock_client.obtain_certificate.assert_called_once_with(['isnot.org'],
+                        os.path.join(self.config.config_dir, "live/sample-renewal/privkey.pem"))
+                else:
+                    mock_client.obtain_certificate.assert_called_once_with(['isnot.org'], None)
             else:
                 self.assertEqual(mock_client.obtain_certificate.call_count, 0)
         except:
@@ -1127,6 +1134,17 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
         args = ["renew", "--dry-run", "-tvv"]
         self._test_renewal_common(True, [], args=args, should_renew=True)
 
+    def test_reuse_key(self):
+        test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
+        args = ["renew", "--dry-run", "--reuse-key"]
+        self._test_renewal_common(True, [], args=args, should_renew=True, reuse_key=True)
+
+    @mock.patch('certbot.storage.RenewableCert.save_successor')
+    def test_reuse_key_no_dry_run(self, unused_save_successor):
+        test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
+        args = ["renew", "--reuse-key"]
+        self._test_renewal_common(True, [], args=args, should_renew=True, reuse_key=True)
+
     @mock.patch('certbot.renewal.should_renew')
     def test_renew_skips_recent_certs(self, should_renew):
         should_renew.return_value = False

tests/boulder-integration.sh

@@ -166,6 +166,14 @@ CheckRenewHook() {
     CheckSavedRenewHook $1
 }
 
+# Return success only if input contains exactly $1 lines of text, of
+# which $2 different values occur in the first field.
+TotalAndDistinctLines() {
+    total=$1
+    distinct=$2
+    awk '{a[$1] = 1}; END {exit(NR !='$total' || length(a) !='$distinct')}'
+}
+
 # Cleanup coverage data
 coverage erase
 
@@ -347,6 +355,26 @@ if common certificates | grep "fail\.dns1\.le\.wtf"; then
     exit 1
 fi
 
+# reuse-key
+common --domains reusekey.le.wtf --reuse-key
+common renew --cert-name reusekey.le.wtf
+CheckCertCount "reusekey.le.wtf" 2
+ls -l "${root}/conf/archive/reusekey.le.wtf/privkey"*
+# The final awk command here exits successfully if its input consists of
+# exactly two lines with identical first fields, and unsuccessfully otherwise.
+sha256sum "${root}/conf/archive/reusekey.le.wtf/privkey"* | TotalAndDistinctLines 2 1
+
+# don't reuse key (just by forcing reissuance without --reuse-key)
+common --cert-name reusekey.le.wtf --domains reusekey.le.wtf --force-renewal
+CheckCertCount "reusekey.le.wtf" 3
+ls -l "${root}/conf/archive/reusekey.le.wtf/privkey"*
+# Exactly three lines, of which exactly two identical first fields.
+sha256sum "${root}/conf/archive/reusekey.le.wtf/privkey"* | TotalAndDistinctLines 3 2
+
+# Nonetheless, all three certificates are different even though two of them
+# share the same subject key.
+sha256sum "${root}/conf/archive/reusekey.le.wtf/cert"* | TotalAndDistinctLines 3 3
+
 # ECDSA
 openssl ecparam -genkey -name secp384r1 -out "${root}/privkey-p384.pem"
 SAN="DNS:ecdsa.le.wtf" openssl req -new -sha256 \