Very simple asymmetric RSA encryption in C#

Question

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..

1. Can anyone help make it even better? Are there any major flaws in this that makes it really easy to decrypt?
2. 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 < 512)
                throw new Exception("Key should be multiple of two and greater than 512.");

            var response = new EncryptorRSAKeys();

            using (var provider = new RSACryptoServiceProvider(keySize))
            {
                var publicKey = provider.ToXmlString(false);
                var privateKey = provider.ToXmlString(true);

                var publicKeyWithSize= IncludeKeyInEncryptionString(publicKey, keySize);
                var privateKeyWithSize = IncludeKeyInEncryptionString(privateKey, keySize);

                response.PublicKey = publicKeyWithSize;
                response.PrivateKey = privateKeyWithSize;
            }

            return response;
        }

        public static string EncryptText(string text, string publicKey)
        {
            int keySize = 0;
            string publicKeyXml = "";

            GetKeyFromEncryptionString(publicKey, out keySize, out publicKeyXml);

            var encrypted = Encrypt(Encoding.UTF8.GetBytes(text), keySize, publicKeyXml);
            return Convert.ToBase64String(encrypted);
        }

        private static byte[] Encrypt(byte[] data, int keySize, string publicKeyXml)
        {
            if (data == null || data.Length == 0) throw new ArgumentException("Data are empty", "data");
            int maxLength = GetMaxDataLength(keySize);
            if (data.Length > 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 provider.Encrypt(data, _optimalAsymmetricEncryptionPadding);
            }
        }

        public static string DecryptText(string text, string privateKey)
        {
            int keySize = 0;
            string publicAndPrivateKeyXml = "";

            GetKeyFromEncryptionString(privateKey, out keySize, out publicAndPrivateKeyXml);

            var decrypted = Decrypt(Convert.FromBase64String(text), keySize, publicAndPrivateKeyXml);
            return Encoding.UTF8.GetString(decrypted);
        }

        private static byte[] Decrypt(byte[] data, int keySize, string publicAndPrivateKeyXml)
        {
            if (data == null || data.Length == 0) throw new ArgumentException("Data are empty", "data");
            if (!IsKeySizeValid(keySize)) throw new ArgumentException("Key size is not valid", "keySize");
            if (String.IsNullOrEmpty(publicAndPrivateKeyXml)) throw new ArgumentException("Key is null or empty", "publicAndPrivateKeyXml");

            using (var provider = new RSACryptoServiceProvider(keySize))
            {
                provider.FromXmlString(publicAndPrivateKeyXml);
                return provider.Decrypt(data, _optimalAsymmetricEncryptionPadding);
            }
        }

        public static int GetMaxDataLength(int keySize)
        {
            if (_optimalAsymmetricEncryptionPadding)
            {
                return ((keySize - 384) / 8) + 7;
            }
            return ((keySize - 384) / 8) + 37;
        }

        public static bool IsKeySizeValid(int keySize)
        {
            return keySize >= 384 &&
                    keySize <= 16384 &&
                    keySize % 8 == 0;
        }

        private static string IncludeKeyInEncryptionString(string publicKey, int keySize)
        {
            return Convert.ToBase64String(Encoding.UTF8.GetBytes( keySize.ToString() + "!" + publicKey));
        }

        private static void GetKeyFromEncryptionString(string rawkey, out int keySize, out string xmlKey)
        {
            keySize = 0;
            xmlKey = "";

            if (rawkey != null && rawkey.Length > 0)
            {
                byte[] keyBytes = Convert.FromBase64String(rawkey);
                var stringKey = Encoding.UTF8.GetString(keyBytes);

                if (stringKey.Contains("!"))
                {
                    var splittedValues = stringKey.Split(new char[] { '!' }, 2);

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

Answer 1

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().