closeio / flask-common

Some commonly shared code for Flask, MongoEngine, and Python apps
26 stars 4 forks source link

Swap out pycrypto #69

Closed jpmelos closed 5 years ago

jpmelos commented 5 years ago

Closes https://github.com/closeio/flask-common/issues/63

Technical details:

We were encrypting using IVs with the wrong size, and pycrypto was silently truncating our IVs for us. pycryptodome won't do that for us. As a result, we now have to have two versions of encrypted things: the old ones are not marked with a version and have wrong size IVs that we have to fix before pycryptodome can process them, and the new ones are marked with v2 at the start of the encrypted data string.

Then, we have to treat the edge case of an old data starting with v2, which will basically fail the decryption process because the IV with v2 stripped out won't match, and then we try with the IV with v2 in and in the old size.

There are tests to ensure we can seamlessly decrypt old and new encrypted data with these new functions.

The AES block size is always 16 bytes, as specified by NIST (source: https://en.wikipedia.org/wiki/Advanced_Encryption_Standard), so I also renamed some variables to more meaningfully convey what they are used for.

Why not the cryptography package as suggested in the issue?

Because of this issue https://github.com/pyca/cryptography/issues/4020 that has been reported several times (has several duplicates in that repo) and has never been fixed. I've tried several ways to fix this from all duplicates, and nothing worked in my environment, at least. Then, I decided to evaluate if we could fork and fix this ourselves, but I think that would take too long and I decided to look for a different alternative. I found pycryptodome, which is a fork, in-place replacement, of pycrypto, that is currently maintained.

Repo: https://github.com/Legrandin/pycryptodome (last commit 20 days ago as of now) Docs: https://pycryptodome.readthedocs.io/en/latest/

jpmelos commented 5 years ago

Tests passed for Python 2.7, not for the Python 3.x. Working on it.

tsx commented 5 years ago

What's actually wrong with cryprography?

mpessas commented 5 years ago

Where do we use these functions?

tsx commented 5 years ago

This is the primary (only?) usage https://github.com/closeio/flask-common/blob/master/flask_common/mongo/fields/crypto.py#L37-L43

jpmelos commented 5 years ago

This ended up decomposed in several PRs.