|
1 This patch addresses CVE-2014-6414 and is tracked under Launchpad bug |
|
2 1357379. It is addressed in the Juno trunk and Icehouse 2014.1.3. There |
|
3 is no patch for Havana since it is EOL'ed by upstream. Therefore, this |
|
4 patch is derived from the patch for Icehouse 2014.1.3 |
|
5 |
|
6 commit dd4b77ff53479389a5af8b45fd95ee2987562f29 |
|
7 Author: Elena Ezhova <[email protected]> |
|
8 Date: Tue Aug 19 15:54:36 2014 +0400 |
|
9 |
|
10 Forbid regular users to reset admin-only attrs to default values |
|
11 |
|
12 A regular user can reset an admin-only attribute to its default |
|
13 value due to the fact that a corresponding policy rule is |
|
14 enforced only in the case when an attribute is present in the |
|
15 target AND has a non-default value. |
|
16 |
|
17 Added a new attribute "attributes_to_update" which contains a list |
|
18 of all to-be updated attributes to the body of the target that is |
|
19 passed to policy.enforce. |
|
20 |
|
21 Changed a check for whether an attribute is explicitly set. |
|
22 Now, in the case of update, the function should not pay attention |
|
23 to a default value of an attribute, but check whether it was |
|
24 explicitly marked as being updated. |
|
25 |
|
26 Added unit-tests. |
|
27 |
|
28 Conflicts: |
|
29 neutron/common/constants.py |
|
30 |
|
31 Closes-Bug: #1357379 |
|
32 Related-Bug: #1338880 |
|
33 Change-Id: I6537bb1da5ef0d6899bc71e4e949f2c760c103c2 |
|
34 (cherry picked from commit 74d10939903984d5f06c1749a8707fa3257e44ff) |
|
35 |
|
36 --- neutron-2013.2.3/neutron/api/v2/base.py 2014-04-03 11:49:01.000000000 -0700 |
|
37 +++ NEW/neutron/api/v2/base.py 2014-09-30 10:03:51.003850651 -0700 |
|
38 @@ -24,6 +24,7 @@ |
|
39 from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api |
|
40 from neutron.api.v2 import attributes |
|
41 from neutron.api.v2 import resource as wsgi_resource |
|
42 +from neutron.common import constants as const |
|
43 from neutron.common import exceptions |
|
44 from neutron.openstack.common import log as logging |
|
45 from neutron.openstack.common.notifier import api as notifier_api |
|
46 @@ -469,6 +470,10 @@ |
|
47 orig_obj = self._item(request, id, field_list=field_list, |
|
48 parent_id=parent_id) |
|
49 orig_obj.update(body[self._resource]) |
|
50 + # Make a list of attributes to be updated to inform the policy engine |
|
51 + # which attributes are set explicitly so that it can distinguish them |
|
52 + # from the ones that are set to their default values. |
|
53 + orig_obj[const.ATTRIBUTES_TO_UPDATE] = body[self._resource].keys() |
|
54 try: |
|
55 policy.enforce(request.context, |
|
56 action, |
|
57 --- neutron-2013.2.3/neutron/common/constants.py 2014-04-03 11:49:01.000000000 -0700 |
|
58 +++ NEW/neutron/common/constants.py 2014-09-30 10:24:19.837488691 -0700 |
|
59 @@ -92,3 +92,5 @@ |
|
60 PROTO_NUM_ICMP = 1 |
|
61 PROTO_NUM_ICMP_V6 = 58 |
|
62 PROTO_NUM_UDP = 17 |
|
63 + |
|
64 +ATTRIBUTES_TO_UPDATE = 'attributes_to_update' |
|
65 --- neutron-2013.2.3/neutron/policy.py 2014-04-03 11:49:01.000000000 -0700 |
|
66 +++ NEW/neutron/policy.py 2014-09-30 10:12:16.819335023 -0700 |
|
67 @@ -18,12 +18,15 @@ |
|
68 """ |
|
69 Policy engine for neutron. Largely copied from nova. |
|
70 """ |
|
71 + |
|
72 +import collections |
|
73 import itertools |
|
74 import re |
|
75 |
|
76 from oslo.config import cfg |
|
77 |
|
78 from neutron.api.v2 import attributes |
|
79 +from neutron.common import constants as const |
|
80 from neutron.common import exceptions |
|
81 import neutron.common.utils as utils |
|
82 from neutron import manager |
|
83 @@ -118,14 +121,28 @@ |
|
84 policy.set_rules(policies) |
|
85 |
|
86 |
|
87 -def _is_attribute_explicitly_set(attribute_name, resource, target): |
|
88 - """Verify that an attribute is present and has a non-default value.""" |
|
89 +def _is_attribute_explicitly_set(attribute_name, resource, target, action): |
|
90 + """Verify that an attribute is present and is explicitly set.""" |
|
91 + if 'update' in action: |
|
92 + # In the case of update, the function should not pay attention to a |
|
93 + # default value of an attribute, but check whether it was explicitly |
|
94 + # marked as being updated instead. |
|
95 + return (attribute_name in target[const.ATTRIBUTES_TO_UPDATE] and |
|
96 + target[attribute_name] is not attributes.ATTR_NOT_SPECIFIED) |
|
97 return ('default' in resource[attribute_name] and |
|
98 attribute_name in target and |
|
99 target[attribute_name] is not attributes.ATTR_NOT_SPECIFIED and |
|
100 target[attribute_name] != resource[attribute_name]['default']) |
|
101 |
|
102 |
|
103 +def _should_validate_sub_attributes(attribute, sub_attr): |
|
104 + """Verify that sub-attributes are iterable and should be validated.""" |
|
105 + validate = attribute.get('validate') |
|
106 + return (validate and isinstance(sub_attr, collections.Iterable) and |
|
107 + any([k.startswith('type:dict') and |
|
108 + v for (k, v) in validate.iteritems()])) |
|
109 + |
|
110 + |
|
111 def _build_subattr_match_rule(attr_name, attr, action, target): |
|
112 """Create the rule to match for sub-attribute policy checks.""" |
|
113 # TODO(salv-orlando): Instead of relying on validator info, introduce |
|
114 @@ -174,16 +191,14 @@ |
|
115 for attribute_name in res_map[resource]: |
|
116 if _is_attribute_explicitly_set(attribute_name, |
|
117 res_map[resource], |
|
118 - target): |
|
119 + target, action): |
|
120 attribute = res_map[resource][attribute_name] |
|
121 if 'enforce_policy' in attribute: |
|
122 attr_rule = policy.RuleCheck('rule', '%s:%s' % |
|
123 (action, attribute_name)) |
|
124 - # Build match entries for sub-attributes, if present |
|
125 - validate = attribute.get('validate') |
|
126 - if (validate and any([k.startswith('type:dict') and v |
|
127 - for (k, v) in |
|
128 - validate.iteritems()])): |
|
129 + # Build match entries for sub-attributes |
|
130 + if _should_validate_sub_attributes( |
|
131 + attribute, target[attribute_name]): |
|
132 attr_rule = policy.AndCheck( |
|
133 [attr_rule, _build_subattr_match_rule( |
|
134 attribute_name, attribute, |
|
135 --- neutron-2013.2.3/neutron/tests/unit/test_api_v2.py 2014-04-03 11:49:01.000000000 -0700 |
|
136 +++ NEW/neutron/tests/unit/test_api_v2.py 2014-09-30 10:06:01.484803124 -0700 |
|
137 @@ -1210,6 +1210,18 @@ |
|
138 network_id='id1', |
|
139 dummy=body) |
|
140 |
|
141 + def test_update_subresource_to_none(self): |
|
142 + instance = self.plugin.return_value |
|
143 + |
|
144 + dummy_id = _uuid() |
|
145 + body = {'dummy': {}} |
|
146 + self.api.put_json('/networks/id1' + _get_path('dummies', id=dummy_id), |
|
147 + body) |
|
148 + instance.update_network_dummy.assert_called_once_with(mock.ANY, |
|
149 + dummy_id, |
|
150 + network_id='id1', |
|
151 + dummy=body) |
|
152 + |
|
153 def test_delete_sub_resource(self): |
|
154 instance = self.plugin.return_value |
|
155 |
|
156 --- neutron-2013.2.3/neutron/tests/unit/test_policy.py 2014-04-03 11:49:01.000000000 -0700 |
|
157 +++ NEW/neutron/tests/unit/test_policy.py 2014-09-30 10:09:03.707133011 -0700 |
|
158 @@ -24,6 +24,7 @@ |
|
159 |
|
160 import neutron |
|
161 from neutron.api.v2 import attributes |
|
162 +from neutron.common import constants as const |
|
163 from neutron.common import exceptions |
|
164 from neutron import context |
|
165 from neutron import manager |
|
166 @@ -280,9 +281,11 @@ |
|
167 self.addCleanup(self.manager_patcher.stop) |
|
168 |
|
169 def _test_action_on_attr(self, context, action, attr, value, |
|
170 - exception=None): |
|
171 + exception=None, **kwargs): |
|
172 action = "%s_network" % action |
|
173 target = {'tenant_id': 'the_owner', attr: value} |
|
174 + if kwargs: |
|
175 + target.update(kwargs) |
|
176 if exception: |
|
177 self.assertRaises(exception, policy.enforce, |
|
178 context, action, target) |
|
179 @@ -291,10 +294,10 @@ |
|
180 self.assertEqual(result, True) |
|
181 |
|
182 def _test_nonadmin_action_on_attr(self, action, attr, value, |
|
183 - exception=None): |
|
184 + exception=None, **kwargs): |
|
185 user_context = context.Context('', "user", roles=['user']) |
|
186 self._test_action_on_attr(user_context, action, attr, |
|
187 - value, exception) |
|
188 + value, exception, **kwargs) |
|
189 |
|
190 def test_nonadmin_write_on_private_fails(self): |
|
191 self._test_nonadmin_action_on_attr('create', 'shared', False, |
|
192 @@ -311,9 +314,11 @@ |
|
193 def test_nonadmin_read_on_shared_succeeds(self): |
|
194 self._test_nonadmin_action_on_attr('get', 'shared', True) |
|
195 |
|
196 - def _test_enforce_adminonly_attribute(self, action): |
|
197 + def _test_enforce_adminonly_attribute(self, action, **kwargs): |
|
198 admin_context = context.get_admin_context() |
|
199 target = {'shared': True} |
|
200 + if kwargs: |
|
201 + target.update(kwargs) |
|
202 result = policy.enforce(admin_context, action, target) |
|
203 self.assertEqual(result, True) |
|
204 |
|
205 @@ -321,7 +326,14 @@ |
|
206 self._test_enforce_adminonly_attribute('create_network') |
|
207 |
|
208 def test_enforce_adminonly_attribute_update(self): |
|
209 - self._test_enforce_adminonly_attribute('update_network') |
|
210 + kwargs = {const.ATTRIBUTES_TO_UPDATE: ['shared']} |
|
211 + self._test_enforce_adminonly_attribute('update_network', **kwargs) |
|
212 + |
|
213 + def test_reset_adminonly_attr_to_default_fails(self): |
|
214 + kwargs = {const.ATTRIBUTES_TO_UPDATE: ['shared']} |
|
215 + self._test_nonadmin_action_on_attr('update', 'shared', False, |
|
216 + exceptions.PolicyNotAuthorized, |
|
217 + **kwargs) |
|
218 |
|
219 def test_enforce_adminonly_attribute_no_context_is_admin_policy(self): |
|
220 del self.rules[policy.ADMIN_CTX_POLICY] |