openstack/neutron
Code Issues Proposed changes

Stop device_owner from being set to 'network:*'

Browse Source 
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
This commit is contained in:
Kevin Benton 2015-08-25 22:03:27 -07:00 committed by Tristan Cacqueray
parent dbe420c2b7
commit bbca973986
5 changed files with 26 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
etc/policy.json
View File

@ -56,7 +56,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 or rule:context_is_advsvc",
"create_port:mac_address": "rule:admin_or_network_owner or rule:context_is_advsvc",
"create_port:fixed_ips": "rule:admin_or_network_owner or rule:context_is_advsvc",
"create_port:port_security_enabled": "rule:admin_or_network_owner or rule:context_is_advsvc",
@ -71,6 +73,7 @@
"get_port:binding:host_id": "rule:admin_only",
"get_port:binding:profile": "rule:admin_only",
"update_port": "rule:admin_or_owner or rule:context_is_advsvc",
"update_port:device_owner": "not rule:network_device or rule:admin_or_network_owner or rule:context_is_advsvc",
"update_port:mac_address": "rule:admin_only or rule:context_is_advsvc",
"update_port:fixed_ips": "rule:admin_or_network_owner or rule:context_is_advsvc",
"update_port:port_security_enabled": "rule:admin_or_network_owner or rule:context_is_advsvc",

2
neutron/api/v2/attributes.py
View File

@ -731,7 +731,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True},
'device_owner': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': DEVICE_OWNER_MAX_LEN},
'default': '',
'default': '', 'enforce_policy': True,
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': TENANT_ID_MAX_LEN},

3
neutron/policy.py
View File

@ -290,6 +290,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, enforcer):
target_value = target_dict.get(self.field)
@ -299,6 +300,8 @@ class FieldCheck(policy.Check):
"%(target_dict)s",
{'field': self.field, 'target_dict': target_dict})
return False
if self.regex:
return bool(self.regex.match(target_value))
return target_value == self.value

3
neutron/tests/etc/policy.json
View File

@ -56,7 +56,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 or rule:context_is_advsvc",
"create_port:mac_address": "rule:admin_or_network_owner or rule:context_is_advsvc",
"create_port:fixed_ips": "rule:admin_or_network_owner or rule:context_is_advsvc",
"create_port:port_security_enabled": "rule:admin_or_network_owner or rule:context_is_advsvc",
@ -71,6 +73,7 @@
"get_port:binding:host_id": "rule:admin_only",
"get_port:binding:profile": "rule:admin_only",
"update_port": "rule:admin_or_owner or rule:context_is_advsvc",
"update_port:device_owner": "not rule:network_device or rule:admin_or_network_owner or rule:context_is_advsvc",
"update_port:mac_address": "rule:admin_only or rule:context_is_advsvc",
"update_port:fixed_ips": "rule:admin_or_network_owner or rule:context_is_advsvc",
"update_port:port_security_enabled": "rule:admin_or_network_owner or rule:context_is_advsvc",

16
neutron/tests/unit/test_policy.py
View File

@ -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",
@ -252,6 +253,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
"create_subnet": "rule:admin_or_network_owner",
"create_port:mac": "rule:admin_or_network_owner or "
"rule:context_is_advsvc",
"create_port:device_owner": "not rule:network_device",
"update_port": "rule:admin_or_owner or rule:context_is_advsvc",
"get_port": "rule:admin_or_owner or rule:context_is_advsvc",
"delete_port": "rule:admin_or_owner or rule:context_is_advsvc",
@ -330,6 +332,20 @@ class NeutronPolicyTestCase(base.BaseTestCase):
self._test_nonadmin_action_on_attr('create', 'shared', True,
oslo_policy.PolicyNotAuthorized)
def test_create_port_device_owner_regex(self):
blocked_values = ('network:', 'network:abdef', 'network:dhcp',
'network:router_interface')
for val in blocked_values:
self._test_advsvc_action_on_attr(
'create', 'port', 'device_owner', val,
oslo_policy.PolicyNotAuthorized
)
ok_values = ('network', 'networks', 'my_network:test', 'my_network:')
for val in ok_values:
self._test_advsvc_action_on_attr(
'create', 'port', 'device_owner', val
)
def test_advsvc_get_network_works(self):
self._test_advsvc_action_on_attr('get', 'network', 'shared', False)