Linq select list decryption is very slow

Question

My application is MVC 5 using EF 6.2. I am decrypting certain columns while generating a list, it works but slow. Is there a better way to improve the performance of this approach?

 var mylist = await _db.vw_LearnerCourse.AsNoTracking().ToListAsync();
        var grid1 = mylist.Select(c => new
        {
            FirstName = Encryption.Decrypt5(c.FirstName),
            LastName = Encryption.Decrypt5(c.LastName)
        }).ToList();

   public static string Decrypt5(string cipherText)
    {
        if (string.IsNullOrWhiteSpace(cipherText)) return null;
        if (!string.IsNullOrWhiteSpace(cipherText))
        {
            string strOut;
            try
            {
                var arrOffsets = new ArrayList();
                arrOffsets.Insert(0, 73);
                arrOffsets.Insert(1, 56);
                arrOffsets.Insert(2, 31);
                arrOffsets.Insert(3, 58);
                arrOffsets.Insert(4, 77);
                arrOffsets.Insert(5, 75);
                strOut = "";
                int intCounter;
                for (intCounter = 0;
                    intCounter <= cipherText.Length - 1;
                    intCounter += 2)
                {
                    var strSub = cipherText.Substring(intCounter, 1);
                    var strSub1 = cipherText.Substring(intCounter + 1, 1);
                    var intVal = int.Parse(strSub,
                                     NumberStyles.HexNumber) * 16 + int.Parse(strSub1,
                                     NumberStyles.HexNumber);
                    var intMod = intCounter / 2 % arrOffsets.Count;
                    var intNewVal = intVal -
                                    Convert.ToInt32(arrOffsets[intMod]) + 256;
                    intNewVal = intNewVal % 256;
                    var strDecimal = ((char)intNewVal).ToString();
                    strOut = strOut + strDecimal;
                }
            }
            catch (Exception err)
            {
                throw new Exception(err.Message);
            }
            var encryptionKey = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";
            cipherText = strOut;
            cipherText = cipherText.Replace(" ", "+");
            var cipherBytes = Convert.FromBase64String(cipherText);
            using (var encryptor = Aes.Create())
            {
                var pdb = new Rfc2898DeriveBytes(encryptionKey, new byte[]
                {
                    xxxxxxx
                });
                encryptor.Key = pdb.GetBytes(32);
                encryptor.IV = pdb.GetBytes(16);
                using (var ms = new MemoryStream())
                {
                    using (var cs = new CryptoStream(ms, encryptor.CreateDecryptor(),
                        CryptoStreamMode.Write))
                    {
                        cs.Write(cipherBytes, 0, cipherBytes.Length);
                        cs.Close();
                    }
                    cipherText = Encoding.Unicode.GetString(ms.ToArray());
                }
            }
            return cipherText;
        }
        return null;
    }

Answer 1

The main reason for the poor performance is the execution of the PBKDF2 key derivation (via Rfc2898DeriveBytes) for every encryption. Key derivations are intentionally slow to slow down attackers. I.e. each encryption adds an artificial delay, which leads to the observed performance loss over runtime.

The execution time of PBKDF2 can be tuned via the iteration count. In general, the value is set as high as possible while maintaining acceptable performance. A typical value is 10,000 iterations, see e.g. here.
The posted code uses the default value of 1000 iterations (see here), which is already very low. Reducing the value also reduces security and is therefore less recommended.
Apart from that, changing the iteration count would lead to incompatibility with already existing data, so a data migration would be necessary.

Another alternative to improve performance is to perform key derivation on list level (instead of for each encryption of the list) and pass key and IV to the Decrypt5() method. This also ensures that compatibility with the old data is not lost:

// Key/IV derivation
var encryptionKey = "This is my passphrase";
var salt = new byte[] {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07};
var pdb = new Rfc2898DeriveBytes(encryptionKey, salt);
byte[] key = pdb.GetBytes(32);
byte[] iv = pdb.GetBytes(16);

// Decryption, key/IV are passed
var mylist = await _db.vw_LearnerCourse.AsNoTracking().ToListAsync();
var grid1 = mylist.Select(c => new
{
    FirstName = Encryption.Decrypt5(c.FirstName, key, iv),
    LastName = Encryption.Decrypt5(c.LastName, key, iv)
}).ToList();

...

// Decryption without key/IV derivation 
public static string Decrypt5(string cipherText, byte[] key, byte[] iv)
{
...
    var cipherBytes = Convert.FromBase64String(cipherText);
    using (var encryptor = Aes.Create())
    {
        encryptor.Key = key;
        encryptor.IV = iv;
        using (var ms = new MemoryStream())
        {
            using (var cs = new CryptoStream(ms, encryptor.CreateDecryptor(), CryptoStreamMode.Write))
            {
                cs.Write(cipherBytes, 0, cipherBytes.Length);
                cs.Close();
            }
            cipherText = Encoding.Unicode.GetString(ms.ToArray());
        }
    }
    return cipherText;
...
}

This change significantly improves performance while maintaining compatibility with existing data.

---

Security:

A vulnerability of the current code is the static salt. A static salt is generally insecure because in case of a successful attack the entire data is affected. Since in the current case the key derivation additionally derives the IV besides the key, this is further aggravated by the inevitable key/IV reuse.

To increase security, these vulnerabilities should be eliminated. However, it is impossible to avoid losing compatibility with the old data, so data migration is necessary.

To improve security, instead of the static salt, a random salt should be generated for each encryption or here with regard to performance at least for each list.
To avoid reuse of key/IV pairs within a list, the IV should not be derived via key derivation, but a fresh, random IV should be generated for each encryption. Although the generation of the random IV has a negative impact on performance, this is significantly outweighed by the gain due to the shift of the key derivation.

Salt and IV are not secret and are usually concatenated with the ciphertext: salt|IV|ciphertext. During decryption, the portions are separated based on the known lengths of salt and IV. Since the same salt is used here for all encryptions of a list, the salt could also be stored separately.

For the iteration count not the default value (of 1000 iterations) should be used, but the largest possible value with acceptable performance as well.

Hard coding of the password should be refrained from (otherwise, e.g. access to the code is enough to compromise the password), instead the password should be entered at runtime.

Answer 2

The one thing I'm certain there is no need for manual Encryption\Decryption.

OPTION1 You should use EncryptedColumns EF feature and let encryption/decryption be handled naturally.

https://sd.blackball.lv/articles/read/18805 see example of implementation. Github project https://github.com/emrekizildas/EntityFrameworkCore.EncryptColumn.

Init context with encryption key:

private readonly IEncryptionProvider _provider;
public ExampleDbContext()
{
    Initialize.EncryptionKey = "example_encrypt_key";
    this._provider = new GenerateEncryptionProvider();
}

Let modelBinder know about encryption:

modelBuilder.UseEncryption(this._provider);

Set the columns

public class User
{
    public Guid ID { get; set; }
    public string Firstname { get; set; }
    public string Lastname { get; set; }
    [EncryptColumn]
    public string EmailAddress { get; set; }
    [EncryptColumn]
    public string IdentityNumber { get; set; }
}

OPTION2 You can also opt for Always Encrypted, this will encrypt entire DB https://techcommunity.microsoft.com/t5/sql-server-blog/using-always-encrypted-with-entity-framework-6/ba-p/384433 for details and some limitations of approach.

The idea is straightforward, we delegate all encryption process to database (for ex MS SQL Server) if it supports this and using SQL Server features organize encryption. The EF Core we instruct via connection string that DB is encrypted and that's really it. Noticeable difference to normal DB design:

• Entity Framework assumes order-comparability of PKs in many cases. If PK is encrypted, some scenarios will not work.

• Also EF will sometimes print values of the Entity key in exception messages. This could cause sensitive information to be leaked to inappropriate parties. (issue tracked, MS recommended to use surrogate keys)

• EF query will fail if we compare encrypted column to a constant, instead need to pass constant argument as closure

• EF can transparently replace constants in the query with parameters. This can be achieved using query interception extensibility features.

• Sorting based on encrypted column is not supported on the database due to limitations of Always Encrypted feature. This need to be done on client side.

• Some GroupBy operations are not supported (namely the LINQ specific grouping, without projecting group key or aggregate function) if entity key is encrypted. Reason is that those queries (that simply aggregate elements into IGrouping<,> statements) are translated into TSQL containing ORDER BY operation on the key. If the key is encrypted, the query will fail. You have to use unencrypted surrogate key, to let OrderBy work

• Queries that project a collection don’t work with encrypted columns if the key (or any part of the composite key) is encrypted. The reason is same OrderBy not work for encrypted columns. Solution as previous omit encrypting columns that need server side sorting.

• Similarly to the above case, Include operation performed on a collection is not supported if the PK of the principal entity is encrypted

Answer 3

Perhaps try and use a concrete class instead of a dynamic object. Dynamic objects create some overhead and if there's a lot of them I guess that could be what slows it down

Like so:

class Names
{
    public string? FirstName { get; }
    public string? LastName { get; }

    public Names(string? firstName, string? lastName)
    {
        FirstName = firstName;
        LastName = lastName;
    }
}

var grid1 = mylist.Select(c => 
    new Names(
        Encryption.Decrypt5(c.FirstName),
        Encryption.Decrypt5(c.LastName))
    .ToList();

It also looks like you're unnecessarily checking if cipherText is null or white space twice

Answer 4

You may create a Decryptor instance every time when you Decrypt a string. Try create a static decryptor, and use this instance every time.

TripleDESCryptoServiceProvider des = new TripleDESCryptoServiceProvider
                {
                    Key = key,
                    Mode = CipherMode.ECB
                };
// use this instance all time
var decryptor = des.CreateDecryptor();     
var mylist = await _db.vw_LearnerCourse.AsNoTracking().ToListAsync();
    var grid1 = mylist.Select(c => new
    {
        FirstName = Encryption.Decrypt5(decryptor,c.FirstName),
        LastName = Encryption.Decrypt5(decryptor,c.LastName)
    }).ToList();

public static string Decrypt5(ICryptoTransform decryptor, string cipherText)
    {
        if (string.IsNullOrWhiteSpace(cipherText)) return null;
        if (!string.IsNullOrWhiteSpace(cipherText))
        {
            xxxxxxxx
    }