From e7c539d3e99f667c27a779ac52482bcaa230c24d Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Mon, 2 Feb 2026 16:25:13 -0800 Subject: [PATCH] Deprecate functions using `acme.crypto_util.Format`to be able to soon remove OpenSSL (#10485) A few largely unused functions/types have been deprecated in our effort to remove our pyOpenSSL dependency: * Deprecated: `certbot.crypto_util.get_sans_from_cert` * Deprecated: `certbot.crypto_util.get_names_from_cert` * Deprecated: `certbot.crypto_util.get_names_from_req` * Deprecated: `certbot.crypto_util.import_csr_file` (and replaced by `certbot.crypto_util.read_csr_file`) * Deprecated: `acme.crypto_util.Format` `read_csr_file` now always returns a PEM formatted CSR, since that's what was happening in practice, and therefore lets us stop having to return a `Format`, so we will be able to stop importing it. first half of #10433 --------- Co-authored-by: Brad Warren --- .../acme/_internal/tests/crypto_util_test.py | 7 +- acme/src/acme/crypto_util.py | 37 +++++- certbot/src/certbot/_internal/cli/helpful.py | 4 +- certbot/src/certbot/_internal/main.py | 2 +- .../_internal/tests/crypto_util_test.py | 74 +++++++++--- certbot/src/certbot/crypto_util.py | 105 +++++++++++++----- newsfragments/10433.changed | 6 + 7 files changed, 183 insertions(+), 52 deletions(-) create mode 100644 newsfragments/10433.changed diff --git a/acme/src/acme/_internal/tests/crypto_util_test.py b/acme/src/acme/_internal/tests/crypto_util_test.py index 9d781ec4c..577c26ec5 100644 --- a/acme/src/acme/_internal/tests/crypto_util_test.py +++ b/acme/src/acme/_internal/tests/crypto_util_test.py @@ -16,9 +16,10 @@ from acme._internal.tests import test_util class FormatTest(unittest.TestCase): def test_to_cryptography_encoding(self): - from acme.crypto_util import Format - assert Format.DER.to_cryptography_encoding() == serialization.Encoding.DER - assert Format.PEM.to_cryptography_encoding() == serialization.Encoding.PEM + with pytest.warns(DeprecationWarning, match='Format is deprecated'): + from acme.crypto_util import Format + assert Format.DER.to_cryptography_encoding() == serialization.Encoding.DER + assert Format.PEM.to_cryptography_encoding() == serialization.Encoding.PEM class MiscTests(unittest.TestCase): diff --git a/acme/src/acme/crypto_util.py b/acme/src/acme/crypto_util.py index 765cd7c10..587e52430 100644 --- a/acme/src/acme/crypto_util.py +++ b/acme/src/acme/crypto_util.py @@ -3,11 +3,14 @@ import enum from datetime import datetime, timedelta, timezone import ipaddress import logging +from types import ModuleType import typing +from typing import Any from typing import Literal from typing import Optional from typing import Union import warnings +import sys from cryptography import x509 from cryptography.hazmat.primitives import hashes, serialization @@ -18,8 +21,38 @@ from OpenSSL import crypto logger = logging.getLogger(__name__) +# https://github.com/pyca/cryptography/blob/1eab7a67dcc34b568e3c0df64e6222a2ac74b1ee/src/cryptography/utils.py#L66-L113 +class _ClientDeprecationModule(ModuleType): + """ + Internal class delegating to a module, and displaying warnings when attributes + related to deprecated attributes in the acme.client module. + """ + def __init__(self, module: ModuleType) -> None: + super().__init__(module.__name__) + self.__dict__['_module'] = module + + def __getattr__(self, attr: str) -> Any: + if attr == 'Format': + warnings.warn("acme.crypto_util.Format is deprecated and will be removed in " + "the next major release.", DeprecationWarning) + return getattr(self._module, attr) + + def __setattr__(self, attr: str, value: Any) -> None: # pragma: no cover + setattr(self._module, attr, value) + + def __delattr__(self, attr: str) -> None: # pragma: no cover + delattr(self._module, attr) + + def __dir__(self) -> list[str]: # pragma: no cover + return ['_module'] + dir(self._module) + + +# Patching ourselves to warn about deprecation and planned removal of some elements in the module. +sys.modules[__name__] = _ClientDeprecationModule(sys.modules[__name__]) + + class Format(enum.IntEnum): - """File format to be used when parsing or serializing X.509 structures. + """File format to be used when parsing or serializing X.509 structures. Deprecated. Backwards compatible with the `FILETYPE_ASN1` and `FILETYPE_PEM` constants from pyOpenSSL. @@ -36,7 +69,7 @@ class Format(enum.IntEnum): return Encoding.PEM -# Even *more* annoyingly, due to a mypy bug, we can't use Union[] types in +# Annoyingly, due to a mypy bug, we can't use Union[] types in # isinstance expressions without causing false mypy errors. So we have to # recreate the type collection as a tuple here. And no, typing.get_args doesn't # work due to another mypy bug. diff --git a/certbot/src/certbot/_internal/cli/helpful.py b/certbot/src/certbot/_internal/cli/helpful.py index 4c356123a..f496b8536 100644 --- a/certbot/src/certbot/_internal/cli/helpful.py +++ b/certbot/src/certbot/_internal/cli/helpful.py @@ -330,7 +330,7 @@ class HelpfulArgumentParser: raise errors.Error("--allow-subset-of-names cannot be used with --csr") csrfile, contents = config.csr[0:2] - typ, util_csr, _ = crypto_util.import_csr_file(csrfile, contents) + util_csr = crypto_util.read_csr_file(csrfile, contents) x509_req = x509.load_pem_x509_csr(util_csr.data) domains, ip_addresses = san.from_x509(x509_req.subject, x509_req.extensions) @@ -344,7 +344,7 @@ class HelpfulArgumentParser: f"Unfortunately, your CSR {config.csr[0]} needs to have a SubjectAltName for " + "every domain or IP address") - config.actual_csr = (util_csr, typ) + config.actual_csr = util_csr # Check that the original values for --domain and --ip-address set by the user were # a subset of the domains listed in the CSR. diff --git a/certbot/src/certbot/_internal/main.py b/certbot/src/certbot/_internal/main.py index 6d1aec1df..e8654185c 100644 --- a/certbot/src/certbot/_internal/main.py +++ b/certbot/src/certbot/_internal/main.py @@ -1501,7 +1501,7 @@ def _csr_get_and_save_cert(config: configuration.NamespaceConfig, :rtype: `tuple` of `str` """ - util_csr, _ = config.actual_csr + util_csr = config.actual_csr x509_req = x509.load_pem_x509_csr(util_csr.data) domains, ip_addresses = san.from_x509(x509_req.subject, x509_req.extensions) display_util.notify( diff --git a/certbot/src/certbot/_internal/tests/crypto_util_test.py b/certbot/src/certbot/_internal/tests/crypto_util_test.py index e8af06bee..e767076c5 100644 --- a/certbot/src/certbot/_internal/tests/crypto_util_test.py +++ b/certbot/src/certbot/_internal/tests/crypto_util_test.py @@ -128,35 +128,31 @@ class CSRMatchesPubkeyTest(unittest.TestCase): test_util.load_vector('csr_512.pem'), P256_KEY) -class ImportCSRFileTest(unittest.TestCase): - """Tests for certbot.certbot_util.import_csr_file.""" +class ReadCSRFileTest(unittest.TestCase): + """Tests for certbot.certbot_util.read_csr_file.""" @classmethod def _call(cls, *args, **kwargs): - from certbot.crypto_util import import_csr_file - return import_csr_file(*args, **kwargs) + from certbot.crypto_util import read_csr_file + return read_csr_file(*args, **kwargs) def test_der_csr(self): csrfile = test_util.vector_path('csr_512.der') data = test_util.load_vector('csr_512.der') data_pem = test_util.load_vector('csr_512.pem') - assert (acme_crypto_util.Format.PEM, - util.CSR(file=csrfile, + assert util.CSR(file=csrfile, data=data_pem, - form="pem"), - ["Example.com"]) == \ + form="pem") == \ self._call(csrfile, data) def test_pem_csr(self): csrfile = test_util.vector_path('csr_512.pem') data = test_util.load_vector('csr_512.pem') - assert (acme_crypto_util.Format.PEM, - util.CSR(file=csrfile, + assert util.CSR(file=csrfile, data=data, - form="pem"), - ["Example.com"],) == \ + form="pem") == \ self._call(csrfile, data) def test_bad_csr(self): @@ -165,6 +161,46 @@ class ImportCSRFileTest(unittest.TestCase): test_util.load_vector('cert_512.pem')) +class ImportCSRFileTest(unittest.TestCase): + """Tests for certbot.certbot_util.import_csr_file.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.crypto_util import import_csr_file + with pytest.warns(DeprecationWarning, match='import_csr_file is deprecated'): + return import_csr_file(*args, **kwargs) + + def test_der_csr(self): + csrfile = test_util.vector_path('csr_512.der') + data = test_util.load_vector('csr_512.der') + data_pem = test_util.load_vector('csr_512.pem') + + with pytest.warns(DeprecationWarning, match='Format is deprecated'): + assert (acme_crypto_util.Format.PEM, + util.CSR(file=csrfile, + data=data_pem, + form="pem"), + ["Example.com"]) == \ + self._call(csrfile, data) + + def test_pem_csr(self): + csrfile = test_util.vector_path('csr_512.pem') + data = test_util.load_vector('csr_512.pem') + + with pytest.warns(DeprecationWarning, match='Format is deprecated'): + assert (acme_crypto_util.Format.PEM, + util.CSR(file=csrfile, + data=data, + form="pem"), + ["Example.com"],) == \ + self._call(csrfile, data) + + def test_bad_csr(self): + with pytest.raises(errors.Error): + self._call(test_util.vector_path('cert_512.pem'), + test_util.load_vector('cert_512.pem')) + + class MakeKeyTest(unittest.TestCase): """Tests for certbot.crypto_util.make_key.""" @@ -335,7 +371,8 @@ class GetSANsFromCertTest(unittest.TestCase): @classmethod def _call(cls, *args, **kwargs): from certbot.crypto_util import get_sans_from_cert - return get_sans_from_cert(*args, **kwargs) + with pytest.warns(DeprecationWarning, match='get_sans_from_cert is deprecated'): + return get_sans_from_cert(*args, **kwargs) def test_single(self): assert [] == self._call(test_util.load_vector('cert_512.pem')) @@ -351,7 +388,8 @@ class GetNamesFromCertTest(unittest.TestCase): @classmethod def _call(cls, *args, **kwargs): from certbot.crypto_util import get_names_from_cert - return get_names_from_cert(*args, **kwargs) + with pytest.warns(DeprecationWarning, match='get_names_from_cert is deprecated'): + return get_names_from_cert(*args, **kwargs) def test_single(self): assert ['example.com'] == \ @@ -378,7 +416,8 @@ class GetNamesFromReqTest(unittest.TestCase): @classmethod def _call(cls, *args, **kwargs): from certbot.crypto_util import get_names_from_req - return get_names_from_req(*args, **kwargs) + with pytest.warns(DeprecationWarning, match='get_names_from_req is deprecated'): + return get_names_from_req(*args, **kwargs) def test_nonames(self): assert [] == \ @@ -394,8 +433,9 @@ class GetNamesFromReqTest(unittest.TestCase): self._call(test_util.load_vector('csr-6sans_512.pem')) def test_der(self): - assert ['Example.com'] == \ - self._call(test_util.load_vector('csr_512.der'), typ=acme_crypto_util.Format.DER) + with pytest.warns(DeprecationWarning, match='Format is deprecated'): + assert ['Example.com'] == \ + self._call(test_util.load_vector('csr_512.der'), typ=acme_crypto_util.Format.DER) class NotBeforeTest(unittest.TestCase): diff --git a/certbot/src/certbot/crypto_util.py b/certbot/src/certbot/crypto_util.py index 05f77e858..436ab9475 100644 --- a/certbot/src/certbot/crypto_util.py +++ b/certbot/src/certbot/crypto_util.py @@ -12,6 +12,7 @@ import re from typing import Optional from typing import TYPE_CHECKING from typing import Union +import warnings from cryptography import x509 from cryptography.exceptions import InvalidSignature @@ -171,9 +172,36 @@ def csr_matches_pubkey(csr: bytes, privkey: bytes) -> bool: return req.is_signature_valid and req.public_key() == pkey.public_key() +def read_csr_file( + csrfile: str, data: bytes +) -> util.CSR: + """Reads a CSR file, which can be either PEM or DER, and returns a + `certbot.util.CSR` object. + + :param str csrfile: CSR filename + :param bytes data: contents of the CSR file + + :returns: object representing the CSR + :rtype: util.CSR + + """ + try: + # Try to parse as DER first, then fall back to PEM. + csr = x509.load_der_x509_csr(data) + except ValueError: + try: + csr = x509.load_pem_x509_csr(data) + except ValueError: + raise errors.Error("Failed to parse CSR file: {0}".format(csrfile)) + + # Internally we always use PEM, so re-encode as PEM before returning. + data_pem = csr.public_bytes(serialization.Encoding.PEM) + return util.CSR(file=csrfile, data=data_pem, form="pem") + + def import_csr_file( csrfile: str, data: bytes -) -> tuple[acme_crypto_util.Format, util.CSR, list[str]]: +) -> tuple['acme_crypto_util.Format', util.CSR, list[str]]: """Import a CSR file, which can be either PEM or DER. :param str csrfile: CSR filename @@ -185,6 +213,9 @@ def import_csr_file( :rtype: tuple """ + warnings.warn("certbot.crypto_util.import_csr_file is deprecated and " + "will be removed in the next major release. Please use " + "certbot.crypto_util.read_csr_file instead.", DeprecationWarning) try: # Try to parse as DER first, then fall back to PEM. csr = x509.load_der_x509_csr(data) @@ -197,11 +228,13 @@ def import_csr_file( domains = acme_crypto_util.get_names_from_subject_and_extensions(csr.subject, csr.extensions) # Internally we always use PEM, so re-encode as PEM before returning. data_pem = csr.public_bytes(serialization.Encoding.PEM) - return ( - acme_crypto_util.Format.PEM, - util.CSR(file=csrfile, data=data_pem, form="pem"), - domains, - ) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", "acme.crypto_util.Format is deprecated") + return ( + acme_crypto_util.Format.PEM, + util.CSR(file=csrfile, data=data_pem, form="pem"), + domains, + ) def make_key(bits: int = 2048, key_type: str = "rsa", @@ -389,7 +422,7 @@ def verify_fullchain(renewable_cert: interfaces.RenewableCert) -> None: def get_sans_from_cert( - cert: bytes, typ: Union[acme_crypto_util.Format, int] = acme_crypto_util.Format.PEM + cert: bytes, typ: 'acme_crypto_util.Format | int | None' = None ) -> list[str]: """Get a list of Subject Alternative Names from a certificate. @@ -400,12 +433,18 @@ def get_sans_from_cert( :rtype: list """ - typ = acme_crypto_util.Format(typ) - if typ == acme_crypto_util.Format.PEM: - x509_cert = x509.load_pem_x509_certificate(cert) - else: - assert typ == acme_crypto_util.Format.DER - x509_cert = x509.load_der_x509_certificate(cert) + warnings.warn("get_sans_from_cert is deprecated and will be removed in the next " + "major release.", DeprecationWarning) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", "acme.crypto_util.Format is deprecated") + if typ is None: + typ = acme_crypto_util.Format.PEM + typ = acme_crypto_util.Format(typ) + if typ == acme_crypto_util.Format.PEM: + x509_cert = x509.load_pem_x509_certificate(cert) + else: + assert typ == acme_crypto_util.Format.DER + x509_cert = x509.load_der_x509_certificate(cert) try: san_ext = x509_cert.extensions.get_extension_for_class( @@ -418,7 +457,7 @@ def get_sans_from_cert( def get_names_from_cert( - cert: bytes, typ: Union[acme_crypto_util.Format, int] = acme_crypto_util.Format.PEM + cert: bytes, typ: 'acme_crypto_util.Format | int | None' = None ) -> list[str]: """Get a list of domains from a cert, including the CN if it is set. @@ -429,19 +468,25 @@ def get_names_from_cert( :rtype: list """ - typ = acme_crypto_util.Format(typ) - if typ == acme_crypto_util.Format.PEM: - x509_cert = x509.load_pem_x509_certificate(cert) - else: - assert typ == acme_crypto_util.Format.DER - x509_cert = x509.load_der_x509_certificate(cert) + warnings.warn("get_names_from_cert is deprecated and will be removed in the next " + "major release.", DeprecationWarning) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", "acme.crypto_util.Format is deprecated") + if typ is None: + typ = acme_crypto_util.Format.PEM + typ = acme_crypto_util.Format(typ) + if typ == acme_crypto_util.Format.PEM: + x509_cert = x509.load_pem_x509_certificate(cert) + else: + assert typ == acme_crypto_util.Format.DER + x509_cert = x509.load_der_x509_certificate(cert) return acme_crypto_util.get_names_from_subject_and_extensions( x509_cert.subject, x509_cert.extensions ) def get_names_from_req( - csr: bytes, typ: Union[acme_crypto_util.Format, int] = acme_crypto_util.Format.PEM + csr: bytes, typ: 'acme_crypto_util.Format | int | None' = None ) -> list[str]: """Get a list of domains from a CSR, including the CN if it is set. @@ -451,12 +496,18 @@ def get_names_from_req( :rtype: list """ - typ = acme_crypto_util.Format(typ) - if typ == acme_crypto_util.Format.PEM: - x509_req = x509.load_pem_x509_csr(csr) - else: - assert typ == acme_crypto_util.Format.DER - x509_req = x509.load_der_x509_csr(csr) + warnings.warn("get_names_from_req is deprecated and will be removed in the next " + "major release.", DeprecationWarning) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", "acme.crypto_util.Format is deprecated") + if typ is None: + typ = acme_crypto_util.Format.PEM + typ = acme_crypto_util.Format(typ) + if typ == acme_crypto_util.Format.PEM: + x509_req = x509.load_pem_x509_csr(csr) + else: + assert typ == acme_crypto_util.Format.DER + x509_req = x509.load_der_x509_csr(csr) return acme_crypto_util.get_names_from_subject_and_extensions( x509_req.subject, x509_req.extensions ) diff --git a/newsfragments/10433.changed b/newsfragments/10433.changed new file mode 100644 index 000000000..2ca5fa200 --- /dev/null +++ b/newsfragments/10433.changed @@ -0,0 +1,6 @@ +A few largely unused functions/types have been deprecated in our effort to remove our pyOpenSSL dependency: + * Deprecated: `certbot.crypto_util.get_sans_from_cert` + * Deprecated: `certbot.crypto_util.get_names_from_cert` + * Deprecated: `certbot.crypto_util.get_names_from_req` + * Deprecated: `certbot.crypto_util.import_csr_file` (and replaced by `certbot.crypto_util.read_csr_file`) + * Deprecated: `acme.crypto_util.Format`