Use network RBAC feature for external access

This allows access to external networks to be controlled via the
RBAC framework added during Liberty with a new 'access_as_external'
action.

A migration adds all current external networks to the RBAC policies
table with a wildcard indicating that all tenants can access the network
as RBAC.

Unlike the conversion of shared networks to RBAC, the external table
is left in the DB to avoid invasive changes throughout the codebase
to calculate the flag relative to the caller. So the current 'external'
flag is used throughout the code base as it previously was for wiring
up floating IPs, router gateway ports, etc. Then the RBAC entries are
only referenced when determining what networks to show the tenants.

API Behavior:
 * Marking a network as 'external' will automatically create a wildcard
   entry that allows that network to be accessed by all tenants.
 * An external network may have all of its RBAC entries deleted and then
   only an admin will be able to attach to it.
 * An RBAC 'access_as_external' entry cannot be deleted if it is required
   for a tenant that currently has a router attached to that network.
 * Creating an 'access_as_external' RBAC entry will automatically convert
   the network into an external network. (This is to enable a workflow
   where a private external network is never visible to everyone.)
 * The default policy.json will prevent a non-admin from creating wildcard
   'access_as_external' RBAC entries to align with the current default policy
   we have on setting the 'external' field on the network to prevent poluting
   everyone else's network lists.
 * The default policy.json will allow a tenant to create an
   'access_as_external' RBAC entry to allow specific tenants
   (including itself) the ability to use its network as an external network.

Closes-Bug: #1547985
DocImpact: External networks can now have access restricted to small subsets
           of tenants
APIImpact: 'access_as_external' will be allowed as an action in the RBAC
           API for networks
Change-Id: I4d8ee78a9763c58884e4fd3d7b40133da659cd61

neutron/db/db_base_plugin_v2.py

@@ -124,8 +124,8 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
         On update and delete, make sure the tenant losing access does not have
         resources that depend on that access.
         """
-        if object_type != 'network':
-            # we only care about network policies
+        if object_type != 'network' or policy['action'] != 'access_as_shared':
+            # we only care about shared network policies
             return
         # The object a policy targets cannot be changed so we can look
         # at the original network for the update event as well.

neutron/db/external_net_db.py

@@ -26,9 +26,12 @@ from neutron.callbacks import resources
 from neutron.common import constants as l3_constants
 from neutron.common import exceptions as n_exc
 from neutron.db import db_base_plugin_v2
+from neutron.db import l3_db
 from neutron.db import model_base
 from neutron.db import models_v2
+from neutron.db import rbac_db_models as rbac_db
 from neutron.extensions import external_net
+from neutron.extensions import rbac as rbac_ext
 from neutron import manager
 from neutron.plugins.common import constants as service_constants
 
@@ -65,8 +68,14 @@ class External_net_db_mixin(object):
         # Apply the external network filter only in non-admin and non-advsvc
         # context
         if self.model_query_scope(context, original_model):
-            conditions = expr.or_(ExternalNetwork.network_id != expr.null(),
-                                  *conditions)
+            # the table will already be joined to the rbac entries for the
+            # shared check so we don't need to worry about ensuring that
+            rbac_model = original_model.rbac_entries.property.mapper.class_
+            tenant_allowed = (
+                (rbac_model.action == 'access_as_external') &
+                (rbac_model.target_tenant == context.tenant_id) |
+                (rbac_model.target_tenant == '*'))
+            conditions = expr.or_(tenant_allowed, *conditions)
         return conditions
 
     def _network_result_filter_hook(self, query, filters):
@@ -124,13 +133,16 @@ class External_net_db_mixin(object):
                 # raise the underlying exception
                 raise e.errors[0].error
             context.session.add(ExternalNetwork(network_id=net_data['id']))
+            context.session.add(rbac_db.NetworkRBAC(
+                  object_id=net_data['id'], action='access_as_external',
+                  target_tenant='*', tenant_id=net_data['tenant_id']))
             registry.notify(
                 resources.EXTERNAL_NETWORK, events.AFTER_CREATE,
                 self, context=context,
                 request=req_data, network=net_data)
         net_data[external_net.EXTERNAL] = external
 
-    def _process_l3_update(self, context, net_data, req_data):
+    def _process_l3_update(self, context, net_data, req_data, allow_all=True):
         try:
             registry.notify(
                 resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE,
@@ -151,6 +163,10 @@ class External_net_db_mixin(object):
         if new_value:
             context.session.add(ExternalNetwork(network_id=net_id))
             net_data[external_net.EXTERNAL] = True
+            if allow_all:
+                context.session.add(rbac_db.NetworkRBAC(
+                      object_id=net_id, action='access_as_external',
+                      target_tenant='*', tenant_id=net_data['tenant_id']))
         else:
             # must make sure we do not have any external gateway ports
             # (and thus, possible floating IPs) on this network before
@@ -163,6 +179,8 @@ class External_net_db_mixin(object):
 
             context.session.query(ExternalNetwork).filter_by(
                 network_id=net_id).delete()
+            context.session.query(rbac_db.NetworkRBAC).filter_by(
+                object_id=net_id, action='access_as_external').delete()
             net_data[external_net.EXTERNAL] = False
 
     def _process_l3_delete(self, context, network_id):
@@ -177,3 +195,82 @@ class External_net_db_mixin(object):
             raise n_exc.TooManyExternalNetworks()
         else:
             return nets[0]['id'] if nets else None
+
+    def _process_ext_policy_create(self, resource, event, trigger, context,
+                                   object_type, policy, **kwargs):
+        if (object_type != 'network' or
+                policy['action'] != 'access_as_external'):
+            return
+        net = self.get_network(context, policy['object_id'])
+        if not context.is_admin and net['tenant_id'] != context.tenant_id:
+            msg = _("Only admins can manipulate policies on networks they "
+                    "do not own.")
+            raise n_exc.InvalidInput(error_message=msg)
+        if not self._network_is_external(context, policy['object_id']):
+            # we automatically convert the network into an external network
+            self._process_l3_update(context, net,
+                                    {external_net.EXTERNAL: True},
+                                    allow_all=False)
+
+    def _validate_ext_not_in_use_by_tenant(self, resource, event, trigger,
+                                           context, object_type, policy,
+                                           **kwargs):
+        if (object_type != 'network' or
+                policy['action'] != 'access_as_external'):
+            return
+        if event == events.BEFORE_UPDATE:
+            new_tenant = kwargs['policy_tenant']['target_tenant']
+            if new_tenant == policy['target_tenant']:
+                # nothing to validate if the tenant didn't change
+                return
+        ports = context.session.query(models_v2.Port.id).filter_by(
+            device_owner=DEVICE_OWNER_ROUTER_GW,
+            network_id=policy['object_id'])
+        router = context.session.query(l3_db.Router).filter(
+            l3_db.Router.gw_port_id.in_(ports))
+        rbac = rbac_db.NetworkRBAC
+        if policy['target_tenant'] != '*':
+            router = router.filter(
+                l3_db.Router.tenant_id == policy['target_tenant'])
+            # if there is a wildcard entry we can safely proceed without the
+            # router lookup because they will have access either way
+            if context.session.query(rbac_db.NetworkRBAC).filter(
+                    rbac.object_id == policy['object_id'],
+                    rbac.action == 'access_as_external',
+                    rbac.target_tenant == '*').count():
+                return
+        else:
+            # deleting the wildcard is okay as long as the tenants with
+            # attached routers have their own entries and the network is
+            # not the default external network.
+            is_default = context.session.query(ExternalNetwork).filter_by(
+                network_id=policy['object_id'], is_default=True).count()
+            if is_default:
+                msg = _("Default external networks must be shared to "
+                        "everyone.")
+                raise rbac_ext.RbacPolicyInUse(object_id=policy['object_id'],
+                                               details=msg)
+            tenants_with_entries = (
+                context.session.query(rbac.target_tenant).
+                filter(rbac.object_id == policy['object_id'],
+                       rbac.action == 'access_as_external',
+                       rbac.target_tenant != '*'))
+            router = router.filter(
+                ~l3_db.Router.tenant_id.in_(tenants_with_entries))
+        if router.count():
+            msg = _("There are routers attached to this network that "
+                    "depend on this policy for access.")
+            raise rbac_ext.RbacPolicyInUse(object_id=policy['object_id'],
+                                           details=msg)
+
+    def _register_external_net_rbac_hooks(self):
+        registry.subscribe(self._process_ext_policy_create,
+                           'rbac-policy', events.BEFORE_CREATE)
+        for e in (events.BEFORE_UPDATE, events.BEFORE_DELETE):
+            registry.subscribe(self._validate_ext_not_in_use_by_tenant,
+                               'rbac-policy', e)
+
+    def __new__(cls, *args, **kwargs):
+        new = super(External_net_db_mixin, cls).__new__(cls, *args, **kwargs)
+        new._register_external_net_rbac_hooks()
+        return new

neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD

@@ -1 +1 @@
-c6c112992c9
+5ffceebfada

neutron/db/migration/alembic_migrations/versions/mitaka/contract/5ffceebfada_rbac_network_external.py

@@ -0,0 +1,73 @@
+# Copyright 2015 OpenStack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+"""network_rbac_external
+
+Revision ID: 5ffceebfada
+Revises: c6c112992c9
+Create Date: 2015-06-14 13:12:04.012457
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '5ffceebfada'
+down_revision = 'c6c112992c9'
+depends_on = ()
+
+from alembic import op
+from oslo_utils import uuidutils
+import sqlalchemy as sa
+
+from neutron.api.v2 import attributes
+
+
+# A simple model of the external network table with only the fields needed for
+# the migration.
+external = sa.Table('externalnetworks', sa.MetaData(),
+                    sa.Column('network_id', sa.String(length=36),
+                              nullable=False))
+TENANT_ID_MAX_LEN = attributes.TENANT_ID_MAX_LEN
+network = sa.Table('networks', sa.MetaData(),
+                   sa.Column('id', sa.String(length=36), nullable=False),
+                   sa.Column('tenant_id', sa.String(length=TENANT_ID_MAX_LEN)))
+
+networkrbacs = sa.Table(
+    'networkrbacs', sa.MetaData(),
+    sa.Column('id', sa.String(length=36), nullable=False),
+    sa.Column('object_id', sa.String(length=36), nullable=False),
+    sa.Column('tenant_id', sa.String(length=TENANT_ID_MAX_LEN), nullable=True,
+              index=True),
+    sa.Column('target_tenant', sa.String(length=TENANT_ID_MAX_LEN),
+              nullable=False),
+    sa.Column('action', sa.String(length=255), nullable=False))
+
+
+def upgrade():
+    op.bulk_insert(networkrbacs, get_values())
+
+
+def get_values():
+    session = sa.orm.Session(bind=op.get_bind())
+    values = []
+    net_to_tenant_id = {}
+    for row in session.query(network).all():
+        net_to_tenant_id[row[0]] = row[1]
+    for row in session.query(external).all():
+        values.append({'id': uuidutils.generate_uuid(), 'object_id': row[0],
+                       'tenant_id': net_to_tenant_id[row[0]],
+                       'target_tenant': '*', 'action': 'access_as_external'})
+    # this commit appears to be necessary to allow further operations
+    session.commit()
+    return values

neutron/db/rbac_db_models.py

@@ -23,9 +23,11 @@ from neutron._i18n import _
 from neutron.api.v2 import attributes as attr
 from neutron.common import exceptions as n_exc
 from neutron.db import model_base
+from neutron import manager
 
 
 ACCESS_SHARED = 'access_as_shared'
+ACCESS_EXTERNAL = 'access_as_external'
 
 
 class InvalidActionForType(n_exc.InvalidInput):
@@ -94,7 +96,11 @@ class NetworkRBAC(RBACColumns, model_base.BASEV2):
     object_type = 'network'
 
     def get_valid_actions(self):
-        return (ACCESS_SHARED,)
+        actions = (ACCESS_SHARED,)
+        pl = manager.NeutronManager.get_plugin()
+        if 'external-net' in pl.supported_extension_aliases:
+            actions += (ACCESS_EXTERNAL,)
+        return actions
 
 
 class QosPolicyRBAC(RBACColumns, model_base.BASEV2):

neutron/extensions/rbac.py

@@ -70,7 +70,10 @@ RESOURCE_ATTRIBUTE_MAP = {
                    # action depends on type so validation has to occur in
                    # the extension
                    'validate': {'type:string': attr.DESCRIPTION_MAX_LEN},
-                   'is_visible': True},
+                   # we set enforce_policy so operators can define policies
+                   # that restrict actions
+                   'is_visible': True, 'enforce_policy': True,
+                   'default': None},
     }
 }
 

neutron/tests/api/admin/test_external_network_extension.py

@@ -10,8 +10,11 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_config import cfg
 from tempest.lib.common.utils import data_utils
+from tempest.lib import exceptions as lib_exc
 from tempest import test
+import testtools
 
 from neutron.tests.api import base
 
@@ -125,3 +128,169 @@ class ExternalNetworksTestJSON(base.BaseAdminNetworkTest):
                          (s['id'] for s in subnet_list))
         # Removes subnet from the cleanup list
         self.subnets.remove(subnet)
+
+
+class ExternalNetworksRBACTestJSON(base.BaseAdminNetworkTest):
+
+    credentials = ['primary', 'alt', 'admin']
+
+    @classmethod
+    def resource_setup(cls):
+        if not test.is_extension_enabled('rbac_policies', 'network'):
+            msg = "rbac extension not enabled."
+            raise cls.skipException(msg)
+        super(ExternalNetworksRBACTestJSON, cls).resource_setup()
+        cls.client2 = cls.alt_manager.network_client
+
+    def _create_network(self, external=True):
+        post_body = {'name': data_utils.rand_name('network-')}
+        if external:
+            post_body['router:external'] = external
+        body = self.admin_client.create_network(**post_body)
+        network = body['network']
+        self.addCleanup(self.admin_client.delete_network, network['id'])
+        return network
+
+    @test.attr(type='smoke')
+    @test.idempotent_id('afd8f1b7-a81e-4629-bca8-a367b3a144bb')
+    def test_regular_client_shares_with_another(self):
+        net = self.create_network()
+        self.client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant=self.client2.tenant_id)
+        body = self.client2.list_networks()
+        networks_list = [n['id'] for n in body['networks']]
+        self.assertIn(net['id'], networks_list)
+        r = self.client2.create_router(
+            data_utils.rand_name('router-'),
+            external_gateway_info={'network_id': net['id']})['router']
+        self.addCleanup(self.admin_client.delete_router, r['id'])
+
+    @test.idempotent_id('afd8f1b7-a81e-4629-bca8-a367b3a144bb')
+    def test_regular_client_blocked_from_creating_external_wild_policies(self):
+        net = self.create_network()
+        with testtools.ExpectedException(lib_exc.Forbidden):
+            self.client.create_rbac_policy(
+                object_type='network', object_id=net['id'],
+                action='access_as_external',
+                target_tenant='*')
+
+    @test.attr(type='smoke')
+    @test.idempotent_id('a2e19f06-48a9-4e4c-b717-08cb2008707d')
+    def test_wildcard_policy_created_from_external_network_api(self):
+        # create external makes wildcard
+        net_id = self._create_network(external=True)['id']
+        self.assertEqual(1, len(self.admin_client.list_rbac_policies(
+            object_id=net_id, action='access_as_external',
+            target_tenant='*')['rbac_policies']))
+        # update to non-external clears wildcard
+        self.admin_client.update_network(net_id, **{'router:external': False})
+        self.assertEqual(0, len(self.admin_client.list_rbac_policies(
+            object_id=net_id, action='access_as_external',
+            target_tenant='*')['rbac_policies']))
+        # create non-external has no wildcard
+        net_id = self._create_network(external=False)['id']
+        self.assertEqual(0, len(self.admin_client.list_rbac_policies(
+            object_id=net_id, action='access_as_external',
+            target_tenant='*')['rbac_policies']))
+        # update to external makes wildcard
+        self.admin_client.update_network(net_id, **{'router:external': True})
+        self.assertEqual(1, len(self.admin_client.list_rbac_policies(
+            object_id=net_id, action='access_as_external',
+            target_tenant='*')['rbac_policies']))
+
+    @test.idempotent_id('a5539002-5bdb-48b5-b124-e9eedd5975e6')
+    def test_external_conversion_on_policy_create(self):
+        net_id = self._create_network(external=False)['id']
+        self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net_id,
+            action='access_as_external',
+            target_tenant=self.client2.tenant_id)
+        body = self.admin_client.show_network(net_id)['network']
+        self.assertTrue(body['router:external'])
+
+    @test.idempotent_id('01364c50-bfb6-46c4-b44c-edc4564d61cf')
+    def test_policy_allows_tenant_to_allocate_floatingip(self):
+        net = self._create_network(external=False)
+        # share to the admin client so it gets converted to external but
+        # not shared to everyone
+        self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant=self.admin_client.tenant_id)
+        self.create_subnet(net, client=self.admin_client, enable_dhcp=False)
+        with testtools.ExpectedException(lib_exc.NotFound):
+            self.client2.create_floatingip(
+                floating_network_id=net['id'])
+        self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant=self.client2.tenant_id)
+        self.client2.create_floatingip(
+            floating_network_id=net['id'])
+
+    @test.idempotent_id('476be1e0-f72e-47dc-9a14-4435926bbe82')
+    def test_policy_allows_tenant_to_attach_ext_gw(self):
+        net = self._create_network(external=False)
+        self.create_subnet(net, client=self.admin_client, enable_dhcp=False)
+        self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant=self.client2.tenant_id)
+        r = self.client2.create_router(
+            data_utils.rand_name('router-'),
+            external_gateway_info={'network_id': net['id']})['router']
+        self.addCleanup(self.admin_client.delete_router, r['id'])
+
+    @test.idempotent_id('d54decee-4203-4ced-91a2-ea42ca63e154')
+    def test_delete_policies_while_tenant_attached_to_net(self):
+        net = self._create_network(external=False)
+        self.create_subnet(net, client=self.admin_client, enable_dhcp=False)
+        wildcard = self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant='*')['rbac_policy']
+        r = self.client2.create_router(
+            data_utils.rand_name('router-'),
+            external_gateway_info={'network_id': net['id']})['router']
+        # delete should fail because the wildcard is required for the tenant's
+        # access
+        with testtools.ExpectedException(lib_exc.Conflict):
+            self.admin_client.delete_rbac_policy(wildcard['id'])
+        tenant = self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant=self.client2.tenant_id)['rbac_policy']
+        # now we can delete the policy because the tenant has its own policy
+        # to allow it access
+        self.admin_client.delete_rbac_policy(wildcard['id'])
+        # but now we can't delete the tenant's policy without the wildcard
+        with testtools.ExpectedException(lib_exc.Conflict):
+            self.admin_client.delete_rbac_policy(tenant['id'])
+        wildcard = self.admin_client.create_rbac_policy(
+            object_type='network', object_id=net['id'],
+            action='access_as_external',
+            target_tenant='*')['rbac_policy']
+        # with the wildcard added back we can delete the tenant's policy
+        self.admin_client.delete_rbac_policy(tenant['id'])
+        self.admin_client.delete_router(r['id'])
+        # now without the tenant attached, the wildcard can be deleted
+        self.admin_client.delete_rbac_policy(wildcard['id'])
+        # finally we ensure that the tenant can't attach to the network since
+        # there are no policies allowing it
+        with testtools.ExpectedException(lib_exc.NotFound):
+            self.client2.create_router(
+                data_utils.rand_name('router-'),
+                external_gateway_info={'network_id': net['id']})
+
+    @test.idempotent_id('7041cec7-d8fe-4c78-9b04-b51b2fd49dc9')
+    def test_wildcard_policy_delete_blocked_on_default_ext(self):
+        public_net_id = cfg.CONF.network.public_network_id
+        # ensure it is default before so we don't wipe out the policy
+        self.admin_client.update_network(public_net_id, is_default=True)
+        policy = self.admin_client.list_rbac_policies(
+            object_id=public_net_id, action='access_as_external',
+            target_tenant='*')['rbac_policies'][0]
+        with testtools.ExpectedException(lib_exc.Conflict):
+            self.admin_client.delete_rbac_policy(policy['id'])

neutron/tests/unit/extensions/test_external_net.py

@@ -141,12 +141,17 @@ class ExtNetDBTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
         plugin = manager.NeutronManager.get_plugin()
         ctx = context.Context('edinson', 'cavani')
         model = models_v2.Network
-        txt = "externalnetworks.network_id IS NOT NULL"
+        txt = ("networkrbacs.action = :action_1 AND "
+               "networkrbacs.target_tenant = :target_tenant_1 OR "
+               "networkrbacs.target_tenant = :target_tenant_2")
         conditions = plugin._network_filter_hook(ctx, model, [])
         self.assertEqual(conditions.__str__(), txt)
         # Try to concatenate conditions
+        txt2 = (txt.replace('tenant_1', 'tenant_3').
+                replace('tenant_2', 'tenant_4').
+                replace('action_1', 'action_2'))
         conditions = plugin._network_filter_hook(ctx, model, conditions)
-        self.assertEqual(conditions.__str__(), "%s OR %s" % (txt, txt))
+        self.assertEqual(conditions.__str__(), "%s OR %s" % (txt, txt2))
 
     def test_create_port_external_network_non_admin_fails(self):
         with self.network(router__external=True) as ext_net:

releasenotes/notes/access_as_external_rbac-455dc74b9fa22761.yaml

@@ -0,0 +1,19 @@
+---
+prelude: >
+    External networks can now be controlled using the
+    RBAC framework that was added in Liberty. This allows
+    networks to be made available to specific tenants
+    (as opposed to all tenants) to be used as an external
+    gateway for routers and floating IPs.
+features:
+  - External networks can now be controlled using the
+    RBAC framework that was added in Liberty. This allows
+    networks to be made available to specific tenants
+    (as opposed to all tenants) to be used as an external
+    gateway for routers and floating IPs.
+    By default this feature will also allow regular tenants
+    to make their networks available as external networks
+    to other individual tenants (or even themselves), but
+    they are prevented from using the wildcard to share to
+    all tenants. This behavior can be adjusted via
+    policy.json by the operator if desired.