ASA: support service-object ip (#5887)

Fix #5875.
batfish · Jun 10, 2020 · e82abcf · e82abcf
1 parent bead446
commit e82abcf
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 2 deletions.
diff --git a/...h-common-protocol/src/test/java/org/batfish/datamodel/matchers/ConfigurationMatchers.java b/...h-common-protocol/src/test/java/org/batfish/datamodel/matchers/ConfigurationMatchers.java
@@ -46,6 +46,7 @@
 import org.batfish.datamodel.tracking.TrackMethod;
 import org.batfish.datamodel.vendor_family.VendorFamily;
 import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
 
 public class ConfigurationMatchers {
 
@@ -152,6 +153,13 @@ public static HasInterfaces hasInterfaces(
     return new HasInterfaces(subMatcher);
   }
 
+  /**
+   * Provides a matcher that matches if the configuration has an IpAccessList with specified name.
+   */
+  public static HasIpAccessList hasIpAccessList(@Nonnull String name) {
+    return hasIpAccessList(name, Matchers.any(IpAccessList.class));
+  }
+
   /**
    * Provides a matcher that matches if the provided {@code subMatcher} matches the configuration's
    * IpAccessList with specified name.

diff --git a/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java b/projects/batfish/src/main/java/org/batfish/grammar/cisco/CiscoControlPlaneExtractor.java
@@ -3561,7 +3561,11 @@ public void exitService_specifier_icmp(Service_specifier_icmpContext ctx) {
 
   @Override
   public void exitService_specifier_protocol(Service_specifier_protocolContext ctx) {
-    _currentServiceObject.addProtocol(toIpProtocol(ctx.protocol()));
+    @Nullable IpProtocol protocol = toIpProtocol(ctx.protocol());
+    if (protocol != null) {
+      _currentServiceObject.addProtocol(protocol);
+    }
+    // Else protocol is ip or ipv4, so anything is valid.
   }
 
   @Override

diff --git a/projects/batfish/src/main/java/org/batfish/representation/cisco/ServiceObject.java b/projects/batfish/src/main/java/org/batfish/representation/cisco/ServiceObject.java
@@ -38,7 +38,7 @@ public void addDstPorts(List<SubRange> dstPorts) {
     _dstPorts.addAll(dstPorts);
   }
 
-  public void addProtocol(IpProtocol protocol) {
+  public void addProtocol(@Nonnull IpProtocol protocol) {
     _protocols.add(protocol);
   }
 

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cisco/BUILD b/projects/batfish/src/test/java/org/batfish/grammar/cisco/BUILD
@@ -19,6 +19,7 @@ junit_tests(
         "//projects/batfish:batfish_testlib",
         "//projects/batfish-common-protocol:common",
         "//projects/batfish-common-protocol:common_testlib",
+        "//projects/batfish-common-protocol/src/test/java/org/batfish/common/bdd:matchers",
         "//projects/batfish-common-protocol/src/test/java/org/batfish/datamodel/matchers",
         "//projects/batfish/src/main/java/org/batfish/grammar/cisco",
         "//projects/batfish/src/main/java/org/batfish/representation/cisco",

diff --git a/projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java b/projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java
@@ -264,6 +264,9 @@
 import org.batfish.common.NetworkSnapshot;
 import org.batfish.common.Warnings;
 import org.batfish.common.WellKnownCommunity;
+import org.batfish.common.bdd.BDDMatchers;
+import org.batfish.common.bdd.BDDPacket;
+import org.batfish.common.bdd.IpAccessListToBdd;
 import org.batfish.common.util.CommonUtil;
 import org.batfish.common.util.IpsecUtil;
 import org.batfish.config.Settings;
@@ -1037,6 +1040,31 @@ public void testAsaSecurityLevelAndFilters() throws IOException {
             highIface3, hasPreTransformationOutgoingFilter(accepts(anyFlow, lowIface4, c))));
   }
 
+  @Test
+  public void testAsaGh5875() throws IOException {
+    Configuration c = parseConfig("asa-gh-5875");
+    BDDPacket p = new BDDPacket();
+
+    {
+      String aclName = computeServiceObjectGroupAclName("IP_GROUP");
+      assertThat(c, hasIpAccessList(aclName));
+      IpAccessList acl = c.getIpAccessLists().get(aclName);
+      assertThat(IpAccessListToBdd.toBDD(p, acl), BDDMatchers.isOne());
+    }
+    {
+      String aclName = computeServiceObjectGroupAclName("TCP_GROUP");
+      assertThat(c, hasIpAccessList(aclName));
+      IpAccessList acl = c.getIpAccessLists().get(aclName);
+      assertThat(IpAccessListToBdd.toBDD(p, acl), equalTo(p.getIpProtocol().value(IpProtocol.TCP)));
+    }
+    {
+      String aclName = computeServiceObjectGroupAclName("AH_GROUP");
+      assertThat(c, hasIpAccessList(aclName));
+      IpAccessList acl = c.getIpAccessLists().get(aclName);
+      assertThat(IpAccessListToBdd.toBDD(p, acl), equalTo(p.getIpProtocol().value(IpProtocol.AHP)));
+    }
+  }
+
   @Test
   public void testAsaServiceObject() throws IOException {
     String hostname = "asa-service-object";

diff --git a/projects/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/asa-gh-5875 b/projects/batfish/src/test/resources/org/batfish/grammar/cisco/testconfigs/asa-gh-5875
@@ -0,0 +1,16 @@
+!
+hostname asa-gh-5875
+! test for https://github.com/batfish/batfish/issues/5875
+!
+object-group service IP_GROUP
+ ! This should mean any ip packet, and not crash.
+ service-object ip
+!
+object-group service TCP_GROUP
+ ! This should mean any tcp packet, and not crash.
+ service-object tcp
+!
+object-group service AH_GROUP
+ ! This should mean any AH packet, and not crash.
+ service-object ah
+!