Stop device_owner from being set to 'network:*'
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. Conflicts: etc/policy.json neutron/api/v2/attributes.py neutron/tests/etc/policy.json neutron/tests/unit/test_policy.py Closes-Bug: #1489111 Change-Id: Ia64cf16142e0e4be44b5b0ed72c8e00792d770f9 (cherry picked from commit 959a2f28cbbfc309381ea9ffb55090da6fb9c78f)
This commit is contained in:
parent
2f344ee088
commit
ebb0c5403b
|
@ -37,7 +37,9 @@
|
|||
"update_network:router:external": "rule:admin_only",
|
||||
"delete_network": "rule:admin_or_owner",
|
||||
|
||||
"network_device": "field:port:device_owner=~^network:",
|
||||
"create_port": "",
|
||||
"create_port:device_owner": "not rule:network_device or rule:admin_or_network_owner",
|
||||
"create_port:mac_address": "rule:admin_or_network_owner",
|
||||
"create_port:fixed_ips": "rule:admin_or_network_owner",
|
||||
"create_port:port_security_enabled": "rule:admin_or_network_owner",
|
||||
|
@ -51,6 +53,7 @@
|
|||
"get_port:binding:host_id": "rule:admin_only",
|
||||
"get_port:binding:profile": "rule:admin_only",
|
||||
"update_port": "rule:admin_or_owner",
|
||||
"update_port:device_owner": "not rule:network_device or rule:admin_or_network_owner",
|
||||
"update_port:fixed_ips": "rule:admin_or_network_owner",
|
||||
"update_port:port_security_enabled": "rule:admin_or_network_owner",
|
||||
"update_port:binding:host_id": "rule:admin_only",
|
||||
|
|
|
@ -751,7 +751,7 @@ RESOURCE_ATTRIBUTE_MAP = {
|
|||
'is_visible': True},
|
||||
'device_owner': {'allow_post': True, 'allow_put': True,
|
||||
'validate': {'type:string': None},
|
||||
'default': '',
|
||||
'default': '', 'enforce_policy': True,
|
||||
'is_visible': True},
|
||||
'tenant_id': {'allow_post': True, 'allow_put': False,
|
||||
'validate': {'type:string': None},
|
||||
|
|
|
@ -335,6 +335,7 @@ class FieldCheck(policy.Check):
|
|||
|
||||
self.field = field
|
||||
self.value = conv_func(value)
|
||||
self.regex = re.compile(value[1:]) if value.startswith('~') else None
|
||||
|
||||
def __call__(self, target_dict, cred_dict):
|
||||
target_value = target_dict.get(self.field)
|
||||
|
@ -345,6 +346,8 @@ class FieldCheck(policy.Check):
|
|||
{'field': self.field,
|
||||
'target_dict': target_dict})
|
||||
return False
|
||||
if self.regex:
|
||||
return bool(self.regex.match(target_value))
|
||||
return target_value == self.value
|
||||
|
||||
|
||||
|
|
|
@ -241,6 +241,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
|
|||
"regular_user": "role:user",
|
||||
"shared": "field:networks:shared=True",
|
||||
"external": "field:networks:router:external=True",
|
||||
"network_device": "field:port:device_owner=~^network:",
|
||||
"default": '@',
|
||||
|
||||
"create_network": "rule:admin_or_owner",
|
||||
|
@ -251,7 +252,9 @@ class NeutronPolicyTestCase(base.BaseTestCase):
|
|||
"get_network": "rule:admin_or_owner or "
|
||||
"rule:shared or "
|
||||
"rule:external",
|
||||
"create_port": "",
|
||||
"create_port:mac": "rule:admin_or_network_owner",
|
||||
"create_port:device_owner": "not rule:network_device",
|
||||
"create_something": "rule:admin_or_owner",
|
||||
"create_something:attr": "rule:admin_or_owner",
|
||||
"create_something:attr:sub_attr_1": "rule:admin_or_owner",
|
||||
|
@ -313,6 +316,23 @@ class NeutronPolicyTestCase(base.BaseTestCase):
|
|||
self._test_nonadmin_action_on_attr('create', 'shared', True,
|
||||
exceptions.PolicyNotAuthorized)
|
||||
|
||||
def test_create_port_device_owner_regex(self):
|
||||
uctx = context.Context('', "user", roles=['user'])
|
||||
action = 'create_port'
|
||||
target = {'tenant_id': 'user'}
|
||||
blocked_values = ('network:', 'network:abdef', 'network:dhcp',
|
||||
'network:router_interface')
|
||||
for val in blocked_values:
|
||||
target['device_owner'] = val
|
||||
self.assertRaises(
|
||||
exceptions.PolicyNotAuthorized, policy.enforce,
|
||||
uctx, action, target
|
||||
)
|
||||
ok_values = ('network', 'networks', 'my_network:test', 'my_network:')
|
||||
for val in ok_values:
|
||||
target['device_owner'] = val
|
||||
self.assertTrue(policy.enforce(uctx, action, target))
|
||||
|
||||
def test_nonadmin_read_on_shared_succeeds(self):
|
||||
self._test_nonadmin_action_on_attr('get', 'shared', True)
|
||||
|
||||
|
|
Loading…
Reference in New Issue