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

Use PyCryptodome #80

Merged
merged 2 commits into from
Mar 13, 2022

Conversation

bit
Copy link

@bit bit commented Dec 12, 2021

Use PyCryptodome instead of PyCrypto

  • remove Counter
  • sing/verify with long values instead of bytes using _ functions

#68

@koolfy
Copy link
Contributor

koolfy commented Jan 31, 2022

Hello, I saw this PR in my emails 2 months ago and honestly dragged a bit because it was going to be some small changes to code I almost lost hope for.

But… from the looks of it you might have finished the pycryptodome transition I was working on for years and was stuck at because of some pycryptodome limitation o___o

I'll pay very close attention to this PR in the coming days, test it out, see what I missed on my side, what changed on pycryptodome since I last attempted to do exactly what you did, etc!

Check this for context of how much alcoholic/sugary beverage of your choice I will forever owe you if this turns out to put the project back on rails: #68

Huge thanks!!!

@bit
Copy link
Author

bit commented Feb 22, 2022

Would be great to see it merged and always happy to meet for drinks.
I had to rework your use of the Counter and call the _sign function in PyCryptodome to get low level enough access to the signing primitives. With that the change was manageable.

@koolfy koolfy changed the base branch from staging to bugfix/issue#68-use-pycryptodome March 13, 2022 11:15
@koolfy koolfy merged commit 22c4d90 into python-otr:bugfix/issue#68-use-pycryptodome Mar 13, 2022
@koolfy
Copy link
Contributor

koolfy commented Mar 13, 2022

Solved some quick conflicts, I'm merging into the dedicated pycryptodome branch, I'll test it out some more there and merge back into master and release if the merge makes sense (worst case scenario I just drop my previous pycryptodome branch and just use yours)

Then a quick release of a new major version and we should be good to go :)

@lanodan
Copy link

lanodan commented Mar 13, 2022

It looks like there is a slight issue, it is declared as depending on PyCryptodome (Crypto module) while it is importing the Cryptodome module (pycryptodomex). (See: https://github.com/Legrandin/pycryptodome/blob/master/README.rst#pycryptodome)

And I don't know for the other distros but it looks like only the Crypto module variant is available on Gentoo.

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

Successfully merging this pull request may close these issues.

3 participants