Encryption is Vulnerable to Adaptive Chosen-Ciphertext Attacks

jrnl-org / jrnl

Collect your thoughts and notes without leaving the command line.

https://jrnl.sh

GNU General Public License v3.0

6.51k stars 529 forks source link

Encryption is Vulnerable to Adaptive Chosen-Ciphertext Attacks #549

Closed paragonie-scott closed 5 years ago

paragonie-scott commented 6 years ago

https://github.com/maebert/jrnl/blob/a2462ed7c36d8a4eed0bdf257703632abf21d441/jrnl/Journal.py#L43-L79

AES-CBC without HMAC (in Encrypt-then-MAC mode, with the MAC covering the IV and ciphertext, with a secure comparison mode; encryption is hard!) is vulnerable to padding oracle attacks which allows for decryption without access to the key.

Recommendation:

Encryption: Calculate HMAC-SHA256 over iv || ciphertext (where || means concatenate), prepend or append it to the file
Decryption: Recalculate the HMAC-SHA256 over the extracted IV and ciphertext, then compare it with the stored HMAC output, using hmac.compare_digest()

gautamk commented 6 years ago

My understanding is that moving to an authenticated encryption mode could solve this issue ? AES GCM ?

paragonie-scott commented 6 years ago

AES-GCM would work, provided you have a processor with the PCLMULQDQ instruction available to compute GHASH in constant-time.

Further reading: https://eprint.iacr.org/2013/157.pdf

AES-256-CBC + HMAC-SHA256 is secure even on older hardware, provided:

The IV is generated from a secure RNG
- IVs have to be unpredictable and unique, not just unique
The HMAC is computed over the IV and ciphertext
- Otherwise, you're giving attackers free control over the first 16 bytes of the plaintext
The HMAC is compared in constant-time
- Otherwise, you can use the time it takes for decryption to fail to produce a valid MAC for a chosen ciphertext, which reintroduces the padding oracle vulnerability we're trying to side-step

However, if you have a solid AES-GCM implementation available in Python, and you're fairly certain that your users have AES-NI and PCLMULQDQ available, that's definitely my recommendation.

maebert commented 6 years ago

Hey @paragonie-scott , thanks for pointing this out. We're changing the encryption method in 2.0, could you have a look at https://github.com/maebert/jrnl/blob/2.0-wm/jrnl/EncryptedJournal.py#L29 - I'd really appreciate your feedback on that!

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

wren commented 5 years ago

@paragonie-scott Hi! Thanks for pointing this out. The project is just getting back and up and running with new maintainers. It looks like the encryption method was changed since you filed this. Can you confirm that this is still an issue, or has it been resolved?

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

paragonie-scott commented 5 years ago

Wow, I somehow missed the email/notification on July 30!

Yes, moving to Fernet resolves this issue.

wren commented 5 years ago

@paragonie-scott Thanks for reporting back!

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.