patterncsharpModerate
C# AES Encryption
Viewed 0 times
encryptionaesstackoverflow
Problem
I've been researching AES encryption a bit over the past several days. The official (MSDN) examples I've seen are encrypting and decrypting using the same AES instance. They don't go in to what to do when generating and saving an encrypted value with AES and needing to decrypt it later with another AES instance.
I came up with the following and am wondering if there is anything wrong with it, aside from defaulting the password to a static value (I will be developing something else to manage encryption passwords)? It generates a random salt on encryption and stores it with the encrypted cipher prior to Base64 encoding. This assures that running the encryption twice on the same input does not result in the same cipher text.
```
public static string Encrypt(string plainText, string password = "BadgersAreAwesome")
{
if (plainText == null)
throw new ArgumentNullException("plainText");
if (password == null)
throw new ArgumentNullException("password");
// Will return the cipher text
string cipherText = "";
// Utilizes helper function to generate random 16 byte salt using RNG
byte[] salt = GenerateSaltBytes(SaltSize);
// Convert plain text to bytes
byte[] plainBytes = Encoding.Unicode.GetBytes(plainText);
// create new password derived bytes using password/salt
using (Rfc2898DeriveBytes pdb = new Rfc2898DeriveBytes(password, salt))
{
using (Aes aes = AesManaged.Create())
{
// Generate key and iv from password/salt and pass to aes
aes.Key = pdb.GetBytes(aes.KeySize / 8);
aes.IV = pdb.GetBytes(aes.BlockSize / 8);
// Open a new memory stream to write the encrypted data to
using (MemoryStream ms = new MemoryStream())
{
// Create a crypto stream to perform encryption
using (CryptoStream cs = new Crypt
I came up with the following and am wondering if there is anything wrong with it, aside from defaulting the password to a static value (I will be developing something else to manage encryption passwords)? It generates a random salt on encryption and stores it with the encrypted cipher prior to Base64 encoding. This assures that running the encryption twice on the same input does not result in the same cipher text.
```
public static string Encrypt(string plainText, string password = "BadgersAreAwesome")
{
if (plainText == null)
throw new ArgumentNullException("plainText");
if (password == null)
throw new ArgumentNullException("password");
// Will return the cipher text
string cipherText = "";
// Utilizes helper function to generate random 16 byte salt using RNG
byte[] salt = GenerateSaltBytes(SaltSize);
// Convert plain text to bytes
byte[] plainBytes = Encoding.Unicode.GetBytes(plainText);
// create new password derived bytes using password/salt
using (Rfc2898DeriveBytes pdb = new Rfc2898DeriveBytes(password, salt))
{
using (Aes aes = AesManaged.Create())
{
// Generate key and iv from password/salt and pass to aes
aes.Key = pdb.GetBytes(aes.KeySize / 8);
aes.IV = pdb.GetBytes(aes.BlockSize / 8);
// Open a new memory stream to write the encrypted data to
using (MemoryStream ms = new MemoryStream())
{
// Create a crypto stream to perform encryption
using (CryptoStream cs = new Crypt
Solution
- You're obtaining more than 20 bytes from PBKDF2-HMAC-SHA-1 and the attacker doesn't need the data from the second block (12 IV bytes), so your code slows down defenders by a factor 2 without affecting attackers.
-
generate random 16 bit salt using RNG
16 bits is very short. You should use 16 bytes or 128 bits. I suspect this is a typo in the comment, since
Rfc2898DeriveBytes rejects salts shorter than 8 bytes.-
1000 iterations of PBKDF2-HMAC-SHA-1 is pretty low. I'd use at least 10000.
- You don't have a MAC, leaving you open to active attacks, such as padding oracles
- if you use
aes.CreateEncryptor().TransformFinalBlockyou can throw out all those streams
- I'd consider using UTF-8 over UTF-16. It's shorter for most non-Asian texts.
-
You're using password based encryption. That can be the right choice, but depending on your application you should consider:
-
Key based encryption, with the key stored in a password encrypted file key file (similar to SSH keys).
This means you only need to run the expensive key derivation operation when opening the password file, instead of per message. So you can probably afford more iterations, improving security.
- Key based encryption with a randomly generated key
- An encrypted network protocol like TLS
-
Consider using a better password hash, like bcrypt or scrypt and/or a faster (native) implementation with more iterations. That way you strengthen the password more, making password guessing attacks more expensive.
Rfc2898DeriveBytesgenerates a random salt if you pass in the length of the salt, so you don't need your own method.
Context
StackExchange Code Review Q#78434, answer score: 12
Revisions (0)
No revisions yet.