2

I've been working on a simple helper for dotnet core that should encode and decode a string, based on a user provided password (key) and a salt.

In contradiction to the full .NET Framework, dotnet core currently does not have an implementation for the RijndaelManaged class. Pretty much every decent encrypt/decrypt sample for C# is based on this class, rendering it useless for dotnet core. There is a lot of debate on it on the CoreFX repo on GitHub.

However, with the combination of both the (old) MSDN article on AesManaged and an article by Troy Hunt - not to mention some refactoring - I was able to brew the following semi-working example:

internal class CryptographyHelpers
{
    internal static string Decrypt(string password, string salt, string encrypted_value)
    {
        string decrypted;

        using (var aes = Aes.Create())
        {
            var keys = GetAesKeyAndIV(password, salt, aes);
            aes.Key = keys.Item1;
            aes.IV = keys.Item2;

            // create a decryptor to perform the stream transform.
            var decryptor = aes.CreateDecryptor(aes.Key, aes.IV);

            // create the streams used for encryption.
            var encrypted_bytes = ToByteArray(encrypted_value);
            using (var memory_stream = new MemoryStream(encrypted_bytes))
            {
                using (var crypto_stream = new CryptoStream(memory_stream, decryptor, CryptoStreamMode.Read))
                {
                    using (var reader = new StreamReader(crypto_stream))
                    {
                        decrypted = reader.ReadToEnd();
                    }
                }
            }
        }

        return decrypted;
    }

    internal static string Encrypt(string password, string salt, string plain_text)
    {
        string encrypted;

        using (var aes = Aes.Create())
        {
            var keys = GetAesKeyAndIV(password, salt, aes);
            aes.Key = keys.Item1;
            aes.IV = keys.Item2;

            var encryptor = aes.CreateEncryptor(aes.Key, aes.IV);

            using (var memory_stream = new MemoryStream())
            {
                using (var crypto_stream = new CryptoStream(memory_stream, encryptor, CryptoStreamMode.Write))
                {
                    using (var writer = new StreamWriter(crypto_stream))
                    {
                        writer.Write(plain_text);
                    }

                    var encrypted_bytes = memory_stream.ToArray();
                    encrypted = ToString(encrypted_bytes);
                }
            }
        }

        return encrypted;
    }

    private static byte[] ToByteArray(string input)
    {
        return Encoding.Unicode.GetBytes(input);
    }

    private static string ToString(byte[] input)
    {
        return Encoding.Unicode.GetString(input);
    }

    private static Tuple<byte[], byte[]> GetAesKeyAndIV(string password, string salt, SymmetricAlgorithm symmetricAlgorithm)
    {
        const int bits = 8;
        var key = new byte[16];
        var iv = new byte[16];

        var derive_bytes = new Rfc2898DeriveBytes(password, ToByteArray(salt));
        key = derive_bytes.GetBytes(symmetricAlgorithm.KeySize / bits);
        iv = derive_bytes.GetBytes(symmetricAlgorithm.BlockSize / bits);

        return new Tuple<byte[], byte[]>(key, iv);
    }

}

I said semi-working example, which means it works... sometimes. And there lies the problem. When I run my test arbitrarily, it succeeds most of the time. Randomly, about one out of four times, it fails with the following message:

Message: Expected string to be "lorem ipsum dom dolor sit amet", but " �R��k6��o��do�Lr sit amet" differs near ">�R" (index 0).

It looks like a unicode problem, but changing the Encoding.Unicode in the helpers to Encoding.UTF8, leads to the error:

System.Security.Cryptography.CryptographicException : The input data is not a complete block.

Changing the encoding to Encoding.ASCII leads to the following error:

System.Security.Cryptography.CryptographicException : Specified padding mode is not valid for this algorithm.

The test I'm using is:

[Fact]
public void Decrypt_Should_Decode_An_Encrypted_String()
{
    // arrange
    var key = Guid.NewGuid().ToString();
    var salt = Guid.NewGuid().ToString();
    var original_value = "lorem ipsum dom dolor sit amet";
    var encrypted_value = CryptographyHelpers.Encrypt(key, salt, original_value);

    // act
    var target = CryptographyHelpers.Decrypt(key, salt, encrypted_value);

    // assert
    target.Should().NotBeNullOrEmpty();
    target.Should().Be(original_value);
}

So, my primary question is: why does my imlementation (test) sometimes fail?

I am definitely not an expert on cryptography, but on a high level am aware of some basic concepts. Also, a similar question "How to use Rijndael encryption with a .Net Core class library?" was posted, but the only answer stays on a conceptual level, lacking concrete implementation.

Any tips and arguments as to if this is a 'good enough' implementation would also be very much appreciated.

Thanks!

Community
  • 1
  • 1
Juliën
  • 9,047
  • 7
  • 49
  • 80
  • If you come to this answer later and are wondering why you get block size errors, try to use the new Rfc2898DeriveBytes with the _salt_ as a byte array, as an int it will not give consistent results for IV. – Garry Polley Dec 13 '17 at 14:13

1 Answers1

3

Your problem has nothing to do with cryptography and instead is caused by this pair of functions

private static byte[] ToByteArray(string input)
{
    return Encoding.Unicode.GetBytes(input);
}

private static string ToString(byte[] input)
{
    return Encoding.Unicode.GetString(input);
}

You are not allowed to call GetString on arbitrary byte arrays, you will cause a loss of information if you do. You need to use a encoding that allows for arbitrary byte arrays, for example Base64:

private static byte[] ToByteArray(string input)
{
    return Convert.FromBase64String(input);
}

private static string ToString(byte[] input)
{
    return Convert.ToBase64String(input);
}
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • Awesome, that seems to be it. I'm still puzzled about the 'randomness' of it, but using Base64 (without '-' characters) fixed it. Thanks! – Juliën Feb 25 '17 at 18:10