open-quantum-safe / liboqs-python

Python 3 bindings for liboqs
https://openquantumsafe.org/
MIT License
108 stars 39 forks source link

Refactor liboqs-python to project guidelines #6

Closed truth-quark closed 5 years ago

truth-quark commented 5 years ago

Partial refactor: moved some files into sub dirs and renamed them. A init.py has been added to allow core elements to be imported from the oqs package to make namespaces simpler.

These docs are being used to guide structuring the project: https://docs.python-guide.org/

christianpaquin commented 5 years ago

I'll take a look

truth-quark commented 5 years ago

I reviewed the changes in the diff tool; looks good. I tried to run the project but the REAMDE instructions don't work anymore. Can you also modify the README to explain how to run the lib/test/sample?

Ooops, that's fixed. Users just need to set the PYTHONPATH env variable & give the right file paths.

christianpaquin commented 5 years ago

@truth-quark, I tried the updated README instructions. When I use "export PYTHONPATH=" and try to run the example, I get a "No liboqs shared libraries found" error. When I add "/oqs" to the project path (location of the wrapper; shouldn't the README use that?), then I get a "ImportError: No module named 'oqs'" error. What am I missing?

truth-quark commented 5 years ago

Ok, for testing, I symlinked liboqs.so into the project root & ran tests/examples there. Is liboqs.so copied or symlinked into your liboqs-python/oqs dir when running the tests?

Here's what I did:

$ pwd
/home/ben/projects/liboqs-python
$ ls
examples  liboqs.so  LICENSE.txt  oqs  README.md  tests
$ export PYTHONPATH=`pwd`
$ 
$ echo $PYTHONPATH 
/home/ben/projects/liboqs-python
$ python3 tests/test_wrapper.py 
......
----------------------------------------------------------------------
Ran 6 tests in 2.513s

OK
$ python3 examples/example.py 
Enabled KEM mechanisms: DEFAULT, FrodoKEM-640-AES, FrodoKEM-640-cSHAKE, FrodoKEM-976-AES, FrodoKEM-976-cSHAKE, NewHope-512-CCA-KEM, NewHope-1024-CCA-KEM, Sidh-p503, Sidh-p751, Sike-p503, Sike-p751
Starting key encapsulation
{'name': 'Sike-p503', 'version': 'https://github.com/Microsoft/PQCrypto-SIDH/tree/77044b76181eb61c744ac8eb7ddc7a8fe72f6919', 'claimed_nist_level': 1, 'is_ind_cca': True, 'length_public_key': 378, 'length_secret_key': 434, 'length_ciphertext': 402, 'length_shared_secret': 16}
success: shared secrets are equal

Enabled signature mechanisms: DEFAULT, picnic_L1_FS, picnic_L1_UR, picnic_L3_FS, picnic_L3_UR, picnic_L5_FS, picnic_L5_UR, qTESLA_I, qTESLA_III_size, qTESLA_III_speed
Starting signature
{'name': 'qTESLA_I', 'version': 'https://github.com/qtesla/qTesla/commit/5e921da989b9b44aba95f63d9c28927d518f630c', 'claimed_nist_level': 1, 'is_euf_cma': True, 'length_public_key': 1504, 'length_secret_key': 2112, 'length_signature': 1376}
signature is valid
christianpaquin commented 5 years ago

Ok, I was missing the OQS lib in the new directory I created; oops. Anyway, works in my Linux env. I'm trying to set it up correctly in my Windows IDE, currently not finding the right files. Let me double check to see if we'd need an extra line in the README for that; then I'll merge.

truth-quark commented 5 years ago

I'm trying to set it up correctly in my Windows IDE, currently not finding the right files. Let me double check to see if we'd need an extra line in the README for that; then I'll merge.

Ok. Did you try the liboqs library path env var for windows at all?

christianpaquin commented 5 years ago

The LoadLibrary(path) call fails on Windows because liboqs.so should be oqs.dll or simply oqs. Is there a way to for LoadLibrary to automatically autocomplete the prefix lib and the suffix .so on Linux (I tried to simply using oqs on Linux but it failed)? If so, we can use that; if not, we’ll need two set the search string based on the platform.

truth-quark commented 5 years ago

Ooops, that's a bug. I don't have a Win system to try though. Does it resolve if wrapper.py at line 12 is tweaked to: LIBOQS_SHARED_OBJ = 'oqs.dll' if platform.system() == 'Windows' else 'liboqs.so'

christianpaquin commented 5 years ago

It should; I'll try it tomorrow.

christianpaquin commented 5 years ago

Works on Windows; you only need to specify 'oqs', the '.dll' is appended automatically.

truth-quark commented 5 years ago

Sweet, I'll fix it now.

truth-quark commented 5 years ago

@dstebila do you have a preference for the type of merge?

dstebila commented 5 years ago

I'm not sure what you mean by "type of merge".

truth-quark commented 5 years ago

Ah right, I was going to merge the code in the, but there's 3 options: merge commit, squash and merge etc. Just wanted to know if you have a preference for the project, or if you want to browse the changes beforehand?

I've got more refactoring changes based on this branch, and was hoping to push them up so others can have a look.

dstebila commented 5 years ago

Unless there's any reason to keep the history of development of this particular fix, a squash-and-merge is fine.

truth-quark commented 5 years ago

The history isn't needed here, so l'll squash merge soon.