simbiose / Encryption

Encryption is a simple way to encrypt and decrypt strings on Android and Java project.
MIT License
354 stars 79 forks source link

Suboptimal key derivation #27

Open Vinc0682 opened 5 years ago

Vinc0682 commented 5 years ago

There are several points making the key derivation weaker than it could be, the main problem being SHA1 which caps the entropy at 160 bit. Also there already are known SHA1-collision making SHA1 a broken hash function. While the impact on key derivation is limited, you better are safe-than-sorry when it comes to encryption.

  1. The hashing before using PBKDF2 is kinda useless as PBKDF2 also hashes the incoming password. Also the usually used SHA1 limits the maximum security of the current scheme to 160 bits as mentioned above. Removing the hash-step would also speed up encryption a little bit.
  2. The usually used "PBKDF2WithHmacSHA1" also used SHA1 which, as mentioned above, rather should not be used. AFAIK you can just use "PBKDF2WithHmacSHA256" or "PBKDF2WithHmacSHA512", so you could change the default value to switch to more secure hash methods.
  3. The default iteration count of 1 is very low and thus kind of makes the usage of PBKDF2 unnecessary as it is supposed to be a slower key-derivation function to make brute-forcing passwords harder. I know that there were performance complaints but you can use a compromise like 4096 which is used in WPA2 according to Wikipedia. This would still make brute-forcing the password roughly 4000 times more time consuming that the iteration count of 1 but should still be reasonably fast even on mobile devices.
ademar111190 commented 4 years ago

wow 3 good points, I'll need time to study it, thanks!