Update docs for `tls` for listeners

jamielinux commented 1 year ago

A note for the community

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Problem

Many of the sources have the tls.verify_certificate option. These options usually have this line in their documentation:

Do NOT set this to false unless you understand the risks of not verifying the validity of certificates.

I find it odd that this warning is here, and yet the default is false.

Should tls.verify_certificate default to true (which would be the safest default)?

Configuration

No response

Version

0.27.0

Debug Output

No response

Example Data

No response

Additional Context

No response

References

No response

jszwedko commented 1 year ago

Hi @jamielinux !

That option is a bit confusing with respect to defaults. When tls.enabled is used on components that use an HTTP client (i.e. they make HTTP requests) it defaults to true. When used on components that use an HTTP server, it defaults to false as otherwise it would require connecting clients to present a HTTP client certificate (less common).

Which component in particular are you interested in the default for?

jamielinux commented 1 year ago

Hi @jszwedko thanks for the reply! I did indeed get confused.

I'm using the vector sinks on various hosts to send data to a vector source on a centralized host, with certs on both sides (a client cert for the sink, and server cert for the source, all self-signed).

I just noticed that the default for tls.verify_certificate is false for the vector source but true for the vector sink. That matches with your explanation. So that clears that up! I think I got my sinks and sources mixed up in my head.

Unless I misunderstand, TLS options for the vector source are only relevant for incoming connections(?). If that's correct, I do still think the text on the vector source documentation for tls.verify_certificate could be improved (and by extension, the documentation for other sources too):

Instead of Enables certificate verification, should it say Enables client certificate verification?
Instead of Relevant for both incoming and outgoing connections, should it say Relevant for incoming connections?
Should the Do NOT set this to false warning be removed?

jszwedko commented 1 year ago

Hi @jamielinux !

I agree with all of those docs updates. I'll leave this issue open to track that.

vectordotdev / vector