Add --new-key (#9252)

* add --new-key

* add tests

certbot-ci/certbot_integration_tests/certbot_tests/assertions.py

@@ -37,16 +37,19 @@ def assert_elliptic_key(key: str, curve: Type[EllipticCurve]) -> None:
     assert isinstance(key.curve, curve)
 
 
-def assert_rsa_key(key: str) -> None:
+def assert_rsa_key(key: str, key_size: Optional[int] = None) -> None:
     """
     Asserts that the key at the given path is an RSA key.
     :param str key: path to key
+    :param int key_size: if provided, assert that the RSA key is of this size
     """
     with open(key, 'rb') as file:
         privkey1 = file.read()
 
     key = load_pem_private_key(data=privkey1, password=None, backend=default_backend())
     assert isinstance(key, RSAPrivateKey)
+    if key_size:
+        assert key_size == key.key_size
 
 
 def assert_hook_execution(probe_path: str, probe_content: str) -> None:

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -8,6 +8,7 @@ import subprocess
 import time
 from typing import Iterable
 from typing import Generator
+from typing import Tuple
 from typing import Type
 
 from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurve
@@ -463,6 +464,42 @@ def test_reuse_key(context: IntegrationTestsContext) -> None:
     assert len({cert1, cert2, cert3}) == 3
 
 
+def test_new_key(context: IntegrationTestsContext) -> None:
+    """Tests --new-key and its interactions with --reuse-key"""
+    def private_key(generation: int) -> Tuple[str, str]:
+        pk_path = join(context.config_dir, f'archive/{certname}/privkey{generation}.pem')
+        with open(pk_path, 'r') as file:
+            return file.read(), pk_path
+
+    certname = context.get_domain('newkey')
+
+    context.certbot(['--domains', certname, '--reuse-key',
+                     '--key-type', 'rsa', '--rsa-key-size', '4096'])
+    privkey1, _ = private_key(1)
+
+    # renew: --new-key should replace the key, but keep reuse_key and the key type + params
+    context.certbot(['renew', '--cert-name', certname, '--new-key'])
+    privkey2, privkey2_path = private_key(2)
+    assert privkey1 != privkey2
+    assert_saved_lineage_option(context.config_dir, certname, 'reuse_key', 'True')
+    assert_rsa_key(privkey2_path, 4096)
+
+    # certonly: it should replace the key but the key size will change
+    context.certbot(['certonly', '-d', certname, '--reuse-key', '--new-key'])
+    privkey3, privkey3_path = private_key(3)
+    assert privkey2 != privkey3
+    assert_saved_lineage_option(context.config_dir, certname, 'reuse_key', 'True')
+    assert_rsa_key(privkey3_path, 2048)
+
+    # certonly: it should be possible to change the key type and keep reuse_key
+    context.certbot(['certonly', '-d', certname, '--reuse-key', '--new-key', '--key-type', 'ecdsa',
+                     '--cert-name', certname])
+    privkey4, privkey4_path = private_key(4)
+    assert privkey3 != privkey4
+    assert_saved_lineage_option(context.config_dir, certname, 'reuse_key', 'True')
+    assert_elliptic_key(privkey4_path, SECP256R1)
+
+
 def test_incorrect_key_type(context: IntegrationTestsContext) -> None:
     with pytest.raises(subprocess.CalledProcessError):
         context.certbot(['--key-type="failwhale"'])

certbot/CHANGELOG.md

@@ -6,7 +6,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-*
+* Added `--new-key`. When renewing or replacing a certificate that has `--reuse-key`
+  set, it will force a new private key to be generated.
+  Combining `--reuse-key` and `--new-key` will replace the certificate's private key
+  and then reuse it for future renewals.
 
 ### Changed
 

certbot/certbot/_internal/cli/__init__.py

@@ -223,6 +223,13 @@ def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[st
              "certificate. Not reusing private keys is the default behavior of "
              "Certbot. This option may be used to unset --reuse-key on an "
              "existing certificate.")
+    helpful.add(
+        "automation", "--new-key",
+        dest="new_key", action="store_true", default=flag_default("new_key"),
+        help="When renewing or replacing a certificate, generate a new private key, "
+             "even if --reuse-key is set on the existing certificate. Combining "
+             "--new-key and --reuse-key will result in the private key being replaced and "
+             "then reused in future renewals.")
 
     helpful.add(
         ["automation", "renew", "certonly"],

certbot/certbot/_internal/constants.py

@@ -74,6 +74,7 @@ CLI_DEFAULTS: Dict[str, Any] = dict(  # noqa
     validate_hooks=True,
     directory_hooks=True,
     reuse_key=False,
+    new_key=False,
     disable_renew_updates=False,
     random_sleep_on_renew=True,
     eab_hmac_key=None,

certbot/certbot/_internal/renewal.py

@@ -336,7 +336,7 @@ def renew_cert(config: configuration.NamespaceConfig, domains: Optional[List[str
         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.
-    if config.reuse_key:
+    if config.reuse_key and not config.new_key:
         new_key = os.path.normpath(lineage.privkey)
         _update_renewal_params_from_key(new_key, config)
     else:

certbot/certbot/configuration.py

@@ -300,6 +300,13 @@ class NamespaceConfig:
         """
         return self.namespace.issuance_timeout
 
+    @property
+    def new_key(self) -> bool:
+        """This option specifies whether Certbot should generate a new private
+        key when replacing a certificate, even if reuse_key is set.
+        """
+        return self.namespace.new_key
+
     # Magic methods
 
     def __deepcopy__(self, _memo: Any) -> 'NamespaceConfig':

certbot/tests/main_test.py

@@ -1155,7 +1155,7 @@ class MainTest(test_util.ConfigTestCase):
     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(),
-                             reuse_key=False):
+                             reuse_key=False, new_key=False):
         cert_path = test_util.vector_path('cert_512.pem')
         chain_path = os.path.normpath(os.path.join(self.config.config_dir,
                                                    'live/foo.bar/fullchain.pem'))
@@ -1205,7 +1205,7 @@ class MainTest(test_util.ConfigTestCase):
                                             traceback.format_exc())
 
             if should_renew:
-                if reuse_key:
+                if reuse_key and not new_key:
                     # The location of the previous live privkey.pem is passed
                     # to obtain_certificate
                     mock_client.obtain_certificate.assert_called_once_with(['isnot.org'],
@@ -1276,6 +1276,13 @@ class MainTest(test_util.ConfigTestCase):
         args = ["renew", "--reuse-key"]
         self._test_renewal_common(True, [], args=args, should_renew=True, reuse_key=True)
 
+    @mock.patch('certbot._internal.storage.RenewableCert.save_successor')
+    def test_new_key(self, unused_save_successor):
+        test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf')
+        args = ["renew", "--reuse-key", "--new-key"]
+        self._test_renewal_common(True, [], args=args, should_renew=True, reuse_key=True,
+                                  new_key=True)
+
     @mock.patch('sys.stdin')
     def test_noninteractive_renewal_delay(self, stdin):
         stdin.isatty.return_value = False

certbot/tests/renewal_test.py

@@ -99,6 +99,32 @@ class RenewalTest(test_util.ConfigTestCase):
 
         assert self.config.elliptic_curve == 'secp256r1'
 
+    def test_new_key(self):
+        # When renewing with both reuse_key and new_key, the key should be regenerated,
+        # the key type, key parameters and reuse_key should be kept.
+        self.config.reuse_key = True
+        self.config.new_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)
+
+        self.assertEqual(self.config.rsa_key_size, 2048)
+        self.assertEqual(self.config.key_type, 'rsa')
+        self.assertTrue(self.config.reuse_key)
+        # None is passed as the existing key, i.e. the key is not actually being reused.
+        le_client.obtain_certificate.assert_called_with(mock.ANY, None)
+
     @test_util.patch_display_util()
     @mock.patch('certbot._internal.renewal.cli.set_by_cli')
     def test_remove_deprecated_config_elements(self, mock_set_by_cli, unused_mock_get_utility):