Add Sink configure option to effectively disable SSL Certificate Verification

[jasonsummers]

trafficstars

Add the 'ignoreServerCertificateValidation' Sink configuration parameter to LoggerConfigurationElasticSearchExtensions.cs which, when true will configure the Elasticsearch client with a ServerCertificateValidationCallback which always returns true.

The default behaviour is unchanged.

What issue does this PR address? #384

Does this PR introduce a breaking change? No

Please check if the PR fulfills these requirements

• [x] The commit follows our guidelines
• [ ] Unit Tests for the changes have been added (for bug fixes / features) - Could not find a sensible place to put tests, grateful for direction if tests are required for this change.

Other information:

[mivano]

Thanks for the PR. As it is an extra parameter (even with a default value), it still is a breaking change. Not a big issue though. More troublesome; this introduces a risk as you should check the certificates and not just return true. You most likely have a good reason for this change. Is there another way to get the behaviour as I m not that in favour of adding this option to the already large list of parameters?

[jasonsummers]

Hi @mivano,

Thank you very much for the prompt and useful feedback. I've pushed another commit which addresses the concern of the growing parameter list by combining the new SSL options with the existing ConnectionGlobalHeaders option. This approach is clearly more of a breaking change but does keep the number of parameters the same. I've also made the settings more granular to ignore specific types of SSL Error.

I'd appreciate your guidance on what you feel would be an acceptable solution from a security perspective, I've come up with the following options and would appreciate your perspective:

1. Add an 'AllowedSignatures' option to the new configuration where consumers can place something like the hash or the public string of the certificates that they would like to ignore.
2. Scan the current working directory for certificates which should be allowed (arguably removes the need for a change in the configuration options altogether)

How deep do you think we should go from a security perspective? The security risk as far as I can see is that a threat actor could redirect the log traffic (and potentially sensitive information) to their own elastic search instance. In order for such an attack to be successful the attacker would need to either: a) compromise appsettings.json and change the nodeUris property to point to their Elastic search instance; or b) compromise the DNS/IP of the current Elastic search instance which would only be successful if the target had one or more of the new 'IgnoreSsl' options set to true.

If either of these scenarios occurred I'd possibly argue that the target has bigger problems. Very grateful for your thoughts on this.

[mivano]

I like this new approach. Makes it more concise what needs to be enabled and set up. For me, the simple switch of setting some boolean was what I not really liked, this solution requires more explicit steps. And you are right; it is never completely securable. It is however more of a breaking change, so I will need to document that.