From 0e45c8654bbf7045b0a66271294788d268143e00 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Tue, 1 Apr 2014 17:36:50 +0300 Subject: [PATCH] Fixes Hyper-V agent security group ICMP rules Converts ICMP protocol to the equivalent protocol number. Adds default ICMP reject rules. Adds default ANY protocol rules if the rule does not contain any protocol. Closes-Bug: #1299159 Change-Id: Iff51a58fdb532eda0fe7a63abf96004ee74bb073 --- .../hyperv/agent/security_groups_driver.py | 10 ++++++- neutron/plugins/hyperv/agent/utilsv2.py | 27 +++---------------- .../test_hyperv_security_groups_driver.py | 14 ++++++++++ .../tests/unit/hyperv/test_hyperv_utilsv2.py | 3 ++- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/neutron/plugins/hyperv/agent/security_groups_driver.py b/neutron/plugins/hyperv/agent/security_groups_driver.py index ac3f4c930d..9c997f18ba 100644 --- a/neutron/plugins/hyperv/agent/security_groups_driver.py +++ b/neutron/plugins/hyperv/agent/security_groups_driver.py @@ -33,6 +33,7 @@ class HyperVSecurityGroupsDriver(firewall.FirewallDriver): 'egress': utilsv2.HyperVUtilsV2._ACL_DIR_OUT}, 'ethertype': {'IPv4': utilsv2.HyperVUtilsV2._ACL_TYPE_IPV4, 'IPv6': utilsv2.HyperVUtilsV2._ACL_TYPE_IPV6}, + 'protocol': {'icmp': utilsv2.HyperVUtilsV2._ICMP_PROTOCOL}, 'default': "ANY", 'address_default': {'IPv4': '0.0.0.0/0', 'IPv6': '::/0'} } @@ -83,7 +84,7 @@ class HyperVSecurityGroupsDriver(firewall.FirewallDriver): 'direction': self._ACL_PROP_MAP['direction'][rule['direction']], 'acl_type': self._ACL_PROP_MAP['ethertype'][rule['ethertype']], 'local_port': local_port, - 'protocol': self._get_rule_prop_or_default(rule, 'protocol'), + 'protocol': self._get_rule_protocol(rule), 'remote_address': self._get_rule_remote_address(rule) } @@ -130,6 +131,13 @@ class HyperVSecurityGroupsDriver(firewall.FirewallDriver): return rule[ip_prefix] return self._ACL_PROP_MAP['address_default'][rule['ethertype']] + def _get_rule_protocol(self, rule): + protocol = self._get_rule_prop_or_default(rule, 'protocol') + if protocol in self._ACL_PROP_MAP['protocol'].keys(): + return self._ACL_PROP_MAP['protocol'][protocol] + + return protocol + def _get_rule_prop_or_default(self, rule, prop): if prop in rule: return rule[prop] diff --git a/neutron/plugins/hyperv/agent/utilsv2.py b/neutron/plugins/hyperv/agent/utilsv2.py index c8d8ff3573..9b1b467b62 100644 --- a/neutron/plugins/hyperv/agent/utilsv2.py +++ b/neutron/plugins/hyperv/agent/utilsv2.py @@ -59,6 +59,7 @@ class HyperVUtilsV2(utils.HyperVUtils): _IPV6_ANY = '::/0' _TCP_PROTOCOL = 'tcp' _UDP_PROTOCOL = 'udp' + _ICMP_PROTOCOL = '1' _MAX_WEIGHT = 65500 _wmi_namespace = '//./root/virtualization/v2' @@ -314,7 +315,9 @@ class HyperVUtilsV2(utils.HyperVUtils): ipv6_pair = (self._ACL_TYPE_IPV6, self._IPV6_ANY) for direction in [self._ACL_DIR_IN, self._ACL_DIR_OUT]: for acl_type, address in [ipv4_pair, ipv6_pair]: - for protocol in [self._TCP_PROTOCOL, self._UDP_PROTOCOL]: + for protocol in [self._TCP_PROTOCOL, + self._UDP_PROTOCOL, + self._ICMP_PROTOCOL]: self._bind_security_rule( port, direction, acl_type, self._ACL_ACTION_DENY, self._ACL_DEFAULT, protocol, address, weight) @@ -380,28 +383,6 @@ class HyperVUtilsV2R2(HyperVUtilsV2): _PORT_EXT_ACL_SET_DATA = 'Msvm_EthernetSwitchPortExtendedAclSettingData' _MAX_WEIGHT = 65500 - def create_security_rule(self, switch_port_name, direction, acl_type, - local_port, protocol, remote_address): - protocols = [protocol] - if protocol is self._ACL_DEFAULT: - protocols = [self._TCP_PROTOCOL, self._UDP_PROTOCOL] - - for proto in protocols: - super(HyperVUtilsV2R2, self).create_security_rule( - switch_port_name, direction, acl_type, local_port, - proto, remote_address) - - def remove_security_rule(self, switch_port_name, direction, acl_type, - local_port, protocol, remote_address): - protocols = [protocol] - if protocol is self._ACL_DEFAULT: - protocols = ['tcp', 'udp'] - - for proto in protocols: - super(HyperVUtilsV2R2, self).remove_security_rule( - switch_port_name, direction, acl_type, - local_port, proto, remote_address) - def _create_security_acl(self, direction, acl_type, action, local_port, protocol, remote_addr, weight): acl = self._get_default_setting_data(self._PORT_EXT_ACL_SET_DATA) diff --git a/neutron/tests/unit/hyperv/test_hyperv_security_groups_driver.py b/neutron/tests/unit/hyperv/test_hyperv_security_groups_driver.py index 1411cfa3c4..bcbe6ba0e9 100644 --- a/neutron/tests/unit/hyperv/test_hyperv_security_groups_driver.py +++ b/neutron/tests/unit/hyperv/test_hyperv_security_groups_driver.py @@ -157,6 +157,20 @@ class TestHyperVSecurityGroupsDriver(base.BaseTestCase): self.assertEqual(self._driver._ACL_PROP_MAP['address_default']['IPv6'], actual) + def test_get_rule_protocol_icmp(self): + self._test_get_rule_protocol( + 'icmp', self._driver._ACL_PROP_MAP['protocol']['icmp']) + + def test_get_rule_protocol_no_icmp(self): + self._test_get_rule_protocol('tcp', 'tcp') + + def _test_get_rule_protocol(self, protocol, expected): + rule = self._create_security_rule() + rule['protocol'] = protocol + actual = self._driver._get_rule_protocol(rule) + + self.assertEqual(expected, actual) + def _get_port(self): return { 'device': self._FAKE_DEVICE, diff --git a/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py b/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py index 52970aa52a..af70f33b93 100644 --- a/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py +++ b/neutron/tests/unit/hyperv/test_hyperv_utilsv2.py @@ -361,7 +361,8 @@ class TestHyperVUtilsV2(base.BaseTestCase): for direction in [self._utils._ACL_DIR_IN, self._utils._ACL_DIR_OUT]: for acl_type, address in [ipv4_pair, ipv6_pair]: for protocol in [self._utils._TCP_PROTOCOL, - self._utils._UDP_PROTOCOL]: + self._utils._UDP_PROTOCOL, + self._utils._ICMP_PROTOCOL]: calls.append(mock.call(m_port, direction, acl_type, self._utils._ACL_ACTION_DENY, self._utils._ACL_DEFAULT,