[API Proposal]: AsnWriter Encode with callback

[vcsjones]

Background and motivation

AsnWriter has Encode in two forms: writing to a Span<byte> and allocate-and-return byte[].

There are several uses of AsnWriter where we write something to ASN.1 and then do something with it, like sign it or hash it. The allocating return well, allocates. The span-writing forces developers to come up with an intermediate, either on the stack, a pool rental, etc. Both require the contents to be copied.

However, AsnWriter already has the data available internally. I propose we make this available with a callback to Encode.

API Proposal

namespace System.Formats.Asn1;

public partial class AsnWriter {
#if NET9_0_OR_GREATER
        public TReturn Encode<TReturn>(Func<ReadOnlySpan<byte>, TReturn> encodeCallback);
        public TReturn Encode<TState, TReturn>(TState state, Func<TState, ReadOnlySpan<byte>, TReturn> encodeCallback)  where TState : allows ref struct;
#endif
}

The proposal allows passing in a state and a return. The state and return can be used to feed in state and allow the callback to remain static and not capture anything.

An overload without state exists so that passing in something is not required if it is not needed.

Since System.Formats.Asn1 is supported down level, these APIs will only exist for .NET9+ since ref structs as type arguments is not permitted until then.

API Usage

Consider this snippet in the runtime today:

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs

Lines 355 to 359 in b1f5e97

	byte[] rented = CryptoPool.Rent(_writer.GetEncodedLength());
	int encoded = _writer.Encode(rented);
	X500DistinguishedName name = new X500DistinguishedName(rented.AsSpan(0, encoded));
	CryptoPool.Return(rented, clearSize: 0); // Distinguished Names do not contain sensitive information.
	return name;

This would become:

return _writer.Encode(static (ReadOnlySpan<byte> encoded) => {
    return new X500DistinguishedName(encoded);
});

The same pattern is used for decoding DSA keys for macOS and Android:

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.macOS.cs

Line 95 in b1f5e97

DSA dsa = DSA.Create();

Encrypting CMS in PKCS:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/AnyOS/ManagedPal.Encrypt.cs

Lines 144 to 146 in b1f5e97

	AsnWriter writer = new AsnWriter(AsnEncodingRules.DER);
	envelopedData.Encode(writer);
	return PkcsHelpers.EncodeContentInfo(writer.Encode(), Oids.Pkcs7Enveloped);

Encoding a CMS:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs

Line 136 in b1f5e97

return PkcsHelpers.EncodeContentInfo(writer.Encode(), Oids.Pkcs7Signed);

Hashing signed attributes:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs

Line 646 in b1f5e97

hasher.AppendData(writer.Encode());

Etc.

Alternative Designs

No response

Risks

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-system-formats-asn1, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

AsnWriter has Encode in two forms: writing to a Span<byte> and allocate-and-return byte[].

There are several uses of AsnWriter where we write something to ASN.1 and then do something with it, like sign it or hash it. The allocating return well, allocates. The span-writing forces developers to come up with an intermediate, either on the stack, a pool rental, etc. Both require the contents to be copied.

However, AsnWriter already has the data available internally. I propose we make this available with a callback to Encode.

API Proposal

namespace System.Formats.Asn1;

public partial class AsnWriter {
    public delegate TReturn MemoryEncodeFunc<TState, TReturn>(TState state, ReadOnlyMemory<byte> encoded);

    public TReturn Encode<TState, TReturn>(TState state, MemoryEncodeFunc<TState, TReturn> encodeFunc);
}

The proposal allows passing in a state and a return. The state and return can be used to feed in state and allow the callback to remain static and not capture anything.

API Usage

Consider this snippet in the runtime today:

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X500DistinguishedNameBuilder.cs

Lines 355 to 359 in b1f5e97

	byte[] rented = CryptoPool.Rent(_writer.GetEncodedLength());
	int encoded = _writer.Encode(rented);
	X500DistinguishedName name = new X500DistinguishedName(rented.AsSpan(0, encoded));
	CryptoPool.Return(rented, clearSize: 0); // Distinguished Names do not contain sensitive information.
	return name;

This would become:

// In this example, the state parameter is not used.
return _writer.Encode((object)null, static (object state, ReadOnlyMemory<byte> encoded) => {
    return new X500DistinguishedName(encoded.Span);
});

The same pattern is used for decoding DSA keys for macOS and Android:

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.macOS.cs

Line 95 in b1f5e97

DSA dsa = DSA.Create();

Encrypting CMS in PKCS:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/AnyOS/ManagedPal.Encrypt.cs

Lines 144 to 146 in b1f5e97

	AsnWriter writer = new AsnWriter(AsnEncodingRules.DER);
	envelopedData.Encode(writer);
	return PkcsHelpers.EncodeContentInfo(writer.Encode(), Oids.Pkcs7Enveloped);

Encoding a CMS:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs

Line 136 in b1f5e97

return PkcsHelpers.EncodeContentInfo(writer.Encode(), Oids.Pkcs7Signed);

Hashing signed attributes:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs

Line 646 in b1f5e97

hasher.AppendData(writer.Encode());

Etc.

Alternative Designs

No response

Risks

No response

Author:	vcsjones
Assignees:	-
Labels:	api-suggestion, area-System.Formats.Asn1
Milestone:	-

[bartonjs]

I feel like in my original proposal for AsnWriter I had a SpanAction (or custom SpanAction-like delegate) for Encode; but I don't see it in https://github.com/dotnet/designs/blob/main/accepted/2020/asnreader/asnreader.md or https://github.com/dotnet/corefxlab/blob/archive/src/System.Security.Cryptography.Asn1.Experimental/System/Security/Cryptography/Asn1/AsnWriter.cs so someone must have convinced me to cut it pretty early. (I also don't see it in the private implementation that led up to the public one... huh.)

Anyways, the question from that is mainly why ReadOnlyMemory instead of ReadOnlySpan? I was going to say next that AsnWriter is pretty defensive about its buffer, but exposing the ReadOnlySpan in a SpanAction would still allow unsafe (or Unsafe) access to turn it back into something writable; so it probably doesn't matter that with ReadOnlyMemory it's slightly easier to just reach in and grab the array and turn it into something writable...

Oh, there is one difference (aside from ReadOnlyMemory being able to flow to more places): As a ReadOnlyMemory someone can capture it and exit the callback with the reference. As a ReadOnlySpan it's tightly contained.

I'll also ask if you think there's value in an Action-based variant, or if we should just make anyone who only cares about draining the value into (e.g.) a hash accumulator return 0 (until it becomes too popular to do so, or whatever).

[vcsjones]

So someone must have convinced me to cut it pretty early

I think the original proposal had SpanAction in it, but SpanAction isn't in netstandard2.0, so we couldn't use it, and it became a "eh we'll add it when someone wants it", so, "Hi."

I also don't see it in the private implementation that led up to the public one

Right now we have an internal API that just returns a ReadOnlySpan<byte>.

why ReadOnlyMemory instead of ReadOnlySpan

I started off this way, but changed it because a ROM is at least as useful as a ROS, but a ROS is not as useful as a ROM. For example, in the above examples I found of places this would be useful, PkcsHelpers.EncodeContentInfo takes a ROM, and it couldn't easily be made to take a ROS, unless we did the "pin it and use a custom memory manager" trick.

it probably doesn't matter that with ReadOnlyMemory it's slightly easier to just reach in and grab the array and turn it into something writable

Right. Plus the capturing ergonomics you already mentioned.

I'll also ask if you think there's value in an Action-based variant

I thought about it... but "just return null" seemed fine to me instead of introducing an overload.

[GrabYourPitchforks]

I'm not thrilled about the idea of adding a delegate that exposes an ephemeral ROM<T>. If your API takes ROM<T> instead of ROS<T>, it means that you're intending to hold on to it or otherwise "capture" it. But this API would by definition be giving you something that's borrowed, and if you pass it to a capturing API it could lead to weird instability.

ReadOnlyMemory<byte> bytes = default;
instance.DoSomethingWith((/* ..., */ memory) => { bytes = memory; });
byte[] array = bytes.ToArray(); // ?????

This code sample is basically unsafe-equivalent, because you could be pointing to something that has been returned to the array pool, or to random stackalloc memory in a frame which has already been popped, or <whatever>.

Since we don't have a runtime / language-wide concept of borrowing an object, the best course of action is to rely on our existing type which represents "borrowed memory" - and that existing type is ROS<T>.

[vcsjones]

That's a good point that I agree with. Span makes sense from that perspective. Proposal updated.

[bartonjs]

The delegate probably wants appropriate co-and-contra-variance annotations

public delegate TReturn SpanEncodeFunc<in TState, out TReturn>(TState state, ReadOnlySpan<byte> encoded);

For naming and ordering. Based on SpanAction, it probably should be (ReadOnlySpan<byte> ???, TState state). The only delegates I see that currently take spans call them span, which is pretty weak. So encoded might be the best name to start with.

TState is probably the right name for the generic parameter on the Encode method, but based on Action and Func it should probably just be <T>.

namespace System.Formats.Asn1
{
    partial class AsnWriter
    {
        public delegate TReturn SpanEncodeFunc<in T, out TReturn>(ReadOnlySpan<byte> encoded, T arg);

        public TReturn Encode<TState, TReturn>(TState state, SpanEncodeFunc<TState, TReturn> encodeFunc);
    }
}

There's also an interesting question of whether we should be doing

a) SpanFunc, here
b) SpanFunc in System.Buffers
c) Just use the existing SpanAction (and make return values go via a mutable TState)

Since we have a netstandard2.0 reference assembly, (c) is awkward, and (c) is not netstandard2.0, I'd suggest not (c). I feel like we don't like having public delegate types as nested types, so (a) is really (a') (System.Formats.Asn1.SpanEncodeFunc). Since nothing in the signature really ties it to the ASN.1 namespace that feels like it might be (b).

// Do we have a place to do this ns2.0, or are we splitting our ref.cs now?
namespace System.Buffers
{
    public delegate TReturn ReadOnlySpanFunc<in T, out TReturn>(ReadOnlySpan<byte> span, T arg);
}

namespace System.Formats.Asn1
{
    partial class AsnWriter
    {
        public TReturn Encode<TState, TReturn>(TState state, SpanEncodeFunc<TState, TReturn> encodeFunc);
    }
}

@stephentoub We've avoided (RO)SpanFunc thus far... it it popped up with this proposal would that be something you'd endorse, fight, or not really care about?

[stephentoub]

We've avoided (RO)SpanFunc thus far... it it popped up with this proposal would that be something you'd endorse, fight, or not really care about?

I'd like to see if we can get to a point in .NET 8 / C# 12 where ref struct constraints would enable us to define a more general set of delegates (cc: @AaronRobinsonMSFT, @steveharter), or better yet, enable us to use the existing Action<...>/Func<...> as long as the Ts used are so constrained.

In the meantime, we can make forward progress on these kinds of issues with whatever custom delegates we need to define, but it'd be good to keep track of the ones we'd like to rip out prior to shipping .NET 8 should we be able to replace them successfully.

[AaronRobinsonMSFT]

enable us to use the existing Action<...>/Func<...> as long as the Ts used are so constrained.

@stephentoub I assume you mean #65112? As in when ref structs are permitted in Generics we ensure the constraints allow us to express signatures with them rather than creating new unique delegate types. If so, we are in agreement.

[stephentoub]

Right, e.g. the goal here would be to be able to write:

public TReturn Encode<TState, TReturn>(TState state, Func<ReadOnlySpan<byte>, TState, TReturn> encodeFunc) { ... }

with the existing Func<...> (augmented however it might need to be augmented).

[vcsjones]

Updated the proposal with Func now that it can be used.

[bartonjs]

Video

• We talked about the consequences and future upgradability of TReturn not being allows ref struct, and are good with it as proposed.

namespace System.Formats.Asn1;

public partial class AsnWriter {
#if NET9_0_OR_GREATER
    public TReturn Encode<TReturn>(Func<ReadOnlySpan<byte>, TReturn> encodeCallback);
    public TReturn Encode<TState, TReturn>(TState state, Func<TState, ReadOnlySpan<byte>, TReturn> encodeCallback)
        where TState : allows ref struct;
#endif
}