Skip to content
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

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

scott-xu
Copy link
Collaborator

@scott-xu scott-xu commented Apr 6, 2024

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 [API Proposal]: Add ECDiffieHellman.DeriveSecretAgreement method dotnet/runtime#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 scott-xu marked this pull request as ready for review April 6, 2024 13:41
@scott-xu scott-xu requested a review from Rob-Hague April 6, 2024 13:41
@scott-xu
Copy link
Collaborator Author

scott-xu commented Apr 6, 2024

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

@scott-xu scott-xu self-assigned this Apr 6, 2024
@Rob-Hague
Copy link
Collaborator

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
Copy link
Collaborator Author

scott-xu commented Apr 7, 2024

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

@scott-xu
Copy link
Collaborator Author

scott-xu commented Apr 9, 2024

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
Copy link
Contributor

mus65 commented Apr 16, 2024

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. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants