Add support to cryptographic PInvoke for ROM<byte> ROS<byte> to reduce allocations

[SteveSyfuhs]

Is your feature request related to a problem? Please describe. The existing implementations require a lot of allocating and copying because of historical interfaces in .NET. We can bypass these interfaces and call directly into a PAL (Win32 on Windows, custom on other platforms).

This requires issue #57 to be completed first.

[gfoidl]

I'll have a look at this.

[gfoidl]

We can bypass these interfaces and call directly into a PAL

Another option to avoid some allocations is using the new APIs introduced with .NET Standard 2.1, which offer more span-based methods.

Is this option (.NET Standard 2.1 as second target) viable?

[gfoidl]

Thinking about this more...

Current crypto-methods have a pattern that "needs" an allocation, like https://github.com/dotnet/Kerberos.NET/blob/a24a9fa221679b30f8126a3a5972110ecf2f93bc/Kerberos.NET/Crypto/Pal/IHashAlgorithm.cs#L7 (For the returning result an allocation must happen.)

Is it possible to change to a Try...-pattern so these allocations could be avoided? E.g. to

bool TryComputeHash(ReadOnlySpan<byte> data, Span<byte> hash, out int bytesWritten);

So a caller could stack-alloc the destination.

This would be a breaking change, as these APIs are public https://github.com/dotnet/Kerberos.NET/blob/a24a9fa221679b30f8126a3a5972110ecf2f93bc/Kerberos.NET/Crypto/Pal/IHashAlgorithm.cs#L5 So either take the break or add the Try-APIs as additional overload. (The existing methods could then just delegate to the Try-methods -- similar things are done in runtime/libraries.)

For .NET Standard 2.0 (current target) one could then call the native apis directly without any allocations and copying between buffers.

But this would be quite a duplication of native interop provided by .NET Core. It's doable, of course, but with a target for .NET Standard 2.1 this isn't needed and one could use just the managed APIs provided (also by Try-pattern).

Downside for .NET Standard 2.1 is that it's there aren't a lot of runtimes that adopt / implement it and that .NET Full isn't supported at all.

[SteveSyfuhs]

Unfortunately .NET Standard 2.0 is the highest this can go as it MUST stay backwards compatible with the full framework.

Its an interesting approach though. I'm not terribly concerned about breaking changes throughout these particular interfaces since they're pretty low-level in an already low-level library.

---

From: Günther Foidl [email protected] Sent: Tuesday, March 31, 2020 3:56:14 AM To: dotnet/Kerberos.NET [email protected] Cc: Steve Syfuhs [email protected]; Author [email protected] Subject: Re: [dotnet/Kerberos.NET] Add support to cryptographic PInvoke for ROM ROS to reduce allocations (#58)

Thinking about this more...

Current crypto-methods have a pattern that "needs" an allocation, like https://github.com/dotnet/Kerberos.NET/blob/a24a9fa221679b30f8126a3a5972110ecf2f93bc/Kerberos.NET/Crypto/Pal/IHashAlgorithm.cs#L7 (For the returning result an allocation must happen.)

Is it possible to change to a Try...-pattern so these allocations could be avoided? E.g. to

bool TryComputeHash(ReadOnlySpan data, Span hash, out int bytesWritten);

So a caller could stack-alloc the destination.

This would be a breaking change, as these APIs are public https://github.com/dotnet/Kerberos.NET/blob/a24a9fa221679b30f8126a3a5972110ecf2f93bc/Kerberos.NET/Crypto/Pal/IHashAlgorithm.cs#L5 So either take the break or add the Try-APIs as additional overload. (The existing methods could then just delegate to the Try-methods -- similar things are done in runtime/libraries.)

For .NET Standard 2.0 (current target) one could then call the native apis directly without any allocations and copying between buffers.

But this would be quite a duplication of native interop provided by .NET Corehttps://github.com/dotnet/runtime/tree/d3366b86bc2a1415ae1998334f6a5241b14f0c4f/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native. It's doable, of course, but with a target for .NET Standard 2.1 this isn't needed and one could use just the managed APIs provided (also by Try-pattern).

Downside for .NET Standard 2.1https://devblogs.microsoft.com/dotnet/announcing-net-standard-2-1/ is that it's there aren't a lot of runtimes that adopt / implement it and that .NET Full isn't supported at all.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/dotnet/Kerberos.NET/issues/58#issuecomment-606552856, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAJHTYPGZB4L6UMBWBVRQILRKHD45ANCNFSM4IZSBJ5A.

[gfoidl]

I'm not terribly concerned about breaking changes throughout these particular interfaces

Fine. So I'll try something...(which should ideally result in a PR :wink:)

[gfoidl]

.NET Standard 2.0 is the highest this can go as it MUST stay backwards compatible with the full framework

Without duplication of all the native interop from .NET Core (especially for non windows) it is nearly impossible to remove a good amount of allocations with the .NET Standard 2.0 target.

What about adding .NET Standard 2.1 as additional target? So it can be compatible to .NET Full and have alternate allocation-less code for .NET ST 2.1.

This can be packaged (NuGet handles this properly) and tested. So it would be the best of both worlds.

[gfoidl]

With a target for .NET ST 2.1 it's possible to remove quite a lot of allocations, but this results in a mess (scroll half-way down to see what I mean) with the need for different api-shape and all the #ifdefs. Note: the branch is in wip state and won't compile, as it touches just a few parts. These changes are "viral" as the allocation-less apis (see https://github.com/dotnet/Kerberos.NET/issues/58#issuecomment-606552856) are quite different. Thus this way would result in rewrite / duplication of a lot of code. RC4Transformer is an example for such changes.

I doubt this a way we should go (for now).

PS: in the comparison please ignore minor changes also.

---

If we stick solely to .NET ST 2.0 and want to reduce allocation, we can't rely on .NET APIs, as they are byte[] based, thus needing allocations -- even when using {RO}Memory<byte>.

So as proposed in the initial comment to this issue, one could call the native operations direclty (via a PAL as suggested) with the span-based Try-pattern.

For windows this is doable, as most native methods are from advapi32.dll and bcrypt.dll. So just a thin managed wrapper is needed.

For non-windows this will get cumbersome, as there exists no equivalent standard api. In .NET a shim around openssl is natively compiled and wrapped by the managed api. To take effort of this approach, these whole apis would need to be duplicated here. And what happens if openssl isn't available on a target-machine?

I advocate to not do this -- it's better to layer this library on top of a solid basement, especially that security concerns are major point here.

With the current set-up I see a dead-end-road. Either it's just me not having a suitable idea, or it's quite not doable with .NET ST 2.0 and with keeping the effort adequate.

[SteveSyfuhs]

Thanks for looking into this. That was quite the undertaking already. I agree it gets messy pretty quickly and that makes it unmanageable. In the near term we have internal projects that rely on this being compatible with the full framework so until those change to Core we're a bit stuck.

My original intent with the PAL was to rewrite the Windows bits to p/invoke directly and leave the Linux/Mac PALs alone until a suitable API was surfaced in .NET directly. I had no desire to duplicate the OpenSSL shim. It wasn't a perfect plan, but it covered the majority of the user base.

[gfoidl]

Ah, I gotcha. Now I've a plan for Windows only.