Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Add API to enable comparision of corefx's OpenSSL with a 3rd party P/Invoke #32900

Merged
merged 5 commits into from
Oct 20, 2018

Conversation

bartonjs
Copy link
Member

This adds SafeEvpPKeyHandle.OpenSslVersion.

3rd party code which is doing a P/Invoke into OpenSSL (direct or via their own shim) and using RSAOpenSsl..ctor(SafeEvpPKeyHandle) (or DSAOpenSsl or ECDsaOpenSsl) is encouraged to invoke the appropriate version function for their library (SSLeay() or OpenSSL_version_number()) and compare that against the value of SafeEvpPKeyHandle.OpenSslVersion.

As a structural change, this also changes the shim method from a 32-bit to a 64-bit answer, to ensure that we don't truncate in the future if 64-bit OpenSSL changes versioning schemes to exceed a 32-bit value (the native API returns a *NIX C long).

Fixes #32718.

@bartonjs bartonjs added this to the 3.0 milestone Oct 18, 2018
@bartonjs bartonjs self-assigned this Oct 18, 2018
int minor = (int)((versionNumber >> 20) & 0xFF);
int fix = (int)((versionNumber >> 12) & 0xFF);

s_opensslVersion = new Version(major, minor, fix);
Copy link
Member

Choose a reason for hiding this comment

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

Where is this used? I'm just surprised to see us parsing it like this, given that we explicitly opted to expose a long due to it being opaque data. Is this just for test code? And if so, should this OpenSslVersion property be moved into a test assembly somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this just for test code?

Yeah.

And if so, should this OpenSslVersion property be moved into a test assembly somewhere?

What a remarkably reasonable question that means I have more work to do. Okie dokie 😄.

@bartonjs
Copy link
Member Author

@dotnet-bot Test Linux arm64 Release Build please (System.Net.Internals.SocketExceptionFactory+ExtendedSocketException : Resource temporarily unavailable during NameResolution tests)

@stephentoub
Copy link
Member

@dotnet-bot Test Linux arm64 Release Build please
@dotnet-bot test Packaging All Configurations x64 Debug Build please

@stephentoub stephentoub merged commit 05d119e into dotnet:master Oct 20, 2018
@bartonjs bartonjs deleted the openssl_version_api branch October 23, 2018 15:06
@bartonjs bartonjs removed their assignment Nov 23, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…Invoke (dotnet/corefx#32900)

* Add OpenSslVersionNumber

* Fix native compilation

* Seed some API doc comments

* Add a test

* Move (test) PlatformDetection's OpenSslVersion into PlatformDetection.


Commit migrated from dotnet/corefx@05d119e
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.

2 participants