patterncsharpMinor
AES Encryption C# .NET
Viewed 0 times
encryptionnetaes
Problem
I have written a tool for encrypting string using the AesCryptoServiceProvider in .NET. The folllowing parameters are used:
per encryption operation), stored in last 16 bytes of final message
Overview of code
I have added two functions below. The first prepares a message for encryption, takes the result and appends the Initialization Vector to the cipher message. The second function encrypts the provided message using the specified key and initialization vector.
Obviously, i'm interested in any security holes that exist with this code. Also any optimizations are very much appreciated.
Side questions. In development I always store bytes as strings in their hex representation, seen below. Is this bad? The only other alternative I see is base64 encoding? Which is better?
```
public void cryptoExample {
AesCryptoServiceProvider aes = new AesCryptoServiceProvider();
aes.KeySize = 128;
aes.BlockSize = 128;
aes.Key = Helper.StringToByteArray("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"); // Sample key
aes.GenerateIV();
String plainMessage = "Hello AES Crypto!!";
String encryptedMessage = Helper.ByteArrayToString(CryptoHelper.EncryptStringToBytes_Aes(plainMessage, aes.Key, aes.IV));
// Must always use a random IV, meaning it needs to be stored with the cipher message.
// Append it to the end. IV is a constant 16 bytes so we just extract and remove it before decryption.
String strIv = Helper.ByteArrayToString(aes.IV);
String messageToTransmit = encryptedMessage += strIv;
}
public static byte[] EncryptStringToBytes_Aes(string plainText, byte[] Key, byte[] IV)
{
// Check arguments.
if (plainText == null || plainText.Length <= 0)
throw new ArgumentNullException("plainText");
if (Key == null || Key.Length <= 0)
throw new ArgumentNullException("Key");
if (IV == null || IV.Length <= 0)
- Block Cipher Mode: CBC
- Initialization Vector: 16 bytes (randomized
per encryption operation), stored in last 16 bytes of final message
- Key/Block size: 128 bit Padding: PKCS #7
Overview of code
I have added two functions below. The first prepares a message for encryption, takes the result and appends the Initialization Vector to the cipher message. The second function encrypts the provided message using the specified key and initialization vector.
Obviously, i'm interested in any security holes that exist with this code. Also any optimizations are very much appreciated.
Side questions. In development I always store bytes as strings in their hex representation, seen below. Is this bad? The only other alternative I see is base64 encoding? Which is better?
```
public void cryptoExample {
AesCryptoServiceProvider aes = new AesCryptoServiceProvider();
aes.KeySize = 128;
aes.BlockSize = 128;
aes.Key = Helper.StringToByteArray("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"); // Sample key
aes.GenerateIV();
String plainMessage = "Hello AES Crypto!!";
String encryptedMessage = Helper.ByteArrayToString(CryptoHelper.EncryptStringToBytes_Aes(plainMessage, aes.Key, aes.IV));
// Must always use a random IV, meaning it needs to be stored with the cipher message.
// Append it to the end. IV is a constant 16 bytes so we just extract and remove it before decryption.
String strIv = Helper.ByteArrayToString(aes.IV);
String messageToTransmit = encryptedMessage += strIv;
}
public static byte[] EncryptStringToBytes_Aes(string plainText, byte[] Key, byte[] IV)
{
// Check arguments.
if (plainText == null || plainText.Length <= 0)
throw new ArgumentNullException("plainText");
if (Key == null || Key.Length <= 0)
throw new ArgumentNullException("Key");
if (IV == null || IV.Length <= 0)
Solution
- You can simplify it by replacing all those streams with a call to
encrypter.TransformFinalBlock.
- You don't have a MAC, so an attacker who can send a modifier message to the decrypter can probably exploit a padding oracle to decrypt the message
- Why ask the caller for an IV? You could generate it internally. If you want to keep a deterministic implementation around, at least create an overload without IV.
Helper.ByteArrayToStringandStringToByteArrayshould have a name that says which encoding (hex, Base64, etc.) they use. I'd useBytesToHexStringandHexStringToBytes.
Context
StackExchange Code Review Q#36453, answer score: 6
Revisions (0)
No revisions yet.