Handle overlaps in except and allow CIDRs

[Pavani-Panakanti]

Issue #, if available:
If there is an overlap of IPs in an allow rule and in an except block of other rule, we do not handle the merging

Description of changes:

Example:

egress:
  - to:
      - ipBlock:
          cidr: 192.168.0.0/16
    ports:
      - port: 3306
  - to:
      - ipBlock:
          cidr: 0.0.0.0/0
          except:
            - 192.168.0.0/16

Before this change we evaluate this as

• 0.0.0.0/0 - startPort:0 endPort: 0
• except block - 192.168.0.0/16 - startPort: 0 endPort:0
• 192.168.0.0/16 - allow on port 3306, as this is part of 0.0.0.0/0 allow on all ports. Append previous ports to this new L4 info. Final L4 ports info looks as below
  • startPort: 3306, endPort: 0 - allow
  • startPort:0, endPort: 0 - allow
  • startPort:0, endPort:0 - deny

The correct behavior for 192.168.0.0/16 should be allow on 3306 as explicit allow takes precedence and deny on rest all ports due to except
New evaluation logic - all except blocks are handled at end

• 0.0.0.0/0 - startPort:0 endPort: 0
• 192.168.0.0/16 - allow on port 3306. This is part of 0.0.0.0/0, so check if it is part of any except under this cidr. If yes do not add ports of 0.0.0.0/0 to 192.168.0.0/16. If no, add ports. In this case it is part of except so do not add it
• Handle all except at end. For every except key, check if entry is already present in cidrsMap. If yes, there is some explicit allow and everything else will be default deny so no change required. If no, we need to add deny all entry to make sure we deny all traffic and do not allow it if it is part of some other superset CIDR

Other Changes in the PR:

• Code refactoring to make it simpler and easy to follow
  • catch all need not be handled separately. We evaluate rules in the order of prefix length, so catch all will be handled first followed by others
• If no protocol specified, it should be allow any protocol. Changed default to any_ip_protocol from tcp
• For any_ip_protocol, check start and end ports for traffic evaluation. Before fix if it is any_protocol we allow on all ports
  • example:
    • Port:53 → This should be evaluated as allow any_protocol on port 53

Testing done with multiple cases of except overlaps

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[jayanthvn]

Can you also add a test case for this in our integration suite?

[orsenthil]

As an enhancement, could you document the behavior of this function as function doc string itself. We pack a lot inside the function here.

[orsenthil]

Enhancement Request: We can format this and explain the behavior of in the comment.
a) match any problem and ensure either destination port matches or falls within the range.

[orsenthil]

We need more test cases here that will cover for the scenarios described in the description of the PR are handled by the test case.

192.168.0.0/16 - allow on port 3306. This is part of 0.0.0.0/0, so check if it is part of any except under this cidr. If yes do not add ports of 0.0.0.0/0 to 192.168.0.0/16. If no, add ports. 

Handle all except at end. For every except key, check if entry is already present in cidrsMap. If yes, there is some explicit allow and everything else will be default deny so no change required. If no, we need to add deny all entry to make sure we deny all traffic and do not allow it if it is part of some other superset CIDR

The code change and the logic looks good to me. As noted in the review of the design doc, I wanted to see if there is any simpler way, but handling all except at the end looks alright.

Coverage with test case can help us push this PR to finish line here. Thank you.