attachment-show is including `connection_info` for non-admin callers, it shouldn't

Bug #1736773 reported by John Griffith on 2017-12-06
262
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Cinder
High
Rajat Dhasmana
OpenStack Security Advisory
Undecided
Unassigned

Bug Description

The V3 attachment API's include storing of the connection info of an attachment in the attachment-specs table. So the connection info no longer needs to be managed/persisted on the Nova side, instead an `attachment-show` will retrieve this data.

That's cool, but the problem is that currently that info is included for non-admin callers which is no good, that needs to be fixed up and hidden from normal users.

John Griffith (john-griffith) wrote :
Download full text (3.2 KiB)

Details of the call:

ubuntu@os-1:~$ cinder attachment-show 7efd5656-dc84-4996-ae7f-323142986a85
+-------------+--------------------------------------+
| Property | Value |
+-------------+--------------------------------------+
| attach_mode | rw |
| attached_at | 2017-11-28T22:38:55.000000 |
| detached_at | |
| id | 7efd5656-dc84-4996-ae7f-323142986a85 |
| instance | 2d509dbc-8b62-49a9-b5fe-ad5d9c169bdd |
| status | attached |
| volume_id | b822fb57-b820-47ae-b97f-0c85ea7bcfa5 |
+-------------+--------------------------------------+
+--------------------+-----------------------------------------------------------------------+
| Property | Value |
+--------------------+-----------------------------------------------------------------------+
| access_mode | rw |
| attachment_id | 7efd5656-dc84-4996-ae7f-323142986a85 |
| auth_method | CHAP |
| auth_password | Aa8WDrNaZAYBN8sa |
| auth_username | C4W5WBxj3WgxvCaKMEvR |
| driver_volume_type | iscsi |
| encrypted | False |
| qos_specs | None |
| target_discovered | False |
| target_iqn | iqn.2010-10.org.openstack:volume-b822fb57-b820-47ae-b97f-0c85ea7bcfa5 |
| target_lun | 1 |
| target_portal | 10.117.36.28:3260 |
| volume_id | b822fb57-b820-47ae-b97f-0c85ea7bcfa5 |
+--------------------+-----------------------------------------------------------------------+

### Raw response below from the view-builder, note the connection_info key should be popped for non-admin callers

{'attachment': {'status': u'attached', 'detached_at': '', 'connection_info': {u'auth_password': u'Aa8WDrNaZAYBN8sa', u'attachment_id': u'7efd5656-dc84-4996-ae7f-323142986a85', u'target_discovered': False, u'encrypted': False, u'driver_volume_type': u'iscsi', u'qos_specs': None, u'target_iqn': u'iqn.2010-10.org.openstack:volume-b822fb57-b820-47ae-b97f-0c85ea7bcfa5', u'target_portal': u'10.117.36.28:3260', u'volume_id': u'b822fb57-b820-47ae-b97f-0c85ea7bcfa5', u'target_lun': 1, u'access_mode': u'rw', u'auth_username': u'C4W5WBxj3WgxvCaKMEvR', u'auth_method': u'CHAP'}, 'attached_at': datetime.datetime(2017, 11, 28, 22, 38, 55), 'attach_mode': u'rw', 'instance': '2d509dbc-8b62-49a9-b5fe-ad5d9c169bdd', 'volume_id': 'b822fb57-b820-47ae-b97f-0c85ea7bcfa5', 'id': '...

Read more...

Changed in cinder:
status: New → Triaged
importance: Undecided → High
assignee: nobody → John Griffith (john-griffith)
Matt Riedemann (mriedem) wrote :

We should probably mark this as a public security issue.

Jeremy Stanley (fungi) wrote :

Since this report concerns a possible security risk (based on IRC discussion in #openstack-cinder), 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.

Changed in ossa:
status: New → Incomplete
information type: Public → Public Security
Matt Riedemann (mriedem) wrote :

Note that this API was added in Ocata (Cinder v3.27). Nothing in OpenStack is using this API yet so no attachments would come back outside of testing (nova is working on using this in Queens). However, anything using Cinder standalone (k8s?) could be affected.

John Griffith (john-griffith) wrote :

The *good* news is that external consumers like K8's are using the old API's as well, and there's no facility in gophercloud (yet) to expose the mv to utilize this anyway. This should not be something that's consumed anywhere yet.

Matt Riedemann (mriedem) wrote :

@John, I got to thinking about this. How is this any different than the os-initialize_connection volume action API which we are using today (old style attach)? That returns the connection_info document in the REST API response and contains credentials just like we're seeing in the attachment list/get response. Both are scoped to a specific volume, and should be restricted to the owner (or admin) for that volume.

So if it's the same issue, this bug has always existed since Cinder was birthed from nova-volume, right? Or are there differences in policy or something on the Cinder side?

Also, if we make the attachments API only return the connection_info for an admin token, I'm wondering if that will break the new volume attach flow in Nova where we don't use an admin token, we use the compute API request user's token, i.e. the owner of the instance/volume. We'd then likely have to do something in nova like we do for some of the neutron API where we have to use an admin token to get port binding details.

Matt Riedemann (mriedem) wrote :

(10:07:52 AM) mriedem: looks like this is a policy check on updating an attachment https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L2075
(10:08:38 AM) mriedem: comparing to os-initialize_connection, there is a policy rule on that too https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L782
(10:08:52 AM) ildikov: ok, so what does it depend on whether or not to use admin context to get those details?
(10:09:02 AM) mriedem: https://github.com/openstack/cinder/blob/master/cinder/policies/volume_actions.py#L164
(10:09:30 AM) mriedem: same as https://github.com/openstack/cinder/blob/master/cinder/policies/attachments.py#L37
(10:09:44 AM) mriedem: but there isn't a policy rule for listing/showing attachment details
(10:09:52 AM) jungleboyj: Would be good to fix that with a policy setting so if there are people who want that exposed we can still get it.
(10:10:13 AM) ildikov: ah, ok, yeah
(10:10:18 AM) mriedem: ildikov: the issue is that i, as a non-admin user, can create a volume and attach it to an instance,
(10:10:30 AM) mriedem: and then get the details, as a non-admin, about the storage connection
(10:10:40 AM) mriedem: including target IP and credentials
(10:11:01 AM) mriedem: as i said, this has always been a problem with os-initialize_connection as far as i can tell
(10:11:28 AM) mriedem: the main difference is there was never a CLI for initializing a connection to get the connection_info back, but the REST API was always there for anyone that knows how to use curl
(10:11:42 AM) mriedem: there is a CLI for listing and showing volume attachment details

Jay Bryant (jsbryant) wrote :

One other wrinkle here is there are cases for standalone Cinder where you may want non-admin to be able to get this data. So, is there a way that this can be set with a policy?

Matt Riedemann (mriedem) wrote :
Download full text (3.2 KiB)

We talked about this at length in the nova/cinder cross-project meeting today:

http://eavesdrop.openstack.org/meetings/cinder_nova_api_changes/2017/cinder_nova_api_changes.2017-12-07-16.00.log.html#l-19

To try and summarize, there are 5 APIs in cinder with this issue:

1. The os-initialize_connection volume action API which is controlled by an admin_or_owner policy rule (not documented in the API reference).

2. The create attachment API (there is no policy rule by default for this):

https://developer.openstack.org/api-ref/block-storage/v3/#create-attachment

4. The attachment_update API (PUT /v3/{project_id}/attachments/{attachment_id}) which is controlled by an admin_or_owner policy rule.

https://developer.openstack.org/api-ref/block-storage/v3/#update-an-attachment

5. The show attachment API:

https://developer.openstack.org/api-ref/block-storage/v3/#show-attachment-details

6. The list attachments with details API:

https://developer.openstack.org/api-ref/block-storage/v3/#list-attachments-with-details

There are no direct policy checks on #5 or #6. If you're showing details about an attachment id, you have to be an admin or owner to lookup the record from the database, same for #6 - you can only list attachments that you are the owner if via the model query in the database.

Impacts to consumers:

If we added an admin-only by default policy rule to restrict showing the connection_info in the response, that would break any existing nova deployment that doesn't have the [cinder] section in nova.conf configured with a user that has the admin role. There is nothing documenting the need for an admin user for nova to talk to cinder today:

https://docs.openstack.org/cinder/latest/install/cinder-controller-install-ubuntu.html#configure-compute-to-use-block-storage

And devstack doesn't set that up either (for reference):

https://github.com/openstack-dev/devstack/blob/d37119e797d3140aeb0038a1129ce5e9016c1a36/lib/nova#L470-L476

If you compare that to nova's communication with neutron, nova indeed does require that it's configured with a user that has the admin role so it can use the networking service port binding API. It's less obvious, but you can infer from the networking install guide that is required:

a) https://docs.openstack.org/neutron/latest/install/controller-install-ubuntu.html

"Add the admin role to the neutron user"

b) https://docs.openstack.org/neutron/latest/install/compute-install-ubuntu.html#configure-the-compute-service-to-use-the-networking-service

^ tells you to configure nova.conf to use the neutron user, which is an admin user.

Given this, no deployments today require that the cinder section of nova.conf be configured with an admin user. Adding an admin-only policy rule to existing APIs that nova uses would break anyone that doesn't also configure nova to use an admin user for cinder.

Therefore, I think if policy rules are added to Cinder to mask the connection_info in the response for those APIs mentioned above, it will have to default to admin_or_owner to not break existing deployments, and then fully document the fact that for any deployment which wants to make that admin-only, they will also need to configur...

Read more...

Jeremy Stanley (fungi) wrote :

Given that analysis, it sounds highly likely we can't safely backport fixes to stable branches for such an issue and so would be a class B1 report per our report taxonomy: "A vulnerability that can only be fixed in master, security note for stable branches, e.g., default config value is insecure" https://security.openstack.org/vmt-process.html#incident-report-taxonomy

Does that sounds right?

Matt Riedemann (mriedem) wrote :

I think that's correct. I got to looking at the [cinder] config options for nova and today you can't configure a specific user like you can for some of the other services that nova talks to, like neutron. So even if we added an admin_or_owner by default policy rule to cinder, we'd have to add support to nova for configuring a specific user and backport that as well.

FWIW, https://review.openstack.org/#/c/522112/ gets us the ability in nova to configure [cinder] with specific user credentials like the other services. We would just have to prioritize that change, or split out the nova/conf/cinder.py changes in there.

ye (dakele) on 2019-04-01
no longer affects: ubuntu
Jeremy Stanley (fungi) wrote :

Marking the advisory task won't-fix since there were no objections to classifying as B1 (per comment #10).

Changed in ossa:
status: Incomplete → Won't Fix
Changed in cinder:
assignee: John Griffith (john-griffith) → Rajat Dhasmana (whoami-rajat)
status: Triaged → In Progress
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers