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

Transitive dependency - cryptography is no longer optional #4493

Open
frenzymadness opened this issue Jan 13, 2025 · 5 comments
Open

Transitive dependency - cryptography is no longer optional #4493

frenzymadness opened this issue Jan 13, 2025 · 5 comments

Comments

@frenzymadness
Copy link

I see that cryptography is listed as an optional dependency here:

However, since this change: 5d2fa78

cryptography is no longer optional because slip10 has it as a runtime dependency so it got installed anyway.

Was that intentional or expected?

This might cause issues installing on some platforms because cryptography has no wheels for s390x and ppc64le, so users have to compile them there, which requires additional dependencies and tools.

@matejcik
Copy link
Contributor

thanks for the report.
definitely not intentional, but i'm not sure if we can manage to avoid it now :( except by making slip10 optional, which seems like a bad idea.
what do you think @andrewkozlik ? seems cryptography is used for their ed25519 impl, could we do with the private implementation that we have in trezorctl?

@matejcik
Copy link
Contributor

for context: we don't reeeeeally care about ppc64le or s390x, those are kind of unlikely to be users of Trezor devices. (i'm sniffing traces of something like Open Build Service here? where those archs are targets?)
but cryptography is pretty heavy and I would prefer for it to remain optional anyway

@andrewkozlik
Copy link
Contributor

Correct, in python-slip10 the cryptography module is used for the conversion of private keys to public keys in ed25519 and x25519.

We use SLIP10 in trezorlib for the new entropy check workflow. However, we do not necessarily have to support the ed25519, x25519 and NIST P-256 curves in trezorlib's entropy check workflow. So technically we could replace the SLIP10 module with some BIP32 module, which doesn't need these curves. Or perhaps we could make the ed25519 and x25519 curves somehow optional in python-slip10, in which case the cryptography module would become optional as well.

@prusnak
Copy link
Member

prusnak commented Jan 13, 2025

I would not overcomplicate stuff, though. There are multiple thousands of Python packages that require cryptography module and I am fine with requiring it as well. Honestly, if the only problem is with obscure platforms like s390x and ppc64le, then I would just close this and let the downstream (RedHat/SUSE guys) to figure this one out.

@frenzymadness
Copy link
Author

This is not something I'd insist on fixing. We were just using this project for some testing in a containerized environment where we cannot build cryptography from sources, so it started to fail, and I thought it was a good idea to report it because of the cryptography listed in the optional deps.

Feel free to close this. Just make sure to remove the cryptography from the optional dependencies and move it to the required ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

4 participants