From 10af328885ac0f85f161c454dd53463cee57bb23 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Tue, 5 Mar 2024 11:59:05 -0500 Subject: [PATCH] Use the system-dependent string for IP protocol 4 iptables-save uses a system-dependent value, usually that found in /etc/protocols, when 'ipip' is given as the security group protocol. The intent is to always use the string value for IP protocol '4', as iptables-save has no '-n' flag to print values numerically. This updates a previous change (793dfb04d) that hard-coded that string to 'ipencap', which broke CentOS/Fedora, which uses 'ipv4'. For this reason we cannot hard-code anything in neutron-lib, this needs to be added dynamically, so this one-line change needs to stay here, and effectively closes the bug. Closes-bug: #2054324 Change-Id: Ic40b539c9ef5cfa4cbbd6575e19e653342e8342b (cherry picked from commit cd1d191e335dca707ac9324fd81e642cb453a6cf) --- neutron/agent/linux/iptables_firewall.py | 12 +++++--- .../agent/linux/test_iptables_firewall.py | 28 +++++++++++-------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index c6e2da8e1be..05d78fc1526 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -769,10 +769,14 @@ class IptablesFirewallDriver(firewall.FirewallDriver): if not self._iptables_protocol_name_map: tmp_map = constants.IPTABLES_PROTOCOL_NAME_MAP.copy() tmp_map.update(self._local_protocol_name_map()) - # TODO(haleyb): remove once neutron-lib with fix is available - # - 'ipip' uses 'ipencap' to match IPPROTO_IPIP from in.h, - # which is IP-ENCAP/'4' in /etc/protocols (see bug #2054324) - tmp_map[constants.PROTO_NAME_IPIP] = 'ipencap' + # iptables-save uses different strings for 'ipip' (protocol 4) + # depending on the distro, which corresponds to the entry for + # '4' in /etc/protocols. For example: + # - 'ipencap' in Ubuntu + # - 'ipv4' in CentOS/Fedora + # For this reason, we need to map the string for 'ipip' to the + # system-dependent string for '4', see bug #2054324. + tmp_map[constants.PROTO_NAME_IPIP] = tmp_map['4'] self._iptables_protocol_name_map = tmp_map return self._iptables_protocol_name_map diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py index a2bf18c3652..fec4e9c1eb9 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py +++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py @@ -490,37 +490,43 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase): self._test_prepare_port_filter(rule, ingress, egress) def test_filter_ipv4_ingress_protocol_ipip(self): - # 'ipip' via the API uses 'ipencap' to match what iptables-save - # uses, which is IP-ENCAP/'4' from /etc/protocols (see bug #2054324) + # We want to use what the system-dependent string here is for 'ipip', + # as it could be 'ipencap' or 'ipv4' depending on the distro. + # See bug #2054324. rule = {'ethertype': 'IPv4', 'direction': 'ingress', 'protocol': 'ipip'} + expected_proto_name = self.firewall._iptables_protocol_name('ipip') ingress = mock.call.add_rule('ifake_dev', - '-p ipencap -j RETURN', + '-p %s -j RETURN' % expected_proto_name, top=False, comment=None) egress = None self._test_prepare_port_filter(rule, ingress, egress) - def test_filter_ipv4_ingress_protocol_ipip_by_num(self): - # '4' via the API uses 'ipencap' to match what iptables-save - # uses, which is IP-ENCAP/'4' from /etc/protocols (see bug #2054324) + def test_filter_ipv4_ingress_protocol_4(self): + # We want to use what the system-dependent string here is for '4', + # as it could be 'ipencap' or 'ipv4' depending on the distro. + # See bug #2054324. rule = {'ethertype': 'IPv4', 'direction': 'ingress', 'protocol': '4'} + expected_proto_name = self.firewall._iptables_protocol_name('4') ingress = mock.call.add_rule('ifake_dev', - '-p ipencap -j RETURN', + '-p %s -j RETURN' % expected_proto_name, top=False, comment=None) egress = None self._test_prepare_port_filter(rule, ingress, egress) - def test_filter_ipv4_ingress_protocol_ipencap_by_num(self): - # '94' via the API uses 'ipip' to match what iptables-save - # uses, which is IPIP/'94' from /etc/protocols (see bug #2054324) + def test_filter_ipv4_ingress_protocol_94(self): + # We want to use what the system-dependent string here is for '94', + # as it could be 'ipip' or something else depending on the distro. + # See bug #2054324. rule = {'ethertype': 'IPv4', 'direction': 'ingress', 'protocol': '94'} + expected_proto_name = self.firewall._iptables_protocol_name('94') ingress = mock.call.add_rule('ifake_dev', - '-p ipip -j RETURN', + '-p %s -j RETURN' % expected_proto_name, top=False, comment=None) egress = None self._test_prepare_port_filter(rule, ingress, egress)