Use BCL ECDiffieHellman for KeyExchange instead of BouncyCastle (.NET 8.0 onward only)

[scott-xu]

This PR leverages BCL's ECDiffieHellman for key exchange instead of using BouncyCastle. Cryptographic operations in BCL are done by operating system (OS) libraries.

Advantages:

It inherits the advantages described in https://learn.microsoft.com/en-us/dotnet/standard/security/cross-platform-cryptography

• .NET apps benefit from OS reliability. Keeping cryptography libraries safe from vulnerabilities is a high priority for OS vendors. To do that, they provide updates that system administrators should be applying.
• .NET apps have access to FIPS-validated algorithms if the OS libraries are FIPS-validated.

Notes:

1. It is only for .NET 8.0+ because method ECDiffieHellman.DeriveRawSecretAgreement(ECDiffieHellmanPublicKey) is avaliable only at .NET 8.0+. See https://github.com/dotnet/runtime/issues/71613.
2. It will throw PlatformNotSupportedException if the OS is Windows and below Windows 10. See this line of code. For this case, we use BouncyCastle as fallback. Based on below lifecycle table, I think it should be fine to throw PlatformNotSupportedException in Windows 8.1 and Windows Server 2012 R2 rather than use BouncyCastle as fallback. Then we can remove the BouncyCastle dependency when target .NET 8.0 onward.

[scott-xu]

Windows 8.1 and Windows Server 2012 R2 does not support ECDiffieHellman.DeriveRawSecretAgreement(ECDiffieHellmanPublicKey). Here's their lifecyle for reference.

Windows 8.1 Lifecycle

Support Dates

Listing	Start Date	Mainstream End Date	Extended End Date
Windows 8.1	Nov 13, 2013	Jan 9, 2018	Jan 10, 2023

Windows 8.1 EOL announcement: https://learn.microsoft.com/en-us/lifecycle/announcements/windows-8-1-end-support-january-2023

Windows Server 2012 R2 Lifecycle

Support Dates

Listing	Start Date	Mainstream End Date	Extended End Date
Windows Server 2012 R2	Nov 25, 2013	Oct 9, 2018	Oct 10, 2023

Releases

Version	Start Date	End Date
Extended Security Update Year 3	Oct 15, 2025	Oct 13, 2026
Extended Security Update Year 2	Oct 9, 2024	Oct 14, 2025
Extended Security Update Year 1	Oct 11, 2023	Oct 8, 2024
Original Release	Nov 25, 2013	Oct 10, 2023

Windows Server 2012 R2 EOL announcement: https://learn.microsoft.com/en-us/lifecycle/announcements/windows-server-2012-r2-end-of-support

Here's the link for Extended Security Updates Overview

[Rob-Hague]

Thanks. It's a nice idea but I'm not sure it's worth the additional complexity at this time - with respect to the OS support and that we still have the old code which is no longer exercised in CI. Of course we could add a target for net6.0 on the integration tests but my gut feeling is that this change would be better left for the future. I could be convinced otherwise.

[scott-xu]

Thanks, I updated the description with Advantages to elaborate on the change. I also updated note2.

[scott-xu]

I think we don't need to worry about OS issue too much. Using an EOL Windows with the latest .NET 8.0+ seems bizarre to me. If they really want, they can use .NET Framework or .NET 6.0. What do you think? @Rob-Hague

[mus65]

Using an EOL Windows with the latest .NET 8.0+ seems bizarre to me.

I don't think it's that bizarre, I have to deal with this exact situation at work myself. Our application is already on .NET 8 but there are quite a few customers who self-host and are still running Win2012.

But I think dropping support for this would be fine as long as this is mentioned in the release notes. This would actually give me a good reason to drop support for Win2012 in our application. :)

[Rob-Hague]

I think we can take this, if:

1. We guard it in the same way the BCL does, with a fallback to BouncyCastle, i.e.:

   
   #if NET8_0_OR_GREATER
   if (!OperatingSystem.IsWindows() || OperatingSystem.IsWindowsVersionAtLeast(10))
   {
   // BCL stuff
   // wrap this in try { } catch (PlatformNotSupportedException) just to be safe?
   
   return ...
   }
   #endif

// BouncyCastle fallback

2. It provides a worthwhile performance improvement over the BouncyCastle path

We don't want to break anyone for no reason, and we will inevitably depend on BouncyCastle, so the only tangible benefit here might be performance

[scott-xu]

Done except

// wrap this in try { } catch (PlatformNotSupportedException) just to be safe?

What do you mean by safe?

[Rob-Hague]

I was thinking, since support does not seem to be universal across platforms, that maybe we wrap the BCL code in try-catch so that it still falls back to BouncyCastle if it's not supported. But I think it's OK, only Windows seems like a problem case from what I can tell: https://github.com/dotnet/runtime/commit/be823a152f76e9a71a752e41777069d2a47fd34e

[Rob-Hague]

2. It provides a worthwhile performance improvement over the BouncyCastle path

Any idea here?

[scott-xu]

2. It provides a worthwhile performance improvement over the BouncyCastle path

Any idea here?

Actually I'm more leaning towards to the policy "we don't implement cryptographic algorithms, but instead defer to an OS implementation" rather than performance improvement.

[scott-xu]

Now the code structure is very similar with AesCipher's implementation. Ready to review and merge @Rob-Hague @WojciechNagorski