np-guard / vpc-network-config-analyzer Goto Github PK

• Please open an issue on merging overlapping/touching IP-blocks. It can improve results here. E.g., replacing 161.26.0.0/16,161.27.0.0/16 with 161.26.0.0/15
  Originally posted by @zivnevo in #95 (comment)

• grouping of ip-blocks should be always applied

1. The input file does not contain "list" of address prefixes, thus it does not know what is the address prefixes of the VPCs.
2. Private addresses are considered private by the analyzer

Thus, there could be sg's rules referring solely to private address that are not within the VPC. At the current setting the analyzer will not be able to identify them since the address prefixes are not known to it. Perhaps it makes sense to add warning for these cases as they may indicate a mistake.

Priority: nice to have

• Usage
• How it works with collector
• Understanding the output
• Build instructions

The result should reflect allowed/denied connections considering both types of config resources

support grouping with all_subnets:

1. Group external addresses
2. Group subnets

not relevant to single_subnet

• remove the "architecture" part in the endpoint connectivity output

public gateway can be attached to multiple subnets in the zone.
current implementation assumed only one subnet per pgw.

The function GetExternalNetworkNodes seems to have an issue, where not all public internet ranges are added after computation of disjoint ip-blocks.

The concept of an IP block and the manipulation of IP blocks will be common to all vendors for sure.

consider the the output with flag -grouping , and support it with drawio format

• connectivity diff - see issue #163
• permits/forbids
• linting - see issue #116
• explainability - see issue #42

uid as CRN and not ID

• initial support: consider full subnet ranges only
• extend: support partial subnet range if required (NACL connectivity differs on certain ranges within the subnet).

• how should it look
• how close can it get to graphs from documentation?

Textual connectivity report - currently plan to handle two use cases:

• src to dst connectivity considering the supported vpc resources
• NACL connectivity considering only its attached subnet, no certain pair of src-dst as input

Visualization of connectivity report :

• TODO

input: vpc config dump in json file
output: connectivity results (ingress/ egress) for vpc elements considering SG/NACL

          ```suggestion

vsi3a-ky[10.240.30.5],vsi3b-ky[10.240.30.6],vsi3c-ky[10.240.30.4] => vsi3a-ky[10.240.30.5],vsi3b-ky[10.240.30.6],vsi3c-ky[10.240.30.4] : All Connections
vsi3a-ky[10.240.30.5],vsi3b-ky[10.240.30.6],vsi3c-ky[10.240.30.4] => vsi1-ky[10.240.10.4] : All Connections

This requires adding a self loop to each VSI, run the grouping algorithm, then remove left-over self loops.
Can be handled in a future PR.

_Originally posted by @zivnevo in https://github.com/np-guard/vpc-network-config-analyzer/pull/92#discussion_r1253989708_
            

fip is applied on interface level and its connectivity not affected by nacl rules, only sg rules.

• merge endpoints with equivalent connectivity (subnet / sets of subnets...)
• merge external ip-ranges with equivalent connectivity

Two modifications:

1. The VPC rectangle should be contained in an "IBM Cloud" rectangle.
2. The "public network" rectangle should be to the left of "IBM Cloud" (not containing anything)

See standard examples in the official repo: https://github.ibm.com/ibmcloud/ibm-cloud-stencils#v20-examples

Consider moving Details(), DetailsMap() and Kind() to NamedResourceIntf (and maybe rename it to VPCResourceIntf)

• SG
• NACL
• vpc
• subnets
• vsi
• list of resources

• VSI with multiple network interfaces and subnets

• extend the support of ip-block grouping to all_subnets analysis type as well (see issue #124)

• support grouping of ip-blocks for json format : note that the non-grouped version has more information about the connections than stateful/stateless)

• grouped output issue: external nodes are printed without "Public Internet" prefix

          Open an issue for handling of unidirectional connections under grouping

Originally posted by @zivnevo in #96 (comment)

currently using
VsiLevel / SubnetsLevel / DebugSubnet

• model: e.g. vpc should point to its subnets
  Remaining items:
  (1) [done] keep in VPCConfig an abstract member of type VPCResourceIntf which points to the concrete VPC the config belongs to, and delete vpcsMap
  (2) [done] support with formats: json
  (3) change map from object name to object reference in vpc config to be based on unique ids (see issue #256 )
  (4) decide on JSON format for aggregation - list or map (see #207 (comment) )
  (5) handle drawio output for multiple vpcs , see #207 (comment)

• should add a flag to choose which vpc to analyze

see issue #164

End-to-end examples:

1. Examples in which the grouping differs due to #125 in the content and number of lines, and not only in having the additional '*'; an example for external addresses and one for internal ones. Unit test?
2. Grouped external addresses results in an interval of few cidrs that is presented as ip range in output.

This issue is for brainstorming ideas for checks we may include as part of a linting functionality. At some point we will review the list with a stakeholder and decide which ones make sense.

1. partial connections. When a request can be sent to destination but the response will be blocked. #706 solves
2. nACL splits a subnet. When a cidr that does not cover the entire subnet is used in the source of an outbound rule or the destination of an inbound rule. #698 solves
3. sg split subnet as above. #698 solves
4. LB analysis: warn about SG that split subnet of a configured LB (without running full connectivity analysis)
5. As the above two items, but when there is an actual connectivity gap between two endpoints in the same subnet [define: what is considered an endpoint]
6. overlapping subnet ranges (relevant mostly for the multiple VPCs use case) #700 solves
7. source-port limitation may block response. This will help identify misuse of source-port. Note that this is relevant when the connection is complete. (@kyorav please elaborate)
8. Non-connected tables: #722 solves
   1. SG not connected to an endpoint
   2. NACL not connected to a subnet
9. Non-connected Routers
   1. tgw not connected
   2. fip not connnected
   3. pgw not connected
   4. loadbalancer without pool members
10. Redundant Subnets (no connected endpoints)
11. Redundant VPC....
12. Redundant rules:
    1. SG/NACL rule that is useless w.r.t. the relevant address prefixes #115 solved by #731
    2. SG rule that is implied by other SG rules in the same table
    3. NACL rule that is implied by other, higher priories, NACL rules in the same table
    4. SG/NACL rules that do not actually effect the connectivity
13. Routing Tables analysis: warn about non deterministic routing
14. NACL rules with values in src/dst min/max port but no value in protocol - in which case the ports are ignored (discuss)

see comments from:

#125 (comment)

few options to consider:

• the info message should / should not be always added to the output
• the info message should be added once, not from various code functions

current state: info msg is "connections are stateful unless marked with *", and is always added to textual output (txt,md)

• #292
• #294
• Avoid involvement in unit tests expected output ?

Make sure that the info message will be added once, not from various code functions

consider also the comment from another PR : #207 (comment)
The report of aggregated VPCs should contain this line only once rather than after every VPC report.

consider any resources with impact on connectivity results

• implicit routing table
• egress routing table (per subnet)
• service network

• multiple vpcs + TGW to be handled at issue #164

Add as a new command line query , input: src / dst (endpoint/subnet/) , optionally protocol and port to explore
output: explain which resources (SG/NACL) are relevant for connectivity blocked/allowed , and which rules in the resource.

if blocked: which NACLs is blocking (and if there is deny rule / deny by default) , and which SGs can allow (if exist)
if allowed: which NACLs + SGs allow (and which rules)

phase2: gateways / LBs / PGW (+ ip-address as src/dst ) / TGW

• Currently a Node interface has a Cidr(). If we assume that a node is always a single address then we should change this to Address().
• Make ExternalNetwork an abstract type
• Add uid to all abstractions
• consider renaming VPCConfig to something like CloudConfig, because it may contain several VPCs and the gateways between them

example:

protocol: TCP src-ports: 1-442,444-65535 dst-ports: 443; protocol: TCP src-ports: 443

should be simplified to

protocol: TCP dst-ports: 443; protocol: TCP src-ports: 443

explore other examples to generalize the problem

• define resources for demo
• add collector?
• textual output
• visual output
• add details - what is not handled yet , what is in the agenda

• support more than one dimension
• support more protocols (icmp)
• consider connectivity dimensions: protocol, src-ports, dst-ports, icmp type and code

Initial version assumes the following:

• no SGs are considered, thus connectivity analysis considers only the nodes' subnets and their NACLs. (and also PGW attached to subnets).

supporting both all_endpoints and all_subnets analysis types.

The attached config_object causes the tool to crash when invoked as follows:
vpcanalyzer -vpc-config config_object.json -analysis-type all_endpoints -format arch_drawio
crashes_drawio_config_object.txt

SG: Rules are stateful. To have stateful rules means when you create a rule that allows traffic one way, the rule automatically permits reverse traffic. For example, if you create a rule to allow inbound TCP traffic on port 80, the rule also allows replying outbound TCP traffic on port 80 back to the originating host, without the need for another rule.

NACL: An Access Control List (ACL) can manage (that is, it can allow or deny) inbound and outbound traffic for a subnet. An ACL is stateless, which means that inbound and outbound rules must be specified separately and explicitly. Each ACL consists of rules, based on a source IP, source port, destination IP, destination port, and protocol.

In the connectivity output, a connection A -> B is stateful (allows bidrectional flow) if both SG and NACL (of A and B) allow connection (ingress and egress) from A to B , AND if NACL (of A and B) allow connection (ingress and egress) from B to A .

should be lower case

Currently parser generates the ResourcesContainer from the input config object.
Now the collector supports the creation of this object, thus should use the collector's object instead.

• Add a "not-yet supported" section
• Explain about each one of the formats