PAN support application any with service application-default

[sfraint]

Add support for any application, which was previously unhandled in Batfish.

In Palo Alto security rules, application-default can be specified as service match, which effectively defers traffic matching to application inspection. In that case, the special any application should match any application defined on the device.

After this PR, Batfish assumes that all traffic will match the any application.

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #5310 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #5310      +/-   ##
============================================
+ Coverage      73.4%    73.4%   +<.01%     
- Complexity    31992    32007      +15     
============================================
  Files          2624     2624              
  Lines        128981   129057      +76     
  Branches      15512    15523      +11     
============================================
+ Hits          94682    94739      +57     
- Misses        26797    26815      +18     
- Partials       7502     7503       +1

Impacted Files	Coverage Δ	Complexity Δ
...epresentation/palo_alto/PaloAltoConfiguration.java	85.8% <100%> (+0.03%)	253 <0> (+1)	⬆️
...tfish/datamodel/visitors/HeaderSpaceConverter.java	27.27% <0%> (-11.19%)	3% <0%> (ø)
...ish/representation/cumulus/CumulusConversions.java	88.01% <0%> (-8.77%)	128% <0%> (+117%)
.../src/main/java/org/batfish/datamodel/flow/Hop.java	53.33% <0%> (-6.67%)	5% <0%> (-1%)
...col/src/main/java/org/batfish/role/InferRoles.java	89.54% <0%> (-1.37%)	50% <0%> (-1%)
...src/main/java/org/batfish/coordinator/PoolMgr.java	64.04% <0%> (-1.13%)	15% <0%> (-1%)
...rg/batfish/dataplane/ibdp/EigrpRoutingProcess.java	92.72% <0%> (-0.39%)	68% <0%> (-1%)
...batfish/src/main/java/org/batfish/main/Driver.java	38.25% <0%> (-0.25%)	28% <0%> (ø)
...sh/representation/cisco_xr/CiscoXrConversions.java	30.47% <0%> (-0.04%)	49% <0%> (ø)
.../cumulus_nclu/CumulusNcluConfigurationBuilder.java	86.25% <0%> (-0.02%)	251% <0%> (ø)
... and 8 more

[sfraint]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1018 at r2 (raw file):

Quoted 7 lines of code…

        if (rule.getAction() == LineAction.PERMIT) {
          // Since Batfish cannot currently match above L4, we follow Cisco-fragments-like logic:
          // When permitting an application, optimistically permit all traffic where the L4 rule
          // matches, assuming it is this application. But when blocking a specific application, do
          // not block all matching L4 traffic, since we can't know it is this specific application.
          serviceDisjuncts.addAll(matchApplications(rule, vsys));
        }

is this the desired behavior even when any is in the set of applications for a deny rule? i.e. no traffic will match a "deny any application" rule w/ application-default specified

I haven't seen any deny rules referencing any application with application-default specified, but it is surely possible

[dhalperi]

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1018 at r2 (raw file):

Previously, sfraint (Spencer Fraint) wrote…

        if (rule.getAction() == LineAction.PERMIT) {
          // Since Batfish cannot currently match above L4, we follow Cisco-fragments-like logic:
          // When permitting an application, optimistically permit all traffic where the L4 rule
          // matches, assuming it is this application. But when blocking a specific application, do
          // not block all matching L4 traffic, since we can't know it is this specific application.
          serviceDisjuncts.addAll(matchApplications(rule, vsys));
        }

is this the desired behavior even when any is in the set of applications for a deny rule? i.e. no traffic will match a "deny any application" rule w/ application-default specified

I haven't seen any deny rules referencing any application with application-default specified, but it is surely possible

Don't you mean all traffic will match?

[dhalperi]

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1044 at r2 (raw file):

Quoted 4 lines of code…

      // Assume all traffic matches some application under the "any" definition
      if (name.equals(CATCHALL_APPLICATION_NAME)) {
        return ImmutableList.of(TrueExpr.INSTANCE);
      }

This almost certainly needs to go down below where we check built-ins? Although hopefully no one overrides the any built-in?

[sfraint]

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @dhalperi, @progwriter, and @sfraint)

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1018 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Don't you mean all traffic will match?

yes, meant all traffic will match, and no traffic will be permitted

[dhalperi]

Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @progwriter and @sfraint)

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1018 at r2 (raw file):

Previously, sfraint (Spencer Fraint) wrote…

yes, meant all traffic will match, and no traffic will be permitted

Is that not the only case this comes up?

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1035 at r2 (raw file):

  }

  private List<AclLineMatchExpr> matchApplications(SecurityRule rule, Vsys vsys) {

Is this function only used above? If so, rename to something that indicates that?

bad attempt: matchServiceForApplications

[dhalperi]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @progwriter and @sfraint)

a discussion (no related file):
Please rename PR to better indicate the change. We absolutely have support for the any application.

---

[sfraint]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dhalperi, @progwriter, and @sfraint)

a discussion (no related file):

Previously, dhalperi (Dan Halperin) wrote…

Please rename PR to better indicate the change. We absolutely have support for the any application.

Updated

---

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1018 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Is that not the only case this comes up?

Chatted more offline about this and decided maybe we should handle the case where a deny rule uses any application with application-default service, since we currently will never match or deny traffic due to a rule like that.

This also seems like something we can add separately from this PR.

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1044 at r2 (raw file):
Did you mean create a built-in for any instead of special-asing it here?

And you cannot override the any built-in 🙂

>>> set vsys vsys1 application any description testing
Server error :  'any' is invalid. 'any' is not allowed

[sfraint]

Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @dhalperi and @progwriter)

---

projects/batfish/src/main/java/org/batfish/representation/palo_alto/PaloAltoConfiguration.java, line 1035 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Is this function only used above? If so, rename to something that indicates that?

bad attempt: matchServiceForApplications

done, didn't think of anything much better so sticking with that

[dhalperi]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @progwriter)