From 636d116491a3ae0a85bed9b91ed9041be6ae8549 Mon Sep 17 00:00:00 2001 From: Tom Pallister Date: Sun, 22 Apr 2018 12:05:49 +0100 Subject: [PATCH] #309 allow users to ignore ssl warnings, not sure this is advisable (#325) * #309 allow users to ignore ssl warnings, not sure this is advisable * #309 docs for ssl ignore --- docs/features/configuration.rst | 14 +- .../Builder/DownstreamReRouteBuilder.cs | 10 +- .../FileInternalConfigurationCreator.cs | 1 + src/Ocelot/Configuration/DownstreamReRoute.cs | 5 +- src/Ocelot/Configuration/File/FileReRoute.cs | 1 + src/Ocelot/Requester/HttpClientBuilder.cs | 25 +-- .../Ocelot.AcceptanceTests.csproj | 3 + test/Ocelot.AcceptanceTests/SslTests.cs | 146 ++++++++++++++++++ .../Requester/HttpClientBuilderTests.cs | 24 +++ 9 files changed, 217 insertions(+), 12 deletions(-) create mode 100644 test/Ocelot.AcceptanceTests/SslTests.cs diff --git a/docs/features/configuration.rst b/docs/features/configuration.rst index 4c96eade6..343d7cbe9 100644 --- a/docs/features/configuration.rst +++ b/docs/features/configuration.rst @@ -64,7 +64,8 @@ Here is an example ReRoute configuration, You don't need to set all of these thi "UseCookieContainer": true, "UseTracing": true }, - "UseServiceDiscovery": false + "UseServiceDiscovery": false, + "DangerousAcceptAnyServerCertificateValidator": false } More information on how to use these options is below.. @@ -160,3 +161,14 @@ noticed that the cookies were being shared. I tried to think of a nice way to ha that means each request gets a new client and therefore a new cookie container. If you clear the cookies from the cached client container you get race conditions due to inflight requests. This would also mean that subsequent requests dont use the cookies from the previous response! All in all not a great situation. I would avoid setting UseCookieContainer to true unless you have a really really good reason. Just look at your response headers and forward the cookies back with your next request! + +SSL Errors +---------- + +Id you want to ignore SSL warnings / errors set the following in your ReRoute config. + +.. code-block:: json + + "DangerousAcceptAnyServerCertificateValidator": false + +I don't reccomend doing this, I suggest creating your own certificate and then getting it trusted by your local / remote machine if you can. \ No newline at end of file diff --git a/src/Ocelot/Configuration/Builder/DownstreamReRouteBuilder.cs b/src/Ocelot/Configuration/Builder/DownstreamReRouteBuilder.cs index 49931421c..15fddb978 100644 --- a/src/Ocelot/Configuration/Builder/DownstreamReRouteBuilder.cs +++ b/src/Ocelot/Configuration/Builder/DownstreamReRouteBuilder.cs @@ -40,6 +40,7 @@ public class DownstreamReRouteBuilder private List _delegatingHandlers; private List _addHeadersToDownstream; private List _addHeadersToUpstream; + private bool _dangerousAcceptAnyServerCertificateValidator; public DownstreamReRouteBuilder() { @@ -241,6 +242,12 @@ public DownstreamReRouteBuilder WithAddHeadersToUpstream(List addHead return this; } + public DownstreamReRouteBuilder WithDangerousAcceptAnyServerCertificateValidator(bool dangerousAcceptAnyServerCertificateValidator) + { + _dangerousAcceptAnyServerCertificateValidator = dangerousAcceptAnyServerCertificateValidator; + return this; + } + public DownstreamReRoute Build() { return new DownstreamReRoute( @@ -272,7 +279,8 @@ public DownstreamReRoute Build() _reRouteKey, _delegatingHandlers, _addHeadersToDownstream, - _addHeadersToUpstream); + _addHeadersToUpstream, + _dangerousAcceptAnyServerCertificateValidator); } } } diff --git a/src/Ocelot/Configuration/Creator/FileInternalConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileInternalConfigurationCreator.cs index c14e4aab1..46eec9d68 100644 --- a/src/Ocelot/Configuration/Creator/FileInternalConfigurationCreator.cs +++ b/src/Ocelot/Configuration/Creator/FileInternalConfigurationCreator.cs @@ -213,6 +213,7 @@ private DownstreamReRoute SetUpDownstreamReRoute(FileReRoute fileReRoute, FileGl .WithDelegatingHandlers(fileReRoute.DelegatingHandlers) .WithAddHeadersToDownstream(hAndRs.AddHeadersToDownstream) .WithAddHeadersToUpstream(hAndRs.AddHeadersToUpstream) + .WithDangerousAcceptAnyServerCertificateValidator(fileReRoute.DangerousAcceptAnyServerCertificateValidator) .Build(); return reRoute; diff --git a/src/Ocelot/Configuration/DownstreamReRoute.cs b/src/Ocelot/Configuration/DownstreamReRoute.cs index 90e96ec53..4fe89f22e 100644 --- a/src/Ocelot/Configuration/DownstreamReRoute.cs +++ b/src/Ocelot/Configuration/DownstreamReRoute.cs @@ -35,8 +35,10 @@ public DownstreamReRoute( string reRouteKey, List delegatingHandlers, List addHeadersToDownstream, - List addHeadersToUpstream) + List addHeadersToUpstream, + bool dangerousAcceptAnyServerCertificateValidator) { + DangerousAcceptAnyServerCertificateValidator = dangerousAcceptAnyServerCertificateValidator; AddHeadersToDownstream = addHeadersToDownstream; DelegatingHandlers = delegatingHandlers; Key = key; @@ -97,5 +99,6 @@ public DownstreamReRoute( public List DelegatingHandlers {get;private set;} public List AddHeadersToDownstream {get;private set;} public List AddHeadersToUpstream { get; private set; } + public bool DangerousAcceptAnyServerCertificateValidator { get; private set; } } } diff --git a/src/Ocelot/Configuration/File/FileReRoute.cs b/src/Ocelot/Configuration/File/FileReRoute.cs index 5948c268b..acc6572ab 100644 --- a/src/Ocelot/Configuration/File/FileReRoute.cs +++ b/src/Ocelot/Configuration/File/FileReRoute.cs @@ -49,5 +49,6 @@ public FileReRoute() public List DelegatingHandlers {get;set;} public int Priority { get;set; } public int Timeout { get; set; } + public bool DangerousAcceptAnyServerCertificateValidator { get; set; } } } diff --git a/src/Ocelot/Requester/HttpClientBuilder.cs b/src/Ocelot/Requester/HttpClientBuilder.cs index fe80b11bf..fdc90ebf6 100644 --- a/src/Ocelot/Requester/HttpClientBuilder.cs +++ b/src/Ocelot/Requester/HttpClientBuilder.cs @@ -16,7 +16,6 @@ public class HttpClientBuilder : IHttpClientBuilder private string _cacheKey; private HttpClient _httpClient; private IHttpClient _client; - private HttpClientHandler _httpclientHandler; private readonly TimeSpan _defaultTimeout; public HttpClientBuilder( @@ -33,9 +32,9 @@ public HttpClientBuilder( _defaultTimeout = TimeSpan.FromSeconds(90); } - public IHttpClient Create(DownstreamContext request) + public IHttpClient Create(DownstreamContext context) { - _cacheKey = GetCacheKey(request); + _cacheKey = GetCacheKey(context); var httpClient = _cacheHandlers.Get(_cacheKey); @@ -44,18 +43,26 @@ public IHttpClient Create(DownstreamContext request) return httpClient; } - _httpclientHandler = new HttpClientHandler + var httpclientHandler = new HttpClientHandler { - AllowAutoRedirect = request.DownstreamReRoute.HttpHandlerOptions.AllowAutoRedirect, - UseCookies = request.DownstreamReRoute.HttpHandlerOptions.UseCookieContainer, + AllowAutoRedirect = context.DownstreamReRoute.HttpHandlerOptions.AllowAutoRedirect, + UseCookies = context.DownstreamReRoute.HttpHandlerOptions.UseCookieContainer, CookieContainer = new CookieContainer() }; - var timeout = request.DownstreamReRoute.QosOptionsOptions.TimeoutValue == 0 + if(context.DownstreamReRoute.DangerousAcceptAnyServerCertificateValidator) + { + httpclientHandler.ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator; + + _logger + .LogWarning($"You have ignored all SSL warnings by using DangerousAcceptAnyServerCertificateValidator for this DownstreamReRoute, UpstreamPathTemplate: {context.DownstreamReRoute.UpstreamPathTemplate}, DownstreamPathTemplate: {context.DownstreamReRoute.DownstreamPathTemplate}"); + } + + var timeout = context.DownstreamReRoute.QosOptionsOptions.TimeoutValue == 0 ? _defaultTimeout - : TimeSpan.FromMilliseconds(request.DownstreamReRoute.QosOptionsOptions.TimeoutValue); + : TimeSpan.FromMilliseconds(context.DownstreamReRoute.QosOptionsOptions.TimeoutValue); - _httpClient = new HttpClient(CreateHttpMessageHandler(_httpclientHandler, request.DownstreamReRoute)) + _httpClient = new HttpClient(CreateHttpMessageHandler(httpclientHandler, context.DownstreamReRoute)) { Timeout = timeout }; diff --git a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj index 1824df0d6..9edbfd745 100644 --- a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj +++ b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj @@ -21,6 +21,9 @@ PreserveNewest + + PreserveNewest + diff --git a/test/Ocelot.AcceptanceTests/SslTests.cs b/test/Ocelot.AcceptanceTests/SslTests.cs new file mode 100644 index 000000000..309bbf280 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/SslTests.cs @@ -0,0 +1,146 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Net; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration.File; +using Shouldly; +using TestStack.BDDfy; +using Xunit; + +namespace Ocelot.AcceptanceTests +{ + public class SslTests : IDisposable + { + private IWebHost _builder; + private readonly Steps _steps; + private string _downstreamPath; + + public SslTests() + { + _steps = new Steps(); + } + + [Fact] + public void should_dangerous_accept_any_server_certificate_validator() + { + int port = 51129; + + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "https", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = port, + } + }, + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + DangerousAcceptAnyServerCertificateValidator = true + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn($"https://localhost:{port}", "/", 200, "Hello from Laura", port)) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) + .BDDfy(); + } + + [Fact] + public void should_not_dangerous_accept_any_server_certificate_validator() + { + int port = 52129; + + var configuration = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamPathTemplate = "/", + DownstreamScheme = "https", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = port, + } + }, + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new List { "Get" }, + DangerousAcceptAnyServerCertificateValidator = false + } + } + }; + + this.Given(x => x.GivenThereIsAServiceRunningOn($"https://localhost:{port}", "/", 200, "Hello from Laura", port)) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway("/")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.NotFound)) + .BDDfy(); + } + + private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int statusCode, string responseBody, int port) + { + _builder = new WebHostBuilder() + .UseUrls(baseUrl) + .UseKestrel(options => + { + options.Listen(IPAddress.Loopback, port, listenOptions => + { + listenOptions.UseHttps("idsrv3test.pfx", "idsrv3test"); + }); + }) + .UseContentRoot(Directory.GetCurrentDirectory()) + .Configure(app => + { + app.UsePathBase(basePath); + app.Run(async context => + { + _downstreamPath = !string.IsNullOrEmpty(context.Request.PathBase.Value) ? context.Request.PathBase.Value : context.Request.Path.Value; + + if(_downstreamPath != basePath) + { + context.Response.StatusCode = statusCode; + await context.Response.WriteAsync("downstream path didnt match base path"); + } + else + { + context.Response.StatusCode = statusCode; + await context.Response.WriteAsync(responseBody); + } + }); + }) + .Build(); + + _builder.Start(); + } + + internal void ThenTheDownstreamUrlPathShouldBe(string expectedDownstreamPath) + { + _downstreamPath.ShouldBe(expectedDownstreamPath); + } + + public void Dispose() + { + _builder?.Dispose(); + _steps.Dispose(); + } + } +} diff --git a/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs b/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs index 2f2bf1c71..d4bc994ce 100644 --- a/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs +++ b/test/Ocelot.UnitTests/Requester/HttpClientBuilderTests.cs @@ -60,6 +60,25 @@ public void should_build_http_client() .BDDfy(); } + [Fact] + public void should_log_if_ignoring_ssl_errors() + { + var reRoute = new DownstreamReRouteBuilder() + .WithIsQos(false) + .WithHttpHandlerOptions(new HttpHandlerOptions(false, false, false)) + .WithReRouteKey("") + .WithQosOptions(new QoSOptionsBuilder().Build()) + .WithDangerousAcceptAnyServerCertificateValidator(true) + .Build(); + + this.Given(x => GivenTheFactoryReturns()) + .And(x => GivenARequest(reRoute)) + .When(x => WhenIBuild()) + .Then(x => ThenTheHttpClientShouldNotBeNull()) + .Then(x => ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged()) + .BDDfy(); + } + [Fact] public void should_call_delegating_handlers_in_order() { @@ -111,6 +130,11 @@ public void should_re_use_cookies_from_container() .BDDfy(); } + private void ThenTheDangerousAcceptAnyServerCertificateValidatorWarningIsLogged() + { + _logger.Verify(x => x.LogWarning($"You have ignored all SSL warnings by using DangerousAcceptAnyServerCertificateValidator for this DownstreamReRoute, UpstreamPathTemplate: {_context.DownstreamReRoute.UpstreamPathTemplate}, DownstreamPathTemplate: {_context.DownstreamReRoute.DownstreamPathTemplate}"), Times.Once); + } + private void GivenTheClientIsCached() { _cacheHandlers.Setup(x => x.Get(It.IsAny())).Returns(_httpClient);