diff options
author | Mads Jensen <mje@inducks.org> | 2022-02-04 04:34:04 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-04 04:34:04 +0300 |
commit | fe0c0dc3ae6c25c6087e51717a223f38a9b23d2f (patch) | |
tree | 21b0a16c784ed1a4503e81a5224634c432455739 | |
parent | 5b17a18355222c511b3d5e4442f01520765e8b8f (diff) |
Add support for revoking ecdsa keys without --cert-name. (#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.
-rw-r--r-- | acme/acme/client.py | 2 | ||||
-rw-r--r-- | certbot-ci/certbot_integration_tests/certbot_tests/test_main.py | 55 | ||||
-rw-r--r-- | certbot/CHANGELOG.md | 2 | ||||
-rw-r--r-- | certbot/certbot/_internal/client.py | 22 | ||||
-rw-r--r-- | certbot/tests/main_test.py | 2 |
5 files changed, 79 insertions, 4 deletions
diff --git a/acme/acme/client.py b/acme/acme/client.py index 7e87e7474..aacbbc263 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -1076,7 +1076,7 @@ class ClientNetwork: logger.debug('JWS payload:\n%s', jobj) kwargs = { "alg": self.alg, - "nonce": nonce + "nonce": nonce, } if acme_version == 2: kwargs["url"] = url 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 d04fbff6c..146ba58bb 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/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') diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 685466685..17ed2a635 100644 --- a/certbot/CHANGELOG.md +++ b/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. diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index ad5d99b61..bc9cf6937 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -16,6 +16,10 @@ from cryptography.hazmat.backends import default_backend 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(): diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 18c8b8081..3813e4d50 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -735,7 +735,7 @@ class MainTest(test_util.ConfigTestCase): 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') |