19716228 problem in SERVICE/NEUTRON s11-update
authorGirish Moodalbail <Girish.Moodalbail@oracle.COM>
Fri, 03 Oct 2014 14:30:07 -0700
branchs11-update
changeset 3366 dba288608e69
parent 3359 a3bb8489d586
child 3367 ed5024e47b53
19716228 problem in SERVICE/NEUTRON
components/openstack/neutron/patches/07-CVE-2014-6414.patch
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/openstack/neutron/patches/07-CVE-2014-6414.patch	Fri Oct 03 14:30:07 2014 -0700
@@ -0,0 +1,220 @@
+This patch addresses CVE-2014-6414 and is tracked under Launchpad bug
+1357379. It is addressed in the Juno trunk and Icehouse 2014.1.3. There
+is no patch for Havana since it is EOL'ed by upstream. Therefore, this
+patch is derived from the patch for Icehouse 2014.1.3
+
+commit dd4b77ff53479389a5af8b45fd95ee2987562f29
+Author: Elena Ezhova <[email protected]>
+Date:   Tue Aug 19 15:54:36 2014 +0400
+
+    Forbid regular users to reset admin-only attrs to default values
+
+    A regular user can reset an admin-only attribute to its default
+    value due to the fact that a corresponding policy rule is
+    enforced only in the case when an attribute is present in the
+    target AND has a non-default value.
+
+    Added a new attribute "attributes_to_update" which contains a list
+    of all to-be updated attributes to the body of the target that is
+    passed to policy.enforce.
+
+    Changed a check for whether an attribute is explicitly set.
+    Now, in the case of update, the function should not pay attention
+    to a default value of an attribute, but check whether it was
+    explicitly marked as being updated.
+
+    Added unit-tests.
+
+    Conflicts:
+        neutron/common/constants.py
+
+    Closes-Bug: #1357379
+    Related-Bug: #1338880
+    Change-Id: I6537bb1da5ef0d6899bc71e4e949f2c760c103c2
+    (cherry picked from commit 74d10939903984d5f06c1749a8707fa3257e44ff)
+
+--- neutron-2013.2.3/neutron/api/v2/base.py	2014-04-03 11:49:01.000000000 -0700
++++ NEW/neutron/api/v2/base.py	2014-09-30 10:03:51.003850651 -0700
+@@ -24,6 +24,7 @@
+ from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api
+ from neutron.api.v2 import attributes
+ from neutron.api.v2 import resource as wsgi_resource
++from neutron.common import constants as const
+ from neutron.common import exceptions
+ from neutron.openstack.common import log as logging
+ from neutron.openstack.common.notifier import api as notifier_api
+@@ -469,6 +470,10 @@
+         orig_obj = self._item(request, id, field_list=field_list,
+                               parent_id=parent_id)
+         orig_obj.update(body[self._resource])
++        # Make a list of attributes to be updated to inform the policy engine
++        # which attributes are set explicitly so that it can distinguish them
++        # from the ones that are set to their default values.
++        orig_obj[const.ATTRIBUTES_TO_UPDATE] = body[self._resource].keys()
+         try:
+             policy.enforce(request.context,
+                            action,
+--- neutron-2013.2.3/neutron/common/constants.py	2014-04-03 11:49:01.000000000 -0700
++++ NEW/neutron/common/constants.py	2014-09-30 10:24:19.837488691 -0700
+@@ -92,3 +92,5 @@
+ PROTO_NUM_ICMP = 1
+ PROTO_NUM_ICMP_V6 = 58
+ PROTO_NUM_UDP = 17
++
++ATTRIBUTES_TO_UPDATE = 'attributes_to_update'
+--- neutron-2013.2.3/neutron/policy.py	2014-04-03 11:49:01.000000000 -0700
++++ NEW/neutron/policy.py	2014-09-30 10:12:16.819335023 -0700
+@@ -18,12 +18,15 @@
+ """
+ Policy engine for neutron.  Largely copied from nova.
+ """
++
++import collections
+ import itertools
+ import re
+ 
+ from oslo.config import cfg
+ 
+ from neutron.api.v2 import attributes
++from neutron.common import constants as const
+ from neutron.common import exceptions
+ import neutron.common.utils as utils
+ from neutron import manager
+@@ -118,14 +121,28 @@
+     policy.set_rules(policies)
+ 
+ 
+-def _is_attribute_explicitly_set(attribute_name, resource, target):
+-    """Verify that an attribute is present and has a non-default value."""
++def _is_attribute_explicitly_set(attribute_name, resource, target, action):
++    """Verify that an attribute is present and is explicitly set."""
++    if 'update' in action:
++        # In the case of update, the function should not pay attention to a
++        # default value of an attribute, but check whether it was explicitly
++        # marked as being updated instead.
++        return (attribute_name in target[const.ATTRIBUTES_TO_UPDATE] and
++                target[attribute_name] is not attributes.ATTR_NOT_SPECIFIED)
+     return ('default' in resource[attribute_name] and
+             attribute_name in target and
+             target[attribute_name] is not attributes.ATTR_NOT_SPECIFIED and
+             target[attribute_name] != resource[attribute_name]['default'])
+ 
+ 
++def _should_validate_sub_attributes(attribute, sub_attr):
++    """Verify that sub-attributes are iterable and should be validated."""
++    validate = attribute.get('validate')
++    return (validate and isinstance(sub_attr, collections.Iterable) and
++            any([k.startswith('type:dict') and
++                 v for (k, v) in validate.iteritems()]))
++
++
+ def _build_subattr_match_rule(attr_name, attr, action, target):
+     """Create the rule to match for sub-attribute policy checks."""
+     # TODO(salv-orlando): Instead of relying on validator info, introduce
+@@ -174,16 +191,14 @@
+             for attribute_name in res_map[resource]:
+                 if _is_attribute_explicitly_set(attribute_name,
+                                                 res_map[resource],
+-                                                target):
++                                                target, action):
+                     attribute = res_map[resource][attribute_name]
+                     if 'enforce_policy' in attribute:
+                         attr_rule = policy.RuleCheck('rule', '%s:%s' %
+                                                      (action, attribute_name))
+-                        # Build match entries for sub-attributes, if present
+-                        validate = attribute.get('validate')
+-                        if (validate and any([k.startswith('type:dict') and v
+-                                              for (k, v) in
+-                                              validate.iteritems()])):
++                        # Build match entries for sub-attributes
++                        if _should_validate_sub_attributes(
++                                attribute, target[attribute_name]):
+                             attr_rule = policy.AndCheck(
+                                 [attr_rule, _build_subattr_match_rule(
+                                     attribute_name, attribute,
+--- neutron-2013.2.3/neutron/tests/unit/test_api_v2.py	2014-04-03 11:49:01.000000000 -0700
++++ NEW/neutron/tests/unit/test_api_v2.py	2014-09-30 10:06:01.484803124 -0700
+@@ -1210,6 +1210,18 @@
+                                                               network_id='id1',
+                                                               dummy=body)
+ 
++    def test_update_subresource_to_none(self):
++        instance = self.plugin.return_value
++
++        dummy_id = _uuid()
++        body = {'dummy': {}}
++        self.api.put_json('/networks/id1' + _get_path('dummies', id=dummy_id),
++                          body)
++        instance.update_network_dummy.assert_called_once_with(mock.ANY,
++                                                              dummy_id,
++                                                              network_id='id1',
++                                                              dummy=body)
++
+     def test_delete_sub_resource(self):
+         instance = self.plugin.return_value
+ 
+--- neutron-2013.2.3/neutron/tests/unit/test_policy.py	2014-04-03 11:49:01.000000000 -0700
++++ NEW/neutron/tests/unit/test_policy.py	2014-09-30 10:09:03.707133011 -0700
+@@ -24,6 +24,7 @@
+ 
+ import neutron
+ from neutron.api.v2 import attributes
++from neutron.common import constants as const
+ from neutron.common import exceptions
+ from neutron import context
+ from neutron import manager
+@@ -280,9 +281,11 @@
+         self.addCleanup(self.manager_patcher.stop)
+ 
+     def _test_action_on_attr(self, context, action, attr, value,
+-                             exception=None):
++                             exception=None, **kwargs):
+         action = "%s_network" % action
+         target = {'tenant_id': 'the_owner', attr: value}
++        if kwargs:
++            target.update(kwargs)
+         if exception:
+             self.assertRaises(exception, policy.enforce,
+                               context, action, target)
+@@ -291,10 +294,10 @@
+             self.assertEqual(result, True)
+ 
+     def _test_nonadmin_action_on_attr(self, action, attr, value,
+-                                      exception=None):
++                                      exception=None, **kwargs):
+         user_context = context.Context('', "user", roles=['user'])
+         self._test_action_on_attr(user_context, action, attr,
+-                                  value, exception)
++                                  value, exception, **kwargs)
+ 
+     def test_nonadmin_write_on_private_fails(self):
+         self._test_nonadmin_action_on_attr('create', 'shared', False,
+@@ -311,9 +314,11 @@
+     def test_nonadmin_read_on_shared_succeeds(self):
+         self._test_nonadmin_action_on_attr('get', 'shared', True)
+ 
+-    def _test_enforce_adminonly_attribute(self, action):
++    def _test_enforce_adminonly_attribute(self, action, **kwargs):
+         admin_context = context.get_admin_context()
+         target = {'shared': True}
++        if kwargs:
++            target.update(kwargs)
+         result = policy.enforce(admin_context, action, target)
+         self.assertEqual(result, True)
+ 
+@@ -321,7 +326,14 @@
+         self._test_enforce_adminonly_attribute('create_network')
+ 
+     def test_enforce_adminonly_attribute_update(self):
+-        self._test_enforce_adminonly_attribute('update_network')
++        kwargs = {const.ATTRIBUTES_TO_UPDATE: ['shared']}
++        self._test_enforce_adminonly_attribute('update_network', **kwargs)
++
++    def test_reset_adminonly_attr_to_default_fails(self):
++        kwargs = {const.ATTRIBUTES_TO_UPDATE: ['shared']}
++        self._test_nonadmin_action_on_attr('update', 'shared', False,
++                                           exceptions.PolicyNotAuthorized,
++                                           **kwargs)
+ 
+     def test_enforce_adminonly_attribute_no_context_is_admin_policy(self):
+         del self.rules[policy.ADMIN_CTX_POLICY]