1

The application that will use this code must be secure enough to encrypt confidential data at the level SECRET.

I know that the rijndaelManaged class isn't FIPS approved but that doesn't affect the security, so I thought It would be fine to use this class as long as the file is encrypted and decrypted with the same application.

Is this Encryption code secure enough for confidential information?

public static class AESEncryption
{
    private static readonly byte[] initVectorBytes = Encoding.ASCII.GetBytes("tu89geji340t89u2");
    private const int keysize = 256;
    public static string Encrypt(string plainText, string passPhrase)
    {
        byte[] plainTextBytes = Encoding.UTF8.GetBytes(plainText);
        using (PasswordDeriveBytes password = new PasswordDeriveBytes(passPhrase, null))
        {
            byte[] keyBytes = password.GetBytes(keysize / 8);
            using (RijndaelManaged symmetricKey = new RijndaelManaged())
            {
                symmetricKey.Mode = CipherMode.CBC;
                using (ICryptoTransform encryptor = symmetricKey.CreateEncryptor(keyBytes, initVectorBytes))
                {
                    using (MemoryStream memoryStream = new MemoryStream())
                    {
                        using (CryptoStream cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write))
                        {
                            cryptoStream.Write(plainTextBytes, 0, plainTextBytes.Length);
                            cryptoStream.FlushFinalBlock();
                            byte[] cipherTextBytes = memoryStream.ToArray();
                            return Convert.ToBase64String(cipherTextBytes);
                        }
                    }
                }
            }
        }
    }

    public static string Decrypt(string cipherText, string passPhrase)
    {
        byte[] cipherTextBytes = Convert.FromBase64String(cipherText);
        using (PasswordDeriveBytes password = new PasswordDeriveBytes(passPhrase, null))
        {
            byte[] keyBytes = password.GetBytes(keysize / 8);
            using (RijndaelManaged symmetricKey = new RijndaelManaged())
            {
                symmetricKey.Mode = CipherMode.CBC;
                using (ICryptoTransform decryptor = symmetricKey.CreateDecryptor(keyBytes, initVectorBytes))
                {
                    using (MemoryStream memoryStream = new MemoryStream(cipherTextBytes))
                    {
                        using (CryptoStream cryptoStream = new CryptoStream(memoryStream, decryptor, CryptoStreamMode.Read))
                        {
                            byte[] plainTextBytes = new byte[cipherTextBytes.Length];
                            int decryptedByteCount = cryptoStream.Read(plainTextBytes, 0, plainTextBytes.Length);
                            return Encoding.UTF8.GetString(plainTextBytes, 0, decryptedByteCount);
                        }
                    }
                }
            }
        }
    }
}
javaseaayameradost
  • 119
  • 1
  • 3
  • 12
  • 5
    Well it is not anymore. – nsgocev Aug 18 '14 at 13:01
  • From [wikipedia](http://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Initialization_vector_.28IV.29): "For CBC and CFB, reusing an IV leaks some information about the first block of plaintext, and about any common prefix shared by the two messages.". CBC, check. fixed (and therefore reused) IV, check. – Damien_The_Unbeliever Aug 18 '14 at 13:07
  • 1
    @nsgocev how isn't it secure anymore? the IV is supposed to be public right? – javaseaayameradost Aug 18 '14 at 13:26
  • 1
    1) No authenticated encryption/MAC 2) No salt in the KDF 3) Fixed IV which missed the point of the IV. You should never reuse (IV, Key) pairs. This wouldn't be a problem if you used a salt, since that way keys would be single use. 4) `encryptor.TransformFinalBlock` results in much simpler code since you don't need those useless streams. – CodesInChaos Aug 18 '14 at 13:28

1 Answers1

4

You are not using authenticated encryption. This allows an attack to modify messages although he cannot read them.

You are using a constant IV. That is an information leak because an attacker can tell if you are encrypting the same message multiple times.

The purpose of an IV is not to be hard-coded to be some specific value. Let the crypto API generate you one.

You are vulnerable to the Padding Oracle attack because you are leaking the information that an invalid padding was detected.

This code actually looks quite good besides these problems.

I recommend that you use AES-GCM which is available for .NET. It is an integrated primitive that encrypts and authenticates data. Hard to get wrong.

usr
  • 168,620
  • 35
  • 240
  • 369