0
$\begingroup$

I created a crypto service using AES-GCM in order to encrypt the sensitive data in database. Firstly, I'm generating a cryptographic key from a password (probably will be stored in Kubernetes Secrets) by using Rfc2898DeriveBytes. Then passing this key to AesGcm instance. You can find the implementation down below.

public class CryptoService : ICryptoService, IDisposable
{
    private readonly AesGcm _aesGcm;

    public CryptoService(string password, string salt)
    {
        byte[] key = new Rfc2898DeriveBytes(password, Encoding.UTF8.GetBytes(salt), 200000, HashAlgorithmName.SHA512).GetBytes(32);
        
        //Gets securely random generated encrypted data encryption key from Azure Vault.
        string encryptedEncryptionKey = AzureVault.GetDataEncryptionKey();
        byte[] encryptionKey = AzureVault.Decrypt(encryptedEncryptionKey, key);

        _aesGcm = new AesGcm(encryptionKey);
    }

    public string Encrypt(string plainText)
    {
        byte[] plainBytes = Encoding.UTF8.GetBytes(plainText);

        int nonceSize = AesGcm.NonceByteSizes.MaxSize;
        int tagSize = AesGcm.TagByteSizes.MaxSize;
        int cipherSize = plainBytes.Length;

        // Combine for easier encoding
        int encryptedDataLength = 4 + nonceSize + 4 + tagSize + cipherSize;
        Span<byte> encryptedData = encryptedDataLength < 1024 ? stackalloc byte[encryptedDataLength] : new byte[encryptedDataLength].AsSpan();


        BinaryPrimitives.WriteInt32LittleEndian(encryptedData.Slice(0, 4), nonceSize);
        BinaryPrimitives.WriteInt32LittleEndian(encryptedData.Slice(4 + nonceSize, 4), tagSize);
        var nonce = encryptedData.Slice(4, nonceSize);
        var tag = encryptedData.Slice(4 + nonceSize + 4, tagSize);
        var cipherBytes = encryptedData.Slice(4 + nonceSize + 4 + tagSize, cipherSize);

        RandomNumberGenerator.Fill(nonce);

        _aesGcm.Encrypt(nonce, plainBytes.AsSpan(), cipherBytes, tag);

        return Convert.ToBase64String(encryptedData);
    }   

    public string Decrypt(string cipherText)
    {
        Span<byte> encryptedData = Convert.FromBase64String(cipherText).AsSpan();

        int nonceSize = BinaryPrimitives.ReadInt32LittleEndian(encryptedData.Slice(0, 4));
        int tagSize = BinaryPrimitives.ReadInt32LittleEndian(encryptedData.Slice(4 + nonceSize, 4));
        int cipherSize = encryptedData.Length - 4 - nonceSize - 4 - tagSize;

        var nonce = encryptedData.Slice(4, nonceSize);
        var tag = encryptedData.Slice(4 + nonceSize + 4, tagSize);
        var cipherBytes = encryptedData.Slice(4 + nonceSize + 4 + tagSize, cipherSize);

        Span<byte> plainBytes = cipherSize < 1024 ? stackalloc byte[cipherSize] : new byte[cipherSize];
        _aesGcm.Decrypt(nonce, cipherBytes, tag, plainBytes);

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

Here is my question. I am wondering that if this implementation is secure enough since I am not an expert in security. Am I missing a point or security hole except password security? Any advice and suggestions would be greatly appreciated.

Thanks.

$\endgroup$
0
$\begingroup$

This is not codereview, but there are some things that are specific to crypto:

depending on your threat model, converting the source data to a byte array instead of reading its contents from memory could be problematic, especially if it is large enough.

The main thing I see is you are encrypting the data directly with the password, I would suggest generating a secure random key instead (DEK - data encryption key), then using the password to generate a key to encrypt that key (KEK - key encryption key). That allows you to change the password without reencrypting all the data, and gives a key+nonce combo size of 352 bits.

Another thing is that if you want 256-bit security for AES, your password key derivation should be using SHA512.

You will also want to use any programming methods available to you to prevent key material from being paged to disk. And consider what happens when you provide the wrong password for decryption.

$\endgroup$
5
  • $\begingroup$ Thanks @Richie Frame. These are great suggesstions. To clarify these steps: I'll generate a cryptographically secure random data (32 byte?) for DEK. Then encrypt DEK with the key (KEK) generated by Rfc2898DeriveBytes and store the encrypted DEK. When I want to encrypt/decrypt the data, first I decrypt the encrypted DEK with the KEK. At the end of the day I should store both password (for KEK) and encrypted DEK. $\endgroup$
    – lagrance
    Sep 24 at 8:20
  • $\begingroup$ Pretty close, you should be storing the salt for the password based key derivation function ( is there one in the code ? ) and remembering the password or storing it securely outside the context of the data. The encrypted data blob would be the pbkdf salt, encrypted DEK, nonce, tag, ciphertext, and any non encrypted metadata. $\endgroup$ Sep 24 at 17:23
  • $\begingroup$ Thanks for all suggestions @Richie Frame. I updated the code snippet to give intuition to the other people. I decided to use Key Vault for my DEK. $\endgroup$
    – lagrance
    Sep 28 at 8:51
  • $\begingroup$ @lagrance you may also want to explicitly specify the nonce and tag sizes, instead of just retrieving the max value from the class, this does not make a difference for the tag, however they may someday decide to support 128-bit nonces, then all the code breaks $\endgroup$ Sep 28 at 18:10
  • $\begingroup$ It's unlikely that will happen, but you're right that I should set them myself for robust code. $\endgroup$
    – lagrance
    Oct 1 at 5:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.