Neutron RBAC not working for multiple extensions

Bug #1784259 reported by Felipe Monteiro on 2018-07-30
260
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Security Advisory
Undecided
Unassigned
neutron
Undecided
Mykola Yakovliev

Bug Description

* Description *

Using QA automation as well as manual CLI validation, it appears to me as though many Neutron extensions aren't enforcing RBAC at all. This is because the extensions lack the "enforce_policy": True key/value pair in the extension resource definition code. Example: https://review.openstack.org/#/c/584217/2/neutron/extensions/subnet_service_types.py

This appears to be affecting many Neutron extensions.

* Pre-conditions *

1) Enable neutron-trunk plugin in local.conf by adding: enable_service neutron-trunk
2) Set create_trunk to "rule:admin_only" in /etc/neutron/policy.json, i.e.: "create_trunk": "rule:admin_only" or even "create_trunk": "!" which should deny absolutely everyone.
3) source ~/devstack/openrc demo demo or even source ~/devstack/openrc admin admin after setting "create_trunk" with the "!" rule.

* Reproduction Steps *

1) Run the following CLI commands:

- openstack network create test-network
- openstack port create --enable --network test-network test-port
- openstack network trunk create --parent-port test-port --enable --project demo test-trunk

* Expected Output *

Expected result: trunk creation fails with a 403 Unauthorized.

* Actual Output *

Observed result: trunk creation succeeds.

* Affected Plugins *
As far as I can tell:

- subnet service type
- subnet segment_id
- trunks
- trunk subports

Possibly many more.

* Outstanding Patches *

Outstanding patches that begin fixing these issues:

- https://review.openstack.org/#/c/584217/ (subnet service type RBAC fix)
- https://review.openstack.org/#/c/584601/ (subnet segment_id RBAC fix)

Validation patches that identify some of these issues:
- https://review.openstack.org/#/c/584424/
- https://review.openstack.org/#/c/582388/

Felipe Monteiro (fm577c) on 2018-07-30
description: updated
Felipe Monteiro (fm577c) on 2018-07-30
description: updated
description: updated
description: updated
Jeremy Stanley (fungi) wrote :

Since this report concerns a possible security risk, 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
description: updated
Jeremy Stanley (fungi) wrote :

The public changes in review at least hint pretty heavily at this reported vulnerability, so there's probably very little point in trying to keep it under wraps with an embargo.

Felipe Monteiro (fm577c) wrote :

@Jeremy I apologize. I should've been more prudent about it. I just stumbled into this and am now trying to course correct.

Jeremy Stanley (fungi) wrote :

No need to apologize. I wasn't trying to point out any wrongdoing, merely highlighting that the additional effort needed for embargoed discussion and development probably isn't warranted given the public nature of the linked reviews (but I'd like to see someone from neutron-coresec agree with that assertion first, so I haven't switched the report to public security yet).

Akihiro Motoki (amotoki) wrote :

According to my test results, this topic is split into two:
(a) when a user does not have the admin role, I cannot reproduce the issue
(b) when a user has the admin role, the policy check (at least when creating a resource) seems skipped.

(a) is the expected result, but (b) needs further investigation.

The following is the detail.

Regarding (a), I cannot reproduce the issue reported here.

After running DevStack with 'enable_service neutron-trunk', I did the following steps:
1) Edit /etc/neutron/policy.json to have "create_trunk": "rule:admin_only"
2) Restart neutron-api service to ensure the updated policy.json is reloaded. (service devstack@neutron-api restart)
3) Set OS_CLOUD envvar to devstack (which is equivalent to "openrc demo demo")
4) Run "openstack network trunk create --parent-port p1 trunk1"
5) I got the message:
---
rule:create_trunk is disallowed by policy
Neutron server returns request_ids: ['req-df278e9a-f449-4a4e-a16f-e03b063c283c']
---

Regarding (b), when a user has the admin role, it seems a network trunk can be created regardless of policy configuration
1) Edit /etc/neutron/policy.json to have "create_trunk": "!" (or "create_trunk": "role:nothing)
2) Restart neutron-api service to ensure the updated policy.json is reloaded. (service devstack@neutron-api restart)
3) Set OS_CLOUD envvar to devstack-admin (which is equivalent to "openrc admin admin")
4) Run "openstack network trunk create --parent-port p1 trunk1"
5) The trunk was created successfully. <--- This is different from what we expect

Felipe Monteiro (fm577c) wrote :

@Akihiro, will comment on (b). Will look into (a) more closely when time permits.

I believe this is due to the following code: https://github.com/openstack/neutron/blob/1779a86712d2eaf42e1089369632df90942b1763/neutron/policy.py#L410

Meaning that, with role: admin, context.is_admin == True, causing the function to return True, prior to oslo.policy authorization check.

Miguel Lavalle (minsel) wrote :

I agree that patches are already public, so I don't the cost of the embargo is warranted any more

Jeremy Stanley (fungi) wrote :

Thanks, Miguel. In that case I've switched the report to Public Security and ended the embargo so discussion can continue in the open.

description: updated
information type: Private Security → Public Security

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

Changed in neutron:
assignee: nobody → Mykola Yakovliev (vegasq)
status: New → In Progress

Change abandoned by Felipe Monteiro (<email address hidden>) on branch: master
Review: https://review.openstack.org/584217

Reviewed: https://review.openstack.org/598334
Committed: https://git.openstack.org/cgit/openstack/neutron-lib/commit/?id=d14b379d1c16d451f35d026ab83702ca631f2e4f
Submitter: Zuul
Branch: master

commit d14b379d1c16d451f35d026ab83702ca631f2e4f
Author: Mykola Yakovliev <email address hidden>
Date: Thu Aug 30 14:51:57 2018 -0500

    Let Neutron enforce rule on create_subnet with segment_id [neutron-lib part]

    Neutron ignores rule in policy file [0], that allows non-admin users
    to create subnets with segment_id.

    [0] https://github.com/openstack/neutron/blob/master/etc/policy.json#L19

    Change-Id: I313aadc53f728663fd774957c1bd92247d1513ca
    Partial-Bug: 1784259

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/584601
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

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