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

Remove basic auth from node exporter #100

Closed
brian-brazil opened this issue Aug 11, 2015 · 14 comments

Comments

Projects
None yet
10 participants
@brian-brazil
Copy link
Member

commented Aug 11, 2015

As we've now clarified that our general stance is that exporter auth should be done via a reverse proxy, we should remove the basic auth support from node exporter.

@grobie grobie closed this in 78cc741 Nov 5, 2015

grobie added a commit that referenced this issue Nov 5, 2015

Merge pull request #160 from trumant/issues/100
Closes #100 by removing support for HTTP basic auth
@svagner

This comment has been minimized.

Copy link

commented Mar 11, 2016

Why? May be you should add oppotunity for anable or disable this auth? The point is that many systems doesn't have any reverse proxyes and install this proxies at all nodes isn't correct politic for organization (in highload systems it can give many problems). If we install some reverse proxy we have more one point of deniel. It isn't good.

@brian-brazil

This comment has been minimized.

Copy link
Member Author

commented Mar 11, 2016

Basic auth is just one of many potential ways that a user could do auth, there's LDAP, Kerberos, OTP plus all the directory systems and policies that may be associated with them.
Rather than spending an ever increasing amount of time creating and maintaining such integrations, we've chosen to delegate the responsibility to other projects that handle this problem already.

@svagner

This comment has been minimized.

Copy link

commented Mar 11, 2016

Yes, I agree with you. But I think system must have his own basic simple auth for simple installation with security support. It is very important, beleave me:) This option can be disable by default.

@pete0emerson

This comment has been minimized.

Copy link

commented Apr 1, 2016

I agree with @svagner. I understand the desire to not go down the slippery slope of supporting a plethora of auth types, but having basic auth in there is very valuable for people who want simple protection and don't want the added cost of installing, configuring, and running a reverse proxy.

@klobucar

This comment has been minimized.

Copy link

commented Apr 1, 2016

I understand this slippery slope as well +1 to adding in only basic support and an option to disable (if both are blank) or a flag.

@stapelberg

This comment has been minimized.

Copy link

commented May 19, 2016

Built-in basic auth would be appreciated from my end as well. Of course I can reverse-proxy the node-exporter, but that’s one more config file and dependency in my setup :|.

@juliusv

This comment has been minimized.

Copy link
Member

commented May 24, 2016

Let's say we did support basic auth (which doesn't seem like the plan), then we'd also need to support TLS to make the basic auth useful. So now we have a much more complex problem already (there are many ways of configuring TLS). There's an FAQ item on this now btw.: https://prometheus.io/docs/introduction/faq/#why-don't-the-prometheus-server-components-support-tls-or-authentication?-can-i-add-those?

@m-o-e

This comment has been minimized.

Copy link

commented Jul 30, 2018

This should really be implemented in prometheus where it
can be peer-reviewed and is then available to all users.

Shifting the burden onto your users is not only a royal pain in the ass
but also results in brittle and insecure deployments all over the place.

node_exporter is supposed to run on many hosts, which in most environments
translates to many different OS's and patch levels.

Prometheus should not ask admins to build and maintain multi-platform
training wheels for something as basic as their monitoring probe.

@juliusv

This comment has been minimized.

Copy link
Member

commented Jul 30, 2018

@m-o-e "should" is a subjective judgement. There's pros and cons to each side, but the fact is that anything that is in Prometheus proper needs to be maintained by Prometheus people, and that needs capacity and ongoing commitment.

I am personally ambivalent about this issue, as I feel both sides have good arguments. I would personally like to have a Prometheus supporting all that, I'm less sure about wanting to maintain it :) People like @SuperQ (also Prometheus team) are very much in favor of adding TLS+auth to Prometheus components themselves, but that isn't consensus yet. @brian-brazil has been historically the most cautious person against it, but also indicated that he might be willing to reconsider that if someone puts together a security and response plan for all this, along with recommendations from security people on what to watch out for. I think there was a mailing list thread about this once started by @SuperQ, maybe he can dig it out.

@SuperQ

This comment has been minimized.

Copy link
Member

commented Jul 30, 2018

Yes, transport security and auth very much on my mind. I've already added this to the agenda for the next developer's summit which happens in a couple weeks as part of PromCon.

This issue is not the place to discuss changes, as it's an ecosystem wide policy change.

Feel free to start a conversation on the Prometheus community developers list. We need to consider the whole scope of securing monitoring endpoints.

Please keep the discussion professional. Rants and aggressive language will not be tolerated.

@m-o-e

This comment has been minimized.

Copy link

commented Jul 30, 2018

@juliusv Sorry, my goal was to express strong disagreement, not to be impolite.

For perspective and reference, here's a list of competing/related tools that have TLS:

I was going to add a list of monitoring tools that don't implement TLS as well, but couldn't find any. Not a single one.

@discordianfish

This comment has been minimized.

Copy link
Member

commented Aug 1, 2018

Agreeing in general. I also think this should be added (well, no suprise given that I added basic auth back then).

@Jasper-Ben

This comment has been minimized.

Copy link

commented Sep 2, 2018

As a reverse proxy is now needed for any security/authentication measurement, shouldn't the node exporter only listen on localhost by default?

@discordianfish

This comment has been minimized.

Copy link
Member

commented Sep 2, 2018

I'd suggest to raise this general topic on the prometheus mailing list and discuss it there. Not sure what the latest state of this discussion is.

@Jasper-Ben Yeah, that could make sense indeed.. @SuperQ wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.