Project admin gets treated as de-facto Admin

[Erno Kuvaja]

If an user is granted admin in a project it gets treated as de-facto admin in Cinder.

stack@ubnt-devstack:~/devstack$ openstack project create --enable priv-test
+-------------+----------------------------------+
| Field | Value |
+-------------+----------------------------------+
| description | |
| domain_id | default |
| enabled | True |
| id | c377dc4721c244429e49a52245bc4b35 |
| is_domain | False |
| name | priv-test |
| options | {} |
| parent_id | default |
| tags | [] |
+-------------+----------------------------------+
stack@ubnt-devstack:~/devstack$ openstack user create --project priv-test --password <SNIP> --email <email address hidden> --ignore-change-password-upon-first-use --disable-multi-factor-auth --enable privtest
+---------------------+-------------------------------------------------------------------------------------+
| Field | Value |
+---------------------+-------------------------------------------------------------------------------------+
| default_project_id | c377dc4721c244429e49a52245bc4b35 |
| domain_id | default |
| email | <email address hidden> |
| enabled | True |
| id | 014365b2e2794a35afa0110122ed8755 |
| name | privtest |
| options | {'ignore_change_password_upon_first_use': True, 'multi_factor_auth_enabled': False} |
| password_expires_at | None |
+---------------------+-------------------------------------------------------------------------------------+
stack@ubnt-devstack:~/devstack$ openstack role add --project priv-test --user privtest admin

stack@ubnt-devstack:~/devstack$ openstack role assignment list --names
+---------------------------+-----------------------------+-------------------+----------------------------+---------+--------+-----------+
| Role | User | Group | Project | Domain | System | Inherited |
+---------------------------+-----------------------------+-------------------+----------------------------+---------+--------+-----------+
| admin | privtest@Default | | priv-test@Default | | | False |
| service | designate@Default | | service@Default | | | False |
| admin | placement@Default | | service@Default | | | False |
| service | placement@Default | | service@Default | | | False |
| audit | project_a_auditor@Default | | project_a@Default | | | False |
| admin | | admins@Default | admin@Default | | | False |
| service | neutron@Default | | service@Default | | | False |
| creator | project_a_creator_2@Default | | project_a@Default | | | False |
| key-manager:service-admin | service-admin@Default | | service@Default | | | False |
| anotherrole | demo@Default | | demo@Default | | | False |
| member | demo@Default | | demo@Default | | | False |
| member | demo@Default | | invisible_to_admin@Default | | | False |
| admin | barbican@Default | | service@Default | | | False |
| creator | project_b_creator@Default | | project_b@Default | | | False |
| anotherrole | | nonadmins@Default | alt_demo@Default | | | False |
| member | | nonadmins@Default | alt_demo@Default | | | False |
| anotherrole | | nonadmins@Default | demo@Default | | | False |
| member | | nonadmins@Default | demo@Default | | | False |
| admin | project_a_admin@Default | | project_a@Default | | | False |
| service | glance@Default | | service@Default | | | False |
| observer | project_a_observer@Default | | project_a@Default | | | False |
| admin | project_b_admin@Default | | project_b@Default | | | False |
| observer | project_b_observer@Default | | project_b@Default | | | False |
| anotherrole | alt_demo@Default | | alt_demo@Default | | | False |
| member | alt_demo@Default | | alt_demo@Default | | | False |
| service | cinder@Default | | service@Default | | | False |
| admin | nova@Default | | service@Default | | | False |
| service | nova@Default | | service@Default | | | False |
| creator | project_a_creator@Default | | project_a@Default | | | False |
| audit | project_b_auditor@Default | | project_b@Default | | | False |
| admin | admin@Default | | alt_demo@Default | | | False |
| admin | admin@Default | | demo@Default | | | False |
| admin | admin@Default | | admin@Default | | | False |
| admin | admin@Default | | | Default | | False |
| admin | admin@Default | | | | all | False |
+---------------------------+-----------------------------+-------------------+----------------------------+---------+--------+-----------+
NOTE that only role privtest has is admin in priv-test@Default project.

stack@ubnt-devstack:~/devstack$ openstack user show demo
+---------------------+----------------------------------+
| Field | Value |
+---------------------+----------------------------------+
| domain_id | default |
| email | <email address hidden> |
| enabled | True |
| id | 5ef91cd7e9a946e0b37fd9db3111955a |
| name | demo |
| options | {} |
| password_expires_at | None |
+---------------------+----------------------------------+

stack@ubnt-devstack:~/devstack$ openstack project show demo
+-------------+----------------------------------+
| Field | Value |
+-------------+----------------------------------+
| description | |
| domain_id | default |
| enabled | True |
| id | 46117ada64914e75939330ddfa14af6c |
| is_domain | False |
| name | demo |
| options | {} |
| parent_id | default |
| tags | [] |
+-------------+----------------------------------+

stack@ubnt-devstack:~/devstack$ openstack volume create --image b1edae2e-8a84-4c49-8034-afc9f6845505 --size 2 --availability-zone nova test-volume
+---------------------+--------------------------------------+
| Field | Value |
+---------------------+--------------------------------------+
| attachments | [] |
| availability_zone | nova |
| bootable | false |
| consistencygroup_id | None |
| created_at | 2021-06-23T11:29:43.401592 |
| description | None |
| encrypted | False |
| id | a6762551-8e4b-4b1b-ac1a-6d3303509e6c |
| multiattach | False |
| name | test-volume |
| properties | |
| replication_status | None |
| size | 2 |
| snapshot_id | None |
| source_volid | None |
| status | creating |
| type | lvmdriver-1 |
| updated_at | None |
| user_id | 5ef91cd7e9a946e0b37fd9db3111955a |
+---------------------+--------------------------------------+

stack@ubnt-devstack:~/devstack$ env | grep OS_
OS_REGION_NAME=RegionOne
OS_PROJECT_DOMAIN_ID=default
OS_CACERT=
OS_AUTH_URL=http://172.24.1.39/identity
OS_TENANT_NAME=priv-test
OS_USER_DOMAIN_ID=default
OS_USERNAME=privtest
OS_VOLUME_API_VERSION=3
OS_AUTH_TYPE=password
OS_PROJECT_NAME=priv-test
OS_PASSWORD=<SNIP>
OS_IDENTITY_API_VERSION=3

stack@ubnt-devstack:~/devstack$ openstack volume set a6762551-8e4b-4b1b-ac1a-6d3303509e6c --size 4
stack@ubnt-devstack:~/devstack$

stack@ubnt-devstack:~/devstack$ cinder show a6762551-8e4b-4b1b-ac1a-6d3303509e6c
+--------------------------------+--------------------------------------------------------------------+
| Property | Value |
+--------------------------------+--------------------------------------------------------------------+
| attached_servers | [] |
| attachment_ids | [] |
| availability_zone | nova |
| bootable | true |
| consistencygroup_id | None |
| created_at | 2021-06-23T11:29:43.000000 |
| description | None |
| encrypted | False |
| id | a6762551-8e4b-4b1b-ac1a-6d3303509e6c |
| metadata | |
| migration_status | None |
| multiattach | False |
| name | test-volume |
| os-vol-host-attr:host | ubnt-devstack@lvmdriver-1#lvmdriver-1 |
| os-vol-mig-status-attr:migstat | None |
| os-vol-mig-status-attr:name_id | None |
| os-vol-tenant-attr:tenant_id | 46117ada64914e75939330ddfa14af6c |
| replication_status | None |
| size | 4 |
| snapshot_id | None |
| source_volid | None |
| status | available |
| updated_at | 2021-06-23T11:33:12.000000 |
| user_id | 5ef91cd7e9a946e0b37fd9db3111955a |
| volume_image_metadata | checksum : b874c39491a2377b8490f5f1e89761a4 |
| | container_format : bare |
| | disk_format : qcow2 |
| | hw_rng_model : virtio |
| | image_id : b1edae2e-8a84-4c49-8034-afc9f6845505 |
| | image_name : cirros-0.5.2-x86_64-disk |
| | min_disk : 0 |
| | min_ram : 0 |
| | owner_specified.openstack.md5 : |
| | owner_specified.openstack.object : images/cirros-0.5.2-x86_64-disk |
| | owner_specified.openstack.sha256 : |
| | signature_verified : False |
| | size : 16300544 |
| volume_type | lvmdriver-1 |
+--------------------------------+--------------------------------------------------------------------+

[Brian Rosmaita (brian-rosmaita)]

Rejecting this as a bug because Cinder doesn't support scoped tokens yet. So the only way to be a cinder administrator is to have the 'admin' role. We suggest that people not give that role out prematurely to people who are not intended to be full administrators of cinder.

We're working on implementing scoped token recognition in Xena so that "personas" such as 'project admin' can be a thing. But at this point, it's not.

[Rajat Dhasmana (whoami-rajat)]

I think the concept of cinder admin will be applicable when keystone supports system scope: cinder (instead of 'all' as of now)
Agree with Brian that we currently don't have project/system scope support but also which operation signifies that user privtest is a cinder admin?

[Jeremy Stanley (fungi)]

I've added a Won't Fix security advisory task for now to indicate that there's likely no need to publish one about this, given the feedback from Cinder's security reviewers. Given that, it looks like this should be safe to switch to public as well, but I'll give Erno an opportunity to reply before doing so.

[Erno Kuvaja (jokke)]

Like the Keystone bug this is related to https://bugs.launchpad.net/keystone/+bug/968696 which was marked "Fix Released" in Cinder in Liberty and same in Keystone in Train. Thus I'd like to argue that either this is a new issue or regression considering the original bug was closed as fixed.

Personally I think the main issue here is that all the identity documentation is explaining the roles as if they were enforced and safe to use. With note that different projects treating the roles bit differently while the implementation is worked on. I've yet to see a single proper warning that "Hey it's not like you might not have admin rights because the scopes are not enforced, but you might give out global admin rights because the scope is not enforced". Does Cinder actually document somehwere that any admin role gets treated as de-facto admin in Cinder?

[Brian Rosmaita (brian-rosmaita)]

I'll address Erno's points in reverse order.

> Does Cinder actually document somehwere that any admin role gets treated as de-facto admin in Cinder?

We do, but I appreciate Erno bringing this up, because it would be good for the VMT to decide whether our docs are clear enough. We have this documented in at least 2 places:

(1) "Policy Configuration"
https://docs.openstack.org/cinder/latest/configuration/block-storage/policy.html
(This is a generated file.)
The very first item is:
rule: context_is_admin
default: role_admin
description: Decides what is required for the "is_admin:True" check to succeed

Then, two items down:
rule: admin_api
default: is_admin:True or ( [0] )
description: Default rule for most Admin APIs.

[0] this is added by https://review.opendev.org/c/openstack/cinder/+/384642, which is the "Fix Released" in Bug #968696. In what sense that's a fix is a good question. I believe it was or'd so it wouldn't break existing deployments, and then people were supposed to move away from relying on context_is_admin and could adjust the policy file.

Because the file is generated and will be changing during Xena development, here's a permalink to the relevant source code for the above:
https://opendev.org/openstack/cinder/src/commit/f340058145f9473b6cf2694e0620c2ca8964b728/cinder/policies/base.py#L79-L89

(2) "Policy configuration HowTo"
in particular, the section "Pre-Defined Policy Rules"
https://docs.openstack.org/cinder/latest/configuration/block-storage/policy-config-HOWTO.html#pre-defined-policy-rules
Gives an explanation of "context-is-admin".
Also note the introductory paragraphs of the "Example: Configuring a Read-Only Administrator" section (up to and including the "Warning" box).

[Brian Rosmaita (brian-rosmaita)]

> Personally I think the main issue here is that all the identity documentation is explaining the roles as if they were enforced and safe to use.

Erno raises another good point here. It would be good for the VMT to review the Keystone docs to determine the extent to which they explain what can be done with Keystone, vs. what the actual situation is.

A big problem is that the oslo.policy config options enabling the scoped "personas" ('enforce_scope' and 'enforce_new_defaults') must be set in *each* service's conf file. If an operator has a "mixed" deployment (some services respecting scope, some not), you get the situation described in this bug.

We've got at least 3 types of policy configurations possible now:
(1) Legacy, relying on 'roles' only (roughly, current Cinder)
(2) Legacy + reliance on is_admin_project (not sure whether most services use this exclusively, or are still using transition rules like Cinder is)
(3) "Consistent and Secure RBAC" ('enforce_scope' + 'enforce_new_defaults' -- in progress in most services)

(1) + (3) is a really bad combination. (2) + (3) is maybe a bit better, though probably not. The key point is that coordination across services in a deployment is very important, and hopefully that comes across in the OpenStack docs somewhere.

[Gage Hugo (gagehugo)]

I looked over the keystone docs and did some google'ing and I did not find anywhere that keystone specifically calls out to enable "enforce_scope" to avoid this outside of the releasenotes, so this is definitely an area for improvement.

I imagine a warning or note could be added to the roles documentation in keystone, perhaps here[0].

This may need to be propagated across all the services documentation as well since it needs to be configured in each service's conf.

[0] https://docs.openstack.org/keystone/latest/admin/service-api-protection.html

[Gage Hugo (gagehugo)]

On the VMT level, I don't see a reason for this bug to be private anymore. This is a known issue that is currently being worked on (but could use some documentation improvements).

[Brian Rosmaita (brian-rosmaita)]

I agree with Gage about making this public.

[Brian Rosmaita (brian-rosmaita)]

This has been sitting for a week, so I think everyone is on board with Jeremy and Gage's suggestion that it doesn't need to be private. Making it public so the issue can be discussed in connection with the cinder xena spec: https://review.opendev.org/c/openstack/cinder-specs/+/797655

[Jeremy Stanley (fungi)]

After discussing, the Vulnerability Management Team members have concluded that the in-progress but incomplete RBAC implementation in various projects does not rise to the level of requiring a published security advisory, particularly as this work is likely to take place primarily in development branches and not be backported to supported stable branches. Some clearer documentation on behalf of the implementing projects is likely warranted in order to warn users of the caveats and potential pitfalls of relying on RBAC in its current state, but that's separate from whether or not we publish advisories about any fixes which may merge to complete the implementation.