-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Drop support for OpenSSL primitives on macOS #116481
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
Conversation
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes vestigial OpenSSL-based implementations on macOS, replacing them with stubs that throw PlatformNotSupportedException, and updates project files to stop building or running those tests on macOS.
- Deleted the SafeEvpPKeyHandle OpenSSL macOS implementation.
- Added NotSupported source files and adjusted
<Compile>
includes in the main crypto csproj. - Updated test projects to ignore OpenSSL tests on macOS.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/libraries/tests.proj | Removed ARM64/macOS exclusion for OpenSSL tests (no longer needed) |
src/libraries/System.Security.Cryptography/src/.../SafeEvpPKeyHandle.OpenSsl.macOS.cs | Deleted macOS-specific OpenSSL key handle implementation |
src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj | Reworked OpenSSL ItemGroups: removed AEAD group, added NotSupported stubs and moved compile items |
src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj | Extended IgnoreForCI to also skip OpenSSL tests on macOS |
Comments suppressed due to low confidence (2)
src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj:5
- There are no new tests verifying that AesCcm and other OpenSSL-based APIs on macOS throw PlatformNotSupportedException. Consider adding tests (with
ActiveIssue
for macOS) to cover the new NotSupported stubs.
<IgnoreForCI Condition="'$(TargetOS)' == 'android' or '$(TargetOS)' == 'ios' or '$(TargetOS)' == 'tvos' or '$(TargetOS)' == 'osx'">true</IgnoreForCI>
src/libraries/System.Security.Cryptography.OpenSsl/tests/System.Security.Cryptography.OpenSsl.Tests.csproj:4
- [nitpick] The comment could explicitly list
macOS
alongsideiOS
andtvOS
for clarity, e.g. “The library is not supported on Android or Apple platforms (iOS, tvOS, macOS)”.
<!-- The library is not supported on Android / Apple (PNSE) -->
src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj
Show resolved
Hide resolved
/ba-g iOS queue is dead-lettered, and this change doesn't affect iOS. |
Breaking change doc. dotnet/docs#46789 |
macOS has historically supported
AesCcm
andOpenSsl
suffixed types likeRSAOpenSsl
as a "light up" option.This support is largely vestigial from when .NET used OpenSSL on macOS as its main cryptographic implementation. It hasn't had test coverage in some time due to the hardened runtime preventing loading the openssl libraries.
This pull request changes the OpenSSL primitives on macOS to unconditionally throw PlatformNotSupportedException.
It does not fully remove building of System.Security.Cryptography.Native or shipping the native library. That will be done as a follow up pending some open questions.