alertmanager
alertmanager copied to clipboard
prometheus
Feature request: restrict the identity of silence creator to one passed by some HTTP header
What did you do? We configure a proxy in front of Alert Manager to authenticate, authorize and audit access, especially with regard to creating silences.
Currently a user can still enter any value they like for the "Creator" field of a silence through the web UI.
What did you expect to see?
It would be super if we could have an option to force the Creator value to one set via some HTTP header that is inserted into the request by the authenticating proxy. For example, if the proxy set the X-Alertmanager-User header, then the Alertmanager UI application would prepopulate the field and only permit values for "Creator" that matched that header value.
Ideally the header fields should be configurable as well. My identity proxy gives me: X-Auth-Request-Email
Any authorisation features would have to be implemented by a proxy in front of the alertmanager, as that's a rabbit hole we don't want to go down inside the project itself. I think defaulting the value of this field based on hardcoded header name would be okay.
I'm happy to spend some time working on this. I think it should be pretty simple to implement even with a configurable header.
Ok so I may have completely misunderstood the point of this feature request.
I have implemented a part of the api that overrides the createdBy field sent by the proxy, but I haven't endeavored to make the ui auto-populate the field yet. I guess that making the api return an error on mismatch might be better then just silently overriding the createdBy field.
So I guess I ask @mitchellrj @simonswine if I am on the right track? Or am I completely off in left field?
The original feature request was for forcing via a header, however as per https://github.com/prometheus/alertmanager/issues/1196#issuecomment-373429029 that's out of scope for the alertmanager. Returning a error if the user chooses to tweak the created by field is similar.
The best we can do is default the field value, and leave the rest to code outside the alertmanager.
Hi Kellen, first let me say thanks for stepping up and helping!
For some more context:
We run alert managerbl behind a proxy application that provides
authz/authn, and then passes authorised requests through to Alertmanager.
Along with the original request, the proxy application adds headers (e.g.
X-Auth-User). In order to have a sort of audit / non-repudiation function
around who has created / modified silences, we would like to force the
Creator field for end-user requests to always take the value of the header
passed through. That needn't be reflected in the UI, but would be much
clearer to the end user if it were. The threat scenario driving this desire
is that a malicious actor who is otherwise authorised, could silence alerts
and then do some other nasty deed, delaying response from other teams.
On Thu, 7 Jun 2018, 17:16 Kellen Fox, [email protected] wrote:
Ok so I may have completely misunderstood the point of this feature request.
I have implemented a part of the api that overrides the createdBy field sent by the proxy, but I haven't endeavored to make the ui auto-populate the field yet. I guess that making the api return an error on mismatch might be better then just silently overriding the createdBy field.
So I guess I ask @mitchellrj https://github.com/mitchellrj @simonswine https://github.com/simonswine if I am on the right track? Or am I completely off in left field?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prometheus/alertmanager/issues/1196#issuecomment-395479635, or mute the thread https://github.com/notifications/unsubscribe-auth/AAI-fodC_VgsP4FDnZslQGCW4CPz9ZK8ks5t6VHxgaJpZM4RdnmX .
Alternatively, there could be an option to pass a correlation ID through from the proxy and have that be logged along with actions be Alertmanager in some verbose/audit log mode. That would meet our needs and also satisfy Brian's understandable desire not to see a strict authz/authn function creep into Alertmanager.
On Thu, 7 Jun 2018, 17:26 Brian Brazil, [email protected] wrote:
The original feature request was for forcing via a header, however as per #1196 (comment) https://github.com/prometheus/alertmanager/issues/1196#issuecomment-373429029 that's out of scope for the alertmanager. Returning a error if the user chooses to tweak the created by field is similar.
The best we can do is default the field value, and leave the rest to code outside the alertmanager.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prometheus/alertmanager/issues/1196#issuecomment-395482797, or mute the thread https://github.com/notifications/unsubscribe-auth/AAI-fuYTZYdOJ8Rx-YzKCuqJ4YFX_dl4ks5t6VRQgaJpZM4RdnmX .
What I'd suggest here is having your proxy log all silence related POST requests (and whether they succeeded) from users to the Alertmanager. That'd allow you to track down who created a specific silence if someone was being malicious.
Silences in the Alertmanager are not intended to be used as an a audit trail, similarly to how the Alertmanager generally is not intended to hold a log of all alerts/notifications passing through it.
From https://twitter.com/juliusvolz/status/1031054925768388608 perhaps we should take another look at this?
This is an authorization feature, which is still out of scope. All of my previous comments still apply.
Silences in the Alertmanager are not intended to be used as an a audit trail, similarly to how the Alertmanager generally is not intended to hold a log of all alerts/notifications passing through it.
@brian-brazil Then why is there a "Creator" field at all? Not having an option to make that field auto-populate results in many issues:
- inconsistency and incorrect data
- fatigue: (why do I have to keep entering my name dozens of times a day?
There are separate issues:
- Auto-populating "Creator" field. This is what is needed the most. #1406 appears to solve this.
- Restricting changes to the field: This requires Authorization which AlertManager doesn't have today.
@brian-brazil: Could we continue with addressing item 1 without getting hung up on item 2?
fatigue: (why do I have to keep entering my name dozens of times a day?
You should only need to enter it once, if not there's something wrong.
Yes, that can be addressed separately.
@brian-brazil Would the following be an acceptable design?:
-
Add a GET api that provides defaults for ui preferences. I think it would be appropriate for this to have
Cache-Control: privatein addition to the other cache-suppressing headers. -
Add a flag for the admin to specify a header to get the default
defaultCreatorfrom. If the flag is set, and if a request to the ui preferences api has the specified header, the trimmed value of that header would be returned as the defaultdefaultCreator. Otherwise, the response would specify an empty string as the defaultdefaultCreator.
Add a GET api that provides defaults for ui preferences.
I don't see why we'd need this, rather than baking a default into the binary.
I think it would be appropriate for this to have Cache-Control: private in addition to the other cache-suppressing headers.
Our existing headers should already be preventing all caching for api results, if that's not the case it it a bug that should be fixed.
Add a flag for the admin to specify a header to get the default defaultCreator from.
I don't see a need for this to be configurable.
The cost to have it configurable in AM is quite low vs the cost to actually change reverse proxies config
I don't see a need for this to be configurable.
There is no standard header for this information; it varies between IAP solutions.
The cost to have it configurable in AM is quite low vs the cost to actually change reverse proxies config
It's yet another configuration flag that users have to keep in mind, and if you've already a reverse proxy if should be able to set whatever header you like.
Neither Pomerium nor Google's IAP allow configuration of the headers they add. This is for the benefit of users who do not want to manage running an alertmanager-specific reverse proxy.
This is already a niche use case, and I don't think we should be adding complexity for all alertmanager users for the sake of something that helps a small number of users, and only then on the very first time they use a given AM for this.
I also note that many requestors above were looking for auth features, which would require an alertmanager-specific reverse proxy.
I indicated almost two years ago what the parameters of such a feature are, and a PR to add it has yet to be sent.
We have discussed it during the Alertmanager working group. Now that Alertmanager supports basic auth, we are open to adding basic auth user and http header as creator for silences.
Basic auth is rubbish for SSO auth / meaningful / productionized deployment of alertmanager which aim to serve a multitude of users.
Is using basic auth a stop-gap or is that really the peak of technical progress that the alertmanager working group can muster?
Hello, "basic auth user and http header"
In a SSO setup, the user will likely be passed on as a HTTP header so this is covered.
@gotjosh , @roidelapluie : Thank you for keeping this topic going. At least for me, it's a long awaited wish becoming true. :heart:
I would like to see this feature as we have a reverse proxy in front Alertmanager for authentication that could easily add a username header for Alertmanager to consume and prefill the author, or even enforce the author.
