[gentoo-commits] gentoo-x86 commit in sys-cluster/neutron/files: 2013.2.2-CVE-2014-0056.patch

[Matt Thode (prometheanfire)]

prometheanfire    14/03/27 22:34:02

  Added:                2013.2.2-CVE-2014-0056.patch
  Log:
  fix for bug 505980 CVE-2014-0056
  
  (Portage version: 2.2.8-r1/cvs/Linux x86_64, signed Manifest commit with key 0x2471eb3e40ac5ac3)

Revision  Changes    Path
1.1                  sys-cluster/neutron/files/2013.2.2-CVE-2014-0056.patch

file : http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-cluster/neutron/files/2013.2.2-CVE-2014-0056.patch?rev=1.1&view=markup
plain: http://sources.gentoo.org/viewvc.cgi/gentoo-x86/sys-cluster/neutron/files/2013.2.2-CVE-2014-0056.patch?rev=1.1&content-type=text/plain

Index: 2013.2.2-CVE-2014-0056.patch
===================================================================
From 1faec8354a0fab953524eaeb6042ad38461a58bc Mon Sep 17 00:00:00 2001
From: Aaron Rosen <aaronorosen@×××××.com>
Date: Wed, 26 Mar 2014 16:36:56 -0700
Subject: [PATCH] Prevent cross plugging router ports from other tenants

Previously, a tenant could plug an interface into another tenant's
router if he knew their router_id by creating a port with the correct
device_id and device_owner. This patch prevents this from occuring
by preventing non-admin users from creating ports with device_owner
network:router_interface with a device_id that matches another tenants router.
In addition, it prevents one from updating a ports device_owner and device_id
so that the device_id won't match another tenants router with device_owner
being network:router_interface.

NOTE: with this change it does open up the possiblity for a tenant to discover
router_id's of another tenant's by guessing them and updating a port till
a conflict occurs. That said, randomly guessing the router id would be hard
and in theory should not matter if exposed. We also need to allow a tenant
to update the device_id on network:router_interface ports as this would be
used for by anyone using a vm as a service router. This issue will be fixed in
another patch upstream as a db migration is required but since this needs
to be backported to all stable branches this is not possible.

NOTE: The only plugins affect by this are the ones that use the l3-agent.

NOTE: **One should perform and audit of the ports that are already
        attached to routers after applying this patch and remove ports
        that a tenant may have cross plugged.**

Closes-bug: #1243327

Conflicts:
    neutron/common/exceptions.py
    neutron/db/db_base_plugin_v2.py

Change-Id: I8bc6241f537d937e5729072dcc76871bf407cdb3
---
 neutron/common/exceptions.py         |  5 +++
 neutron/db/db_base_plugin_v2.py      | 62 +++++++++++++++++++++++++++++++++++
 neutron/tests/unit/test_l3_plugin.py | 63 +++++++++++++++++++++++++++++++++++-
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index 7b02647..88fa6e4 100644
--- a/neutron/common/exceptions.py
+++ b/neutron/common/exceptions.py
@@ -301,3 +301,8 @@ def __init__(self, **kwargs):
 
 class NetworkVxlanPortRangeError(object):
     message = _("Invalid network VXLAN port range: '%(vxlan_range)s'")
+
+
+class DeviceIDNotOwnedByTenant(Conflict):
+    message = _("The following device_id %(device_id)s is not owned by your "
+                "tenant or matches another tenants router.")
diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py
index 2afbac5..872463f 100644
--- a/neutron/db/db_base_plugin_v2.py
+++ b/neutron/db/db_base_plugin_v2.py
@@ -27,14 +27,18 @@
 from neutron.api.v2 import attributes
 from neutron.common import constants
 from neutron.common import exceptions as q_exc
+from neutron import context as ctx
 from neutron.db import api as db
 from neutron.db import models_v2
 from neutron.db import sqlalchemyutils
+from neutron.extensions import l3
+from neutron import manager
 from neutron import neutron_plugin_base_v2
 from neutron.openstack.common import excutils
 from neutron.openstack.common import log as logging
 from neutron.openstack.common import timeutils
 from neutron.openstack.common import uuidutils
+from neutron.plugins.common import constants as service_constants
 
 
 LOG = logging.getLogger(__name__)
@@ -1311,6 +1315,9 @@ def create_port(self, context, port):
         # NOTE(jkoelker) Get the tenant_id outside of the session to avoid
         #                unneeded db action if the operation raises
         tenant_id = self._get_tenant_id_for_create(context, p)
+        if p.get('device_owner') == constants.DEVICE_OWNER_ROUTER_INTF:
+            self._enforce_device_owner_not_router_intf_or_device_id(context, p,
+                                                                    tenant_id)
 
         with context.session.begin(subtransactions=True):
             network = self._get_network(context, network_id)
@@ -1374,6 +1381,23 @@ def update_port(self, context, id, port):
         changed_ips = False
         with context.session.begin(subtransactions=True):
             port = self._get_port(context, id)
+            if 'device_owner' in p:
+                current_device_owner = p['device_owner']
+                changed_device_owner = True
+            else:
+                current_device_owner = port['device_owner']
+                changed_device_owner = False
+            if p.get('device_id') != port['device_id']:
+                changed_device_id = True
+
+            # if the current device_owner is ROUTER_INF and the device_id or
+            # device_owner changed check device_id is not another tenants
+            # router
+            if ((current_device_owner == constants.DEVICE_OWNER_ROUTER_INTF)
+                    and (changed_device_id or changed_device_owner)):
+                self._enforce_device_owner_not_router_intf_or_device_id(
+                    context, p, port['tenant_id'], port)
+
             # Check if the IPs need to be updated
             if 'fixed_ips' in p:
                 changed_ips = True
@@ -1483,3 +1507,41 @@ def get_ports(self, context, filters=None, fields=None,
 
     def get_ports_count(self, context, filters=None):
         return self._get_ports_query(context, filters).count()
+
+    def _enforce_device_owner_not_router_intf_or_device_id(self, context,
+                                                           port_request,
+                                                           tenant_id,
+                                                           db_port=None):
+        if not context.is_admin:
+            # find the device_id. If the call was update_port and the
+            # device_id was not passed in we use the device_id from the
+            # db.
+            device_id = port_request.get('device_id')
+            if not device_id and db_port:
+                device_id = db_port.get('device_id')
+            # check to make sure device_id does not match another tenants
+            # router.
+            if device_id:
+                if hasattr(self, 'get_router'):
+                    try:
+                        ctx_admin = ctx.get_admin_context()
+                        router = self.get_router(ctx_admin, device_id)
+                    except l3.RouterNotFound:
+                        return
+                else:
+                    l3plugin = (
+                        manager.NeutronManager.get_service_plugins().get(
+                            service_constants.L3_ROUTER_NAT))
+                    if l3plugin:
+                        try:
+                            ctx_admin = ctx.get_admin_context()
+                            router = l3plugin.get_router(ctx_admin,
+                                                         device_id)
+                        except l3.RouterNotFound:
+                            return
+                    else:
+                        # raise as extension doesn't support L3 anyways.
+                        raise q_exc.DeviceIDNotOwnedByTenant(
+                            device_id=device_id)
+                if tenant_id != router['tenant_id']:
+                    raise q_exc.DeviceIDNotOwnedByTenant(device_id=device_id)
diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py
index 4f75b57..9cc5cf9 100644
--- a/neutron/tests/unit/test_l3_plugin.py
+++ b/neutron/tests/unit/test_l3_plugin.py
@@ -379,7 +379,8 @@ def _remove_external_gateway_from_router(self, router_id, network_id,
 
     def _router_interface_action(self, action, router_id, subnet_id, port_id,
                                  expected_code=exc.HTTPOk.code,
-                                 expected_body=None):
+                                 expected_body=None,
+                                 tenant_id=None):
         interface_data = {}
         if subnet_id:
             interface_data.update({'subnet_id': subnet_id})
@@ -388,6 +389,10 @@ def _router_interface_action(self, action, router_id, subnet_id, port_id,
 
         req = self.new_action_request('routers', interface_data, router_id,
                                       "%s_router_interface" % action)
+        # if tenant_id was specified, create a tenant context for this request
+        if tenant_id:
+            req.environ['neutron.context'] = context.Context(
+                '', tenant_id)
         res = req.get_response(self.ext_api)
         self.assertEqual(res.status_int, expected_code)
         response = self.deserialize(self.fmt, res)
@@ -968,6 +973,62 @@ def test_router_add_gateway_tenant_ctx(self):
                 gw_info = body['router']['external_gateway_info']
                 self.assertEqual(gw_info, None)
 
+    def test_create_router_port_with_device_id_of_other_teants_router(self):
+        with self.router() as admin_router:
+            with self.network(tenant_id='tenant_a',
+                              set_context=True) as n:
+                with self.subnet(network=n):
+                    self._create_port(
+                        self.fmt, n['network']['id'],
+                        tenant_id='tenant_a',
+                        device_id=admin_router['router']['id'],
+                        device_owner='network:router_interface',
+                        set_context=True,
+                        expected_res_status=exc.HTTPConflict.code)
+
+    def test_create_non_router_port_device_id_of_other_teants_router_update(
+        self):
+        # This tests that HTTPConflict is raised if we create a non-router
+        # port that matches the device_id of another tenants router and then
+        # we change the device_owner to be network:router_interface.
+        with self.router() as admin_router:
+            with self.network(tenant_id='tenant_a',
+                              set_context=True) as n:
+                with self.subnet(network=n):
+                    port_res = self._create_port(
+                        self.fmt, n['network']['id'],
+                        tenant_id='tenant_a',
+                        device_id=admin_router['router']['id'],
+                        set_context=True)
+                    port = self.deserialize(self.fmt, port_res)
+                    neutron_context = context.Context('', 'tenant_a')
+                    data = {'port': {'device_owner':
+                                     'network:router_interface'}}
+                    self._update('ports', port['port']['id'], data,
+                                 neutron_context=neutron_context,
+                                 expected_code=exc.HTTPConflict.code)
+                    self._delete('ports', port['port']['id'])
+
+    def test_update_port_device_id_to_different_tenants_router(self):
+        with self.router() as admin_router:
+            with self.router(tenant_id='tenant_a',
+                             set_context=True) as tenant_router:
+                with self.network(tenant_id='tenant_a',
+                                  set_context=True) as n:
+                    with self.subnet(network=n) as s:
+                        port = self._router_interface_action(
+                            'add', tenant_router['router']['id'],
+                            s['subnet']['id'], None, tenant_id='tenant_a')
+                        neutron_context = context.Context('', 'tenant_a')
+                        data = {'port':
+                                {'device_id': admin_router['router']['id']}}
+                        self._update('ports', port['port_id'], data,
+                                     neutron_context=neutron_context,
+                                     expected_code=exc.HTTPConflict.code)
+                        self._router_interface_action(
+                            'remove', tenant_router['router']['id'],
+                            s['subnet']['id'], None, tenant_id='tenant_a')
+
     def test_router_add_gateway_invalid_network_returns_404(self):
         with self.router() as r:
             self._add_external_gateway_to_router(
-- 
1.8.5.5