From 622b090f6ae4b2a58a789b6a60eb848c93de6254 Mon Sep 17 00:00:00 2001
From: rootkiller6788 <c8688rickowens@outlook.com>
Date: Thu, 20 Aug 2026 03:10:08 +0800
Subject: [PATCH] Fix iptables rendering for combined icmp/icmpv6 protocols

The iptables generator dropped a term whenever its protocol list
contained any protocol that did not match the term's address family,
so a term with `protocol:: icmp icmpv6` rendered no rules for either
inet or inet6. Filter the protocol list down to the matching address
family instead, and skip the term only when no protocols remain.
---
 capirca/lib/iptables.py    | 24 ++++++++++++++++--------
 tests/lib/iptables_test.py | 25 +++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/capirca/lib/iptables.py b/capirca/lib/iptables.py
index ad242bdd..5303afe0 100644
--- a/capirca/lib/iptables.py
+++ b/capirca/lib/iptables.py
@@ -118,9 +118,21 @@ def __str__(self):
 
     ret_str = []
 
-    # Don't render icmpv6 protocol terms under inet, or icmp under inet6
-    if ((self.af == 'inet6' and 'icmp' in self.term.protocol) or
-        (self.af == 'inet' and 'icmpv6' in self.term.protocol)):
+    # Determine the protocols to render for this address family.
+    if self.term.protocol:
+      protocol = self.term.protocol
+    else:
+      protocol = ['all']
+
+    # Don't render icmpv6 protocol terms under inet, or icmp under inet6.
+    # A term may specify both (e.g. 'protocol:: icmp icmpv6'); in that case
+    # render only the protocols matching this address family, skipping the
+    # term entirely only when none remain.
+    if self.af == 'inet':
+      protocol = [p for p in protocol if p != 'icmpv6']
+    elif self.af == 'inet6':
+      protocol = [p for p in protocol if p != 'icmp']
+    if not protocol:
       logging.debug(self.NO_AF_LOG_PROTO.substitute(
           term=self.term.name,
           proto=', '.join(self.term.protocol),
@@ -180,11 +192,7 @@ def __str__(self):
       return ('# skipped %s due to source or destination prefix rule' %
               self.term.name)
 
-    # protocol
-    if self.term.protocol:
-      protocol = self.term.protocol
-    else:
-      protocol = ['all']
+    # protocol was determined above; skip 'hopopt' in IPv4 context.
     if 'hopopt' in protocol and self.af == 'inet':
       logging.warning('Term %s is using hopopt in IPv4 context.',
                       self.term_name)
diff --git a/tests/lib/iptables_test.py b/tests/lib/iptables_test.py
index 5e987191..faca549f 100644
--- a/tests/lib/iptables_test.py
+++ b/tests/lib/iptables_test.py
@@ -235,6 +235,13 @@
 }
 """
 
+ICMP_AND_ICMPV6_TERM = """
+term permit-icmp {
+  protocol:: icmp icmpv6
+  action:: accept
+}
+"""
+
 HOPOPT_TERM = """
 term hopopt-term {
   protocol:: hopopt
@@ -1179,6 +1186,24 @@ def testIcmpInet6Mismatch(self, mock_debug):
         ' as it has icmp match specified but '
         'the ACL is of inet6 address family.')
 
+  def testIcmpAndIcmpv6Protocol(self):
+    pol = policy.ParsePolicy(GOOD_HEADER_1 + ICMP_AND_ICMPV6_TERM, self.naming)
+    acl = iptables.Iptables(pol, EXP_INFO)
+    result = str(acl)
+    self.assertIn('-p icmp', result,
+                  'icmp protocol should be rendered for inet.')
+    self.assertNotIn('ipv6-icmp', result,
+                     'icmpv6 protocol should not be rendered for inet.')
+
+  def testIcmpAndIcmpv6ProtocolInet6(self):
+    pol = policy.ParsePolicy(IPV6_HEADER_1 + ICMP_AND_ICMPV6_TERM, self.naming)
+    acl = iptables.Iptables(pol, EXP_INFO)
+    result = str(acl)
+    self.assertIn('-p ipv6-icmp', result,
+                  'icmpv6 protocol should be rendered for inet6.')
+    self.assertNotIn('-p icmp', result,
+                     'icmp protocol should not be rendered for inet6.')
+
   def testOwner(self):
     pol = policy.ParsePolicy(GOOD_HEADER_1 + GOOD_TERM_10, self.naming)
     acl = iptables.Iptables(pol, EXP_INFO)