From 2033fe37c12071eaf161e54bbc2401a5503c2ef5 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Tue, 27 Jun 2017 12:15:31 +0300 Subject: [PATCH] NSX|v3: Enforce address scopes for no-NAT routers Do not allow aetting a router interface/GW if this is a no-snat router and the address scopes of the GW and interface do not match Change-Id: I8a90d600bbf4abe5481048aa4cf18f898ea12327 --- vmware_nsx/plugins/common/plugin.py | 33 +++++ vmware_nsx/plugins/nsx_v/plugin.py | 31 ----- vmware_nsx/plugins/nsx_v3/plugin.py | 21 ++- vmware_nsx/tests/unit/nsx_v3/test_plugin.py | 136 +++++++++++++++++++- 4 files changed, 187 insertions(+), 34 deletions(-) diff --git a/vmware_nsx/plugins/common/plugin.py b/vmware_nsx/plugins/common/plugin.py index a78391b692..23505ed5ef 100644 --- a/vmware_nsx/plugins/common/plugin.py +++ b/vmware_nsx/plugins/common/plugin.py @@ -20,6 +20,7 @@ from neutron.db import address_scope_db from neutron.db import api as db_api from neutron.db import db_base_plugin_v2 from neutron.db import l3_db +from neutron.extensions import address_scope as ext_address_scope from neutron_lib.api.definitions import network as net_def from neutron_lib.api.definitions import port as port_def from neutron_lib.api.definitions import subnet as subnet_def @@ -28,6 +29,7 @@ from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory from vmware_nsx._i18n import _ +from vmware_nsx.common import exceptions as nsx_exc LOG = logging.getLogger(__name__) @@ -93,3 +95,34 @@ class NsxPluginBase(db_base_plugin_v2.NeutronDbPluginV2, "has SNAT disabled") raise n_exc.InvalidInput(error_message=msg) return router_id + + def _get_network_address_scope(self, context, net_id): + network = self.get_network(context, net_id) + return network.get(ext_address_scope.IPV4_ADDRESS_SCOPE) + + def _get_subnet_address_scope(self, context, subnet_id): + subnet = self.get_subnet(context, subnet_id) + if not subnet['subnetpool_id']: + return + subnetpool = self.get_subnetpool(context, subnet['subnetpool_id']) + return subnetpool.get('address_scope_id', '') + + # TODO(asarfaty): the NSX-V3 needs a very similar code too + def _validate_address_scope_for_router_interface(self, context, router_id, + gw_network_id, subnet_id): + """Validate that the GW address scope is the same as the interface""" + gw_address_scope = self._get_network_address_scope(context, + gw_network_id) + if not gw_address_scope: + return + subnet_address_scope = self._get_subnet_address_scope(context, + subnet_id) + if (not subnet_address_scope or + subnet_address_scope != gw_address_scope): + raise nsx_exc.NsxRouterInterfaceDoesNotMatchAddressScope( + router_id=router_id, address_scope_id=gw_address_scope) + + def _get_router_interfaces(self, context, router_id): + port_filters = {'device_id': [router_id], + 'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]} + return self.get_ports(context, filters=port_filters) diff --git a/vmware_nsx/plugins/nsx_v/plugin.py b/vmware_nsx/plugins/nsx_v/plugin.py index 190c332f1e..ac1fde8fc1 100644 --- a/vmware_nsx/plugins/nsx_v/plugin.py +++ b/vmware_nsx/plugins/nsx_v/plugin.py @@ -65,7 +65,6 @@ from neutron.db import portsecurity_db from neutron.db import quota_db # noqa from neutron.db import securitygroups_db from neutron.db import vlantransparent_db -from neutron.extensions import address_scope as ext_address_scope from neutron.extensions import allowedaddresspairs as addr_pair from neutron.extensions import availability_zone as az_ext from neutron.extensions import external_net as ext_net_extn @@ -3229,11 +3228,6 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, intf_net_ids = list(set([port['network_id'] for port in intf_ports])) return intf_net_ids - def _get_router_interfaces(self, context, router_id): - port_filters = {'device_id': [router_id], - 'device_owner': [l3_db.DEVICE_OWNER_ROUTER_INTF]} - return self.get_ports(context, filters=port_filters) - def _get_address_groups(self, context, router_id, network_id): address_groups = [] ports = self._get_router_interface_ports_by_network( @@ -3383,31 +3377,6 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin, context, subnet_id)['network_id'] return net_id, subnet_id - def _get_network_address_scope(self, context, net_id): - network = self.get_network(context, net_id) - return network.get(ext_address_scope.IPV4_ADDRESS_SCOPE) - - def _get_subnet_address_scope(self, context, subnet_id): - subnet = self.get_subnet(context, subnet_id) - if not subnet['subnetpool_id']: - return - subnetpool = self.get_subnetpool(context, subnet['subnetpool_id']) - return subnetpool.get('address_scope_id', '') - - # TODO(asarfaty): the NSX-V3 needs a very similar code too - def _validate_address_scope_for_router_interface(self, context, router_id, - gw_network_id, subnet_id): - gw_address_scope = self._get_network_address_scope(context, - gw_network_id) - if not gw_address_scope: - return - subnet_address_scope = self._get_subnet_address_scope(context, - subnet_id) - if (not subnet_address_scope or - subnet_address_scope != gw_address_scope): - raise nsx_exc.NsxRouterInterfaceDoesNotMatchAddressScope( - router_id=router_id, address_scope_id=gw_address_scope) - def add_router_interface(self, context, router_id, interface_info): router = self.get_router(context, router_id) net_id, subnet_id = self._get_interface_info(context, interface_info) diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py index 72409f3b2a..b020983f30 100644 --- a/vmware_nsx/plugins/nsx_v3/plugin.py +++ b/vmware_nsx/plugins/nsx_v3/plugin.py @@ -2897,6 +2897,18 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, # TODO(berlin): admin_state_up support gw_info = self._extract_external_gw(context, router, is_extract=False) router_data = router['router'] + + # if setting this router as no-snat, make sure gw address scope match + # those of the subnets + if (validators.is_attr_set(gw_info) and + not gw_info.get('enable_snat', cfg.CONF.enable_snat_by_default)): + router_ports = self._get_router_interfaces(context, router_id) + for port in router_ports: + for fip in port['fixed_ips']: + self._validate_address_scope_for_router_interface( + context, router_id, + gw_info['network_id'], fip['subnet_id']) + nsx_router_id = None routes_added = [] routes_removed = [] @@ -3057,6 +3069,14 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, network_id = subnet['network_id'] nsx_net_id, nsx_port_id = nsx_db.get_nsx_switch_and_port_id( context.session, port['id']) + router_db = self._get_router(context, router_id) + + # If it is a no-snat router, interface address scope must be the + # same as the gateways + if not router_db.enable_snat: + gw_network_id = router_db.gw_port.network_id + self._validate_address_scope_for_router_interface( + context, router_id, gw_network_id, subnet['id']) nsx_router_id = nsx_db.get_nsx_router_id(context.session, router_id) @@ -3076,7 +3096,6 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, logical_switch_port_id=nsx_port_id, address_groups=address_groups) - router_db = self._get_router(context, router_id) if router_db.gw_port and not router_db.enable_snat: # TODO(berlin): Announce the subnet on tier0 if enable_snat # is False diff --git a/vmware_nsx/tests/unit/nsx_v3/test_plugin.py b/vmware_nsx/tests/unit/nsx_v3/test_plugin.py index 4f005e4525..a2637b29cf 100644 --- a/vmware_nsx/tests/unit/nsx_v3/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_v3/test_plugin.py @@ -14,11 +14,13 @@ # limitations under the License. import mock +import netaddr import six from webob import exc from neutron.api.v2 import attributes from neutron.db import models_v2 +from neutron.extensions import address_scope from neutron.extensions import external_net from neutron.extensions import extraroute from neutron.extensions import l3 @@ -26,6 +28,7 @@ from neutron.extensions import l3_ext_gw_mode from neutron.extensions import securitygroup as secgrp from neutron.tests.unit import _test_extension_portbindings as test_bindings from neutron.tests.unit.db import test_db_base_plugin_v2 as test_plugin +from neutron.tests.unit.extensions import test_address_scope from neutron.tests.unit.extensions import test_extra_dhcp_opt as test_dhcpopts from neutron.tests.unit.extensions import test_extraroute as test_ext_route from neutron.tests.unit.extensions import test_l3 as test_l3_plugin @@ -535,7 +538,10 @@ class TestL3ExtensionManager(object): # Finally add l3 resources to the global attribute map attributes.RESOURCE_ATTRIBUTE_MAP.update( l3.RESOURCE_ATTRIBUTE_MAP) - return l3.L3.get_resources() + attributes.RESOURCE_ATTRIBUTE_MAP.update( + address_scope.RESOURCE_ATTRIBUTE_MAP) + return (l3.L3.get_resources() + + address_scope.Address_scope.get_resources()) def get_actions(self): return [] @@ -555,7 +561,8 @@ def restore_l3_attribute_map(map_to_restore): l3.RESOURCE_ATTRIBUTE_MAP = map_to_restore -class L3NatTest(test_l3_plugin.L3BaseForIntTests, NsxV3PluginTestCaseMixin): +class L3NatTest(test_l3_plugin.L3BaseForIntTests, NsxV3PluginTestCaseMixin, + test_address_scope.AddressScopeTestCase): def _restore_l3_attribute_map(self): l3.RESOURCE_ATTRIBUTE_MAP = self._l3_attribute_map_bk @@ -759,6 +766,131 @@ class TestL3NatTestCase(L3NatTest, def test_floatingip_update_to_same_port_id_twice(self): self.skipTest('Plugin changes floating port status') + def _test_create_subnetpool(self, prefixes, expected=None, + admin=False, **kwargs): + keys = kwargs.copy() + keys.setdefault('tenant_id', self._tenant_id) + with self.subnetpool(prefixes, admin, **keys) as subnetpool: + self._validate_resource(subnetpool, keys, 'subnetpool') + if expected: + self._compare_resource(subnetpool, expected, 'subnetpool') + return subnetpool + + def _update_router_enable_snat(self, router_id, network_id, enable_snat): + return self._update('routers', router_id, + {'router': {'external_gateway_info': + {'network_id': network_id, + 'enable_snat': enable_snat}}}) + + def test_router_no_snat_with_different_address_scope(self): + """Test that if the router has no snat, you cannot add an interface + from a different address scope than the gateway. + """ + # create an external network on one address scope + with self.address_scope(name='as1') as addr_scope, \ + self.network() as ext_net: + self._set_net_external(ext_net['network']['id']) + as_id = addr_scope['address_scope']['id'] + subnet = netaddr.IPNetwork('10.10.10.0/24') + subnetpool = self._test_create_subnetpool( + [subnet.cidr], name='sp1', + min_prefixlen='24', address_scope_id=as_id) + subnetpool_id = subnetpool['subnetpool']['id'] + data = {'subnet': { + 'network_id': ext_net['network']['id'], + 'subnetpool_id': subnetpool_id, + 'ip_version': 4, + 'enable_dhcp': False, + 'tenant_id': ext_net['network']['tenant_id']}} + req = self.new_create_request('subnets', data) + ext_subnet = self.deserialize(self.fmt, req.get_response(self.api)) + + # create a regular network on another address scope + with self.address_scope(name='as2') as addr_scope2, \ + self.network() as net: + as_id2 = addr_scope2['address_scope']['id'] + subnet2 = netaddr.IPNetwork('20.10.10.0/24') + subnetpool2 = self._test_create_subnetpool( + [subnet2.cidr], name='sp2', + min_prefixlen='24', address_scope_id=as_id2) + subnetpool_id2 = subnetpool2['subnetpool']['id'] + data = {'subnet': { + 'network_id': net['network']['id'], + 'subnetpool_id': subnetpool_id2, + 'ip_version': 4, + 'tenant_id': net['network']['tenant_id']}} + req = self.new_create_request('subnets', data) + int_subnet = self.deserialize( + self.fmt, req.get_response(self.api)) + + # create a no snat router with this gateway + with self.router() as r: + self._add_external_gateway_to_router( + r['router']['id'], + ext_subnet['subnet']['network_id']) + self._update_router_enable_snat( + r['router']['id'], + ext_subnet['subnet']['network_id'], + False) + + # should fail adding the interface to the router + err_code = exc.HTTPBadRequest.code + self._router_interface_action('add', + r['router']['id'], + int_subnet['subnet']['id'], + None, + err_code) + + def test_router_no_snat_with_same_address_scope(self): + """Test that if the router has no snat, you can add an interface + from the same address scope as the gateway. + """ + # create an external network on one address scope + with self.address_scope(name='as1') as addr_scope, \ + self.network() as ext_net: + self._set_net_external(ext_net['network']['id']) + as_id = addr_scope['address_scope']['id'] + subnet = netaddr.IPNetwork('10.10.10.0/21') + subnetpool = self._test_create_subnetpool( + [subnet.cidr], name='sp1', + min_prefixlen='24', address_scope_id=as_id) + subnetpool_id = subnetpool['subnetpool']['id'] + data = {'subnet': { + 'network_id': ext_net['network']['id'], + 'subnetpool_id': subnetpool_id, + 'ip_version': 4, + 'enable_dhcp': False, + 'tenant_id': ext_net['network']['tenant_id']}} + req = self.new_create_request('subnets', data) + ext_subnet = self.deserialize(self.fmt, req.get_response(self.api)) + + # create a regular network on the same address scope + with self.network() as net: + data = {'subnet': { + 'network_id': net['network']['id'], + 'subnetpool_id': subnetpool_id, + 'ip_version': 4, + 'tenant_id': net['network']['tenant_id']}} + req = self.new_create_request('subnets', data) + int_subnet = self.deserialize( + self.fmt, req.get_response(self.api)) + + # create a no snat router with this gateway + with self.router() as r: + self._add_external_gateway_to_router( + r['router']['id'], + ext_subnet['subnet']['network_id']) + self._update_router_enable_snat( + r['router']['id'], + ext_subnet['subnet']['network_id'], + False) + + # should succeed adding the interface to the router + self._router_interface_action('add', + r['router']['id'], + int_subnet['subnet']['id'], + None) + class ExtGwModeTestCase(test_ext_gw_mode.ExtGwModeIntTestCase, L3NatTest):