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

Possibility to have optional nitrokey 3 support #162

Closed
dvzrv opened this issue Jan 21, 2022 · 12 comments
Closed

Possibility to have optional nitrokey 3 support #162

dvzrv opened this issue Jan 21, 2022 · 12 comments
Labels
device/Nitrokey 3 enhancement New feature or request

Comments

@dvzrv
Copy link

dvzrv commented Jan 21, 2022

Hi! When upgrading to 0.4.10 I realized (unfortunately only afterwards), that you added a new dependency (spsdk) in bd5b91a#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711.
The package on Arch Linux is currently broken.

The spsdk dependency drags in around two dozens of other packages, which will take me probably two weeks or more to get all packaged, as some of them rely on outdated and conflicting dependencies, have missing sources, or missing licenses (the list will probably go on like that for a while).

Would it be possible to make the nitrokey3 support optional for the nitropy CLI? The spsdk package unfortunately drags in a fairly large ecosystem of stuff.

@szszszsz
Copy link
Member

szszszsz commented Jan 22, 2022

Hi! Thank you for your work!

Sure, we will take a look into it to at least ease the transition. Another thing is that Nitrokey 3 support might not be relevant to all users as well, so a default smaller package might be even better.

Connected:

@szszszsz szszszsz added device/Nitrokey 3 enhancement New feature or request labels Jan 22, 2022
@robin-nitrokey
Copy link
Member

Unfortunately, it is not possible to set default extras in Python, so this would mean that users would not install the nk3 feature by default but would have to select it manually. I would prefer the nk3 command being part of the default installation.

@dvzrv Would it be possible for you to just remove the nk3 parts in the Arch package? This should be possible with a rather small patch (remove pynitrokey/nk3 and pynitrokey/cli/nk3and two lines in pynitrokey/cli/__init__.py).

@szszszsz
Copy link
Member

szszszsz commented Jan 22, 2022

@robin-nitrokey

  1. Can we interface with Pip to ask it for downloading additional package?
  2. This should not be a problem for users, as update is not an often operation. In my idea we could test for the spsdk package availability, and suggest to install the spsdk variant of pynitrokey when needed.
  3. Regarding patch, we can do it on our side already, as suggested in point 2.

Edit: I admit though with this we are entering into hidden dependencies territory

@dvzrv
Copy link
Author

dvzrv commented Jan 22, 2022

Unfortunately, it is not possible to set default extras in Python, so this would mean that users would not install the nk3 feature by default but would have to select it manually. I would prefer the nk3 command being part of the default installation.

Hm, yes, that's the case.

We have quite a few optional dependency packages for python packages though. In system package management the situation is similar in regards to visibility and way of installing (there is no "default optional", either it is a requirement or it is an optional dependency - sometimes also referred to as "suggests" in other package management systems), but also reflected in the metadata of a package, which is easily reviewed via our website or the package manager (pacman) itself. So in general we provide the python package's optional dependency as optional dependency in the system package.

@dvzrv Would it be possible for you to just remove the nk3 parts in the Arch package? This should be possible with a rather small patch (remove pynitrokey/nk3 and pynitrokey/cli/nk3and two lines in pynitrokey/cli/__init__.py).

I can do that as well. Definitely the "working solution" for now.

@robin-nitrokey
Copy link
Member

We have quite a few optional dependency packages for python packages though.

Just to be clear, my concern was the default installation with pip. Distributions are of course free to use their own package layout and default features.

This should not be a problem for users, as update is not an often operation. In my idea we could test for the spsdk package availability, and suggest to install the spsdk variant of pynitrokey when needed.

Yeah, we can do that. I can prepare a patch for further discussion. I think it is just contrary to our general approach of making things as easy as possible for users.

@dvzrv
Copy link
Author

dvzrv commented Jan 22, 2022

I've now used a fairly small patch to make this work on my side (for 0.4.13) until spsdk is packaged:

diff --git i/pynitrokey/cli/__init__.py w/pynitrokey/cli/__init__.py
index 3dd8ca3..a22ff1f 100644
--- i/pynitrokey/cli/__init__.py
+++ w/pynitrokey/cli/__init__.py
@@ -17,7 +17,6 @@ import pynitrokey
 import pynitrokey.fido2.operations
 from pynitrokey.cli.fido2 import fido2
 from pynitrokey.cli.nethsm import nethsm
-from pynitrokey.cli.nk3 import nk3
 from pynitrokey.cli.pro import pro
 from pynitrokey.cli.start import start
 from pynitrokey.cli.storage import storage
@@ -57,7 +56,6 @@ def nitropy():

 nitropy.add_command(fido2)
 nitropy.add_command(nethsm)
-nitropy.add_command(nk3)
 nitropy.add_command(start)
 nitropy.add_command(storage)
 nitropy.add_command(pro)
@@ -78,7 +76,6 @@ def ls():

     fido2.commands["list"].callback()
     start.commands["list"].callback()
-    nk3.commands["list"].callback()


 nitropy.add_command(ls)
diff --git i/pyproject.toml w/pyproject.toml
index 8c69386..c61c206 100644
--- i/pyproject.toml
+++ w/pyproject.toml
@@ -21,7 +21,6 @@ requires = [
   "requests",
   "pygments",
   "python-dateutil",
-  "spsdk >= 1.5.0",
   "tqdm",
   "urllib3",
        "cffi",

(btw: you have some tab poisoning in the pyproject.toml file)

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Jan 22, 2022
Remove nitrokey 3 support until python-spsdk is packaged.
Nitrokey/pynitrokey#162

git-svn-id: file:///srv/repos/svn-community/svn@1113423 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Jan 22, 2022
Remove nitrokey 3 support until python-spsdk is packaged.
Nitrokey/pynitrokey#162

git-svn-id: file:///srv/repos/svn-community/svn@1113423 9fca08f4-af9d-4005-b8df-a31f2cc04f65
robin-nitrokey added a commit to robin-nitrokey/pynitrokey that referenced this issue Jan 22, 2022
This patch makes the nk3 command optional by creating a nk3 extra.  This
makes it possible to package and install pynitrokey without the spsdk
dependency.

Fixes Nitrokey#162
robin-nitrokey added a commit to robin-nitrokey/pynitrokey that referenced this issue Jan 22, 2022
This patch makes the nk3 command optional by creating a nk3 extra.  This
makes it possible to package and install pynitrokey without the spsdk
dependency.

Fixes Nitrokey#162
robin-nitrokey added a commit to robin-nitrokey/pynitrokey that referenced this issue Jan 22, 2022
This patch makes the nk3 command optional by creating a nk3 extra.  This
makes it possible to package and install pynitrokey without the spsdk
dependency.

Fixes Nitrokey#162
@dvzrv
Copy link
Author

dvzrv commented Feb 12, 2022

Just as a follow-up: I have now managed to at least already get pyocd packaged and am working towards packaging spsdk as well.

However, I am seeing some issues with their use of prebuilt shared libraries (via pypemicro and libusbsio). Is the functionality provided by libusbsio in use for nitrokey3? I am asking because I will remove it, if I can not build it from source.

@dvzrv
Copy link
Author

dvzrv commented Feb 13, 2022

Digging further into spsdk revealed that larger parts of it rely on libusbsio (which vendors prebuilt shared libraries for USB discovery). The removal of libusbsio is quite invasive and I am at this point not sure whether this will leave spsdk in a functioning state (in any case this is not a feasible way of working with an upstream and I am absolutely not happy about that situation).

Additionally, the list of dependencies is pinned to sometimes quite outdated versions and tests do not succeed with newer versions of those dependencies.

I'll add spsdk to the repos in testing and let people figure out whether it is at all usable.

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Feb 13, 2022
…pends.

Python-spsdk is required for nitrokey3 support.
Unfortunately the spsdk project is problematic in regards to packaging:
Nitrokey/pynitrokey#162
Problems might occur due to required extensive devendoring.

git-svn-id: file:///srv/repos/svn-community/svn@1133102 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Feb 13, 2022
…pends.

Python-spsdk is required for nitrokey3 support.
Unfortunately the spsdk project is problematic in regards to packaging:
Nitrokey/pynitrokey#162
Problems might occur due to required extensive devendoring.

git-svn-id: file:///srv/repos/svn-community/svn@1133102 9fca08f4-af9d-4005-b8df-a31f2cc04f65
@robin-nitrokey
Copy link
Member

Thanks @dvzrv for working on this! As you already saw, libusbsio is unfortunately one of the few spsdk dependencies that we actually need. Without it, we cannot communicate with the nk3 bootloader so especially the nk3 update command does not work. There are two possible short-term workarounds:

In the medium term, we might replace spsdk with a lpc55 wrapper but there is no schedule for that yet.

@dvzrv
Copy link
Author

dvzrv commented Feb 14, 2022

As you already saw, libusbsio is unfortunately one of the few spsdk dependencies that we actually need. Without it, we cannot communicate with the nk3 bootloader so especially the nk3 update command does not work.

That is unfortunate and currently I wish I had looked into libusbsio first as it is a show stopper (for distributing it as a binary package and even for distributing it as an sdist tarball for pypi.org).

There are two possible short-term workarounds:

If those are viable options I'd be happy to test them (I just don't yet have a nitrokey 3 to test with).

In the medium term, we might replace spsdk with a lpc55 wrapper but there is no schedule for that yet.

That would be very much appreciated (rather sooner than later). Meanwhile I will pull the plug on spsdk as a package for Arch Linux, as the vendored shared libraries for libusbsio are full on proprietary and would make me liable for prosecution if I re-distributed them (see respective comment on the upstream ticket).

@dvzrv
Copy link
Author

dvzrv commented Feb 17, 2022

FWIW, in case you see a future in using spsdk for your toolchain, I would urge you to get in contact with NXP (see nxp-mcuxpresso/spsdk#36 (comment)) to either switch to using libusb or providing open sources for libusbsio. As it stands their tooling is unfortunately not trustworthy or auditable which (IMHO) is a problem for users of nitrokey 3 as well. I am of the impression that raising concerns with them might have a better chance of success if they are coming from an industry partner and not only from a software distributor.

@dvzrv
Copy link
Author

dvzrv commented Mar 9, 2022

The upstream issue has now been resolved, because NXP made libusbsio sources available under the terms of the BSD-3-clause and I have been able to create a package from it for Arch Linux.

I think this ticket can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
device/Nitrokey 3 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants