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

Detect presence of OpenSSL for crypto primitives #52648

Merged
merged 7 commits into from
May 13, 2021

Conversation

vcsjones
Copy link
Member

This exposes a property that determines if OpenSSL can be loaded by attempting to dlopen it using the existing routines that respect environment variable overrides, etc. The result of this is cached in a Lazy<bool>.

I was able to run tests in S.S.Cryptography.Algorithms.Tests locally with no OpenSSL present on the system without hitting a sigabrt.

Closes #52598

@ghost
Copy link

ghost commented May 12, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

This exposes a property that determines if OpenSSL can be loaded by attempting to dlopen it using the existing routines that respect environment variable overrides, etc. The result of this is cached in a Lazy<bool>.

I was able to run tests in S.S.Cryptography.Algorithms.Tests locally with no OpenSSL present on the system without hitting a sigabrt.

Closes #52598

Author: vcsjones
Assignees: -
Labels:

area-System.Security

Milestone: -

@vcsjones vcsjones force-pushed the openssl-available branch from 740fc76 to 7e3ae65 Compare May 12, 2021 17:21
@vcsjones
Copy link
Member Author

Sorry for the force push. None of the force-pushed changes had code differences. Needed to fix my authorship of the commit.

@@ -678,6 +678,17 @@ public static IEnumerable<object[]> GetNistCcmTestCasesWithNonEmptyPT()

public class AesCcmIsSupportedTests
{
public static bool RuntimeSaysIsNotSupported => !AesCcm.IsSupported;

[ConditionalFact(nameof(RuntimeSaysIsNotSupported))]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if these tests will get exercised during CI or not, but I confirmed they work locally without openssl.

@vcsjones
Copy link
Member Author

I'm fairly sure the failing legs are unrelated from the changes. The failing Browser legs passed in the penultimate commit, and the last commit was just a merge commit.

The macOS failure also seems unrelated and appears to have been flaky in the past.

@bartonjs bartonjs merged commit 189105a into dotnet:main May 13, 2021
@vcsjones vcsjones deleted the openssl-available branch May 13, 2021 20:15
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AesGcm.IsSupported and AesCcm.IsSupported should not unconditionally return true for MacOS
5 participants