From: Claudiu Belu Date: Tue, 1 Apr 2014 14:36:50 +0000 (+0300) Subject: Fixes Hyper-V agent security group ICMP rules X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f56972193be903c67cd54c57bcc2becf6222aced;p=openstack-build%2Fneutron-build.git 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 --- diff --git a/neutron/plugins/hyperv/agent/security_groups_driver.py b/neutron/plugins/hyperv/agent/security_groups_driver.py index ac3f4c930..9c997f18b 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 c8d8ff357..9b1b467b6 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 1411cfa3c..bcbe6ba0e 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 52970aa52..af70f33b9 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,