From 3074e4f67c8295767b3dc06cce1e92bcf7ec2ff2 Mon Sep 17 00:00:00 2001 From: Michal Kelner Mishali Date: Thu, 2 Aug 2018 14:10:28 +0300 Subject: [PATCH] NSX|V3: Support new icmp codes and types list Support changes in backend for ICMP types and codes while maintaining backwards compatibility. Change-Id: I7478904b5549345d7e2227ee89836e0b9dbe9d11 Signed-off-by: Michal Kelner Mishali --- vmware_nsxlib/tests/unit/v3/test_security.py | 104 ++++++++++++------- vmware_nsxlib/v3/__init__.py | 6 +- vmware_nsxlib/v3/constants.py | 16 +++ vmware_nsxlib/v3/nsx_constants.py | 3 +- vmware_nsxlib/v3/security.py | 37 +++++-- 5 files changed, 116 insertions(+), 50 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/test_security.py b/vmware_nsxlib/tests/unit/v3/test_security.py index e6184a2c..48e9fb7d 100644 --- a/vmware_nsxlib/tests/unit/v3/test_security.py +++ b/vmware_nsxlib/tests/unit/v3/test_security.py @@ -172,38 +172,42 @@ class TestNsxLibFirewallSection(nsxlib_testcase.NsxLibTestCase): "ALLOW", rules, {rule_id: 'dummy'}) def test_create_rule_with_icmp(self): - with mock.patch("vmware_nsxlib.v3.security.NsxLibFirewallSection" - ".add_rules") as add_rules: - rule_id = uuidutils.generate_uuid() - rule = {'id': rule_id, - 'ethertype': 'IPv4', - 'protocol': 'icmp', - 'direction': 'egress', - 'port_range_min': 33, - 'port_range_max': 0, - 'remote_ip_prefix': None} - rules = [rule] - section_id = 'section-id' - group_id = 'nsgroup-id' - target_id = 'dummy' - self.nsxlib.firewall_section.create_rules( - None, section_id, group_id, False, - "ALLOW", rules, {rule_id: target_id}) - add_rules.assert_called_once_with([ - {'display_name': mock.ANY, - 'ip_protocol': 'IPV4', - 'direction': 'OUT', - 'services': [{'service': { - 'protocol': 'ICMPv4', - 'icmp_type': 33, - 'icmp_code': 0, - 'resource_type': 'ICMPTypeNSService'}}], - 'disabled': False, - 'destinations': [{'target_id': target_id, - 'target_type': 'NSGroup'}], - 'sources': [{'target_id': group_id, - 'target_type': 'NSGroup'}], - 'logged': False, 'action': 'ALLOW'}], section_id) + nsx_ver = ["2.3.0", "2.4.0"] + for nsx_ver in nsx_ver: + with mock.patch("vmware_nsxlib.v3.security.NsxLibFirewallSection" + ".add_rules") as add_rules: + with mock.patch("vmware_nsxlib.v3.NsxLib.get_version", + return_value=nsx_ver): + rule_id = uuidutils.generate_uuid() + rule = {'id': rule_id, + 'ethertype': 'IPv4', + 'protocol': 'icmp', + 'direction': 'egress', + 'port_range_min': 33, + 'port_range_max': 0, + 'remote_ip_prefix': None} + rules = [rule] + section_id = 'section-id' + group_id = 'nsgroup-id' + target_id = 'dummy' + self.nsxlib.firewall_section.create_rules( + None, section_id, group_id, False, + "ALLOW", rules, {rule_id: target_id}) + add_rules.assert_called_once_with([ + {'display_name': mock.ANY, + 'ip_protocol': 'IPV4', + 'direction': 'OUT', + 'services': [{'service': { + 'protocol': 'ICMPv4', + 'icmp_type': 33, + 'icmp_code': 0, + 'resource_type': 'ICMPTypeNSService'}}], + 'disabled': False, + 'destinations': [{'target_id': target_id, + 'target_type': 'NSGroup'}], + 'sources': [{'target_id': group_id, + 'target_type': 'NSGroup'}], + 'logged': False, 'action': 'ALLOW'}], section_id) def test_create_rule_with_illegal_icmp(self): rule_id = uuidutils.generate_uuid() @@ -218,10 +222,38 @@ class TestNsxLibFirewallSection(nsxlib_testcase.NsxLibTestCase): section_id = 'section-id' group_id = 'nsgroup-id' target_id = 'dummy' - self.assertRaises(nsxlib_exc.InvalidInput, - self.nsxlib.firewall_section.create_rules, - None, section_id, group_id, False, - "ALLOW", rules, {rule_id: target_id}) + with mock.patch("vmware_nsxlib.v3.NsxLib.get_version", + return_value="2.3.0"): + self.assertRaises(nsxlib_exc.InvalidInput, + self.nsxlib.firewall_section.create_rules, + None, section_id, group_id, False, + "ALLOW", rules, {rule_id: target_id}) + with mock.patch("vmware_nsxlib.v3.NsxLib.get_version", + return_value="2.4.0"): + self.assertRaises(nsxlib_exc.InvalidInput, + self.nsxlib.firewall_section.create_rules, + None, section_id, group_id, False, + "ALLOW", rules, {rule_id: target_id}) + + def test_create_rule_with_illegal_icmp_2_4(self): + rule_id = uuidutils.generate_uuid() + rule = {'id': rule_id, + 'ethertype': 'IPv4', + 'protocol': 'icmp', + 'direction': 'egress', + 'port_range_min': 4, + 'port_range_max': 0, + 'remote_ip_prefix': None} + rules = [rule] + section_id = 'section-id' + group_id = 'nsgroup-id' + target_id = 'dummy' + with mock.patch("vmware_nsxlib.v3.NsxLib.get_version", + return_value="2.4.0"): + self.assertRaises(nsxlib_exc.InvalidInput, + self.nsxlib.firewall_section.create_rules, + None, section_id, group_id, False, + "ALLOW", rules, {rule_id: target_id}) def test_create_with_rules(self): expected_body = { diff --git a/vmware_nsxlib/v3/__init__.py b/vmware_nsxlib/v3/__init__.py index 1b3717e6..b98312d7 100644 --- a/vmware_nsxlib/v3/__init__.py +++ b/vmware_nsxlib/v3/__init__.py @@ -321,10 +321,12 @@ class NsxLib(NsxLibBase): def feature_supported(self, feature): if (version.LooseVersion(self.get_version()) >= - version.LooseVersion(nsx_constants.NSX_VERSION_2_3_0)): - # Features available since 2.3 + version.LooseVersion(nsx_constants.NSX_VERSION_2_4_0)): + # Features available since 2.4 if (feature == nsx_constants.FEATURE_ENS_WITH_SEC): return True + if (feature == nsx_constants.FEATURE_ICMP_STRICT): + return True if (version.LooseVersion(self.get_version()) >= version.LooseVersion(nsx_constants.NSX_VERSION_2_2_0)): diff --git a/vmware_nsxlib/v3/constants.py b/vmware_nsxlib/v3/constants.py index d29224c3..9da043c0 100644 --- a/vmware_nsxlib/v3/constants.py +++ b/vmware_nsxlib/v3/constants.py @@ -111,3 +111,19 @@ IPV4_ICMP_TYPES = {0: [0], # Echo reply 35: [0], # Mobile registration request 36: [0], # Mobile registration reply } +# Supported strict ICMP types and their codes +IPV4_ICMP_STRICT_TYPES = {0: [0], # Echo reply + 8: [0], # Echo request + 9: [0], # Router advertisement + 10: [0], # Router Selection + 13: [0], # Timestamp + 14: [0], # Timestamp reply + 15: [0], # Information request + 16: [0], # Information reply + 17: [0], # Address mask request + 18: [0], # Address mask reply + 33: [0], # Where-Are-You + 34: [0], # I-Am-Here + 35: [0], # Mobile registration request + 36: [0], # Mobile registration reply + } diff --git a/vmware_nsxlib/v3/nsx_constants.py b/vmware_nsxlib/v3/nsx_constants.py index dcf6a8f4..eaa40e2f 100644 --- a/vmware_nsxlib/v3/nsx_constants.py +++ b/vmware_nsxlib/v3/nsx_constants.py @@ -126,7 +126,7 @@ NSX_VERSION_2_0_0 = '2.0.0' NSX_VERSION_2_1_0 = '2.1.0' NSX_VERSION_2_2_0 = '2.2.0' NSX_VERSION_2_3_0 = '2.3.0' -NSX_VERSION_2_3_0 = '2.4.0' +NSX_VERSION_2_4_0 = '2.4.0' NSX_VERSION_3_0_0 = '3.0.0' # Features available depending on the backend version @@ -145,3 +145,4 @@ FEATURE_TRUNK_VLAN = 'Trunk Vlan' FEATURE_ROUTER_TRANSPORT_ZONE = 'Router Transport Zone' FEATURE_NO_DNAT_NO_SNAT = 'No DNAT/No SNAT' FEATURE_ENS_WITH_SEC = 'ENS with security' +FEATURE_ICMP_STRICT = 'Strict list of supported ICMP types and codes' diff --git a/vmware_nsxlib/v3/security.py b/vmware_nsxlib/v3/security.py index 57e7763f..f67555ea 100644 --- a/vmware_nsxlib/v3/security.py +++ b/vmware_nsxlib/v3/security.py @@ -341,17 +341,32 @@ class NsxLibFirewallSection(utils.NsxLibApiBase): # Validate the icmp type & code icmp_type = sg_rule['port_range_min'] icmp_code = sg_rule['port_range_max'] - if icmp_type and icmp_type not in constants.IPV4_ICMP_TYPES: - raise exceptions.InvalidInput( - operation='create_rule', - arg_val=icmp_type, - arg_name='icmp_type') - if (icmp_code and - icmp_code not in constants.IPV4_ICMP_TYPES[icmp_type]): - raise exceptions.InvalidInput( - operation='create_rule', - arg_val=icmp_code, - arg_name='icmp_code for this icmp_type') + icmp_strict = self.nsxlib.feature_supported( + consts.FEATURE_ICMP_STRICT) + if icmp_type: + if (icmp_strict and icmp_type not in + constants.IPV4_ICMP_STRICT_TYPES): + raise exceptions.InvalidInput( + operation='create_rule', + arg_val=icmp_type, + arg_name='icmp_type') + if icmp_type not in constants.IPV4_ICMP_TYPES: + raise exceptions.InvalidInput( + operation='create_rule', + arg_val=icmp_type, + arg_name='icmp_type') + if (icmp_code and icmp_strict and icmp_code not in constants. + IPV4_ICMP_STRICT_TYPES[icmp_type]): + raise exceptions.InvalidInput( + operation='create_rule', + arg_val=icmp_code, + arg_name='icmp_code for this icmp_type') + if (icmp_code and icmp_code not in + constants.IPV4_ICMP_TYPES[icmp_type]): + raise exceptions.InvalidInput( + operation='create_rule', + arg_val=icmp_code, + arg_name='icmp_code for this icmp_type') return self.get_nsservice( consts.ICMP_TYPE_NSSERVICE,