From 3fd27427ae7e128c3e542302e7190ef36224735f Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Wed, 21 Mar 2018 11:47:34 +0200 Subject: [PATCH] NSX-V3: Enhance VPNaaS related validations a No-SNAT router subnets cannot overlap with VPN subnets becasue of the rotuer advertisment. This patch adds validations when changing the rotuer GW or addign an interface. Also change the local endpoint port creation so this port will have a distingished name and device id/owner and won't be queried by mistake. Change-Id: I41faf97bae67ca85b38da3ade47894865eac8d51 --- vmware_nsx/plugins/nsx_v3/plugin.py | 2 +- .../services/vpnaas/nsxv3/ipsec_driver.py | 82 +++++++++++++++++-- .../services/vpnaas/nsxv3/ipsec_validator.py | 7 +- 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py index 3dcb29be1d..b2a4546c21 100644 --- a/vmware_nsx/plugins/nsx_v3/plugin.py +++ b/vmware_nsx/plugins/nsx_v3/plugin.py @@ -3741,7 +3741,7 @@ class NsxV3Plugin(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, raise n_exc.InvalidInput(error_message=msg) # VPNaaS need to be notified on router GW changes (there is - # currentlyno matching upstream registration for this) + # currently no matching upstream registration for this) vpn_plugin = directory.get_plugin(plugin_const.VPN) if vpn_plugin: vpn_driver = vpn_plugin.drivers[vpn_plugin.default_provider] diff --git a/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py b/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py index ae06076cb1..02b0ef12bb 100644 --- a/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py +++ b/vmware_nsx/services/vpnaas/nsxv3/ipsec_driver.py @@ -38,7 +38,7 @@ from vmware_nsxlib.v3 import vpn_ipsec LOG = logging.getLogger(__name__) IPSEC = 'ipsec' -VPN_PORT_OWNER = constants.DEVICE_OWNER_NEUTRON_PREFIX + 'vpnservice' +VPN_PORT_OWNER = 'vpnservice' class RouterWithSNAT(nexception.BadRequest): @@ -46,6 +46,16 @@ class RouterWithSNAT(nexception.BadRequest): "SNAT") +class RouterWithOverlapNoSnat(nexception.BadRequest): + message = _("Router %(router_id)s has a subnet overlapping with a VPN " + "local subnet, and cannot disable SNAT") + + +class RouterOverlapping(nexception.BadRequest): + message = _("Router %(router_id)s interface is overlapping with a VPN " + "local subnet and cannot be added") + + class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): def __init__(self, service_plugin): @@ -63,6 +73,10 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): self._delete_local_endpoint, resources.ROUTER_GATEWAY, events.AFTER_DELETE) + registry.subscribe( + self._verify_overlap_subnet, resources.ROUTER_INTERFACE, + events.BEFORE_CREATE) + @property def l3_plugin(self): return self._core_plugin @@ -364,7 +378,7 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): def _find_vpn_service_port(self, context, router_id): """Look for the neutron port created for the vpnservice of a router""" - filters = {'device_id': [router_id], + filters = {'device_id': ['router-' + router_id], 'device_owner': [VPN_PORT_OWNER]} ports = self.l3_plugin.get_ports(context, filters=filters) if ports: @@ -383,18 +397,68 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): if port: self.l3_plugin.delete_port(ctx, port['id']) + def _check_subnets_overlap_with_all_conns(self, context, subnets): + # find all vpn services with connections + filters = {'status': [constants.ACTIVE]} + connections = self.vpn_plugin.get_ipsec_site_connections( + context, filters=filters) + for conn in connections: + srv_id = conn.get('vpnservice_id') + srv = self.vpn_plugin._get_vpnservice(context, srv_id) + srv_subnet = self.l3_plugin.get_subnet( + context, srv['subnet_id']) + if netaddr.IPSet(subnets) & netaddr.IPSet([srv_subnet['cidr']]): + return False + + return True + + def _verify_overlap_subnet(self, resource, event, trigger, **kwargs): + """Upon router interface creation validation overlapping with vpn""" + router_db = kwargs.get('router_db') + port = kwargs.get('port') + if not port or not router_db: + LOG.warning("NSX V3 VPNaaS ROUTER_INTERFACE BEFORE_CRAETE " + "callback didn't get all the relevant information") + return + + if router_db.enable_snat: + # checking only no-snat routers + return + + admin_con = n_context.get_admin_context() + subnet_id = port['fixed_ips'][0].get('subnet_id') + if subnet_id: + subnet = self._core_plugin.get_subnet(admin_con, subnet_id) + # find all vpn services with connections + if not self._check_subnets_overlap_with_all_conns( + admin_con, [subnet['cidr']]): + raise RouterOverlapping(router_id=kwargs.get('router_id')) + def validate_router_gw_info(self, context, router_id, gw_info): """Upon router gw update - verify no-snat""" - # ckeck if this router has a vpn service + # check if this router has a vpn service + admin_con = context.elevated() filters = {'router_id': [router_id], 'status': [constants.ACTIVE]} - services = self.vpn_plugin.get_vpnservices( - context.elevated(), filters=filters) + services = self.vpn_plugin.get_vpnservices(admin_con, filters=filters) if services: # do not allow enable-snat if (gw_info and gw_info.get('enable_snat', cfg.CONF.enable_snat_by_default)): raise RouterWithSNAT(router_id=router_id) + else: + # if this is a non-vpn router. if snat was disabled, should check + # there is no overlapping with vpn connections + if (gw_info and + not gw_info.get('enable_snat', + cfg.CONF.enable_snat_by_default)): + # get router subnets + subnets = self._core_plugin._find_router_subnets_cidrs( + context, router_id) + # find all vpn services with connections + if not self._check_subnets_overlap_with_all_conns( + admin_con, subnets): + raise RouterWithOverlapNoSnat(router_id=router_id) def _get_session_rules(self, context, connection, vpnservice): # TODO(asarfaty): support vpn-endpoint-groups too @@ -600,7 +664,7 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): def _create_vpn_service(self, tier0_uuid): try: service = self._nsx_vpn.service.create( - 'Neutron VPN service for router ' + tier0_uuid, + 'Neutron VPN service for T0 router ' + tier0_uuid, tier0_uuid, enabled=True, ike_log_level=ipsec_utils.DEFAULT_LOG_LEVEL, @@ -658,13 +722,15 @@ class NSXv3IPsecVpnDriver(service_drivers.VpnDriver): port = self._find_vpn_service_port(context, router_id) if not port: # create a new port, on the external network of the router + # Note(asarfaty): using a unique device owner and device id to + # make sure tis port will be ignored in certain queries ext_net = vpnservice.router.gw_port['network_id'] port_data = { 'port': { 'network_id': ext_net, - 'name': None, + 'name': 'VPN local address port', 'admin_state_up': True, - 'device_id': vpnservice.router['id'], + 'device_id': 'router-' + vpnservice.router['id'], 'device_owner': VPN_PORT_OWNER, 'fixed_ips': constants.ATTR_NOT_SPECIFIED, 'mac_address': constants.ATTR_NOT_SPECIFIED, diff --git a/vmware_nsx/services/vpnaas/nsxv3/ipsec_validator.py b/vmware_nsx/services/vpnaas/nsxv3/ipsec_validator.py index 49fd274bf9..3654bb711b 100644 --- a/vmware_nsx/services/vpnaas/nsxv3/ipsec_validator.py +++ b/vmware_nsx/services/vpnaas/nsxv3/ipsec_validator.py @@ -14,6 +14,7 @@ # under the License. import netaddr +from oslo_config import cfg from oslo_log import log as logging from neutron_lib import constants @@ -235,7 +236,8 @@ class IPsecV3Validator(vpn_validator.VpnReferenceValidator): if (rtr['id'] != this_router and rtr.get('external_gateway_info') and not rtr['external_gateway_info'].get( - 'enable_snat', True))] + 'enable_snat', + cfg.CONF.enable_snat_by_default))] for rtr in nosnat_routers: if rtr['id'] == this_router: continue @@ -271,9 +273,6 @@ class IPsecV3Validator(vpn_validator.VpnReferenceValidator): 'conn': conn['id']}) raise nsx_exc.NsxVpnValidationError(details=msg) - # TODO(asarfaty): also add this validation when adding an interface - # or no-snat to a router through the nsx-v3 plugin - def validate_ipsec_site_connection(self, context, ipsec_site_conn): """Called upon create/update of a connection"""