[release/2.1] Add hybrid support for OpenSSL 1.0 and 1.1 #34443

omajid · 2019-01-08T23:00:00Z

See https://github.com/dotnet/corefx/issues/33870 for the background.

This includes ports of:

And a new commit (60e4274) that makes portable builds of .NET Core 2.1 prefer OpenSSL 1.0.x over 1.1.x. This is to try and avoid any surprises if both OpenSSL 1.0 and 1.1 are installed on a system and the user upgrades from a previous 2.1 release to one with this update.

bartonjs · 2019-01-08T23:10:41Z

@omajid Why 2.2? If it's put into 2.1 it'll auto-merge to 2.2.

omajid · 2019-01-09T14:56:20Z

@omajid Why 2.2? If it's put into 2.1 it'll auto-merge to 2.2.

Crud. I didn't realize that. I figured we would backport into 2.2 and then 2.1 if 2.2 works okay. Let me do this against 2.1, then.

src/Native/Unix/System.Security.Cryptography.Native/openssl.h

* 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.

omajid · 2019-01-10T01:34:24Z

src/Native/Unix/System.Security.Cryptography.Native/apibridge.cpp

+ return 1;
+}
+
+#ifndef SSLEAY_VERSION


This ifndef/endif is new.

omajid · 2019-01-10T01:35:15Z

src/Native/Unix/System.Security.Cryptography.Native/openssl.cpp

@@ -1242,6 +1271,26 @@ extern "C" int32_t CryptoNative_LookupFriendlyNameByOid(const char* oidValue, co
 return 0;
 }

+#ifndef OPENSSL_VERSION


This hunk is new. The code in master uses OpenSSL_version_num instead.

src/Native/Unix/System.Security.Cryptography.Native/openssl.cpp

omajid · 2019-01-10T01:36:19Z

src/Native/Unix/System.Security.Cryptography.Native/openssl.h

+#include "pal_compiler.h"
+#include "opensslshim.h"
+
+extern "C" char* CryptoNative_SSLEayVersion();


I have reduced the number of delcarations in this file to the bare minimum that's required to build.

src/Native/Unix/System.Security.Cryptography.Native/pal_asn1.cpp

src/Native/Unix/System.Security.Cryptography.Native/pal_ssl.cpp

omajid · 2019-01-10T23:56:28Z

The System.Security.Cryptography.X509Certificates.Tests failure (before my last change) looks like #30889

bartonjs · 2019-01-11T15:25:04Z

The System.Security.Cryptography.X509Certificates.Tests failure (before my last change) looks like #30889

Ah, the part of that change that stopped looking at the string seems worth taking; but the part that changed how the error code was processed seems unrelated to this effort.

src/Native/Unix/System.Security.Cryptography.Native/opensslshim.cpp

src/Native/Unix/System.Security.Cryptography.Native/pal_dsa.cpp

src/Native/Unix/System.Security.Cryptography.Native/pal_ssl.cpp

src/System.Security.Cryptography.OpenSsl/tests/EcDsaOpenSslTests.cs

src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs

src/Native/Unix/System.Security.Cryptography.Native/opensslshim.cpp

bartonjs

It would be nice to reduce the temporary strings in the version number builder; otherwise this looks ready for the servicing process.

omajid · 2019-01-17T16:32:11Z

@bartonjs Do the CI failures look related to my changes? Or are they known issues? I am wondering what I need to do about that.

bartonjs · 2019-01-17T17:54:46Z

@omajid My guess is that they're intermittent failures. If you're going to add a commit for the temporary strings problem I'd just wait for that automatic rerun, or we can just run them again.

@karelz https://mc.dot.net/#/user/omajid/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Frelease~2F2.1~2F/test~2Ffunctional~2Fcli~2F/0f602865e769f68a58d9921a17ae06b86c0a9972/workItem/System.Net.Security.Tests/analysis/xunit/System.Net.Security.Tests.ServerAsyncAuthenticateTest~2FServerAsyncAuthenticate_AllClientVsIndividualServerSupportedProtocols_Success(serverProtocol:%20Ssl3) looks an awful lot like https://github.com/dotnet/corefx/issues/24356; but I don't know a good way to find out if it has been recently failing in release/2.1. I'm fairly certain that changing how we bind to OpenSSL on Linux shouldn't cause these loopback adapter-based tests to fail on Windows, though 😄.

* Drop pal_asn1_print in favor of the managed code that is already used on macOS. * Add handling of T61 strings to ManagedCertificateFinder.DerStringToManagedString.

omajid · 2019-01-17T20:38:25Z

@bartonjs I fixed the span issues (and pushed a rebase) but I found another issue building non-portable against openssl 1.1. Working on a fix for that now.

src/Native/Unix/System.Security.Cryptography.Native/pal_asn1_print.cpp

omajid · 2019-01-19T02:35:45Z

@bartonjs should we look into backporting #33468 (as a separate PR) too?

bartonjs · 2019-01-19T03:02:31Z

Hmm, that PR is slightly harder. It should be a separate thing, since shiproom might feel differently about that particular part of the change (that it should be more nuanced, etc).

omajid · 2019-01-21T23:27:29Z

Okay. Lets ignore #33468 for now, then.

bartonjs · 2019-01-22T23:12:57Z

Description

Backport support for OpenSSL 1.1 to .NET Core 2.1. This enables 2.1 to continue running on newer Linux distros as they start to drop support for OpenSSL 1.0.

Customer Impact

As a differentiator from .NET Core 3.0, on 2.1 (and 2.2) the runtime will prefer OpenSSL 1.0. So existing customers on existing OSes will be unaffected. Customers who do not have OpenSSL 1.0, but do have OpenSSL 1.1 will cease getting an error that the dependency cannot be found and their scenarios will work.

Regression?

No, reaction to evolution of new OS versions

Packaging reviewed?

Affected components are all part of Microsoft.NETCore.App

Risk

Low, this is a backport of a change that has been in 3.0 since August 2018.

leecow · 2019-01-31T19:31:31Z

@danmosemsft to shepherd through shiproom

jamshedd · 2019-02-07T19:02:47Z

Approved for 2.1.9.

omajid · 2019-02-13T20:26:29Z

@bartonjs Do you know what the schedule for merging this looks like? Are we waiting for some repo to open up for commits?

bartonjs · 2019-02-13T20:43:11Z

Do you know what the schedule for merging this looks like? Are we waiting for some repo to open up for commits?

The release branches have controlled merging, with lockouts during the time that a release is in progress. Since 2.1.8 and 2.2.2 released yesterday I'd expect the branches to open soon (today/tomorrow?)

This was a mistake in e4bcbd5 (dotnet#34443) made when backporting it to the release branches. All other uses of NEED_OPENSSL_1_0 are guarded by an #ifdef, not an #if. This one should be too. The warning is seen when building corefx using source-build. I couldn't observe it directly: source-build/src/corefx/src/Native/Unix/System.Security.Cryptography.Native/pal_asn1_print.cpp:34:5: error: 'NEED_OPENSSL_1_0' is not defined, evaluates to 0 [-Werror,-Wundef] #if NEED_OPENSSL_1_0 ^

* Change #if NEED_OPENSSL_1_0 to #ifdef NEED_OPENSSL_1_0 This was a mistake in e4bcbd5 (#34443) made when backporting it to the release branches. All other uses of NEED_OPENSSL_1_0 are guarded by an #ifdef, not an #if. This one should be too. The warning is seen when building corefx using source-build. I couldn't observe it directly: source-build/src/corefx/src/Native/Unix/System.Security.Cryptography.Native/pal_asn1_print.cpp:34:5: error: 'NEED_OPENSSL_1_0' is not defined, evaluates to 0 [-Werror,-Wundef] #if NEED_OPENSSL_1_0 ^ * Disable used-but-marked-unused warnings in System.Security.Cryptography.Native When building in non-portable mode, some OpenSSL 1.1 function defnitions that are marked as unused can be picked up by our build. When those functions are called, clang reports a warning and fails the build: src/Native/Unix/System.Security.Cryptography.Native/openssl.cpp:432:12: error: 'sk_ASN1_OBJECT_num' was marked unused but was used [-Werror,-Wused-but-marked-unused] return sk_ASN1_OBJECT_num(eku); ^ This 'unused' attribute was recently added to sk_* methods in OpenSSL 1.1: openssl/openssl#8246

krwq reviewed Jan 9, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/openssl.h Outdated Show resolved Hide resolved

NetFX compatibility fixes for X500DistinguishedName.

7f5ccbd

* 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.

omajid force-pushed the 2.2-openssl-1.1 branch from 3cf174a to 757ab1f Compare January 10, 2019 01:32

omajid changed the base branch from release/2.2 to release/2.1 January 10, 2019 01:33

omajid commented Jan 10, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/openssl.cpp Show resolved Hide resolved

omajid commented Jan 10, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/pal_asn1.cpp Show resolved Hide resolved

omajid commented Jan 10, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/pal_ssl.cpp Outdated Show resolved Hide resolved

bartonjs changed the title ~~[WIP] [release/2.2] Add hybrid support for OpenSSL 1.0 and 1.1~~ [WIP] [release/2.1] Add hybrid support for OpenSSL 1.0 and 1.1 Jan 10, 2019

bartonjs reviewed Jan 11, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/opensslshim.cpp Outdated Show resolved Hide resolved

bartonjs reviewed Jan 11, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/pal_dsa.cpp Outdated Show resolved Hide resolved

bartonjs reviewed Jan 11, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/pal_ssl.cpp Outdated Show resolved Hide resolved

bartonjs reviewed Jan 11, 2019

View reviewed changes

src/System.Security.Cryptography.OpenSsl/tests/EcDsaOpenSslTests.cs Outdated Show resolved Hide resolved

omajid force-pushed the 2.2-openssl-1.1 branch from 600d472 to 0f60286 Compare January 14, 2019 21:40

bartonjs reviewed Jan 17, 2019

View reviewed changes

src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs Show resolved Hide resolved

bartonjs reviewed Jan 17, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/opensslshim.cpp Show resolved Hide resolved

bartonjs approved these changes Jan 17, 2019

View reviewed changes

omajid force-pushed the 2.2-openssl-1.1 branch from 0f60286 to 717cda2 Compare January 17, 2019 18:27

Drop pal_asn1_print in favor of the managed code

3fd578c

* Drop pal_asn1_print in favor of the managed code that is already used on macOS. * Add handling of T61 strings to ManagedCertificateFinder.DerStringToManagedString.

omajid force-pushed the 2.2-openssl-1.1 branch from 717cda2 to 813afa3 Compare January 17, 2019 21:37

omajid commented Jan 17, 2019

View reviewed changes

src/Native/Unix/System.Security.Cryptography.Native/pal_asn1_print.cpp Outdated Show resolved Hide resolved

This was referenced Jan 19, 2019

RHEL 8 support dotnet/source-build#876

Closed

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

Closed

danmoseley added the Servicing-consider Issue for next servicing release review label Feb 1, 2019

danmoseley added this to the 2.1.x milestone Feb 1, 2019

jamshedd modified the milestones: 2.1.x, 2.1.9 Feb 7, 2019

jamshedd added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 7, 2019

bartonjs mentioned this pull request Feb 13, 2019

Unable to install latest x64 sdk on Debian Buster dotnet/core#1632

Closed

wtgodbe merged commit 16a6d0b into dotnet:release/2.1 Feb 13, 2019

omajid mentioned this pull request Mar 6, 2019

[release/2.1] Fix build errors in some build configurations #35805

Merged

JustArchi mentioned this pull request Apr 30, 2019

SSL connection JustArchiNET/ArchiSteamFarm#1212

Closed

teqdruid mentioned this pull request Jun 3, 2019

dotnet restore --interactive fails with SSL errors on WSL microsoft/artifacts-credprovider#97

Closed

juliobbv mentioned this pull request Aug 5, 2019

Cannot configured on Debian 10 microsoft/azure-pipelines-agent#2373

Closed

Wuerfelbecher mentioned this pull request Aug 28, 2019

Add support for Raspbian Buster (openssl 1.1.1c) jellyfin/jellyfin#1670

Closed

qmfrederik mentioned this pull request Nov 18, 2019

Use .NET Core 2.1.13 microsoft/azure-pipelines-agent#2616

Merged

IlyaBiryukov mentioned this pull request Dec 10, 2019

Use libssl-dev on Ubuntu microsoft/live-share#3044

Merged

dagood mentioned this pull request Jan 30, 2020

In 2.1/2.2, upgrade runtime-deps OpenSSL dependency to 1.1 dotnet/runtime#3426

Closed

jkotas mentioned this pull request May 4, 2021

RFC: dotnet [Arm64 | x64] coexistence dotnet/sdk#16896

Closed

omajid mentioned this pull request Nov 17, 2022

Update Linux support statements dotnet/core#7971

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release/2.1] Add hybrid support for OpenSSL 1.0 and 1.1 #34443

[release/2.1] Add hybrid support for OpenSSL 1.0 and 1.1 #34443

omajid commented Jan 8, 2019 •

edited

Loading

bartonjs commented Jan 8, 2019

omajid commented Jan 9, 2019

omajid Jan 10, 2019

omajid Jan 10, 2019

omajid Jan 10, 2019

omajid commented Jan 10, 2019

bartonjs commented Jan 11, 2019

bartonjs left a comment

omajid commented Jan 17, 2019

bartonjs commented Jan 17, 2019

omajid commented Jan 17, 2019

omajid commented Jan 19, 2019

bartonjs commented Jan 19, 2019

omajid commented Jan 21, 2019

bartonjs commented Jan 22, 2019

leecow commented Jan 31, 2019

jamshedd commented Feb 7, 2019

omajid commented Feb 13, 2019

bartonjs commented Feb 13, 2019

[release/2.1] Add hybrid support for OpenSSL 1.0 and 1.1 #34443

[release/2.1] Add hybrid support for OpenSSL 1.0 and 1.1 #34443

Conversation

omajid commented Jan 8, 2019 • edited Loading

bartonjs commented Jan 8, 2019

omajid commented Jan 9, 2019

omajid Jan 10, 2019

Choose a reason for hiding this comment

omajid Jan 10, 2019

Choose a reason for hiding this comment

omajid Jan 10, 2019

Choose a reason for hiding this comment

omajid commented Jan 10, 2019

bartonjs commented Jan 11, 2019

bartonjs left a comment

Choose a reason for hiding this comment

omajid commented Jan 17, 2019

bartonjs commented Jan 17, 2019

omajid commented Jan 17, 2019

omajid commented Jan 19, 2019

bartonjs commented Jan 19, 2019

omajid commented Jan 21, 2019

bartonjs commented Jan 22, 2019

Description

Customer Impact

Regression?

Packaging reviewed?

Risk

leecow commented Jan 31, 2019

jamshedd commented Feb 7, 2019

omajid commented Feb 13, 2019

bartonjs commented Feb 13, 2019

omajid commented Jan 8, 2019 •

edited

Loading