After updating pylint, it started emitting additional "R"
warnings in some cases, fix some of them.
use-a-generator,
unnecessary-lambda-assignment,
consider-using-max-builtin,
consider-using-generator,
consider-using-in,
use-list-literal,
consider-using-from-import
Trivialfix
Change-Id: Ife6565cefcc30b4e8a0df9121c9454cf744225df
As part of the Secure RBAC community goal, we should switch options
"enforce_new_defaults" and "enforce_scope" to be True by default.
It will be still possible to fallback to old policy rules by configuring
those config options to False in Neutron config.
Change-Id: I09c0026ccf87e6c0bb1fa59165c03dc508fba6fa
When new default policy rules and scope enforcement are enabled, Neutron
needs to handle properly not only PolicyNotAuthorized exception from
oslo_policy module but also InvalidScope exception.
This patch adds handling of that exception to the neutron policy
modules.
In the check() method from the neutron.policy module we are calling
ENFORCER.enforce() method with do_raise=False which means that
PolicyNotAuthorized isn't rasised. Unfortunately it seems that there is
bug in oslo.policy module and InvalidScope is raised even with
do_raise=False.
For now, lets workaround it in Neutron by properly handling InvalidScope
exception in the check() method.
This workaround can be cleaned when bug [1] will be fixed in
oslo.policy.
[1] https://bugs.launchpad.net/oslo.policy/+bug/1965315
Partial-Bug: #1959333
Change-Id: I973f8896248c8222031c53343bb53ce48254da74
To be able to do policy checks for e.g. subattriutes, Neutron is
creating rule objects which are subclasses of the oslo_policy.BaseCheck
objects but we didn't set scope_types in that newly created rules.
As we are now moving to the new secure RBAC rules, which allows us to
enforce scope checks, we need to set those scope types for such objects
to let oslo.policy to enforce it when needed.
Related-Bug: #1923503
Change-Id: Ie9b4b908621a0e2ca7c9f7d8a3057c95e28f0dbd
When new policy default rules are enforced in the config, we shouldn't
be checking if context.is_admin is true and stop performing checks if
that is True.
As Neutron's policy rules are going to understand and use new
personas like system-admin, project-admin and e.g. reader roles, it
needs to be aware of those and simply checking if
context.is_admin == True
in the neutron.policy.check() and neutron.policy.enforce() functions is
not enough. We need to perform all checks in such cases as well to e.g.
avoid giving list of the system resources to the user who has
project's admin role.
Such change will require a lot of changes in the code (e.g. unit tests)
and as we are close to the release point, this patch left that
context.is_admin check logic in case when
CONF.oslo_policy.enforce_new_defaults option is set to False.
In next release we need to get rid of that check if context.is_admin ==
True completly and adjust all required places in code as well.
Related-blueprint: #secure-rbac-roles
Change-Id: I403ca661dceee17aff9295caf8721c4a237a58cf
The oslo.policy Enforcer object understand what to do with instances of
oslo.context.RequestContext. This makes it easier to invoke enforcement
because we don't need to translate the context object to policy values
before we pass it into the policy engine, oslo.policy will do that for
us.
Change-Id: I08348503c092dc5665a0d81d01a2ef164ba7209e
Patch [1] workarounded problem with deepcopy of the FieldCheck objects
when running with Python 3.6 or older.
But it seems that we have yet another problem with oslo_policy 3.6.2.
This time it is issue with doing deepcopy of the instances of OwnerCheck
class when Neutron runs under mod_uwsgi.
This patch fixes/workarounds that by adding same implementation of the
__deepcopy__ method to the OwnerCheck class as [1] added to FieldCheck.
[1] https://review.opendev.org/c/openstack/neutron/+/774621
Closes-Bug: #1915494
Change-Id: Idf1dcdeaa94ae76cf057c4c04982dc1ee0478f86
This resolves an issue with oslo.policy 3.6.2. Previously, we couldn't
deepcopy these objects since they store a regex object which is not
deepcopy'able before Python 3.7. From the release notes [1]:
Compiled regular expression and match objects can now be copied using
copy.copy() and copy.deepcopy(). (Contributed by Serhiy Storchaka in
bpo-10076.)
[1] https://docs.python.org/3/whatsnew/3.7.html#re
Change-Id: Ic2c66f304e300521c71828d3e811f18b43718d7e
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Related-Bug: #1914095
As we are in the middle of the migration to new secure RBAC policies
and we have a lot of deprecated default rules, our log in e.g.
functional tests has a lot of messages about deprecated rules.
So lets suppress those deprecation warnings in the tests to make our
test outputs smaller.
Related blueprint: secure-rbac-roles
Change-Id: Iab3966bad81b469eccf1050f0e0e48b9e2573750
As per the community goal of migrating the policy file
the format from JSON to YAML[1], we need to do two things:
1. Change the default value of '[oslo_policy] policy_file''
config option from 'policy.json' to 'policy.yaml' with
upgrade checks.
2. Deprecate the JSON formatted policy file on the project side
via warning in doc and releasenotes.
Also replace policy.json to policy.yaml ref from doc and tests.
[1]https://governance.openstack.org/tc/goals/selected/wallaby/migrate-policy-format-from-json-to-yaml.html
Change-Id: I0dbb8484e749e645627756e88ec79c1b26a6414a
With python 3.x, six.text_type and six.string_type
are just str.
Also removed a six.integer_type since it was the only
one left in a file.
Another step in removing all of six usage from neutron.
Change-Id: I5208dc41bff1983ecd323286f427296b722da62a
In case when user's security group contains rules created e.g.
by admin, and such rules has got admin's tenant as tenant_id,
owner of security group should be able to see those rules.
Some time ago this was addressed for request:
GET /v2.0/security-groups/<sec_group_id>
But it is also required to behave in same way for
GET /v2.0/security-group-rules
So this patch fixes this behaviour for listing of security
group rules.
To achieve that this patch also adds new policy rule:
ADMIN_OWNER_OR_SG_OWNER which is similar to already existing
ADMIN_OWNER_OR_NETWORK_OWNER used e.g. for listing or creating
ports.
Change-Id: I09114712582d2d38d14cf1683b87a8ce3a8e8c3c
Closes-Bug: #1824248
In patch [1] handle of networks with "shared" flag set to True was
fixed and it is now possible to use "rule:shared" in API policy in
actions related e.g. to ports or subnets.
But network can be shared with some specific tenant only by doing it
with RBAC mechanism and in such case it didn't work with [1] only.
It was like that because context.get_admin_context() was used to get
network so this returned network had got shared=False set even if
request comes from tenant for which network was shared through RBAC.
Now network will be always get with context which have got set proper
tenant_id so "shared" flag will be set properly even in case if it's
shared through RBAC.
[1] https://review.opendev.org/#/c/652636/
Change-Id: I38615c0d18bb5a1f22f3e7865ce24615a540aa9a
Closes-Bug: #1833455
Some policy rules e.g. for create_port are using rule "network:shared"
in which "shared" field is related to network resource instead of
port directly.
Because of that, "shared" was missing from "target" in policy
enforce module thus validation wasn't working properly for such rule.
This patch fixes it by adding to FieldCheck checker possibility to
get network object and use its "shared" field to validate policy.
Change-Id: I56c99883fce40c37a5ee26e6e661c0cc0783c42f
Closes-Bug: #1808112
In case of policy rule checks for rules like e.g.
"create_port:fixed_ips:subnet" couldn't be created to be
passed to policy enforcer because policy module could only
create rule checks for subattributes which are dict types.
With this patch checks for such rules can be created also for
attributes which are list of dicts, like e.g. fixed_ips in port
resource.
Change-Id: I02fffe77f57a513d2362df78885d327042bb8095
Closes-Bug: #1822105
All of the externally consumed variables from neutron.common.constants
now live in neutron-lib. This patch removes neutron.common.constants
and switches all uses over to lib.
NeutronLibImpact
Depends-On: https://review.openstack.org/#/c/647836/
Change-Id: I3c2f28ecd18996a1cee1ae3af399166defe9da87
The neutron.common.constants.EXT_PARENT_RESOURCE_MAPPING constant lives
in neutron-lib. This patch consumes it by removing it from neutron
and switching all uses over to libs constant.
NeutronLibImpact
Change-Id: Ib7ea6dea58bf211a2da2b103cb526233df04ba49
This commit introduces a framework for policy-in-code support
in the neutron stadium and converts the existing policy.json
in the neutron repository into the policy-in-code style.
NOTES:
1) This commit tries not to change the existing policy behavior
provided by the neutron repository even if there are some stale policies
or policies to be defined in a neutron-related project.
They should be clean up later in Stein release.
2) 'default' policy should be dropped from the default policies
as all default policies should be defined in the code (as many projects
which already completed policy-in-code do). However, dropping 'default'
policy potentially affects policy behavior in neutron-related projects,
so it needs to be visit carefully. Considering this, this commit decides
to keep the 'default' policy.
Partially Implements: blueprint neutron-policy-in-code
Change-Id: I6a61079da4d4f5080ee32d640144e6bdb14735fa
Add common parent owner check for the resources which introduced by
service plugin.
Then port forwarding resource will share the same tenant_id with
floatingip. That means only the fip owner can create/update/get/delete
the associated port forwarding resource.
Partially-Implements: blueprint port-forwarding
Partial-Bug: #1491317
Change-Id: I450c674e55ca15e1d9a6a6224138f3305427da68
Today we shim the RESOURCE_ATTRIBUTE_MAP in neutron; it references the
equivelant in neutron-lib named RESOURCES.
This patch removes neutron's RESOURCE_ATTRIBUTE_MAP and cleans up
neutron.api.v2.attributes in prep to delete it.
To do so:
- CORE_RESOURCES and RESOURCE_FOREIGN_KEYS are moved to the single
module that references them respectively and the are made private (no
consumers use them).
- get_collection_info is removed and instead the 2 uses in neutron
just use the get() method of the RESOURCES map. There are no external
uses of get_collection_info.
- References using RESOURCE_ATTRIBUTE_MAP are switched over to
neutron-lib's RESOURCES.
- The neutron.api.v2.attributes module is removed as it's empty now.
- A few api attribute UTs are removed; there's nothing to test as per
this patch.
NeutronLibImpact
Change-Id: Iaacee584d499c4d33d6d2dd9609c7ac0f2cfc386
Since Pike log messages should not be translated.
This patch removes calls to i18n _LC, _LI, _LE, _LW from
logging logic throughout the code. Translators definition
from neutron._i18n is removed as well.
This patch also removes log translation verification from
ignore directive in tox.ini.
Change-Id: If9aa76fcf121c0e61a7c08088006c5873faee56e
Rather than generating an entire log line for each policy that
filters out an attribute, we can instead log the excluded attributes
once after all of the policy checks are done.
This only applies to the policy checks, policy enforcements
(e.g. preventing a user from entering a field value on create)
are still logged individually.
Partial-Bug: #1707307
Change-Id: I77401e30cb220901dacd61028acde4c79001ed9b
according to https://wiki.openstack.org/wiki/Python3, now we should avoid
using six.iteritems and replace it with dict.items.
Change-Id: I58a399baa2275f280acc0e6d649f81838648ce5c
Closes-Bug: #1680761
Register a new extension into Neutron server, and this extension
contains a member action. if new member action contains the "update"
string. As a result, Neutron server check the target whether contains
"ATTRIBUTES_TO_UPDATE". Because this is a member action so that neutron
server will not go "_update" method normally but "_handle_action"
method. So the exception happens KeyError: 'attributes_to_update'.
Co-Authored-By: zhang ping<ping.zhang@easystack.cn>
Change-Id: Ie8bb7b9af42a82aada28d2bcdfd5b0e573ad9778
Closes-Bug: #1606455
Use the common policy values for all services when enforcing policy
decisions. We add all possibly used policy values to maintain backwards
compatibility.
Change-Id: Ie1d0739ab4dfb0654d8767693dbdba5cd52a30b2
Closes-Bug: #1602081
Parent foreign key extraction requires another database fetch per object
validates, which does not scale well. To mitigate the issue, cache
extracted parent key values in a global policy cache dictionary.
Use oslo.cache to maintain cache. Hardcode expiration timeout for
policy lookups to 5 secs.
Change-Id: I6b3d7c96b7487c9bef6d39a28c76fea0721c3098
Related-Bug: #1513782
Neutron Manager is loaded at the very startup of the neutron
server process and with it plugins are loaded and stored for
lookup purposes as their references are widely used across the
entire neutron codebase.
Rather than holding these references directly in NeutronManager
this patch refactors the code so that these references are held
by a plugin directory.
This allows subprojects and other parts of the Neutron codebase
to use the directory in lieu of the manager. The result is a
leaner, cleaner, and more decoupled code.
Usage pattern [1,2] can be translated to [3,4] respectively.
[1] manager.NeutronManager.get_service_plugins()[FOO]
[2] manager.NeutronManager.get_plugin()
[3] directory.get_plugin(FOO)
[4] directory.get_plugin()
The more entangled part is in the neutron unit tests, where the
use of the manager can be simplified as mocking is typically
replaced by a call to the directory add_plugin() method. This is
safe as each test case gets its own copy of the plugin directory.
That said, unit tests that look more like API tests and that rely on
the entire plugin machinery, need some tweaking to avoid stumbling
into plugin loading failures.
Due to the massive use of the manager, deprecation warnings are
considered impractical as they cause logs to bloat out of proportion.
Follow-up patches that show how to adopt the directory in neutron
subprojects are tagged with topic:plugin-directory.
NeutronLibImpact
Partially-implements: blueprint neutron-lib
Change-Id: I7331e914234c5f0b7abe836604fdd7e4067551cf
In Python3 [1], if we need filter on python3, replace
filter(lambda obj: test(obj), data) with:
[obj for obj in data if test(obj)]. This patch replaces
filter function and introduces a hacking rule to prevent
using filter in future.
[1] https://wiki.openstack.org/wiki/Python3
Change-Id: I83d22108c02f8da007a7233e71a4a7fb833170ec
Remove deprecation warnings for various constants
and exceptions that have moved to neutron_lib.
Fix miscellaneous other deprecations.
Uses constants instead of l3_constants when importing
neutron-lib constants.
Co-Authored By: Henry Gessau <gessau@gmail.com>
Co-Authored By: Gary Kotton <gkotton@vmware.com>
Change-Id: Ib0e8ff5c3e23677c1009241a1818cbc8a3430c38
Currently the default value of target_tenant in rbac_policies resource
is None, if a user creates rbac policy without a target_tenant,
neutron-server raises an internal error.
This patch adds a check for target_tenant, it must be a string which
length is TENANT_ID_MAX_LEN.
This patch also does a change for policy engine which now allows
enforce_policy to work on attrs without default.
Change-Id: I757b4b56a8cb1a8060cee3103ee2aead9be574de
Closes-bug: #1517331
Network owner should be able to get all ports and delete ports on
network as policy allowed. But current code fails to support this.
Current model query for Port is still based on tenant_id, it forgets
to check for network owner when context tenant_id is not port owner.
For port_delete action, policy will generate checking rules for port
attributes, such as:
rule:delete_port:binding:vif_details
rule:delete_port:binding:vif_type
This doesn't make sense, only single policy rule "rule:delete_port"
is enough to check.
This patch fixes this issue.
Co-Authored-By: Kevin Benton <kevinbenton@buttewifi.com>
Change-Id: I55328cb43207654b9bb4cfb732923982d020ab0a
Closes-Bug: #1498790
Python 3 deprecated the logger.warn method, see:
https://docs.python.org/3/library/logging.html#logging.warning
so we prefer to use warning to avoid DeprecationWarning.
Closes-Bugs: #1529913
Change-Id: Icc01ce5fbd10880440cf75a2e0833394783464a0
Co-Authored-By: Gary Kotton <gkotton@vmware.com>
During a port list operation, a port and its parent network
may be concurrently deleted from the database after they have
been retrieved from the DB but before policy is enforced.
Then when the policy engine tries to do a get_network to check
network ownership for a port on a network that no longer exists,
it will encounter a NetworkNotFound exception from the core plugin.
This exception was being propagated all of the way up to the whole
API operation as a 404, which made no sense in the context of a
port list.
This patch adjusts the logic to catch any NotFound exceptions during
this processing and convert them into a RetryRequest to trigger the
API to restart the operation. At this point the objects will be gone
from the database so the problematic items will not be passed to the
policy engine for enforcement.
Closes-Bug: #1528031
Change-Id: I89d12fe0767e1c7ecb68138b5f6f17aa68a68769
- This does NOT break other projects that rely on neutron.i18n,
as this change includes a debtcollector shim to maintain those
older entry points, until they can migrate.
- Also updates _i18n.py to the latest pattern defined by oslo_i18n
- Guidance and template are from the reference:
http://docs.openstack.org/developer/oslo.i18n/usage.html
Partially-Closes-Bug: #1519493
Change-Id: I1aa3a5fd837d9156da4643a367013c869ed8bf9d
1. Scope mock of 'open' to module
By mocking 'open' at the module level, we can avoid affecting
'open' calls from other modules.
2. Stop using LOG.exception in contexts with no sys.exc_info set
Python 3.4 logger fills in record.exc_info with sys.exc_info() result
[1], and then it uses it to determine the current exception [2] to
append to the log message. Since there is no exception, exc_info[1] is
None, and we get AttributeError inside traceback module.
It's actually a bug in Python interpreter that it attempt to access the
attribute when there is no exception. It turns out that it's fixed in
latest master of cPython [3] (the intent of the patch does not seem
relevant, but it removes the offending code while reshuffling the code).
Note that now cPython correctly checks the exception value before
accessing its attributes [4].
The patch in cPython that resulted in the failure is [5] and is present
since initial Python 3k releases.
The patch in fixtures that broke us is [6].
[1]: https://hg.python.org/cpython/file/tip/Lib/logging/__init__.py#l1412
[2]: https://hg.python.org/cpython/file/tip/Lib/logging/__init__.py#l575
[3]: https://hg.python.org/cpython/rev/73afda5a4e4c
[4]: https://hg.python.org/cpython/rev/73afda5a4e4c#l6.484
[5]: https://hg.python.org/cpython/rev/2ee09afee126
[6]: 67dd295694
Closes-Bug: #1503847
Closes-Bug: #1504053
Co-Authored-By: Ihar Hrachyshka <ihrachys@redhat.com>
Change-Id: I456b7846b8a53e4d3f8c91583685e0e1eaa84719
This patch adds a policy.rst document which describes how the
neutron.policy module works, how it uses oslo_policy and how
it's wired in API controllers.
The patch also remove an obsolete docstring from neutron/policy.py
Change-Id: I3353a23f4e97824a5a91eaedf57a91ace031a9de
This patch adjusts the FieldCheck class in the policy engine to
allow a regex rule. It then leverages that to prevent users from
setting the device_owner field to anything that starts with
'network:' on networks which they do not own.
This policy adjustment is necessary because any ports with a
device_owner that starts with 'network:' will not have any security
group rules applied because it is assumed they are trusted network
devices (e.g. router ports, DHCP ports, etc). These security rules
include the anti-spoofing protection for DHCP, IPv6 ICMP messages,
and IP headers.
Without this policy adjustment, tenants can abuse this trust when
connected to a shared network with other tenants by setting their
VM port's device_owner field to 'network:<anything>' and hijack other
tenants' traffic via DHCP spoofing or MAC/IP spoofing.
Closes-Bug: #1489111
Change-Id: Ia64cf16142e0e4be44b5b0ed72c8e00792d770f9
Zuul