DB access to show volumes may not be properly controlled

[Jay Bryant]

This bug was opened to note the fact that a user can show details for a volume they don't own in the case that they had the UUID of the volume: https://launchpad.net/bugs/1475422 This was recreated in the following manner:

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.

This problem can be circumvented by limiting the policy to 'rule:admin_or_owner' but we should investigate if there is a way to avoid this happening at the DB API level.

[OpenStack Infra (hudson-openstack)]

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

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

Hi, guys. I am working on the bug. And there are many ways to solve the bug. The first way is https://review.openstack.org/#/c/206450/1/cinder/volume/api.py . Pros: Easy to implement. Only one database connection. Cons: Just a workaround. The second way is https://review.openstack.org/#/c/206450/2/cinder/volume/api.py . Pros: The code seems to be better. Cons: It needs extra database connection. Any suggestion for solve the issue ?

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

I list some solutions for the bug. https://gist.github.com/ya790206/ea5e6b980ae1a472411a

[Scott DAngelo (scott-dangelo)]

jelly, we discussed this at the mid-cycle. Your latest patch (#2) looks fine to everyone. Please remove the WIP and fix the commit message issue the JayBryant has pointed out and people will continue to review.

[OpenStack Infra (hudson-openstack)]

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

commit b7c7bb8f951d6d5155f70887282ddb9e4fd2a3fb
Author: Kuo-tung Kao <email address hidden>
Date: Tue Jul 28 17:44:57 2015 +0800

    Don't use context.elevated to get volume

    Original Problem:
    =================
    Since the metadata(readonly and attached_mode) is stored in admin metadata,
    normal user need `run context.elevated` to retrieves admin metadata.
    The above way will bring a side effect. Normal user can also get
    any volume which the user shouldn't access when the user knows the UUID.

    Solution:
    =========
    Use context instead of context.elevated to get volume.
    And add admin metadata to it.
    Based on cinder-meetup-summer-2015 conclusion, we use the solution.
    The solution will need extra database connection.

    Change-Id: I06f21e7578b65a59c0fe4d3afe0e882ed73c4725
    Closes-Bug: #1477625