.Net core string symmetric encryption

Question

I need to store sensitive strings in DB, so decided to encrypt it in a DB and decrypt it at the application layer. And noticed that it is not so easy to find ready to use an example. Please check my code below. It is based on this damienbod's topic, but I included IV to the string itself and also want to know if it is production-ready. And a somewhat unrelated question: probably I should use an old Rijndael solution like this?

public class SymmetricEncryptDecrypt
{
    private const int ivBytes = 128;

    public (string Key, string IVBase64) InitSymmetricEncryptionKeyIV()
    {
        var key = GetEncodedRandomString(32); // 256
        using (Aes cipher = CreateCipher(key))
        {
            cipher.GenerateIV();
            var IVBase64 = Convert.ToBase64String(cipher.IV);
            return (key, IVBase64);
        }
    }

    private byte[] GetNewIv()
    {
        using (Aes cipher = CreateCipher(GetEncodedRandomString(32)))
        {
            cipher.GenerateIV();
            return cipher.IV;
        }
    }

    /// <summary>
    /// Encrypt using AES
    /// </summary>
    /// <param name="text">any text</param>
    /// <param name="IV">Base64 IV string/param>
    /// <param name="key">Base64 key</param>
    /// <returns>Returns an encrypted string</returns>
    public string Encrypt(string text, string key)
    {
        var iv = this.GetNewIv();

        using (Aes cipher = CreateCipher(key))
        {
            cipher.IV = iv;

            ICryptoTransform cryptTransform = cipher.CreateEncryptor();
            byte[] plaintext = Encoding.UTF8.GetBytes(text);
            byte[] cipherText = cryptTransform.TransformFinalBlock(plaintext, 0, plaintext.Length);

            return Convert.ToBase64String(iv.Concat(cipherText).ToArray());
        }
    }

    /// <summary>
    /// Decrypt using AES
    /// </summary>
    /// <param name="text">Base64 string for an AES encryption</param>
    /// <param name="key">Base64 key</param>
    /// <returns>Returns a string</returns>
    public string Decrypt(string encryptedText, string key)
    {
        var cipherTextBytesWithSaltAndIv = Convert.FromBase64String(encryptedText);

        var ivStringBytes = cipherTextBytesWithSaltAndIv.Take(ivBytes / 8).ToArray();
        // Get the actual cipher text bytes by removing the first 64 bytes from the cipherText string.
        var cipherTextBytes = cipherTextBytesWithSaltAndIv.Skip(ivBytes / 8).Take(cipherTextBytesWithSaltAndIv.Length - (ivBytes / 8)).ToArray();

        using (Aes cipher = CreateCipher(key))
        {
            cipher.IV = ivStringBytes;

            ICryptoTransform cryptTransform = cipher.CreateDecryptor();
            byte[] plainBytes = cryptTransform.TransformFinalBlock(cipherTextBytes, 0, cipherTextBytes.Length);

            return Encoding.UTF8.GetString(plainBytes);
        }
    }

    private string GetEncodedRandomString(int length)
    {
        var base64 = Convert.ToBase64String(GenerateRandomBytes(length));
        return base64;
    }

    /// <summary>
    /// Create an AES Cipher using a base64 key
    /// </summary>
    /// <param name="key"></param>
    /// <returns>AES</returns>
    private Aes CreateCipher(string keyBase64)
    {
        // Default values: Keysize 256, Padding PKC27
        Aes cipher = Aes.Create();
        cipher.Mode = CipherMode.CBC; // Ensure the integrity of the ciphertext if using CBC
        cipher.Padding = PaddingMode.ISO10126;
        cipher.Key = Convert.FromBase64String(keyBase64);
        return cipher;
    }

    private byte[] GenerateRandomBytes(int length)
    {
        var byteArray = new byte[length];
        RandomNumberGenerator.Fill(byteArray);
        return byteArray;
    }
}

Answer 1

This code looks complicated for me. Not because I don't know how to use AES encrypter but it contains a lot of redundancy. From setting CipherMode.CBC which is default to GetNewIv() which is random by default.

Is a lot of code is targeting the a security improvement? If it is, you have a breach storing key in a string. I can easily get the key from App's memory even if it currently not in use because string is immutable and can be stored in memory for undefined period of time. Storing key in byte[] array allows to cleanup the array at any moment.

If security on DB side is enough then you may simplify the code as following two methods.

I use streams because it more friendly for me. As a bonus the code can be easily ported to streams use.

private static string Encrypt(string text, byte[] key)
{
    using AesManaged aes = new AesManaged() { Key = key };
    using MemoryStream ms = new MemoryStream();
    ms.Write(aes.IV);
    using (CryptoStream cs = new CryptoStream(ms, aes.CreateEncryptor(), CryptoStreamMode.Write, true))
    {
        cs.Write(Encoding.UTF8.GetBytes(text));
    }
    return Convert.ToBase64String(ms.ToArray());
}

private static string Decrypt(string base64, byte[] key)
{
    using MemoryStream ms = new MemoryStream(Convert.FromBase64String(base64));
    byte[] iv = new byte[16];
    ms.Read(iv);
    using AesManaged aes = new AesManaged() { Key = key, IV = iv };
    using CryptoStream cs = new CryptoStream(ms, aes.CreateDecryptor(), CryptoStreamMode.Read, true);
    using MemoryStream output = new MemoryStream();
    cs.CopyTo(output);
    return Encoding.UTF8.GetString(output.ToArray());
}

Demo

string text = "Hello world";
byte[] key = Enumerable.Range(0, 32).Select(x => (byte)x).ToArray(); // just for example :)
string base64 = Encrypt(text, key);
Console.WriteLine(base64);
Console.WriteLine(Decrypt(base64, key));

Output

Qh+XfnIuIdgllOiKFgzCpTURW+bUuj91S91zA1przRQ=
Hello world

I think that's enough to keep it secure on DB side. Btw, you may keep the Key generator methods if you need it.

P.S. old Rijndael solution - Rijndael was superseded by Aes in .NET, i don't remember the exact reason but something like Rijndael has a padding issue and Aes fixed it.

---

One more tip. If you're on Windows and the encrypted data is allowed to be lost (in case of emergency), you can use DPAPI to protect the key (MS NuGet package exists, Current User/Local Machine protecting modes available) and store protected key in DB in the same sequence as IV. Then you can use totally random key for each Encrypt() call and store it with data. The data may be lost on Windows reinstall or moving the ASP.NET server to other machine, because DPAPI uses key associated with Current User credentials or Local Machine ID. But protected data can't be restored if DB was stolen or in any other way on other PC. Anyway learn how DPAPI works, it may be useful if the server runs under Windows.

Answer 2

There is already a security review and a hint on how to better code the class, but I would like to point out the issues with the coding quality and documentation.

Initialization

private const int ivBytes = 128;

Constants should be in all caps, and an IV for AES-CBC is 16 bytes / 128 bits, not 128 bytes.

public (string Key, string IVBase64) InitSymmetricEncryptionKeyIV()

The parameters should not be capitalized. As it is already in a class called SymmetricEncryptDecrypt you can loose the SymmetricEncryption part.

var key = GetEncodedRandomString(32); // 256

Encoded as what? What does 256 even mean here? And a key is not a string, it's a byte array.

cipher.GenerateIV();

This call is repeated twice, which is generally considered code smell if it is only needed once (but kudos for using a method for it).

Encryption

/// Encrypt using AES

AES in which mode? Shouldn't you specify the padding mode either? And the fact that you prefix the IV? And the fact that you use UTF-8?

/// <returns>Returns an encrypted string</returns>

But here the encoding is not specified (again).

return Convert.ToBase64String(iv.Concat(cipherText).ToArray());

Decryption

One-liners like this just make reading and debugging harder. Split into 3 lines.

var cipherTextBytesWithSaltAndIv = Convert.FromBase64String(encryptedText);

Oh, right, where is the salt in the encryption phase?

    var ivStringBytes = cipherTextBytesWithSaltAndIv.Take(ivBytes / 8).ToArray();

Where is the string? Do you mean octet string?

The rest

var base64 = Convert.ToBase64String(GenerateRandomBytes(length));
return base64;

I'd think that var randomBytes = GenerateRandomBytes(length) followed by return Convert.ToBase64String(randomBytes) is acceptable: don't perform two functions in one line unless it helps readability.

// Default values: Keysize 256, Padding PKC27

The padding mode is called PKCS#7 compatible padding, and you're not using it.

 cipher.Mode = CipherMode.CBC; // Ensure the integrity of the ciphertext if using CBC

That's a good hint, but #1: you're not doing it and #2 the comment should be in the documentation of the class if you're not doing it yourself, not in an end-of-line comment.