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

[WIP: DO NOT MERGE] OpenSSL but pick up and use 1.1 where availalbe #34729

Closed
wants to merge 8 commits into from

Conversation

omajid
Copy link
Member

@omajid omajid commented Jan 21, 2019

This is #34443 but actually picks up OpenSSL 1.1 if it's installed (instead of always preferring 1.0). This PR is for testing how things look in CI. It's not meant to be reviewed or merged.

Please do not review or merge this. This is for testing #34443 only.

filipnavara and others added 8 commits January 9, 2019 20:11
* Don't write a separator after the empty DN
* Make T61String behave like it does on Windows (UTF-8 with a Latin-1 fallback)
* Use the managed decoder on Linux, instead of a lot of P/Invokes back into OpenSSL.
* Drop pal_asn1_print in favor of the managed code that is already used on macOS.

* Add handling of T61 strings to ManagedCertificateFinder.DerStringToManagedString.
This changes the functional code to use OpenSSL 1.1 API in the places where the API changed. "apibridge" provides equivalent methods for the OpenSSL 1.0 environment.

The following configurations have been tested:

* Non-portable against OpenSSL 1.0
* Non-portable against OpenSSL 1.1
* Portable, built against OpenSSL 1.0 and run against OpenSSL 1.0
* Portable, built against OpenSSL 1.0 and run against OpenSSL 1.1
* Portable, built against OpenSSL 1.1 and run against OpenSSL 1.0
* Portable, built against OpenSSL 1.1 and run against OpenSSL 1.1

In opensslshim, the PER_FUNCTION_BLOCK macro style has been broken up into a named purposes:

* REQUIRED_FUNCTION(fn)
  * API that we use unconditionally, regardless of version
  * Formerly PER_FUNCTION_BLOCK(fn, true)
* NEW_REQUIRED_FUNCTION(fn)
  * API that we use unconditionally in paths that only exist against OpenSSL 1.1, is not probed for when the runtime is 1.0
* LIGHTUP_FUNCTION(fn)
  * API that might not exist, must be probed with API_EXISTS checks before being utilized
  * Formerly PER_FUNCTION_BLOCK(fn, false)
* FALLBACK_FUNCTION(fn)
  * API that is required on OpenSSL 1.1, and when not found will bind to a method named local_#fn in the shim library
* RENAMED_FUNCTION(fn,oldfn)
  * Handles a rename with no signature change from oldfn to newfn, binds appropriately based on the runtime library.
* LEGACY_FUNCTION(fn)
  * API that we use unconditionally in paths that only exist against OpenSSL 1.0, is not probed for when the runtime is 1.1.

Two new #defines are available, but ideally need no further usage:

* NEED_OPENSSL_1_0
  * Defined when building portable, or on non-portable when the headers are OpenSSL 1.0
* NEED_OPENSSL_1_1
  * Defined when building portable, or on non-portable when the headers are OpenSSL 1.1
This is a leftover from before the OpenSSL 1.0/1.1 hybridization. There
was no HMAC_CTX_free() in 1.0.

Fixes #34210
Future releases of .NET Core prefer OpenSSL 1.1.x. For the sake of
compatiblity, 2.x releases should prefer 1.0.x.
…bcurl

Rather than check a generic 1.0/1.1, test for the specific library version that
the crypto shim has loaded.  This makes things work when both libcurl
and the crypto shim are using OpenSSL 1.1 and also prevents a state where two
different copies of the library (at different patch versions) are utilized.
OpenSSL 1.0 and 1.1 have different messages for the errors, but the same
error code. So only check the error code, not the exact error message.

This is a port of dotnet#30889.
This reverts commit 60e4274.

FOR TESTING ONLY. DO NOT MERGE.
@omajid omajid changed the title [WIP: DO NOT MERGE] [WIP: DO NOT MERGE] OpenSSL but pick up and use 1.1 where availalbe Jan 21, 2019
@bartonjs
Copy link
Member

@omajid Is this PR still needed for anything?

@omajid
Copy link
Member Author

omajid commented Feb 12, 2019

Nope. Closing now. Thanks for reminding me.

@omajid omajid closed this Feb 12, 2019
@karelz karelz added this to the 2.1.x milestone Mar 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants