Very simple asymmetric RSA encryption in C#

Based on this: asymmetric encryption in C#

I added some more functionality to make it even easier to use (I combined the keySize and the keys into one base64 string).

My requirements are:

• I want a public and private string key

• A simple method to call to encrypt or decrypt another string.

This works, and the following test case returns that all is good.

[TestMethod]
public void EncryptionRSATest()
{
    int keySize = 1024;   
    var keys = EncryptorRSA.GenerateKeys(keySize);

    string text = "text for encryption";

    string encrypted = EncryptorRSA.EncryptText(text, keys.PublicKey);
    string decrypted = EncryptorRSA.DecryptText(encrypted, keys.PrivateKey);

    Assert.IsTrue(text == decrypted);
}

So..

• Can anyone help make it even better? Are there any major flaws in this that makes it really easy to decrypt?

• Is it a horrible idea to add the keySize to the key?

This is the class:

```
using System;
using System.Web;
using System.Text;
using System.Linq;
using System.Collections.Generic;
using System.Security.Cryptography;

namespace JensB.Encryption
{
[Serializable]
public class EncryptorRSAKeys
{
public string PublicKey { get; set; }
public string PrivateKey { get; set; }
}

public static class EncryptorRSA
{
private static bool _optimalAsymmetricEncryptionPadding = false;

public static EncryptorRSAKeys GenerateKeys(int keySize)
{
if (keySize % 2 != 0 || keySize maxLength) throw new ArgumentException(String.Format("Maximum data length is {0}", maxLength), "data");
if (!IsKeySizeValid(keySize)) throw new ArgumentException("Key size is not valid", "keySize");
if (String.IsNullOrEmpty(publicKeyXml)) throw new ArgumentException("Key is null or empty", "publicKeyXml");

using (var provider = new RSACryptoServiceProvider(keySize))
{
provider.FromXmlString(publicKeyXml);
return p

Use proper Exception types

throw new Exception("Key should be multiple of two and greater than 512.");

It's not recommended to throw objects of type Exception. That's the base type for many kinds of exceptions! If you catch it you'll end up catching a bunch of other stuff. It's preferable to create your own type that inherits from Exception or one of its child classes such as ArgumentOutOfRangeException.

Also, use the most specific exception possible; for instance

if (String.IsNullOrEmpty(publicKeyXml)) throw new ArgumentException("Key is null or empty", "publicKeyXml");

should use ArgumentNullException. If you end up having a lot of exceptions that are more specific than something provided, create a child class.

Magic constants make code hard to understand

var publicKey = provider.ToXmlString(false);
var privateKey = provider.ToXmlString(true);

The false and true look like magic here. It's not clear what their purpose is. Instead of just using the bool literal, use the name of the parameter:

var publicKey = provider.ToXmlString(includePrivateParameters: false);
var privateKey = provider.ToXmlString(includePrivateParameters: true);

Pokemon exception handling

try
{
   keySize = int.Parse(splittedValues[0]);
   xmlKey = splittedValues[1];
}
catch (Exception e) { }

Why Pokemon? Because this code "gotta catch 'em all!". Seriously, this is a bad practice, it ends up catching exceptions you don't want to (for instance what if splittedValues ends up having only one element). Instead you should use int.TryParse().

StackExchange Code Review Q#92761, answer score: 4