From 34694251dd48d5e5d8ee30f0f3a52f7a1893020e Mon Sep 17 00:00:00 2001
From: Adrien Ferrand <adferrand@users.noreply.github.com>
Date: Mon, 5 Oct 2020 20:50:45 +0200
Subject: [PATCH] Reuse key renewal params (#8343)

* Ensure key params are stored in renewal config when --reuse-key is set.

* Fix mypy definition

* Add unit test

* Clean code.
---
 certbot/certbot/_internal/renewal.py | 28 ++++++++++++++++++++++++----
 certbot/tests/renewal_test.py        | 22 +++++++++++++++++++++-
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/certbot/certbot/_internal/renewal.py b/certbot/certbot/_internal/renewal.py
index c8402b018..50e4d3a4c 100644
--- a/certbot/certbot/_internal/renewal.py
+++ b/certbot/certbot/_internal/renewal.py
@@ -9,16 +9,21 @@ import sys
 import time
 import traceback
 
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives.asymmetric import rsa
+from cryptography.hazmat.primitives.serialization import load_pem_private_key
 import OpenSSL
 import six
 import zope.component
 
 from acme.magic_typing import List
+from acme.magic_typing import Optional  # pylint: disable=unused-import
 from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
 from certbot import util
 from certbot._internal import cli
+from certbot._internal import client  # pylint: disable=unused-import
 from certbot._internal import constants
 from certbot._internal import hooks
 from certbot._internal import storage
@@ -308,7 +313,8 @@ def _avoid_invalidating_lineage(config, lineage, original_server):
 
 
 def renew_cert(config, domains, le_client, lineage):
-    "Renew a certificate lineage."
+    # type: (interfaces.IConfig, Optional[List[str]], client.Client, storage.RenewableCert) -> None
+    """Renew a certificate lineage."""
     renewal_params = lineage.configuration["renewalparams"]
     original_server = renewal_params.get("server", cli.flag_default("server"))
     _avoid_invalidating_lineage(config, lineage, original_server)
@@ -316,11 +322,14 @@ def renew_cert(config, domains, le_client, lineage):
         domains = lineage.names()
     # 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 = os.path.normpath(lineage.privkey) if config.reuse_key else None
+    if config.reuse_key:
+        new_key = os.path.normpath(lineage.privkey)
+        _update_renewal_params_from_key(new_key, config)
+    else:
+        new_key = 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))
+        logger.debug("Dry run: skipping updating lineage at %s", os.path.dirname(lineage.cert))
     else:
         prior_version = lineage.latest_common_version()
         # TODO: Check return value of save_successor
@@ -335,6 +344,7 @@ def report(msgs, category):
     lines = ("%s (%s)" % (m, category) for m in msgs)
     return "  " + "\n  ".join(lines)
 
+
 def _renew_describe_results(config, renew_successes, renew_failures,
                             renew_skipped, parse_failures):
 
@@ -489,3 +499,13 @@ def handle_renewal_request(config):
     # Windows installer integration tests rely on handle_renewal_request behavior here.
     # If the text below changes, these tests will need to be updated accordingly.
     logger.debug("no renewal failures")
+
+
+def _update_renewal_params_from_key(key_path, config):
+    # type: (str, interfaces.IConfig) -> None
+    with open(key_path, 'rb') as file_h:
+        key = load_pem_private_key(file_h.read(), password=None, backend=default_backend())
+    if isinstance(key, rsa.RSAPrivateKey):
+        config.rsa_key_size = key.key_size
+    else:
+        raise errors.Error('Key at {0} is of an unsupported type: {1}.'.format(key_path, type(key)))
diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py
index 83796cd4f..0957b5c31 100644
--- a/certbot/tests/renewal_test.py
+++ b/certbot/tests/renewal_test.py
@@ -3,7 +3,7 @@ import unittest
 
 try:
     import mock
-except ImportError: # pragma: no cover
+except ImportError:  # pragma: no cover
     from unittest import mock
 
 from acme import challenges
@@ -54,6 +54,26 @@ class RenewalTest(test_util.ConfigTestCase):
         self.assertEqual(self.config.webroot_map, {})
         self.assertEqual(self.config.webroot_path, ['/var/www/test'])
 
+    def test_reuse_key_renewal_params(self):
+        self.config.rsa_key_size = 'INVALID_VALUE'
+        self.config.reuse_key = True
+        self.config.dry_run = True
+        config = configuration.NamespaceConfig(self.config)
+
+        rc_path = test_util.make_lineage(
+            self.config.config_dir, 'sample-renewal.conf')
+        lineage = storage.RenewableCert(rc_path, config)
+
+        le_client = mock.MagicMock()
+        le_client.obtain_certificate.return_value = (None, None, None, None)
+
+        from certbot._internal import renewal
+
+        with mock.patch('certbot._internal.renewal.hooks.renew_hook'):
+            renewal.renew_cert(self.config, None, le_client, lineage)
+
+        assert self.config.rsa_key_size == 2048
+
 
 class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase):
     """Tests for certbot._internal.renewal.restore_required_config_elements."""