[Hawkular-dev] Cross-Tenant endpoints in Alerting on OS

Is there any reason why this being sent in private emails and not to a mailing list?

----- Original Message ----- > There was an IRC discussion today about $SUBJECT. Here is a summary of > a conversation Matt and I had to drill down into whether there was a > cross-tenant security concern with the Alerting API in OS. In short, > the answer seems to be no. Alerting (1.4+) offers two endpoints for > fetching cross-tenant: /alerts/admin/alerts and /alerts/admin/events. > Note that the 'admin' is just in the path, and was chosen just to group > what we deemed were admin-level endpoints, the first two of which are > these cross-tenant fetches. The 'admin' does not mean anything else in > this context, it does not reflect a special user or tenant. The way > these endpoints work is that that they accept a Hawkular-Tenant HTTP > header that can be a comma-separated-list of tenantIds. As with any of > the alerting endpoints. Alerting does not perform any security in the > request handling. But, in OS the HAM deployments both have the OS > security filtering in place. That filtering does two things, for a > cluster-admin user it's basically a pass-thru, the CSL Hawkular-Tenant > header is passed on and the endpoints work. For all other users the > Hawkular-Tenant header is validated. Because each project name is a > tenant name, the value must match a project name. As such, the > validation fails if a CSL is supplied. This is decent behavior for now > as it prevents any undesired access. Note that as a corner-case, these > endpoints will work fine if the header just supplies a single tenant, in > which case they are basically the same as the typical single-tenant > fetch endpoints. What has happened is now Alerts is not considering the Hawkular-tenant header to contain just a string, but a comma separated lists of strings. eg "Hawkular-tenant: projectA,projectB"

The OpenShift filter still considers this to be a string, so it will check with OpenShift if the user has permission to access the project named with a string value of "projectA,projectB". Since a project cannot have a ',' within its name, this check will always fail and return an access denied error. If the user is a cluster level user they are given access to everything, even impossibly named projects. So a cluster level user will happen to be able to use the current setup just due to how this works. So there doesn't appear to be any security issue that we need to deal with immediately, but we do probably want to handle this properly in the future. It might not be too difficult to add support to the tenant to consider a csl. > I'm not totally familiar with the Metrics approach to cross-tenant > handling but going forward we (Metrics and Alerting) should probably > look for some consistency, if possible. Moreover, any solution should > reflect what best serves OS. The idea of a CSL for the header is fairly > simple and flexible. It may be something to consider, for the OS filter > it would mean validating that the bearer has access to each of the > individual tenants before forwarding the request. I don't recall any meetings about adding multitenancy to Metrics. From what I recall, there is no plans at all to introduce multitenancy at all for metrics. If I was aware of this discussion when this was brought up for alerts, I would have probably objected to the endpoint being called 'admin' since I don't think that reflects what the true purpose of this is suppose to be. Its not really an admin endpoint, but an endpoint for cross-tenancy. I could have access to projectA and projectB, but not be an 'admin' If we are making changes like this which affect security, I would really like to be notified so that I can make sure our security filters will function properly. Even if I am in the meeting when its being discussed it would be good to ping me on the PR with the actual implementation.