protected property change not rejected if a subsequent rule match accepts them

[Mark Washenberger]

See initial report here: http://lists.openstack.org/pipermail/openstack-dev/2014-January/024861.html

What is happening is that if there is a specific rule that would reject an action and a less specific rule that comes after that would accept the action, then the action is being accepted. It should be rejected.

This is because we iterate through the property protection rules rather than just finding the first match. This bug does not occur when policies are used to determine property protections, only when roles are used directly.

[Stuart McLaren (stuart-mclaren)]

This is a candidate for backporting to havana stable.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/68420
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=b6dd538569ebf0f1580c8e1fadc5e0f8054c9b08
Submitter: Jenkins
Branch: master

commit b6dd538569ebf0f1580c8e1fadc5e0f8054c9b08
Author: Thomas Leaman <email address hidden>
Date: Wed Jan 22 16:02:56 2014 +0000

    Check first matching rule for protected properties

    When using roles to define protected properties, the first matching rule
    in the config file should be used to grant/deny access. This change
    enforces that behaviour.

    Fixes bug 1271426

    Change-Id: I11ece25ae85ff868516bcd1839a4e430e9c51370

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/68952

[Grant Murphy (gmurphy)]

This seems like something that might catch out unsuspecting sysadmins. Do you think it is worth issuing an OSSN for this?

[Robert Clark (robert-clark)]

I'm happy to draft up an OSSN, I'll work with Stuart to get the details right.

-Rob

[Robert Clark (robert-clark)]

So the remediation step for implementations that are not backported or up to date is to order properties such that specific property rules should follow generic ones?

To use the example given here: http://lists.openstack.org/pipermail/openstack-dev/2014-January/024861.html if the foo_property settings were placed after the [.*] settings the problem would be avoided?

[Grant Murphy (gmurphy)]

@robert-clark That is my understanding.

[Robert Clark (robert-clark)]

Just finishing up the OSSN now, please accept my apologies for it taking so long, dropped off the radar for some reason.

How many versions back does this need to go?

[Nathan Kinder (nkinder)]

This has been published as OSSN-0013 to the mailing lists (openstack and openstack-dev), and the OpenStack wiki:

  https://wiki.openstack.org/wiki/OSSN/OSSN-0013

[Nathan Kinder (nkinder)]

Reopening this OSSN bug. The workaround in the OSSN has been reported to not work. Details from the reporter to come shortly.

[Mathieu Gagné (mgagne)]

The workaround proposed in OSSN-0013 does not work (tested against 2013.2):

"Users of affected releases should review and reorder the entries in property-protections.conf to place the most open permissions at the start of the configuration and more restrictive ones at the end, as demonstrated below."

The implementation (in 2013.2) will stop the processing of rules at the first positive (authorized) match. This means by placing the most generic rule first, all following rules will be ignored.

One possible workaround would be to replace the generic rule [.*] by a very complex regex crafted in a way to NOT match all the other rules protecting your properties.

As explained above, the faulty implementation WON'T stop processing the rules if a matching rule (property regex) is found. You therefore don't want a protected property to be authorized by a wildcard. Hopefully, it is made so if NO match is found, the property is still protected.

Example of such workaround:

[^provider_.*$]
create = admin
read = _member_
update = admin
delete = admin

# Note the trailing $ to make sure all the string is matched
[^((?!provider_).)*$]
create = _member_
read = _member_
update = _member_
delete = _member_

[Mathieu Gagné (mgagne)]

It should also be noted that the @ wildcard is not available in Havana.

[Mathieu Gagné (mgagne)]

admin role needs to be added to read permission to be able to update it. (In the case where your user does not have the _member_ role, only admin)

New example:

[^provider_.*$]
create = admin
read = admin,_member_
update = admin
delete = admin

# Note the trailing $ to make sure all the string is matched
[^((?!provider_).)*$]
create = _member_
read = _member_
update = _member_
delete = _member_

[Robert Clark (robert-clark)]

Thank you for catching this and providing guidance on the improved content.

I'll cut a new OSSN for this

[Nathan Kinder (nkinder)]

I have tested the new proposed workaround listed in comment #13 using the 2013.2 release (in devstack). The workaround is behaving correctly for me:

[nkinder@localhost devstack]$ source openrc admin admin
[nkinder@localhost devstack]$ glance image-create --property provider_foo=foo --name foo
+-------------------------+--------------------------------------+
| Property | Value |
+-------------------------+--------------------------------------+
| Property 'provider_foo' | foo |
| checksum | None |
| container_format | None |
| created_at | 2014-05-31T00:04:44 |
| deleted | False |
| deleted_at | None |
| disk_format | None |
| id | a8fbd50a-5871-433d-9097-5d3320339d98 |
| is_public | False |
| min_disk | 0 |
| min_ram | 0 |
| name | foo |
| owner | 6cad2594f37549798970593027b587b3 |
| protected | False |
| size | 0 |
| status | queued |
| updated_at | 2014-05-31T00:04:44 |
| virtual_size | None |
+-------------------------+--------------------------------------+

[nkinder@localhost devstack]$ source openrc demo demo
[nkinder@localhost devstack]$ glance image-create --property provider_bar=bar --name bar
403 Forbidden
Property 'provider_bar' is protected
    (HTTP 403)
[nkinder@localhost devstack]$ glance image-create --property junk_bar=bar --name bar
+---------------------+--------------------------------------+
| Property | Value |
+---------------------+--------------------------------------+
| Property 'junk_bar' | bar |
| checksum | None |
| container_format | None |
| created_at | 2014-05-31T00:05:46 |
| deleted | False |
| deleted_at | None |
| disk_format | None |
| id | 4ce3b1de-d5ad-46ac-acbe-2d0ed50d53c8 |
| is_public | False |
| min_disk | 0 |
| min_ram | 0 |
| name | bar |
| owner | 6cad2594f37549798970593027b587b3 |
| protected | False |
| size | 0 |
...

[Nathan Kinder (nkinder)]

I have proposed a patch to correct OSSN-0013 here:

  https://review.openstack.org/#/c/98267

[Nathan Kinder (nkinder)]

The new revision of OSSN-0013 has been published to the mailing lists and the wiki:

  https://wiki.openstack.org/wiki/OSSN/OSSN-0013