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

Replace PyOpenSSL with PyCA Crytography #501

Open
tiran opened this issue Jan 29, 2018 · 4 comments
Open

Replace PyOpenSSL with PyCA Crytography #501

tiran opened this issue Jan 29, 2018 · 4 comments

Comments

@tiran
Copy link

tiran commented Jan 29, 2018

fedmsg uses PyCA cryptography and PyOpenSSL's OpenSSL.crypto module. Please consider to use only PyCA cryptography.

https://pyopenssl.readthedocs.io/en/stable/api/crypto.html

pyca/cryptography is likely a better choice than using this module. It contains a complete set of cryptographic primitives as well as a significantly better and more powerful X509 API. If necessary you can convert to and from cryptography objects using the to_cryptography and from_cryptography methods on X509, X509Req, CRL, and PKey.

PyOpenSSL is used in fedmsg.crypto.x509_ng:

try:
# We require cryptography 1.6+ and pyOpenSSL 16.1+
#
# Until cryptography can do full chain certificate validation
# (https://github.com/pyca/cryptography/issues/2381) we need to use
# pyOpenSSL. However, pyOpenSSL is not meant to be a long-term solution
# since the ultimate goal is for it to be obsoleted:
# https://mail.python.org/pipermail/cryptography-dev/2017-June/000774.html
from cryptography import x509
from cryptography.exceptions import InvalidSignature
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import asymmetric, hashes, serialization
from OpenSSL.crypto import (X509Store, X509StoreContext, X509StoreContextError,
load_certificate, load_crl, FILETYPE_PEM, X509StoreFlags)
_cryptography = True

@jeremycline
Copy link
Member

@tiran I'd be very happy to drop the PyOpenSSL dependency. If I recall correctly, I needed it to do certificate chain validation and cryptography didn't have an API for that. Is that still the case? I saw it was mentioned that Alex was working on an implementation, but I couldn't find a PR or changelog entry so I'm guessing that's not landed yet.

@tiran
Copy link
Author

tiran commented Jan 29, 2018

PyOpenSSL is implemented on top of cryptography. You should be able to do anything with cryptography, but cryptography may not have a public API for that. I'll get back to you.

@tiran
Copy link
Author

tiran commented Jan 30, 2018

For the record, cryptography does not have an API for cert chain validation against a CRL and trust anchors yet,

def _validate_signing_cert(ca_certificate, certificate, crl=None):
"""
Validate an X509 certificate using pyOpenSSL.
.. note::
pyOpenSSL is a short-term solution to certificate validation. pyOpenSSL
is basically in maintenance mode and there's a desire in upstream to move
all the functionality into cryptography.
Args:
ca_certificate (str): A PEM-encoded Certificate Authority certificate to
validate the ``certificate`` with.
certificate (str): A PEM-encoded certificate that is in need of validation.
crl (str): A PEM-encoded Certificate Revocation List which, if provided, will
be taken into account when validating the certificate.
Raises:
X509StoreContextError: If the certificate failed validation. The
exception contains the details of the error.
"""
pyopenssl_cert = load_certificate(FILETYPE_PEM, certificate)
pyopenssl_ca_cert = load_certificate(FILETYPE_PEM, ca_certificate)
cert_store = X509Store()
cert_store.add_cert(pyopenssl_ca_cert)
if crl:
pyopenssl_crl = load_crl(FILETYPE_PEM, crl)
cert_store.add_crl(pyopenssl_crl)
cert_store.set_flags(X509StoreFlags.CRL_CHECK | X509StoreFlags.CRL_CHECK_ALL)
cert_store_context = X509StoreContext(cert_store, pyopenssl_cert)
cert_store_context.verify_certificate()

@jeremycline
Copy link
Member

Thanks for looking into that. I think that in the medium to long term, fedmsg will drop the signing/verifying messages feature. ZeroMQ provides a feature that covers message authentication so fedmsg doesn't need its own. If the API shows up in cryptography great, but we really should get rid of all the crypto code anyway.

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

No branches or pull requests

2 participants