Encrypts using AES with random initialization vector meant for data with limited visibility and then long rest

I've done some research, reviewed countless SO questions that appeared similar, leading me to endless rabbit holes.

My solution is working, but I don't like it. I want to create a random IV for AES and store it with my cipher. You'll see various approaches based on the comments in my code. This final version is mostly from another source I found with some changes. What I was hoping to do is instead of generating two separate strings and concatenating them together, I could write into the same byte array and break it apart later. Goal is AES256 with random IV.

This led to errors like:

• Invalid characters in base64 string
• length of data to encrypt is invalid
• padding is invalid and cannot be removed

So here's my code. And of course, if there's a hole or ten, advice welcome. Feel free to share examples in c#. This project just happens to be in VB.net

Public Function EncryptText(input As String) As String

Dim password As String = "THISISONLYTEMPORARY"

Dim bytesToBeEncrypted As Byte() = Encoding.UTF8.GetBytes(input)
Dim result As String = AES_Encrypt(bytesToBeEncrypted, passwordBytes)

Return result
End Function

Public Function DecryptText(ByVal input As String) As String

Dim password As String = "THISISONLYTEMPORARY"
'Dim savedSalt As Byte() = Convert.FromBase64String(input.Split("%"c)(0))
'Dim savedValue As String = input.Split("%"c)(1)
Dim savedSalt As Byte() = Convert.FromBase64String(input.Substring(0, 44))
Dim savedValue As String = input.Substring(44, input.Length - 44)

Dim bytesToBeDecrypted As Byte() = Convert.FromBase64String(savedValue)
Dim result As String = AES_Decrypt(bytesToBeDecrypted, passwordBytes, savedSalt)
Return result
End Function

Public Function AES_Encrypt(ByVal bytesToBeEncrypted As Byte(), ByVal passwordBytes As Byte()) As String
Dim encryptedBytes As Byte() = Nothing

'Dim saltBytes As Byte() = New Byte() {1, 2, 3, 4, 5, 6, 7, 8}

Dim saltBytes = New Byte(31) {}
Using rng As RandomNumberGenerator = New RNGCryptoServiceProvider()
rng.GetBytes(saltBytes)
End Using

Dim saltToSave As String = Convert.ToBase64String(saltBytes)

Using ms As MemoryStream = New MemoryStream()

Using AES = New AesManaged()
AES.KeySize = 256
AES.BlockSize = 128
Dim key = New Rfc2898DeriveBytes(passwordBytes, saltBytes, 1000)
AES.Key = key.GetBytes(CInt(AES.KeySize / 8))
AES.IV = key.GetBytes(CInt(AES.BlockSize / 8))
AES.Mode = CipherMode.CBC

Using cs = New CryptoStream(ms, AES.CreateEncryptor(), CryptoStreamMode.Write)
'cs.Write(saltBytes, 0, saltBytes.Length) ' new
cs.Write(bytesToBeEncrypted, 0, bytesToBeEncrypted.Length)
cs.Close()
End Using

encryptedBytes = ms.ToArray()
End Using
End Using

Return saltToSave & Convert.ToBase64String(encryptedBytes)
End Function

Public Function AES_Decrypt(ByVal bytesToBeDecrypted As Byte(), ByVal passwordBytes As Byte(), savedSalt As Byte()) As String
Dim decryptedBytes As Byte() = Nothing
'Dim saltBytes As Byte() = New Byte() {1, 2, 3, 4, 5, 6, 7, 8}

Using ms As MemoryStream = New MemoryStream()

Using AES = New AesManaged()
AES.KeySize = 256
AES.BlockSize = 128
Dim key = New Rfc2898DeriveBytes(passwordBytes, savedSalt, 1000)
AES.Key = key.GetBytes(CInt(AES.KeySize / 8))
AES.IV = key.GetBytes(CInt(AES.BlockSize / 8))
AES.Mode = CipherMode.CBC

Using cs = New CryptoStream(ms, AES.CreateDecryptor(), CryptoStreamMode.Write)
cs.Write(bytesToBeDecrypted, 0, bytesToBeDecrypted.Length)
cs.Close()
End Using

decryptedBytes = ms.ToArray()
End Using
End Using

'Return Encoding.UTF8.GetString(decryptedBytes.Skip(savedSalt.Length).ToArray())
Return Encoding.UTF8.GetString(decryptedBytes.ToArray())
End Function


I was at one point joining my IV and cipher text with a % character and splitting those, but didn't like the visible clear separation which made the IV obvious.

Again this is working. My question is more about hardening it within the constraints that you see (A single private password and random IV) and ditching this string concatenation.

I posted this on stackoverflow but it was suggested to move to here because my solution is working. It was also suggested to not be concerned about splitting my encrypted string with "%" as you see commented out in my code and was pointed to this link: https://en.wikipedia.org/wiki/Kerckhoffs%27s_principle

Disclaimer: I'm not an expert in VB.NET / C#, so I can't and won't comment on things like code-style. I will however address the security of this code. My points are in order of decreasing severity.

The password is currently hard-coded into the source. I really hope this is just for demonstration purposes to produce a minimal working example. This is very bad practice that I would strongly recommend against, as it usually leads to the password ending up in some (public) code repository and not exclusively in the hands of your sysadmin or end-user where it belongs. Also putting the password into the source means that you need to re-compile and re-distribute the binary whenever you change the password, which will probably be a hassle...

The usual approach for handling passwords is to either put them into an environment variable or to query the user for them at runtime. If you have tight control over the file system a plain file or a file encrypted using a cloud key management service are also options.

Use of (plain) CBC-Mode

Currently this code uses the infamous CBC-mode which time and again has lead to attacks in when it was used in TLS. The problem here is that an attacker may modify the message in a malicious way, e.g. to exploit a parser bug or trigger some other reaction without the receiver having a high chance of noticing it before it's too late. The better solution is to use authenticated encryption (like AES-GCM) which will make sure the message is unmodified before handing out the plaintext.

Use of PBKDF2

Right now PKCS#5 PBKDF2 (Rfc2898DeriveBytes) is used to derive the key and IV from the password and the salt. This is an outdated standard, because it allows for easy parallelization of the password-guessing attacks using GPUs and similar specialized hardware. It is recommended to use Argon2 these days.

On a side note, .NET by default uses HMAC-SHA1 for PBKDF2 which has a 20 byte output width. The code at hand however is asking for 48 bytes of output. This means that (due to the bad design of PBKDF2) the complete 1000 iterations are actually run three times in a parallelizable fashion. An attacker can just run these three instances in parallel to speed up their attack and attackers of password hashing scheme usually have a lot of parallel hardware available, so that is not good.

The IV / Salt

The current generation strategy for the salt is fine even though 31 bytes is a bit of an odd choice for the size. As for the actual IV, as you are altering the key with each encrypted message due to always altering the salt, it can actually be constant. The contract of modern encryption schemes is that you must not use any (key,IV) pair after all and if you always change the key you are not violating that contract.

As for the salt, don't worry about it being public and recognizable. After all a determined attacker will only take a short amount of time to extract the salt from your format and you have no good method available to change that.