-
Notifications
You must be signed in to change notification settings - Fork 4.9k
[release/3.1] Always build the OpenSSL shim in portable mode on macOS. #42801
Conversation
70396aa
to
59755c1
Compare
src/Native/Unix/System.Security.Cryptography.Native/opensslshim.c
Outdated
Show resolved
Hide resolved
# and our prebuilts should not assume a specific ABI version for the types | ||
# that use OpenSSL at runtime. | ||
if (APPLE) | ||
set(FEATURE_DISTRO_AGNOSTIC_SSL True) |
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.
It seems that rather than hardcoding this in the cmake script, we should pass the -portable option to the build-native.sh like we do for Linux and update the build-native.sh to accept that option for OSX too.
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.
I thought about that, but my inclination was to change the build from "never portable" to "always portable", and it seems like rather than failing the build when you run build-native.sh
without -portable
that I'd just set it to true in here.
If you think that we want to enable the option and update the rpath rewrites to also rpath rewrite 1.1, that's valid. But, unlike Linux--where I expect the distros to build non-portable--I don't think that we'd really go back to a tightly-linked version for macOS.
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.
After thinking about it a bit more, I agree with you.
Marking servicing-consider just so we can talk about it now. |
Shiproom okays change in principle - please set servicing-consider when this is ready to go, template filled out etc just to follow the usual process. |
DescriptionOn macOS OpenSSL is only used for 2 platform-agnostic types (AesCcm, AesGcm), both added in .NET Core 3.0, as well as with 5 interop types: DSAOpenSsl, ECDiffieHellmanOpenSsl, ECDsaOpenSsl, RSAOpenSsl, SafeEvpPKeyHandle. Currently, our macOS builds use a single ABI version of OpenSSL which is determined at compile time. Further, the version is the now out-of-support OpenSSL 1.0.2. This change will make macOS use the same late binding that the portable Linux release does, and dynamically support the better of OpenSSL 1.0.2 or OpenSSL 1.1. Customer ImpactCustomers who use the interop types will be broken if they are P/Invoking against a different version of OpenSSL than we end up binding to and using those pointers with our *OpenSsl classes. If they are following the guidance for interop with these types they will be aware of the state by reading the SafeEvpPKeyHandle.OpenSslVersion property. These customers can force the downgrade by use of the CLR_OPENSSL_VERSION_OVERRIDE environment variable. Regression?No, reacting to dependency lifetimes. Packaging reviewed?Only impacts the shared runtime. RiskLow. This code has been used for multiple releases with Linux, and has been tested with all 4 combinations of OpenSSL 1.0.2 and OpenSSL 1.1 being present on the system. |
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.
LGTM, thank you!
approved in email for 3.1.2 |
No description provided.