From 8464aab22107e3402783518ff28adb9176439aed Mon Sep 17 00:00:00 2001
From: Capirca Team <no-reply@google.com>
Date: Wed, 19 Feb 2025 18:01:23 -0800
Subject: [PATCH] Fixing issue in the Palo Alto generator if there is an
 address family mismatch in ICMP/ICMPv6 rules for a Mixed address family
 filter

PiperOrigin-RevId: 728888559
---
 capirca/lib/paloaltofw.py    | 14 ++++++
 tests/lib/paloaltofw_test.py | 87 +++++++++++++++++++++++++++++++++++-
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/capirca/lib/paloaltofw.py b/capirca/lib/paloaltofw.py
index 26ec7d9c..33e6fbe7 100644
--- a/capirca/lib/paloaltofw.py
+++ b/capirca/lib/paloaltofw.py
@@ -586,9 +586,23 @@ def _TranslatePolicy(self, pol, exp_info):
           exclude_address_family.append(4)
         elif filter_type == "mixed":
           if "ip4-ip4" in flows and "ip6-ip6" not in flows:
+            if "icmpv6" in term.protocol:
+              logging.warning(
+                  "WARNING: Term %s in policy %s>%s references ICMPv6,"
+                  "but does not have IPv6 traffic flows, "
+                  "term will not be rendered.", term.name, self.from_zone,
+                  self.to_zone)
+              continue
             exclude_address_family.append(6)
             pass
           elif "ip6-ip6" in flows and "ip4-ip4" not in flows:
+            if "icmp" in term.protocol:
+              logging.warning(
+                  "WARNING: Term %s in policy %s>%s references ICMP protocol, "
+                  "but does not have IPv4 traffic flows, "
+                  "term will not be rendered.", term.name, self.from_zone,
+                  self.to_zone)
+              continue
             exclude_address_family.append(4)
             pass
           elif "ip4-ip4" in flows and "ip6-ip6" in flows:
diff --git a/tests/lib/paloaltofw_test.py b/tests/lib/paloaltofw_test.py
index 9ff7dbab..e800e52d 100644
--- a/tests/lib/paloaltofw_test.py
+++ b/tests/lib/paloaltofw_test.py
@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Unit test for Palo Alto Firewalls acl rendering module."""
-
 from absl.testing import absltest
 from unittest import mock
 
@@ -1814,5 +1813,91 @@ def testAddrObj(self):
     addrs = {elem.text for elem in x}
     self.assertEqual({"6000::/3"}, addrs, output)
 
+  def testMixedICMPFlow(self):
+    definitions = naming.Naming()
+    definitions._ParseLine('NET1 = 10.1.0.0/24', 'networks')
+    definitions._ParseLine('NET2 = 10.2.0.0/24', 'networks')
+    definitions._ParseLine('NET3 = 2001:db8:0:aa::/64', 'networks')
+    definitions._ParseLine('       2001:db8:0:bb::/64', 'networks')
+    definitions._ParseLine('NET4 = 4000::/2', 'networks')
+
+    POL = """
+header {
+  target:: paloalto from-zone trust to-zone untrust %s
+}
+term rule-1 {
+%s
+  action:: accept
+  protocol::icmpv6
+}"""
+
+    T = """
+  source-address:: NET1
+  destination-address:: NET2 NET3
+"""
+    # This checks if the rule is properly dropped due to ICMPv6 being used with
+    # no IPv6 source address
+    pol = policy.ParsePolicy(POL % ('mixed', T), definitions)
+    paloalto = paloaltofw.PaloAltoFW(pol, EXP_INFO)
+    output = str(paloalto)
+    x = paloalto.config.findall(PATH_RULES + "/entry[@name='rule-1']/")
+    self.assertFalse(x, output)
+
+    T = """
+  source-address:: NET3
+  destination-address:: NET4
+"""
+    pol = policy.ParsePolicy(POL % ('mixed', T), definitions)
+    paloalto = paloaltofw.PaloAltoFW(pol, EXP_INFO)
+    output = str(paloalto)
+    x = paloalto.config.findall(
+        PATH_RULES + "/entry[@name='rule-1']/application"
+    )
+    self.assertIsNotNone(x, output)
+    x = paloalto.config.findall(
+        PATH_RULES + "/entry[@name='rule-1']/application/member"
+    )
+    apps = {elem.text for elem in x}
+    self.assertIn('ipv6-icmp', apps)
+
+    POL = """
+header {
+  target:: paloalto from-zone trust to-zone untrust %s
+}
+term rule-1 {
+%s
+  action:: accept
+  protocol::icmp
+}"""
+
+    T = """
+  source-address:: NET1
+  destination-address:: NET2 NET3
+"""
+
+    pol = policy.ParsePolicy(POL % ('mixed', T), definitions)
+    paloalto = paloaltofw.PaloAltoFW(pol, EXP_INFO)
+    output = str(paloalto)
+    x = paloalto.config.findall(
+        PATH_RULES + "/entry[@name='rule-1']/application"
+    )
+    self.assertIsNotNone(x, output)
+    x = paloalto.config.findall(
+        PATH_RULES + "/entry[@name='rule-1']/application/member"
+    )
+    apps = {elem.text for elem in x}
+    self.assertIn('icmp', apps)
+    T = """
+  source-address:: NET3
+  destination-address:: NET4
+"""
+    # This checks if the rule is properly dropped due to ICMP being used with
+    # no IPv4 source or destination address
+    pol = policy.ParsePolicy(POL % ('mixed', T), definitions)
+    paloalto = paloaltofw.PaloAltoFW(pol, EXP_INFO)
+    output = str(paloalto)
+    x = paloalto.config.findall(PATH_RULES + "/entry[@name='rule-1']/")
+    self.assertFalse(x, output)
+
 if __name__ == '__main__':
   absltest.main()