# Implementing RSA-AES encryption protocol

I am trying to write a simple program that allows to encrypt a string of plaintext so that it can be transmitted via an "insecure channel" by which I mean a "regular chat"; be it Skype, Facebook, or even email or something similar. This is what I came up with:

At first, one user, Alice, will generate a cryptographically secure key for this "session" (which can - I think - be very long, so long as new IVs can be generated). It will be used for the AES algorithm.

byte[] sessionKey = GenerateNewKey();


I/You don't need to worry about GenerateNewKey().

This key needs to be transferred. RSA should be used as the asymmetric encryption algorithm. At first, Bob will extract a new private-public Key set RSAParameters from a RSACryptoServiceProvider . Furthermore, I will take the public key as MyPublicKey from that RSAParameters.

RSAParameters extractedParameters;
using (RSACryptoServiceProvider prov = new RSACryptoServiceProvider(
RsaKeyLength /* const, >= 4096  */)
{
try
{
extractedParameters = prov.ExportParameters(true);
}
finally
{
prov.PersistKeyInCsp = false;
}
}
byte[] bobsPublicKey = extractedParameters.Modulus;


Now, this byte[] bobsPublicKey will be given to Alice, by writing it into the chat. She will encrypt the byte[] sessionKey as follows:

byte[] encryptedAES;
using (RSACryptoServiceProvider prov = new RSACryptoServiceProvider())
{
try
{
prov.PersistKeyInCsp = false;
RSAParameters param = prov.ExportParameters(false);
param.Modulus = bobsPublicKey;
prov.ImportParameters(param);
prov.PersistKeyInCsp = false;
encryptedAES = prov.Encrypt(sessionKey, true /* use OAEP */);
}
finally
{
prov.PersistKeyInCsp = false;
}
}


Then, she will write encryptedAES into the chat, which will be received by Bob, who finally gets the AES key sessionKey back via:

byte[] sessionKey;
using (RSACryptoServiceProvider prov = new RSACryptoServiceProvider())
{
try
{
prov.PersistKeyInCsp = false;
prov.ImportParameters(extractedParameters);
prov.PersistKeyInCsp = false;
sessionKey = prov.Decrypt(encryptedAES, true /* use OAEP */);
}
finally
{
prov.PersistKeyInCsp = false;
}
}


Now, lets say Alice wants to send a message to Bob. Assume that for example _aes = "AES, 256 key, CBC, PKCS7"

byte[] iv = GenerateIV(8 /*byte length*/);
byte[] encryptedMessage;
using (MemoryStream ms = new MemoryStream())
{
ms.Write(iv, 0, iv.Length); // Prepend the IV to the Stream
// Append the encrypted message
using (ICryptoTransform trans = _aes.CreateEncryptor(sessionKey, iv))
using (CryptoStream cs = new CryptoStream(ms, trans, CryptoStreamMode.Write)
using (StreamWriter sw = new StreamWriter(cs))
{
sw.Write(message);
sw.Flush();
cs.FlushFinalBlock();
}
encryptedMessage = ms.ToArray();
}


encryptedMessage will be written to the chat. Bob decypts like:

// extract the IV
byte[] iv = GetIV(encryptedMessage);
// the rest will be the actual message
byte[] data = GetData(encryptedMessage); // Don't worry about those two

byte[] decryptedMessage;
using (MemoryStream ms = new MemoryStream(data, iv.Length, 0))
using (ICryptoTransform trans = _aes.CreateDecryptor(sessionKey, iv))
using (CryptoStream cs = new CryptoStream(ms, trans, CryptoStreamMode.Read)


And finally, decryptedMessage should contain the message Alice tried to send.

My request is, that this code should be reviewed regarding

• whether this is a correct - that is secure - implementation of such an "algorithm pair" for transmitting messages via an unsafe and permanently visible channel
• whether there are redundant calls to the .Net classes
• (more things that can be improved regarding the first point)

whether this is a correct - that is secure - implementation of such an "algorithm pair" for transmitting messages via an unsafe and permanently visible channel

Your approach provides zero forward security. As soon as Bobs private key is compromised, all past communication with Bob is compromised as well.

You should use a Diffie-Hellman like handshake protocol for agreeing on a common secret for the actual communication.

You are not providing any form of authentication either.

Right now, Eva can perform a trivial man-in-the-middle attack. While Alice could in theory still verify Bobs identity by comparing the provided public key against a trusted database, Bob in return has no means to verify Alice's identity.

A simple band-aid would be to sign the chosen communication key with Alice's public RSA key, so that Bob has the confirmation that the key was in fact chosen by Alice, and not Eva.

For that, just apply a cryptographic hash function to the key, and encrypt the hash with Alice's private RSA key. Bob then uses Alice's known public RSA key to decrypt the signature and verifies that he can generate the same hash. Alternatively you can hash the encrypted shared secret instead of the plain text, if you don't wish to leak information on the plain text.

Be wary that this is still potentially subject to replay attacks. In order to protect against that, Bob should start the communication with a plaintext nonce which Alice needs to include in the signature. This may be an arbitrary string, but something as simple as the current time stamp suffices.

This is still only authentication though, and does not remove the need to agree on a new key via DH to provide forward security. You need to get rid of the temporary key as fast as possible.

The problem of replay attacks also applies to the actual messages themselves. As each message is only protected by the shared secret, you are unable to detect when Eve maliciously repeats a recorded message of the same session repeatedly.

A practical solution to that, is to append a fresh nonce to each sent message, which is expected to be repeated in the next message. Now you can tell for sure if the order of messages in one direction has been altered.

As for ensuring the coherence of a dialog, you might want to think about acknowledgment messages, which confirm receiving a content message.

byte[] bobsPublicKey = extractedParameters.Modulus;


That's not the public key. You need both the public exponent and the modulus.

using (MemoryStream ms = new MemoryStream(data, iv.Length, 0))
using (ICryptoTransform trans = _aes.CreateDecryptor(sessionKey, iv))
using (CryptoStream cs = new CryptoStream(ms, trans, CryptoStreamMode.Read)


More curly braces and indentation please. The syntax with multiple using directives on the same indentation level should IMHO only be used when the declared disposables are independent. If they are dependent, they can no longer have the same life time, and that should be reflected by the code structure:

using (MemoryStream ms = new MemoryStream(data, iv.Length, 0))
using (ICryptoTransform trans = _aes.CreateDecryptor(sessionKey, iv))
{
using (CryptoStream cs = new CryptoStream(ms, trans, CryptoStreamMode.Read)
{