spsdk icon indicating copy to clipboard operation
spsdk copied to clipboard

Make use of pypemicro optional or remove dependency on it altogether

Open dvzrv opened this issue 3 years ago • 15 comments

Hi! As raised in https://github.com/NXPmicro/pypemicro/issues/10 I believe that the origins of the shared libraries provided by the pypemicro package are not trustworthy enough to be shipped to users.

Please consider removing this dependency altogether or at least making it an optional dependency for this project.

From a packaging perspective I do not feel comfortable packaging spsdk with it depending on pypemicro and would remove its use from the source code if necessary, until legal claims and claims of origin are resolved for the shared libraries in question.

dvzrv avatar Jan 28 '22 17:01 dvzrv

@saper @mstarecek @mwsk sorry for pinging you here as well. The issue in https://github.com/NXPmicro/pypemicro/issues/10 is still unresolved, which means that packaging pypemicro somwhere downstream is still legally very questionable and from a security perspective also quite dubious.

It would be very good if NXP could address this issue.

dvzrv avatar Sep 24 '22 11:09 dvzrv

That is very kind of you to tag me here but I don't have the powers to do anything about it. Still running a locally modified version of https://github.com/NXPmicro/spsdk/releases/tag/0.2.2, wonder how much it will take to update - so I can't even battle-test it.

saper avatar Sep 24 '22 12:09 saper

That is very kind of you to tag me here but I don't have the powers to do anything about it.

My bad! I was under the impression that you were somehow affiliated, having contributed to the repository!

Still running a locally modified version of https://github.com/NXPmicro/spsdk/releases/tag/0.2.2, wonder how much it will take to update

ouf...

dvzrv avatar Sep 24 '22 12:09 dvzrv

Don't we have the same problem with https://pypi.org/project/pylink-square/ ?

Maybe those imports should be wrapped in try: ... except ImportError: and remove them from requirements.txt?

J-Link works with OpenOCD so one can mostly work it around, but I don't think OpenOCD can talk to pemicro stuff.

saper avatar Sep 24 '22 15:09 saper

Don't we have the same problem with https://pypi.org/project/pylink-square/ ?

Why? Does it also bundle shared libraries with obfuscated code from unknown origin? ;-)

Maybe those imports should be wrapped in try: ... except ImportError: and remove them from requirements.txt?

J-Link works with OpenOCD so one can mostly work it around, but I don't think OpenOCD can talk to pemicro stuff.

That's largely what I'm doing for the downstream package: https://github.com/archlinux/svntogit-community/blob/19938f5cab9adf93da26c09ebeb8111ed1bdc59b/trunk/python-spsdk-1.6.0-remove_pypemicro.patch

But given that no pull requests in this repository ever get merged I don't think providing one would make much sense for me (also I don't know what the maintainers think about this change either).

dvzrv avatar Sep 24 '22 16:09 dvzrv

Don't we have the same problem with https://pypi.org/project/pylink-square/ ?

Why? Does it also bundle shared libraries with obfuscated code from unknown origin? ;-)

https://pypi.org/project/pylink-square/

Python interface for the SEGGER J-Link.

Requirements

maybe it works for you... *

saper avatar Sep 24 '22 16:09 saper

(nxp) m.saper.info> /usr/local/bin/virtualenv-3.8 ~/nxp                                                                                          
Using base prefix '/usr/local'
New python executable in /home/saper/nxp/bin/python3.8
Also creating executable in /home/saper/nxp/bin/python
Installing setuptools, pip, wheel...
done.
(nxp) m.saper.info> pip install pylink-square
Collecting pylink-square
  Downloading pylink_square-0.14.2-py2.py3-none-any.whl (82 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 82.3/82.3 kB 6.1 MB/s eta 0:00:00
Collecting psutil>=5.2.2
  Downloading psutil-5.9.2.tar.gz (479 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 479.8/479.8 kB 6.5 MB/s eta 0:00:00
  Preparing metadata (setup.py) ... done
Collecting six
  Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Collecting future
  Using cached future-0.18.2.tar.gz (829 kB)
  Preparing metadata (setup.py) ... done
Building wheels for collected packages: psutil, future
  Building wheel for psutil (setup.py) ... done
  Created wheel for psutil: filename=psutil-5.9.2-cp38-cp38-freebsd_13_0_release_p11_amd64.whl size=247198 sha256=7d2c122b11af7f441bd1f7265cea95c034cdcd1f893efa77fc1fa7b861cee0ea
  Stored in directory: /home/saper/.cache/pip/wheels/c3/d9/d7/b55aee405934fcc854a0d75456d1116bd4a848bdda0a07df46
  Building wheel for future (setup.py) ... done
  Created wheel for future: filename=future-0.18.2-py3-none-any.whl size=491058 sha256=54a49a1f9fc3561275a181db69c960fa7b9979a946a619b965b360c9587549b5
  Stored in directory: /home/saper/.cache/pip/wheels/8e/70/28/3d6ccd6e315f65f245da085482a2e1c7d14b90b30f239e2cf4
Successfully built psutil future
Installing collected packages: six, psutil, future, pylink-square
Successfully installed future-0.18.2 psutil-5.9.2 pylink-square-0.14.2 six-1.16.0
(nxp) m.saper.info> pip install pypemicro    
Collecting pypemicro
  Using cached pypemicro-0.1.9-py3-none-any.whl (3.7 MB)
Installing collected packages: pypemicro
Successfully installed pypemicro-0.1.9
(nxp) m.saper.info> python
Python 3.8.13 (default, May 10 2022, 01:15:55) 
[Clang 11.0.1 ([email protected]:llvm/llvm-project.git llvmorg-11.0.1-0-g43ff75f2c on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> import pylink
>>> 
(nxp) m.saper.info> python
Python 3.8.13 (default, May 10 2022, 01:15:55) 
[Clang 11.0.1 ([email protected]:llvm/llvm-project.git llvmorg-11.0.1-0-g43ff75f2c on freebsd13
Type "help", "copyright", "credits" or "license" for more information.
>>> import pylink
>>> p = pylink.JLink()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/saper/nxp/lib/python3.8/site-packages/pylink/jlink.py", line 295, in __init__
    raise TypeError('Expected to be given a valid DLL.')
TypeError: Expected to be given a valid DLL.
>>> import pypemicro
>>> dir(pypemicro)
['Any', 'CDLL', 'IntEnum', 'PEMicroArmRegisters', 'PEMicroException', 'PEMicroInterfaces', 'PEMicroMemoryAccessResults', 'PEMicroMemoryAccessSize', 'PEMicroPortType', 'PEMicroSpecialFeatures', 'PEMicroSpecialFeaturesSwdStatus', 'PEMicroTransferException', 'PyPemicro', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', 'byref', 'c_bool', 'c_byte', 'c_char_p', 'c_ulong', 'c_ushort', 'c_void_p', 'cdll', 'logger', 'logging', 'os', 'pemicro', 'pemicro_const', 'platform', 'sys']
>>> c = pypemicro.PyPemicro()
Traceback (most recent call last):
  File "/home/saper/nxp/lib/python3.8/site-packages/pypemicro/pemicro.py", line 446, in get_pemicro_lib
    raise PEMicroException("There is any suitable library for this OS.")
pypemicro.pemicro.PEMicroException: There is any suitable library for this OS.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/saper/nxp/lib/python3.8/site-packages/pypemicro/pemicro.py", line 537, in __init__
    self.lib = PyPemicro.get_pemicro_lib(dll_path)
  File "/home/saper/nxp/lib/python3.8/site-packages/pypemicro/pemicro.py", line 452, in get_pemicro_lib
    raise PEMicroException(str(exc))
pypemicro.pemicro.PEMicroException: There is any suitable library for this OS.
>>> 

saper avatar Sep 24 '22 16:09 saper

But good news is, those packages do install, they just don't work - I don't see a problem here.

saper avatar Sep 24 '22 16:09 saper

pylink-square installs just fine. If people then require some custom additional non-free blob, that is not my problem as a system's packager. However, if you think that this is problematic it would probably be good to bring this up with the upstream. E.g. the SEGGER downloads are only available behind a EULA.

With pypemicro it is more problematic, as the python package itself vendors a shared library of unknown origin and license (see https://github.com/NXPmicro/pypemicro/issues/10). This is nothing I can even package/redistribute legally as there is no proof whatsoever that it would be allowed.

dvzrv avatar Sep 24 '22 16:09 dvzrv

I see what you mean:

> find nxp/lib/python3.8/site-packages/pypemicro -type f
nxp/lib/python3.8/site-packages/pypemicro/__init__.py
nxp/lib/python3.8/site-packages/pypemicro/pemicro_const.py
nxp/lib/python3.8/site-packages/pypemicro/libs/Linux/unitacmp-64.so
nxp/lib/python3.8/site-packages/pypemicro/libs/MacOS/unitacmp-64.dylib
nxp/lib/python3.8/site-packages/pypemicro/libs/MacOS/libusb.dylib
nxp/lib/python3.8/site-packages/pypemicro/libs/Windows/unitacmp-32.dll
nxp/lib/python3.8/site-packages/pypemicro/libs/Windows/unitacmp-64.dll
nxp/lib/python3.8/site-packages/pypemicro/__pycache__/pemicro_const.cpython-38.pyc
nxp/lib/python3.8/site-packages/pypemicro/__pycache__/pemicro.cpython-38.pyc
nxp/lib/python3.8/site-packages/pypemicro/__pycache__/__init__.cpython-38.pyc
nxp/lib/python3.8/site-packages/pypemicro/pemicro.py

I'd say this is a distribution policy problem. But if you want to disallow this, you have to patch. Maybe maintainers would be open to make both dependencies optional, i.e. removed from requirements.txt and wrapped with ImportError catch.

saper avatar Sep 24 '22 16:09 saper

I'd say this is a distribution policy problem.

I'd go as far as saying that that is a supply chain nightmare. Spsdk is pulled in for projects such as pynitrokey which is used to interact with security tokens that store private key information. There should be no unknown shared lib involved, that can not be accounted for at all.

I was very happy to see that the maintainers changed their mind in regards to libusbsio (see https://github.com/NXPmicro/spsdk/issues/36).

dvzrv avatar Sep 24 '22 17:09 dvzrv

When I was packaging it for myself for Gentoo as a dependency of pynitrokey I decided to delete the PEmicro code as I won't need it. There are two other choices of debug probes (any pyOCD probe, or JLink/pyLink) which are either fully open source or (pyLink) have the proprietary shared library as an optional part the user can install manually. https://github.com/jjakob/gentoo-overlay/blob/master/dev-python/spsdk/files/02-Delete-PEMicro-due-to-dependency-on-non-free-blobs.patch

jjakob avatar May 14 '23 20:05 jjakob

Note that since 2.0.0 the patch has to be modified a bit to not modify spsdk/debuggers/__init__.py any more:

diff -ruN a/spsdk/debuggers/utils.py b/spsdk/debuggers/utils.py
--- a/spsdk/debuggers/utils.py  2022-02-04 14:27:29.000000000 +0100
+++ b/spsdk/debuggers/utils.py  2022-02-14 00:05:11.017196467 +0100
@@ -15,7 +15,6 @@
 from spsdk import SPSDKError
 from spsdk.debuggers.debug_probe import DebugProbe, SPSDKDebugProbeError, SPSDKProbeNotFoundError
 from spsdk.debuggers.debug_probe_jlink import DebugProbePyLink
-from spsdk.debuggers.debug_probe_pemicro import DebugProbePemicro
 
 # Import all supported debug probe classes
 from spsdk.debuggers.debug_probe_pyocd import DebugProbePyOCD
@@ -23,7 +22,6 @@
 PROBES = {
     "pyocd": DebugProbePyOCD,
     "jlink": DebugProbePyLink,
-    "pemicro": DebugProbePemicro,
 }
 
 logger = logging.getLogger(__name__)

All tests pass successfully with this.

Note that pynitrokey isn't yet compatible with spsdk 2.0.0.

PureTryOut avatar Dec 08 '23 10:12 PureTryOut

@PureTryOut thanks for the heads up! A new version of pynitrokey supporting spsdk > 2, <2.1 has just been released and adapting the patch comes in handy. :bow:

dvzrv avatar Mar 20 '24 11:03 dvzrv

And with 2.1.0 the patch yet again would have to address spsdk/debuggers/__init__.py.

I'm less and less inclined wanting to package any of this stuff.. :roll_eyes:

dvzrv avatar Mar 20 '24 11:03 dvzrv

pypemicro is no longer in requirements.txt

mwsk avatar Jun 17 '24 08:06 mwsk

This lib still has a dependency on pyocd-pemicro which depends on pypemicro

frogamic avatar Jun 20 '24 07:06 frogamic

So do you want to reopen this issue?

mwsk avatar Jun 25 '24 06:06 mwsk