From dfb95588687b97b6a44bee8427939a577d17e0b9 Mon Sep 17 00:00:00 2001 From: ta264 Date: Thu, 18 Nov 2021 21:19:49 +0000 Subject: [PATCH] Fixed: Broken SSL certificate validation option (cherry picked from commit a6a761cb32a268c4447071dc692c428789063fce) --- .../Http/HttpClientFixture.cs | 32 +++++++++++++++++-- .../ICertificateValidationService.cs | 11 +++++++ .../Http/Dispatchers/ManagedHttpDispatcher.cs | 9 ++++++ src/NzbDrone.Core.Test/Framework/CoreTest.cs | 4 ++- .../X509CertificateValidationService.cs | 27 ++++++---------- 5 files changed, 62 insertions(+), 21 deletions(-) create mode 100644 src/NzbDrone.Common/Http/Dispatchers/ICertificateValidationService.cs diff --git a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs index c9ec92ec4..2240966a7 100644 --- a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs +++ b/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().Setup(c => c.Name).Returns("TestOS"); Mocker.GetMock().Setup(c => c.Version).Returns("9.0.0"); + Mocker.GetMock().SetupGet(x => x.CertificateValidation).Returns(CertificateValidationType.Enabled); + Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant(Mocker.Resolve()); + Mocker.SetConstant(new X509CertificateValidationService(Mocker.GetMock().Object, TestLogger)); Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant>(new IHttpRequestInterceptor[0]); Mocker.SetConstant(Mocker.Resolve()); @@ -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().SetupGet(x => x.CertificateValidation).Returns(validationType); + var request = new HttpRequest($"https://expired.badssl.com"); + + Assert.Throws(() => Subject.Execute(request)); + ExceptionVerification.ExpectedErrors(2); + } + + [Test] + public void bad_ssl_should_pass_if_remote_validation_disabled() + { + Mocker.GetMock().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() { diff --git a/src/NzbDrone.Common/Http/Dispatchers/ICertificateValidationService.cs b/src/NzbDrone.Common/Http/Dispatchers/ICertificateValidationService.cs new file mode 100644 index 000000000..187c1fd43 --- /dev/null +++ b/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); + } +} diff --git a/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs b/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs index 08b464b79..b7acefb3a 100644 --- a/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs +++ b/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 _httpClientCache; private readonly ICached _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) diff --git a/src/NzbDrone.Core.Test/Framework/CoreTest.cs b/src/NzbDrone.Core.Test/Framework/CoreTest.cs index ebb0611b5..4b4e990fc 100644 --- a/src/NzbDrone.Core.Test/Framework/CoreTest.cs +++ b/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(new HttpProxySettingsProvider(Mocker.Resolve())); Mocker.SetConstant(new ManagedWebProxyFactory(Mocker.Resolve())); - Mocker.SetConstant(new ManagedHttpDispatcher(Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), TestLogger)); + Mocker.SetConstant(new X509CertificateValidationService(Mocker.Resolve(), TestLogger)); + Mocker.SetConstant(new ManagedHttpDispatcher(Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), TestLogger)); Mocker.SetConstant(new HttpClient(new IHttpRequestInterceptor[0], Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), TestLogger)); Mocker.SetConstant(new ReadarrCloudRequestBuilder()); Mocker.SetConstant(Mocker.Resolve()); diff --git a/src/NzbDrone.Core/Security/X509CertificateValidationService.cs b/src/NzbDrone.Core/Security/X509CertificateValidationService.cs index ba58e1eff..0c03388b5 100644 --- a/src/NzbDrone.Core/Security/X509CertificateValidationService.cs +++ b/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 + 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 (request == null) + if (sender is not SslStream request) { return true; } - var cert2 = certificate as X509Certificate2; - if (cert2 != null && request != null && cert2.SignatureAlgorithm.FriendlyName == "md5RSA") + if (certificate is X509Certificate2 cert2 && 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; - } } }