Fixed: Broken SSL certificate validation option

(cherry picked from commit a6a761cb32a268c4447071dc692c428789063fce)

src/NzbDrone.Common.Test/Http/HttpClientFixture.cs

@@ -4,6 +4,7 @@ using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Net;
+using System.Net.Http;
 using System.Threading;
 using FluentAssertions;
 using Moq;
@@ -15,6 +16,8 @@ using NzbDrone.Common.Http;
 using NzbDrone.Common.Http.Dispatchers;
 using NzbDrone.Common.Http.Proxy;
 using NzbDrone.Common.TPL;
+using NzbDrone.Core.Configuration;
+using NzbDrone.Core.Security;
 using NzbDrone.Test.Common;
 using NzbDrone.Test.Common.Categories;
 using HttpClient = NzbDrone.Common.Http.HttpClient;
@@ -41,7 +44,7 @@ namespace NzbDrone.Common.Test.Http
             var mainHost = "httpbin.servarr.com";
 
             // Use mirrors for tests that use two hosts
-            var candidates = new[] { "eu.httpbin.org", /* "httpbin.org", */ "www.httpbin.org" };
+            var candidates = new[] { "httpbin1.servarr.com" };
 
             // httpbin.org is broken right now, occassionally redirecting to https if it's unavailable.
             _httpBinHost = mainHost;
@@ -49,7 +52,7 @@ namespace NzbDrone.Common.Test.Http
 
             TestLogger.Info($"{candidates.Length} TestSites available.");
 
-            _httpBinSleep = _httpBinHosts.Count() < 2 ? 100 : 10;
+            _httpBinSleep = 10;
         }
 
         private bool IsTestSiteAvailable(string site)
@@ -84,10 +87,13 @@ namespace NzbDrone.Common.Test.Http
             Mocker.GetMock<IOsInfo>().Setup(c => c.Name).Returns("TestOS");
             Mocker.GetMock<IOsInfo>().Setup(c => c.Version).Returns("9.0.0");
 
+            Mocker.GetMock<IConfigService>().SetupGet(x => x.CertificateValidation).Returns(CertificateValidationType.Enabled);
+
             Mocker.SetConstant<IUserAgentBuilder>(Mocker.Resolve<UserAgentBuilder>());
 
             Mocker.SetConstant<ICacheManager>(Mocker.Resolve<CacheManager>());
             Mocker.SetConstant<ICreateManagedWebProxy>(Mocker.Resolve<ManagedWebProxyFactory>());
+            Mocker.SetConstant<ICertificateValidationService>(new X509CertificateValidationService(Mocker.GetMock<IConfigService>().Object, TestLogger));
             Mocker.SetConstant<IRateLimitService>(Mocker.Resolve<RateLimitService>());
             Mocker.SetConstant<IEnumerable<IHttpRequestInterceptor>>(new IHttpRequestInterceptor[0]);
             Mocker.SetConstant<IHttpDispatcher>(Mocker.Resolve<TDispatcher>());
@@ -127,6 +133,28 @@ namespace NzbDrone.Common.Test.Http
             response.Content.Should().NotBeNullOrWhiteSpace();
         }
 
+        [TestCase(CertificateValidationType.Enabled)]
+        [TestCase(CertificateValidationType.DisabledForLocalAddresses)]
+        public void bad_ssl_should_fail_when_remote_validation_enabled(CertificateValidationType validationType)
+        {
+            Mocker.GetMock<IConfigService>().SetupGet(x => x.CertificateValidation).Returns(validationType);
+            var request = new HttpRequest($"https://expired.badssl.com");
+
+            Assert.Throws<HttpRequestException>(() => Subject.Execute(request));
+            ExceptionVerification.ExpectedErrors(2);
+        }
+
+        [Test]
+        public void bad_ssl_should_pass_if_remote_validation_disabled()
+        {
+            Mocker.GetMock<IConfigService>().SetupGet(x => x.CertificateValidation).Returns(CertificateValidationType.Disabled);
+
+            var request = new HttpRequest($"https://expired.badssl.com");
+
+            Subject.Execute(request);
+            ExceptionVerification.ExpectedErrors(0);
+        }
+
         [Test]
         public void should_execute_typed_get()
         {

src/NzbDrone.Common/Http/Dispatchers/ICertificateValidationService.cs

@@ -0,0 +1,11 @@
+using System.Net.Http;
+using System.Net.Security;
+using System.Security.Cryptography.X509Certificates;
+
+namespace NzbDrone.Common.Http.Dispatchers
+{
+    public interface ICertificateValidationService
+    {
+        bool ShouldByPassValidationError(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors);
+    }
+}

src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs

@@ -3,6 +3,7 @@ using System.IO;
 using System.Net;
 using System.Net.Http;
 using System.Net.Http.Headers;
+using System.Net.Security;
 using System.Net.Sockets;
 using System.Text;
 using System.Threading;
@@ -24,6 +25,7 @@ namespace NzbDrone.Common.Http.Dispatchers
 
         private readonly IHttpProxySettingsProvider _proxySettingsProvider;
         private readonly ICreateManagedWebProxy _createManagedWebProxy;
+        private readonly ICertificateValidationService _certificateValidationService;
         private readonly IUserAgentBuilder _userAgentBuilder;
         private readonly ICached<System.Net.Http.HttpClient> _httpClientCache;
         private readonly ICached<CredentialCache> _credentialCache;
@@ -31,12 +33,14 @@ namespace NzbDrone.Common.Http.Dispatchers
 
         public ManagedHttpDispatcher(IHttpProxySettingsProvider proxySettingsProvider,
             ICreateManagedWebProxy createManagedWebProxy,
+            ICertificateValidationService certificateValidationService,
             IUserAgentBuilder userAgentBuilder,
             ICacheManager cacheManager,
             Logger logger)
         {
             _proxySettingsProvider = proxySettingsProvider;
             _createManagedWebProxy = createManagedWebProxy;
+            _certificateValidationService = certificateValidationService;
             _userAgentBuilder = userAgentBuilder;
             _logger = logger;
 
@@ -158,7 +162,12 @@ namespace NzbDrone.Common.Http.Dispatchers
                 AllowAutoRedirect = false,
                 Credentials = GetCredentialCache(),
                 PreAuthenticate = true,
+                MaxConnectionsPerServer = 12,
                 ConnectCallback = onConnect,
+                SslOptions = new SslClientAuthenticationOptions
+                {
+                    RemoteCertificateValidationCallback = _certificateValidationService.ShouldByPassValidationError
+                }
             };
 
             if (proxySettings != null)

src/NzbDrone.Core.Test/Framework/CoreTest.cs

@@ -11,6 +11,7 @@ using NzbDrone.Common.TPL;
 using NzbDrone.Core.Configuration;
 using NzbDrone.Core.Http;
 using NzbDrone.Core.MetadataSource;
+using NzbDrone.Core.Security;
 using NzbDrone.Test.Common;
 
 namespace NzbDrone.Core.Test.Framework
@@ -25,7 +26,8 @@ namespace NzbDrone.Core.Test.Framework
 
             Mocker.SetConstant<IHttpProxySettingsProvider>(new HttpProxySettingsProvider(Mocker.Resolve<ConfigService>()));
             Mocker.SetConstant<ICreateManagedWebProxy>(new ManagedWebProxyFactory(Mocker.Resolve<CacheManager>()));
-            Mocker.SetConstant<IHttpDispatcher>(new ManagedHttpDispatcher(Mocker.Resolve<IHttpProxySettingsProvider>(), Mocker.Resolve<ICreateManagedWebProxy>(), Mocker.Resolve<UserAgentBuilder>(), Mocker.Resolve<CacheManager>(), TestLogger));
+            Mocker.SetConstant<ICertificateValidationService>(new X509CertificateValidationService(Mocker.Resolve<ConfigService>(), TestLogger));
+            Mocker.SetConstant<IHttpDispatcher>(new ManagedHttpDispatcher(Mocker.Resolve<IHttpProxySettingsProvider>(), Mocker.Resolve<ICreateManagedWebProxy>(), Mocker.Resolve<ICertificateValidationService>(), Mocker.Resolve<UserAgentBuilder>(), Mocker.Resolve<CacheManager>(), TestLogger));
             Mocker.SetConstant<IHttpClient>(new HttpClient(new IHttpRequestInterceptor[0], Mocker.Resolve<CacheManager>(), Mocker.Resolve<RateLimitService>(), Mocker.Resolve<IHttpDispatcher>(), TestLogger));
             Mocker.SetConstant<IReadarrCloudRequestBuilder>(new ReadarrCloudRequestBuilder());
             Mocker.SetConstant<IMetadataRequestBuilder>(Mocker.Resolve<MetadataRequestBuilder>());

src/NzbDrone.Core/Security/X509CertificateValidationService.cs

@@ -4,13 +4,12 @@ using System.Net.Security;
 using System.Security.Cryptography.X509Certificates;
 using NLog;
 using NzbDrone.Common.Extensions;
+using NzbDrone.Common.Http.Dispatchers;
 using NzbDrone.Core.Configuration;
-using NzbDrone.Core.Lifecycle;
-using NzbDrone.Core.Messaging.Events;
 
 namespace NzbDrone.Core.Security
 {
-    public class X509CertificateValidationService : IHandle<ApplicationStartedEvent>
+    public class X509CertificateValidationService : ICertificateValidationService
     {
         private readonly IConfigService _configService;
         private readonly Logger _logger;
@@ -21,19 +20,16 @@ namespace NzbDrone.Core.Security
             _logger = logger;
         }
 
-        private bool ShouldByPassValidationError(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
+        public bool ShouldByPassValidationError(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
         {
-            var request = sender as HttpWebRequest;
+            if (sender is not SslStream request)
-
-            if (request == null)
             {
                 return true;
             }
 
-            var cert2 = certificate as X509Certificate2;
+            if (certificate is X509Certificate2 cert2 && cert2.SignatureAlgorithm.FriendlyName == "md5RSA")
-            if (cert2 != null && request != null && cert2.SignatureAlgorithm.FriendlyName == "md5RSA")
             {
-                _logger.Error("https://{0} uses the obsolete md5 hash in it's https certificate, if that is your certificate, please (re)create certificate with better algorithm as soon as possible.", request.RequestUri.Authority);
+                _logger.Error("https://{0} uses the obsolete md5 hash in it's https certificate, if that is your certificate, please (re)create certificate with better algorithm as soon as possible.", request.TargetHostName);
             }
 
             if (sslPolicyErrors == SslPolicyErrors.None)
@@ -41,12 +37,12 @@ namespace NzbDrone.Core.Security
                 return true;
             }
 
-            if (request.RequestUri.Host == "localhost" || request.RequestUri.Host == "127.0.0.1")
+            if (request.TargetHostName == "localhost" || request.TargetHostName == "127.0.0.1")
             {
                 return true;
             }
 
-            var ipAddresses = GetIPAddresses(request.RequestUri.Host);
+            var ipAddresses = GetIPAddresses(request.TargetHostName);
             var certificateValidation = _configService.CertificateValidation;
 
             if (certificateValidation == CertificateValidationType.Disabled)
@@ -60,7 +56,7 @@ namespace NzbDrone.Core.Security
                 return true;
             }
 
-            _logger.Error("Certificate validation for {0} failed. {1}", request.Address, sslPolicyErrors);
+            _logger.Error("Certificate validation for {0} failed. {1}", request.TargetHostName, sslPolicyErrors);
 
             return false;
         }
@@ -74,10 +70,5 @@ namespace NzbDrone.Core.Security
 
             return Dns.GetHostEntry(host).AddressList;
         }
-
-        public void Handle(ApplicationStartedEvent message)
-        {
-            ServicePointManager.ServerCertificateValidationCallback = ShouldByPassValidationError;
-        }
     }
 }