diff --git a/src/Diagnostics.HealthChecks/HealthCheckHttpClientServiceCollectionExtensions.cs b/src/Diagnostics.HealthChecks/HealthCheckHttpClientServiceCollectionExtensions.cs
index a6c8195f8..44b1fe8e6 100644
--- a/src/Diagnostics.HealthChecks/HealthCheckHttpClientServiceCollectionExtensions.cs
+++ b/src/Diagnostics.HealthChecks/HealthCheckHttpClientServiceCollectionExtensions.cs
@@ -1,8 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.
+using System;
using System.Net.Http;
+using System.Net.Security;
using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
namespace Microsoft.Omex.Extensions.Diagnostics.HealthChecks;
@@ -14,19 +17,74 @@ public static class HealthCheckHttpClientServiceCollectionExtensions
{
///
/// Defines a in the DI designed for Health Checks.
- /// The main difference is that these clients in development will ignore SSL errors.
- /// This is an acceptable compromise considering that the health checks are executed in the same machine
- /// as the Service Fabric service itself, and that each client will be specifically used for the
- /// health checks, as the name parameter in input is mandatory.
- /// Every other uses of this extension methods apart from health checks should be avoided.
+ ///
+ /// Server certificate errors are ignored only for requests that target the local machine, meaning a
+ /// loopback host such as localhost, 127.0.0.1 or [::1], or a bare IP literal.
+ /// This is an acceptable compromise because health checks are executed on the same machine as the
+ /// Service Fabric service itself and are commonly addressed by node IP rather than by the subject
+ /// name present on the certificate, so validation cannot succeed by construction.
+ ///
+ ///
+ /// Requests that target a DNS name, meaning any external or FQDN addressed endpoint, are fully
+ /// validated. Use when the health check only ever
+ /// calls remote endpoints and the local relaxation is not wanted at all.
+ ///
+ ///
+ /// In the Development environment all certificate errors are ignored, including for DNS hosts, to
+ /// support self signed certificates on developer machines.
+ ///
+ ///
+ /// Every other use of this extension method apart from health checks should be avoided.
+ ///
///
/// The services.
/// The logical name of the to configure.
/// The .
public static IHttpClientBuilder AddHealthCheckHttpClient(this IServiceCollection services, string name) =>
+ services.AddHealthCheckHttpClient(name, allowLocalCertificateErrors: true);
+
+ ///
+ /// Defines a in the DI designed for Health Checks that always validates
+ /// server certificates, including for loopback and IP addressed endpoints.
+ ///
+ /// The only exception is the Development environment, where certificate errors are ignored to support
+ /// self signed certificates on developer machines.
+ ///
+ ///
+ /// Prefer this method when the health check targets an endpoint that presents a certificate valid for
+ /// the address being called.
+ ///
+ ///
+ /// The services.
+ /// The logical name of the to configure.
+ /// The .
+ public static IHttpClientBuilder AddValidatedHealthCheckHttpClient(this IServiceCollection services, string name) =>
+ services.AddHealthCheckHttpClient(name, allowLocalCertificateErrors: false);
+
+ private static IHttpClientBuilder AddHealthCheckHttpClient(this IServiceCollection services, string name, bool allowLocalCertificateErrors) =>
services.AddHttpClient(name)
- .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler
+ .ConfigurePrimaryHttpMessageHandler(provider =>
{
- ServerCertificateCustomValidationCallback = (_, _, _, _) => true
+ // If the environment cannot be resolved we keep the secure default of treating the
+ // application as non Development.
+ bool isDevelopment = provider.GetService()?.IsDevelopment() == true;
+
+ return new HttpClientHandler
+ {
+ ServerCertificateCustomValidationCallback = (request, _, _, errors) =>
+ errors == SslPolicyErrors.None
+ || isDevelopment
+ || (allowLocalCertificateErrors && IsLocalEndpoint(request.RequestUri))
+ };
});
+
+ ///
+ /// Determines whether a request targets the local machine, either through a loopback host or through
+ /// a bare IP literal, in which case a certificate issued for a DNS name cannot match the address used.
+ ///
+ private static bool IsLocalEndpoint(Uri? requestUri) =>
+ requestUri != null
+ && (requestUri.IsLoopback
+ || requestUri.HostNameType == UriHostNameType.IPv4
+ || requestUri.HostNameType == UriHostNameType.IPv6);
}
diff --git a/tests/Diagnostics.HealthChecks.UnitTests/HealthCheckHttpClientServiceCollectionExtensionsTests.cs b/tests/Diagnostics.HealthChecks.UnitTests/HealthCheckHttpClientServiceCollectionExtensionsTests.cs
new file mode 100644
index 000000000..89a19586f
--- /dev/null
+++ b/tests/Diagnostics.HealthChecks.UnitTests/HealthCheckHttpClientServiceCollectionExtensionsTests.cs
@@ -0,0 +1,156 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Net.Http;
+using System.Net.Security;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Http;
+using Microsoft.Extensions.Options;
+using Microsoft.Omex.Extensions.Testing.Helpers;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+
+namespace Microsoft.Omex.Extensions.Diagnostics.HealthChecks.UnitTests
+{
+ [TestClass]
+ public class HealthCheckHttpClientServiceCollectionExtensionsTests
+ {
+ private const string ClientName = "TestHealthCheckClient";
+
+ [TestMethod]
+ [DataRow("https://localhost:8080/health")]
+ [DataRow("https://127.0.0.1:8080/health")]
+ [DataRow("https://[::1]:8080/health")]
+ [DataRow("https://10.0.0.4:8080/health")]
+ [DataRow("https://[fd00::1]:8080/health")]
+ public void AddHealthCheckHttpClient_ForLocalOrIpEndpoint_IgnoresCertificateErrors(string url)
+ {
+ Assert.IsTrue(
+ Validate(CertificateCallback(Environments.Production, validated: false), url, SslPolicyErrors.RemoteCertificateNameMismatch),
+ FormattableString.Invariant($"Certificate errors should be ignored for local endpoint '{url}'"));
+ }
+
+ [TestMethod]
+ [DataRow("https://service.contoso.com/health")]
+ [DataRow("https://my-service.westus.cloudapp.azure.com/health")]
+ public void AddHealthCheckHttpClient_ForDnsEndpoint_ValidatesCertificate(string url)
+ {
+ Func callback = CertificateCallback(Environments.Production, validated: false);
+
+ Assert.IsFalse(
+ Validate(callback, url, SslPolicyErrors.RemoteCertificateNameMismatch),
+ FormattableString.Invariant($"Certificate errors should be rejected for DNS endpoint '{url}'"));
+
+ Assert.IsTrue(
+ Validate(callback, url, SslPolicyErrors.None),
+ FormattableString.Invariant($"A valid certificate should be accepted for DNS endpoint '{url}'"));
+ }
+
+ [TestMethod]
+ [DataRow("https://service.contoso.com/health")]
+ [DataRow("https://localhost:8080/health")]
+ public void AddHealthCheckHttpClient_InDevelopment_IgnoresCertificateErrors(string url)
+ {
+ Assert.IsTrue(
+ Validate(CertificateCallback(Environments.Development, validated: false), url, SslPolicyErrors.RemoteCertificateChainErrors),
+ FormattableString.Invariant($"Certificate errors should be ignored in Development for '{url}'"));
+ }
+
+ [TestMethod]
+ [DataRow("Production")]
+ [DataRow("Staging")]
+ [DataRow("EDog")]
+ [DataRow("Int")]
+ [DataRow(null)]
+ public void AddValidatedHealthCheckHttpClient_OutsideDevelopment_ValidatesLocalEndpoints(string? environmentName)
+ {
+ Func callback = CertificateCallback(environmentName, validated: true);
+
+ Assert.IsFalse(
+ Validate(callback, "https://localhost:8080/health", SslPolicyErrors.RemoteCertificateNameMismatch),
+ FormattableString.Invariant($"Local certificate errors should be rejected in '{environmentName ?? ""}'"));
+
+ Assert.IsFalse(
+ Validate(callback, "https://10.0.0.4:8080/health", SslPolicyErrors.RemoteCertificateNameMismatch),
+ FormattableString.Invariant($"IP certificate errors should be rejected in '{environmentName ?? ""}'"));
+
+ Assert.IsTrue(
+ Validate(callback, "https://localhost:8080/health", SslPolicyErrors.None),
+ "A valid certificate should always be accepted");
+ }
+
+ [TestMethod]
+ public void AddValidatedHealthCheckHttpClient_InDevelopment_IgnoresCertificateErrors() =>
+ Assert.IsTrue(
+ Validate(CertificateCallback(Environments.Development, validated: true), "https://localhost:8080/health", SslPolicyErrors.RemoteCertificateChainErrors),
+ "Certificate errors should be ignored in Development");
+
+ private static bool Validate(Func callback, string url, SslPolicyErrors errors)
+ {
+ using HttpRequestMessage request = new(HttpMethod.Get, url);
+ return callback(request, errors);
+ }
+
+ private static Func CertificateCallback(string? environmentName, bool validated)
+ {
+ ServiceCollection services = new();
+
+ if (environmentName != null)
+ {
+ Mock environmentMock = new();
+ environmentMock.SetupGet(e => e.EnvironmentName).Returns(environmentName);
+ services.AddSingleton(environmentMock.Object);
+ }
+
+ if (validated)
+ {
+ services.AddValidatedHealthCheckHttpClient(ClientName);
+ }
+ else
+ {
+ services.AddHealthCheckHttpClient(ClientName);
+ }
+
+ IServiceProvider provider = services.BuildServiceProvider();
+
+ HttpClientFactoryOptions options = provider
+ .GetRequiredService>()
+ .Get(ClientName);
+
+ TestHttpMessageHandlerBuilder builder = new(ClientName, provider);
+ foreach (Action action in options.HttpMessageHandlerBuilderActions)
+ {
+ action(builder);
+ }
+
+ Assert.IsInstanceOfType(builder.PrimaryHandler, typeof(HttpClientHandler));
+ HttpClientHandler handler = (HttpClientHandler)builder.PrimaryHandler;
+
+ NullableAssert.IsNotNull(handler.ServerCertificateCustomValidationCallback, "A validation callback should be configured");
+
+ return (request, errors) => handler.ServerCertificateCustomValidationCallback(request, null, null, errors);
+ }
+
+ private sealed class TestHttpMessageHandlerBuilder : HttpMessageHandlerBuilder
+ {
+ public TestHttpMessageHandlerBuilder(string name, IServiceProvider services)
+ {
+ Name = name;
+ Services = services;
+ }
+
+ public override string? Name { get; set; }
+
+ public override HttpMessageHandler PrimaryHandler { get; set; } = new HttpClientHandler();
+
+ public override IList AdditionalHandlers { get; } = new List();
+
+ public override IServiceProvider Services { get; }
+
+ public override HttpMessageHandler Build() => PrimaryHandler;
+ }
+ }
+}