-
-
Notifications
You must be signed in to change notification settings - Fork 910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use BCL ECDiffieHellman for KeyExchange instead of BouncyCastle (.NET 8.0 onward only) #1371
base: develop
Are you sure you want to change the base?
Conversation
Windows 8.1 and Windows Server 2012 R2 does not support ECDiffieHellman.DeriveRawSecretAgreement(ECDiffieHellmanPublicKey). Windows 8.1 LifecycleSupport Dates
Windows 8.1 EOL announcement: https://learn.microsoft.com/en-us/lifecycle/announcements/windows-8-1-end-support-january-2023 Windows Server 2012 R2 LifecycleSupport Dates
Releases
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 |
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. |
Thanks, I updated the description with Advantages to elaborate on the change. I also updated note2. |
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 |
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. :) |
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
Notes:
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 throwPlatformNotSupportedException
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.