From ebaa9687df97f816af3434812bd9757cb2b72997 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Thu, 12 Apr 2018 11:00:23 +0300 Subject: [PATCH] NSX-V3: fail FWaaS rules with 0.0.0.0/x cidrs The NSX backend does not support 0.0.0.0/cidrs in the edge firewall rules. This patch will issue a driver error in case a similar cidr is in the rule for the FWaaS V1 & V2 drivers Change-Id: Iebc3642a58bd2e737a6ac2b87dfd99206603a37d --- .../fwaas/nsx_v3/edge_fwaas_driver_base.py | 15 ++++++--- .../tests/unit/nsx_v3/test_fwaas_v1_driver.py | 33 +++++++++++++++---- .../tests/unit/nsx_v3/test_fwaas_v2_driver.py | 25 +++++++++++--- 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/vmware_nsx/services/fwaas/nsx_v3/edge_fwaas_driver_base.py b/vmware_nsx/services/fwaas/nsx_v3/edge_fwaas_driver_base.py index 616390b516..eb5eee63a8 100644 --- a/vmware_nsx/services/fwaas/nsx_v3/edge_fwaas_driver_base.py +++ b/vmware_nsx/services/fwaas/nsx_v3/edge_fwaas_driver_base.py @@ -113,13 +113,18 @@ class CommonEdgeFwaasV3Driver(fwaas_base.FwaasDriverBase): 'action': fwaas_action, 'id': fwaas_rule_id}) raise self.driver_exception(driver=self.driver_name) - def _translate_cidr(self, cidr): + def _translate_cidr(self, cidr, fwaas_rule_id): + if cidr and cidr.startswith('0.0.0.0/'): + LOG.error("Unsupported FWAAS cidr %(cidr)s for rule %(id)s", { + 'cidr': cidr, 'id': fwaas_rule_id}) + raise self.driver_exception(driver=self.driver_name) + return self.nsx_firewall.get_ip_cidr_reference( cidr, consts.IPV6 if netaddr.valid_ipv6(cidr) else consts.IPV4) - def translate_addresses_to_target(self, cidrs): - return [self._translate_cidr(ip) for ip in cidrs] + def translate_addresses_to_target(self, cidrs, fwaas_rule_id=None): + return [self._translate_cidr(ip, fwaas_rule_id) for ip in cidrs] @staticmethod def _translate_protocol(fwaas_protocol): @@ -191,7 +196,7 @@ class CommonEdgeFwaasV3Driver(fwaas_base.FwaasDriverBase): nsx_rule['direction'] = 'IN' elif rule.get('destination_ip_address'): nsx_rule['destinations'] = self.translate_addresses_to_target( - [rule['destination_ip_address']]) + [rule['destination_ip_address']], rule['id']) if replace_src: # set this value as the source logical switch, # and set the rule to egress @@ -200,7 +205,7 @@ class CommonEdgeFwaasV3Driver(fwaas_base.FwaasDriverBase): nsx_rule['direction'] = 'OUT' elif rule.get('source_ip_address'): nsx_rule['sources'] = self.translate_addresses_to_target( - [rule['source_ip_address']]) + [rule['source_ip_address']], rule['id']) if rule.get('protocol'): nsx_rule['services'] = self._translate_services(rule) if logged: diff --git a/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v1_driver.py b/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v1_driver.py index caa8bf50c4..95dc3fd3a4 100644 --- a/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v1_driver.py +++ b/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v1_driver.py @@ -17,6 +17,7 @@ import copy import mock +from neutron_lib.exceptions import firewall_v1 as exceptions from neutron_lib.plugins import directory from vmware_nsx.services.fwaas.nsx_v3 import edge_fwaas_driver_base @@ -76,13 +77,13 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): rule['action'] = consts.FW_ACTION_ALLOW return rule - def _fake_rules_v4(self): + def _fake_rules_v4(self, cidr='10.24.4.0/24'): rule1 = {'enabled': True, 'action': 'allow', 'ip_version': 4, 'protocol': 'tcp', 'destination_port': '80', - 'source_ip_address': '10.24.4.2', + 'source_ip_address': cidr, 'id': 'fake-fw-rule1', 'description': 'first rule'} rule2 = {'enabled': True, @@ -100,11 +101,11 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): rule4 = {'enabled': True, 'action': 'deny', 'ip_version': 4, - 'source_ip_address': '10.25.5.2', + 'source_ip_address': cidr, 'id': 'fake-fw-rule4'} return [rule1, rule2, rule3, rule4] - def _fake_translated_rules(self, logged=False): + def _fake_translated_rules(self, logged=False, cidr='10.24.4.0/24'): # The expected translation of the rules in _fake_rules_v4 service1 = {'l4_protocol': 'TCP', 'resource_type': 'L4PortSetNSService', @@ -112,7 +113,7 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): 'source_ports': []} rule1 = {'action': 'ALLOW', 'services': [{'service': service1}], - 'sources': [{'target_id': '10.24.4.2', + 'sources': [{'target_id': cidr, 'target_type': 'IPv4Address'}], 'display_name': 'Fwaas-fake-fw-rule1', 'notes': 'first rule'} @@ -133,7 +134,7 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): {'service': service3_2}], 'display_name': 'Fwaas-fake-fw-rule3'} rule4 = {'action': 'DROP', - 'sources': [{'target_id': '10.25.5.2', + 'sources': [{'target_id': cidr, 'target_type': 'IPv4Address'}], 'display_name': 'Fwaas-fake-fw-rule4'} @@ -243,6 +244,26 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): def test_update_firewall_with_rules(self): self._setup_firewall_with_rules(self.firewall.update_firewall) + def test_create_firewall_with_illegal_cidr(self): + apply_list = self._fake_apply_list() + rule_list = self._fake_rules_v4(cidr='0.0.0.0/24') + firewall = self._fake_firewall(rule_list) + with mock.patch.object(self.plugin, '_get_router_interfaces', + return_value=[]), \ + mock.patch.object(self.plugin, 'get_ports', + return_value=[]), \ + mock.patch.object(self.plugin, 'get_router', + return_value=apply_list[0]), \ + mock.patch.object(self.plugin.fwaas_callbacks, + '_get_router_firewall_id', + return_value=firewall['id']), \ + mock.patch.object(self.plugin.fwaas_callbacks, + '_get_fw_from_plugin', + return_value=firewall): + self.assertRaises(exceptions.FirewallInternalDriverError, + self.firewall.create_firewall, 'nsx', + apply_list, firewall) + def test_delete_firewall(self): apply_list = self._fake_apply_list() firewall = self._fake_firewall_no_rule() diff --git a/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v2_driver.py b/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v2_driver.py index 37fc3e7207..0b64b82878 100644 --- a/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v2_driver.py +++ b/vmware_nsx/tests/unit/nsx_v3/test_fwaas_v2_driver.py @@ -73,7 +73,7 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): rule['action'] = consts.FW_ACTION_ALLOW return rule - def _fake_rules_v4(self, is_ingress=True): + def _fake_rules_v4(self, is_ingress=True, cidr='10.24.4.0/24'): rule1 = {'enabled': True, 'action': 'allow', 'ip_version': 4, @@ -99,10 +99,10 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): 'id': 'fake-fw-rule4'} if is_ingress: # source ips are allowed - rule1['source_ip_address'] = '10.24.4.2' + rule1['source_ip_address'] = cidr else: # dest ips are allowed for egress rules - rule1['destination_ip_address'] = '10.24.4.2' + rule1['destination_ip_address'] = cidr return [rule1, rule2, rule3, rule4] @@ -115,7 +115,7 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): 'source_ports': []} rule1 = {'action': 'ALLOW', 'services': [{'service': service1}], - 'sources': [{'target_id': '10.24.4.2', + 'sources': [{'target_id': '10.24.4.0/24', 'target_type': 'IPv4Address'}], 'display_name': 'Fwaas-fake-fw-rule1', 'notes': 'first rule'} @@ -286,6 +286,23 @@ class Nsxv3FwaasTestCase(test_v3_plugin.NsxV3PluginTestCaseMixin): self.firewall.create_firewall_group, 'nsx', apply_list, firewall) + def test_create_firewall_with_illegal_cidr(self): + apply_list = self._fake_apply_list() + rule_list = self._fake_rules_v4(cidr='0.0.0.0/24') + firewall = self._fake_firewall_group(rule_list) + port = {'id': FAKE_PORT_ID, 'network_id': FAKE_NET_ID} + with mock.patch.object(self.plugin, '_get_router_interfaces', + return_value=[port]),\ + mock.patch.object(self.plugin, 'get_port', + return_value=port),\ + mock.patch.object(self.plugin.fwaas_callbacks, 'get_port_fwg', + return_value=firewall),\ + mock.patch("vmware_nsx.db.db.get_nsx_switch_and_port_id", + return_value=(FAKE_NSX_LS_ID, 0)): + self.assertRaises(exceptions.FirewallInternalDriverError, + self.firewall.create_firewall_group, 'nsx', + apply_list, firewall) + def test_delete_firewall(self): apply_list = self._fake_apply_list() firewall = self._fake_empty_firewall_group()