Rules from the policy directory files are not reapplied after changes to the primary policy file

Bug #1880959 reported by Dmitrii Shcherbakov on 2020-05-27
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Ubuntu Cloud Archive
High
Unassigned
Mitaka
High
Unassigned
Queens
High
Unassigned
Rocky
High
Unassigned
Stein
High
Unassigned
Train
High
Unassigned
Ussuri
High
Unassigned
oslo.policy
Undecided
Dmitrii Shcherbakov
python-oslo.policy (Ubuntu)
Status tracked in Groovy
Xenial
High
Unassigned
Bionic
High
Unassigned
Eoan
High
Unassigned
Focal
Undecided
Unassigned
Groovy
High
Unassigned

Bug Description

[Impact]
Based on the investigation here https://bugs.launchpad.net/charm-keystone/+bug/1880847 it was determined that rules from policy files located in the directory specified in the policy_dirs option (/etc/<config_dir>/policy.d by default) are not re-applied after the rules from the primary policy file is re-applied due to a change.

This leads to scenarios where incorrect rule combinations are active.

Example from the test case in 1880847:

* policy.json gets read with the following rule;
    "identity:list_credentials": "rule:admin_required or user_id:%(user_id)s",
* rule.yaml from policy.d is read with the following rule;
{'identity:list_credentials': '!'}
* policy.json's mtime gets updated (with or without a content change) and overrides the rule to be
    "identity:list_credentials": "rule:admin_required or user_id:%(user_id)s",
* rule.yaml doesn't get reapplied since it hasn't changed.

[Test Case]
== ubuntu ==

The patches include unit tests that ensure the code is behaving as expected and has not regressed. These tests are run during every package build.

== upstream ==
For a particular version of oslo.policy:

* put the attached test (https://bugs.launchpad.net/ubuntu/+source/python-oslo.policy/+bug/1880959/+attachment/5377753/+files/test_1880959.py) under oslo_policy/tests/test_1880959.py;

* run tox -e cover -- oslo_policy.tests.test_1880959.EnforcerTest;
* observe the failure;
# ...
testtools.matchers._impl.MismatchError: 'role:fakeA' != 'rule:admin'
Ran 1 tests in 0.005s (+0.001s)
FAILED (id=1, failures=1)

* apply the patch;
* run tox -e cover -- oslo_policy.tests.test_1880959.EnforcerTest
* observe that the failure is no longer there.

[Regression Potential]
The regression potential is low given that there is test coverage in the olso.policy unit tests.

Fix proposed to branch: master
Review: https://review.opendev.org/731218

Changed in oslo.policy:
assignee: nobody → Dmitrii Shcherbakov (dmitriis)
status: New → In Progress
Corey Bryant (corey.bryant) wrote :

@dmitrii, Thanks for the patch. What releases does this affect?

Changed in python-oslo.policy (Ubuntu Groovy):
status: New → Triaged
importance: Undecided → High
Dmitrii Shcherbakov (dmitriis) wrote :

Corey, I believe the issue got introduced in Liberty and affects all releases since then: https://opendev.org/openstack/oslo.policy/commit/b5f07dfe4cd4a5d12c7fecbc3954694d934de642

The check in question is still the same in Ussuri and that code hasn't seen much change:
https://opendev.org/openstack/oslo.policy/src/commit/d4a2c64fad8b0d8d2df6a447c6c9826deaf24593/oslo_policy/policy.py#L581-L585

Corey Bryant (corey.bryant) wrote :

Thanks Dmitrii, we'll need to get this backported as far as we can upstream once the fix lands in master. I'll target the rest of the ubuntu and cloud archive releases. Would you be able to add some simple repro steps to the [Test Case] section in the bug description? This can include a juju based deploy. That will help us verify SRU fixes for each release combination once we have fixes in corresponding -proposed pockets available for testing.

description: updated
Changed in python-oslo.policy (Ubuntu Xenial):
status: New → Triaged
Changed in python-oslo.policy (Ubuntu Bionic):
status: New → Triaged
Changed in python-oslo.policy (Ubuntu Eoan):
status: New → Triaged
importance: Undecided → High
Changed in python-oslo.policy (Ubuntu Bionic):
importance: Undecided → High
Changed in python-oslo.policy (Ubuntu Xenial):
importance: Undecided → High
Dmitrii Shcherbakov (dmitriis) wrote :

Corey,

The simplest way to test this for a particular version would be to put the attached test under oslo_policy/tests/test_1880959.py in a checked out version of oslo.policy and running:

tox -e cover -- oslo_policy.tests.test_1880959.EnforcerTest

It will fail like below without the patch applied and pass if it is.

# ...
testtools.matchers._impl.MismatchError: 'role:fakeA' != 'rule:admin'
Ran 1 tests in 0.005s (+0.001s)
FAILED (id=1, failures=1)

description: updated
Corey Bryant (corey.bryant) wrote :

@Dmitrii, Couple questions. Will we be able to test the Ubuntu package with that? Would it make sense for that test to land upstream? Whatever test we decide on we'll need to be able to test against the package version in -proposed to ensure it is fixed. We run upstream unit tests during package builds so that may be an option if the SRU Team is ok with it.

Reviewed: https://review.opendev.org/731218
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=75677a31108243e0adddc89f1fbf669053f9573b
Submitter: Zuul
Branch: master

commit 75677a31108243e0adddc89f1fbf669053f9573b
Author: Dmitrii Shcherbakov <email address hidden>
Date: Wed May 27 17:06:25 2020 +0300

    Reload files in policy_dirs on primary file change

    It was determined that rules from policy files located in the directory
    specified in the policy_dirs option (/etc/<config_dir>/policy.d by
    default) are not re-applied after the rules from the primary policy file
    is re-applied due to a change.

    This change introduces additional behavior to make sure the rules from
    policy_dirs are reapplied if there is a change to the primary policy
    file.

    Change-Id: I8a6f8e971d881365c41ea409966723319d5b239a
    Closes-Bug: #1880959
    Related-Bug: #1880847

Changed in oslo.policy:
status: In Progress → Fix Released

Corey, to your question in #6 about being able to add this test to the package, the test that Dmitrii linked is actually pulled, almost verbatim, from the linked review. After pulling in the associated change, this test will run as part of the package tests (AFAIU?) so we should be able to validate it fairly cleanly

tags: added: cpe-onsite
Dmitrii Shcherbakov (dmitriis) wrote :

Yes, the backports themselves include the unit tests necessary to validate the change.

https://review.opendev.org/#/q/topic:bug/1880959+(status:open+OR+status:merged)

We just need to get a second +2 on some of them.

Rocky and Queens backports are blocked on devstack backports to devstack/rocky and devstack/queens referenced in the respective reviews.

Reviewed: https://review.opendev.org/734151
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=c8c138e69d9189345a7366a10b0779ef70b7250e
Submitter: Zuul
Branch: stable/ussuri

commit c8c138e69d9189345a7366a10b0779ef70b7250e
Author: Dmitrii Shcherbakov <email address hidden>
Date: Wed May 27 17:06:25 2020 +0300

    Reload files in policy_dirs on primary file change

    It was determined that rules from policy files located in the directory
    specified in the policy_dirs option (/etc/<config_dir>/policy.d by
    default) are not re-applied after the rules from the primary policy file
    is re-applied due to a change.

    This change introduces additional behavior to make sure the rules from
    policy_dirs are reapplied if there is a change to the primary policy
    file.

    Change-Id: I8a6f8e971d881365c41ea409966723319d5b239a
    Closes-Bug: #1880959
    Related-Bug: #1880847
    (cherry picked from commit 75677a31108243e0adddc89f1fbf669053f9573b)

description: updated
Changed in cloud-archive:
status: In Progress → Triaged
Corey Bryant (corey.bryant) wrote :

Eoan is EOL in July so will upload directly to train.

Changed in python-oslo.policy (Ubuntu Eoan):
status: Triaged → Won't Fix
Corey Bryant (corey.bryant) wrote :

Dmitrii, thanks for all the patches. For ubuntu, fixes have been uploaded to groovy, focal unapproved, train-staging ppa, and stein-staging ppa.

Changed in cloud-archive:
status: Triaged → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package python-oslo.policy - 3.2.0-0ubuntu2

---------------
python-oslo.policy (3.2.0-0ubuntu2) groovy; urgency=medium

  * d/p/reload-policy-files.patch: Cherry-picked from upstream master
    to ensure policy directory files are reapplied after change to primary
    policy file (LP: #1880959).

 -- Corey Bryant <email address hidden> Thu, 25 Jun 2020 10:47:11 -0400

Changed in python-oslo.policy (Ubuntu Groovy):
status: Triaged → Fix Released
Corey Bryant (corey.bryant) wrote :

This bug was fixed in the package python-oslo.policy - 3.2.0-0ubuntu2~cloud0
---------------

 python-oslo.policy (3.2.0-0ubuntu2~cloud0) focal-victoria; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 python-oslo.policy (3.2.0-0ubuntu2) groovy; urgency=medium
 .
   * d/p/reload-policy-files.patch: Cherry-picked from upstream master
     to ensure policy directory files are reapplied after change to primary
     policy file (LP: #1880959).

Changed in cloud-archive:
status: Fix Committed → Fix Released

Reviewed: https://review.opendev.org/734154
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=5904564bf13bbac7d66e00ec6312487c507f09c4
Submitter: Zuul
Branch: stable/train

commit 5904564bf13bbac7d66e00ec6312487c507f09c4
Author: Dmitrii Shcherbakov <email address hidden>
Date: Wed May 27 17:06:25 2020 +0300

    Reload files in policy_dirs on primary file change

    It was determined that rules from policy files located in the directory
    specified in the policy_dirs option (/etc/<config_dir>/policy.d by
    default) are not re-applied after the rules from the primary policy file
    is re-applied due to a change.

    This change introduces additional behavior to make sure the rules from
    policy_dirs are reapplied if there is a change to the primary policy
    file.

    Change-Id: I8a6f8e971d881365c41ea409966723319d5b239a
    Closes-Bug: #1880959
    Related-Bug: #1880847
    (cherry picked from commit 75677a31108243e0adddc89f1fbf669053f9573b)
    (cherry picked from commit c8c138e69d9189345a7366a10b0779ef70b7250e)

Hello Dmitrii, or anyone else affected,

Accepted python-oslo.policy into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/python-oslo.policy/3.1.0-0ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in python-oslo.policy (Ubuntu Focal):
status: New → Fix Committed
tags: added: verification-needed verification-needed-focal
Corey Bryant (corey.bryant) wrote :

Hello Dmitrii, or anyone else affected,

Accepted python-oslo.policy into ussuri-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:ussuri-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-ussuri-needed to verification-ussuri-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-ussuri-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-ussuri-needed
Corey Bryant (corey.bryant) wrote :

Hello Dmitrii, or anyone else affected,

Accepted python-oslo.policy into train-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:train-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-train-needed to verification-train-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-train-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-train-needed
Corey Bryant (corey.bryant) wrote :

Hello Dmitrii, or anyone else affected,

Accepted python-oslo.policy into stein-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:stein-proposed
  sudo apt-get update

Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-stein-needed to verification-stein-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-stein-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-stein-needed

Reviewed: https://review.opendev.org/734155
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=a49f31d3fc7690e3d15bc2428660b537967c89cc
Submitter: Zuul
Branch: stable/stein

commit a49f31d3fc7690e3d15bc2428660b537967c89cc
Author: Dmitrii Shcherbakov <email address hidden>
Date: Wed May 27 17:06:25 2020 +0300

    Reload files in policy_dirs on primary file change

    It was determined that rules from policy files located in the directory
    specified in the policy_dirs option (/etc/<config_dir>/policy.d by
    default) are not re-applied after the rules from the primary policy file
    is re-applied due to a change.

    This change introduces additional behavior to make sure the rules from
    policy_dirs are reapplied if there is a change to the primary policy
    file.

    Change-Id: I8a6f8e971d881365c41ea409966723319d5b239a
    Closes-Bug: #1880959
    Related-Bug: #1880847
    (cherry picked from commit 75677a31108243e0adddc89f1fbf669053f9573b)
    (cherry picked from commit 5904564bf13bbac7d66e00ec6312487c507f09c4)

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments