Add support for revoking ecdsa keys without --cert-name. (certbot#8725)

* Add support for revoking ecdsa keys without --cert-name.

Co-Authored-By: commonism <commonism@users.noreply.github.com>

* Move alg to acme_client.ClientNetwork instantiating in acme_from_config_key

* Fix argument for RS256/ES256

* Support also ES384 and ES512 signing algorithms.

acme/acme/client.py

@@ -1076,7 +1076,7 @@ def _wrap_in_jws(self, obj: jose.JSONDeSerializable, nonce: str, url: str,
         logger.debug('JWS payload:\n%s', jobj)
         kwargs = {
             "alg": self.alg,
-            "nonce": nonce
+            "nonce": nonce,
         }
         if acme_version == 2:
             kwargs["url"] = url

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -643,6 +643,61 @@ def test_revoke_and_unregister(context: IntegrationTestsContext) -> None:
     assert cert3 in stdout
 
 
+@pytest.mark.parametrize('curve,curve_cls,skip_servers', [
+    ('secp256r1', SECP256R1, []),
+    ('secp384r1', SECP384R1, []),
+    ('secp521r1', SECP521R1, ['boulder-v2'])]
+)
+def test_revoke_ecdsa_cert_key(
+    context: IntegrationTestsContext, curve: str, curve_cls: Type[EllipticCurve],
+    skip_servers: Iterable[str]) -> None:
+    """Test revoking a certificate """
+    if context.acme_server in skip_servers:
+        pytest.skip(f'ACME server {context.acme_server} does not support ECDSA curve {curve}')
+    cert: str = context.get_domain('curve')
+    context.certbot([
+        'certonly',
+        '--key-type', 'ecdsa', '--elliptic-curve', curve,
+        '-d', cert,
+    ])
+    key = join(context.config_dir, "live", cert, 'privkey.pem')
+    cert_path = join(context.config_dir, "live", cert, 'cert.pem')
+    assert_elliptic_key(key, curve_cls)
+    context.certbot([
+        'revoke', '--cert-path', cert_path, '--key-path', key,
+        '--no-delete-after-revoke',
+    ])
+    stdout, _ = context.certbot(['certificates'])
+    assert stdout.count('INVALID: REVOKED') == 1, 'Expected {0} to be REVOKED'.format(cert)
+
+
+@pytest.mark.parametrize('curve,curve_cls,skip_servers', [
+    ('secp256r1', SECP256R1, []),
+    ('secp384r1', SECP384R1, []),
+    ('secp521r1', SECP521R1, ['boulder-v2'])]
+)
+def test_revoke_ecdsa_cert_key_delete(
+    context: IntegrationTestsContext, curve: str, curve_cls: Type[EllipticCurve],
+    skip_servers: Iterable[str]) -> None:
+    """Test revoke and deletion for each supported curve type"""
+    if context.acme_server in skip_servers:
+        pytest.skip(f'ACME server {context.acme_server} does not support ECDSA curve {curve}')
+    cert: str = context.get_domain('curve')
+    context.certbot([
+        'certonly',
+        '--key-type', 'ecdsa', '--elliptic-curve', curve,
+        '-d', cert,
+    ])
+    key = join(context.config_dir, "live", cert, 'privkey.pem')
+    cert_path = join(context.config_dir, "live", cert, 'cert.pem')
+    assert_elliptic_key(key, curve_cls)
+    context.certbot([
+        'revoke', '--cert-path', cert_path, '--key-path', key,
+        '--delete-after-revoke',
+    ])
+    assert not exists(cert_path)
+
+
 def test_revoke_mutual_exclusive_flags(context: IntegrationTestsContext) -> None:
     """Test --cert-path and --cert-name cannot be used during revoke."""
     cert = context.get_domain('le1')

certbot/CHANGELOG.md

@@ -20,6 +20,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 * GCP Permission list for certbot-dns-google in plugin documentation
 * dns-digitalocean used the SOA TTL for newly created records, rather than 30 seconds.
+* Revoking a certificate based on an ECDSA key can now be done with `--key-path`.
+  See [GH #8569](https://github.com/certbot/certbot/issues/8569).
 
 More details about these changes can be found on our GitHub repo.
 

certbot/certbot/_internal/client.py

@@ -16,6 +16,10 @@
 from cryptography.hazmat.primitives.asymmetric.rsa import generate_private_key
 import josepy as jose
 import OpenSSL
+from josepy import ES256
+from josepy import ES384
+from josepy import ES512
+from josepy import RS256
 
 from acme import client as acme_client
 from acme import crypto_util as acme_crypto_util
@@ -48,8 +52,22 @@ def acme_from_config_key(config: configuration.NamespaceConfig, key: jose.JWK,
                          regr: Optional[messages.RegistrationResource] = None
                          ) -> acme_client.ClientV2:
     """Wrangle ACME client construction"""
-    # TODO: Allow for other alg types besides RS256
-    net = acme_client.ClientNetwork(key, account=regr, verify_ssl=(not config.no_verify_ssl),
+    if key.typ == 'EC':
+        public_key = key.key
+        if public_key.key_size == 256:
+            alg = ES256
+        elif public_key.key_size == 384:
+            alg = ES384
+        elif public_key.key_size == 521:
+            alg = ES512
+        else:
+            raise errors.NotSupportedError(
+                "No matching signing algorithm can be found for the key"
+            )
+    else:
+        alg = RS256
+    net = acme_client.ClientNetwork(key, alg=alg, account=regr,
+                                    verify_ssl=(not config.no_verify_ssl),
                                     user_agent=determine_user_agent(config))
 
     with warnings.catch_warnings():

certbot/tests/main_test.py

@@ -735,7 +735,7 @@ def test_user_agent(self, gsc, _obt, det, _, __, ___):
             args += ["--user-agent", ua]
             self._call_no_clientmock(args)
             acme_net.assert_called_once_with(mock.ANY, account=mock.ANY, verify_ssl=True,
-                user_agent=ua)
+                user_agent=ua, alg=jose.RS256)
 
     @mock.patch('certbot._internal.main.plug_sel.record_chosen_plugins')
     @mock.patch('certbot._internal.main.plug_sel.pick_installer')