Some components like neutron-lib builds its own sub-enforcer which
enforces policy rules partially. However even these enforcer may load
the full policy rules in the file and this causes a lot of warnings
about "undefined rules".
This introduces a new flag so that users can disable undefined check,
when they know the undefined rules are "expected".
Note that the flag is not formally exposed, because we don't know if
this requirement is common. If we find similar problems with different
components then we may add an argument to __init__ .
Related-Bug: #2048198
Change-Id: Ibb4e8e877640e8488aaffb40560e930b0cbfcbce
The bug scenario:
- define deprecated rule in policy folder
- start a service
- enforce policies
- remove the rule in policy folder
- enforce policies
New default is applied to the rule,
but new and old defaults should be applied
(OR logic)
The patch fixes it.
Closes-Bug: 1977549
Change-Id: If11fe2da1163d6d3f16d133aeb207a055cf30de4
Generation of sample policy files was broken when exclude_deprecated was
added as an extra argument to the generate_sample function in
I6d02eb4d8f94323a806fab991ba2f1c3bbf71d04. It was passed as the fourth
argument, which is actually include_help. Because it defaults to False,
this turned sample policy files into actual policy files.
Fix by using keyword arguments instead.
Change-Id: I5478b1c8e7fd2f1b01f63602998194bab3683f7c
Closes-Bug: #1975682
The '--exclude-deprecated' parameter should only be passed to
oslo.config to parse when it is True.
The final generated sphinx syntax is[1] where [--exclude-deprecated]
doesn't require True/False value and only should be passed when True.
The change introducing this[2] causes parsing issue in oslo.config[3]
while checking <bool>.startswith (we pass True/False value) and even
after that while calling argparse[4] with following error[5].
[1] usage: sphinx-build [-h] [--config-dir DIR] [--config-file PATH] [--exclude-deprecated] [--format FORMAT] [--namespace NAMESPACE]
[--noexclude-deprecated] [--output-file OUTPUT_FILE]
[2] https://review.opendev.org/c/openstack/oslo.policy/+/830514
[3] https://opendev.org/openstack/oslo.config/src/branch/master/oslo_config/cfg.py#L2937
[4] https://opendev.org/openstack/oslo.config/src/branch/master/oslo_config/cfg.py#L2960
[5] > /usr/lib/python3.8/argparse.py(1781)parse_args()
-> if argv:
(Pdb)
> /usr/lib/python3.8/argparse.py(1782)parse_args()
-> msg = _('unrecognized arguments: %s')
(Pdb)
> /usr/lib/python3.8/argparse.py(1783)parse_args()
-> self.error(msg % ' '.join(argv))
(Pdb)
TypeError: sequence item 0: expected str instance, bool found
> /usr/lib/python3.8/argparse.py(1783)parse_args()
-> self.error(msg % ' '.join(argv))
Handler <function generate_sample at 0x7fc0d6697d30> for event 'builder-inited' threw an exception (exception: sequence item 0: expected str instance, bool found)
Closes-Bug: #1970725
Change-Id: I95745b8d1cbdb6a7cf442d431a998b7e3ff600e4
In the Enforcer.enforce() method there is boolean parameter do_raise.
When it is set to False, enforce() method should return True/False as an
enforcement result and not raise exception. It works like that with
PolicyNotAuthorized exception but since some time this method can also
raise InvalidScope exception and in such case behaviour was different.
This patch changes that behaviour so InvalidScope exception will also
not be raised when do_raise=False.
Closes-bug: #1965315
Change-Id: I37fd682ffa9d6f4c69698e1be42adac28bbfe72a
Deprecated rules can be confusing and downright unfriendly when
evaluating a generated sample output and seeing legacy rules being
aliased to new rules. Technically this is also invalid and results
in a broken sample file with overriding behavior.
Under normal circumstances, this wouldn't be a big deal, but with
the Secure RBAC effort, projects also performed some further
delineation of RBAC policies instead of performing a 1:1 mapping.
As a result of the policy enforcement model, a prior deprecated
rule was required, which meant the prior deprecated rule would
be reported multiple times in the output.
Since we don't have an extra flag in the policy-in-code definitions
of policies, all we can *really* do is both clarify the purpose
and meaning of the entry, not enable the alias by default in
sample output (as it is a sample! not an override of code!),
and provide projects as well as operators with a knob to
exclude deprecated policy inclusion into examples and sample
output.
Closes-Bug: #1945336
Change-Id: I6d02eb4d8f94323a806fab991ba2f1c3bbf71d04
Currently set_defaults() is only able to set the default
value of policy_file config option. In future, for example
scope config option like enforce_scope also needs to be override
the default value per service (service ready with scope enable
can set it to True and for other services it will be False as default
in oslo.policy).
To allow override the other config option, let's expand the existing
set_defaults() method to do so.
Change-Id: I72120efb7c55aab82b765237904c9ae6e91f6b6f
Previously it was checked only for registered rules but not for
rules which are subclasses of the BaseCheck class.
Now it's checked for all rules which have scope_types set.
It's required for e.g. Neutron as it is creating Check objects based
on the defined policy rules to e.g. include in the check attributes
like network's provider parameters, etc.
Depends-On: https://review.opendev.org/c/openstack/neutron/+/815838
Depends-On: https://review.opendev.org/c/openstack/neutron/+/818725
Closes-Bug: #1923503
Change-Id: I55258c1f999c84220518d1fbbf5e1e514361cebe
If an user uses Enforcer without overwriting (Enforcer(overwrite=False))
we should not reset rules and only update loaded rules.
Enforcer without overwriting is a weird behavior, but it is supported at this moment.
Maybe it will be eliminated in future because it's misleading.
Operator cannot conclude what rules are loaded by simply looking in config files.
Change-Id: I2871407f8c7417a016415ccc166c1f37a9e17908
Closes-Bug: 1943584
Policy directory files can only add new rules or
update existing rules in cache, but cannot return back
loaded rules in memory to their default value.
This incorrect behavior was fixed in the patch.
Member "_loaded_files" of class Enforcer should keep
list of loaded policy config files paths.
In fact if the same file is changed many times
then the same file path is added many times.
If a file is deleted it's path not deleted from "_loaded_files".
The member is very misleading and is not used in code.
So this member was deleted in the patch because of
above mentioned resons.
Change-Id: I9ede38d8cf2ae968d3d8c0b1240bd6a51e6aa931
Closes-Bug: 1943584
This patch moves code responsible for scope types enforcement
to the separate method which can be reused in different places,
like e.g. to enforce scope for instances of the BaseCheck class.
Related-Bug: #1923503
Change-Id: I6fd671728582b2f60939764075a8e2a977e78b58
Neutron, based on the defined policy rules is creating check
objects "in flight" to e.g. include check some object's attributes,
like e.g. network's provider parameters.
That use case requires that BaseCheck class and classes which inherits
from it needs to have scope_types defined thus Neutron can set it for
the Check based on the defined policy rule.
This patch adds scope_types attribute to the BaseCheck class to make it
available for use cases like described above.
Related-Bug: #1923503
Change-Id: Ibf30d0ffa5e9b125742089705d3557c02a03bc43
The help text isn't clear what happens when enforce_new_defaults is
False, which is the default behavior. Explicity call that out in the
help text since it's important for users to understand that behavior.
Change-Id: Iaed5682bc72f4c66adb9a40c6510b399314574df
An earlier patch[1] added a mapping for context 'system_scope'
to 'system' when enforce was called with a RequestContext
object. However, enforce can also be called with a creds dictionary
that may contain the context 'system_scope' element. When this
occured, 'system_scope' was not mapped to 'system' and the enforce
would fail with an InvalidScope exception.
This patch moves the 'system_scope' mapping from only occuring
with RequestContext objects to also map it when a creds dictonary
is passed to enforce.
[1] https://review.opendev.org/c/openstack/oslo.policy/+/578995
Change-Id: I83a22c3f825bad0c88018118f8630a20a445965e
This was lost in a previous change, but in the interest of not holding
up the merge of the functional part of it I'm fixing the test in this
followup. The new version of the test verifies that the rule isn't
overwritten by a second call to load_rules, which is what would
happen if we called the deprecation handler again.
Change-Id: Idc9dd353463da63a2108782f7b141d1d8014d180
There's no functional changes here other than moving the initializer
documentation for the 'DeprecatedRule' from the '__init__' docstring to
the class' docstring. This allows us to remove '__init__' entirely since
it was just calling the superclass.
Change-Id: I7437cf00b2ba5c02926dc8c2435e237ef730f2b1
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
We obviously can't do much about people accessing the private functions,
but this should avoid some obvious bugs. If people want to change the
rules, they should either state a different definition in a file or
change the definition in code.
Change-Id: Ibcbf5292977e5264bf7eedd225cbab83e683e394
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
There are two big issues with this. Firstly, as noted in bug #1914095,
we're not copying the provided 'Rule' object which means if we create an
enforcer and the rule is modified, then a second enforcer in the same
process will ultimately end up using the same modified 'Rule' object
will be used. Secondly, while the 'check' attribute is modified the
'check_str' attribute is not. This is immensely confusing for people
debugging.
Resolve both issues by not modifying this check at all and instead build
the combined check and store this.
Change-Id: Iff85a9134f4db7c0355da2858deb8a704d044634
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
For now, policy file unaccessible due to access permission leads to
silent failure without any notifictaion. Side effects of it are also
affected by policy caching and might be quite confusing, like periodic
disappearing of Horizon panels. Proposed patch added handling of such
errors.
Closes-bug: #1836390
Change-Id: I0d67b6e7c2dcaa63d6bb807f013e5e7334efc715
When service register their policy rule oslo policy does not
copy the rule and instead work on the original object.
- bd9d47aa36/oslo_policy/policy.py (L1104)
policy enforcer modify the default rules in
_handle_deprecated_rule().
- bd9d47aa36/oslo_policy/policy.py (L767-L774)
In any case, oslo policy should make copy of the registered
rules.
Another thing it fix is setting of flag
RuleDefault._deprecated_rule_handled.
Flag _deprecated_rule_handled is set to True when
_handle_deprecated_rule() is called irrespective of it
actually handle the deprecated rule and add it in OR checks.
We should set this flag when acutally deprecated rule is
handled so that if any condition change like config flag or
file rules we correctly handle deprecated rules.
Closes-Bug: #1914095
Closes-Bug: #1914592
Story: 2008556
Task: 41687
Change-Id: I154213dabd4d9eef760f0a4c9a852d504638ca8d
The policy engine converts simple strings into instances of rule
objects based on a policy DSL. This engine iterates checks and reduces
them after each iteration if performing the conversion on list of check
strings.
When we deprecate policies we apply a logical OR to make upgrades easier
for operators. The logical OR, implemented with an OrCheck, only needs
to be done once per deprecated rule. Today, we're re-initializing an
OrCheck instance each time we load rules, which happens every time
oslo_policy.policy.Enforcer.enforce() is called.
For most OpenStack usage, this isn't noticiable, especially if you're
only using it to enforce access to a specific endpoint. However, this
can get expensive if you're using the enforcer to protect the API,
protect each resource in a response, and protect each attrbute of the
resource (e.g., Neutron makes extensive usage of this pattern to
implement RBAC for resources it's responsible for).
This commit updates the RuleDefault object to track state of handling
deprecated logic ORs so that we only cast the check strings to OrCheck
instances once per rule no matter how many times we call load_rules().
Closes-Bug: 1913718
Change-Id: I539672fc220b8d7e3c47ab3dfa6670b88e3f4093
collections.MutableMapping has been deprecated since Python 3.3 and
is removed in Python 3.10. The functionality should be identical.
Change-Id: Ic96309fef409ba01dd24a3a70ff132a9f5352f9c
We have many if else condition to pick the
right policy filebut there is no debugging log
to have useful info to know if expected policy file
is not picked.
Change-Id: I507c58a6dca06d0cc6f306bcd063c700c18cc5f7
Currently, the way you replace a rule with another rule is by using the
'deprecated_rule' parameter of '(Documented)RuleDefault'. For example:
deprecated_rule = policy.DeprecatedRule(
name='foo:bar',
check_str='role:bazz'
)
policy.RuleDefault(
name='foo:create_bar',
check_str='role:bang',
description='Create a bar.',
deprecated_rule=deprecated_rule,
deprecated_reason='foo:bar has been replaced by foo:create_bar',
deprecated_since='N',
)
In this instance, we're stating that the 'foo:create_bar' policy
replaces the 'foo:bar' policy and we've used (and indeed have to use, to
avoid a 'ValueError') the 'deprecated_reason' and 'deprecated_since'
parameters on the **new** rule to illustrate why. This is confusing. The
new rule clearly isn't the one that's deprecated, so why are we stating
the 'deprecated_reason' and 'deprecated_since' there? We can clarify
this by instead specifying the reason and timeline on the deprecated
rule, like so:
deprecated_rule = policy.DeprecatedRule(
name='foo:bar',
check_str='role:bazz'
deprecated_reason='foo:bar has been replaced by foo:create_bar',
deprecated_since='N',
)
policy.RuleDefault(
name='foo:create_bar',
check_str='role:bang',
description='Create a bar.',
deprecated_rule=deprecated_rule,
)
Add support for this, with appropriate warnings to nudge people over to
the new, improved way of doing things eventually.
Change-Id: Ie4809c7749242bd092a2677b7545ef281735d984
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Many of requests' APIs accept a 'verify' parameter which can be a
boolean value or a path to either a CA cert bundle or a directory of CA
certs. If 'verify=True' is set, requests will look for two environment
variables, 'REQUESTS_CA_BUNDLE' and 'CURL_CA_BUNDLE', that, if set,
should specify a path to CA certs. If either of these are found,
'requests' overrides the 'True' value with the value of the environment
variable [1]. From the docs [2]:
This list of trusted CAs can also be specified through the
REQUESTS_CA_BUNDLE environment variable. If REQUESTS_CA_BUNDLE is not
set, CURL_CA_BUNDLE will be used as fallback.
This can cause test failures on environments where either of these are
set. Ensure this doesn't happen by using the 'EnvironmentVariable'
fixture to unset these environment variables.
[1] https://github.com/psf/requests/blob/v2.25.0/requests/sessions.py#L717-L719
[2] https://requests.readthedocs.io/en/master/user/advanced/#ssl-cert-verification
Change-Id: I808c9102b214aa25144e88e7773a9890ab0a5bdc
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Description and operators are mandatory parameter for
DocumentedRuleDefault but few services still using the
RuleDefault for exmaple glance
- 1344c45772/glance/policies/image.py (L16)
To make work on oslopolicy-j2y-convertor tool to covert
the JSON file for such services we need to pass description
in preparing DocumentedRuleDefault for this tool to generate
the yaml file with rule description. For such cases, it add
rule name as description.
Also for 'operations', do not write it in generate file
for RuleDefault as that end up with blank space.
Change-Id: I910291a152402051b1eac96c3ec16c3f0bb8bbb7
As part of community goal[1], each services are changing the default
value of 'CONF.oslo_policy.policy_file' config option from 'policy.json'
to 'policy.yaml'. oslo policy select the default value from
CONF.oslo_policy.policy_file which will be policy.yaml as service will
start changing the default. To avoid breaking the existing deployment which
are relying on old default (policy.json) file, a new fallback logic
is implemented. If new default file 'policy.yaml' does not exist but old
default 'policy.json' exist then fallback to use old default file.
Each services are going to add upgrade checks and warnings for using JSON
formatted policy file so in future we cna remove this fallback logic.
This logic was done in nova in Victoria cycle when nova changed the
default value - https://review.opendev.org/#/c/748059/ . Moving this
to oslo policy side will avoid the duplication on services side.
Also it provides a flag to disable this fallback.
[1] https://governance.openstack.org/tc/goals/selected/wallaby/migrate-policy-format-from-json-to-yaml.html
Change-Id: If72b2fcc3cfd8116b575ed7b9e3870df634fd9af
Introduced changes:
- pre-commit config and rules
- Add pre-commit to pep8 gate, Flake8 is covered in the pre-commit hooks.
- Applying fixes for pre-commit compliance in all code.
Also commit hash will be used instead of version tags in pre-commit to
prevend arbitrary code from running in developer's machines.
pre-commit will be used to:
- trailing whitespace;
- Replaces or checks mixed line ending (mixed-line-ending);
- Forbid files which have a UTF-8 byte-order marker (check-byte-order-marker);
- Checks that non-binary executables have a proper
shebang (check-executables-have-shebangs);
- Check for files that contain merge conflict strings (check-merge-conflict);
- Check for debugger imports and py37+ breakpoint()
calls in python source (debug-statements);
- Attempts to load all yaml files to verify syntax (check-yaml);
- Run flake8 checks (flake8) (local)
For further details about tests please refer to:
https://github.com/pre-commit/pre-commit-hooks
Change-Id: Ia9f7040f9966f1492c590a005f55ef7f3b67f0c9
Signed-off-by: Moisés Guimarães de Medeiros <moguimar@redhat.com>
This commit makes the following minor improvements
in YAML output of oslopolicy-sample-generator.
* Add a blank line between policies.
Previously when a deprecated rule exists there was no blank line
between the deprecated rule and the next rule.
It was not easy to identify the beginning of the next rule.
* Drop unnecessary blank line comment.
If a policy is defined by RuleDefault instead of DocumentedRuleDefault
there is no description and unnecessary blank line comment was added
in an output YAML file.
* Honor newlines in deprecated_text.
Previously newlines in deprecated_text were dropped by
_format_help_text(). Main deprecation message and reason are
processed separately and newlines are not dropped now.
Change-Id: I75889a1b05344a47135419d0553525f54c1a51b8
If any rules present in policy file is exactly same as
defaults then operators do not need to keep these
redundant rules in files. 'oslopolicy-list-redundant' tool
is to detects such rule but we can log warnings also for
such rule to communicate it to the deployer in strong way.
Partial implement blueprint policy-json-to-yaml
Change-Id: Ie31ea13e8ea62bc495ceb1c1694407539e2cab8d
JSON support for policy_file has been problematic
since projects started policy-in-code. For example,
generating a sample policy file in JSON results in
all the policy-in-code rules being overridden because
it is not possible to comment out the default rules in JSON.
Asd part of migration of JSON format to YAML, this commit
deprecates the:
1. Deprecate JSON support in oslo.policy.
2. Deprecate JSON output in policy CLI tools including '--format'
option.
Partial implement blueprint policy-json-to-yaml
Change-Id: I5432a8cf80903620f48936cbbfb92ea6b6ff30fa
Add ``oslopolicy-convert-json-to-yaml`` tool which can be
used to convert the json formatted policy file to yaml format.
It takes json formatted policy file as input and convert it to
a yaml formatted policy file similar to 'oslopolicy-sample-generator'
tool except keeping the overridden rule as uncommented.
This tool does the following:
* Comment out any rules that match the default from policy-in-code.
* Keep rules uncommented if rule is overridden.
* Does not auto add the deprecated rules in the file unless it not already
present in the file.
* Keep any extra rules or already exist deprecated rules uncommented
but at the end of the file with a warning text.
I did not add the new functionality in existing 'oslopolicy-policy-upgrade'
tool because the above listed features of new tool end up creating a
complete different code path instead of reusing it from existing tool so it
better to have separate tool which can be removed in future once all deployments
are migrated to YAML formatted file.
This commits add doc and reno also for this tool
Partial implement blueprint policy-json-to-yaml
Change-Id: Icc245951b2992cc09a891516ffd14f3d4c009920
Takashi Kajinami