diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 6cf0ed1f64c..147c5bc4722 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/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. diff --git a/neutron/db/external_net_db.py b/neutron/db/external_net_db.py index d23f60ea94e..594040e5999 100644 --- a/neutron/db/external_net_db.py +++ b/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 diff --git a/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD b/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD index 46d3ecf98bd..d1b0015881f 100644 --- a/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD +++ b/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD @@ -1 +1 @@ -c6c112992c9 +5ffceebfada diff --git a/neutron/db/migration/alembic_migrations/versions/mitaka/contract/5ffceebfada_rbac_network_external.py b/neutron/db/migration/alembic_migrations/versions/mitaka/contract/5ffceebfada_rbac_network_external.py new file mode 100644 index 00000000000..b2b35225cf4 --- /dev/null +++ b/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 diff --git a/neutron/db/rbac_db_models.py b/neutron/db/rbac_db_models.py index ef0c8adf9da..c7f176db299 100644 --- a/neutron/db/rbac_db_models.py +++ b/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): diff --git a/neutron/extensions/rbac.py b/neutron/extensions/rbac.py index fb3b86a0fff..c74d794af7d 100644 --- a/neutron/extensions/rbac.py +++ b/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}, } } diff --git a/neutron/tests/api/admin/test_external_network_extension.py b/neutron/tests/api/admin/test_external_network_extension.py index 8dc236bf298..87fa50982d9 100644 --- a/neutron/tests/api/admin/test_external_network_extension.py +++ b/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']) diff --git a/neutron/tests/unit/extensions/test_external_net.py b/neutron/tests/unit/extensions/test_external_net.py index ef862d248ba..0e706836060 100644 --- a/neutron/tests/unit/extensions/test_external_net.py +++ b/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: diff --git a/releasenotes/notes/access_as_external_rbac-455dc74b9fa22761.yaml b/releasenotes/notes/access_as_external_rbac-455dc74b9fa22761.yaml new file mode 100644 index 00000000000..0fdfe804bbf --- /dev/null +++ b/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.