def encrypt(salt, password, data):
^^^^^^
You're the crypto library. Don't ask the user to choose the salt, they won't do it right. key = _expand_key(salt, password)
Most crypto people would advise you to generate two separate keys for encryption and MAC. hmac = data[-HMAC_HASH.digest_size:]
hmac2 = HMAC.new(key, data[:-HMAC_HASH.digest_size], HMAC_HASH).digest()
if hmac != hmac2:
^^^^^^^^^^^^^
Possible timing attack. Consider doing another HMAC of each HMAC and comparing those instead. See https://www.isecpartners.com/news-events/news/2011/february/... offset = getrandbits(AES.block_size*8)
Counter.new(AES.block_size*8, initial_value=offset, allow_wraparound=True)
I don't like that you allow wraparound counters, in fact, I'd go so far as to call it broken. This is not allowed by NIST-specified CTR mode.Bad Things^TM will happen if multiple messages (or different parts of a single message) are ever encrypted with the same key. Seriously, screwups like this have shortened wars.
return prefix + encrypted + hmac
Put some type of magic constant in front of this. You'll be glad you did later when you have to change the format or simply the PBKDF2 count. HMAC_HASH = SHA256
but later...
def _expand_key(salt, password):
...
return PBKDF2(....)I don't see where you're telling PBKDF2 what hash to use for its HMAC. It's probably defaulting to something like SHA-1.