0

I have a nice class to encrypt and decrypt data. I can pass it this string:

var thisIsATest = "This is a test.";

When I decrypt the data, I get this back:

var thisIsDecrypted = "This is a test.\0";

We both understand that those are basically the same thing, but it still will not pass my unit test.

First I will show the unit test:

var privateKey = Encryption.GetPrivateKey();
var password = new byte[] { 0x0A, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A, 0x0A };
var salt = Salt.CreateSalt();
var passPhrase = Encryption.PasswordAndSalt(password, salt);
var thisIsATest = "This is a test.";
var clearArray = Encoding.UTF8.GetBytes(thisIsATest);
var cypherArray = Encryption.Encrypt(clearArray, passPhrase, privateKey);
var decryptedArray = Encryption.Decrypt(cypherArray, passPhrase, privateKey);
var thisIsDecrypted = Encoding.UTF8.GetString(decryptedArray);
//Assert
Assert.IsTrue(thisIsATest == thisIsDecrypted);

Here are my code, Decrypt:

public static byte[] Decrypt(byte[] cypherBytes, byte[] passPhrase, byte[] privateKey)
{
    var result = new byte[] { };
    using (var rijndael = new RijndaelManaged() { Mode = CipherMode.CBC, Padding = PaddingMode.PKCS7 })
    {
        using (var decryptor = rijndael.CreateDecryptor(passPhrase, privateKey))
        {
            using (var ms = new MemoryStream(cypherBytes))
            {
                using (var cs = new CryptoStream(ms, decryptor, CryptoStreamMode.Read))
                {
                    result = new byte[cypherBytes.Length];
                    var readCount = cs.Read(result, 0, result.Length);
                }
            }
        }
        rijndael.Clear();
    }
    return result;
}

Encrypt:

public static byte[] Encrypt(byte[] clearBytes, byte[] passPhrase, byte[] privateKey)
{
    var result = new byte[] { };
    using (var rijndael = new RijndaelManaged() { Mode = CipherMode.CBC, Padding = PaddingMode.PKCS7 })
    {
        using (var encryptor = rijndael.CreateEncryptor(passPhrase, privateKey))
        {
            using (var ms = new MemoryStream())
            {
                using (var cs = new CryptoStream(ms, encryptor, CryptoStreamMode.Write))
                {
                    cs.Write(clearBytes, 0, clearBytes.Length);
                    cs.FlushFinalBlock();
                }
                result = ms.ToArray();
            }
        }
        rijndael.Clear();
    }
    return result;
}

And PasswordAndSalt:

public const int INITIALIZATION_VECTOR_SIZE = 32;

public static byte[] PasswordAndSalt(byte[] password, byte[] salt)
{
    var result = new byte[] { };
    using (var pwd = new PasswordDeriveBytes(password, salt))
    {
        result = pwd.GetBytes(INITIALIZATION_VECTOR_SIZE);
    }
    return result;
}

Is PaddingMode.PKCS7 causing my buffer to get the the \0 on the end?

Resolved!

Using Maarten's answer below, I was able to rewrite the Decrypt method to work successfully as follows:

public static byte[] Decrypt(byte[] cypherBytes, byte[] passPhrase, byte[] privateKey)
{
    var result = new byte[] { };
    using (var rijndael = new RijndaelManaged() { Mode = CipherMode.CBC, Padding = PaddingMode.PKCS7 })
    {
        using (var decryptor = rijndael.CreateDecryptor(passPhrase, privateKey))
        {
            using (var ms = new MemoryStream(cypherBytes))
            {
                using (var cs = new CryptoStream(ms, decryptor, CryptoStreamMode.Read))
                {
                    var array = new byte[cypherBytes.Length];
                    var readCount = cs.Read(array, 0, array.Length);
                    result = new byte[readCount];
                    Array.Copy(array, result, readCount);
                }
            }
        }
        rijndael.Clear();
    }
    return result;
}
  • 1
    You use very misleading names, for instance the key is called `passPhrase`, the IV is called `privateKey` and the key size is called `INITIALIZATION_VECTOR_SIZE`. In light of the breaking change in .NET 6, you could use `CopyTo()` instead of `Read()`, as described [here](https://stackoverflow.com/a/69911546/9014097). This also implicitly solves the problem addressed in the answer with the too large buffer. – Topaco Jul 05 '22 at 17:09

1 Answers1

1

Wrong question: you are yourself setting the size of the buffer. The last byte is simply never populated.

Why do you assume for CBC mode with padding that the plaintext is the same size as the ciphertext? You should not ignore the readCount, and then only use the leftmost part of the buffer as plaintext.

CBC mode with PKCS#7 padding will pad with 1 to N bytes, where N is the block size of the algorithm. For AES N is always 16 bytes. So your buffer will always be over-dimensioned in your current code.

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263