PAN: Support for rule-type in security rules

projects/batfish/src/main/java/org/batfish/grammar/palo_alto/PaloAltoConfigurationBuilder.java

@@ -289,6 +289,7 @@
 import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_fromContext;
 import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_negate_destinationContext;
 import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_negate_sourceContext;
+import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_rule_typeContext;
 import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_serviceContext;
 import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_sourceContext;
 import org.batfish.grammar.palo_alto.PaloAltoParser.Srs_toContext;
@@ -392,6 +393,7 @@
 import org.batfish.representation.palo_alto.RuleEndpoint;
 import org.batfish.representation.palo_alto.Rulebase;
 import org.batfish.representation.palo_alto.SecurityRule;
+import org.batfish.representation.palo_alto.SecurityRule.RuleType;
 import org.batfish.representation.palo_alto.Service;
 import org.batfish.representation.palo_alto.ServiceBuiltIn;
 import org.batfish.representation.palo_alto.ServiceGroup;
@@ -2472,6 +2474,25 @@ public void exitSrs_action(Srs_actionContext ctx) {
     }
   }
 
+  @Override
+  public void exitSrs_rule_type(Srs_rule_typeContext ctx) {
+    if (ctx.INTERZONE() != null) {
+      _currentSecurityRule.setRuleType(RuleType.INTERZONE);
+    } else if (ctx.INTRAZONE() != null) {
+      if (_currentSecurityRule.getFrom().equals(_currentSecurityRule.getTo())) {
+        _currentSecurityRule.setRuleType(RuleType.INTRAZONE);
+      } else {
+        warn(
+            ctx,
+            "Error: Cannot set 'rule-type intrazone' for security rule with different source and destination zones.");
+      }
+    } else if (ctx.UNIVERSAL() != null) {
+      _currentSecurityRule.setRuleType(RuleType.UNIVERSAL);
+    } else {
+      warn(ctx, "Unsupported security rule-type");
+    }
+  }
+
   private void referenceApplicationLike(
       String name, String uniqueName, PaloAltoStructureUsage usage, ParserRuleContext var) {
     PaloAltoStructureType type =

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java

@@ -149,6 +149,7 @@
 import org.batfish.datamodel.transformation.TransformationStep;
 import org.batfish.representation.palo_alto.OspfAreaNssa.DefaultRouteType;
 import org.batfish.representation.palo_alto.OspfInterface.LinkType;
+import org.batfish.representation.palo_alto.SecurityRule.RuleType;
 import org.batfish.representation.palo_alto.Vsys.NamespaceType;
 import org.batfish.representation.palo_alto.Zone.Type;
 import org.batfish.vendor.StructureUsage;
@@ -490,7 +491,7 @@ private void convertVirtualSystems() {
       }
 
       // Create cross-zone ACLs for each pair of zones, including self-zone.
-      List<Map.Entry<SecurityRule, Vsys>> rules = getAllSecurityRules(vsys);
+      List<Map.Entry<SecurityRule, Vsys>> rules = getAllValidSecurityRules(vsys);
       for (Zone fromZone : vsys.getZones().values()) {
         Type fromType = fromZone.getType();
         for (Zone toZone : vsys.getZones().values()) {
@@ -613,20 +614,7 @@ private IpAccessList generateCrossZoneFilter(
     // Build an ACL Line for each rule that is enabled and applies to this from/to zone pair.
     List<AclLine> lines =
         rules.stream()
-            .filter(
-                e -> {
-                  SecurityRule rule = e.getKey();
-                  if (rule.getDisabled()) {
-                    return false;
-                  } else if (Sets.intersection(
-                          rule.getFrom(), ImmutableSet.of(fromZone.getName(), CATCHALL_ZONE_NAME))
-                      .isEmpty()) {
-                    return false;
-                  }
-                  return !Sets.intersection(
-                          rule.getTo(), ImmutableSet.of(toZone.getName(), CATCHALL_ZONE_NAME))
-                      .isEmpty();
-                })
+            .filter(e -> securityRuleApplies(fromZone.getName(), toZone.getName(), e.getKey(), _w))
             .map(entry -> toIpAccessListLine(entry.getKey(), entry.getValue()))
             .collect(ImmutableList.toImmutableList());
     // Intrazone traffic is allowed by default.
@@ -647,12 +635,46 @@ private IpAccessList generateCrossZoneFilter(
     return IpAccessList.builder().setName(crossZoneFilterName).setLines(lines).build();
   }
 
+  @VisibleForTesting
+  static boolean securityRuleApplies(
+      String fromZoneName, String toZoneName, SecurityRule rule, Warnings warnings) {
+    if (rule.getDisabled()) {
+      return false;
+    }
+    boolean fromZoneInRuleFrom =
+        !Sets.intersection(rule.getFrom(), ImmutableSet.of(fromZoneName, CATCHALL_ZONE_NAME))
+            .isEmpty();
+    boolean toZoneInRuleTo =
+        !Sets.intersection(rule.getTo(), ImmutableSet.of(toZoneName, CATCHALL_ZONE_NAME)).isEmpty();
+
+    if (!fromZoneInRuleFrom || !toZoneInRuleTo) {
+      return false;
+    }
+
+    // rule-type doc:
+    // https://knowledgebase.paloaltonetworks.com/KCSArticleDetail?id=kA10g000000ClomCAC
+    switch (firstNonNull(rule.getRuleType(), RuleType.UNIVERSAL)) {
+      case INTRAZONE:
+        return fromZoneName.equals(toZoneName);
+      case INTERZONE:
+        return !fromZoneName.equals(toZoneName);
+      case UNIVERSAL:
+        return true;
+      default:
+        warnings.redFlag(
+            String.format(
+                "Skipped unhandled rule type '%s' from zone %s to %s",
+                rule.getRuleType(), fromZoneName, toZoneName));
+        return false;
+    }
+  }
+
   /**
    * Collects the security rules from this Vsys and merges the common pre-/post-rulebases from
-   * Panorama.
+   * Panorama. Filters out invalid intrazone rules.
    */
   @SuppressWarnings("PMD.CloseResource") // PMD has a bug for this pattern.
-  private List<Map.Entry<SecurityRule, Vsys>> getAllSecurityRules(Vsys vsys) {
+  private List<Map.Entry<SecurityRule, Vsys>> getAllValidSecurityRules(Vsys vsys) {
     Stream<Map.Entry<SecurityRule, Vsys>> pre =
         _panorama == null
             ? Stream.of()
@@ -667,7 +689,25 @@ private List<Map.Entry<SecurityRule, Vsys>> getAllSecurityRules(Vsys vsys) {
         vsys.getRulebase().getSecurityRules().values().stream()
             .map(r -> new SimpleImmutableEntry<>(r, vsys));
 
-    return Stream.concat(Stream.concat(pre, rules), post).collect(ImmutableList.toImmutableList());
+    return Stream.concat(Stream.concat(pre, rules), post)
+        .filter(e -> checkIntrazoneValidityAndWarn(e.getKey(), _w))
+        .collect(ImmutableList.toImmutableList());
+  }
+
+  /**
+   * Check if the intrazone security rule is valid, and log a warning if it is not. Returns true for
+   * non-intrazone rules.
+   */
+  @VisibleForTesting
+  static boolean checkIntrazoneValidityAndWarn(SecurityRule rule, Warnings w) {
+    if (rule.getRuleType() == RuleType.INTRAZONE && !rule.getFrom().equals(rule.getTo())) {
+      w.redFlag(
+          String.format(
+              "Skipping invalid intrazone security rule: %s. It has different From and To zones: %s vs %s",
+              rule.getName(), rule.getFrom(), rule.getTo()));
+      return false;
+    }
+    return true;
   }
 
   /**

projects/batfish/src/main/java/org/batfish/representation/palo_alto/SecurityRule.java

@@ -14,6 +14,12 @@
 @ParametersAreNonnullByDefault
 public final class SecurityRule implements Serializable {
 
+  public enum RuleType {
+    INTERZONE,
+    INTRAZONE,
+    UNIVERSAL
+  }
+
   // Name of the rule
   @Nonnull private final String _name;
   // Action of the rule
@@ -42,6 +48,9 @@ public final class SecurityRule implements Serializable {
   // Applications
   @Nonnull private final SortedSet<String> _applications;
 
+  // Rule type
+  @Nullable private RuleType _ruleType;
+
   public SecurityRule(String name, Vsys vsys) {
     _action = LineAction.DENY;
     _applications = new TreeSet<>();
@@ -127,6 +136,11 @@ public Vsys getVsys() {
     return _vsys;
   }
 
+  @Nullable
+  public RuleType getRuleType() {
+    return _ruleType;
+  }
+
   public void setAction(LineAction action) {
     _action = action;
   }
@@ -138,4 +152,8 @@ public void setDescription(String description) {
   public void setDisabled(boolean disabled) {
     _disabled = disabled;
   }
+
+  public void setRuleType(@Nullable RuleType ruleType) {
+    _ruleType = ruleType;
+  }
 }

projects/batfish/src/test/java/org/batfish/grammar/palo_alto/PaloAltoGrammarTest.java

@@ -238,6 +238,7 @@
 import org.batfish.representation.palo_alto.RedistRuleRefNameOrPrefix;
 import org.batfish.representation.palo_alto.RuleEndpoint;
 import org.batfish.representation.palo_alto.SecurityRule;
+import org.batfish.representation.palo_alto.SecurityRule.RuleType;
 import org.batfish.representation.palo_alto.ServiceBuiltIn;
 import org.batfish.representation.palo_alto.StaticRoute;
 import org.batfish.representation.palo_alto.Tag;
@@ -2147,6 +2148,28 @@ public void testEmptyZoneOutgoingFilterTraceElements() {
             });
   }
 
+  @Test
+  public void testRulebaseRuleType() {
+    String hostname = "rulebase-rule-type";
+    PaloAltoConfiguration vendorConfig = parsePaloAltoConfig(hostname);
+
+    Map<String, SecurityRule> rules =
+        vendorConfig.getVirtualSystems().get(DEFAULT_VSYS_NAME).getRulebase().getSecurityRules();
+
+    assertThat(rules.get("INTER").getRuleType(), equalTo(RuleType.INTERZONE));
+    assertThat(rules.get("INTRA").getRuleType(), equalTo(RuleType.INTRAZONE));
+    assertThat(rules.get("UNIVERSAL").getRuleType(), equalTo(RuleType.UNIVERSAL));
+    assertThat(rules.get("DEFAULT").getRuleType(), nullValue());
+
+    // the intrazone line should have been rejected -- which leaves this rule as default
+    assertThat(rules.get("BADINTRA").getRuleType(), nullValue());
+    assertThat(
+        vendorConfig.getWarnings().getParseWarnings(),
+        hasItem(
+            hasComment(
+                "Error: Cannot set 'rule-type intrazone' for security rule with different source and destination zones.")));
+  }
+
   @Test
   public void testRulebaseService() {
     String hostname = "rulebase-service";

projects/batfish/src/test/java/org/batfish/representation/palo_alto/PaloAltoConfigurationTest.java

@@ -8,6 +8,7 @@
 import static org.batfish.datamodel.matchers.IpAccessListMatchers.accepts;
 import static org.batfish.datamodel.matchers.IpAccessListMatchers.rejects;
 import static org.batfish.datamodel.matchers.IpAccessListMatchers.rejectsByDefault;
+import static org.batfish.representation.palo_alto.PaloAltoConfiguration.checkIntrazoneValidityAndWarn;
 import static org.batfish.representation.palo_alto.PaloAltoConfiguration.computeObjectName;
 import static org.batfish.representation.palo_alto.PaloAltoConfiguration.generateCrossZoneCalls;
 import static org.batfish.representation.palo_alto.PaloAltoConfiguration.generateCrossZoneCallsFromExternal;
@@ -18,23 +19,30 @@
 import static org.batfish.representation.palo_alto.PaloAltoConfiguration.generateSgSgLines;
 import static org.batfish.representation.palo_alto.PaloAltoConfiguration.generateSharedGatewayOutgoingFilter;
 import static org.batfish.representation.palo_alto.PaloAltoConfiguration.generateVsysSharedGatewayCalls;
+import static org.batfish.representation.palo_alto.PaloAltoConfiguration.securityRuleApplies;
 import static org.batfish.representation.palo_alto.PaloAltoTraceElementCreators.zoneToZoneMatchTraceElement;
 import static org.batfish.representation.palo_alto.PaloAltoTraceElementCreators.zoneToZoneRejectTraceElement;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import java.util.Map;
 import javax.annotation.Nonnull;
 import javax.annotation.ParametersAreNonnullByDefault;
+import org.batfish.common.Warnings;
 import org.batfish.datamodel.Flow;
 import org.batfish.datamodel.Ip;
 import org.batfish.datamodel.IpAccessList;
 import org.batfish.datamodel.IpProtocol;
 import org.batfish.datamodel.NamedPort;
 import org.batfish.datamodel.NetworkFactory;
+import org.batfish.representation.palo_alto.SecurityRule.RuleType;
 import org.batfish.representation.palo_alto.Zone.Type;
 import org.junit.Before;
 import org.junit.Test;
@@ -623,4 +631,75 @@ public void testGenerateVsysSharedGatewayCallsMissingLayer3() {
     // no lines should be returned since externalToZone has no layer-3 zone
     assertEquals(generateVsysSharedGatewayCalls(sharedGateway, vsys).count(), 0L);
   }
+
+  /** Covers universal and default case of no rule-type */
+  @Test
+  public void testSecurityRuleAppliesRuleTypeUniversal() {
+    SecurityRule rule = new SecurityRule("rule", new Vsys(VSYS_NAME));
+    rule.getFrom().addAll(ImmutableList.of("A", "B"));
+    rule.getTo().addAll(ImmutableList.of("C", "B"));
+
+    Warnings w = new Warnings();
+
+    assertTrue(securityRuleApplies("A", "B", rule, w));
+    assertTrue(securityRuleApplies("A", "C", rule, w));
+    assertTrue(securityRuleApplies("B", "B", rule, w));
+
+    assertFalse(securityRuleApplies("A", "A", rule, w));
+    assertFalse(securityRuleApplies("C", "A", rule, w));
+    assertFalse(securityRuleApplies("A", "D", rule, w));
+  }
+
+  @Test
+  public void testSecurityRuleAppliesRuleTypeInterzone() {
+    SecurityRule rule = new SecurityRule("rule", new Vsys(VSYS_NAME));
+    rule.getFrom().addAll(ImmutableList.of("A", "B"));
+    rule.getTo().addAll(ImmutableList.of("C", "B"));
+    rule.setRuleType(RuleType.INTERZONE);
+
+    Warnings w = new Warnings();
+
+    assertTrue(securityRuleApplies("A", "C", rule, w));
+    assertTrue(securityRuleApplies("A", "B", rule, w));
+
+    assertFalse(securityRuleApplies("A", "A", rule, w));
+    assertFalse(securityRuleApplies("B", "B", rule, w));
+    assertFalse(securityRuleApplies("A", "D", rule, w));
+  }
+
+  @Test
+  public void testSecurityRuleAppliesRuleTypeIntrazone() {
+    SecurityRule rule = new SecurityRule("rule", new Vsys(VSYS_NAME));
+    rule.getFrom().addAll(ImmutableList.of("A", "B"));
+    rule.getTo().addAll(ImmutableList.of("A", "B"));
+    rule.setRuleType(RuleType.INTRAZONE);
+
+    Warnings w = new Warnings();
+
+    assertTrue(securityRuleApplies("A", "A", rule, w));
+    assertFalse(securityRuleApplies("A", "B", rule, w));
+    assertFalse(securityRuleApplies("D", "D", rule, w));
+  }
+
+  @Test
+  public void testCheckIntrazoneValidityAndWarn() {
+    SecurityRule rule = new SecurityRule("rule", new Vsys(VSYS_NAME));
+    rule.getFrom().addAll(ImmutableList.of("A", "B"));
+    rule.getTo().addAll(ImmutableList.of("A", "B"));
+
+    // non-intrazone
+    assertTrue(checkIntrazoneValidityAndWarn(rule, new Warnings()));
+
+    // valid intrazone
+    rule.setRuleType(RuleType.INTRAZONE);
+    assertTrue(checkIntrazoneValidityAndWarn(rule, new Warnings()));
+
+    // invalid intrazone
+    Warnings w = new Warnings(true, true, true);
+    rule.getTo().add("C");
+    assertFalse(checkIntrazoneValidityAndWarn(rule, w));
+    assertThat(
+        Iterables.getOnlyElement(w.getRedFlagWarnings()).getText(),
+        containsString("Skipping invalid intrazone security rule"));
+  }
 }

projects/batfish/src/test/resources/org/batfish/grammar/palo_alto/testconfigs/rulebase-rule-type

@@ -0,0 +1,35 @@
+set deviceconfig system hostname rulebase-rule-type
+set network interface ethernet ethernet1/1 layer3 ip 1.1.1.1/24
+set network interface ethernet ethernet1/4 layer3 ip 1.1.4.1/24
+set zone z1 network layer3 [ ethernet1/1 ]
+set zone z2 network layer3 ethernet1/4
+
+set rulebase security rules INTER from z1
+set rulebase security rules INTER to z2
+set rulebase security rules INTER source any
+set rulebase security rules INTER destination any
+set rulebase security rules INTER rule-type interzone
+
+set rulebase security rules INTRA from z1
+set rulebase security rules INTRA to z1
+set rulebase security rules INTRA source any
+set rulebase security rules INTRA destination any
+set rulebase security rules INTRA rule-type intrazone
+
+set rulebase security rules BADINTRA from z1
+set rulebase security rules BADINTRA to z2
+set rulebase security rules BADINTRA source any
+set rulebase security rules BADINTRA destination any
+set rulebase security rules BADINTRA rule-type intrazone
+
+set rulebase security rules UNIVERSAL from z1
+set rulebase security rules UNIVERSAL to z2
+set rulebase security rules UNIVERSAL source any
+set rulebase security rules UNIVERSAL destination any
+set rulebase security rules UNIVERSAL rule-type universal
+
+set rulebase security rules DEFAULT from z1
+set rulebase security rules DEFAULT to z2
+set rulebase security rules DEFAULT source any
+set rulebase security rules DEFAULT destination any
+