Access to sensitive audit data is not properly restricted

[Matthew Edmonds]

Audit data stored in http.request and http.response meters is not being adequately protected. Admins are allowed to access audit data for all projects rather than just their own. Non-admins are allowed to access audit data for all users within their project rather than just themselves. A non-admin user should not be able to see what other users are doing, and being an admin in project A does not make you an admin in project B.

The following blueprints acknowledge the lack of this support. To quote one: "as ceilometer collects more and more different types of data... some of the data collected may be 'privileged' data that only admins should have access to regardless of membership to a tenant (ie. audit data should only be visible to admins)". That day has come, and the implementation of these blueprints is still missing. At this point there is a security hole here (data exposure) which needs to be plugged immediately, either with the implementation of one of these blueprints (which should probably be merged together) or by a less flexible but more easily implemented stopgap measure. Given time constraints and the urgency of closing this hole, I propose the latter, though the blueprints will obviously still be necessary for a more robust and complete solution.

https://blueprints.launchpad.net/ceilometer/+spec/advanced-policy-rule and https://blueprints.launchpad.net/ceilometer/+spec/admin-only-api-access and https://blueprints.launchpad.net/ceilometer/+spec/ready-ceilometer-rbac-keystone-v3

[Jeremy Stanley (fungi)]

I've added an incomplete security advisory task and subscribed the Ceilometer core security reviewers to confirm your report.

[Julien Danjou (jdanjou)]

Ceilometer never supported any kind of policy so far. The only one supported is used to determin if a user is an admin or not. I don't know where you saw these "meter:get_all" rules, but they don't exist at all. Nor does any rule.

[Dina Belova (dbelova)]

Agree with Julien, we don't support any policies stuff right now. There is a BP about defining actually meaningful policies in ceilo https://blueprints.launchpad.net/ceilometer/+spec/advanced-policy-rule - it's all about it, and actually that's the thing we should do for sure, but that's not kind of suddenly appeared bug.

[Matthew Edmonds (edmondsw)]

"The only one supported is used to determin if a user is an admin or not". I.e., you do support restricting access to admins? Ok, how? Because I've tried everything I can think of and I can't get it to restrict access only to admins.

Allowing non-admin users access to all ceilometer data clearly has to be considered a security vulnerability. There is sensitive data there.

[Julien Danjou (jdanjou)]

That's not what I meant. The only thing is describing the rule that defined what an admin is.

Non-admin users don't have access to all data, nor alarm, etc.

[Eoghan Glynn (eglynn)]

Matthew - the point was that we do actually enforce tenant segregation for non-admin callers of the ceilometer-api, but only with a *fixed* view of what constitutes an admin user.

So it's not a vulnerability, more a lack of flexibility.

Say if a user want to invent a new more limited admin role, say alarm-admin, it wouldn't be possible currently to configure ceilometer so that user/tenants with this role could see the alarms of other user/tenants but not their samples for example.

[Matthew Edmonds (edmondsw)]

@jdanjou: I assume you're referring to this in the default policy.json file:

{
    "context_is_admin": [["role:admin"]]
}

But that doesn't do anything by itself... It just defines a rule. It doesn't actually make anything use the rule. Unless you've hardcoded to use this rule for everything, but if you've done that it doesn't seem to be working because...

No matter what the policy file looks like, including using the sample I just mentioned, I have found that non-admin users DO have access to alarms, meters, etc... so the statement that "Non-admin users don't have access to all data, nor alarm, etc." appears to be false. If there is something specific that I need to do to restrict access to non-admin users, please tell me what that is.

@eglynn I am testing with non-admin users in the same tenant. If you handle this better for multi-tenant cases, great, but it is a security vulnerability to allow any non-admin user, even those in the same tenant, access to the sensitive data stored in ceilometer.

[Julien Danjou (jdanjou)]

We scope on project, so users in the same project have access to all meters of the project, like I think the rest of OpenStack services.

So what you describe it the intended behaviour – at least until we enhance our RBAC policy with new features.

[Matthew Edmonds (edmondsw)]

Audit data is highly sensitive. Saying that all users on a project can view the audit data for that project has to be considered a security vulnerability. This being "the intended behavior" just means someone didn't think things through very well, not that it isn't a security vulnerability... and a bad one at that.

[gordon chung (chungg)]

for what it's worth, we have this bp: https://blueprints.launchpad.net/ceilometer/+spec/admin-only-api-access but since we don't have resources (or other items took higher priority/interest) it has yet to be implemented.

[Jeremy Stanley (fungi)]

Okay, so it sounds to me like this is a (perhaps contentious and/or incomplete) design decision in Ceilometer. It's certainly worth discussing how to improve this in the project going forward, and the blueprint linked above appears to be an approved solution if anyone wants to help address the issue. However it does not match the Vulnerability Management Team's criteria for issuing an OpenStack Security Advisory since this sort of ongoing development is not likely to get backported to any existing stable releases, but would instead be a behavior change/new feature in an upcoming release.

The OpenStack Security Group may be interested in issuing a security note about this as an addendum to the Security Guide, so I've tagged it as "security" accordingly.

[Matthew Edmonds (edmondsw)]

I wouldn't expect full implementation of those blueprints to get backported to previous releases, but is full implementation of those blueprints necessary to close the security issue that is allowing non-admin users access to information which only admins should be able to see? I would expect a more limited fix to close this hole in Juno and be backported to previous releases, with the full blueprint implementations coming in later.

[Jeremy Stanley (fungi)]

That's a decision which will have to be left up to the developers. Did you have a particular backportable patch to propose (one which would meet our stable branch guidelines, e.g. no new configuration and no outward behavior changes)?

[Matt Riedemann (mriedem)]

So it sounds like the 'workaround' in this case is for the deployer/application building on ceilometer to change role/project definitions so non-admin users aren't in the same project as admin users, and then they won't share access to the same data.

I don't see anyone working either of the blueprints:

https://blueprints.launchpad.net/ceilometer/+spec/advanced-policy-rule
https://blueprints.launchpad.net/ceilometer/+spec/admin-only-api-access

And those have been open for awhile. If people want the design changed to allow different configurations, the blueprints should be worked.

As Matthew pointed out in comment 12, blueprint changes wouldn't be backport-able, so I'm wondering if there is a more tactical fix here until the BPs could be implemented?

[Matthew Edmonds (edmondsw)]

I don't see any way to workaround this security issue with role/project definition restructuring... Unless you're suggesting we have a separate project for each non-admin user (which is obviously unworkable), non-admins in the same project would have access to each other's data. Non-admin users must not be allowed access to anyone else's data.

I'm not familiar enough with the alarms and resources portions of the ceilometer API to speak there, but when it comes to meters I think the simplest option for a backportable solution would be to restrict meters requests to admins until those blueprints can be implemented. Another option would be allowing anyone to make the requests but filtering the results that are returned such that non-admin users only get back data for themselves.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/130854

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/132097

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.openstack.org/130854
Reason: Going to abandon for Matthew's more targeted fix:

https://review.openstack.org/#/c/132097/

[gordon chung (chungg)]

so there is a possible workaround for this in Kilo. in Kilo, it's possible to publish to different topics so you could create two pipelines, one with non-audit data and another for audit data. they would publish to two different topics. from there you could have two collectors listening to each topic and writing to a separate db/target. that way you can create a separation of data.

that said, it does involve maintaining two different targets (and two apis if both write to db). i don't know if this is a viable workaround?

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matthew Edmonds (<email address hidden>) on branch: master
Review: https://review.openstack.org/132097
Reason: pycadf.middleware.audit was deprecated/removed in favor of keystonemiddleware.audit. The later uses events rather than meters, where this is no longer relevant.

[gordon chung (chungg)]

https://blueprints.launchpad.net/ceilometer/+spec/events-rbac