Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Securing the gossip protocol? #1322

Closed
wrouesnel opened this issue Apr 12, 2018 · 16 comments · Fixed by #2237
Closed

Securing the gossip protocol? #1322

wrouesnel opened this issue Apr 12, 2018 · 16 comments · Fixed by #2237

Comments

@wrouesnel
Copy link

Is there anyway at the moment to add TLS authentication to the gossip protocol (or is it secure as-is?)

I need to comply with a policy that all traffic is encrypted by default within our infrastructure, so ideally there'd be support for specifying TLS server and client certificates on the command line to secure connections. Is this in the roadmap somewhere?

@brian-brazil
Copy link
Contributor

It might happen sometime in the future, but I wouldn't hold your breath.

@hoffie
Copy link
Contributor

hoffie commented Apr 15, 2018

I am also interested in this. Would it have to be TLS? It seems like the newly introduced memberlist implementation also supports AES-GCM encryption using the SecretKey Config option. This could be added somewhere in the memberlist initialization code in cluster.go.

Would this be an improvement which would be accepted by the project? If so, I might try to integrate and test this.

@brian-brazil
Copy link
Contributor

I'd prefer not to get into ad-hoc security mechanisms that some of our dependencies happen to support, that could bring with it the same maintenance challenges we're currently trying to avoid with auth elsewhere.

@wrouesnel
Copy link
Author

The trouble is that dependency does make securing it non-trivial since it likes to mix up UDP and TCP, so most of the usual tricks like putting stunnel in front of it don't work. It's kind of glaring because Prometheus everywhere else can be secured with TLS, but this can't.

Would you be open to a patch which (at least currently) traded some performance of the gossip protocol for allowing a set of regular TLS parameters? - i.e. a flag which enables encryption and limits the gossip library to using TCP only so regular TLS can be used? I wouldn't think that would constrain the future too much, since pretty much everything uses TLS in some form, and if you stick with gossip then nothing stops expanding the choice later to use whatever mechanism that allows.

@brian-brazil
Copy link
Contributor

I think options for our standard tls_config client options would be okay. That'd have to go in the config file somewhere.

@mxinden
Copy link
Member

mxinden commented Feb 22, 2019

@wrouesnel #1763 adds a design document as well as a prove of concept implementation to secure the gossip traffic. Please have a look. I would be interested in your thoughts.

@lilic
Copy link

lilic commented Oct 5, 2020

@simonpasquier @brian-brazil @mxinden I would be interested in tackling this, just curious if the linked proposal doc is up to date? https://github.com/prometheus/alertmanager/blob/master/doc/design/secure-cluster-traffic.md

@roidelapluie
Copy link
Member

Hello, AFAIK, we would like to reuse the same TLS code that the node_exporter is using.

@mxinden
Copy link
Member

mxinden commented Oct 9, 2020

I would be interested in tackling this

Very cool!

just curious if the linked proposal doc is up to date?

It should be. But in the meantime multiple things happened:

  1. I created a proof-of-concept in [WIP] cluster: Secure cluster traffic via mutual TLS #1819.

  2. TLS support landed in node-exporter Adding TLS to node exporter - cleaner version node_exporter#1277. The TLS configuration struct should be reused to configure Alertmanager gossip via TLS.

  3. A new proposal based on the design doc and my proof-of-concept has been created by @hooten in Secure cluster traffic via mutual TLS #2237.

@lilic let me know if this is of some help.

@hooten
Copy link
Contributor

hooten commented Oct 9, 2020

Cool! @sharadgaur and I were waiting for a couple things to happen before continuing #2237:

  1. security audit of node_exporter's tls_config.go code
  2. the code to move to prometheus/common.

Looks like the latter hasn't happened yet. Has the former?

@brian-brazil
Copy link
Contributor

The former happened.

@iplay88keys
Copy link

@lilic Hi! I just wanted to see where you are at with this. My team and I are also looking to have all traffic secure by default and are interested in the state of this work.

@lilic
Copy link

lilic commented Dec 16, 2020

@iplay88keys I was under the impression @hooten was continuing the work first? IF this is not the case @iplay88keys feel free to take over this issue!

@hooten
Copy link
Contributor

hooten commented Dec 16, 2020

Yes. @sharadgaur and I would like to continue the work we’ve done. Thanks!

@iplay88keys
Copy link

@lilic @hooten Thanks for the information! We'll keep an eye on the progress, then!

@hooten
Copy link
Contributor

hooten commented Feb 9, 2021

I'd appreciate a review of #2237!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants