Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
prauscher wants to merge 5 commits into IdentityPython:master
base: master
Choose a base branch
Loading
from prauscher:replace-pyopenssl

Conversation

@prauscher
Copy link

@prauscher prauscher commented Feb 12, 2025
edited by c00kiemon5ter
Loading

Closes #879

Fixes #975


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

  • Checked that no other issues or pull requests exist for the same issue/change
  • Added tests covering the new functionality
  • Updated documentation OR the change is too minor to be documented
  • Updated CHANGELOG.md OR changes are insignificant

msadyk, dgorobets, eirsyl, hciudad, e1mo, axelknock, mostafa, mskrip, maxhowald, jacinda, and 5 more reacted with thumbs up emoji melvyn2, mikicz, eirsyl, mostafa, ducdetronquito, felixxm, jacinda, and miallo reacted with hooray emoji mweinelt, mikicz, e1mo, mostafa, jacinda, felixxm, and miallo reacted with heart emoji
Copy link
Author

@c00kiemon5ter Is there anything you need or I could help you with to bring this forward?

msadyk, axelknock, miettal, e1mo, pshchelo, ducdetronquito, mskrip, mikicz, maxhowald, jacinda, and hsenot reacted with thumbs up emoji

miettal added a commit to girasolenergy/pysaml2 that referenced this pull request Mar 13, 2025
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 
Copy link

Hi !

At work we use @prauscher branch in production on multiple backends since yesterday with Python@3.13 and djangosaml2@1.10.1 and it works like a charm.

I hope this small datapoint will give more confidence to merge this PR :)

prauscher, jacinda, and io-ma reacted with hooray emoji

Copy link
Contributor

I also merged @prauscher branch to our repo and, with my exception above, have seen no problems. Thanks!

felixxm reacted with thumbs up emoji

Copy link
Author

Thanks @johanlundberg, my new commit should eliminiate these problems

johanlundberg and miettal reacted with thumbs up emoji

Copy link

Working great for us too! Just noting that for since the deps have been updated, the poetry.lock file also needs to be regenerated. poetry lock --no-update gives me this patch (I'd make a suggested change, but Github won't let me 😄)

diff --git a/poetry.lock b/poetry.lock
index 93e08556..6357d2b7 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -1248,24 +1248,6 @@ snappy = ["python-snappy"]
 test = ["pytest (>=8.2)", "pytest-asyncio (>=0.24.0)"]
 zstd = ["zstandard"]
-[[package]]
-name = "pyopenssl"
-version = "24.2.1"
-description = "Python wrapper module around the OpenSSL library"
-optional = false
-python-versions = ">=3.7"
-files = [
- {file = "pyOpenSSL-24.2.1-py3-none-any.whl", hash = "sha256:967d5719b12b243588573f39b0c677637145c7a1ffedcd495a487e58177fbb8d"},
- {file = "pyopenssl-24.2.1.tar.gz", hash = "sha256:4247f0dbe3748d560dcbb2ff3ea01af0f9a1a001ef5f7c4c647956ed8cbf0e95"},
-]
-
-[package.dependencies]
-cryptography = ">=41.0.5,<44"
-
-[package.extras]
-docs = ["sphinx (!=5.2.0,!=5.2.0.post0,!=7.2.5)", "sphinx-rtd-theme"]
-test = ["pretend", "pytest (>=3.0.1)", "pytest-rerunfailures"]
-
 [[package]]
 name = "pytest"
 version = "8.3.4"
@@ -1971,4 +1953,4 @@ s2repoze = ["paste", "repoze.who", "zope.interface"]
 [metadata]
 lock-version = "2.0"
 python-versions = "^3.9"
-content-hash = "03ec3d72ebbfd582b62c153e6921c3032ff637943724b8a9222bc9593eae71e0"
+content-hash = "ac583ea6afeb45d9c6a66f1a5a49ce08d58b4c0ef554be7d65c55761bc8e8c42"
ducdetronquito reacted with thumbs up emoji

Copy link
Author

Thank you @alecbarber - I also found types-pyopenssl to be obsolete now

Copy link

It also looks like the dependency on pytz can be removed.

Copy link
Author

@rvanlaar good catch - as dateutil is no longer required too I removed pytz and dateutil as well as both typing classes.

rvanlaar and ducdetronquito reacted with rocket emoji

alecbarber pushed a commit to affinda/pysaml2 that referenced this pull request Jul 21, 2025
Applies [@praushcer](https://github.com/prauscher)'s changes to remove pyopenssl.
see [upstream IdentityPython#977](IdentityPython#977)
Copy link

mikicz commented Jul 25, 2025

I would love it if this went in!

ducdetronquito, jacinda, miettal, and Sahan-Clio reacted with thumbs up emoji

Copy link
Author

@c00kiemon5ter Is there anything I can help you with to get this merged?

ducdetronquito, mskrip, miettal, katonalala, hsenot, and jacinda reacted with thumbs up emoji

Copy link
Author

@c00kiemon5ter I have merged master into this branch, so it should now be mergeable. As I do not understand any of this poetry magic, I hope my poetry lock does not trigger new problems, but please tell me if I should rebuild the poetry.lock somehow.

Copy link

hsenot commented Oct 8, 2025
edited
Loading

Hi @prauscher and @c00kiemon5ter - we've been relying on your fork/branch in our app and have noticed a breaking change as of 2 days ago.

It looks like:

requires-python = "^3.9"

is not accepted by our build chain (pip/uv within docker).

Further research suggests that the caret is not PEP 440-compliant. A possible fix would be:

requires-python = ">=3.9,<4.0"

Happy to be challenged on this, as I am not very versed in poetry magic :-)

Copy link
Member

Latest master (or v7.5.4) have fixed this.
Using >=3.9 is fine for now.

@prauscher I would avoid merges from other branches to keep the diff clean.
IMHO, rebasing is better and I will rebase this to test it.

ducdetronquito reacted with hooray emoji

Copy link
Author

Rebased to current master, so @hsenot should be working again.

Personally I prefer merge over rebase to avoid force pushes, but ymmv. From what I can tell from the change preview, this should be clean-ish now.

ducdetronquito and SPKorhonen reacted with hooray emoji

Copy link

hsenot commented Oct 9, 2025

Thanks heaps @prauscher , much appreciated.
We're keen to see this branch/fork merged too.
Lmk if you need help testing. Cheers!

SPKorhonen and jacinda reacted with thumbs up emoji ducdetronquito and SPKorhonen reacted with hooray emoji

Copy link

I'm also keen to see this merged. I've tested the branch and it works flawlessly and would allow us to remove pyopenssl as dependency in out project.

ducdetronquito and jacinda reacted with thumbs up emoji

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

Reviewers

@johanlundberg johanlundberg johanlundberg left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

usage of pyopenssl library Test failures with pyopenssl 24.3.0

AltStyle によって変換されたページ (->オリジナル) /