Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PAN: trace details for security rules #6208

Merged
merged 6 commits into from
Sep 16, 2020

Conversation

sfraint
Copy link
Member

@sfraint sfraint commented Sep 16, 2020

Add some shallow trace details for Palo Alto security rules. Specifically, adding basic info on first source address, first destination address, and first service matched for a given rule.


Some example traces for cross-zone policies might look like this:

Matched security rule RULE1
   Matched source address
      Matched address-group addr_group1
   Matched destination address
      Matched address object addr2
   Matched a service
Matched security rule RULE2
   Matched source address
      Matched address value 10.10.10.10
   Matched destination address
      Matched address value 10.11.12.0/24
   Matched service application-default
Matched security rule RULE3
   Matched source address
      Matched address any
   Matched destination address
      Matched address any
   Matched service any

@batfish-bot
Copy link

This change is Reviewable

@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #6208 into master will increase coverage by 0.01%.
The diff coverage is 90.00%.

@@             Coverage Diff              @@
##             master    #6208      +/-   ##
============================================
+ Coverage     72.90%   72.92%   +0.01%     
- Complexity    34872    34909      +37     
============================================
  Files          2826     2826              
  Lines        142152   142234      +82     
  Branches      17070    17081      +11     
============================================
+ Hits         103639   103720      +81     
+ Misses        30313    30298      -15     
- Partials       8200     8216      +16     
Impacted Files Coverage Δ Complexity Δ
...n/java/org/batfish/datamodel/acl/AndMatchExpr.java 93.75% <ø> (ø) 9.00 <0.00> (ø)
...in/java/org/batfish/datamodel/acl/OrMatchExpr.java 87.50% <ø> (ø) 8.00 <0.00> (ø)
...a/org/batfish/datamodel/acl/AclLineMatchExprs.java 83.03% <50.00%> (-0.61%) 49.00 <1.00> (+1.00) ⬇️
...tfish/representation/palo_alto/ServiceBuiltIn.java 79.31% <50.00%> (-4.69%) 11.00 <2.00> (+2.00) ⬇️
...epresentation/palo_alto/PaloAltoConfiguration.java 87.60% <90.56%> (+0.50%) 347.00 <14.00> (+16.00)
...tation/palo_alto/PaloAltoTraceElementCreators.java 100.00% <100.00%> (ø) 21.00 <11.00> (+11.00)
...rg/batfish/identifiers/StorageBasedIdResolver.java 88.88% <0.00%> (-4.45%) 27.00% <0.00%> (ø%)
...org/batfish/datamodel/flow/BidirectionalTrace.java 81.81% <0.00%> (-2.28%) 14.00% <0.00%> (-1.00%)
...col/src/main/java/org/batfish/role/InferRoles.java 89.54% <0.00%> (-1.37%) 50.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/PoolMgr.java 59.25% <0.00%> (-1.24%) 15.00% <0.00%> (-1.00%)
... and 13 more

Copy link
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @anothermattbrown, @corinaminer, and @sfraint)


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

// Tracing past NotExpr does not work well, so convert from Not(Or(...)) to
              // And(Not(...)) to push Not further down in trace
              ? new AndMatchExpr(negateMatchIps(dstExprs), matchDestinationAddressTraceElement())

What does the final trace look like here?

Copy link
Member

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @anothermattbrown and @corinaminer)


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

Previously, dhalperi (Dan Halperin) wrote…
// Tracing past NotExpr does not work well, so convert from Not(Or(...)) to
              // And(Not(...)) to push Not further down in trace
              ? new AndMatchExpr(negateMatchIps(dstExprs), matchDestinationAddressTraceElement())

What does the final trace look like here?

OK to handle later.

@dhalperi dhalperi merged commit e6c8e75 into master Sep 16, 2020
@dhalperi dhalperi deleted the pan-shallow-trace-details-w-addr branch September 16, 2020 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants