5
\$\begingroup\$

I built this wrapper over the System.Security.Cryptography.Aes encryption/decryption so that one need only instance the class and call the appropriate methods. This makes it much simpler to work with AES encryption so that developers don't need to reuse the same AES code everywhere they need encryption.

Any and all comments are much appreciated.

/// <summary>
/// Provides Cryptography methods based on AES cryptography implementation.
/// </summary>
public class AesCrypto
{
    /// <summary>
    /// Gets or sets the salt to use with AES encryption.
    /// </summary>
    public byte[] Salt { get; set; }

    /// <summary>
    /// Gets or sets the passphrase for use with AES encryption.
    /// </summary>
    public string Passphrase { get; set; }

    /// <summary>
    /// Constructs a new instance of <see cref="AesCrypto"/> from the specified values.
    /// </summary>
    /// <param name="salt">The <see cref="Salt"/> used in encryption.</param>
    /// <param name="passphrase">The <see cref="Passphrase"/> used in encryption.</param>
    public AesCrypto(byte[] salt, string passphrase)
    {
        if (string.IsNullOrWhiteSpace(passphrase))
        {
            throw new ArgumentException($"The parameter {nameof(passphrase)} is required.");
        }

        if (salt == null || salt.Length == 0)
        {
            throw new ArgumentException($"The parameter {nameof(salt)} is required.");
        }

        Salt = salt;
        Passphrase = passphrase;
    }

    /// <summary>
    /// Uses AES encryption to encrypt a string of data.
    /// </summary>
    /// <param name="clearText">The data to encrypt. Data is expected to be Unicode.</param>
    /// <returns>An encrypted Base64 string.</returns>
    public string AesEncrypt(string clearText)
    {
        byte[] clearBytes = Encoding.Unicode.GetBytes(clearText);

        using (Aes encryptor = Aes.Create())
        {
            Rfc2898DeriveBytes pdb = new Rfc2898DeriveBytes(Passphrase, Salt);
            encryptor.Key = pdb.GetBytes(32);
            encryptor.IV = pdb.GetBytes(16);

            using (MemoryStream ms = new MemoryStream())
            {
                using (CryptoStream cs = new CryptoStream(ms, encryptor.CreateEncryptor(), CryptoStreamMode.Write))
                {
                    cs.Write(clearBytes, 0, clearBytes.Length);
                    cs.Close();
                }

                clearText = StringExtensions.ToBase64String(ms.ToArray());
            }
        }

        return clearText;
    }

    /// <summary>
    /// Uses AES encryption to decrypt a string of data.
    /// </summary>
    /// <param name="cipherText">The encrypted Base64 string to decrypt.</param>
    /// <param name="Passphrase">The secret pre-shared passphrase.</param>
    /// <param name="throwExceptions">If true, will throw exceptions on decryption failure. Else, returns null string on decryption failure.</param>
    /// <returns>The plaintext data. Data is unicode.</returns>
    public string AesDecrypt(string cipherText, bool throwExceptions)
    {
        if (throwExceptions)
        {
            cipherText = AesDecryptInternal(cipherText);
        }
        else
        {
            try
            {
                cipherText = AesDecryptInternal(cipherText);
            }
            catch
            {
                cipherText = null;
            }
        }

        return cipherText;
    }

    private string AesDecryptInternal(string cipherText)
    {
        byte[] cipherBytes = StringExtensions.FromBase64String(cipherText);

        using (Aes encryptor = Aes.Create())
        {
            Rfc2898DeriveBytes saltDerived = new Rfc2898DeriveBytes(Passphrase, Salt);
            encryptor.Key = saltDerived.GetBytes(32);
            encryptor.IV = saltDerived.GetBytes(16);

            using (MemoryStream ms = new MemoryStream())
            {
                using (CryptoStream cs = new CryptoStream(ms, encryptor.CreateDecryptor(), CryptoStreamMode.Write))
                {
                    cs.Write(cipherBytes, 0, cipherBytes.Length);
                    cs.Close();
                }

                cipherText = Encoding.Unicode.GetString(ms.ToArray());
            }
        }

        return cipherText;
    }
}
\$\endgroup\$
  • \$\begingroup\$ How does the decrypter learn which salt the encrypter used? I don't see you storing it alongside the ciphertext. \$\endgroup\$ – CodesInChaos Nov 14 '15 at 16:30
  • \$\begingroup\$ @CodesInChaos The developer is responsible for storing that. That's why I made a Salt property, the developer just needs to save the Salt somewhere. I didn't find it necessary to embed the salt with the result. \$\endgroup\$ – Der Kommissar Nov 14 '15 at 16:32
  • 1
    \$\begingroup\$ Implementing proper salt/IV management is at least half of the work of implementing symmetric crypto, so your code doesn't help the developer much. For example an uneducated developer might choose to use the same salt to encrypt more than one message. \$\endgroup\$ – CodesInChaos Nov 14 '15 at 16:46
  • \$\begingroup\$ @CodesInChaos I suppose it wouldn't be hard to embed the IV or a random salt within the encrypted string. \$\endgroup\$ – Der Kommissar Nov 14 '15 at 16:51
  • \$\begingroup\$ Usually the IV is transmitted with the message, and your heuristic here uses the IV as the first 16 bytes of the key, since both of them use the same salt with PBKDF2, reducing your AES256 to AES128. Though really the IV should just be randomly generated, not computed. You encrypt, transmit [random IV][ciphertext], then on decrypt read the first block as IV, then the rest as ciphertext. \$\endgroup\$ – bartonjs Aug 14 '16 at 8:40
6
\$\begingroup\$
public string AesEncrypt(string clearText)
{
    byte[] clearBytes = Encoding.Unicode.GetBytes(clearText);

    using (Aes encryptor = Aes.Create())
    {
        Rfc2898DeriveBytes pdb = new Rfc2898DeriveBytes(Passphrase, Salt);
        encryptor.Key = pdb.GetBytes(32);
        encryptor.IV = pdb.GetBytes(16);

        using (MemoryStream ms = new MemoryStream())
        {
            using (CryptoStream cs = new CryptoStream(ms, encryptor.CreateEncryptor(), CryptoStreamMode.Write))
            {
                cs.Write(clearBytes, 0, clearBytes.Length);
                cs.Close();
            }

            clearText = StringExtensions.ToBase64String(ms.ToArray());
        }
    }

    return clearText;
}  
  • the call of the Close() method isn't needed, because the disposing of the CryptoStream will close the stream (See the source).

  • Instead of calling ms.ToArray() you should use the MemoryStream.GetBuffer() method to avoid having a copy of the byte array.

The same is true for the AesDecryptInternal() method.

  • because Rfc2898DeriveBytes is inheriting DerivedBytes which is implementing IDisposable you should enclose it in an using block too.

The conditions when you throw an ArgumentExceptionin the constructor aren't sufficient. The call to the Rfc2898DeriveBytes constructor will throw if the Salt isn't at least 8 bytes long. So checking against salt.Length < 8 should be done.

In addition to whats being said about the length of the Salt, you should consider to have a backing field for the Salt property so you can change the property to have the same validation there. This will be true for the Passphrase property too.

\$\endgroup\$
3
\$\begingroup\$

This feels a little clumsy:

public string AesDecrypt(string cipherText, bool throwExceptions)
{
    if (throwExceptions)
    {
        cipherText = AesDecryptInternal(cipherText);
    }
    else
    {
        try
        {
            cipherText = AesDecryptInternal(cipherText);
        }
        catch
        {
            cipherText = null;
        }
    }

    return cipherText;
}

I suggest an exception filter instead:

public string AesDecrypt(string cipherText, bool throwExceptions)
{
    try
    {
        return AesDecryptInternal(cipherText);
    }
    catch when (!throwExceptions)
    {
        return null;
    }
}

You can also combine the two methods now.

\$\endgroup\$
  • \$\begingroup\$ I never knew you could use catch like that, is that new to C#6.0? \$\endgroup\$ – Der Kommissar Nov 14 '15 at 15:37
  • \$\begingroup\$ Yes, it's a new and often overlooked feature of C# 6 \$\endgroup\$ – Jeroen Vannevel Nov 14 '15 at 15:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.