glance v2 api: standard user can create public metadefs

Bug #1545717 reported by Stuart McLaren
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Undecided
Unassigned
OpenStack Security Advisory
Undecided
Unassigned

Bug Description

User 1 can create a new namespace with visibility 'public':

 $ cat /tmp/ns.json
 {"namespace": "NS1001", "visibility": "public"}

 $ curl -X POST http://localhost:9292/v2/metadefs/namespaces -H 'x-auth-token: cb334b03257144049b29cd8859b8a3bd' -H 'Content-Type: application/json' -d@/tmp/ns.json

The new namespace shows up in other users' namespace listing:

 $ glance md-namespace-list
 +------------------------------------------+
 | namespace |
 +------------------------------------------+
 | NS1001 |
 | OS::Compute::GuestMemoryBacking |
 | OS::Compute::VMware |
 | OS::Compute::LibvirtImage |
 | OS::Compute::AggregateIoOpsFilter |
 | OS::Compute::Hypervisor |
 | OS::Compute::VirtCPUTopology |
 | OS::Compute::RandomNumberGenerator |

This allows a regular user to spam all other users' namespace listing.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

Changed in ossa:
status: New → Incomplete
description: updated
Revision history for this message
Stuart McLaren (stuart-mclaren) wrote :

Adding Travis here.

@Travis

Does this look like a bug or expected behaviour?

I'm not 100% sure of the security model of the metadef stuff.

Would making the various 'write' metadef policies (add_metadef_namespace, modify_metadef_object, etc) admin only by default make sense/help here?

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

What kind of damage an user can cause by adding public metadef ? Is there a quota for those properties ?

Revision history for this message
Stuart McLaren (stuart-mclaren) wrote :

@Tristan

FWIW Travis put up a really nice, short, demo on the metadef stuff -- useful for anyone not familiar with it:

 https://youtu.be/zJpHXdBOoeM

> What kind of damage an user can cause by adding public metadef?

Travis can probably answer this better.

Here's what I can think of: By spamming with many bogus namespaces they can make it very difficult for users of Glance's metadef API (or Horizon's metadef pages) to find/differentiate between bogus and real namespaces. So kind of a DOS of the metadef API. They could possibly cause upgrade issues by injecting doctored versions of real namespaces introduced in Release X+1 into a server running Release X (namespace grab). I don't think there'd be any effect on non-metadef API calls, so no impact on Glance's main "/v2/images" API or Nova's API. Just the metadef stuff impacted I think.

Revision history for this message
Travis Tripp (travis-tripp) wrote :

As Stuart mentioned, the ability to add, change, get are all controlled by the policy.json. All of the available policy settings are copy / pasted at the end of this comment. So, there certainly is plenty of available control on them if an admin wants to lock them down. As Stuart pointed out, there is not a default set in the checked in policy.json, so there is no restriction. A "role:admin" default would certainly lock this down further. In horizon, the metadata definition administration panel is only in the "admin" dashboard, making it only visible to end users to create new namespaces. So, I think it would make sense for the default for creating and changing namespaces to be restricted to admins. However, I'm hesitant to lock down the ability to add properties and particularly tags. I'd rather see that if a namespace is protected that properties and tags can not be added to it. However if it is not protected, then it could be modified.

Here is my initial recommendation, but I'd also like to chat with Lakshmi Sampath about it:

So default policy should be:

    "modify_metadef_namespace":"role:admin",
    "add_metadef_namespace":"role:admin",

Only Admins can make a namespace public (requres new code).

Policies followed for modifying adding / properties on a namespace using rules below, but in addition, a namespace must not be protected for anybody to modify properties and tags.

It is worth noting that namespaces have a unique constraint on the namespace (e.g. OS::Glance::Image) so that 1) there are no conflicts and 2) somebody couldn't come along and spoof another namespace directly. All API's take the namespace as input for CRUD operations. That said, there is no restriction (by design) that two namespaces have the same display name. This way project Foo and project Bar could potentially have their own namespace of tags or properties that they use but in the UI or CLI they'd could use a similar display name.

List of current policies:

    "get_metadef_namespace": "",
    "get_metadef_namespaces":"",
    "modify_metadef_namespace":"",
    "add_metadef_namespace":"",

    "get_metadef_object":"",
    "get_metadef_objects":"",
    "modify_metadef_object":"",
    "add_metadef_object":"",

    "list_metadef_resource_types":"",
    "get_metadef_resource_type":"",
    "add_metadef_resource_type_association":"",

    "get_metadef_property":"",
    "get_metadef_properties":"",
    "modify_metadef_property":"",
    "add_metadef_property":"",

    "get_metadef_tag":"",
    "get_metadef_tags":"",
    "modify_metadef_tag":"",
    "add_metadef_tag":"",
    "add_metadef_tags":""

Revision history for this message
Stuart McLaren (stuart-mclaren) wrote :

@Travis

Thanks for finding the time to comment here (I know you're swamped).

Reducing the default policies and adding a new policy for 'public' namespaces both sound like good ideas.

Revision history for this message
Travis Tripp (travis-tripp) wrote :

Here's what I posted on the other bug with some updated thoughts:

So, the policy controls allow setting it so only admins can do these kind of operations, but in *briefly* looking at the code, I think there is a mistake in the fact that the policy enforcer rules are also not taking into account ownership and protected status.

I think we might need to do something slightly different than glance handling of the protected field for images. I think ultimately, I think we'd want to get to a model where non admins can add / remove tags and properties to namespaces that are both public and not-protected as long as the policy allows it. But admin of project owning a namespace should always be able to modify a namespaces properties and tags.

So maybe, all the objects, properties, and tags rules have something like (not tested - so kind of pseudo:

"admin_required": "role:admin or is_admin:1",
"owner": "project_id:%(owner)s",
"admin_or_owner": "rule:admin_required or rule:owner",
"public": "%(visibility)s:public",
"owner": "project_id:%(project_id)s",
"not_protected": "%(protected)s:False",
"admin_and_owner_and_not_protected": "rule:admin_required and rule:owner and rule:not_protected",
"owner_or_public_and_not_protected": "(rule:owner or rule:public) and rule:not_protected",
"owner_or_public":"rule:owner or rule:public",

    "get_metadef_namespace":"rule:owner_or_public",
    "get_metadef_namespaces":"",
    "modify_metadef_namespace":"rule:admin_and_owner_and_not_protected",
    "add_metadef_namespace":"rule:admin_required",
    "delete_metadef_namespace": "rule:admin_and_owner_and_not_protected",

    "get_metadef_object": "rule:owner_or_public",
    "get_metadef_objects": "rule:owner_or_public",
    "modify_metadef_object": "rule:owner_or_public_and_not_protected",
    "add_metadef_object":"rule:owner_or_public_and_not_protected",

    "list_metadef_resource_types":"",
    "get_metadef_resource_type":"",
    "add_metadef_resource_type_association":"rule:admin_owner_not_protected",

    "get_metadef_property":"rule:owner_or_public",
    "get_metadef_properties":"rule:owner_or_public",
    "modify_metadef_property":"rule:owner_or_public_and_not_protected",
    "add_metadef_property":"rule:owner_or_public_and_not_protected",

    "get_metadef_tag":"rule:owner_or_public",
    "get_metadef_tags":"rule:owner_or_public",
    "modify_metadef_tag":"rule:owner_or_public_and_not_protected",
    "add_metadef_tag":"rule:owner_or_public_and_not_protected",
    "add_metadef_tags":"rule:owner_or_public_and_not_protected"

So, I think it would make sense for the default for creating and changing namespaces to be restricted to admins. However, I'm hesitant to lock down the ability to add properties and particularly tags. I'd rather see that if a namespace is protected that properties and tags can not be added to it. However if it is not protected, then it could be modified.

Sending this now for more discussion.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Unless this can cause disruption for other users using public metadef, this seems like a class D type of bug according to VMT taxonomy ( https://security.openstack.org/vmt-process.html#incident-report-taxonomy ). Any concerns if I remove the privacy setting and close the OSSA ?

Revision history for this message
Flavio Percoco (flaper87) wrote :

Stuart, thanks for reporting this issue.

Travis, thanks a lot for finding the time. Do you have some feedback for comment #8?

As far as the bug goes, it looks like tweaking the policies should do it, right? Stuart, any comments on this?

Revision history for this message
Travis Tripp (travis-tripp) wrote :

It does seem like the severity could be reduced, but I have to say that I wrote that policy up purely using policy syntax and not in context of current policy enforcer code. I'm not sure if the data needed to perform the policy check is being passed in or not (sorry, didn't trace that far). Lakshmi Sampath would be good to get his opinion if you can add him to this bug.

https://github.com/openstack/glance/blob/cbfd4260424636c037a29f4d743e94752c4cf69f/glance/api/policy.py#L439

Revision history for this message
Flavio Percoco (flaper87) wrote :

I've subscribed Lakshmi!

Thanks, Travis!

Revision history for this message
Stuart McLaren (stuart-mclaren) wrote :

> Unless this can cause disruption for other users using public metadef

See my comment on 2016-02-15 for how a user can potentially disrupt other users, upgrades etc.

@Flavio/Travis

Yes, I think having more restrictive default metadef policies is the way to go here. (As far as I understand it it's ok for regular users just to be able to read the metadefs that an admin has defined).

This one would prevent users creating public metadef namespaces:

"add_metadef_namespace":"rule:admin_required"

Hopefully one patch restricting the policies can address the various bugs I put in.

Revision history for this message
Travis Tripp (travis-tripp) wrote :

@Stuart, I agree that what I put in above is ideal state, but we can simplify most of it to two rules (also make modify be admin required):

    "add_metadef_namespace":"rule:admin_required"
    "modify_metadef_namespace":"rule:admin_required",

And doing the same for properties and objects would be okay as well if the my above ideal state is not feasible.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

I've subscribed OSSG-coresec to discuss an eventual document about rate limiting.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Is the policy change going to be proposed for master branch only ?

In the meantime I've switched the OSSA task to Opinion.

Changed in ossa:
status: Incomplete → Opinion
Revision history for this message
Jeremy Stanley (fungi) wrote :

In keeping with recent OpenStack vulnerability management policy changes, no report should remain under private embargo for more than 90 days. Because this report predates the change in policy, the deadline for public disclosure is being set to 90 days from today. If the report is not resolved within the next 90 days, it will revert to our public workflow as of 2020-05-27. Please see http://lists.openstack.org/pipermail/openstack-discuss/2020-February/012721.html for further details.

description: updated
Jeremy Stanley (fungi)
description: updated
description: updated
Revision history for this message
Jeremy Stanley (fungi) wrote :

The embargo for this report has expired and is now lifted, so it's acceptable to discuss further in public.

description: updated
information type: Private Security → Public Security
information type: Public Security → Public
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers