Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent server certificate callback runtime exception #2213

Merged
merged 3 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
stevejgordon marked this conversation as resolved.
Show resolved Hide resolved
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
Loading