cinder/api/v<n>/types.get returns admin info without context check

[John Griffith]

Currently the types api module in Cinder grabs a specified volume_type directly from the volume_types module which does a context elevation. This means that the returned volume_type object that is then just fed directly into the general view_builder includes the extra_specs info which we've always considered "admin" data.

Example response from view_builder:
{'volume_type': {'extra_specs': {u'volume_backend_name': u'lvmdriver-1'}, 'name': u'lvmdriver-1', 'id': 'd30ce89f-57d0-43df-a70b-e75af1757357'}}

We should at the very least check the credentials of the caller in the api/v<n>/types module, and filter out the extra-specs for non-admin users. Someone may want to make a policy for this... but typically we don't want to allow things like volume_backend_name to be exposed to the end user.

This also exposes some sloppiness we've gotten into over time regarding calls directly to modules versus calls that go through cinder/volume/api.py. There are def cases where it seems silly to create a wrapper in volume/api, and at one point volume/api was in fact thought of specifically for volume api actions (that's changed quite a bit). Anyway, the point is I think we should discuss having a consistent design and at least an 'expected' sequence.

[Jordan Pittier (jordan-pittier)]

Other projects such as Nova (nova flavor-show XX) and Glance (glance image-show XX) publicly show extra_specs and properties.

So, I am not in favor of hiding extra_specs in Cinder. There's no secret information in info such as the backend name.

There's the same exact problem with "keystone endpoint-list" that is marked as 'admin only' but the information are publicly available in the return of (keystone --debug token-get).

[Duncan Thomas (duncan-thomas)]

I'm definitely *not* in favour of exposing extra specs. It can contain
backend info (name, vendor, driver) and should not normally exposed

Duncan Thomas
On Aug 5, 2014 11:25 AM, "Jordan Pittier" <email address hidden>
wrote:

> Other projects such as Nova (nova flavor-show XX) and Glance (glance
> image-show XX) publicly show extra_specs and properties.
>
> So, I am not in favor of hiding extra_specs in Cinder. There's no secret
> information in info such as the backend name.
>
> There's the same exact problem with "keystone endpoint-list" that is
> marked as 'admin only' but the information are publicly available in the
> return of (keystone --debug token-get).
>
> --
> You received this bug notification because you are a member of Cinder
> Bug Team, which is subscribed to Cinder.
> https://bugs.launchpad.net/bugs/1351971
>
> Title:
> cinder/api/v<n>/types.get returns admin info without context check
>
> Status in Cinder:
> Triaged
>
> Bug description:
> Currently the types api module in Cinder grabs a specified volume_type
> directly from the volume_types module which does a context elevation.
> This means that the returned volume_type object that is then just fed
> directly into the general view_builder includes the extra_specs info
> which we've always considered "admin" data.
>
> Example response from view_builder:
> {'volume_type': {'extra_specs': {u'volume_backend_name':
> u'lvmdriver-1'}, 'name': u'lvmdriver-1', 'id':
> 'd30ce89f-57d0-43df-a70b-e75af1757357'}}
>
>
> We should at the very least check the credentials of the caller in the
> api/v<n>/types module, and filter out the extra-specs for non-admin users.
> Someone may want to make a policy for this... but typically we don't want
> to allow things like volume_backend_name to be exposed to the end user.
>
> This also exposes some sloppiness we've gotten into over time
> regarding calls directly to modules versus calls that go through
> cinder/volume/api.py. There are def cases where it seems silly to
> create a wrapper in volume/api, and at one point volume/api was in
> fact thought of specifically for volume api actions (that's changed
> quite a bit). Anyway, the point is I think we should discuss having a
> consistent design and at least an 'expected' sequence.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/cinder/+bug/1351971/+subscriptions
>

[Rushi Agrawal (rushiagr)]

I'm too in not a favour of exposing extra specs. In a public cloud scenario, one might not want to tell what backend the cloud provider is using, etc..

[Xing Yang (xing-yang)]

There's another bug submitted for this: https://bugs.launchpad.net/cinder/+bug/1475285

Winston,
Are you still working on this?

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/214731
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=0cfa9b62410ab74ce840c3b580318eae6ddfc804
Submitter: Jenkins
Branch: master

commit 0cfa9b62410ab74ce840c3b580318eae6ddfc804
Author: John Griffith <email address hidden>
Date: Wed Aug 19 17:42:40 2015 +0000

    Filter out extra-specs from type get for non-admin

    Currently when a get_volume type call is made to the
    db api, we don't check context and auto-fill the
    extra-specs info for that type.

    Extra specs are intended to be admin only info, but anybody
    that calls the API directly to list or get volume-types is
    also given this info which is not intended.

    This patch just adds a context check to the db api's
    private extra-specs builder method. In the case of
    non-admin, we just skip adding the extra-specs.

    Change-Id: Ia46695a6c890c06d94e6903ed9c54794107fa132
    Closes-Bug: #1351971
    Closes-Bug: #1475285

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sean McGinnis (<email address hidden>) on branch: master
Review: https://review.openstack.org/150046
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.