Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyOpenSSL X509Store / Context parity in Cryptography #10393

Open
vEpiphyte opened this issue Feb 14, 2024 · 3 comments
Open

PyOpenSSL X509Store / Context parity in Cryptography #10393

vEpiphyte opened this issue Feb 14, 2024 · 3 comments

Comments

@vEpiphyte
Copy link

Hello!

I've been working on updating some code to utilize cryptography in favor of PyOpenSSL due to the API deprecation in the older project.

The only code that I can not currently remove is related to the use of X509Store and X509StoreContext. That is utilized for doing certificate validation. For example:

    def verifyACertificate(self, cert: c_x509.Certificate):
    	'''
    	Check if a Certificate is trusted by a set of CAs and is not revoked.
    	'''
        crls = self.getCaCrls()  # type: List[c_x509.CertificateRevocationList]
        cacerts = self.getCaCerts()  # type: List[c_x509.Certificate]

        store = crypto.X509Store()  # This is pyopenssl...
        [store.add_cert(crypto.X509.from_cryptography(cacert)) for cacert in cacerts]

        if crls:
            # Setting flags is important. Other uses use things such as PARTIAL_CHAIN flags.
            store.set_flags(crypto.X509StoreFlags.CRL_CHECK | crypto.X509StoreFlags.CRL_CHECK_ALL)
            [store.add_crl(crypto.CRL.from_cryptography(crl)) for crl in crls]

        ctx = crypto.X509StoreContext(store, crypto.X509.from_cryptography(cert))
        try:
            ctx.verify_certificate()  # raises X509StoreContextError if unable to verify
        except crypto.X509StoreContextError as e:
            mesg = _unpackContextError(e)  # helper function to unpack the X509StoreContextError, not important here.  
            raise Exception(mesg)
        return cert

I believe my use case aligns with #10276 ( doing code signing and/or user cert verification ). Current docs for the verification APIS ( https://cryptography.io/en/42.0.2/x509/verification/ ) don't seem to support setting CRLs or flag setting.

Is this type of use case in scope for work in #10345 ?

@alex
Copy link
Member

alex commented Feb 14, 2024

cc: @woodruffw

Looks like there's two pieces you need here:

  1. Doing verification without checking for a particular SAN, which is what @woodruffw has in his PR
  2. Checking CLRs.

Both of these are tracked in #10034

Figuring out CLRs is going to require some API design work on our part. It looks like your implementation relies on having pre-fetched the CRLs, and not loading them on-demand.

@vEpiphyte
Copy link
Author

@alex Correct - I am assuming that CRLs are present ahead of time ( and very much not assuming that it is the job of cryptography to retrieve them ).

If these pieces are being correctly tracked in #10034 we can close this issue out to avoid duplication.

@vEpiphyte vEpiphyte changed the title PyOpenSSL X509Store / SSLContext parity in Cryptography PyOpenSSL X509Store / Context parity in Cryptography Feb 14, 2024
vEpiphyte added a commit to vertexproject/synapse that referenced this issue Feb 28, 2024
…#3568)

- Cryptography update addresses older version of cryptography package containing CVE-2023-50782 & CVE-2024-26130
- certdir now uses cryptography X509 objects and RSA private key objects, instead of PyOpenSSL X509 and Pkey objects. This is largely due to the removal of APIs from PyOpenSSL which we were utilizing for PKCS12 support and the guidance from PyOpenSSL project to not utilize the ``Crypto`` module in new projects as it is considered deprecated in
favor of Cryptography. Per prior discussion, there should be no API stability concerns related to this change since the CertDir class is not exposed via telepath or storm apis.
- certdir is now fully typed. This identified issues where we were declaring bytes as inputs on certdir and Cortex was passing in PEM strings instead of bytes.
- Remove PyOpenSSL use where it is possible to do so. We now only use it for doing X509 path building and certificate verification, eventually we'll be able to remove this in favor of APIs provided by Cryptography ( see pyca/cryptography#10393 pyca/cryptography#10034 )

---------

Co-authored-by: Cisphyx <[email protected]>
@alex
Copy link
Member

alex commented Mar 21, 2024

#10345 adds verification without a subject.

CRL is the remaining piece here. We still need to figure out what we want to do in terms of API design there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants