Replace unsafe ed25519 from security-critical functions with cryptography #59
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I removed the unsafe ed25519 implementation from critical functions: the unsafe is still in use when only public information is involved (e.g. verification) as this is not a security problem but still very slow (the unsafe should be completely removed on the long term). PyCA's cryptography is used when a private key gets involved to ensure security. https://github.com/pyca/cryptography is well maintained. So, critical security issues are solved. Still, some things remain to be done: the unsafe ed25519 (see hyperledger#55) implementation should be completely removed to fully depend on cryptography from PyCA. The latter is faster and remains maintained if imported. The unsafe Ed25519 is also from PyCA (which marked it to be not safe and for testing use only) but was copied to the iroha-python repo, assuming it to be no longer maintained in there. Also, I suggest to remove Ed25519-related functions from IrohaCrypto class in order to use the cryptography module's classes directly. That's faster and easier to maintain on the long term, avoiding many unnecessary functions, serializations, ... in between.
Further, I have removed os.urandom, which can output cryptographic-level entropy but does not implement blocking. So, if not sufficient entropy is available, os.urandom would not be aware of it. This could be a problem in virtual machine use. Ed25519 private key creation uses entropy from OpenSSL (will be imported with cryotography using low-level bindings).
Therefore, it will be necessary to adjust the wheel file for pip, too: it needs to include cryptography as dependency.
I have tested all changed functions/classes on Fedora 32, including tests against the Iroha-Python usage example (https://github.com/hyperledger/iroha-python/blob/master/README.md) which runs all changed functions (private_key, derive_public_key, _signatur (the latter is covered indirectly through running sign_transaction)).
Still, to be further reviewed to ensure all-encompassing compatibility!
Unused module/variable from #58 solved