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

[Breaking change]: SafeEvpPKeyHandle.DuplicateHandle up-refs the handle #42034

Closed
1 of 3 tasks
krwq opened this issue Aug 5, 2024 · 0 comments · Fixed by #42018
Closed
1 of 3 tasks

[Breaking change]: SafeEvpPKeyHandle.DuplicateHandle up-refs the handle #42034

krwq opened this issue Aug 5, 2024 · 0 comments · Fixed by #42018
Assignees
Labels
breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] in-pr This issue will be closed (fixed) by an active pull request. Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@krwq
Copy link
Member

krwq commented Aug 5, 2024

Description

With the work to enable OpenSSL providers support, a change was made to the System.Security.Cryptography.SafeEvpPKeyHandle.DuplicateHandle method, impacting the System.Security.Cryptography.ECDsaOpenSsl and System.Security.Cryptography.RSAOpenSsl constructors that have overloads with System.Security.Cryptography.SafeEvpPKeyHandle causing that external modifications of the passed-in handle will also affect handle stored in the instances of those classes.

Version

.NET 9 Preview 7

Previous behavior

  • System.Security.Cryptography.SafeEvpPKeyHandle.DuplicateHandle created a new EVP_PKEY instance
  • Modifications to the duplicated key (i.e. through direct calls to OpenSSL APIs) did not impact the original key
  • SafeEvpPKeyHandle.DuplicateHandle is called by the constructors of ECDsaOpenSsl and RSAOpenSsl taking SafeEvpPKeyHandle

New behavior

DuplicateHandle increments the reference count of the existing EVP_PKEY and returns a handle to the same key, causing that external calls to OpenSSL APIs which modify EVP_PKEY will also affect instances of duplicated SafeEvpPKeyHandle which include ECDsaOpenSsl and RSAOpenSsl instances created from such handles.

Type of breaking change

  • Binary incompatible: Existing binaries might encounter a breaking change in behavior, such as failure to load or execute, and if so, require recompilation.
  • Source incompatible: When recompiled using the new SDK or component or to target the new runtime, existing source code might require source changes to compile successfully.
  • Behavioral change: Existing binaries might behave differently at run time.

Reason for change

Recommended action

Avoid modifications of EVP_PKEY passed in to .NET APIs. If modifications to EVP_PKEY can't be avoided user should create a copy of EVP_PKEY on their own (i.e. copy parameters into the new EVP_PKEY instance).

Feature area

Cryptography

Affected APIs

  • System.Security.Cryptography.SafeEvpPKeyHandle.DuplicateHandle

DuplicateHandle is called by following public .NET APIs making them also affected:

  • System.Security.Cryptography.ECDsaOpenSsl..ctor(System.Security.Cryptography.SafeEvpPKeyHandle)
  • System.Security.Cryptography.RSAOpenSsl..ctor(System.Security.Cryptography.SafeEvpPKeyHandle)

Everything taking RSA or ECDsa instances originating from SafeEvpPKeyHandle will also be affected.


Associated WorkItem - 294097

@krwq krwq added doc-idea Indicates issues that are suggestions for new topics [org][type][category] breaking-change Indicates a .NET Core breaking change Pri1 High priority, do before Pri2 and Pri3 labels Aug 5, 2024
@dotnet-bot dotnet-bot added ⌚ Not Triaged Not triaged labels Aug 5, 2024
@gewarren gewarren added the 🗺️ reQUEST Triggers an issue to be imported into Quest. label Aug 5, 2024
@dotnet-bot dotnet-bot removed the ⌚ Not Triaged Not triaged label Aug 5, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr This issue will be closed (fixed) by an active pull request. label Aug 6, 2024
@sequestor sequestor bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates a .NET Core breaking change doc-idea Indicates issues that are suggestions for new topics [org][type][category] in-pr This issue will be closed (fixed) by an active pull request. Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants