Skip to content

Commit

Permalink
Prevent server certificate callback runtime exception (#2213)
Browse files Browse the repository at this point in the history
* Prevent server certificate callback runtime exception

On .NET Full framework <4.7.2, HttpClientHandler.ServerCertificateCustomValidationCallback is not available. Due to dependency binding issues introduced with netstandard,
the incorrect System.Net.Http package may be resolved and lead to exceptions at runtime.

This fix prevents calling this API on net462 targets which includes any .NET Framework version < .NET 4.7.2.

This introduces the net472 target so that we can resume using this API from that version, where it was included in the box.

* Update documentation

* Tweak docs
  • Loading branch information
stevejgordon authored Nov 10, 2023
1 parent a352170 commit 7933659
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 19 deletions.
12 changes: 12 additions & 0 deletions docs/configuration.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,12 @@ Verification can be disabled by changing this setting to false.
| `true` | Boolean
|============

[NOTE]
====
This configuration setting has no effect on .NET Framework versions 4.6.2-4.7.1.
We recommend upgrading to .NET Framework 4.7.2 or newer to use this configuration setting.
====

[float]
[[config-server-cert]]
==== `ServerCert` (added[1.9])
Expand All @@ -726,6 +732,12 @@ the trust store, such as a self-signed certificate.
| `<none>` | String
|============

[NOTE]
====
This configuration setting has no effect on .NET Framework versions 4.6.2-4.7.1.
We recommend upgrading to .NET Framework 4.7.2 or newer to use this configuration setting.
====

[float]
[[config-flush-interval]]
==== `FlushInterval` (added[1.1])
Expand Down
4 changes: 3 additions & 1 deletion docs/setup-auto-instrumentation.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ This approach works with the following

|x64

|.NET Framework 4.6.2+
|.NET Framework 4.6.2+*

.NET Core 2.1+

Expand All @@ -31,6 +31,8 @@ This approach works with the following
.NET 5+
|===

_* Due to binding issues introduced by Microsoft, we recommend at least .NET Framework 4.7.2 for best compatibility._

NOTE: The Profiler based agent only supports 64-bit applications. 32-bit applications aren't supported.

It instruments the following assemblies:
Expand Down
10 changes: 9 additions & 1 deletion docs/supported-technologies.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,15 @@ the agent will not capture transactions.
=== .NET versions

The agent works on every .NET flavor and version that supports .NET Standard 2.0.
This means .NET Core 2.0 or newer, and .NET Framework 4.6.2 or newer.
This means .NET Core 2.0 or newer, and .NET Framework 4.6.2* or newer.

_* Due to binding issues introduced by Microsoft, we recommend at least .NET Framework 4.7.2 for best compatibility._

[IMPORTANT]
====
While this library *should* work on .NET Core 2.0+, we limit our support to only those
versions currently supported by Microsoft - .NET 6.0 and newer.
====

[float]
[[supported-web-frameworks]]
Expand Down
44 changes: 34 additions & 10 deletions src/Elastic.Apm/BackendComm/BackendCommUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Net.Security;
using System.Reflection;
#if NETSTANDARD2_0 || NET6_0_OR_GREATER
#if !NET462
using System.Security.Authentication;
#endif
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
#endif
using System.Text;
using System.Text.RegularExpressions;
using Elastic.Apm.Api;
Expand Down Expand Up @@ -126,8 +126,21 @@ private static void ConfigServicePoint(Uri serverUrlBase, IApmLogger logger) =>

private static HttpClientHandler CreateHttpClientHandler(IConfiguration configuration, IApmLogger logger)
{
#if NET462
try
{
var systemNetHttpVersion = typeof(HttpClientHandler).Assembly.GetName().Version;
logger.Trace()?.Log("System.Net.Http assembly version if {Version}.", systemNetHttpVersion);
}
catch (Exception ex)
{
logger.Error()?.LogException(ex, "Could not determine the assembly version of System.Net.Http.");
}
#endif

#if !NET462
Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> serverCertificateCustomValidationCallback = null;
var useWindowsCredentials = configuration.UseWindowsCredentials;

if (!configuration.VerifyServerCert)
{
serverCertificateCustomValidationCallback = (_, _, _, policyError) =>
Expand Down Expand Up @@ -192,18 +205,29 @@ private static HttpClientHandler CreateHttpClientHandler(IConfiguration configur
return false;
};
}
#endif

var httpClientHandler = new HttpClientHandler
{
ServerCertificateCustomValidationCallback = serverCertificateCustomValidationCallback,
UseDefaultCredentials = useWindowsCredentials
UseDefaultCredentials = configuration.UseWindowsCredentials
};
#if NETFRAMEWORK
ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12;
logger.Info()?.Log($"CreateHttpClientHandler - SslProtocols: {ServicePointManager.SecurityProtocol}");
#else

// Due to the potential for binding issues (e.g.https://github.com/dotnet/runtime/issues/29314)
// and runtime exceptions on .NET Framework versions <4.7.2, we don't attempt to set the certificate
// validation callback or SSL protocols on net462, which should also apply to .NET Framework <4.7.2 runtimes
// which resolve to that target.
#if !NET462
httpClientHandler.ServerCertificateCustomValidationCallback = serverCertificateCustomValidationCallback;
logger.Info()?.Log("CreateHttpClientHandler - Setting ServerCertificateCustomValidationCallback");

httpClientHandler.SslProtocols |= SslProtocols.Tls12;
logger.Info()?.Log($"CreateHttpClientHandler - SslProtocols: {httpClientHandler.SslProtocols}");
#else
// We don't set the ServerCertificateCustomValidationCallback on ServicePointManager here as it would
// apply to the whole AppDomain and that may not be desired. A consumer can set this themselves if they
// need custom validation behaviour.
ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12;
logger.Info()?.Log($"CreateHttpClientHandler - SslProtocols: {ServicePointManager.SecurityProtocol}");
#endif
return httpClientHandler;
}
Expand Down
10 changes: 8 additions & 2 deletions src/Elastic.Apm/Elastic.Apm.csproj
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>netstandard2.0;net462;net6.0</TargetFrameworks>
<TargetFrameworks>netstandard2.0;net462;net472;net6.0</TargetFrameworks>
<IsPackable>true</IsPackable>
</PropertyGroup>
<PropertyGroup>
Expand Down Expand Up @@ -67,6 +67,11 @@
<Reference Include="System.Configuration" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)'=='net472'">
<Reference Include="System.Net.Http" />
<Reference Include="System.Configuration" />
</ItemGroup>

<ItemGroup Condition="'$(DiagnosticSourceVersion)' == ''">
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="5.0.0" />
</ItemGroup>
Expand All @@ -83,6 +88,7 @@
<ItemGroup Condition="'$(TargetFramework)' != 'netstandard2.0'">
<PackageReference Include="System.Diagnostics.PerformanceCounter" Version="6.0.1" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Threading.Tasks.Dataflow" Version="4.9.0" />
<!-- Used by Ben.Demystifier -->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region License
#region License

// Copyright (c) 2007 James Newton-King
//
Expand Down Expand Up @@ -1259,7 +1259,7 @@ protected virtual IValueProvider CreateMemberValueProvider(MemberInfo member)
// warning - this method use to cause errors with Intellitrace. Retest in VS Ultimate after changes
IValueProvider valueProvider;

#if !(PORTABLE40 || PORTABLE || DOTNET || NETSTANDARD2_0 || NET5_0_OR_GREATER)
#if !(PORTABLE40 || PORTABLE || DOTNET || NET472 || NETSTANDARD2_0 || NET5_0_OR_GREATER)
if (DynamicCodeGeneration)
{
valueProvider = new DynamicValueProvider(member);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region License
#region License

// Copyright (c) 2007 James Newton-King
//
Expand Down Expand Up @@ -468,7 +468,7 @@ public static ReflectionDelegateFactory ReflectionDelegateFactory
{
get
{
#if !(PORTABLE40 || PORTABLE || DOTNET || NETSTANDARD2_0 || NET5_0_OR_GREATER)
#if !(PORTABLE40 || PORTABLE || DOTNET || NET472 || NETSTANDARD2_0 || NET5_0_OR_GREATER)
if (DynamicCodeGeneration)
{
return DynamicReflectionDelegateFactory.Instance;
Expand Down
14 changes: 14 additions & 0 deletions test/Elastic.Apm.Tests.Utilities/XUnit/DisabledOnWindowsFact.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace Elastic.Apm.Tests.Utilities.XUnit;

#pragma warning disable IDE0021 // Use expression body for constructor
public sealed class DisabledOnWindowsFact : FactAttribute
{
public DisabledOnWindowsFact()
Expand All @@ -22,11 +23,22 @@ public sealed class DisabledOnFullFrameworkFact : FactAttribute
public DisabledOnFullFrameworkFact()
{
#if NETFRAMEWORK

Skip = "This test is disabled on .NET Full Framework";
#endif
}
}

public sealed class DisabledOnNet462FrameworkFact : FactAttribute
{
public DisabledOnNet462FrameworkFact()
{
#if NET462
Skip = "This test is disabled on .NET Framework 4.6.2";
#endif
}
}

/// <summary>
/// May be applied to tests which depend on Linux Docker images which will not be
/// available when running on Windows images from GitHub actions.
Expand Down Expand Up @@ -58,3 +70,5 @@ public DisabledOnFullFrameworkTheory()
#endif
}
}

#pragma warning restore IDE0021 // Use expression body for constructor
5 changes: 4 additions & 1 deletion test/Elastic.Apm.Tests/ServerCertificateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Elastic.Apm.Logging;
using Elastic.Apm.Tests.MockApmServer;
using Elastic.Apm.Tests.Utilities;
using Elastic.Apm.Tests.Utilities.XUnit;
using FluentAssertions;
using Xunit;

Expand Down Expand Up @@ -72,7 +73,9 @@ public void ServerCert_Should_Allow_Https_To_Apm_Server()
transaction.Context.Labels.MergedDictionary.Should().ContainKey("self_signed_cert");
}

[Fact]
// We don't support certificate validation on net462 due to .NET Framework binding issues with
// the APIs used.
[DisabledOnNet462FrameworkFact]
public void VerifyServerCert_Should_Allow_Https_To_Apm_Server()
{
var configuration = new MockConfiguration(
Expand Down

0 comments on commit 7933659

Please sign in to comment.