1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
|
From 8138e2fe38ad2cde5963685df47b1e4286776352 Mon Sep 17 00:00:00 2001
From: Kevin Benton <blak111@gmail.com>
Date: Tue, 25 Aug 2015 22:03:27 -0700
Subject: [PATCH] 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.
Closes-Bug: #1489111
Change-Id: Ia64cf16142e0e4be44b5b0ed72c8e00792d770f9
(cherry picked from commit 959a2f28cbbfc309381ea9ffb55090da6fb9c78f)
---
etc/policy.json | 3 +++
neutron/api/v2/attributes.py | 2 +-
neutron/policy.py | 3 +++
neutron/tests/etc/policy.json | 3 +++
neutron/tests/unit/test_policy.py | 16 ++++++++++++++++
5 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/etc/policy.json b/etc/policy.json
index 8a5de9b..0f04eb2 100644
--- a/etc/policy.json
+++ b/etc/policy.json
@@ -46,7 +46,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",
@@ -61,6 +63,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",
diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
index b9c179a..9ceee78 100644
--- a/neutron/api/v2/attributes.py
+++ b/neutron/api/v2/attributes.py
@@ -766,7 +766,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},
diff --git a/neutron/policy.py b/neutron/policy.py
index 9e586dd..961ae21 100644
--- a/neutron/policy.py
+++ b/neutron/policy.py
@@ -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, enforcer):
target_value = target_dict.get(self.field)
@@ -344,6 +345,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
diff --git a/neutron/tests/etc/policy.json b/neutron/tests/etc/policy.json
index 8a5de9b..0f04eb2 100644
--- a/neutron/tests/etc/policy.json
+++ b/neutron/tests/etc/policy.json
@@ -46,7 +46,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",
@@ -61,6 +63,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",
diff --git a/neutron/tests/unit/test_policy.py b/neutron/tests/unit/test_policy.py
index 3888ce3..4be404f 100644
--- a/neutron/tests/unit/test_policy.py
+++ b/neutron/tests/unit/test_policy.py
@@ -232,6 +232,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",
@@ -243,6 +244,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",
@@ -312,6 +314,20 @@ class NeutronPolicyTestCase(base.BaseTestCase):
self._test_nonadmin_action_on_attr('create', 'shared', True,
common_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,
+ common_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)
--
1.9.1
|
GitWeb