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

[John Griffith]

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

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': '...

[Matt Riedemann (mriedem)]

We should probably mark this as a public security issue.

[Jeremy Stanley (fungi)]

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.

[Matt Riedemann (mriedem)]

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

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

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

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

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

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...

[Jeremy Stanley (fungi)]

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

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.

[Jeremy Stanley (fungi)]

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

[Gorka Eguileor (gorka)]

From cinder's point of view this is not a security risk, because the information provided can only contain user specific credentials.

For drivers using system wide credentials (ie. Ceph) these must be provided off-band.

We understand a sysadmin may still not want to expose the internal network addressing to users and only allow Nova and Glance (when using Cinder as a backend) to access this information. For that reason the Cinder team will add a new attachment get policy for when Nova and Glance are able to use an admin user to make the requests or when we start using service roles.

It would be the responsibility of the deployment tool (or the sysadmin) to configure the different services to behave in the desired way.

Cinder backup may also need some changes to its code to support backups with restricted policies.