Non-admin user can perform 'extra-specs-list'

[Julia Varlamova]

In etc/manila/policy.json:

        "share_extension:types_extra_specs": "rule:admin_api"

But non-admin user can perform 'manila extra-specs-list' command. So, policy.json isn't used when we query for extra-specs list.

[Valeriy Ponomaryov (vponomaryov)]

John Griffith,

I have reproduced it with "master" branch of Cinder on my lab.

In "/etc/cinder/policy.json" I have a rule:

"volume_extension:types_extra_specs": "rule:admin_api",

Enabling user creds in CLI command "cinder extra-specs-list" returns 200 Ok.

Here is debug, I set "user_id" to "fake" value and it still works:

"""

> /usr/local/lib/python2.7/dist-packages/oslo_policy/policy.py(501)enforce()
-> if do_raise and not result:
(Pdb) result
True
(Pdb) do_raise
True
(Pdb) self.rules[rule](target, creds, self)
True
(Pdb) creds
{'domain': None, 'project_name': u'demo', 'project_domain': None, 'timestamp': '2015-07-17T09:07:36.375081', 'auth_token': '34ec89889d0f496db0ac56568774c2ac', 'remote_address': '172.18.198.52', 'quota_class': None, 'resource_uuid': None, 'is_admin': False, 'user': u'05aa9930de444c91a937b312ae1249bf', 'service_catalog': [{u'endpoints': [{u'adminURL': u'http://172.18.198.52:35357/v2.0', u'region': u'RegionOne', u'internalURL': u'http://172.18.198.52:5000/v2.0', u'publicURL': u'http://172.18.198.52:5000/v2.0'}], u'type': u'identity', u'name': u'keystone'}, {u'endpoints': [{u'adminURL': u'http://172.18.198.52:8774/v2/c63ce66c72124c41abb9e6e9fc75e76a', u'region': u'RegionOne', u'internalURL': u'http://172.18.198.52:8774/v2/c63ce66c72124c41abb9e6e9fc75e76a', u'publicURL': u'http://172.18.198.52:8774/v2/c63ce66c72124c41abb9e6e9fc75e76a'}], u'type': u'compute', u'name': u'nova'}], 'tenant': u'c63ce66c72124c41abb9e6e9fc75e76a', 'read_only': False, 'project_id': u'c63ce66c72124c41abb9e6e9fc75e76a', 'user_id': u'05aa9930de444c91a937b312ae1249bf', 'show_deleted': False, 'roles': [u'Member', u'anotherrole'], 'user_identity': '05aa9930de444c91a937b312ae1249bf c63ce66c72124c41abb9e6e9fc75e76a - - -', 'read_deleted': 'no', 'request_id': 'req-03d03509-771a-45d2-989e-b63fbc9a45bf', 'user_domain': None}
(Pdb) target
{'project_id': u'c63ce66c72124c41abb9e6e9fc75e76a', 'user_id': u'05aa9930de444c91a937b312ae1249bf'}
(Pdb) rule
'volume_extension:volume_type_access'
(Pdb) target
{'project_id': u'c63ce66c72124c41abb9e6e9fc75e76a', 'user_id': u'05aa9930de444c91a937b312ae1249bf'}
(Pdb) target["user_id"] = "5791f97019d04291a8064de84dc0eecb"
(Pdb) self.rules[rule](target, creds, self)
True
(Pdb) target["user_id"] = "fake"
(Pdb) self.rules[rule](target, creds, self)
True
(Pdb) str(self.rules["volume_extension:types_extra_specs"])
'rule:admin_api'

"""

But policy works ok for another rule "volume_extension:types_manage" where I get expected 403 code.

So, I set here confirmed until it is proven that bug does not exist.

[John Griffith (john-griffith)]

@Valeriy
Oops, you're absolutely correct. I marked this as invalid for Cinder because at first glance I thought you were pointing out that Manilla was elevating the credentials in the call to the client, and ignoring the policy.

My bad, marking this as verified.

[Kuo-tung Kao (jelly) (coding1314)]

This is not a bug. When you said "cinder extra-specs-list" , it just means type-list with different format. `cinder extra-specs-list` and `cinder type-list` invoke the same api, "/v1/43559c44ec8c4f899f55568b1d34725e/types". By the way, it seems that there are no command to show the result about "types_extra_specs".

[Valeriy Ponomaryov (vponomaryov)]

Jelly, this is exactly a bug, because it is expected to be able to change policy for this API, and only this API not in group with some another API. And now admin want to change it and can not.

[Xing Yang (xing-yang)]

In Cinder, this is a duplicate of https://bugs.launchpad.net/cinder/+bug/1351971.

Winston has submitted a patch for it a while back:
https://review.openstack.org/#/c/150046/

[Valeriy Ponomaryov (vponomaryov)]

Xing,

patch-set 3 in https://review.openstack.org/#/c/150046/ does not fix described bug.

Fix will be proper when we can influence extra specs API behaviour by changing policy file.

[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)]

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

[Xing Yang (xing-yang)]

Clinton,

Please be aware of the following bug when fixing this. Thanks.

https://bugs.launchpad.net/cinder/+bug/1495764

[Clinton Knight (clintonk)]

Thanks, Xing. I tested the non-admin scenario, and the capabilities scheduler works fine with both public and private extra specs.

And I think that makes sense for 2 reasons. Our fix for this issue is to change the view builder, not the database layer. And because Manila's create code always elevates the context to admin internally before reading the share type & extra specs, all info would always be available to the CapabilitiesFilter.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/222738
Committed: https://git.openstack.org/cgit/openstack/manila/commit/?id=20a0b6bce0946fc09d98c84771a690e414115e43
Submitter: Jenkins
Branch: master

commit 20a0b6bce0946fc09d98c84771a690e414115e43
Author: Clinton Knight <email address hidden>
Date: Tue Sep 8 18:47:18 2015 -0400

    Non-admin user can perform 'extra-specs-list'

    This bug, inherited from Cinder, allows a tenant to view share
    extra specs using the extra-specs-list CLI command. The Cinder
    fix was to check the admin context in the DB layer and filter out
    all extra specs for non-admins. This approach doesn't work for
    Manila, because some extra specs are required and are effectively
    part of the Manila API (DHSS, snapshot_support). So in Manila we
    define a set of tenant-visible extra specs and restrict the extra
    spec values to that set in the share type view builder. Also, we
    add policies for the share type list APIs so that admins can
    control access to those if desired.

    The separate API to list extra specs already has adequate checking
    for non-admin users; the CLI was listing the extra specs returned
    by the share type API, which is now filtered as described.

    Co-Authored-By: Andrew Kerr <email address hidden>
    Change-Id: I9b0a8ddc064c246286f26760b703db6e3e1bcd46
    Closes-Bug: #1475285