Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cryptography problems #40

Open
vngzs opened this issue Mar 6, 2023 · 1 comment
Open

Cryptography problems #40

vngzs opened this issue Mar 6, 2023 · 1 comment

Comments

@vngzs
Copy link

vngzs commented Mar 6, 2023

Hello aes-everywhere folks;

I did a quick review of the code in this repo to determine its suitability for some sensitive applications, where the security guarantees provided by the encryption are mission-critical.

I strive to be friendly in my disclosures; I think this is a noble effort. Unfortunately, certain design choices in this repository make the encryption unsuitable for sensitive applications. Among them:

  1. md5 is not suitable for a key derivation function. If you look at Latacora's Cryptographic Right Answers, they offer the following guidance on password handling:

    In order of preference, use scrypt, argon2, bcrypt, and then if nothing else is available PBKDF2.

    For a key derivation function to be secure, it should use a standard algorithm with security rooted in a cryptograhpically secure pseudorandom function. I recommend following Latacora's guidance here, and selecting a derivation function from one of their listed options.

    It's unclear if the key derivation algorithm is standard or homerolled. In any case, it's best to move to a modern, standard key derivation function rooted in a non-broken pseudorandom function if you must support password-based key derivation.

  2. This library implements pkcs7 padding, but it doesn't authenticate the encrypted/padded data. This means adversaries can play games with the padding used. If you look at the Cryptopals post on exploiting CBC padding oracles, they explain how unauthenticated padding can be attacked to reveal secret information.

    To fix this, you should authenticate the padding information in addition to encrypting the secret information. There are two ways to go about doing this. The older way (a) is to add a MAC around the entire data packet, thereby ensuring that attackers who do not possess the secret key cannot modify the padding. The more modern approach (b) is to use what's called an AEAD construction. The common implementation here is NaCl secretbox, which has implementations in many languages (e.g., Go). The idea here is that rather than supporting MAC and Encrypt as separate functions that must be nested in a particular order, AEADs combine encryption and authentication into one function that does everything in the right order. This averts some notable issues with padding and encryption separately, summarized in Moxie Marlinspike's cryptographic doom principle.

I think these are important problems that undermine the security guarantees aes-everywhere is intended to provide. It's great that you've done all this implementation work bringing cross-platform, multi-language cryptography to the masses, but I think the cryptography choices could use some re-evaluation and are probably deserving of a new release that resolves these issues.

As things stand, the encryption is vulnerable when short passphrases are permitted (the 8 byte salt is too short to be very meaningful), and the unauthenticated padding means that black box network applications that support untrusted decrypt() calls might be attacked to reveal the secret data. All of these problems are fixable, but they would take some major changes to the library.

This could mislead users into a false sense of safety using the library, and if they're doing anything mission-critical these implementations are probably unsuitable.

Again, thanks for your work and the tough business of making something open-source. I hope I can convince you to make this into something better, secure enough to be worthy of widespread adoption.

@Parinita-AA
Copy link

https://github.com/piyapan039285/Multi-platform-AES-Encryption

Could you please review this? If this is suitable ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants