pyca/pyopenssl

Deprecate `X509Extension` and `CRL` APIs

facutuesca opened this issue · 5 comments

facutuesca commented

These have better alternatives in cryptography, and users should be pointed to use them instead.

Here is a list of packages that depend on pyOpenSSL, along with their importance to the ecosystem (number of direct+indirect dependents), current use of these APIs and places where pyOpenSSL is being used.

Format:

  • package-name (number of dependents)
    • Does it use either X509Extension or CRL
    • List of places in its codebase where it uses pyOpenSSL

Dependents:

facutuesca commented

Here's another list of important packages dependent on PyOpenSSL, this time sorted by # of downloads last month (I removed the packages already present in the previous list):

Format:

  • package-name (number of downloads)
    • Does it use either X509Extension or CRL
    • List of places in its codebase where it uses pyOpenSSL

Dependents:

alex commented

I just realized there's at least one public API that relies on CRL: add_crl on X509Store. My recommendation would be to change that method to allow it to accept a pyca/cryptography CRL or a pyOpenSSL CRL. This gives users who rely on that method a deprecation-free path.

facutuesca commented

@alex To have add_crl accept a x509.CertificateRevocationList, we would need to convert it so that _lib.X509_STORE_add_crl() can take it. Currently, the logic for that is in CRL::from_cryptography() and _load_crl(), two functions that are in the set to be deprecated.
Should we duplicate that logic in X509Store::add_crl, so that when those two are deprecated, add_crl() still works?

To put it in code, what we want is:

def add_crl(self, crl: Union["CRL", x509.CertificateRevocationList]) -> None:
    converted_crl = crl if isinstance(crl, CRL) else CRL.from_cryptography(crl)
    _openssl_assert(_lib.X509_STORE_add_crl(self._store, converted_crl._crl) != 0)

But since from_cryptography() (and _load_crl(), used by from_cryptography) are going to be deprecated, we would need to duplicate their logic in add_crl's definition

alex commented
facutuesca commented

I just realized there's at least one public API that relies on CRL: add_crl on X509Store. My recommendation would be to change that method to allow it to accept a pyca/cryptography CRL or a pyOpenSSL CRL. This gives users who rely on that method a deprecation-free path.

@alex Here's the PR for that: #1252