Symmetric Encryption with large strings returns same string but a loss of 1-5 bytes-CodePudding

I have been playing around with an old code I wrote using SHA1 to work with SHA256 or SHA512. I am fairly new in the cryptography world and I wondered if someone can tell me why my input value in the Encrypt method returns through the Decrypt method with a loss of 1-5 bytes.

I can run the methods with short strings no problem but apparently not larger e.g. 16 bytes .

The two methods in Question are:

    public string Decrypt(string data, EncryptionValue eV, string passPhrase)
{
    byte[] bytes = Encoding.ASCII.GetBytes(eV.InitVector);
    byte[] rgbSalt = Encoding.ASCII.GetBytes(eV.SaltValue);
    byte[] buffer = Convert.FromBase64String(data);
    byte[] rgbKey = new Rfc2898DeriveBytes(passPhrase, rgbSalt, eV.PassIterations, hash).GetBytes(eV.KeySize / 8);

    var managed = Aes.Create("AesManaged");
    managed.Padding = PaddingMode.PKCS7;
    managed.Mode = CipherMode.CBC;
    ICryptoTransform transform = managed.CreateDecryptor(rgbKey, bytes);
    MemoryStream stream = new(buffer);
    CryptoStream stream2 = new(stream, transform, CryptoStreamMode.Read);
    byte[] buffer5 = new byte[buffer.Length];
    int count = stream2.Read(buffer5, 0, buffer5.Length);
    stream.Close();
    stream2.Close();
    return Encoding.UTF8.GetString(buffer5, 0, count);
}
public string Encrypt(string data, EncryptionValue eV, string passPhrase)
{
    byte[] bytes = Encoding.ASCII.GetBytes(eV.InitVector);
    byte[] rgbSalt = Encoding.ASCII.GetBytes(eV.SaltValue);
    byte[] buffer = Encoding.UTF8.GetBytes(data);
    byte[] rgbKey = new Rfc2898DeriveBytes(passPhrase, rgbSalt, eV.PassIterations, hash).GetBytes(eV.KeySize / 8);

    var managed = Aes.Create("AesManaged");
    managed.Padding = PaddingMode.PKCS7;
    
    managed.Mode = CipherMode.CBC;
    ICryptoTransform transform = managed.CreateEncryptor(rgbKey, bytes);
    MemoryStream stream = new();
    CryptoStream stream2 = new(stream, transform, CryptoStreamMode.Write);
    stream2.Write(buffer, 0, buffer.Length);
    stream2.FlushFinalBlock();
    byte[] inArray = stream.ToArray();
    stream.Close();
    stream2.Close();
    return Convert.ToBase64String(inArray);
}

The Encryption Value Class contains randomised randomised values and it's not the issue here but I am posting it anyway here:

 public int PassIterations { get; set; }
 public int KeySize { get; set; }
 public string InitVector { get; set; }
 public string SaltValue { get; set; }

The Iterations are around 1000 in all my tests. I've done tests with less and more. KeySize is always 256 or 128, InitVector = "~1B2c3D4e5F6g7H8";

I ran some tests and like I said, smaller strings are okay but I want it to be able to store at least 120 bytes. I thought this would work?

CodePudding user response：

The problem is actually with the stream2.Read call in your Decrypt method. The stream's Read method does NOT guarantee that it will read buffer5.Length bytes. It's quite possible that it reads fewer bytes in a single call. Which is perfectly valid behavior, and depends on how precisely a particular CryptoStream implementation (depending on the .NET version and/or host OS running your program) or configuration is behaving with regard to read operations.

You'll need to check how many bytes stream2.Read has read, and if it read fewer than buffer5.Length bytes, then you'll need to call stream2.Read again and again until buffer5 is completely filled or stream2.Read is indicating end-of-stream by returning 0. A simpler solution than implementing this is perhaps by using the BinaryReader.ReadBytes method, which will read as many bytes as requested (unless an end-of-stream is reached before all requested bytes have been read, of course).

See this dotnetfiddle example that exposes this behavior: https://dotnetfiddle.net/ATLVTm
Note the "cheap" Console.WriteLine instrumentation i have added making this behavior visible.

P.S.: Just a nitpicky side note unrelated to your problem: Instead of manually closing all those streams, it would be better to use the `using` statement in conjunction with the streams (because streams are `IDisposable`'s) which would automatically take care of closing/disposing the streams when the respective stream variable goes out of scope, even when exceptions are being thrown by your code.

CodePudding user response：

The error was what @MySkullCaveIsADarkPlace, I simply set it to check whether the buffer was fully read or not.

if (count < buffer5.Length)
    {
        int count2 = stream2.Read(buffer5, count, buffer5.Length - count);
        count = count   count2;
    }

It now passes all my xUnit Tests even if I go crazy on the string length in the data input.

Thanks for the help all involved. :)