search

SecurityInnovation / PGPy

Pretty Good Privacy for Python
BSD 3-Clause "New" or "Revised" License
317 stars 99 forks source link

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

Closed thesuperzapper closed 1 year ago

thesuperzapper commented 2 years ago

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: https://github.com/SecurityInnovation/PGPy/issues/402

There is a PR is attempting to fix it (https://github.com/SecurityInnovation/PGPy/pull/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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/SecurityInnovation/PGPy/issues/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 commented 2 years ago

@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 commented 2 years ago

@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 https://github.com/SecurityInnovation/PGPy/pull/403 to support cryptography 38?

If no, do you know who does?

KOLANICH commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Tomorrow that is, not tonight

KOLANICH commented 2 years ago
  1. I think that before the release we should get the tests working. Currently the tests themselves are broken. #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 commented 2 years ago

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 commented 1 year ago

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

SI-jvictors commented 1 year ago

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

Commod0re commented 1 year ago

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 commented 1 year ago

Any chances for this to happen this week?

Commod0re commented 1 year ago

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 commented 1 year ago

follow along progress fixing everything here: #423

Commod0re commented 1 year ago

just released PGPy v0.6.0

thesuperzapper commented 1 year ago

just released PGPy v0.6.0

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