pyca/pynacl

Fix the order in which ffi.cdef() is called for the included header files

wbolster opened this issue · 5 comments

Currently PyNaCl obtains a list of header files using glob.glob(), then calls ffi.cdef() for each header file. This breaks horribly at runtime if the order of glob() is different from what it was at build time (which may happen with some file systems, or after restoring a backup), resulting in very cryptic errors at runtime, even though the build/install step succeeded just fine:

python -m nacl.public
Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/home/uws/.virtualenvs/38aee272d0994fde/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/public.py", line 17, in <module>
    import nacl.c
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/c/__init__.py", line 16, in <module>
    from nacl.c.crypto_box import (
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/c/crypto_box.py", line 23, in <module>
    crypto_box_SECRETKEYBYTES = lib.crypto_box_secretkeybytes()
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/_lib/__init__.py", line 76, in __getattr__
    self._lib = self.ffi.verifier.load_library()
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/cffi-0.7.2-py2.7-linux-x86_64.egg/cffi/verifier.py", line 68, in load_library
    self.compile_module()
  File "/home/uws/.virtualenvs/38aee272d0994fde/local/lib/python2.7/site-packages/PyNaCl-0.2.3-py2.7-linux-x86_64.egg/nacl/_lib/__init__.py", line 71, in _compile_module
    raise RuntimeError("Cannot compile module during runtime")
RuntimeError: Cannot compile module during runtime

Under the hood CFFI generates file names like _cffi__xd64d2119xefb54d7c.so at compile time, and tries to load a different file at runtime. The reason for this is that the CRC32 part of the file name is based on (among other things) a concatenation of the strings passed fo ffi.cdef(), in the order it was called.

The fix is to always .cdef() the headers in the same order.

Is there anything I can do to help get this issue resolved?

lvh commented

LGTM. Unfortunate that there's no test coverage, but at least it's documented there...

@lvh, thanks.

Fwiw, I think things like these are hard to test. In practice the glob() order will remain the same as long as the directory is not touched (e.g. during a test run). It's when you have a home dir on NFS, or after you have restored a backup, that the problem manifests. Situations like these are outside the scope of unit tests, imho.

lvh commented

Unit tests probably shouldn't touch filesystems in the first place; I'm thinking more of a fake glob.glob that returns all the same files in two different orders.

lvh commented

(In this case, that doesn't really mean you end up not touching the filesystem, I suppose, but you get the idea: instead of trying to coerce the filesystem to produce different orders for glob.glob within a test run, you stub it out and guarantee that it does. Another reason why FilePath is better than everything in the stdlib :))