-
Notifications
You must be signed in to change notification settings - Fork 29
Description
- You're using the sha256 hash of a user-supplied password as a key
Line 13 in 785289b
self.key = hashlib.sha256(key).digest() #turns the password into a 32char long key
This is bad because:
- sha256 is a fast hash, meaning an attacker can brute-force this algo quicker
You should use instead:
- argon2i, scrypt, bcrypt or pbkdf2
- you're using
Random.newfor the IV
Line 28 in 785289b
iv = Random.new().read(AES.block_size)
This is bad because:
- Random isn't a CSPRNG, and uses a predictable MT algo.
- Each fresh invocation of this script will generate the same IVs
You should use instead:
import os
iv = os.urandom(AES.block_size)This uses the OS's CSPRNG (/dev/urandom on Linux, CryptGenRandom() on Windows)
- You have no MAC on the encryption
This is bad because:
- Without a MAC, an attacker can manipulate your crypto (doubly true as you're using CBC)
- Without a MAC you can't ensure that the data you upload is the data you download (both from authentication reasons and data corruption reasons)
You should use instead:
- Since PyCrypto 2.6.1 lacks any AEAD AES types (Basically there's no AES_GCM), you'll have to use HMAC.new from Crypto.Hash import HMAC
- Make sure you encrypt-then-mac
- Make sure you check the MAC before you decrypt anything; discard any encrypted data that fails the MAC check (and do this in a timing-safe way)
Summary:
It looks like the crypto code is copied from various places. This is fine if you have enough crypto-know-how to separate the wheat from the chaff, but in cases where people don't, they make bad choices like the above.
You can fix all of these in a really simple manner:
Use pynacl: https://pynacl.readthedocs.io/en/stable/
Remember: even if you don't maintain it anymore, crypto code that is broken that other people can use means that other people will assume it's fine to use, meaning more projects have insecure python cryptography.