From dfa0c242cd9b329554971cf80c094e8f58d756e3 Mon Sep 17 00:00:00 2001 From: Jeremy Barton 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 s_requiredOpenSslDescription = + new Lazy(() => DetermineRequiredOpenSslDescription()); + + private static readonly Lazy s_hasMatchingOpenSsl = + new Lazy(() => 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 @@ Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Crypto.cs + + Common\Interop\Unix\System.Security.Cryptography.Native\Interop.OpenSslVersion.cs + Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Ssl.cs 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 @@ {C85CF035-7804-41FF-9557-48B7C948B58D} $(DefineConstants);netcoreapp $(DefineConstants);TargetsWindows + $(DefineConstants);SYSNETHTTP_NO_OPENSSL true @@ -169,4 +170,4 @@ - \ No newline at end of file + 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