Skip to content

Switch from pycryptodomex to cryptography.io#278

Open
geofft wants to merge 3 commits intosibson:mainfrom
geofft:switch-to-cryptography
Open

Switch from pycryptodomex to cryptography.io#278
geofft wants to merge 3 commits intosibson:mainfrom
geofft:switch-to-cryptography

Conversation

@geofft
Copy link

@geofft geofft commented Jan 8, 2024

cryptography.io wraps an existing well-known cryptography library (OpenSSL) instead of implementing routines itself, and it's already a dependency of Twisted, so this reduces the overall number of cryptography libraries needed.

@geofft
Copy link
Author

geofft commented Jan 8, 2024

I've done a little bit of unit testing by hand but let me try to make some real connections with this too, since I don't think there's much test coverage for this code. Also I'm open to feedback on whether this is a desirable thing at all.

@geofft geofft marked this pull request as ready for review January 8, 2024 23:09
@geofft
Copy link
Author

geofft commented Jan 8, 2024

OK, I've confirmed that this works in both of the cases that are affected, i.e., password auth (with Xtightvnc) and Apple Remote Desktop (with macOS 13.6), so I think I got the code right :)

cryptography.io wraps an existing well-known cryptography library
(OpenSSL) instead of implementing routines itself, and it's already a
dependency of Twisted, so this reduces the overall number of
cryptography libraries needed.
@geofft geofft force-pushed the switch-to-cryptography branch from 7b48f2c to 030e7c6 Compare January 8, 2024 23:16
@Neustradamus
Copy link

@sibson: Have you seen this @geofft PR?

@sibson
Copy link
Owner

sibson commented Jan 18, 2025

@geofft could you resolve conflicts?

I don't have a strong opinion here, my quick google suggests either is fine but gives cryptography a slight edge due to fewer sharp edges

@pmhahn you most recently updated the dependency here. Do you have an opinion?

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