From dfa0c242cd9b329554971cf80c094e8f58d756e3 Mon Sep 17 00:00:00 2001

From: Jeremy Barton <jbarton@microsoft.com>

Date: Wed, 26 Sep 2018 15:35:35 -0700

Subject: [PATCH 6/7] Check for the specific in-use version of OpenSSL when

 working with libcurl

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.

---

 .../Interop.Initialization.cs                 |  8 +--

 .../Interop.VersionInfo.cs                    | 44 +++++++++++-

 .../Interop.OpenSslVersion.cs                 |  2 +-

 .../src/System.Net.Http.csproj                |  3 +

 .../CurlHandler.SslProvider.Linux.cs          |  8 +--

 .../FunctionalTests/HttpClientEKUTest.cs      |  2 +-

 ...ttpClientHandlerTest.ClientCertificates.cs | 14 +---

 ...ientHandlerTest.ServerCertificates.Unix.cs |  9 +--

 ...HttpClientHandlerTest.SslProtocols.Unix.cs |  3 +-

 .../System.Net.Http.Functional.Tests.csproj   |  3 +-

 .../tests/FunctionalTests/TestHelper.cs       | 69 +++++++++++++++++++

 11 files changed, 130 insertions(+), 35 deletions(-)

diff --git a/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.Initialization.cs b/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.Initialization.cs

index eef56ec0b3..d6bcc8df02 100644

--- a/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.Initialization.cs

+++ b/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.Initialization.cs

@@ -26,11 +26,11 @@ internal static partial class Interop

 #if !SYSNETHTTP_NO_OPENSSL

             string opensslVersion = Interop.Http.GetSslVersionDescription();

             if (string.IsNullOrEmpty(opensslVersion) ||

-                opensslVersion.IndexOf(Interop.Http.OpenSsl10Description, StringComparison.OrdinalIgnoreCase) != -1)

+                opensslVersion.IndexOf(Interop.Http.OpenSslDescriptionPrefix, StringComparison.OrdinalIgnoreCase) != -1)

             {

-                // CURL uses OpenSSL which we must initialize first to guarantee thread-safety

-                // Only initialize for OpenSSL/1.0, any newer versions may have mismatched

-                // pointers, resulting in segfaults.

+                // CURL uses OpenSSL which we must initialize first to guarantee thread-safety.

+                // We'll wake up whatever OpenSSL we're going to run against, but might later determine that

+                // they aren't compatible.

                 CryptoInitializer.Initialize();

             }

 #endif

diff --git a/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs b/src/Common/src/Interop/Unix/System.Net.Http.Native/Interop.VersionInfo.cs

index 1899fd0af3..8175159b6f 100644

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

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

@@ -3,6 +3,7 @@

 // See the LICENSE file in the project root for more information.

 using System;

+using System.Diagnostics;

 using System.Runtime.InteropServices;

 internal static partial class Interop

@@ -47,8 +48,49 @@ internal static partial class Interop

         [DllImport(Libraries.HttpNative, EntryPoint = "HttpNative_GetSslVersionDescription")]

         internal static extern string GetSslVersionDescription();

-        internal const string OpenSsl10Description = "openssl/1.0";

+        internal const string OpenSslDescriptionPrefix = "OpenSSL/";

         internal const string SecureTransportDescription = "SecureTransport";

         internal const string LibreSslDescription = "LibreSSL";

+

+#if !SYSNETHTTP_NO_OPENSSL

+        private static readonly Lazy<string> s_requiredOpenSslDescription =

+            new Lazy<string>(() => DetermineRequiredOpenSslDescription());

+

+        private static readonly Lazy<bool> s_hasMatchingOpenSsl =

+            new Lazy<bool>(() => RequiredOpenSslDescription == GetSslVersionDescription());

+

+        internal static string RequiredOpenSslDescription => s_requiredOpenSslDescription.Value;

+        internal static bool HasMatchingOpenSslVersion => s_hasMatchingOpenSsl.Value;

+

+        private static string DetermineRequiredOpenSslDescription()

+        {

+            string versionDescription = Interop.OpenSsl.OpenSslVersionDescription();

+            var version = versionDescription.AsSpan();

+

+            // OpenSSL version description looks like this:

+            //

+            // OpenSSL 1.1.1 FIPS  11 Sep 2018

+            //

+            // libcurl's OpenSSL vtls backend ignores status in the version string.

+            // Major, minor, and fix are encoded (by libcurl) as unpadded hex

+            // (0 => "0", 15 => "f", 16 => "10").

+            //

+            // Patch is encoded as in the way OpenSSL would do it.

+

+            string prefix = "OpenSSL ";

+            if (version.StartsWith(prefix))

+            {

+                version = version.Slice(prefix.Length).Trim();

+            }

+            int end = version.IndexOf(" ");

+            if (end != -1)

+            {

+                version = version.Slice(0, end);

+            }

+            version = version.Trim();

+

+            return $"{OpenSslDescriptionPrefix}{version.ToString()}";

+        }

+#endif

     }

 }

diff --git a/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSslVersion.cs b/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSslVersion.cs

index 70805706ef..13c2339ce6 100644

--- a/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSslVersion.cs

+++ b/src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSslVersion.cs

@@ -12,7 +12,7 @@ internal static partial class Interop

         private static Version s_opensslVersion;

         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SSLEayVersion")]

-        private static extern string OpenSslVersionDescription();

+        internal static extern string OpenSslVersionDescription();

         internal static Version OpenSslVersion

         {

diff --git a/src/System.Net.Http/src/System.Net.Http.csproj b/src/System.Net.Http/src/System.Net.Http.csproj

index 66e5b8d5f8..34b4d22e15 100644

--- a/src/System.Net.Http/src/System.Net.Http.csproj

+++ b/src/System.Net.Http/src/System.Net.Http.csproj

@@ -500,6 +500,9 @@

     <Compile Include="$(CommonPath)\Interop\Unix\System.Security.Cryptography.Native\Interop.Crypto.cs">

       <Link>Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Crypto.cs</Link>

     </Compile>

+    <Compile Include="$(CommonPath)\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs">

+      <Link>Common\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs</Link>

+    </Compile>

     <Compile Include="$(CommonPath)\Interop\Unix\System.Security.Cryptography.Native\Interop.Ssl.cs">

       <Link>Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Ssl.cs</Link>

     </Compile>

diff --git a/src/System.Net.Http/src/System/Net/Http/CurlHandler/CurlHandler.SslProvider.Linux.cs b/src/System.Net.Http/src/System/Net/Http/CurlHandler/CurlHandler.SslProvider.Linux.cs

index 55e583e137..2fdcde686d 100644

--- a/src/System.Net.Http/src/System/Net/Http/CurlHandler/CurlHandler.SslProvider.Linux.cs

+++ b/src/System.Net.Http/src/System/Net/Http/CurlHandler/CurlHandler.SslProvider.Linux.cs

@@ -55,7 +55,7 @@ namespace System.Net.Http

                 // Configure the options.  Our best support is when targeting OpenSSL/1.0.  For other backends,

                 // we fall back to a minimal amount of support, and may throw a PNSE based on the options requested.

-                if (CurlSslVersionDescription.IndexOf(Interop.Http.OpenSsl10Description, StringComparison.OrdinalIgnoreCase) != -1)

+                if (Interop.Http.HasMatchingOpenSslVersion)

                 {

                     // Register the callback with libcurl.  We need to register even if there's no user-provided

                     // server callback and even if there are no client certificates, because we support verifying

@@ -169,12 +169,12 @@ namespace System.Net.Http

             {

                 if (certProvider != null)

                 {

-                    throw new PlatformNotSupportedException(SR.Format(SR.net_http_libcurl_clientcerts_notsupported_sslbackend, CurlVersionDescription, CurlSslVersionDescription, Interop.Http.OpenSsl10Description));

+                    throw new PlatformNotSupportedException(SR.Format(SR.net_http_libcurl_clientcerts_notsupported_sslbackend, CurlVersionDescription, CurlSslVersionDescription, Interop.Http.RequiredOpenSslDescription));

                 }

                 if (easy._handler.CheckCertificateRevocationList)

                 {

-                    throw new PlatformNotSupportedException(SR.Format(SR.net_http_libcurl_revocation_notsupported_sslbackend, CurlVersionDescription, CurlSslVersionDescription, Interop.Http.OpenSsl10Description));

+                    throw new PlatformNotSupportedException(SR.Format(SR.net_http_libcurl_revocation_notsupported_sslbackend, CurlVersionDescription, CurlSslVersionDescription, Interop.Http.RequiredOpenSslDescription));

                 }

                 if (easy._handler.ServerCertificateCustomValidationCallback != null)

@@ -187,7 +187,7 @@ namespace System.Net.Http

                     }

                     else

                     {

-                        throw new PlatformNotSupportedException(SR.Format(SR.net_http_libcurl_callback_notsupported_sslbackend, CurlVersionDescription, CurlSslVersionDescription, Interop.Http.OpenSsl10Description));

+                        throw new PlatformNotSupportedException(SR.Format(SR.net_http_libcurl_callback_notsupported_sslbackend, CurlVersionDescription, CurlSslVersionDescription, Interop.Http.RequiredOpenSslDescription));

                     }

                 }

                 else

diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientEKUTest.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientEKUTest.cs

index c6badc770e..9ac90390e3 100644

--- a/src/System.Net.Http/tests/FunctionalTests/HttpClientEKUTest.cs

+++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientEKUTest.cs

@@ -22,7 +22,7 @@ namespace System.Net.Http.Functional.Tests

 #if TargetsWindows

             true;

 #else

-            Interop.Http.GetSslVersionDescription()?.StartsWith(Interop.Http.OpenSsl10Description, StringComparison.OrdinalIgnoreCase) ?? false;

+            TestHelper.NativeHandlerSupportsSslConfiguration();

 #endif

         private static bool CanTestCertificates =>

diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ClientCertificates.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ClientCertificates.cs

index 78d0fdb09f..217726db64 100644

--- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ClientCertificates.cs

+++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ClientCertificates.cs

@@ -319,19 +319,7 @@ namespace System.Net.Http.Functional.Tests

 #if TargetsWindows

                 return true;

 #else

-                if (UseSocketsHttpHandler)

-                {

-                    return true;

-                }

-

-                if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))

-                {

-                    return false;

-                }

-

-                // For other Unix-based systems it's true if (and only if) the openssl backend

-                // is used with libcurl.

-                return (Interop.Http.GetSslVersionDescription()?.StartsWith(Interop.Http.OpenSsl10Description, StringComparison.OrdinalIgnoreCase) ?? false);

+                return TestHelper.NativeHandlerSupportsSslConfiguration();

 #endif

             }

         }

diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ServerCertificates.Unix.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ServerCertificates.Unix.cs

index d19a63f598..eb38f93615 100644

--- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ServerCertificates.Unix.cs

+++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.ServerCertificates.Unix.cs

@@ -58,14 +58,7 @@ namespace System.Net.Http.Functional.Tests

                     return true;

                 }

-                if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))

-                {

-                    return false;

-                }

-

-                // For other Unix-based systems it's true if (and only if) the openssl backend

-                // is used with libcurl.

-                return (Interop.Http.GetSslVersionDescription()?.StartsWith(Interop.Http.OpenSsl10Description, StringComparison.OrdinalIgnoreCase) ?? false);

+                return TestHelper.NativeHandlerSupportsSslConfiguration();

             }

         }

diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.SslProtocols.Unix.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.SslProtocols.Unix.cs

index 615f2cb4fa..e7631e3940 100644

--- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.SslProtocols.Unix.cs

+++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.SslProtocols.Unix.cs

@@ -17,7 +17,6 @@ namespace System.Net.Http.Functional.Tests

     public abstract partial class HttpClientHandler_SslProtocols_Test

     {

         private bool BackendSupportsSslConfiguration =>

-            UseSocketsHttpHandler ||

-            (Interop.Http.GetSslVersionDescription()?.StartsWith(Interop.Http.OpenSsl10Description, StringComparison.OrdinalIgnoreCase) ?? false);

+            UseSocketsHttpHandler || TestHelper.NativeHandlerSupportsSslConfiguration();

     }

 }

diff --git a/src/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj b/src/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

index 68c87c2b6e..b3b9d2437d 100644

--- a/src/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

+++ b/src/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

@@ -5,6 +5,7 @@

     <ProjectGuid>{C85CF035-7804-41FF-9557-48B7C948B58D}</ProjectGuid>

     <DefineConstants Condition="'$(TargetGroup)'=='netcoreapp'">$(DefineConstants);netcoreapp</DefineConstants>

     <DefineConstants Condition="'$(TargetsWindows)'=='true'">$(DefineConstants);TargetsWindows</DefineConstants>

+    <DefineConstants>$(DefineConstants);SYSNETHTTP_NO_OPENSSL</DefineConstants>

     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>

   </PropertyGroup>

   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netcoreapp-Unix-Debug|AnyCPU'" />

@@ -169,4 +170,4 @@

     <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />

   </ItemGroup>

   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />

-</Project>

\ No newline at end of file

+</Project>

diff --git a/src/System.Net.Http/tests/FunctionalTests/TestHelper.cs b/src/System.Net.Http/tests/FunctionalTests/TestHelper.cs

index 9bf6f216d1..2e29128a92 100644

--- a/src/System.Net.Http/tests/FunctionalTests/TestHelper.cs

+++ b/src/System.Net.Http/tests/FunctionalTests/TestHelper.cs

@@ -6,6 +6,8 @@ using System.Collections.Generic;

 using System.Linq;

 using System.Net.NetworkInformation;

 using System.Net.Security;

+using System.Reflection;

+using System.Runtime.InteropServices;

 using System.Security.Cryptography;

 using System.Security.Cryptography.X509Certificates;

 using System.Text;

@@ -107,5 +109,72 @@ namespace System.Net.Http.Functional.Tests

                 .Select(a => a.Address)

                 .Where(a => a.IsIPv6LinkLocal)

                 .FirstOrDefault();

+

+        public static void EnsureHttp2Feature(HttpClientHandler handler)

+        {

+            // All .NET Core implementations of HttpClientHandler have HTTP/2 enabled by default except when using

+            // SocketsHttpHandler. Right now, the HTTP/2 feature is disabled on SocketsHttpHandler unless certain

+            // AppContext switches or environment variables are set. To help with testing, we can enable the HTTP/2

+            // feature for a specific handler instance by using reflection.

+            FieldInfo field_socketsHttpHandler = typeof(HttpClientHandler).GetField(

+                "_socketsHttpHandler",

+                BindingFlags.NonPublic | BindingFlags.Instance);

+            if (field_socketsHttpHandler == null)

+            {

+                // Not using .NET Core implementation, i.e. could be .NET Framework or UAP.

+                return;

+            }

+

+            object _socketsHttpHandler = field_socketsHttpHandler.GetValue(handler);

+            if (_socketsHttpHandler == null)

+            {

+                // Not using SocketsHttpHandler, i.e. using WinHttpHandler or CurlHandler.

+                return;

+            }

+

+            // Get HttpConnectionSettings object from SocketsHttpHandler.

+            Type type_SocketsHttpHandler = typeof(HttpClientHandler).Assembly.GetType("System.Net.Http.SocketsHttpHandler");

+            FieldInfo field_settings = type_SocketsHttpHandler.GetField(

+                "_settings",

+                BindingFlags.NonPublic | BindingFlags.Instance);

+            Assert.NotNull(field_settings);

+            object _settings = field_settings.GetValue(_socketsHttpHandler);

+            Assert.NotNull(_settings);

+

+            // Set _maxHttpVersion field to HTTP/2.0.

+            Type type_HttpConnectionSettings = typeof(HttpClientHandler).Assembly.GetType("System.Net.Http.HttpConnectionSettings");

+            FieldInfo field_maxHttpVersion = type_HttpConnectionSettings.GetField(

+                "_maxHttpVersion",

+                BindingFlags.NonPublic | BindingFlags.Instance);

+            field_maxHttpVersion.SetValue(_settings, new Version(2, 0));

+        }

+

+        public static bool NativeHandlerSupportsSslConfiguration()

+        {

+#if TargetsWindows

+            return true;

+#else

+            if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))

+            {

+                return false;

+            }

+

+            // For other Unix-based systems it's true if (and only if) the currect openssl backend

+            // is used with libcurl.

+            bool hasAnyOpenSsl =

+                Interop.Http.GetSslVersionDescription()?.StartsWith(Interop.Http.OpenSslDescriptionPrefix, StringComparison.OrdinalIgnoreCase) ?? false;

+

+            if (!hasAnyOpenSsl)

+            {

+                return false;

+            }

+

+            // We're on an OpenSSL-based system, with an OpenSSL backend.

+            // Ask the product how it feels about this.

+            Type interopHttp = typeof(HttpClient).Assembly.GetType("Interop+Http");

+            PropertyInfo hasMatchingOpenSslVersion = interopHttp.GetProperty("HasMatchingOpenSslVersion", BindingFlags.Static | BindingFlags.NonPublic);

+            return (bool)hasMatchingOpenSslVersion.GetValue(null);

+#endif

+        }

     }

 }

-- 

2.20.1