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

dev-python/cryptography: bump version to 2.3 #9405

Closed
wants to merge 1 commit into from

Conversation

oz123
Copy link
Contributor

@oz123 oz123 commented Aug 1, 2018

Closes: https://bugs.gentoo.org/662564
Package-Manager: Portage-2.3.40, Repoman-2.3.9

Closes: https://bugs.gentoo.org/662564
Package-Manager: Portage-2.3.40, Repoman-2.3.9
@gentoo-bot
Copy link

Pull Request assignment

Areas affected: ebuilds
Packages affected: dev-python/cryptography, dev-python/cryptography-vectors

dev-python/cryptography: @gentoo/python
dev-python/cryptography-vectors: @gentoo/python

Bugs linked: 662564

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Aug 1, 2018
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2018-08-01 09:15 UTC
Newest commit scanned: 4dfd4b1
Status: ✅ good

No issues found

@ghost
Copy link

ghost commented Aug 1, 2018

Thanks @oz123 for the heads up. While I review this, would you mind splitting it up in two separate commits? We generally don't include more than one package in a commit (I'm surprised that repoman let you do this).

@ghost ghost self-assigned this Aug 1, 2018
@ghost
Copy link

ghost commented Aug 1, 2018

Have you tried installing it? on my machine, patches don't apply cleanly and build fail.

@oz123
Copy link
Contributor Author

oz123 commented Aug 1, 2018

@hsoft , I was afraid to remove the patches. I respect people who understand crypto better than me.
I thought they where there for a reason.
Removing those patch, the package will build successfully. Would you like me to remove those patches?

@ghost
Copy link

ghost commented Aug 1, 2018

No, those patches are probably necessary to the "libressl" USE flag. What is needed is:

  1. Track down the reason for these patches, there's probably a bug associated to it.
  2. Verify that they're still needed (I saw something about libressl in the changelog, maybe it's fixed upstream).
  3. Either:
    3a. Remove the patch and document why they're not necessary anymore
    3b. Update the patches so they apply cleanly to v2.3

All of this implies setting yourself a libressl environment. Sorry, sometimes minor bumps are not so minor :)

If that's too much for you, no problem, we'll handle it internally, but please tell us soon so we don't duplicate work and act fast (it's a CVE after all). If you're up to the challenge, no problem either, I can help you if you're stuck.

@oz123
Copy link
Contributor Author

oz123 commented Aug 1, 2018

I see that the patches where add in 079600f.
However, if they are only needed for building against libressl, then they should only be selectively applied. I can change that to apply the patches selectively.

All of this implies setting yourself a libressl environment. Sorry, sometimes minor bumps are not so minor :)

Do I need to install gentoo with libressl? that would involve compiling a lot of packages on a different machine?

@ghost
Copy link

ghost commented Aug 1, 2018

Yes, applying the patches selectively sounds like a good idea. If it works, go ahead.

For testing in different environments, yes, setting up a new environment is time consuming, but depending on your setup, you can end up being efficient in it.

For example, I use LXC and have a "pristine" gentoo env from the -t download gentoo image, minimally set up and updated. It mounts my host's /usr/portage for quick testing. From time to time, I update it.

When I need a new environment, I simply lxc-copy it and proceed with my tests. It's relatively effective.

In the case of libressl, because it requires a bit of manual setup by itself, I also maintain a "pristine libressl" container around.

Of course, you can use whatever tools work best for you...

@mgorny
Copy link
Member

mgorny commented Aug 1, 2018

No, conditional patches are very bad and should be avoided whenever possible. For example, with conditional patches you'd have committed a broken ebuild without even knowing the patches no longer apply.

@oz123
Copy link
Contributor Author

oz123 commented Aug 1, 2018

@mgorny okay, that is a good argument. How would you than solve it?
Why should one apply a patch for libressl when one does not have this flag or libressl?

@ghost
Copy link

ghost commented Aug 1, 2018

@oz123 those patches enable libressl but don't hinder building against openssl. That's why they can (and should, thanks @mgorny for the info) be applied unconditionally.

The steps to solve the problem stay the same as I outlined in my earlier comment: refresh the patches or remove them after verifications that they're obsolete.

@oz123
Copy link
Contributor Author

oz123 commented Aug 1, 2018

@hsoft, I think at this point I should acknowledge that I am still not able to do this. Can someone else take over from here?

@ghost
Copy link

ghost commented Aug 1, 2018

@oz123 sure, no problem. Thanks for the security scouting and for trying.

I'll proceed with the bump shortly.

@ghost ghost closed this Aug 1, 2018
@oz123 oz123 deleted the cryptography-2.3 branch October 5, 2019 16:12
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR.
Projects
None yet
4 participants