CryptoStream forces me to leak sensitive data into RAM

318 views Asked by At

So here are my goals:

  1. Decrypt a byte[] into a pinned byte[] buffer.
  2. I don't want the plain-text to exist anywhere else in memory, outside of this pinned byte[], which I control.

How can I do this in C#?

I naively used the CryptoStream class. But that requires me to give it an output stream. I must. So I went ahead and gave it a MemoryStream.

I did some sniffing around in memory, using the Memory debug window. I believe I found that MemoryStream has a copy (for buffering?) of anything that comes out of the CryptoStream. So now the plain-text is in both my pinned byte[], and in this other part of memory that can be randomly copied around by the CLR, and over which I have no control.

Here is the code I used. I assume no concurrency here for simplicity:

public class ExampleCode
{
    private SymmetricAlgorithm algorithm;
    private ICryptoTransform decryptor;

    public ExampleCode(byte[] key, byte[] iv) // c'tor
    {
        algorithm = new RijndaelManaged();
        algorithm.Key = key;
        algorithm.IV = iv;
        decryptor = algorithm.CreateDecryptor();
    }

    private long DecryptToPinnedArray( byte[] src, byte[] pinnedDst )
    {
        long bytesWritten = 0;

        using ( var ms = new MemoryStream( pinnedDst, writable: true ) )
        using ( var cs = new CryptoStream( ms, decryptor, CryptoStreamMode.Write ) )
        {
            cs.Write( src, 0, src.Length );
            cs.FlushFinalBlock();
            ms.Flush();

            bytesWritten = ms.Position;
            return bytesWritten;
        }
    }
}

How do I prevent this second copy of the sensitive data from ever being created? Should I forget about CryptoStream and use something more low-level? Is there a best-practice for issues like this?

EDIT: Peeking in with a reflector, this is what I think is happening:

CryptoStream.Write():

  1. cipher_data -=copy-=> _InputBuffer (a CryptoStream internal unpinned byte[]).
  2. _InputBuffer -=transform-=> _OutputBuffer (a CryptoStream internal unpinned byte[]).
  3. _OutputBuffer -=write-=> MemoryStream

If it needs to (and can) transform more than one block at a time, it will use a temporary locally created larger unpinned byte[] (multiBlockArray) to try and transform into it all the blocks in one go (instead of into the usual _OutputBuffer). It writes multiBlockArray to the stream. It then loses the reference to this array, and doesn't even attempt to sanitize it. Of course, it's not pinned anyway.

CryptoStream.FlushFinalBlock() & CryptoStream.Dispose():

Both will Array.Clear the _OutputBuffer. This is better than nothing, although _OutputBuffer is not pinned, so it can still potentially leak the plaintext data.

3

There are 3 answers

0
Tim On

This may be an opinionated answer, so take it for what's worth:

The MemoryStream constructor you are using already is the one that operates on the passed in array pinnedDst. It won't grow, reallocate, etc. that array, it will simply read and write to it. Since it is already pinned, you can be sure it won't get moved by the GC.

However, all stream operations in C# operate by using byte[] buffers to read or write to the stream. When you write to CryptoStream, it is going to create some kind of plain text buffer to be used to copy data to your MemoryStream-- it may only be a few bytes long, or it may be as large as your entire src array. That's (presumably) up to the block length of the cipher it uses and/or how it was written.

If you truly want to avoid any chance of your plain text getting moved during a GC operation and recycled on to the managed heap without being cleared, you would likely have to p/invoke the windows crypto API. However, P/invoking the windows Crypto API would require unsafe code blocks to get pointers to your array, which means you are enabling the very thing that makes you vulnerable to memory attacks-- enabling unchecked pointer dereferencing within your process.

I wouldn't lose sleep over the code you have written above. I would not allow unsafe code blocks in a .net assembly that needed to handle PCI-DSS scoped decryption either. Like you are already doing, use byte arrays instead of strings, operate on MemoryStream using buffers that you allocate and clear when you are done-- and you should be good to go.

1
bartonjs On
ICryptoTransform decryptor = symm.CreateDecryptor();

if (!decryptor.CanTransformMultipleBlocks)
    throw new InvalidOperationException();

// Since we're decrypting src.Length is block aligned.
int written = decryptor.TransformBlock(src, 0, src.Length, pinnedDst, 0);
byte[] lastBlock = decryptor.TransformFinalBlock(Array.Empty<byte>(), 0, 0);
Buffer.BlockCopy(lastBlock, 0, pinnedDst, written, lastBlock.Length);

That makes everything except for the last block only be written to your pinned memory -- unless the algorithm implementation needed to use a managed buffer internally. If you are in PaddingMode.None (or Zeros) then I believe that TransformFinalBlock will return the empty array, since it doesn't need to do a depad holdback.

0
NN_ On

.NET 5.0 adds ImportFromPem:

 using var rsa = new RSACryptoServiceProvider();
 rsa.ImportFromPem(privateKey);