API proposal: Add ReadOnlySpan<byte> to IDataProtector (un)Protect

[KLuuKer]

Background and Motivation

Add ReadOnlySpan<byte> overrides so consumer apps are able to avoid a extra array allocation+blockbuffer.copy every time they try to protect\unprotect data.

Proposed API

namespace Microsoft.AspNetCore.DataProtection;

public interface IDataProtector : IDataProtectionProvider
{
+    byte[] Protect(ReadOnlySpan<byte> plaintext);

+    byte[] Unprotect(ReadOnlySpan<byte> protectedData);
}

Usage Examples

var buffer = new ArrayBufferWriter<byte>();

// write some data to buffer
var blob = new Data { Some = "Payload" };
MessagePackSerializer.Serialize(buffer, blob);

IDataProtector protector = provider.CreateProtector("demo");
return protector.Protect(buffer.WrittenSpan);

Alternative Designs

namespace Microsoft.AspNetCore.DataProtection;

public interface IDataProtector : IDataProtectionProvider
{
+    byte[] Protect(ReadOnlySpan<byte> plaintext) => this.Protect(plaintext.ToArray())

+    byte[] Unprotect(ReadOnlySpan<byte> protectedData) => this.Unprotect(protectedData.ToArray())
}

Risks

All of the existing protector implementations have to be changed to implement this change.
We could also have a default interface implementation to prevent it breaking to aggressively.

Performance impact should be minimal, and actually open the possibility for apps to avoid a unnecessary array copy, if people use the api correctly

[halter73]

@KLuuKer We're interested in learning more about your use case. Do you plan to use it with MessagePackSerializer? Anything else? This seems potentially useful if you need to slice byte arrays. Perhaps we could use this internally if/when we're using array pools.

Note: We'd have to ifdef the new API out of netstandard unless we make a whole new interface because netstandard does not support default interface methods, and we cannot make breaking changes to this interface.

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[adityamandaleeka]

Triage: optimizing this is likely going to help in some hot paths. We believe the API needs some design work though.

Note: We'll need to make sure netstandard2.0 works.

[KLuuKer]

@halter73 yes I do use it with MessagePackSerializer (now with a extra array copy unfortunately), but I do write my own payloads first to the buffer before I read\write to it with messagepack (like version\compatibility flags etc).

Sometimes I also end up writing custom byte's rented from a ArrayPool that ends up returning a slightly larger buffer then my actual payload, and apparently I also use WebEncoders.Base64UrlEncode (decode) allot.

Can't wait until net8.0 so I can start using it
p.s. anybody have a working time machine handy?

[KLuuKer]

Maybe something like a TryProtect \ TryUnprotect and people can input their own source and destination span's.
I understand the potential ways "normal" developers might be able to shoot themselves in the foot with it, if they actually put secret data back into the buffer pool before sanitizing the bytes first.

[KevinH-MS]

My team's services would benefit from this API as well. Our scenario involves StackExchange.Redis, where the values we want to encrypt are of type RedisValue (which can be implicit converted to ReadOnlyMemory<byte>).

https://github.com/StackExchange/StackExchange.Redis/blob/ae6419a164600b4b54dd7ce8a699efe8e98d8f1c/src/StackExchange.Redis/RedisValue.cs#L855

I think it would be sufficient for us to call ReadOnlyMemory<byte>.Span if an overload that accepted ReadOnlySpan<byte> was provided. Initially, I thought it would be nice if IDataProtector provided a method that accepted ReadOnlyMemory<byte>, but if it did, I think it'd need to call .Span internally to iterate/consume the contents of the array it's pointing at anyway, right?

[amcasey]

To get the perf win, we'd have to avoid allocating the array in the API implementation as well, so we'd probably need/want to update these too:

Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.IAuthenticatedEncryptor.Decrypt(System.ArraySegment<byte> ciphertext, System.ArraySegment<byte> additionalAuthenticatedData) -> byte[]!
Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.IAuthenticatedEncryptor.Encrypt(System.ArraySegment<byte> plaintext, System.ArraySegment<byte> additionalAuthenticatedData) -> byte[]!
Microsoft.AspNetCore.DataProtection.ISecret.WriteSecretIntoBuffer(System.ArraySegment<byte> buffer) -> void
Microsoft.AspNetCore.DataProtection.Secret.Secret(System.ArraySegment<byte> value) -> void
Microsoft.AspNetCore.DataProtection.Secret.WriteSecretIntoBuffer(System.ArraySegment<byte> buffer) -> void

And probably these, if only for consistency:

static Microsoft.AspNetCore.Cryptography.KeyDerivation.KeyDerivation.Pbkdf2(string! password, byte[]! salt, Microsoft.AspNetCore.Cryptography.KeyDerivation.KeyDerivationPrf prf, int iterationCount, int numBytesRequested) -> byte[]!
Microsoft.AspNetCore.DataProtection.IPersistedDataProtector.DangerousUnprotect(byte[]! protectedData, bool ignoreRevocationErrors, out bool requiresMigration, out bool wasRevoked) -> byte[]!
Microsoft.AspNetCore.DataProtection.Secret.Secret(byte[]! value) -> void
Microsoft.AspNetCore.DataProtection.ITimeLimitedDataProtector.Protect(byte[]! plaintext, System.DateTimeOffset expiration) -> byte[]!
Microsoft.AspNetCore.DataProtection.ITimeLimitedDataProtector.Unprotect(byte[]! protectedData, out System.DateTimeOffset expiration) -> byte[]!
static Microsoft.AspNetCore.DataProtection.DataProtectionAdvancedExtensions.Protect(this Microsoft.AspNetCore.DataProtection.ITimeLimitedDataProtector! protector, byte[]! plaintext, System.TimeSpan lifetime) -> byte[]!

[amcasey]

This is a pretty big change, but we'll give it further consideration in 9.0.