Replace PyOpenSSL with cryptography#165
Conversation
wosc
force-pushed
the
main
branch
2 times, most recently
from
926319b to
c5b7c2b
Compare
Lekensteyn
left a comment
Lekensteyn
left a comment
There was a problem hiding this comment.
Approving with nits.
Tested this on Debian Trixie with Python 3.13.5. For what's worth, PyOpenSSL-26.3.0 is still installed because acme>=4.0 still depends on it:
https://github.com/certbot/certbot/blob/v5.6.0/acme/pyproject.toml#L33
The main fixes needed for modern Python 3:
- replace
pkg_resourcesfrom setuptools byimportlibfrom stdlib - use
cryptographyinstead of OpenSSL, fixes aAttributeError: module 'OpenSSL.crypto' has no attribute 'X509Req'while importingjosepy.
six is indeed also no longer needed.
Validated with these dependencies: PyOpenSSL-26.3.0 acme-5.6.0 certifi-2026.6.17 cffi-2.0.0 charset_normalizer-3.4.7 cryptography-49.0.0 idna-3.18 josepy-2.2.0 pycparser-3.0 pyrfc3339-2.1.0 requests-2.34.2 simp-le-client-2.0 urllib3-2.7.0
| def setup_logging(verbose): | ||
| """Setup basic logging.""" | ||
| level = logging.DEBUG if verbose else logging.INFO | ||
| level = logging.DEBUG if verbose else logging.WARNING |
There was a problem hiding this comment.
This is not a bugfix, I'd remove it or add a --quiet option to reduce default logging.
It hides messages such as:
2026-07-02 22:08:07,030:INFO:__main__:1469: Generating new certificate private key
2026-07-02 22:08:12,986:INFO:__main__:401: Saving key.pem
2026-07-02 22:08:12,987:INFO:__main__:401: Saving cert.pem
2026-07-02 22:08:12,987:INFO:__main__:401: Saving chain.pem
...
2026-07-02 22:57:59,934:INFO:__main__:1482: Certificates already exist and renewal is not necessary, exiting with status code 1.
Personally I have a wrapper script that checks for exit code is 1 (EXIT_NO_RENEWAL), and ignores its output. For other error codes (0 - EXIT_RENEWAL, 2 - EXIT_ERROR), I show the output. I use chronic from the Debian moreutils package to hide the output if there is nothing notable (exit code 0).
There was a problem hiding this comment.
Yeah, This commit does not belong here at all, I'll drop it. Don't know what happened here, I must have gotten some branches mixed up or something.
| @@ -17,7 +17,7 @@ | |||
| # | |||
| # * https://github.com/zenhack/simp_le/issues/62 | |||
| # * https://github.com/pypa/pip/issues/988 | |||
There was a problem hiding this comment.
Should the comment be removed if pinning is no longer necessary?
There was a problem hiding this comment.
I removed the idna pin mostly to simplify what's going on. The actual issue in #62 was a conflict between some versions of cryptography and requests, which has long been resolved AFAICT. But you're right, the whole thing including the comment should be deleted in that case. I'll amend the commit.
|
|
||
| VERSION = importlib.metadata.version('simp_le-client') | ||
| # VERSION = importlib.metadata.version('simp_le-client') | ||
| VERSION = '1' |
There was a problem hiding this comment.
This VERSION = '1' is hardcoded, it got fixed in the "Replace pyopenssl with cryptography" commit. Could edit this commit to reduce confusion.
There was a problem hiding this comment.
Absolutely. To be honest, I don't remember what the reason was to hardcode this in the first place, maybe just a temporary debugging thing that got left in, or something? I'll amend the commit.
|
I've now updated the commits to take the review comments into account. |
2 participants
PyOpenSSL itself recommends this, and acme>=4.0 also performed this step (and removed openssl helper functions that simp_le used to rely on from josepy).