glance v2 api: standard user can update other user's public metadefs

[Stuart McLaren]

If project 'd12bddf60e4649b2a2cf6a2cc7520d79' owns a global namespace:

 $ openstack token issue
 +------------+----------------------------------+
 | Field | Value |
 +------------+----------------------------------+
 | expires | 2016-02-15T14:23:21Z |
 | id | 7b8b9c6347f54d4ca5f543704068a0bb |
 | project_id | d12bddf60e4649b2a2cf6a2cc7520d79 |
 | user_id | e543889c522c46018c6a8f3ff71c1859 |
 +------------+----------------------------------+

 $ glance md-namespace-show NS1001
 +------------+----------------------------------+
 | Property | Value |
 +------------+----------------------------------+
 | created_at | 2016-02-15T12:56:09Z |
 | namespace | NS1001 |
 | objects | ["ob1"] |
 | owner | d12bddf60e4649b2a2cf6a2cc7520d79 |
 | protected | False |
 | schema | /v2/schemas/metadefs/namespace |
 | updated_at | 2016-02-15T12:56:09Z |
 | visibility | public |
 +------------+----------------------------------+

Another project can update that namespace (eg with a new object):

 $ openstack token issue
 +------------+----------------------------------+
 | Field | Value |
 +------------+----------------------------------+
 | expires | 2016-02-15T14:25:09.152643Z |
 | id | 0df5acec2b884f3c8cff744b4c4f66d0 |
 | project_id | c4f1b829b3af4775abdc9d70059eac10 | <<<
 | user_id | 10f27b7f965a47f98a828e4b342c03fd |
 +------------+----------------------------------+

 $ glance md-object-create --name objectx --schema {} NS1001
 +------------+-----------------------------+
 | Property | Value |
 +------------+-----------------------------+
 | created_at | 2016-02-15T13:25:33Z |
 | name | objectx |
 | schema | /v2/schemas/metadefs/object |
 | updated_at | 2016-02-15T13:25:33Z |
 +------------+-----------------------------+

This seems to also be possible if the namespace is owned by 'admin':

  <as regular user, add an object to an admin owned namespace>

 $ glance md-object-create --name objectx --schema {} OS::Compute::GuestMemoryBacking
 +------------+-----------------------------+
 | Property | Value |
 +------------+-----------------------------+
 | created_at | 2016-02-15T13:28:11Z |
 | name | objectx |
 | schema | /v2/schemas/metadefs/object |
 | updated_at | 2016-02-15T13:28:11Z |
 +------------+-----------------------------+

 $ glance md-namespace-show OS::Compute::GuestMemoryBacking
 +----------------------------+----------------------------------------------------------------------------------+
 | Property | Value |
 +----------------------------+----------------------------------------------------------------------------------+
 | created_at | 2016-02-08T13:37:48Z |
 | description | This provides the preferred backing option for guest RAM. Guest's memory can be |
 | | backed by hugepages to limit TLB lookups. See also: |
 | | https://wiki.openstack.org/wiki/VirtDriverGuestCPUMemoryPlacement |
 | display_name | Guest Memory Backing |
 | namespace | OS::Compute::GuestMemoryBacking |
 | objects | ["objectx"] |
 | owner | admin |
 | properties | ["mem_page_size"] |
 | protected | True |
 | resource_type_associations | ["OS::Glance::Image", "OS::Cinder::Volume", "OS::Nova::Flavor"] |
 | schema | /v2/schemas/metadefs/namespace |
 | visibility | public |
 +----------------------------+----------------------------------------------------------------------------------+

 <as regular user, add a property to an admin owned namespace>

 $ glance md-property-create --name propx --title title1 --schema '{"description": "x", "type":"string"}' OS::Compute::GuestMemoryBacking
 +-------------+--------+
 | Property | Value |
 +-------------+--------+
 | description | x |
 | name | propx |
 | title | title1 |
 | type | string |
 +-------------+--------+

In contrast, updating a private namespace is forbidden:

 $ glance md-object-create --name objectx --schema {} NS1003
 403 Forbidden: Forbidding request, metadata definition namespace=NS1003 is not visible. (HTTP 403)

I'm assuming that public namespaces are intended to be public in a read-only sense (like images).

[Tristan Cacqueray (tristan-cacqueray)]

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.

[Tristan Cacqueray (tristan-cacqueray)]

Even if this report has been public only for a few seconds, it is enough time for launchpad to send notifications to all subscribers of glance bug, so there is no more real value to keep this private.

 What kind of damage an user can cause by adding properties to an admin owned namespace ? Is there a quota for those properties ?

[Stuart McLaren (stuart-mclaren)]

Adding Travis here too.

@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?

[Travis Tripp (travis-tripp)]

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.

[Tristan Cacqueray (tristan-cacqueray)]

Unless this can cause disruption for other tenants using 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 ?

[Stuart McLaren (stuart-mclaren)]

> Any concerns if I remove the privacy setting and close the OSSA ?

I know I fat fingered setting this to public when I entered it, so there may be little grounds for saying it should stay private.

It's (hopefully) reasonably obvious that restricting the policies can workaround this in the short term.

> Unless this can cause disruption for other tenants using metadef

It causes disruption in the sense that if I set a metadef to be public it can then be modified by other tenants.

Compare images: if I set an image to be public, it can't be modified by other tenants.

Like Travis, I'd like to see the metadef APIs have more restrictive default policies.
That would work around this issue for example.

I agree with Travis that this makes sense as a default:

"add_metadef_namespace":"rule:admin_required"

I'm less sure about the 'rule:owner_or_public_and_not_protected' ones though, eg:

"modify_metadef_tag":"rule:owner_or_(public_and_not_protected)"

My guess is that users would expect public metadefs to be unmodifiable by others in the same way as public images.

I'd lean towards a default of:

"modify_metadef_tag":"rule:owner"

But then I'm not as familiar with this stuff as Travis.

@Travis is there a killer use case for all users being able to modify anyone's public metadefs by default?

[Travis Tripp (travis-tripp)]

@Stuart, rule owner would probably handle most of the issue, but the one thing that I am a little hesitant about is how sharing a set of tags or properties could work across projects or at least a hierarchy of projects. For example in the case of software tags or properties, i could see the namespace being owned by a parent project, but not requiring asking the parent project to always add new tags. That's why the protected idea came to mind. The idea with tags is to make it so that people have a shared set of tags across a project or multiple projects, but possibly not require a single owner to approve all new tags.

I'm sitting in the horizon mid-cycle meeting with a lot of people talking, so perhaps I'm not able to fully concentrate on this at the moment. Maybe I'm overthinking this?

[Stuart McLaren (stuart-mclaren)]

@Travis

Ok, thanks for giving me an idea of where you're coming from.

> The idea with tags is to make it so that people have a shared set of tags across a project or multiple projects, but possibly not require a single owner to approve all new tags.

So I'm reading that as, you've got namespace X, and services such as Glance/Cinder/Nova may all have something they want to set in that namespace. And you don't want to have the Nova admin have to e-mail the admin who owns the namespace (so 90s), you want them to use the API. (Am I close?)

If so, would this work?

"modify_metadef_tag":"rule:owner or admin"

'admin' would be the role the Nova/Glance/Cinder folks who you are happy to have update your namespaces would need. (A site may want something finer grained than 'admin', such as a specific metadef role or set of roles.)

[Travis Tripp (travis-tripp)]

Stuart, There are some nuances to this whole scenario that are tricky. From a pure security perspective, owner is better than owner or admin. Because I thought that under v2 keystone you are an admin no matter what project you are an admin for. But if we can assert that in general you trust other admins to do the right thing, then this rule works and isn't that different than how most projects treat admin. The only reason I brought up the protected thing (which does deviate from images and therefore is a point of confusion) is that I may own a set of properties and whether or not I actually want to let regular users from other projects add tags or properties may not always be a static rule and you could control spamming via the protected. However, since we don't have sharing like images, this might just be a bad idea. This starts getting more theoretical though, so perhaps it is better to go with a sensical, simpler rule now and owner or admin makes quite a bit of sense.

[Jeremy Stanley (fungi)]

On the public vs. private front, please be aware that as soon as you set a bug public, Launchpad will send a notification with bug description and other details to anyone who has chosen to subscribe to that project's bugs. In the case of Glance, it looks like that count is currently 222 potentially untrusted parties who received information on this bug. As such, it's not safe to assume this is just a "brief window of visibility."

[Tristan Cacqueray (tristan-cacqueray)]

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

[Tristan Cacqueray (tristan-cacqueray)]

Any progess on that issue ? Based on the other, similar, bug report, I've switched the OSSA task to Opinion.

[Jeremy Stanley (fungi)]

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.

[Jeremy Stanley (fungi)]

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