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

CRITICAL - we must release a version that pins cryptography < 38.0.0 #408

Closed
thesuperzapper opened this issue Nov 1, 2022 · 20 comments
Closed

Comments

@thesuperzapper
Copy link
Contributor

Currently, running pip install PGPy will result in a broken environment, and running import pgpy will fail.

This is because PGP version 0.5.4 is NOT compatible with any version of cryptography after 38.0.0 due to: #402

There is a PR is attempting to fix it (#403) but in the interest of making pip install PGPy work, we should release a small patch that pins cryptography<38.0.0.

I have made the following PR to do this:


If any of the following people are still alive (and have write-access to this repo), I would greatly appreciate you merging this PR and triggering a release of PGPy version 0.5.5:

(Sorry for the mass ping, but I have no idea who is dead/alive)

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 1, 2022

Instead of pinning dependencies using < conditions one should fix PGPy and pin the dependencies using >= condition. Or even better, dispatch on the version and support both old ones and new ones.

@thesuperzapper
Copy link
Contributor Author

Instead of pinning dependencies using < conditions one should fix PGPy and pin the dependencies using >= condition. Or even better, dispatch on the version and support both old ones and new ones.

@KOLANICH having a blanket > condition is quite dangerous (like we currently do for cryptography) because we don't know what breaking changes might happen in those packages in future.

Obviously, we should try and resolve #402 (so that we can support cryptography version 38+), but I think we should still introduce a "max" version pin for 2 reasons:

  1. Doing pip install PGPy literally results in a not-working install in v0.5.4, so we want to push out a fix ASAP
  2. As I said above, we have no idea what future versions of cryptography might break, so we should consider pinning a maximum version and relaxing this version pin as we test new versions of cryptography

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 2, 2022

@KOLANICH having a blanket > condition is quite dangerous

Having < n condition is even more dangerous because it breaks compatibility to other package, the ones requiring <= n.

Doing pip install PGPy literally results in a not-working install in v0.5.4, so we want to push out a fix ASAP

Fix PGPy, and it wil. become working one.

As I said above, we have no idea what future versions of cryptography might break,

You can have even less idea of which other packages requiring >= n can emerge and which packages gonna require PGPy and them together. At least cryptography has some API and usually people try to keep any API as much stable as they can, because changing API means burden for them to change everything that relies on the changed API.

@thesuperzapper
Copy link
Contributor Author

@KOLANICH either way, do you have the permissions to merge PRs and trigger a release?

If yes, can you take a look at merging PR #403 to support cryptography 38?

If no, do you know who does?

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 2, 2022

either way, do you have the permissions to merge PRs and trigger a release?

No, I don't. It's just an opinion. I have no power within this project. I just consider it unwise to intentionally break future software just because of uncertainty in whether the future API will be compatible or not. Instead of creating them out of thin air the compatibility issues should be fixed when they emerge.

If no, do you know who does?

Probably @J-M0, git commits log has shown that it is he who merges PRs.

@thesuperzapper
Copy link
Contributor Author

@KOLANICH do you have any way to contact @J-M0 because I think the only option may be to fork this project (assuming he is dead).

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 2, 2022

According to GitHub contributions chart in his profile he is very well alive, the last contribution was 2022-10-10. Just wait, people are busy.

As a stopgap measure: make a fork and use PEP 508 specifier pointing to that repo: PGPy @ git+https://github.com/thesuperzapper/PGPy.git instead of just PGPy in your pyproject.toml.

@thesuperzapper
Copy link
Contributor Author

The last commit by @J-M0 on the SecurityInnovation/PGPy repo was 3rd Feb 2022. At this point, we need an active person with write access to this repo so we can merge these critical PRs and fix the package.

I see that this repo is owned by @SecurityInnovation, and that @mdulin2, @kn0wm4d, @si-ben, @SI-jvictors, and @awaugh-SI seem to work there. Can one of them please help us assign a new owner this repo?

@Commod0re
Copy link
Contributor

I’ve merged several PRs including the #403 and #407. I should have time to try to help resolve merge conflicts on @KOLANICH ’s remaining PRs and then we shall see whether I still have permission to push new releases to pypi or not

@Commod0re
Copy link
Contributor

Tomorrow that is, not tonight

@KOLANICH
Copy link
Contributor

KOLANICH commented Nov 3, 2022

  1. I think that before the release we should get the tests working. Currently the tests themselves are broken. Fixed gpg_ver initialization in conftest.py. #410 tries to fix it. After this fix tests start to run, but there are a lot of failed tests
  2. I have rebased my branches with the PRs over master, but again in order to merge them we need the tests passing in master first just to make sure my PRs don't break anything.

@SI-jvictors
Copy link

I'm alive, I'll take a look at this this evening when I have some availability, and I've pinged the rest of the team.

@prd-an-leminh
Copy link

I'm expecting the new version will be released soon.

@SI-jvictors
Copy link

I think @Commod0re was merging those branches as mentioned above, we might be ready unless we're stuck somewhere else

@Commod0re
Copy link
Contributor

yes I should be able to spend some time today merging a few more PRs, double checking tests and functionality, and then hopefully doing a new release

@nomorsug
Copy link

Any chances for this to happen this week?

@Commod0re
Copy link
Contributor

it's in a pretty busted state right now and I'm slowly unraveling it, it will take time and I don't have much spare

@Commod0re
Copy link
Contributor

follow along progress fixing everything here: #423

@Commod0re
Copy link
Contributor

just released PGPy v0.6.0

@thesuperzapper
Copy link
Contributor Author

just released PGPy v0.6.0

@Commod0re thank you for your amazing work! You have saved this project!

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

6 participants