C# AES Decryption code

Question

I have the code for string encryption/decryption with AES algorithm. Encryption works fine as expected but I could not get correct plain text when I decrypt it back. There must be something wrong in the decryption code.

I have copied the code below.
Please help with this. Thanks.

Below is the encryption and Decryption code.

public static class EncryptionHelper
{
    private static int BlockSize = 16;

    private static byte[] Key
    {
        get
        {
            byte[] hash = SHA1.Create().ComputeHash(Encoding.UTF8.GetBytes("BlahBlahBlah"));
            byte[] key = new byte[BlockSize];
            Array.Copy(hash, 0, key, 0, BlockSize);
            return key;
        }
    }

    private static byte[] IV
    {
        get
        {
            StringBuilder builder = new StringBuilder();
            Random random = new Random();
            for (int i = 0; i < BlockSize; i++)
            {
                char ch = Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65)));
                builder.Append(ch);
            }
            return Encoding.UTF8.GetBytes(builder.ToString());
        }
    }

    public static string DecodeAndDecrypt(string cipherText)
    {
        string DecodeAndDecrypt = AesDecrypt(Convert.FromBase64String(HttpUtility.UrlDecode(cipherText)));
        return (DecodeAndDecrypt);
    }

    public static string EncryptAndEncode(string plaintext)
    {
        return HttpUtility.UrlEncode(Convert.ToBase64String(AesEncrypt(plaintext))); 
    }

    public static string AesDecrypt(Byte[] inputBytes)
    {
        Byte[] outputBytes = inputBytes;

        string plaintext = string.Empty;

        using (MemoryStream memoryStream = new MemoryStream(outputBytes))
        {
            using (CryptoStream cryptoStream = new CryptoStream(memoryStream, GetCryptoAlgorithm().CreateDecryptor(Key, IV), CryptoStreamMode.Read))
            {
                using (StreamReader srDecrypt = new StreamReader(cryptoStream))
                {
                    plaintext = srDecrypt.ReadToEnd();
                }
            }
        }

        return plaintext;
    }

    public static byte[] AesEncrypt(string inputText)
    {
        byte[] inputBytes = UTF8Encoding.UTF8.GetBytes(inputText); 

        for (int i = 0; i < BlockSize; i++)
        {
            inputBytes[i] ^= IV[i];
        }

        byte[] result = null;
        using (MemoryStream memoryStream = new MemoryStream())
        {
            using (CryptoStream cryptoStream = new CryptoStream(memoryStream, GetCryptoAlgorithm().CreateEncryptor(Key, IV), CryptoStreamMode.Write))
            {
                cryptoStream.Write(inputBytes, 0, inputBytes.Length);
                cryptoStream.FlushFinalBlock();
                result = memoryStream.ToArray();
            }
        }
        return result;
    }

    private static RijndaelManaged GetCryptoAlgorithm()
    {
        RijndaelManaged algorithm = new RijndaelManaged();
        algorithm.Padding = PaddingMode.PKCS7;
        algorithm.Mode = CipherMode.CBC;
        algorithm.KeySize = 128;
        algorithm.BlockSize = 128;
        return algorithm;
    }
}

Answer 1

I made a few changes and will post some source code below. To make it work, just put 3 textBoxes on Form1 and use this code for Form1.cs. I mentioned above I think a major issue was with how you were reading from StreamReader as a string without specifying your encoding. My change uses BinaryReader to read as a byte array, then converts to UTF-8 string after the read. I also deleted that XOR loop. I think you were trying to implement CBC on your own (CBC is a sort of feedback loop of XORs) but it is not needed -- when you specify CBC mode, .NET does the CBC mode for you. To keep my version as close as reasonably possible to yours, I did not make many more changes. But please take some of the above comments to heart. Don't use a simple hash when .NET provides a DeriveBytes function, for example. Treat an IV like a paper towel -- use it once and only once, ever. And so on. Anyway, functional if not "best practice" code is below:

Edit: Sorry I forgot to mention I also changed the plain text type to regular strings, deleted the HTTP-related code. This was just to make it simpler for me to work with. It should work just as well though with your HTTP methods as it does with regular strings.

using System;
using System.IO;
using System.Security.Cryptography;
using System.Text;
using System.Windows.Forms;

namespace StackOverflow
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        private void textBox3_TextChanged(object sender, EventArgs e)
        {
            textBox1.Text = EncryptionHelper.EncryptAndEncode(textBox3.Text);
            textBox2.Text = EncryptionHelper.DecodeAndDecrypt(textBox1.Text);
        }
    }

    public static class EncryptionHelper
    {
        private static int BlockSize = 16;

        private static byte[] Key
        {
            get
            {
                byte[] hash = SHA1.Create().ComputeHash(Encoding.UTF8.GetBytes("BlahBlahBlah"));
                byte[] key = new byte[BlockSize];
                Array.Copy(hash, 0, key, 0, BlockSize);
                return key;
            }
        }

        private static byte[] IV
        {
            get
            {
                StringBuilder builder = new StringBuilder();
                Random random = new Random();
                for (int i = 0; i < BlockSize; i++)
                {
                    char ch = Convert.ToChar(Convert.ToInt32(Math.Floor(26 * random.NextDouble() + 65)));
                    builder.Append(ch);
                }
                return Encoding.UTF8.GetBytes(builder.ToString());
            }
        }

        public static string DecodeAndDecrypt(string cipherText)
        {
            byte[] cipherBytes = Convert.FromBase64String(cipherText);
            string decodeAndDecrypt = AesDecrypt(cipherBytes);
            return decodeAndDecrypt;
        }

        public static string EncryptAndEncode(string plaintext)
        {
            return Convert.ToBase64String(AesEncrypt(plaintext));
        }

        public static string AesDecrypt(Byte[] inputBytes)
        {
            Byte[] outputBytes = inputBytes;

            string plaintext = string.Empty;

            using (MemoryStream memoryStream = new MemoryStream(outputBytes))
            {
                using (CryptoStream cryptoStream = new CryptoStream(memoryStream, GetCryptoAlgorithm().CreateDecryptor(Key, IV), CryptoStreamMode.Read))
                {
                    using (BinaryReader srDecrypt = new BinaryReader(cryptoStream))
                    {
                        outputBytes = srDecrypt.ReadBytes(inputBytes.Length);
                        plaintext = Encoding.UTF8.GetString(outputBytes);
                    }
                }
            }
            return plaintext;
        }

        public static byte[] AesEncrypt(string inputText)
        {
            byte[] inputBytes = UTF8Encoding.UTF8.GetBytes(inputText);
            byte[] result = null;
            using (MemoryStream memoryStream = new MemoryStream())
            {
                using (CryptoStream cryptoStream = new CryptoStream(memoryStream, GetCryptoAlgorithm().CreateEncryptor(Key, IV), CryptoStreamMode.Write))
                {
                    cryptoStream.Write(inputBytes, 0, inputBytes.Length);
                    cryptoStream.FlushFinalBlock();
                    result = memoryStream.ToArray();
                }
            }
            return result;
        }

        private static RijndaelManaged GetCryptoAlgorithm()
        {
            RijndaelManaged algorithm = new RijndaelManaged();
            algorithm.Padding = PaddingMode.PKCS7;
            algorithm.Mode = CipherMode.CBC;
            algorithm.KeySize = 128;
            algorithm.BlockSize = 128;
            return algorithm;
        }
    }
}

Answer 2

The comments above (on the question) have the most important point: you need to use the same IV on decrypt as used on the encrypt, otherwise your first 16 bytes of recovered plaintext will be trashed. (CBC mode will auto-correct after the first block.)

You could simply combine the IV with the ciphertext before sending it, and then just break them apart at the other end before decrypting.

It doesn't matter if the IV is public knowledge, as long as it is cryptographically strong, random data, different for each encryption. But you need that same corresponding, individual IV at the other end to decrypt.

Also, as WDS points out, you should not perform that XOR operation. (But I can't see how WDS's code recovers the first 16 bytes without preserving the IV.)

Also, IMHO using a SHA1 over the password to generate the key is a security risk. You should be using PBKDF2 or similar to derive a password.

Edit: As Artjom B. points out, since your original code XOR's the IV against the first block of plaintext, you could recover the first 16 bytes by using an IV of all 0x00 bytes on decrypt. But this would only work if the ciphertext was at least two blocks (32 bytes), otherwise, you will get padding errors as this will corrupt the padding. Try using an IV of all zero bytes on your decrypt, and find out if the input is always at least 16 bytes -- you have an off-the-end-of-the-array bug anyway in your encrypt if it isn't. (You'll still have an insecure IV, but at least you will get decrypt working.)