serilog-contrib / serilog-sinks-splunk

A Serilog sink that writes to Splunk
https://splunk.com
Apache License 2.0
46 stars 47 forks source link

Add ability to ignore certs for Splunk in sink #26

Closed merbla closed 7 years ago

merbla commented 8 years ago

From #23

In a development environment it would be helpful to ignore sinks.

An implementation similar to the Seq sink may be appropriate.

veccsolutions commented 8 years ago

I too am in need of this. Is there any plan for it? Dotnet core doesn't seem to have a cert validation callback like the original dot net does.

merbla commented 8 years ago

@veccsolutions a new release is out with the HttpMessageHandler exposed. I will update the wiki however it may be of use to your situation.

veccsolutions commented 8 years ago

Thanks! I'll be checking this out in the next couple of days.

veccsolutions commented 7 years ago

@merbla Thanks for exposing it. However, I can't specify whether to ignore it or not through the config files. I was able to write my own sink configuration method to do it using the one you have already created as a template.

My custom configuration method is identical to yours except for the following changes.

  1. In project.json I increased the netstandard framework to 1.6 (I didn't try earlier versions, but the ServerCertificateCustomValidationCallback property wasn't in the netstandard 1.1 framework)
  2. I added an ignoreSsl parameter, and the following between your null checks and newing up the eventcollectorsink object:

            if (ignoreSsl)
            {
                //only ignore the certificates for correct host
                var uri = new Uri(splunkHost);
                var lowerHost = uri.Host?.ToLowerInvariant() ?? string.Empty;
    
    #if NET45
                ServicePointManager.ServerCertificateValidationCallback +=
                    (object sender, X509Certificate cert, X509Chain chain, SslPolicyErrors policyErrors) =>
                        ServerCertificateCallback((sender as WebRequest)?.RequestUri?.Host, lowerHost, policyErrors);
    #else
                HttpClientHandler client;
                //allow custom httpmessagehandlers
                if (messageHandler == null)
                {
                    client = new HttpClientHandler();
                }
                else
                {
                    client = messageHandler as HttpClientHandler;
                }
    
                //if the message handler is either HttpClientHandler or derived from it we can add to the callback
                if (client != null)
                {
                    client.ServerCertificateCustomValidationCallback +=
                        (HttpRequestMessage message, X509Certificate2 cert, X509Chain chain, SslPolicyErrors policyErrors) =>
                            ServerCertificateCallback(message?.RequestUri?.Host, lowerHost, policyErrors);
    
                    messageHandler = client;
                }
    #endif
            }
  3. The ServerCertificateCallback method is:

        private static bool ServerCertificateCallback(string requestHost, string splunkHost, SslPolicyErrors policyErrors)
        {
            //quick check for no error
            if (policyErrors == SslPolicyErrors.None)
            {
                return true;
            }
    
            return requestHost.ToLowerInvariant() == splunkHost;
        }
merbla commented 7 years ago

@veccsolutions you are correct. The change was generally targeted at dotnet core and opening up the HttpMessageHandler for more than just the situation to ignore SSL certs.

We would have overload the config with an ignoreSSLValidation like param for the configuration from file to work. I am a little reluctant to implement however I know it can be a pain in development.

Generally what frameworks are you targeting?

nblumhardt commented 7 years ago

Just to add to @merbla's remarks about being reluctant to add this - I think we'd expose ourselves to some criticism by dealing with SSL certificate policy in the sink. (Even in the case of self-signed certificates, it's usually better to add the certificate to the list trusted by the client machine rather than disable validation entirely.)

veccsolutions commented 7 years ago

That's fine. I'll continue with my separate config method. I just need to be able to do it without managing the certs on the local system and still using the config file for different deployment targets like local development vs QA vs Production etc.

merbla commented 7 years ago

Closing this for now.