Non-admin users can get volumes in non-authorized tenants

Bug #1475422 reported by Tomoki Sekiyama
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
High
Liyingjun

Bug Description

When non-admin users know the volume uuid in the non-authorized tenant, they can get the volume information.

% OS_USERNAME=admin OS_TENANT_NAME=admin cinder list
+--------------------------------------+-----------+------+------+-------------+----------+-------------+-------------+
| ID | Status | Name | Size | Volume Type | Bootable | Multiattach | Attached to |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+-------------+
| 775fafb7-a2ee-497f-9b72-a5467f2cabd4 | available | a1 | 1 | lvmdriver-2 | false | False | |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+-------------+

% OS_USERNAME=demo OS_TENANT_NAME=admin cinder list
ERROR: User 3688045ce23b4859af1c4ede57d63d4d is unauthorized for tenant 0076ae66c26e4614b8de5d453289d2e5 (Disable debug mode to suppress these details.) (HTTP 401) (Request-ID: req-f293f1c8-0801-41b8-ae2a-c5a79ee2a43f)

% OS_USERNAME=demo cinder show 775fafb7-a2ee-497f-9b72-a5467f2cabd4
+---------------------------------------+--------------------------------------+
| Property | Value |
+---------------------------------------+--------------------------------------+
| attachments | [] |
| availability_zone | nova |
| bootable | false |
| consistencygroup_id | None |
| created_at | 2015-07-14T21:28:40.000000 |
| description | None |
| encrypted | False |
| id | 775fafb7-a2ee-497f-9b72-a5467f2cabd4 |
| metadata | {} |
| multiattach | False |
| name | a1 |
| os-vol-tenant-attr:tenant_id | 0076ae66c26e4614b8de5d453289d2e5 |
| os-volume-replication:driver_data | None |
| os-volume-replication:extended_status | None |
| replication_status | disabled |
| size | 1 |
| snapshot_id | None |
| source_volid | None |
| status | available |
| user_id | 030ccc6b1eb546598d8c13512b99ab97 |
| volume_type | lvmdriver-2 |
+---------------------------------------+--------------------------------------+

In this example, demo user can get info of the "a1" volume in the "admin" tenant
(tenant-id = 0076ae66c26e4614b8de5d453289d2e5) where demo user is not authorized to access.

Revision history for this message
Liyingjun (liyingjun) wrote :

This is caused no policy check for 'volume:get'[1].

[1]: http://git.openstack.org/cgit/openstack/cinder/tree/etc/cinder/policy.json#n10

Changed in cinder:
status: New → Confirmed
assignee: nobody → Liyingjun (liyingjun)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: Confirmed → In Progress
Jay Bryant (jsbryant)
Changed in cinder:
importance: Undecided → High
Revision history for this message
Cory Stone (corystone) wrote :

I think the db api is supposed to catch this.

Revision history for this message
John Griffith (john-griffith) wrote :

Sounds like we're on the same page, but I just want to make sure there's no intent to disable the policy file. In other words, enforcing policy check at DB layer (rather than elevating context always) is good, but just setting db access rules that don't take into account policy settings would be bad.

Revision history for this message
John Griffith (john-griffith) wrote :

Ahhh.. I JUST finally got the point here. Yes! Cory is right on Target here IMO.

To try and clarify:
Policy is used to say "this tenant can get his/her volumes" or "all tenant's can get their volumes" etc.

The point is however that Tenants should NEVER be able to get other tenants volumes. That's completely independent of the policy or that particular rule. I get it.

Yes, I agree Tenant-A should get a NotFound when attempting to do a get on Tenant-B's volume.

Revision history for this message
John Griffith (john-griffith) wrote :

@Liyingjun
So the real issue here is the elevated context on the get; the policy should just allow Tenants to get their volumes (or not); we need to fix up the db calls here.

I have an idea on how to address this and get it fixed up, let me know if you're still planning to work on this and need any help; otherwise feel free to assign it to me and i'll fix it up.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.openstack.org/202913
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=5a4f399d6c3c675afab5a1bd961db6a39e37ac1b
Submitter: Jenkins
Branch: master

commit 5a4f399d6c3c675afab5a1bd961db6a39e37ac1b
Author: liyingjun <email address hidden>
Date: Fri Jul 17 15:48:18 2015 +0800

    Set default policy for "volume:get"

    Currently, there is no policy check defined for "volume:get", so
    everyone can get another tenant's volume detail by UUID. It's necessary
    to set policy to "rule:admin_or_owner" for "volume:get" by default.

    Change-Id: Iefdf7e5703a28856b20d97a885267c01bed6bbb4
    Closes-bug: #1475422

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in cinder:
milestone: none → liberty-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in cinder:
milestone: liberty-3 → 7.0.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.