analysis with specific ingress controllers

docs/connlist_output.md

@@ -10,10 +10,7 @@ The connectivity output consists of lines of the form: `src` => `dst` : `connect
 
 For connections inferred from network policy resources only, the `src` and `dst` are workloads or external IP-blocks.
 
-For Ingress/Route analysis, the `src` is specified as `{ingress-controller}`, representing the cluster's ingress controller Pod.
-Its connectivity lines are of the form: `{ingress-controller}` => `dst` : `connections`, where `dst` is a workload in the cluster.
-This analysis is currently activated only with `--dir-path` flag, and not on a live cluster.
-It assumes that the ingress controller Pod is unknown, and thus using this notation of `{ingress-controller}`.
+Details on Ingress/Route analysis are specified [here](docs/ingress_analysis.md).
 
 ## Example Output
 
@@ -95,9 +92,3 @@ $ dot -Tsvg test_outputs/connlist/netpol-analysis-example-minimal_connlist_outpu
 The frames in the graph represent namespaces of the analyzed cluster.
 
 ![svg graph](./connlist_example_svg.svg)
-
-
-### Possible warning
-`Route/Ingress specified workload as a backend, but network policies are blocking ingress connections from an arbitrary in-cluster source to this workload. Connectivity map will not include a possibly allowed connection between the ingress controller and this workload.`
-
-Since the analysis assumes the manifest of the ingress controller is unknown, it checks whether an arbitrary workload can access the destination workloads specified in Ingress/Route rules. If such access is not permitted by network policies, this connection is removed from the report. It may be an allowed connection if a network policy specifically allows ingress access to that workload from a specific workload/namespace of the actual ingress controller installed.

docs/ingress_analysis.md

@@ -0,0 +1,64 @@
+# Ingress Analysis
+
+Ingress Analysis is performed when input resources include Ingress/Route objects.
+
+For workloads that are selected by Ingress/Route objects, we analyze if ingress access from ingress-controllers is allowed to them.
+
+## Supported Ingress Controllers:
+
+Ingress Controllers considered for ingress analysis:
+- Openshift ingress controller ([view official docs](https://docs.openshift.com/container-platform/4.14/networking/ingress-operator.html#nw-ingress-view_configuring-ingress))
+- Nginx ingress controller ([view official docs](https://docs.nginx.com/nginx-ingress-controller/overview))
+
+#### Namespace Labels Checked :
+
+Ingress access to a workload selected by Ingress/Route object may be restricted by network-policies rules.
+In order to check if a network-policy rule enables access to such workload we need to check if a rule's namespaceSelector matches any of supported ingress controllers' namespaces.
+if at least one label of the following is used in the namespaceSelector, so ingress access from the relevant Ingress-Controller is enabled.
+
+|Ingress Controller | Namespace labels supported| More Details |
+|-------------------|---------------------------|--------------|
+|Openshift ingress controller |namespace.name: openshift-ingress-operator|[openshift-ingress-operator namespace link](https://github.com/openshift/cluster-ingress-operator/blob/f9dd81ab522f72233e2608f5e57a43e79a5079b5/manifests/00-namespace.yaml#L10)|
+||kubernetes.io/metadata.name: openshift-ingress-operator||
+||openshift.io/cluster-monitoring: "true"||
+|Openshift ingress controller |namespace.name: openshift-ingress|[openshift-ingress-operator namespace link](https://github.com/openshift/cluster-ingress-operator/blob/f9dd81ab522f72233e2608f5e57a43e79a5079b5/pkg/manifests/assets/router/namespace.yaml#L13)|
+||kubernetes.io/metadata.name: openshift-ingress||
+||openshift.io/cluster-monitoring: "true"||
+||network.openshift.io/policy-group: ingress||
+||policy-group.network.openshift.io/ingress: ""||
+||pod-security.kubernetes.io/enforce: privileged||
+||pod-security.kubernetes.io/audit: privileged||
+||pod-security.kubernetes.io/warn: privileged||
+|Nginx ingress controller |kubernetes.io/metadata.name: nginx-ingress|[nginx-ingress namespace link](https://github.com/nginxinc/kubernetes-ingress/blob/main/deployments/common/ns-and-sa.yaml)|
+
+
+## Allowed Ingress Connections And Output:
+
+- Selecting ingress-controller deployment by `podSelector` in a network-policy rule is not supported yet. 
+If ingress connections from a specific ingress-controller namespace (from above) is allowed to a cluster's peer, we assume it is allowed from any ingress-controller-pod in that namespace.
+
+- For a supported ingress-controller, the connectivity line is of the form:
+
+    `{<ingress-controller-namespace-name>}` => `dst` : `connections`, where `dst` is a workload in the cluster.
+
+- If an ingress-controller may belong to more than one namespace, and they all were captured by the policies, only one of them will appear in the output.
+
+
+- If external ingress to a workload is allowed from any ingress-controller or at least all supported ingress-controllers, then the connectivity line is of the form:
+
+    `{ingress-controller}` => `dst` : `connections`, where `dst` is a workload in the cluster.
+
+- When ingress connection is enabled by Ingress/Route, but restricted by a network-policy rule to be allowed from only one of the supported controllers, then only the connectivity line from the specified ingress-controller namespace will be reported.
+
+- If ingress connection is enabled by Ingress/Route but denied by network-policy rules, then ingress connection will be blocked and will not appear in the connectivity report.
+
+- If ingress connection is enabled by Ingress/Route but restricted by network-policy rules to another ingress-controller (that is not supported) then ingress connection will not appear in the connectivity report either.
+
+- This analysis is currently activated only with `--dirpath` flag, and not on a live cluster.
+It assumes that the ingress controller Pod is unknown, and thus using this notation of its namespace.
+
+
+### Possible warning
+`Route/Ingress specified workload as a backend, but network policies are blocking external ingress access by nginx or openshift ingress controllers. Connectivity map will not include a possibly allowed connection between the ingress controller and this workload.`
+
+Since the analysis considers only the mentioned above specific ingress controllers, it checks whether an arbitrary workload in one of the specified namespaces can access the destination workloads specified in Ingress/Route rules. If such access is not permitted by network policies, this connection is removed from the report. It may be an allowed connection if a network policy specifically allows ingress access to that workload from a specific workload/namespace of an actual different ingress controller installed.

pkg/internal/netpolerrors/netpol_errors.go

@@ -92,7 +92,7 @@ func InvalidPeerErrStr(peer string) string {
 // BlockedIngressWarning returns warning string of a blocked ingress on peer
 func BlockedIngressWarning(objKind, objName, peerStr string) string {
 	return objKind + " resource " + objName + " specified workload " + peerStr + " as a backend, but network policies are blocking " +
-		"ingress connections from an arbitrary in-cluster source to this workload. " +
+		"ingress access by nginx or openshift ingress controllers." +
 		"Connectivity map will not include a possibly allowed connection between the ingress controller and this workload."
 }
 

pkg/netpol/connlist/connlist.go

@@ -16,6 +16,7 @@ package connlist
 import (
 	"context"
 	"errors"
+	"strings"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -341,6 +342,10 @@ func (ca *ConnlistAnalyzer) includePairOfWorkloads(src, dst eval.Peer) bool {
 	if ca.focusWorkload == "" {
 		return true
 	}
+	// accept any ingress controller source as focus-workload: ingress-controller
+	if ca.focusWorkload == common.GeneralIngressControllerName && strings.Contains(src.Name(), common.GeneralIngressControllerName) {
+		return true
+	}
 	// at least one of src/dst should be the focus workload
 	return ca.isPeerFocusWorkload(src) || ca.isPeerFocusWorkload(dst)
 }
@@ -414,7 +419,7 @@ func (ca *ConnlistAnalyzer) getConnectionsList(pe *eval.PolicyEngine, ia *ingres
 // or if it exists in the peers list from the parsed resources
 // if not returns a suitable warning message
 func (ca *ConnlistAnalyzer) existsFocusWorkload(peers []Peer, excludeIngressAnalysis bool) (existFocusWorkload bool, warning string) {
-	if ca.focusWorkload == common.IngressPodName {
+	if strings.Contains(ca.focusWorkload, common.GeneralIngressControllerName) {
 		if excludeIngressAnalysis { // if the ingress-analyzer is empty,
 			// then no routes/k8s-ingress objects -> ingrss-controller pod will not be added
 			return false, netpolerrors.NoIngressSourcesErrStr + netpolerrors.EmptyConnListErrStr
@@ -466,38 +471,59 @@ func (ca *ConnlistAnalyzer) getConnectionsBetweenPeers(pe *eval.PolicyEngine, pe
 func (ca *ConnlistAnalyzer) getIngressAllowedConnections(ia *ingressanalyzer.IngressAnalyzer,
 	pe *eval.PolicyEngine) ([]Peer2PeerConnection, error) {
 	res := make([]Peer2PeerConnection, 0)
+	// ingress connections enabled by Ingress/Route objects
 	ingressConns, err := ia.AllowedIngressConnections()
 	if err != nil {
 		return nil, err
 	}
-	// adding the ingress controller pod to the policy engine,
-	ingressControllerPod, err := pe.AddPodByNameAndNamespace(common.IngressPodName, common.IngressPodNamespace)
+	// adding specific known controller pods to the policy engine
+	specificIngressControllers, err := addSpecificIngressControllers(pe)
 	if err != nil {
 		return nil, err
 	}
+	// 3. computing allowed connections by Ingress controllers for peers that are selected by Ingress/Route objects
 	for peerStr, peerAndConn := range ingressConns {
-		// refines to only relevant connections if ca.focusWorkload is not empty
-		if !ca.includePairOfWorkloads(ingressControllerPod, peerAndConn.Peer) {
-			continue
-		}
-		// compute allowed connections based on pe.policies to the peer, then intersect the conns with
-		// ingress connections to the peer -> the intersection will be appended to the result
-		peConn, err := pe.AllAllowedConnectionsBetweenWorkloadPeers(ingressControllerPod, peerAndConn.Peer)
-		if err != nil {
-			return nil, err
+		// check if ingress is allowed from a specific known ingress controller
+		ingressConnsSet := map[string]Peer2PeerConnection{} // set of the ingress controllers conns allowed
+		for _, ingPod := range specificIngressControllers {
+			ingConn, err := ca.allowedIngressControllerToPeerFromPoliciesRules(ingPod, peerAndConn.Peer, pe)
+			if err != nil {
+				return nil, err
+			}
+			if ingConn == nil {
+				continue
+			}
+			ingConn.Intersection(peerAndConn.ConnSet)
+			if !ingConn.IsEmpty() {
+				p2pConnection := &connection{
+					src:               ingPod,
+					dst:               peerAndConn.Peer,
+					allConnections:    ingConn.AllConnections(),
+					protocolsAndPorts: ingConn.ProtocolsAndPortsMap(),
+				}
+				if _, ok := ingressConnsSet[ingPod.Name()]; !ok {
+					// to avoid having conns from same controller (with different captured namespaces)
+					ingressConnsSet[ingPod.Name()] = p2pConnection
+				}
+			}
 		}
-		peerAndConn.ConnSet.Intersection(peConn)
-		if peerAndConn.ConnSet.IsEmpty() {
+		// append the ingress controllers connections to res
+		switch len(ingressConnsSet) {
+		case 0: // specific ingress controller conn to this peer is blocked by network-policies
 			ca.warnBlockedIngress(peerStr, peerAndConn.IngressObjects)
-			continue
-		}
-		p2pConnection := &connection{
-			src:               ingressControllerPod,
-			dst:               peerAndConn.Peer,
-			allConnections:    peerAndConn.ConnSet.AllConnections(),
-			protocolsAndPorts: peerAndConn.ConnSet.ProtocolsAndPortsMap(),
+		case len(common.SpecificIngressControllerToItsNamespaces): // connections from any ingress controller is allowed -
+			// append to res a general conn
+			sampleConn := ingressConnsSet[common.NginxIngressControllerName]
+			generalConn, err := addGeneralConnectionBasedOnSampleOne(sampleConn, pe)
+			if err != nil {
+				return nil, err
+			}
+			res = append(res, generalConn)
+		default: // network policies allow ingress controllers' connections from only some (1/2) of the supported ones
+			for _, v := range ingressConnsSet {
+				res = append(res, v)
+			}
 		}
-		res = append(res, p2pConnection)
 	}
 	return res, nil
 }
@@ -522,3 +548,47 @@ func (ca *ConnlistAnalyzer) logWarning(msg string) {
 		ca.logger.Warnf(msg)
 	}
 }
+
+// helping func - adds new fake pods in ingress controllers namespaces to the policy engine
+func addSpecificIngressControllers(pe *eval.PolicyEngine) ([]Peer, error) {
+	res := []Peer{}
+
+	for controllerName, nsInfoList := range common.SpecificIngressControllerToItsNamespaces {
+		for _, ns := range nsInfoList {
+			ingPod, err := pe.AddPodByNameAndNamespace(controllerName, ns.ControllerNamespaceName, ns.ControllerNamespaceLabels)
+			if err != nil {
+				return nil, err
+			}
+			res = append(res, ingPod)
+		}
+	}
+	return res, nil
+}
+
+func (ca *ConnlistAnalyzer) allowedIngressControllerToPeerFromPoliciesRules(ingressPeer, dst Peer,
+	pe *eval.PolicyEngine) (*common.ConnectionSet, error) {
+	// refines to only relevant connections if ca.focusWorkload is not empty
+	if !ca.includePairOfWorkloads(ingressPeer, dst) {
+		return nil, nil
+	}
+	peConn, err := pe.AllAllowedConnectionsBetweenWorkloadPeers(ingressPeer, dst)
+	if err != nil {
+		return nil, err
+	}
+	return peConn, nil
+}
+
+// gets sample ingress controller connection and copies it to a general connection to be allowed from any ingress controller
+func addGeneralConnectionBasedOnSampleOne(sampleConn Peer2PeerConnection, pe *eval.PolicyEngine) (Peer2PeerConnection, error) {
+	generalIngController, err := pe.AddPodByNameAndNamespace(common.GeneralIngressControllerName, common.GeneralIngressControllerName, nil)
+	if err != nil {
+		return nil, err
+	}
+	generalConn := &connection{
+		src:               generalIngController,
+		dst:               sampleConn.Dst(),
+		allConnections:    sampleConn.AllProtocolsAndPorts(),
+		protocolsAndPorts: sampleConn.ProtocolsAndPorts(),
+	}
+	return generalConn, nil
+}

pkg/netpol/connlist/connlist_test.go

@@ -709,6 +709,18 @@ var goodPathTests = []struct {
 		testDirName:   "semanticDiff-different-topologies-policy-b-with-ipblock",
 		outputFormats: []string{output.TextFormat},
 	},
+	{
+		testDirName:   "example_with_specific_ingress",
+		outputFormats: []string{output.TextFormat},
+	},
+	{
+		testDirName:   "example_policy_blocking_ingress",
+		outputFormats: []string{output.TextFormat},
+	},
+	{
+		testDirName:   "example-ingress-controller-with-labels",
+		outputFormats: []string{output.TextFormat},
+	},
 	{
 		testDirName:   "test_with_named_ports_changed_netpol_2",
 		outputFormats: []string{output.TextFormat},

pkg/netpol/connlist/conns_formatter_dot.go

@@ -34,7 +34,7 @@ func getEdgeLine(c Peer2PeerConnection) string {
 func peerNameAndColorByType(peer Peer) (nameLabel, color string, isExternal bool) {
 	if peer.IsPeerIPType() {
 		return peer.String(), ipColor, true
-	} else if peer.Name() == common.IngressPodName {
+	} else if strings.Contains(peer.String(), "{") {
 		return peer.String(), nonIPPeerColor, true
 	}
 	return dotformatting.NodeClusterPeerLabel(peer.Name(), peer.Kind()), nonIPPeerColor, false

pkg/netpol/diff/diff.go

@@ -15,6 +15,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"strings"
 
 	v1 "k8s.io/api/core/v1"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -481,7 +482,7 @@ func (c *connsPair) isSrcOrDstPeerIPType(checkSrc bool) bool {
 }
 
 func isIngressControllerPeer(peer eval.Peer) bool {
-	return peer.Name() == common.IngressPodName
+	return strings.Contains(peer.Name(), common.GeneralIngressControllerName)
 }
 
 // updateNewOrLostFields updates ConnsPair's newOrLostSrc and newOrLostDst values

pkg/netpol/diff/diff_formatter_dot.go

@@ -11,7 +11,6 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/np-guard/netpol-analyzer/pkg/netpol/internal/common"
 	"github.com/np-guard/netpol-analyzer/pkg/netpol/internal/dotformatting"
 )
 
@@ -271,7 +270,7 @@ func addNodeKeyLines() []string { // const
 
 // returns the peer name should be displayed in the node of the peer in the graph and whether the peer is a cluster peer or not
 func getNodePeerLabelAndType(peer Peer) (string, bool) {
-	if peer.IsPeerIPType() || peer.Name() == common.IngressPodName {
+	if peer.IsPeerIPType() || strings.Contains(peer.String(), "{") {
 		return peer.String(), true
 	}
 	return dotformatting.NodeClusterPeerLabel(peer.Name(), peer.Kind()), false

pkg/netpol/diff/diff_test.go

@@ -622,6 +622,13 @@ var goodPathTests = []struct {
 		secondDirName: "test_with_named_ports_changed_netpol_3",
 		formats:       []string{output.DefaultFormat},
 	},
+	{
+		// description:
+		// **changed netpol: default/ingress-to-apple
+		firstDirName:  "example_policy_blocking_ingress",
+		secondDirName: "example_with_specific_ingress",
+		formats:       []string{output.DefaultFormat},
+	},
 }
 
 var commonBadPathTestsFatalErr = []struct {

pkg/netpol/eval/internal/k8s/peer.go

@@ -76,7 +76,7 @@ func (p *WorkloadPeer) Kind() string {
 
 func (p *WorkloadPeer) String() string {
 	if p.Pod.FakePod {
-		return "{" + p.Pod.Name + "}"
+		return "{" + p.Pod.Namespace + "}"
 	}
 	return types.NamespacedName{Name: p.Name(), Namespace: p.Namespace()}.String() + "[" + p.Kind() + "]"
 }

pkg/netpol/eval/resources.go

@@ -101,7 +101,7 @@ func (pe *PolicyEngine) resolveMissingNamespaces() error {
 	for _, pod := range pe.podsMap {
 		ns := pod.Namespace
 		if _, ok := pe.namspacesMap[ns]; !ok {
-			if err := pe.resolveSingleMissingNamespace(ns); err != nil {
+			if err := pe.resolveSingleMissingNamespace(ns, nil); err != nil {
 				return err
 			}
 		}
@@ -110,13 +110,17 @@ func (pe *PolicyEngine) resolveMissingNamespaces() error {
 }
 
 // resolveSingleMissingNamespace create a ns object and upsert to PolicyEngine
-func (pe *PolicyEngine) resolveSingleMissingNamespace(ns string) error {
+func (pe *PolicyEngine) resolveSingleMissingNamespace(ns string, nsLabels map[string]string) error {
+	nLabels := nsLabels
+	if len(nLabels) == 0 {
+		nLabels = map[string]string{
+			k8s.K8sNsNameLabelKey: ns,
+		}
+	}
 	nsObj := &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: ns,
-			Labels: map[string]string{
-				k8s.K8sNsNameLabelKey: ns,
-			},
+			Name:   ns,
+			Labels: nLabels,
 		},
 	}
 	if err := pe.upsertNamespace(nsObj); err != nil {
@@ -476,14 +480,14 @@ func (pe *PolicyEngine) ConvertPeerNamedPort(namedPort string, peer Peer) (int32
 }
 
 // AddPodByNameAndNamespace adds a new fake pod to the pe.podsMap, used for adding ingress-controller pod
-func (pe *PolicyEngine) AddPodByNameAndNamespace(name, ns string) (Peer, error) {
+func (pe *PolicyEngine) AddPodByNameAndNamespace(name, ns string, nsLabels map[string]string) (Peer, error) {
 	podStr := types.NamespacedName{Namespace: ns, Name: name}.String()
 	newPod := &k8s.Pod{
 		Name:      name,
 		Namespace: ns,
 		FakePod:   true,
 	}
-	if err := pe.resolveSingleMissingNamespace(ns); err != nil {
+	if err := pe.resolveSingleMissingNamespace(ns, nsLabels); err != nil {
 		return nil, err
 	}
 	pe.podsMap[podStr] = newPod