Remove byte[] allocation in VerifyRsa / VerifyECDsa

[eerhardt]

Calling HashAlgorithm.ComputeHash will always allocate and return a new byte array. On .NET 6+ we can instead use a temporary buffer (either on the stack for normal hashing, or use the array pool for hash algorithms that need more than 2k bits) to reduce the intermediate allocation.

cc @bartonjs - in case you want to take a look.

[bartonjs]

Since you already have the HashAlgorithmName, why not just call VerifyData and let the framework do the hashing for you (which won't create a temporary array)?

[eerhardt]

and let the framework do the hashing for you (which won't create a temporary array)?

This doesn't appear to be the case. When I step into it, I get:

https://github.com/dotnet/runtime/blob/9926d609dab5929941b7924a04521ea2c2df27f3/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs#L661-L662

Which allocates the byte[].

Here is my test code (on windows):

using System.Security.Cryptography;

RSA rSA = RSA.Create();

byte[] data = [0x1, 0x2, 0x3, 0x4];

byte[] signature = rSA.SignData(data, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);

for (int i = 0; i < 1_000; i++)
{
    rSA.VerifyData(data, signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
}

[eerhardt]

Should I submit a similar PR to RSA and ECDsa in dotnet/runtime?

[bartonjs]

It looks like VerifyData(ReadOnlySpan) uses span-hashing, but VerifyData(byte[]) uses array-hashing; because we never override public virtual bool VerifyData(byte[], int, int, byte[], HashAlgorithmName, RSASignaturePadding). So the change in dotnet/runtime to make everyone happier is probably to override that method on all N of our RSA types. (RSACryptoServiceProvider, RSACng, RSAOpenSsl, RSASecurityTransforms, RSAAndroid, and maybe up to double that from the "RSA.Create()" types). Which is going to involve a lot of copy/paste of the argument validation. Bleh.

Here, though, it means

return rsa.VerifyData(
#if NET5_OR_GREATER
    bytes.AsSpan(offset, count),
#else
    bytes, offset, count,
#endif
    signature,
    HashAlgorithmName,
    RSASignaturePadding);

should take your numbers down to 0 (for .NET5+)

[eerhardt]

RSA is still allocating here:

https://github.com/dotnet/runtime/blob/9926d609dab5929941b7924a04521ea2c2df27f3/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSA.cs#L322

HashData returns a new byte[].

Test Code:

using System.Security.Cryptography;

RSA algo = RSA.Create();

byte[] data = [0x1, 0x2, 0x3, 0x4];

byte[] signature = algo.SignData(data, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);

for (int i = 0; i < 1_000; i++)
{
    algo.VerifyData(data.AsSpan(), signature, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
}

[bartonjs]

Weird, since it should have hit the if (this is IRuntimeAlgorithm) above and bypassed the allocation routine. Must've missed a marker interface somewhere, too.

[eerhardt]

In my case, this is an RSABCrypt, which doesn't implement that interface.

https://github.com/dotnet/runtime/blob/acd31754892ab0431ac2c40038f541ffa7168be7/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSABCrypt.cs#L11

On Linux it looks like we get an RSAWrapper, which also doesn't implement it:

https://github.com/dotnet/runtime/blob/9926d609dab5929941b7924a04521ea2c2df27f3/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSAWrapper.cs#L10

[keegan-caruso]

LGTM