How to correctly implement an archetypal encrypt-then-MAC AES-CTR + HMAC mode cipher? I've created an AES implementation in Python as a learning experience (mainly for encrypting/decrypting files), and wanted to make sure that I haven't made any huge mistakes in my logic (of course, implementation is another story).
The block cipher mode of operation is CTR. The implementation supports AES-128, AES-192, and AES-256 (default).
The AES key and HMAC key are generated from a user password using hashlib's pbkdf2_hmac with SHA-256, and a random 16-byte salt (I create a 64-byte key and split it in half for each key; for AES-128, I create a 32-byte key and split it in half).
The CTR IV is created by a random 8 byte nonce and an 8 byte counter that starts at 0. The salt is written as the first block of the ciphertext, followed by the CTR IV as the second block.
Lastly, a 32-byte HMAC value of the ciphertext is created using the HMAC key and SHA-256 and written as the last two blocks of the ciphertext (32 bytes).
When decrypting, the HMAC value is checked against the rest of the ciphertext before decrypting.
Is there any issues with this logic? From a cryptographical point, where are the weak points?
Thank you!
You are describing encrypt-then-MAC using AES-CTR for encryption and HMAC for the MAC. This indeed results in authenticated encryption.
There may be better choices for the key derivation function, but I'm not very familiar with the options, so I'll let others comment. I don't understand why you salt the KDF when deriving the key. This just means that you will need to re-key for each ciphertext. In some ways, re-keying is better for security (maybe not here with low-entropy passwords seeding the key), but usually people like to avoid re-keying for the sake of performance.
The IV is only $8 \text{ bytes} = 64 \text{ bits}$, so you will start to expect collisions after encrypting $2^{32} =$ a few billion ciphertexts. This doesn't seem very good to me. You are devoting a full $8 \text{ bytes} = 64 \text{ bits }$ to the counter. Do you really need to support ciphertexts that are $2^{64}$ blocks long? A better balance between IV+counter  length would be preferable. For example, with a 12-byte IV and 4-byte counter, you avoid IV collisions up to 2^48 and can support ciphertexts that are $2^{32}$ blocks long ( $2^{32} \times 128 \text { bits } = 0.5 \text{ terabits} = 64 \text{ gigabytes} ).$