C# AES256 CBC Implementation I was looking for a proper implementation of the AES256 encryption in C# but didn't find anything I was entirely satisfied with, so I decided to take this opportunity to write a class myself and maybe learn a thing or two.

After doing some research I managed to come up with this class :

class AES256
{

    private readonly byte[] _secretKey;

    public AES256(byte[] secretKey)
    {
        _secretKey = secretKey;
    }

    public byte[] Encrypt(byte[] plainBytes)
    {
        return Encrypt(plainBytes, GetRijndaelManaged(_secretKey));
    }

    public byte[] Decrypt(byte[] encryptedBytes)
    {
        return Decrypt(encryptedBytes, GetRijndaelManaged(_secretKey));
    }

    private byte[] Encrypt(byte[] plainBytes, RijndaelManaged rijndaelManaged)
    {
        byte[] transformFinalBlock = rijndaelManaged.CreateEncryptor().TransformFinalBlock(plainBytes, 0, plainBytes.Length);
        return transformFinalBlock;
    }

    private byte[] Decrypt(byte[] encryptedData, RijndaelManaged rijndaelManaged)
    {
        return rijndaelManaged.CreateDecryptor().TransformFinalBlock(encryptedData, 0, encryptedData.Length);
    }

    private RijndaelManaged GetRijndaelManaged(byte[] key)
    {
        var keyBytes = new byte[16];
        var secretKeyBytes = key;
        Array.Copy(secretKeyBytes, keyBytes, System.Math.Min(keyBytes.Length, secretKeyBytes.Length));
        var rijndaelManaged = new RijndaelManaged
        {
            Mode = CipherMode.CBC,
            Padding = PaddingMode.PKCS7,
            KeySize = 256,
            BlockSize = 128,
            Key = keyBytes,
            IV = keyBytes
        };
        return rijndaelManaged;
    }

}


It works, - but obviously that alone doesn't make the code good.
What can I improve in this class, is there anything I can optimize?
Feedback is welcome !
Peter already pointed out several important issues which I won't repeat, but here's what I've got:


Instead of verifying that the length of the key is correct, your code silently pads or trims the key. I would much rather see an exception being thrown instead. Especially security-related code should be easy to use correctly and difficult to use incorrectly.
Because the encryption key can only be specified when constructing an AES256 instance, your design 'encourages' key reuse. On its own that may not necessarily be a problem, but you're not using IVs correctly, and together that does introduce certain vulnerabilities.
RijndaelManaged and ICryptoTransform are both disposable, but they're not being disposed anywhere.
The private Encrypt/Decrypt methods are so short that merging them into the public methods would actually make the code easier to manage.
You may want to document which cipher and padding modes your code uses (and why), so other programmers (and future you) don't need to look at the implementation to find out (or wonder why certain decisions were made).
MSDN recommends using the Aes class instead of RijndaelManaged. It also appears that the Rijndael implementation isn't 'FIPS compliant', which apparently makes it unusable under certain security configurations.
As far as I understand there's no need to create a new RijndaelManaged instance for each operation, so why not create one instance and store it in a field? Or, instead of creating a very thin wrapper class, you could just create a factory method and some extension methods for encryption/decryption. By having these extension methods operate on SymmetricAlgorithm they can also be reused for other encryption algorithms, should the need ever arise.