[release/9.0] Make SafeEvpPKeyHandle.OpenKeyFromProvider throw PNSE on OSSL less than 3.0 #106753
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
4 tasks
bartonjs
approved these changes
Aug 21, 2024
jeffhandley
approved these changes
Aug 21, 2024
There was a problem hiding this comment.
I support merging this into 9.0 RC2; it's related to new feature work in .NET 9.
@ericstj -- can you review too please for acceptance into 9.0?
ericstj
approved these changes
Aug 21, 2024
krwq
added
the
NO-MERGE
|
Marking as NO-MERGE until I figure out #106794 since we don't want test failures on official builds (this is most likely a test issue which happens sometimes on osx builds) |
krwq
removed
the
NO-MERGE
|
I've incorporated the fix from #106804 into this PR, note that was test only fix. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #106397 to release/9.0 (There is also identical opened against RC1 - only one should be merged depending on how/if this gets approved)
/cc @krwq
Customer Impact
Improves error message and exception type from CryptographicException with cryptic message to PlatformNotSupportedException with actionable error message when feature is not available when running against OpenSSL <= 1.1.1. Feature was introduced in Preview 7.
Additionally it fixes test failures when running manual tests against OpenSSL 1.1.1 when feature is not available.
This isn't particularly blocking for customers but it does improve user experience when running against low versions of OpenSSL. Changing this in just .NET 10 would change exception type and be potentially breaking.
Regression
Testing
Testing against different versions of OpenSSL - change adds a test case but it's not currently exercised in CIs which runs only against OpenSSL >= 3.0
Risk
Low. This only adds a single out bool flag in the native and interop layer which allows for proper feature detection and throwing better exception message. The remainder of the change improves test coverage and fixes test issues when running against OpenSSL 1.1.1 (some tests require specific setup with TPM so are not running against CI, some other are automated).