An implementation of encryption cascade

Question

I'm looking for feedback on some source code, I have everything documented and have corrected some errors. Looking to see what I did correctly, incorrectly, and what can be improved.

 /// <summary>
 /// Sets the password array based on the content of the <see cref="SecureString"/>.
 /// </summary>
 /// <exception cref="ArgumentNullException">Thrown if <see cref="SecureString"/> is null.</exception>
 private static void SetArray()
 {
 try
 {
 // Check if SecurePassword is null.
 if (Login.SecurePassword == null)
 throw new ArgumentNullException(nameof(Login.SecurePassword), "SecurePassword cannot be null.");
 _passwordArray = ConvertSecureStringToCharArray(Login.SecurePassword);
 }
 catch (Exception ex)
 {
 // Log and rethrow exceptions.
 ErrorLogging.ErrorLog(ex);
 throw;
 }
 }
 /// <summary>
 /// Converts a <see cref="SecureString"/> to a character array.
 /// </summary>
 /// <param name="secureString">The SecureString to convert.</param>
 /// <returns>A character array containing the characters from the SecureString.</returns>
 /// <exception cref="ArgumentNullException">Thrown if <paramref name="secureString"/> is null.</exception>
 public static char[] ConvertSecureStringToCharArray(SecureString secureString)
 {
 // Check if secureString is null.
 if (secureString == null)
 throw new ArgumentNullException(nameof(secureString), "SecureString cannot be null.");
 var charArray = new char[secureString.Length];
 var unmanagedString = IntPtr.Zero;
 try
 {
 // Convert SecureString to an unmanaged Unicode string.
 unmanagedString = Marshal.SecureStringToGlobalAllocUnicode(secureString);
 // Copy characters from the unmanaged string to the charArray.
 for (var i = 0; i < secureString.Length; i++)
 charArray[i] = (char)Marshal.ReadInt16(unmanagedString, i * 2);
 }
 finally
 {
 // Zero-free the allocated unmanaged memory.
 if (unmanagedString != IntPtr.Zero) Marshal.ZeroFreeGlobalAllocUnicode(unmanagedString);
 }
 return charArray;
 }
 /// <summary>
 /// Event handler for the Encrypt button click.
 /// Encrypts the currently opened file using the specified password.
 /// </summary>
 /// <param name="sender">The object that triggered the event.</param>
 /// <param name="e">The event arguments.</param>
 private async void EncryptBtn_Click(object sender, EventArgs e)
 {
 try
 {
 // Display a warning message about not closing the program during encryption.
 MessageBox.Show(
 @"Do NOT close the program while loading. This may cause corrupted data that is NOT recoverable.",
 @"Info",
 MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
 // Check if a file is opened.
 if (!_fileOpened)
 throw new ArgumentException(@"No file is opened.", nameof(_fileOpened));
 // Set array and start the encryption animation.
 SetArray();
 StartAnimationEncryption();
 DisableUi();
 // Check if a loaded file exists.
 if (_loadedFile == string.Empty)
 return;
 // Encrypt the file.
 var encryptedFile =
 await Crypto.EncryptFile(Authentication.CurrentLoggedInUser, _passwordArray, _loadedFile);
 // Check if the encrypted file is empty.
 if (encryptedFile == Array.Empty<byte>())
 throw new ArgumentException(@"Value returned empty.", nameof(encryptedFile));
 // Perform garbage collection aggressively.
 GC.Collect(GC.MaxGeneration, GCCollectionMode.Aggressive, true, true);
 // Convert the encrypted file to Base64 string.
 var str = DataConversionHelpers.ByteArrayToBase64String(encryptedFile);
 // Update the result and enable UI.
 if (!string.IsNullOrEmpty(str))
 _result = str;
 EnableUi();
 _isAnimating = false;
 // Display the encrypted file size.
 var size = (long)_result.Length;
 var fileSize = size.ToString("#,0");
 FileSizeNumLbl.Text = $@"{fileSize} bytes";
 // Display success message.
 FileOutputLbl.Text = @"File encrypted.";
 FileOutputLbl.ForeColor = Color.LimeGreen;
 MessageBox.Show(@"File was encrypted successfully.", @"Success", MessageBoxButtons.OK,
 MessageBoxIcon.Information);
 // Reset UI state and clear sensitive information.
 FileOutputLbl.Text = @"Idle...";
 FileOutputLbl.ForeColor = Color.WhiteSmoke;
 Array.Clear(_passwordArray, 0, _passwordArray.Length);
 }
 catch (Exception ex)
 {
 // Handle exceptions, enable UI, and log errors.
 EnableUi();
 _isAnimating = false;
 FileOutputLbl.Text = @"Error encrypting file.";
 FileOutputLbl.ForeColor = Color.Red;
 MessageBox.Show(ex.Message, @"Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
 FileOutputLbl.Text = @"Idle...";
 FileOutputLbl.ForeColor = Color.WhiteSmoke;
 Array.Clear(_passwordArray, 0, _passwordArray.Length);
 ErrorLogging.ErrorLog(ex);
 }
 }
 /// <summary>
 /// Hashes a password inside of a char array or derives a key from a password.
 /// </summary>
 /// <param name="passWord">The char array to hash.</param>
 /// <param name="salt">The salt used during the argon2id hashing process.</param>
 /// <param name="outputSize">The output size in bytes.</param>
 /// <returns>Either a derived key or password hash byte array.</returns>
 public static async Task<byte[]> Argon2Id(char[] passWord, byte[] salt, int outputSize)
 {
 try
 {
 // Check for null or empty values
 if (passWord == null || passWord.Length == 0 || salt == null || salt.Length == 0)
 {
 throw new ArgumentException("Value was null or empty.",
 passWord == null ? nameof(passWord) : nameof(salt));
 }
 // Initialize Argon2id
 using var argon2 = new Argon2id(Encoding.UTF8.GetBytes(passWord));
 argon2.Salt = salt;
 argon2.DegreeOfParallelism = Environment.ProcessorCount * 2;
 argon2.Iterations = CryptoConstants.Iterations;
 argon2.MemorySize = CryptoConstants.MemorySize;
 // Get the result
 var result = await argon2.GetBytesAsync(outputSize);
 return result;
 }
 catch (ArgumentNullException ex)
 {
 // Log ArgumentNullException details
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (ArgumentException ex)
 {
 // Log ArgumentException details
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (CryptographicException ex)
 {
 // Log CryptographicException details
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (Exception ex)
 {
 // Log any other unexpected exceptions
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 }
 /// <summary>
 /// Encrypts the contents of a file using Argon2 key derivation and xChaCha20-Poly1305 + AES-CBC-Blake2b encryption.
 /// </summary>
 /// <param name="userName">The username associated with the user's salt for key derivation.</param>
 /// <param name="passWord">The user's password used for key derivation.</param>
 /// <param name="file">The path to the file to be encrypted.</param>
 /// <returns>
 /// A Task that completes with the encrypted content of the specified file.
 /// If any error occurs during the process, returns an empty byte array.
 /// </returns>
 /// <remarks>
 /// This method performs the following steps:
 /// 1. Retrieves the user-specific salt using the provided username.
 /// 2. Derives an encryption key from the user's password and the obtained salt using Argon2id.
 /// 3. Extracts key components for encryption, including two keys and an HMAC key.
 /// 4. Reads the content of the specified file.
 /// 5. Encrypts the file content using xChaCha20-Poly1305 + AES-CBC-Blake2b encryption.
 /// 6. Clears sensitive information, such as the user's password, from memory.
 /// </remarks>
 public static async Task<byte[]> EncryptFile(string userName, char[] passWord, string file)
 {
 try
 {
 // Check if any required input is null or empty
 if (string.IsNullOrEmpty(userName) || passWord == null || passWord.Length == 0 || string.IsNullOrEmpty(file))
 {
 throw new ArgumentException("Value was empty.",
 string.IsNullOrEmpty(userName) ? nameof(userName) :
 passWord == null || passWord.Length == 0 ? nameof(passWord) : nameof(file));
 }
 // Retrieve user-specific salt
 var salt = await Authentication.GetUserSaltAsync(userName);
 // Derive encryption key from password and salt
 var bytes = await Argon2Id(passWord, salt, 128);
 // Extract key components for encryption
 var key = new byte[CryptoConstants.KeySize];
 var key2 = new byte[CryptoConstants.KeySize];
 var hMacKey = new byte[CryptoConstants.HmacLength];
 Buffer.BlockCopy(bytes, 0, key, 0, key.Length);
 Buffer.BlockCopy(bytes, key.Length, key2, 0, key2.Length);
 Buffer.BlockCopy(bytes, key.Length + key2.Length, hMacKey, 0, hMacKey.Length);
 // Read content of the specified file
 var fileStr = await File.ReadAllTextAsync(file);
 var fileBytes = DataConversionHelpers.StringToByteArray(fileStr);
 // Check if file content or salt is empty
 if (fileBytes == null || fileBytes.Length == 0 || salt == null || salt.Length == 0)
 {
 throw new ArgumentException("Value was empty.",
 fileBytes == null || fileBytes.Length == 0 ? nameof(fileBytes) : nameof(salt));
 }
 // Encrypt the file content
 var encryptedFile = await EncryptAsyncV3(fileBytes, key, key2, hMacKey);
 // Clear sensitive information
 Array.Clear(passWord, 0, passWord.Length);
 return encryptedFile;
 }
 catch (ArgumentException ex)
 {
 // Handle argument-related exceptions
 MessageBox.Show(ex.Message, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (Exception ex)
 {
 // Handle unexpected exceptions
 MessageBox.Show("An unexpected error occurred.", "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 }
 /// <summary>
 /// Decrypts the contents of an encrypted file using Argon2 key derivation and ChaCha20-Poly1305 decryption.
 /// </summary>
 /// <param name="userName">The username associated with the user's salt for key derivation.</param>
 /// <param name="passWord">The user's password used for key derivation.</param>
 /// <param name="file">The path to the encrypted file to be decrypted.</param>
 /// <returns>
 /// A Task that completes with the decrypted content of the specified encrypted file.
 /// If any error occurs during the process, returns an empty byte array.
 /// </returns>
 /// <remarks>
 /// This method performs the following steps:
 /// 1. Validates input parameters to ensure they are not null or empty.
 /// 2. Retrieves the user-specific salts for key derivation.
 /// 3. Derives an encryption key from the user's password and the obtained salt using Argon2id.
 /// 4. Extracts key components for decryption, including two keys and an HMAC key.
 /// 5. Reads and decodes the content of the encrypted file.
 /// 6. Decrypts the file content using ChaCha20-Poly1305 decryption.
 /// 7. Clears sensitive information, such as the user's password, from memory.
 /// </remarks>
 public static async Task<byte[]> DecryptFile(string userName, char[] passWord, string file)
 {
 try
 {
 // Validate input parameters
 if (string.IsNullOrEmpty(userName) || passWord == null || passWord.Length == 0 ||
 string.IsNullOrEmpty(file))
 {
 throw new ArgumentException("Value was empty.",
 string.IsNullOrEmpty(userName) ? nameof(userName) :
 passWord == null || passWord.Length == 0 ? nameof(passWord) : nameof(file));
 }
 // Retrieve user-specific salts for key derivation
 var salt = await Authentication.GetUserSaltAsync(userName);
 // Derive decryption key from password and salts using Argon2id
 var bytes = await Argon2Id(passWord, salt, 128);
 // Extract key components for decryption
 var key = new byte[CryptoConstants.KeySize];
 var key2 = new byte[CryptoConstants.KeySize];
 var hMacKey = new byte[CryptoConstants.HmacLength];
 Buffer.BlockCopy(bytes, 0, key, 0, key.Length);
 Buffer.BlockCopy(bytes, key.Length, key2, 0, key2.Length);
 Buffer.BlockCopy(bytes, key.Length + key2.Length, hMacKey, 0, hMacKey.Length);
 // Read and decode the content of the encrypted file
 var fileStr = await File.ReadAllTextAsync(file);
 var fileBytes = DataConversionHelpers.Base64StringToByteArray(fileStr);
 // Check if file content or salt is empty
 if (fileBytes == Array.Empty<byte>() || salt == Array.Empty<byte>())
 {
 throw new ArgumentException("Value was empty.",
 fileBytes == Array.Empty<byte>() ? nameof(fileBytes) : nameof(salt));
 }
 // Decrypt the file content
 var decryptedFile = await DecryptAsyncV3(fileBytes, key, key2, hMacKey);
 // Clear sensitive information
 Array.Clear(passWord, 0, passWord.Length);
 return decryptedFile;
 }
 catch (Exception ex)
 {
 // Handle exceptions and display an error message
 MessageBox.Show($"An error occurred during file decryption: {ex.Message}", "Error", MessageBoxButtons.OK,
 MessageBoxIcon.Error);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 }
#pragma warning disable
 private const int BlockBitSize = 128;
 private const int KeyBitSize = 256;
 /// <summary>
 /// Encrypts the provided plaintext using AES encryption with CBC mode and PKCS7 padding.
 /// The ciphertext is then authenticated using HMAC-Blake2B.
 /// </summary>
 /// <param name="plainText">The plaintext to be encrypted.</param>
 /// <param name="key">The encryption key.</param>
 /// <param name="iv">The initialization vector.</param>
 /// <param name="hMacKey">The HMAC key for authentication.</param>
 /// <returns>
 /// A Task that completes with the authenticated ciphertext.
 /// If any error occurs during the process, returns an empty byte array.
 /// </returns>
 /// <remarks>
 /// This method performs the following steps:
 /// 1. Validates input parameters to ensure they are not null or empty.
 /// 2. Creates an AES object with specified block size, key size, mode, and padding.
 /// 3. Encrypts the plaintext using AES encryption with CBC mode and PKCS7 padding.
 /// 4. Computes HMAC-Blake2B over the IV and ciphertext for authentication.
 /// 5. Clears sensitive information, such as encryption key and HMAC key, from memory.
 /// </remarks>
 public static async Task<byte[]> EncryptAsync(byte[] plainText, byte[] key, byte[] iv, byte[] hMacKey)
 {
 // Parameter checks
 if (plainText == Array.Empty<byte>())
 throw new ArgumentException(@"Value was empty or null.", nameof(plainText));
 if (key == Array.Empty<byte>())
 throw new ArgumentException(@"Value was empty or null.", nameof(key));
 try
 {
 // Create AES object
 using var aes = Aes.Create();
 aes.BlockSize = BlockBitSize;
 aes.KeySize = KeyBitSize;
 aes.Mode = CipherMode.CBC;
 aes.Padding = PaddingMode.PKCS7;
 // Create byte array to store cipherText into.
 byte[] cipherText;
 // Create streams and copy cipherStream to memStream
 using (var encryptor = aes.CreateEncryptor(key, iv))
 using (var memStream = new MemoryStream())
 {
 await using (var cryptoStream =
 new CryptoStream(memStream, encryptor, CryptoStreamMode.Write))
 {
 using (var cipherStream = new MemoryStream(plainText))
 {
 await cipherStream.FlushAsync();
 await cipherStream.CopyToAsync(cryptoStream, (int)cipherStream.Length);
 }
 await cryptoStream.FlushFinalBlockAsync();
 }
 cipherText = memStream.ToArray();
 }
 // Clear key parameter
 Array.Clear(key, 0, key.Length);
 // Create HMAC object
 using var hmac = new HMACBlake2B(hMacKey, CryptoConstants.HmacLength * 8);
 var prependItems = new byte[cipherText.Length + iv.Length];
 Buffer.BlockCopy(iv, 0, prependItems, 0, iv.Length);
 Buffer.BlockCopy(cipherText, 0, prependItems, iv.Length, cipherText.Length);
 // Hash the IV and cipherText and place them into authenticatedBytes byte array
 var tag = hmac.ComputeHash(prependItems);
 var authenticatedBuffer = prependItems.Length + tag.Length;
 var authenticatedBytes = new byte[authenticatedBuffer];
 Buffer.BlockCopy(prependItems, 0, authenticatedBytes, 0, prependItems.Length);
 Buffer.BlockCopy(tag, 0, authenticatedBytes, prependItems.Length, tag.Length);
 // Clear HMAC key
 Array.Clear(hMacKey, 0, hMacKey.Length);
 // Return authenticatedBytes byte array.
 return authenticatedBytes;
 }
 // Catch blocks
 catch (CryptographicException ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (ArgumentNullException ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (ObjectDisposedException ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (Exception ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 }
 /// <summary>
 /// Decrypts the provided ciphertext using AES decryption with CBC mode and PKCS7 padding.
 /// The authenticity of the ciphertext is verified using HMAC-Blake2B.
 /// </summary>
 /// <param name="cipherText">The ciphertext to be decrypted.</param>
 /// <param name="key">The decryption key.</param>
 /// <param name="hMacKey">The HMAC key for authentication.</param>
 /// <returns>
 /// A Task that completes with the decrypted plaintext.
 /// If any error occurs during the process, returns an empty byte array.
 /// </returns>
 /// <remarks>
 /// This method performs the following steps:
 /// 1. Validates input parameters to ensure they are not null or empty.
 /// 2. Creates an AES object with specified block size, key size, mode, and padding.
 /// 3. Verifies the authenticity of the ciphertext using HMAC-Blake2B.
 /// 4. Decrypts the ciphertext using AES decryption with CBC mode and PKCS7 padding.
 /// 5. Clears sensitive information, such as the decryption key and HMAC key, from memory.
 /// </remarks>
 public static async Task<byte[]> DecryptAsync(byte[] cipherText, byte[] key, byte[] hMacKey)
 {
 // Parameter checks
 if (cipherText == Array.Empty<byte>())
 throw new ArgumentException(@"Value was empty or null.", nameof(cipherText));
 if (key == Array.Empty<byte>())
 throw new ArgumentException(@"Value was empty or null.", nameof(key));
 try
 {
 // Create AES object
 using var aes = Aes.Create();
 aes.BlockSize = BlockBitSize;
 aes.KeySize = KeyBitSize;
 aes.Mode = CipherMode.CBC;
 aes.Padding = PaddingMode.PKCS7;
 // Create HMAC object
 using var hmac = new HMACBlake2B(hMacKey, CryptoConstants.HmacLength * 8);
 var receivedHash = new byte[CryptoConstants.HmacLength];
 // Place the received hash into receivedHash byte array
 Buffer.BlockCopy(cipherText, cipherText.Length - CryptoConstants.HmacLength, receivedHash, 0, CryptoConstants.HmacLength);
 // Create byte array for IV and cipherText
 var cipherWithIv = new byte[cipherText.Length - CryptoConstants.HmacLength];
 // Place cipherText and IV into cipherWithIv byte array
 Buffer.BlockCopy(cipherText, 0, cipherWithIv, 0, cipherText.Length - CryptoConstants.HmacLength);
 // Get the hash value of cipherText and IV
 var hashedInput = hmac.ComputeHash(cipherWithIv);
 // Compare hash byte arrays using fixed timing to prevent timing attacks
 var isMatch = CryptographicOperations.FixedTimeEquals(receivedHash, hashedInput);
 // If hash values do not match, throw an exception
 if (!isMatch)
 throw new CryptographicException();
 // Clear HMAC key array
 Array.Clear(hMacKey, 0, hMacKey.Length);
 // Get cipherResult and IV values
 var iv = new byte[CryptoConstants.Iv];
 var cipherResult = new byte[cipherText.Length - iv.Length - CryptoConstants.HmacLength];
 Buffer.BlockCopy(cipherText, 0, iv, 0, iv.Length);
 Buffer.BlockCopy(cipherText, iv.Length, cipherResult, 0, cipherResult.Length);
 // Create streams, and copy the contents of plainStream to memStream
 using var decryptor = aes.CreateDecryptor(key, iv);
 using var memStream = new MemoryStream();
 await using (var decryptStream = new CryptoStream(memStream, decryptor, CryptoStreamMode.Write))
 {
 using (var plainStream = new MemoryStream(cipherResult))
 {
 await plainStream.CopyToAsync(decryptStream, (int)plainStream.Length);
 await plainStream.FlushAsync();
 }
 await decryptStream.FlushFinalBlockAsync();
 }
 // Clear key array
 Array.Clear(key, 0, key.Length);
 // Return memStream as byte array
 return memStream.ToArray();
 }
 // Catch blocks
 catch (CryptographicException ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (ArgumentNullException ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (ObjectDisposedException ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (Exception ex)
 {
 Array.Clear(key, 0, key.Length);
 Array.Clear(hMacKey, 0, hMacKey.Length);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
#pragma warning restore
 }
 /// <summary>
 /// Encrypts the provided plaintext using a combination of XChaCha20-Poly1305 and AES encryption.
 /// </summary>
 /// <param name="plaintext">The plaintext to be encrypted.</param>
 /// <param name="key">The first encryption key.</param>
 /// <param name="key2">The second encryption key.</param>
 /// <param name="hMacKey">The HMAC key for authentication.</param>
 /// <returns>
 /// A Task that completes with the encrypted ciphertext.
 /// If any error occurs during the process, returns an empty byte array.
 /// </returns>
 /// <remarks>
 /// This method performs the following steps:
 /// 1. Validates input parameters to ensure they are not null or empty.
 /// 2. Generates nonces for XChaCha20-Poly1305 and AES encryption.
 /// 3. Encrypts the plaintext using XChaCha20-Poly1305 with the first key.
 /// 4. Encrypts the XChaCha20-Poly1305 ciphertext using AES encryption with the second key.
 /// 5. Clears sensitive information, such as encryption keys, from memory.
 /// </remarks>
 public static async Task<byte[]> EncryptAsyncV3(byte[] plaintext,
 byte[] key, byte[] key2, byte[] hMacKey)
 {
 try
 {
 // Parameter checks
 if (plaintext == Array.Empty<byte>())
 throw new ArgumentException(@"Value was empty.", nameof(plaintext));
 // Generate nonces
 var nonce = RndByteSized(CryptoConstants.ChaChaNonceSize);
 var nonce2 = RndByteSized(CryptoConstants.Iv);
 // Encrypt using XChaCha20-Poly1305
 var cipherText = SecretAeadXChaCha20Poly1305.Encrypt(plaintext, nonce, key);
 // Encrypt the XChaCha20-Poly1305 ciphertext using AES
 var cipherTextL2 = await EncryptAsync(cipherText, key2, nonce2, hMacKey);
 // Clear sensitive information
 ClearSensitiveData(key, key2, hMacKey);
 // Concatenate nonces and the second level ciphertext
 return nonce.Concat(nonce2).Concat(cipherTextL2).ToArray();
 }
 catch (CryptographicException ex)
 {
 ClearSensitiveData(key, key2, hMacKey);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (Exception ex)
 {
 ClearSensitiveData(key, key2, hMacKey);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 }
 /// <summary>
 /// Decrypts the provided ciphertext using a combination of XChaCha20-Poly1305 and AES decryption.
 /// </summary>
 /// <param name="cipherText">The ciphertext to be decrypted.</param>
 /// <param name="key">The first decryption key.</param>
 /// <param name="key2">The second decryption key.</param>
 /// <param name="hMacKey">The HMAC key for authentication.</param>
 /// <returns>
 /// A Task that completes with the decrypted plaintext.
 /// If any error occurs during the process, returns an empty byte array.
 /// </returns>
 /// <remarks>
 /// This method performs the following steps:
 /// 1. Validates input parameters to ensure they are not null or empty.
 /// 2. Extracts nonces and ciphertext from the provided cipherText.
 /// 3. Decrypts the ciphertext using AES decryption with the second key.
 /// 4. Decrypts the AES ciphertext using XChaCha20-Poly1305 with the first key.
 /// 5. Clears sensitive information, such as decryption keys, from memory.
 /// </remarks>
 public static async Task<byte[]> DecryptAsyncV3(byte[] cipherText,
 byte[] key, byte[] key2, byte[] hMacKey)
 {
 try
 {
 // Parameter checks
 if (cipherText == Array.Empty<byte>())
 throw new ArgumentException(@"Value was empty.", nameof(cipherText));
 // Extract nonces and ciphertext
 var nonce = new byte[CryptoConstants.ChaChaNonceSize];
 Buffer.BlockCopy(cipherText, 0, nonce, 0, nonce.Length);
 var nonce2 = new byte[CryptoConstants.Iv];
 Buffer.BlockCopy(cipherText, nonce.Length, nonce2, 0, nonce2.Length);
 var cipherResult =
 new byte[cipherText.Length - nonce2.Length - nonce.Length];
 Buffer.BlockCopy(cipherText, nonce.Length + nonce2.Length, cipherResult, 0,
 cipherResult.Length);
 // Decrypt using AES with the second key
 var resultL2 = await DecryptAsync(cipherResult, key2, hMacKey);
 // Decrypt the AES ciphertext using XChaCha20-Poly1305 with the first key
 var resultL0 = SecretAeadXChaCha20Poly1305.Decrypt(resultL2, nonce, key);
 // Clear sensitive information
 ClearSensitiveData(key, key2, hMacKey);
 return resultL0;
 }
 catch (CryptographicException ex)
 {
 ClearSensitiveData(key, key2, hMacKey);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 catch (Exception ex)
 {
 ClearSensitiveData(key, key2, hMacKey);
 ErrorLogging.ErrorLog(ex);
 return Array.Empty<byte>();
 }
 }

Tests

 /// <summary>
 /// This method demonstrates the decryption process using the Crypto class, including key derivation,
 /// encryption, and decryption of test data. It uses Argon2Id for key derivation, and two keys along with an HMAC key for encryption.
 /// </summary>
 [TestMethod]
 public async Task Decryption()
 {
 try
 {
 // Derive keys using Argon2Id
 var bytes = await Crypto.Argon2Id(EncryptionExample.passString.ToCharArray(), EncryptionExample.Salt, 128);
 // Initialize key arrays
 var key = new byte[32];
 var key2 = new byte[32];
 var hMacKey = new byte[64];
 // Copy bytes to key arrays
 Buffer.BlockCopy(bytes, 0, key, 0, key.Length);
 Buffer.BlockCopy(bytes, key.Length, key2, 0, key2.Length);
 Buffer.BlockCopy(bytes, key.Length + key2.Length, hMacKey, 0, hMacKey.Length);
 // Encrypt test data using EncryptAsyncV3Debug
 byte[] encryptedTest = await Crypto.EncryptAsyncV3Debug(
 DataConversionHelpers.StringToByteArray(EncryptionExample.plainText),
 EncryptionExample.Nonce, EncryptionExample.Nonce2, key, key2, hMacKey);
 // Convert encrypted result to Base64 string
 string encryptResult = DataConversionHelpers.ByteArrayToBase64String(encryptedTest);
 // Assert the encryption results
 Assert.IsNotNull(encryptedTest);
 Assert.AreEqual(EncryptionExample.ExpectedResult, encryptResult);
 // Derive keys again for decryption
 bytes = await Crypto.Argon2Id(EncryptionExample.passString.ToCharArray(), EncryptionExample.Salt, 128);
 // Reset key arrays
 key = new byte[32];
 key2 = new byte[32];
 hMacKey = new byte[64];
 // Copy bytes to key arrays
 Buffer.BlockCopy(bytes, 0, key, 0, key.Length);
 Buffer.BlockCopy(bytes, key.Length, key2, 0, key2.Length);
 Buffer.BlockCopy(bytes, key.Length + key2.Length, hMacKey, 0, hMacKey.Length);
 // Decrypt the test data
 byte[] decryptedTest = await Crypto.DecryptAsyncV3(
 DataConversionHelpers.Base64StringToByteArray(encryptResult), key, key2, hMacKey);
 // Convert decrypted result to string
 string decryptResult = DataConversionHelpers.ByteArrayToString(decryptedTest);
 // Assert the decryption results
 Assert.IsNotNull(decryptResult);
 Assert.AreEqual(EncryptionExample.plainText, decryptResult);
 }
 catch (Exception ex)
 {
 // Handle any exceptions during decryption and fail the test
 Assert.Fail($"Decryption test failed: {ex.Message}");
 }
 }
 /// <summary>
 /// This method tests the encryption process using the Crypto class, including key derivation, and encryption of test data.
 /// It uses Argon2Id for key derivation, and two keys along with an HMAC key for encryption.
 /// </summary>
 [TestMethod]
 public async Task Encryption()
 {
 try
 {
 // Derive keys using Argon2Id
 var bytes = await Crypto.Argon2Id(
 EncryptionExample.passString.ToCharArray(),
 EncryptionExample.Salt, 128);
 // Initialize key arrays
 var key = new byte[32];
 var key2 = new byte[32];
 var hMacKey = new byte[64];
 // Copy bytes to key arrays
 Buffer.BlockCopy(bytes, 0, key, 0, key.Length);
 Buffer.BlockCopy(bytes, key.Length, key2, 0, key2.Length);
 Buffer.BlockCopy(bytes, key.Length + key2.Length, hMacKey, 0, hMacKey.Length);
 // Encrypt test data using EncryptAsyncV3Debug
 byte[] encryptedTest = await Crypto.EncryptAsyncV3Debug(
 DataConversionHelpers.StringToByteArray(EncryptionExample.plainText),
 EncryptionExample.Nonce, EncryptionExample.Nonce2, key, key2, hMacKey);
 // Convert encrypted result to Base64 string
 string base64Result = DataConversionHelpers.ByteArrayToBase64String(encryptedTest);
 // Assert the encryption results
 Assert.IsNotNull(encryptedTest);
 Assert.AreEqual(EncryptionExample.ExpectedResult, base64Result);
 }
 catch (Exception ex)
 {
 // Handle any exceptions during encryption and fail the test
 Assert.Fail($"Encryption test failed: {ex.Message}");
 }
 }
 /// <summary>
 /// This method tests the password hashing functionality in the Crypto class using Argon2Id.
 /// It hashes a password represented as a character array with a specified salt and checks for a non-null result.
 /// </summary>
 [TestMethod]
 public async Task Hash()
 {
 try
 {
 // Convert the password string to a character array
 char[] passArray = EncryptionExample.passString.ToCharArray();
 // Hash the password using Argon2Id with a specified salt and a target hash length of 32 bytes
 byte[] result = await Crypto.Argon2Id(passArray, EncryptionExample.Salt, 32) ?? Array.Empty<byte>();
 // Assert that the result is not null
 Assert.IsNotNull(result);
 }
 catch (Exception ex)
 {
 // Handle any exceptions during password hashing and fail the test
 Assert.Fail($"Password hashing test failed: {ex.Message}");
 }
 }
 /// <summary>
 /// This method tests the password hash comparison functionality in the Crypto class.
 /// It compares two password hashes, demonstrating both a mismatched and a matched case.
 /// </summary>
 [TestMethod]
 public async Task CompareHash()
 {
 try
 {
 // Define two password hashes as hexadecimal strings
 string hash1 = "5e463665b62f4ec740145bd1a5062602bdec0c462063ff299303cc8d6f413193";
 string hash2 = "a60d2335fc1fbb39b933a2c5acf9f4f9c5ff1b292cf30d82e246107566efef12";
 // Convert hexadecimal strings to byte arrays
 byte[] hashBytes1 = DataConversionHelpers.StringToByteArray(hash1);
 byte[] hashBytes2 = DataConversionHelpers.StringToByteArray(hash2);
 // Assert that the ComparePassword method correctly identifies mismatched hashes
 Assert.IsFalse(await Crypto.ComparePassword(hashBytes1, hashBytes2));
 // Assert that the ComparePassword method correctly identifies matched hashes
 Assert.IsTrue(await Crypto.ComparePassword(hashBytes1, hashBytes1));
 }
 catch (Exception ex)
 {
 // Handle any exceptions during hash comparison and fail the test
 Assert.Fail($"Hash comparison test failed: {ex.Message}");
 }
 }

Answer 1

Design

The list of methods in the first code block shows that little effort has been spent trying to split UI and what I generally refer to as "business logic". The protocol related stuff needs to be separated, otherwise it will be hard to test or integrate into another program at at a later date.

Generally it is best to define the cryptographic protocol and implement that in a separate class or set of classes, usually in a different namespace to make sure that the encapsulation is correctly performed. That said, at least the methods do seem to have this split.

---

SetArray is very far from a good name. Set what array? Where is the array parameter? This leaves any programmer guessing and having to read the code to find out what it does. The comment:

/// Sets the password array based on the content of the <see cref="SecureString"/>.

Makes it worse; it doesn't refer to the name or location of string that is set but it refers only to its type. This also indicates that there is a bigger problem here; the developer is targeting themselves instead of a future programmer.

---

Effort has been taken to make the code function asynchronously. However, clearly the code is not designed as a library. In that sense the:

await Crypto.EncryptFile(Authentication.CurrentLoggedInUser, _passwordArray, _loadedFile)`

seems to be a bit of a letdown. Why go through all this trouble? Then again, it could be useful for future designs, so now it is in there I'd leave it be.

Authentication.GetUserSaltAsync(userName)

Generally things like retrieving a salt is a lightweight operation that always needs to be performed before key derivation takes place. It makes even less sense to make this asynchronous.

---

File encryption seems to be performed as a single block encrypt. Although pre-mature optimization should be frowned upon it is extremely arguable if piecemeal or stream wise encryption should not be used instead. The fact that the garbage collector is called upon right after shows that the developer is aware of the obvious drawbacks of reading all the data into memory and then encrypting it. Basically this means that the memory in the machine should at least hold the data twice (and it is more).

---

A decision has been taken to only encrypt textual files. Besides the obvious loss that can be sustained during the read out of the files - not all byte codes can be converted into characters - it also requires another copy of the bytes in the file to be present as character strings.

The fact that this design decision has not been documented in the method description is ... irresponsible and extremely dangerous.

---

In the UI code the decision also has been taken to base 64 encode the result. This will mean that the memory is hit yet another time. It is also a bit strange that the encryption function requires strings as input, while it returns bytes - which are then converted back into a string. That means that the input / output parameters are not "symmetric".

Cryptography

When designing a protocol it should be clearly documented and referred to from the code. This is especially true if the encryption is performing non-standard practices such as double encryption.

---

It isn't clear why double encryption is performed, or what the use case of it is here. Symmetric encryption using either AES or ChaCha is generally considered safe. Often the problems are around the encryption that is performed, not the algorithms themselves.

---

Currently the salt is always the same for the user, which means each and every file of the user is encrypted with the same set of keys. This is not necessarily wrong but it should be noted that e.g. changing the password means having to reencrypt each and every file. The protocol isn't really flexible; the protocol should have been designed to allow for (key) change.

---

Generally we may need to configure the Argon2 security parameter (work factor or iteration count) because systems get faster all the time. Currently this doesn't seem possible as it is a literal - not even a constant - in the code. Of course, if the parameter is changed it will lead to having to reencrypt every file that has been encrypted.

---

The amount of parallelism is really set too high. Environment.ProcessorCount returns "The number of logical processors on the machine". So doubling this value will just add overhead.

The Argon2 parallelism parameter is often set to 1 because setting it higher only helps Argon2 to return faster. It doesn't bring down CPU-time nor does it help to make Argon2 more secure as the CPU time required by attackers doesn't change either.

---

Destroying temporary keys is a very good idea, but not if they are input parameters. This will lead to all zero or random keys, which may then be reused by e.g. the next programmer. This will result in ciphertext that is either impossible, too simple to decrypt, or both. It is better to leave the destruction to the "owner" of the keys, even though the destruction of the keys has been clearly documented.

---

Using AeadXChaCha20Poly1305 together with a random IV is an absolutely fine choice.

Coding decisions

if (encryptedFile == Array.Empty<byte>()) ... throw

Empty is not a good indicator of something being wrong. Files may well be empty. For instance lock files may possibly have a value, or they may not. So with this decision empty lock files are bad, lock files with a value are OK.

---

return Array.Empty<byte>();

Similarly, returning an empty array instead of ciphertext is really dangerous, as an empty array could be a valid ciphertext in some contexts - although usually some kind of data such as an IV or authentication tag always required.

An argument can also be made that handling the exception at the encryption level is making the decision at the wrong location or level. This also has to do with the problem of not separating the UI code from the encryption code.

---

Quite often strings and arrays are used as parameters, while higher level abstractions such as files and keys exist. Using those will also mean that errors may be caught earlier.

• Maarten Bodewes: Haven't looked at the test code, sorry.