1

I'm trying to use AesCryptoProvider to encrypt and decrypt byte arrays.

Here are my encrypt and decrypt methods:

public static byte[] EncryptAes(byte[] data, out byte[] key, out byte[] iv)
{
    if (data == null || data.Length <= 0)
        throw new ArgumentNullException("data");

    try
    {
        using (AesCryptoServiceProvider aesAlg = new AesCryptoServiceProvider())
        {
            aesAlg.KeySize = 256;
            aesAlg.BlockSize = 128;
            aesAlg.Padding = PaddingMode.PKCS7;
            aesAlg.Mode = CipherMode.CBC;
            aesAlg.GenerateKey();
            aesAlg.GenerateIV();

            key = aesAlg.Key;
            iv = aesAlg.IV;

            using (MemoryStream msEncrypt = new MemoryStream())
            {
                using (CryptoStream csEncrypt = new CryptoStream(msEncrypt, aesAlg.CreateEncryptor(), CryptoStreamMode.Write))
                {
                    csEncrypt.Write(data, 0, data.Length);
                }

                return msEncrypt.ToArray();
            }
        }
    }
    catch (CryptographicException e)
    {
        Log.Error(e);
        key = null;
        iv = null;
        return null;
    }
}

public static byte[] DecryptAes(byte[] encryptedData, byte[] key, byte[] iv)
{
    if (encryptedData == null || encryptedData.Length <= 0)
        throw new ArgumentNullException("encryptedData");
    if (key == null || key.Length <= 0)
        throw new ArgumentNullException("key");
    if (iv == null || iv.Length <= 0)
        throw new ArgumentNullException("iv");

    try
    {
        using (AesCryptoServiceProvider aesAlg = new AesCryptoServiceProvider())
        {
            aesAlg.KeySize = 256;
            aesAlg.BlockSize = 128;
            aesAlg.Padding = PaddingMode.PKCS7;
            aesAlg.Mode = CipherMode.CBC;
            aesAlg.Key = key;
            aesAlg.IV = iv;

            using (MemoryStream msDecrypt = new MemoryStream(encryptedData))
            {
                using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, aesAlg.CreateDecryptor(), CryptoStreamMode.Write))
                {
                    csDecrypt.Write(encryptedData, 0, encryptedData.Length);
                }

                return msDecrypt.ToArray();
            }
        }
    }
    catch (CryptographicException e)
    {
        Log.Error(e);
        return null;
    }
}

Then to test it, I'm using this code:

originalMessage = "This is a test message.";
originalData = System.Text.Encoding.UTF8.GetBytes(originalMessage);

byte[] key, iv;
byte[] encryptedData = Encryption.EncryptAes(originalData, out key, out iv);
byte[] decryptedData = Encryption.DecryptAes(encryptedData, key, iv);
string decryptedMessage = System.Text.Encoding.UTF8.GetString(decryptedData);
Log.Debug(decryptedMessage); // This is a test message.?{?o?}??

The log output shows that the decrypted message has a bunch of garbage characters "?{?o?}??" at the end.

I've seen similar questions, but their answers don't seem to help. I've tried writing to another array during decryption like this:

using (MemoryStream msDecrypt = new MemoryStream(encryptedData))
{
    using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, aesAlg.CreateDecryptor(), CryptoStreamMode.Write))
    {
        byte[] decryptedData = new byte[encryptedData.Length];
        csDecrypt.Write(decryptedData, 0, decryptedData.Length);
    }

    return msDecrypt.ToArray();
}

But that results in this exception:

System.Security.Cryptography.CryptographicException: Padding is invalid and cannot be removed.

So there's gotta be something I'm missing. Any ideas? Thanks!

1 Answers1

2

Yeah, reusing buffers is biting you. You generally don't expect the encrypted and decrypted data to be the same sizes, so reusing a buffer causes you to see left-over encrypted data in the decrypted data.

Make your decrypt similar to encrypt. Don't pass the buffer to the constructor of MemoryStream, let it allocate a buffer of the correct size:

using (MemoryStream msDecrypt = new MemoryStream())
{
    using (CryptoStream csDecrypt =
          new CryptoStream(msDecrypt,
                           aesAlg.CreateDecryptor(),
                           CryptoStreamMode.Write))
    {
        csDecrypt.Write(encryptedData, 0, encryptedData.Length);
    }
    return msDecrypt.ToArray();
}

I've tried writing to another array during decryption like this:

using (MemoryStream msDecrypt = new MemoryStream(encryptedData))
{
    using (CryptoStream csDecrypt =
          new CryptoStream(msDecrypt,
                           aesAlg.CreateDecryptor(),
                           CryptoStreamMode.Write))
    {
        byte[] decryptedData = new byte[encryptedData.Length];
        csDecrypt.Write(decryptedData, 0, decryptedData.Length);
    }

    return msDecrypt.ToArray();
}

No read it back to yourself. You're still configuring the cryptostream to write rather than read. What you're doing here is allocating a new buffer and then telling AES to decrypt that empty buffer into the memory stream which was initialized with the encrypted data.

Damien_The_Unbeliever
  • 234,701
  • 27
  • 340
  • 448
  • Doesn't he need to call `FlushFinalBlock()` too? – gusto2 Feb 01 '19 at 11:37
  • @gusto2 - they would if they were accessing the contents of the memory stream before they `Dispose` of the cryptostream via the inner `using`. Disposing the stream causes it to [call that](https://referencesource.microsoft.com/#mscorlib/system/security/cryptography/cryptostream.cs,701) – Damien_The_Unbeliever Feb 01 '19 at 11:43
  • Ah yeah, that did it! Thank you! I was definitely mixed up about the streams, but it makes sense now. About the renaming, what's misleading about the method names? – OneOfPaperEqualsFourOfCoin Feb 01 '19 at 15:14
  • @OneOfPaperEqualsFourOfCoin - a mistake on my part. Once I'd fixed the code to work correctly, I apparently copied & pasted the encrypt code into my answer rather than the decrypt code (they're so similar now). I then didn't reference back to your question to see I'd made this mistake and realise that the use of `csEncrypt` and `msEncrypt` in the code sample was my mistake, not yours. – Damien_The_Unbeliever Feb 01 '19 at 15:18