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

[WIP/RFC]Adding a simple Basic Authentication mechanism. #972

Closed
wants to merge 1 commit into from

Conversation

prasincs
Copy link

@prasincs prasincs commented Aug 9, 2015

I noticed that both AuthManager and Prometheus web endpoints are wide open. They include configuration information including service keys for various web services like PagerDuty. Securing these web endpoints would require additional service/reverse_proxy to add some kind of authentication. My goal with this fix was to add a very simple bare-minimum, optional authentication mechanism. I didn't find anything accessible in the mailing lists that implied someone was working on this.

How to use it:

  • Caveat: I tried adding the config in the config.yml but the way the web struct is initialized I only get the string form of config and would have to parse again.

./prometheus -config.file=./prometheus.yml -web.auth-enabled=true -web.basicauth.username test -web.basicauth.password '$1$dlPL2MqE$oQmn16q49SqdmhenQuNgs1'

where the password is the digest generated by htpasswd for "hello"

I'm sure there are many stylistic changes that could be made but I'm neither expert on prometheus or Go, so I'm open to any criticisms and improvements to the code. I do think at least having basic authentication for web endpoints is warranted. If there are better ideas in motion, I'd be curious when they'd be implemented.

@brian-brazil
Copy link
Contributor

Per previous discussion and prometheus/docs#149 we believe that any authentication/authorization for the web frontends should be done in a reverse proxy. There's just too many possible configurations (SSL alone has 20+ options that we'd need to expose), and it's better than all be handled in something designed for such a role as it's not a essential component of a monitoring system.

@fabxc
Copy link
Contributor

fabxc commented Aug 9, 2015

For many users a proper set of tutorials for some standard solutions would go a long way. People are running a bunch of exporters so it's not about having another process running but mostly about auth being far more complex than the regular setup of Prometheus itself.

Filed prometheus/docs#158

@prasincs
Copy link
Author

@brian-brazil @fabxc I agree with your points, the suggestion is to have something just-sufficient for a small scale deployment and that works out of the box. I don't think a monitoring system should be in the business of doing SSL termination and such. However, allowing a very basic auth (similar to what haproxy does for its monitoring endpoint) would make the first small deployments for organizations much simpler. It's not even really "secure" -- the goal was to avoid people without access looking at the stringified config in the web endpoint's page.

@brian-brazil
Copy link
Contributor

the goal was to avoid people without access looking at the stringified config in the web endpoint's page.

All secrets should be hidden on the config page already, is this not the case?

@matthiasr
Copy link
Contributor

It's not even really "secure"

This is why I would very strongly caution against even offering the option in Prometheus. The illusion of security is worse than no security. We'll need to write up how to do it, but putting an nginx in front of Prometheus is very easy.

@prasincs
Copy link
Author

@brian-brazil you can see the keys for various services in authmanager on the config page. I don't use services that require auth to read metrics from to know if Prometheus is handling that or not.

@brian-brazil
Copy link
Contributor

@prasincs The alertmanager has not had secrets hidden yet, but this will presumably happen as part of it's rewrite.

@juliusv
Copy link
Member

juliusv commented Aug 11, 2015

Closing this as per the explanations above.

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

Successfully merging this pull request may close these issues.

None yet

5 participants