Nitrokey / pynitrokey

Python client for Nitrokey devices
Apache License 2.0
94 stars 28 forks source link

nitropy - invalid module error with latest python cryptography module #512

Closed tcanabrava closed 2 months ago

tcanabrava commented 3 months ago

Trying to update my nitrokey 3 firmware fails with:

command line tool to interact with Nitrokey devices 0.4.45
Critical error:
An unhandled exception occurred
        Exception encountered: ModuleNotFoundError("No module named 'cryptography.hazmat.backends.openssl.rsa'")

python-cryptography 42.0.5 installed, downgrading to 41 fixes the problem.

sosthene-nitrokey commented 3 months ago

Thank you for reporting this issue. I can reproduce it. I will investigate.

I moved it to the proper repository.

jans23 commented 3 months ago

Do you really mean nitrocli or nitropy?

sosthene-nitrokey commented 3 months ago

It's nitropy.

robin-nitrokey commented 3 months ago

This is a duplicate of: https://github.com/Nitrokey/pynitrokey/issues/510

We depend on spsdk >=2.0,<2.1, which in turn depends on cryptography>=3.4.4,<41.1. How did you install pynitrokey? cryptography 42.0.5 should not be selected as a compatible dependency.

sosthene-nitrokey commented 3 months ago

This appears to be an issue of the packaging on arch-linux.

dvzrv commented 3 months ago

This appears to be an issue of the packaging on arch-linux.

We actually ignore the dependency declarations for spsdk, as there is not much else we can do as downstream. (also upstream pretty much ignores external contributors and I have given up on interacting with them)

In pynitrokey 0.4.45 the dependencies are still different though: https://github.com/Nitrokey/pynitrokey/blob/960d4567e7c7f131425fc3eb900242be745d2305/pyproject.toml#L26-L34

I don't see a way for us to block upgrades to cryptography on something like spsdk (there are dozens of other dependents).

robin-nitrokey commented 3 months ago

I think this will be fixed by https://github.com/Nitrokey/pynitrokey/pull/499. @dvzrv Can you check if installing from the current master branch fixes the problem?

sosthene-nitrokey commented 3 months ago

spsdk 2.1 still has a requirement of cryptography <= 41

sosthene-nitrokey commented 3 months ago

Despite that it still seems to work for spsdk 2.1 and cryptography 42.0.5

sosthene-nitrokey commented 3 months ago

The way forward seems to be: make a new release of nitropy.

Then for the arch packaging: Update spsdk to 2.1 and package the new release.

Meanwhile the workarounds can be to downgrade cryptography

dvzrv commented 3 months ago

This unfortunately does not address pynitrokey not being compatible with cryptography >= 42 (due to the hazmat changes mentioned in my other ticket).

A new release does not really help us (unless it also incorporates changes that make pynitrokey compatible with the newer cryptography, e.g. via conditional imports).

robin-nitrokey commented 3 months ago

@dvzrv Do these hazmat imports really originate from pynitrokey? AFAIS, they were only in spsdk <= 2.0.0.

sosthene-nitrokey commented 3 months ago

= 42 (due to the hazmat changes mentioned in my other ticket).

Which changes? I tested with spsdk 2.1 and cryptography 42.0.5 and all appears to work, even though spskdk does not claim support for cryptography 42, in practice it appears to work.

sosthene-nitrokey commented 3 months ago

There are other uses of hazmat in nitropy itself, but they are for primitives that are still available with 42.0.5.

dvzrv commented 3 months ago

Ah, I did not investigate more deeply (currently afk). If the problematic use is only in spsdk, then yes a new release should fix this!

Thanks for looking into it so quickly!

dvzrv commented 3 months ago

I just tried to run tests for spsdk 2.0.1 (188 fail due to cryptographic primitives being elsewhere) and 2.1.0 (250 fail due to cryptographic primitives being elsewhere).

Upstream develops behind closed doors, so although there may be some fixes for cryptography >=42 somewhere, there is nothing for me to even cherry-pick or backport (releases are just single commit code-dumps changing the entire repository).

A month ago upstream hinted at some new version in https://github.com/nxp-mcuxpresso/spsdk/issues/64 but that yet has to materialize.

In the meantime I'll just do nothing, because there is nothing for me to do.

dvzrv commented 3 months ago

According to upstream a new release will be made on 2024-03-27 which supports cryptography 42: https://github.com/nxp-mcuxpresso/spsdk/issues/64#issuecomment-2009438030

This would be 2.1.1 and I'll try to upgrade to it in the hopes that it will be still compatible with pynitrokey.

sosthene-nitrokey commented 3 months ago

We're in the process of trying to vendor the parts of spsdk that we need in pynitrokey for a new release to avoid such issues in the future.

Since I based the vendored on the latest spsdk master branch I can see that it's not actually fully compatible already.

monogatron commented 3 months ago

@dvzrv I was wondering if you noticed that SPSDK v2.1.1 has been released?

dvzrv commented 3 months ago

@dvzrv I was wondering if you noticed that SPSDK v2.1.1 has been released?

Yes. Just been a tad busy the past days.

dvzrv commented 3 months ago

@monogatron: An updated spsdk is now in testing. please provide signoffs

monogatron commented 2 months ago

@dvzrv I was wondering if you noticed that SPSDK v2.1.1 has been released?

Yes. Just been a tad busy the past days.

@dvzrv: Yes, I can fully understand that the XZ story had top priority. The question wasn't meant to give the impression that I'm an annoyed user waiting for an update - just to explain a little more so that no misunderstandings arise. Thank you for your work and yours! :)

robin-nitrokey commented 2 months ago

This should be fixed when using up-to-date versions of pynitrokey and spsdk. Please open a new issue if you run into other compatibility problems.