oslo.policy

oslopolicy-checker should pass data unaltered to policy enforcer

Bug #1803233 reported by John Dennis
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
oslo.policy
Confirmed
Medium
Unassigned

Bug Description

oslopolicy-checker is making unfounded assumptions on the input data it is provided or not provided as may be the case. The point of the tool is to be able to test policy evaluation standalone without needing to run a full OpenStack deployment.

The following input data is required as input to the policy engine:

* policy rules
* credential data
* target data
* rule to evaluate (or all rules)

In it's current form oslopolicy-checker is making assumptions about the credential data (it calls it access data). It tries to cherry-pick certain value out of the input access data and then omits all other data it didn't cherry-pick out. If the access data did not contain an expected key the tool fails with a KeyError (this is bad). It's also bad it throws away the rest of the data. This means if you know exactly what the inputs are, let's say because you captured it from log data or some other mechanism and want to test the policy enforcement based on the data passed to the oslo.policy enforcer you can't, this means you can't evaluate actual data.

In a similar vein if target data is not passed oslopolicy-checker will "invent" target data (project_id), once again making the wild assumption the project_id key is in the credential data (it might not be, which then raises a KeyError exception).

Actual runtime collection of data being passed to the oslo.policy enforcer demonstrates some of these assumptions cannot be satisfied.

The oslopolicy-checker should pass the EXACT data to the rule evaluation function that is provided as input to the tool, it should not be in the business of trying to impose it's interpretation of the data.

As an aside the --is-admin command line arg required a value of "true" is a non-standard way to enable a value, it should be just --is-admin which is easily done with an arparse action of 'store_true'. If we're going to adjust the behavior of the command line arguments we might as well address this as well.

Revision history for this message
John Dennis (jdennis-a) wrote :

FYI, I do have a patch ready and will submit shortly.

Revision history for this message
Ben Nemec (bnemec) wrote :

Does https://review.openstack.org/#/c/613313/ help with this?

The --is-admin weirdness has been noted in reviews as well and I agree we should try to fix that. The one concern would be if it breaks existing users, but we might just have to do a major release to indicate the breakage.

Changed in oslo.policy:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
John Dennis (jdennis-a) wrote :

I'm aware of the above patch, it was from my co-worker :-)

The --is-admin doesn't even make sense if the tool accepts unaltered credential data (what an unfortunate name because it's not credentials, rather it's auth context).

Anyway, I have a patch almost ready to go, just need to update the unit test.

As I tried to fix the original problem I kept running into other issues and finally decided to forget about backward compatibility (which I don't think you can ever have with the way the credential data worked) and focus instead on making the tool as useful as possible, which to me means behaving as close as possible to the way project use oslo.policy in their project.

I've also patch oslo.policy to log the credential and target data and it looks like this patch will merge as soon as gate issues are addressed, the change is: https://review.openstack.org/#/c/619260/

My olslopolicy-checker re-write adds the following features:

* Accepts raw credential data (along with the patch for target data noted above it now takes the same data)

* Fully integrated with oslo.config. This mean you can use exactly the same policy config as do actual OpenStack projects use via config files -OR- via command line options.

* Uses oslo.config for the tools argument parsing.

* Instantiates and uses an Enforcer object just like OpenStack projects use instead of the anonymous generic Object hack the checker previously used. It's important we emulate the policy behavior by using the same Python classes.

* Uses the same enforcement call (Enforcer.enforce()) that projects use instead of reaching inside the policy object and invoking rules directly.

* Configures logging and connects the logging output to either the console or a file. You can also set the logging level. BTW, I discovered it was very useful to see log messages because oslo.policy can detect problems in rules (e.g. cycles) and if logging is not connected as was previously the case you never see these vital messages.

* Allows an arbitrary number of individual rules to be checked (it used to be 1 or all)

* Keeps a count (and lists) the rules checked, successful rules, rules with failures, undefined rules (e.g. you specify a rule on the command line but it's not in the policy), and the rules which generated an exception.

* Captures the exception information for any rule that generated an exception.

* Provides meaningful shell exit status values so the tool can be used in automated testing.

Revision history for this message
Olaf Seibert (oseibert-sys11) wrote :

This sounds great! I am currently trying to debug some policies that somebody wrote in the dim past, and better checking would be great. Even with the latest oslo.policy it seems that the above features are not all present. Is there any progress in getting them merged?

While we're here, it would also be great to allow to specify multiple --access and --target options, and the checker would output a clear overview of the results of all combinations of access and target for a given rule set.

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.