Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guideline about cryptography with RMS #326

Closed
LeaFrock opened this issue Jan 26, 2024 · 5 comments
Closed

Guideline about cryptography with RMS #326

LeaFrock opened this issue Jan 26, 2024 · 5 comments

Comments

@LeaFrock
Copy link

LeaFrock commented Jan 26, 2024

I've read a related issue #131. But about 3 years passed, something changes and it doesn't solve my confusion.

I'm going to use RMS in a list of ASP.NET Core 8 web APIs. Some of them need to encrypt the user-committed content (such as a form-file) with AES (CBC mode for example).

With the help of Issue#131, I write the following codes,

        private async Task<byte[]> EncryptCore(IFormFile file, byte[] key, byte[] iv)
        {
            using var ms = RMS.GetStream(Guid.NewGuid(), "myTag", file.Length);
            using var aes = Aes.Create();
            aes.Key = key;
            aes.IV = iv;
            using var transform = aes.CreateEncryptor(key, iv);
            using var cs = new CryptoStream(ms, transform, CryptoStreamMode.Write);
            await file.CopyToAsync(cs);
            await cs.FlushFinalBlockAsync();
            var data = ms.ToArray(); // Is it risky?
            return data;
        }

But the README says,

Most applications should not call ToArray and should avoid calling GetBuffer if possible.

Also, .NET 6+ adds new simplified cryptography APIs such as AES.EncryptCbc.

        private async Task<byte[]> EncryptCore(IFormFile file, byte[] key, byte[] iv)
        {
            using var ms = RMS.GetStream(Guid.NewGuid(), "myTag", file.Length);
            await file.CopyToAsync(ms);
            using var aes = Aes.Create();
            aes.Key = key;
            aes.IV = iv;
            var data = aes.EncryptCbc(ms.ToArray(), iv, PaddingMode.PKCS7); // ToArray seems to be risky. How should I do so?
            return data;
        }

The README says GetReadOnlySequence() should be used for reading, but EncryptCbc only accepts byte[] and ReadOnlySpan<byte>. I'm not sure whether it's possible to use RMS here.

Overall, I hope you can give the guideline. Thanks in advance!

@sungam3r
Copy link
Contributor

You may use ToArray safely in both cases but the main idea for RMS usage is to not allocate arrays again and again. The all project is about reusing buffers. If called/returning APIs support only arrays then you may find RMS useless. In the second example I suggest to use byte[] EncryptCbc(readonlyspan) overload so you save one array allocation.

@LeaFrock
Copy link
Author

LeaFrock commented Jan 26, 2024

I appreciate your reply @sungam3r .

In the second example I suggest to use byte[] EncryptCbc(readonlyspan) overload so you save one array allocation

Would you mind paste some codes as a demo here? I don't know how to convert ReadOnlySequence to Span. What I know is the following way, which seems not correct when using AES.

foreach(var m in ms.GetReadOnlySequence())
{
    byte[] block = aes.EncryptCbc(m.Span, iv, PaddingMode.PKCS7); // Then combine several blocks into a full byte array? Seems not correct.
}

(updated)
Well, I realize the way. Do you mean using GetBuffer instead?

        private async Task<byte[]> EncryptCore(IFormFile file, byte[] key, byte[] iv)
        {
            using var ms = RMS.GetStream(Guid.NewGuid(), "myTag", file.Length);
            await file.CopyToAsync(ms);
            using var aes = Aes.Create();
            aes.Key = key;
            var buffer = ms.GetBuffer();
            var data = aes.EncryptCbc(buffer.AsSpan(0, (int)ms.Length), iv, PaddingMode.PKCS7);
            return data;
        }

@sungam3r
Copy link
Contributor

Yes - just use span projection over buffer returned from RMS.

@sungam3r
Copy link
Contributor

sungam3r commented Jan 26, 2024

In that way RMS ecosystem does not allocate arrays. But you as the caller of .EncryptCbc still allocate since that API was designed to return array. You may try to find other overload or combination of calls to save another array allocation if clients of EncryptCore method can work with shared buffers as well.

@LeaFrock
Copy link
Author

You may try to find other overload...

Got it. It's another issue but still thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants