AES Encryption C# .NET I have written a tool for encrypting string using the AesCryptoServiceProvider in .NET. The folllowing parameters are used:


Block Cipher Mode: CBC
Initialization Vector: 16 bytes (randomized
per encryption operation), stored in last 16 bytes of final message
Key/Block size: 128 bit Padding: PKCS #7


Overview of code
I have added two functions below. The first prepares a message for encryption, takes the result and appends the Initialization Vector to the cipher message. The second function encrypts the provided message using the specified key and initialization vector.

Obviously, i'm interested in any security holes that exist with this code. Also any optimizations are very much appreciated.

Side questions. In development I always store bytes as strings in their hex representation, seen below. Is this bad? The only other alternative I see is  base64 encoding? Which is better?

public void cryptoExample {

    AesCryptoServiceProvider aes = new AesCryptoServiceProvider();
    aes.KeySize = 128;
    aes.BlockSize = 128;

    aes.Key = Helper.StringToByteArray("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"); // Sample key
    aes.GenerateIV();

    String plainMessage = "Hello AES Crypto!!";

    String encryptedMessage = Helper.ByteArrayToString(CryptoHelper.EncryptStringToBytes_Aes(plainMessage, aes.Key, aes.IV));

    // Must always use a random IV, meaning it needs to be stored with the cipher message.
    // Append it to the end. IV is a constant 16 bytes so we just extract and remove it before decryption.
    String strIv = Helper.ByteArrayToString(aes.IV);

    String messageToTransmit = encryptedMessage += strIv;
}
public static byte[] EncryptStringToBytes_Aes(string plainText, byte[] Key, byte[] IV)
{
    // Check arguments.
    if (plainText == null || plainText.Length &lt;= 0)
        throw new ArgumentNullException("plainText");
    if (Key == null || Key.Length &lt;= 0)
        throw new ArgumentNullException("Key");
    if (IV == null || IV.Length &lt;= 0)
        throw new ArgumentNullException("IV");
    byte[] encrypted;
    // Create an AesCryptoServiceProvider object
    // with the specified key and IV.
    using (AesCryptoServiceProvider aesAlg = new AesCryptoServiceProvider())
    {
        aesAlg.Key = Key;
        aesAlg.IV = IV;
        aesAlg.Mode = CipherMode.CBC;
        aesAlg.Padding = PaddingMode.PKCS7;
        // Create a decrytor to perform the stream transform.
        ICryptoTransform encryptor = aesAlg.CreateEncryptor(aesAlg.Key, aesAlg.IV);

        // Create the streams used for encryption.
        using (MemoryStream msEncrypt = new MemoryStream())
        {
            using (CryptoStream csEncrypt = new CryptoStream(msEncrypt, encryptor, CryptoStreamMode.Write))
            {
                using (StreamWriter swEncrypt = new StreamWriter(csEncrypt))
                {

                    //Write all data to the stream.
                    swEncrypt.Write(plainText);
                }
                encrypted = msEncrypt.ToArray();
            }
        }
    }
}
You can simplify it by replacing all those streams with a call to encrypter.TransformFinalBlock.
You don't have a MAC, so an attacker who can send a modifier message to the decrypter can probably exploit a padding oracle to decrypt the message
Why ask the caller for an IV? You could generate it internally. If you want to keep a deterministic implementation around, at least create an overload without IV.
Helper.ByteArrayToString and StringToByteArray should have a name that says which encoding (hex, Base64, etc.) they use. I'd use BytesToHexString and HexStringToBytes.