C# AES Encryption random IV per file

Question

I've made some improvments on the code from : Csharp-AES-bits-Encryption-Library-with-Salt

• saltBytes is now the SHA512 of the password.
• Random IV for each encryption call. ( IV length 16 is added to the encrypted file , removed from file before decryption)

Do you see any flows, something that needs optimization ?

Is generateIV() safe and secure ?

The Code bellow is working, I've made a test to encrypt 2 text files with exact same text inside each. the result is both have different data, and decrypt succeed.

also made test before the random IV, both files had same encrypted text, results in same data.

Encryption code :

 private static int IV_LENGTH = 16;
 public static byte[] AES_Encrypt(byte[] bytesToBeEncrypted, byte[] passwordBytes)
 {
 byte[] encryptedBytes = null;
 byte[] encryptedBytesAndIV = null;
 byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);
 using (System.IO.MemoryStream ms = new System.IO.MemoryStream())
 {
 using (AesCryptoServiceProvider AES = new AesCryptoServiceProvider())
 {
 AES.KeySize = 256;
 //AES.BlockSize = 128;
 var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
 AES.Key = key.GetBytes(AES.KeySize / 8);
 AES.IV = generateIV();
 AES.Mode = CipherMode.CBC;
 using (var cs = new CryptoStream(ms, AES.CreateEncryptor(), CryptoStreamMode.Write))
 {
 cs.Write(bytesToBeEncrypted, 0, bytesToBeEncrypted.Length);
 cs.Close();
 }
 encryptedBytes = ms.ToArray();
 encryptedBytesAndIV = new byte[encryptedBytes.Length + AES.IV.Length];
 AES.IV.CopyTo(encryptedBytesAndIV, 0);
 encryptedBytes.CopyTo(encryptedBytesAndIV, IV_LENGTH);
 }
 }
 return encryptedBytesAndIV;
 }
 private static byte[] generateIV()
 {
 using (RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider())
 {
 byte[] nonce = new byte[IV_LENGTH];
 rng.GetBytes(nonce);
 return nonce;
 }
 }

Decryption code :

 public static byte[] AES_Decrypt(byte[] bytesToBeDecrypted, byte[] passwordBytes)
 {
 byte[] decryptedBytes = null;
 byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);
 using (MemoryStream ms = new MemoryStream())
 {
 using (AesCryptoServiceProvider AES = new AesCryptoServiceProvider())
 {
 AES.KeySize = 256;
 //AES.BlockSize = 128;
 var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
 AES.Key = key.GetBytes(AES.KeySize / 8);
 AES.IV = getIV(bytesToBeDecrypted);
 bytesToBeDecrypted = removeTagAndIV(bytesToBeDecrypted);
 AES.Mode = CipherMode.CBC;
 using (var cs = new CryptoStream(ms, AES.CreateDecryptor(), CryptoStreamMode.Write))
 {
 cs.Write(bytesToBeDecrypted, 0, bytesToBeDecrypted.Length);
 cs.Close();
 }
 decryptedBytes = ms.ToArray();
 }
 }
 return decryptedBytes;
 }
 private static byte[] removeTagAndIV(byte[] arr)
 {
 byte[] enc = new byte[arr.Length - IV_LENGTH];
 Array.Copy(arr, IV_LENGTH, enc, 0, arr.Length - IV_LENGTH);
 return enc;
 }
 private static byte[] getIV(byte[] arr)
 {
 byte[] IV = new byte[IV_LENGTH];
 Array.Copy(arr, 0, IV, 0, IV_LENGTH);
 return IV;
 }

• AlexR: A password-dependent salt is not a salt anymore. It's just a slightly different key derivation function wich is still succeptible to rainbow tables - a proper salt is random, just like the IV. You can store the salt in plain as well.
• xhxx: @AlexR , Do i need to store the salt so the final encrypted file will contain salt, IV & data ?
• xhxx: can i derive the IV from the password and Salt or vise versa? so only IV or Salt is stored along with the ciphertext ?
• AlexR: Salt and IV both MUST not depend on the password and must be stored in plain for decryption to work properly. You can ask on security.SE for more advice on the design of your system. The general advice is: Don't roll your own crypto if you're no expert.
• Kiquenet: any unit test using your code? How generate bytesToBeEncrypted and passwordBytes ?

Answer 1

There are three magic numbers in here:

AES.KeySize = 256;
//AES.BlockSize = 128;
var key = new Rfc2898DeriveBytes(passwordBytes, saltBytes, 100);
AES.Key = key.GetBytes(AES.KeySize / 8);

Also, that comment should go. Dead code is dead.

---

As stated in the comments:

byte[] saltBytes = SHA512.Create().ComputeHash(passwordBytes);

Generating the saltBytes based on the password is a bad idea. The salt should be random and if two users with the exact same password have the same salt, well now either of them can attack the other.

The salt does not need to be a secret. You can keep it in plain-text. Many time, in the case of a database, it will have two (or three) columns: EncryptedPassword and Salt (maybe IV if it's not based on the Salt). (Or in the case of ASP.NET Entity Framework / Simple Membership this is all stored in one column with separators.)

The attacker can know the Salt and IV, you can even use the Salt to generate the IV (or initialize the RNG for it), but you cannot use the password to generate either. That defeats the purpose of the password. If I know Salt is generated from the password, then I don't need the password at all, I can just brute force everything to find that Salt, or I can use a rainbow table lookup, et. al.