From: "Lucas Ponce" <lponce(a)redhat.com&gt; To: "Discussions around Hawkular development" <hawkular-dev(a)lists.jboss.org&gt; Sent: Thursday, 2 March, 2017 12:51:46 PM Subject: Re: [Hawkular-dev] Cross-Tenant endpoints in Alerting on OS What is the result of this discussion ?

If I understood well, even if we had a discoordination for these endpoints, those are not a security concern, or is there any additional test to perform to validate this ? The original motivation is still valid, as for Alerting these two endpoints are demanded for users.

Do you want any additional enhancement around this ? I can see two potential actions: - Perhaps to move the /admin endpoint prefix to /tenants to align with the same in metrics.

- Extend the security filter on /admin (/tenants) endpoint. Thanks, Lucas On Fri, Feb 24, 2017 at 2:04 PM, Jay Shaughnessy < jshaughn(a)redhat.com > wrote: On 2/23/2017 6:05 PM, Matt Wringe wrote: > Is there any reason why this being sent in private emails and not to a > mailing list? Matt, Not really, sending to dev-list for anyone interested in the discussion... > ----- 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" Note, not in general, comma-separated-lists handled only for the two cross-tenant endpoints mentioned above. > 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. Of course. This stuff went in in mid November and at that time we (in alerting) were really just getting settled with the initial integration into metrics for OS. Going forward I think we have a better idea of what is relevant to OS and can more easily flag items of import. _______________________________________________ hawkular-dev mailing list hawkular-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hawkular-dev _______________________________________________ hawkular-dev mailing list hawkular-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hawkular-dev

Multiple components beyond Metrics / Alerts or the security filters ? Can you ellaborate this, please ? At the very least we need to discuss how to properly handle it with regards to security integration with OpenShift. Yes. Please, could you prepare some info you would need from Alerting component ? I.e. tenant formats or other related info different from the current logic.