XS✔ ◾ Restore TLS certificate validation for health-check clients

src/Diagnostics.HealthChecks/HealthCheckHttpClientServiceCollectionExtensions.cs

@@ -14,19 +14,13 @@ public static class HealthCheckHttpClientServiceCollectionExtensions
 {
 	/// <summary>
 	/// Defines a <seealso cref="HttpClient"/> 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.
+	/// Uses platform certificate validation in all environments. HTTPS endpoints must present
+	/// trusted certificates that match the configured host.
 	/// </summary>
 	/// <param name="services">The services.</param>
 	/// <param name="name">The logical name of the <see cref="HttpClient"/> to configure.</param>
 	/// <returns>The <see cref="IHttpClientBuilder"/>.</returns>
 	public static IHttpClientBuilder AddHealthCheckHttpClient(this IServiceCollection services, string name) =>
 		services.AddHttpClient(name)
-			.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler
-			{
-				ServerCertificateCustomValidationCallback = (_, _, _, _) => true
-			});
+			.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler());
 }

src/Diagnostics.HealthChecks/ServiceCollectionExtensions.cs

@@ -31,8 +31,7 @@ private static IServiceCollection AddPublisherDependencies(this IServiceCollecti
 				.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler
 				{
 					AllowAutoRedirect = false,
-					Credentials = CredentialCache.DefaultCredentials,
-					ServerCertificateCustomValidationCallback = (sender, x509Certificate, chain, errors) => true
+					Credentials = CredentialCache.DefaultCredentials
 				});
 
 			serviceCollection.TryAddSingleton<ObjectPoolProvider, DefaultObjectPoolProvider>();

tests/Diagnostics.HealthChecks.UnitTests/ServiceCollectionExtensions.cs

@@ -5,6 +5,8 @@
 using System.Collections.Generic;
 using System.Fabric;
 using System.Linq;
+using System.Net;
+using System.Net.Http;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
@@ -20,6 +22,50 @@ namespace Microsoft.Omex.Extensions.Diagnostics.HealthChecks.UnitTests
 	[TestClass]
 	public class ServiceCollectionExtensions
 	{
+		[TestMethod]
+		public void AddHealthCheckHttpClient_UsesDefaultCertificateValidation()
+		{
+			const string clientName = "HealthCheck";
+			ServiceCollection services = new();
+
+			IHttpClientBuilder builder = services.AddHealthCheckHttpClient(clientName);
+			using ServiceProvider provider = services.BuildServiceProvider();
+
+			Assert.AreEqual(clientName, builder.Name);
+			HttpClientHandler handler = GetPrimaryHttpClientHandler(provider, clientName);
+			Assert.IsNull(handler.ServerCertificateCustomValidationCallback,
+				"Health-check clients must use platform certificate validation in every environment.");
+		}
+
+		[TestMethod]
+		public void AddServiceFabricHealthChecks_UsesDefaultCertificateValidationAndPreservesClientSettings()
+		{
+			using ServiceProvider provider = new ServiceCollection()
+				.AddServiceFabricHealthChecks()
+				.Services
+				.BuildServiceProvider();
+
+			HttpClientHandler handler = GetPrimaryHttpClientHandler(provider, HealthCheckConstants.HttpClientLogicalName);
+
+			Assert.IsNull(handler.ServerCertificateCustomValidationCallback,
+				"The legacy health-check client must use platform certificate validation.");
+			Assert.IsFalse(handler.AllowAutoRedirect);
+			Assert.AreSame(CredentialCache.DefaultCredentials, handler.Credentials);
+		}
+
+		private static HttpClientHandler GetPrimaryHttpClientHandler(IServiceProvider provider, string clientName)
+		{
+			HttpMessageHandler handler = provider.GetRequiredService<IHttpMessageHandlerFactory>().CreateHandler(clientName);
+			while (handler is DelegatingHandler delegatingHandler)
+			{
+				NullableAssert.IsNotNull(delegatingHandler.InnerHandler);
+				handler = delegatingHandler.InnerHandler;
+			}
+
+			Assert.IsInstanceOfType(handler, typeof(HttpClientHandler));
+			return (HttpClientHandler)handler;
+		}
+
 		[TestMethod]
 		public void AddHealthCheckPublisher_RegisterPublisherWithoutOverride()
 		{