-
Notifications
You must be signed in to change notification settings - Fork 443
replace pyopenssl with cryptography #977
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
base: master
Are you sure you want to change the base?
Conversation
@c00kiemon5ter Is there anything you need or I could help you with to bring this forward? |
This PR upgrade pyopenssl dependency. Current constraints is `<24.3.0`(up to 24.2.x). New constratints is `<24.4.0`(up to 24.3.x). This PR is for addressing security alert `GHSA-79v4-65xg-pq4g`. GHSA-79v4-65xg-pq4g // I guess this constratints is for pyopenssl->cryptography migration. IdentityPython#977 IdentityPython@735bfa5
Hi ! At work we use @prauscher branch in production on multiple backends since yesterday with [email protected] and [email protected] and it works like a charm. I hope this small datapoint will give more confidence to merge this PR :) |
cert.set_issuer(cert.get_subject()) | ||
cert.set_pubkey(k) | ||
cert.sign(k, hash_alg) | ||
now = datetime.datetime.now(datetime.UTC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be compatible with Python < 3.11 datetime.UTC
needs to be change to datetime.timezone.utc
.
sign_key_str, password=passphrase) | ||
req_cert = x509.load_pem_x509_csr(request_cert_str) | ||
|
||
now = datetime.datetime.now(datetime.UTC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here
cert_str = cert_str.encode("utf-8") | ||
ca_cert = x509.load_pem_x509_certificate(signing_cert_str) | ||
cert = x509.load_pem_x509_certificate(cert_str) | ||
now = datetime.datetime.now(datetime.UTC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
valid_from = dateutil.parser.parse(cert.get_notBefore()) | ||
valid_to = dateutil.parser.parse(cert.get_notAfter()) | ||
active = not cert.has_expired() and valid_from <= now < valid_to | ||
now = datetime.datetime.now(datetime.UTC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and finally here
I also merged @prauscher branch to our repo and, with my exception above, have seen no problems. Thanks! |
closes #879
Description
The feature or problem addressed by this PR
This PR replaces pyopenssl whose usage is discouraged by its own developers. This is especially current since pyopenssl was forced to a version before 24.3.0 in response to #975. This inturn forces older cryptography-versions, making automated vulnerability checkers go brr.
What your changes do and why you chose this solution
The replacement of pyopenssl is quite direct, so probably cryptography could be used to a larger extend, reducing own security functions. On the other hand, cryptography is currently quite fixed to client/server authentification, enforcing stricter regulations on Certificate extensions etc, which might not be suitable here.
I ran the test suite on my machine which looked good, however while pyopenssl usually accepts strings or bytes, cryptography is usually fixed to bytes, forcing encoding to its user, so there may be dragons.
This being my first PR here, I'd highly value feedback and be happy to assist.
Checklist