From 25282d67a16b3c136e21afbcea82b0f99f684d32 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Sun, 20 Jan 2019 10:54:16 +0200 Subject: [PATCH] NSX|P+V3: Do not allow external subnets overlapping with uplink cidr Till now the plugin checked that external subnet ips are not overlapping with the tier0 uplink ip, but the real check should be against the uplink entire cidr. Change-Id: I98aeb9e8fa13ad6d29a4addc7737fc2f5ef922e6 --- vmware_nsx/plugins/common_v3/plugin.py | 16 ++++++++++------ vmware_nsx/plugins/nsx_p/plugin.py | 5 +++-- vmware_nsx/plugins/nsx_v3/plugin.py | 6 ++++-- vmware_nsx/tests/unit/nsx_p/test_plugin.py | 4 ++-- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/vmware_nsx/plugins/common_v3/plugin.py b/vmware_nsx/plugins/common_v3/plugin.py index 4812b33515..74c72c119c 100644 --- a/vmware_nsx/plugins/common_v3/plugin.py +++ b/vmware_nsx/plugins/common_v3/plugin.py @@ -715,7 +715,7 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, """Should be implemented by each plugin""" pass - def _get_tier0_uplink_ips(self, tier0_id): + def _get_tier0_uplink_cidrs(self, tier0_id): """Should be implemented by each plugin""" pass @@ -2248,6 +2248,9 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, return False return True if count == 1 else False + def _cidrs_overlap(self, cidr0, cidr1): + return cidr0.first <= cidr1.last and cidr1.first <= cidr0.last + def _validate_address_space(self, context, subnet): # Only working for IPv4 at the moment if (subnet['ip_version'] != 4): @@ -2277,7 +2280,7 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, LOG.error(msg) raise n_exc.InvalidInput(error_message=msg) - # Ensure that the NSX uplink does not lie on the same subnet as + # Ensure that the NSX uplink cidr does not lie on the same subnet as # the external subnet filters = {'id': [subnet['network_id']], 'router:external': [True]} @@ -2287,12 +2290,13 @@ class NsxPluginV3Base(agentschedulers_db.AZDhcpAgentSchedulerDbMixin, if ext_net.get(pnet.PHYSICAL_NETWORK)] for tier0_rtr in set(tier0_routers): - tier0_ips = self._get_tier0_uplink_ips(tier0_rtr) - for ip_address in tier0_ips: + tier0_cidrs = self._get_tier0_uplink_cidrs(tier0_rtr) + for cidr in tier0_cidrs: + tier0_subnet = netaddr.IPNetwork(cidr).cidr for subnet_network in subnet_networks: - if (netaddr.IPAddress(ip_address) in subnet_network): + if self._cidrs_overlap(tier0_subnet, subnet_network): msg = _("External subnet cannot overlap with T0 " - "router address %s") % ip_address + "router cidr %s") % cidr LOG.error(msg) raise n_exc.InvalidInput(error_message=msg) diff --git a/vmware_nsx/plugins/nsx_p/plugin.py b/vmware_nsx/plugins/nsx_p/plugin.py index 69c58e2243..1e7fb35b93 100644 --- a/vmware_nsx/plugins/nsx_p/plugin.py +++ b/vmware_nsx/plugins/nsx_p/plugin.py @@ -2042,8 +2042,9 @@ class NsxPolicyPlugin(nsx_plugin_common.NsxPluginV3Base): def _has_native_dhcp_metadata(self): return True - def _get_tier0_uplink_ips(self, tier0_id): - return self.nsxpolicy.tier0.get_uplink_ips(tier0_id) + def _get_tier0_uplink_cidrs(self, tier0_id): + # return a list of tier0 uplink ip/prefix addresses + return self.nsxpolicy.tier0.get_uplink_cidrs(tier0_id) def _is_vlan_router_interface_supported(self): return True diff --git a/vmware_nsx/plugins/nsx_v3/plugin.py b/vmware_nsx/plugins/nsx_v3/plugin.py index 4b2bc09a67..161b67b360 100644 --- a/vmware_nsx/plugins/nsx_v3/plugin.py +++ b/vmware_nsx/plugins/nsx_v3/plugin.py @@ -3407,8 +3407,10 @@ class NsxV3Plugin(nsx_plugin_common.NsxPluginV3Base, source_net=subnet['cidr'], bypass_firewall=False) - def _get_tier0_uplink_ips(self, tier0_id): - return self.nsxlib.logical_router_port.get_tier0_uplink_ips(tier0_id) + def _get_tier0_uplink_cidrs(self, tier0_id): + # return a list of tier0 uplink ip/prefix addresses + return self.nsxlib.logical_router_port.get_tier0_uplink_cidrs( + tier0_id) def _get_neutron_net_ids_by_nsx_id(self, context, lswitch_id): return nsx_db.get_net_ids(context.session, lswitch_id) diff --git a/vmware_nsx/tests/unit/nsx_p/test_plugin.py b/vmware_nsx/tests/unit/nsx_p/test_plugin.py index 921b5a86e6..fc92f17aa5 100644 --- a/vmware_nsx/tests/unit/nsx_p/test_plugin.py +++ b/vmware_nsx/tests/unit/nsx_p/test_plugin.py @@ -1084,8 +1084,8 @@ class NsxPTestSubnets(test_db_base_plugin_v2.TestSubnetsV2, 'host_routes': None, 'ip_version': 4}} with mock.patch.object(self.plugin.nsxpolicy.tier0, - 'get_uplink_ips', - return_value=['172.20.1.60']): + 'get_uplink_cidrs', + return_value=['172.20.1.60/24']): self.assertRaises(n_exc.InvalidInput, self.plugin.create_subnet, context.get_admin_context(), data)