diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index a10311a309f..1edc42a9d7e 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -400,8 +400,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver): if rule.get('ethertype') == constants.IPv4: ipv4_sg_rules.append(rule) elif rule.get('ethertype') == constants.IPv6: - if rule.get('protocol') == 'icmp': - rule['protocol'] = 'ipv6-icmp' + if rule.get('protocol') in const.IPV6_ICMP_LEGACY_PROTO_LIST: + rule['protocol'] = constants.PROTO_NAME_IPV6_ICMP ipv6_sg_rules.append(rule) return ipv4_sg_rules, ipv6_sg_rules diff --git a/neutron/common/_constants.py b/neutron/common/_constants.py index 13056a51288..01fd3ebd124 100644 --- a/neutron/common/_constants.py +++ b/neutron/common/_constants.py @@ -41,6 +41,10 @@ IPTABLES_MULTIPORT_ONLY_PROTOCOLS = [ constants.PROTO_NAME_UDPLITE ] +# Legacy IPv6 ICMP protocol list +IPV6_ICMP_LEGACY_PROTO_LIST = [constants.PROTO_NAME_ICMP, + constants.PROTO_NAME_IPV6_ICMP_LEGACY] + # Number of resources for neutron agent side functions to deal # with large sets. # Setting this value does not count on special conditions, it is just a human diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 9eb28dfd198..2f0b0f8d355 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -445,10 +445,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, protocol = constants.IP_PROTOCOL_NAME_ALIASES[protocol] return int(constants.IP_PROTOCOL_MAP.get(protocol, protocol)) - def _get_ip_proto_name_and_num(self, protocol): + def _get_ip_proto_name_and_num(self, protocol, ethertype=None): if protocol is None: return protocol = str(protocol) + # Force all legacy IPv6 ICMP protocol names to be 'ipv6-icmp' + if (ethertype == constants.IPv6 and + protocol in const.IPV6_ICMP_LEGACY_PROTO_LIST): + protocol = constants.PROTO_NAME_IPV6_ICMP if protocol in constants.IP_PROTOCOL_MAP: return [protocol, str(constants.IP_PROTOCOL_MAP.get(protocol))] elif protocol in constants.IP_PROTOCOL_NUM_TO_NAME_MAP: @@ -540,8 +544,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, raise ext_sg.SecurityGroupRulesNotSingleTenant() return sg_groups.pop() + def _make_canonical_ipv6_icmp_protocol(self, rule): + if (rule.get('ethertype') == constants.IPv6 and + rule.get('protocol') in const.IPV6_ICMP_LEGACY_PROTO_LIST): + rule['protocol'] = constants.PROTO_NAME_IPV6_ICMP + def _validate_security_group_rule(self, context, security_group_rule): rule = security_group_rule['security_group_rule'] + self._make_canonical_ipv6_icmp_protocol(rule) self._validate_port_range(rule) self._validate_ip_prefix(rule) self._validate_ethertype_and_protocol(rule) @@ -615,7 +625,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, elif value is None: return none_char elif key == 'protocol': - return str(self._get_ip_proto_name_and_num(value)) + return str(self._get_ip_proto_name_and_num( + value, ethertype=rule.get('ethertype'))) return str(value) comparison_keys = [ diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py index 04ff3274e55..a735b15cbc1 100644 --- a/neutron/tests/unit/extensions/test_securitygroup.py +++ b/neutron/tests/unit/extensions/test_securitygroup.py @@ -519,7 +519,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): with self.security_group(name, description) as sg: security_group_id = sg['security_group']['id'] rule = self._build_security_group_rule( - security_group_id, 'ingress', const.PROTO_NAME_IPV6_ICMP) + security_group_id, 'ingress', const.PROTO_NAME_IPV6_FRAG) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) @@ -1049,7 +1049,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): for k, v, in keys: self.assertEqual(rule['security_group_rule'][k], v) - def test_create_security_group_rule_icmpv6_legacy_protocol_name(self): + def _test_create_security_group_rule_legacy_protocol_name(self, protocol): name = 'webservers' description = 'my webservers' with self.security_group(name, description) as sg: @@ -1057,7 +1057,6 @@ class TestSecurityGroups(SecurityGroupDBTestCase): direction = "ingress" ethertype = const.IPv6 remote_ip_prefix = "2001::f401:56ff:fefe:d3dc/128" - protocol = const.PROTO_NAME_IPV6_ICMP_LEGACY keys = [('remote_ip_prefix', remote_ip_prefix), ('security_group_id', security_group_id), ('direction', direction), @@ -1069,8 +1068,19 @@ class TestSecurityGroups(SecurityGroupDBTestCase): None, None, ethertype) as rule: for k, v, in keys: + # IPv6 ICMP protocol will always be 'ipv6-icmp' + if k == 'protocol': + v = const.PROTO_NAME_IPV6_ICMP self.assertEqual(rule['security_group_rule'][k], v) + def test_create_security_group_rule_ipv6_icmp_legacy_protocol_name(self): + protocol = const.PROTO_NAME_ICMP + self._test_create_security_group_rule_legacy_protocol_name(protocol) + + def test_create_security_group_rule_icmpv6_legacy_protocol_name(self): + protocol = const.PROTO_NAME_IPV6_ICMP_LEGACY + self._test_create_security_group_rule_legacy_protocol_name(protocol) + def test_create_security_group_source_group_ip_and_ip_prefix(self): security_group_id = "4cd70774-cc67-4a87-9b39-7d1db38eb087" direction = "ingress" diff --git a/releasenotes/notes/security-group-ipv6-icmp-221c59dcaf2caa3c.yaml b/releasenotes/notes/security-group-ipv6-icmp-221c59dcaf2caa3c.yaml new file mode 100644 index 00000000000..ed58f09ac6e --- /dev/null +++ b/releasenotes/notes/security-group-ipv6-icmp-221c59dcaf2caa3c.yaml @@ -0,0 +1,14 @@ +--- +upgrade: + - | + Existing IPv6 ICMP security group rules created by using legacy protocol + names ``icmpv6`` and ``icmp`` will now be returned as ``ipv6-icmp`` in + an API GET call. +fixes: + - | + Security group rule code has been changed to better detect duplicate + rules by standardizing on ``ipv6-icmp`` as the protocol field value + for IPv6 ICMP rules. The legacy names ``icmpv6`` and ``icmp`` can still + be used in API POST calls, but API GET calls will return ``ipv6-icmp``. + Partial fix for bug + `1582500 `_.