diff --git a/pkg/netpol/connlist/connlist.go b/pkg/netpol/connlist/connlist.go index 151f4ecf..e4565098 100644 --- a/pkg/netpol/connlist/connlist.go +++ b/pkg/netpol/connlist/connlist.go @@ -347,9 +347,9 @@ func (c *connection) ProtocolsAndPorts() map[v1.Protocol][]common.PortRange { // returns a *common.ConnectionSet from Peer2PeerConnection data func GetConnectionSetFromP2PConnection(c Peer2PeerConnection) *common.ConnectionSet { protocolsToPortSetMap := make(map[v1.Protocol]*common.PortSet, len(c.ProtocolsAndPorts())) - for protocol, portRageArr := range c.ProtocolsAndPorts() { + for protocol, portRangeArr := range c.ProtocolsAndPorts() { protocolsToPortSetMap[protocol] = common.MakePortSet(false) - for _, p := range portRageArr { + for _, p := range portRangeArr { protocolsToPortSetMap[protocol].AddPortRange(p.Start(), p.End()) } } diff --git a/pkg/netpol/connlist/connlist_test.go b/pkg/netpol/connlist/connlist_test.go index bcd62b6d..0bff0ecb 100644 --- a/pkg/netpol/connlist/connlist_test.go +++ b/pkg/netpol/connlist/connlist_test.go @@ -1124,4 +1124,19 @@ var goodPathTests = []struct { exposureAnalysis: true, outputFormats: []string{output.DefaultFormat}, }, + { + // the netpol allows connection to pod-a on a named port "newport" with "protocol UDP", + // but since the configuration of "pod-a" contains a port with same name but a different protocol, + // i.e. there is no matching named port in the pod's configuration; then the output does not contain + // a connection from new-pod to pod-a + testDirName: "netpol_named_port_test", + outputFormats: []string{output.DefaultFormat}, + }, + { + // the netpol allows connection to "pod-b" on multiple named-ports; + // only some of the ports have a matching named-port + protocol in the pod's configuration + // so we see only the successfully converted ports in the connlist output + testDirName: "netpol_named_port_test_2", + outputFormats: []string{output.DefaultFormat}, + }, } diff --git a/pkg/netpol/connlist/internal/ingressanalyzer/ingress_analyzer.go b/pkg/netpol/connlist/internal/ingressanalyzer/ingress_analyzer.go index b91da61b..765d7245 100644 --- a/pkg/netpol/connlist/internal/ingressanalyzer/ingress_analyzer.go +++ b/pkg/netpol/connlist/internal/ingressanalyzer/ingress_analyzer.go @@ -360,11 +360,12 @@ func (ia *IngressAnalyzer) getIngressPeerConnection(peer eval.Peer, actualServic for _, peerPortToFind := range peerPortsToFind { portNum := peerPortToFind.IntValue() if peerPortToFind.StrVal != "" { // if the port we are searching for is namedPort - portInt, err := ia.pe.ConvertPeerNamedPort(peerPortToFind.StrVal, peer) + protocol, portInt, err := ia.pe.ConvertPeerNamedPort(peerPortToFind.StrVal, peer) if err != nil { return nil, err } - if portInt < 0 { // no matching port for the given named port + // only TCP ports are acceptable for Ingress resource + if protocol != string(corev1.ProtocolTCP) || portInt < 0 { // no matching port for the given named port continue } portNum = int(portInt) diff --git a/pkg/netpol/eval/internal/k8s/netpol.go b/pkg/netpol/eval/internal/k8s/netpol.go index 9ed14609..331a67bd 100644 --- a/pkg/netpol/eval/internal/k8s/netpol.go +++ b/pkg/netpol/eval/internal/k8s/netpol.go @@ -72,34 +72,41 @@ func getProtocolStr(p *v1.Protocol) string { return string(*p) } -func (np *NetworkPolicy) convertNamedPort(namedPort string, pod *Pod) int32 { - return pod.ConvertPodNamedPort(namedPort) -} - -// getPortsRange returns the start and end port numbers given input port, endPort and dest peer -// and the portName if it is a named port -// if input port is a named port, and the dst peer is nil or does not have a matching named port defined, return -// an empty range represented by (-1,-1) -func (np *NetworkPolicy) getPortsRange(port *intstr.IntOrString, endPort *int32, dst Peer) (startNum, endNum int32, - namedPort string, err error) { - var start, end int32 - portName := "" - if port.Type == intstr.String { +// getPortsRange given a rule port and dest peer, returns: the start and end port numbers, +// or the port name if it is a named port +// if input port is a named port, and the dst peer is nil or does not have a matching named port defined, returns +// an empty range represented by (-1,-1) with the named port string +func (np *NetworkPolicy) getPortsRange(rulePort netv1.NetworkPolicyPort, dst Peer) (start, end int32, + portName string, err error) { + if rulePort.Port.Type == intstr.String { // rule.Port is namedPort + ruleProtocol := getProtocolStr(rulePort.Protocol) + portName = rulePort.Port.StrVal if dst == nil { - return common.NoPort, common.NoPort, port.StrVal, nil + // dst is nil, so the namedPort can not be converted, return string port name + return common.NoPort, common.NoPort, portName, nil } if dst.PeerType() != PodType { + // namedPort is not defined for IP-blocks return start, end, "", np.netpolErr(netpolerrors.NamedPortErrTitle, netpolerrors.ConvertNamedPortErrStr) } - portName = port.StrVal - portNum := np.convertNamedPort(portName, dst.GetPeerPod()) - start = portNum - end = portNum - } else { - start = port.IntVal + podProtocol, podPortNum := dst.GetPeerPod().ConvertPodNamedPort(portName) + if podProtocol == "" && podPortNum == common.NoPort { + // there is no match for the namedPort in the configuration of the pod, return the portName string as "ports range" + return common.NoPort, common.NoPort, portName, nil + } + if podProtocol != ruleProtocol { + // the pod has a matching namedPort, but not on same protocol as the rule's protocol; so it can not be converted, + // return the string named-port as the "ports range" + return common.NoPort, common.NoPort, portName, nil + } + // else, found match for the rule's named-port in the pod's ports, so it may be converted to port number + start = podPortNum + end = podPortNum + } else { // rule.Port is number + start = rulePort.Port.IntVal end = start - if endPort != nil { - end = *endPort + if rulePort.EndPort != nil { + end = *rulePort.EndPort } } return start, end, portName, nil @@ -124,17 +131,25 @@ func (np *NetworkPolicy) ruleConnections(rulePorts []netv1.NetworkPolicyPort, ds if rulePorts[i].Port == nil { ports = common.MakePortSet(true) } else { - startPort, endPort, portName, err := np.getPortsRange(rulePorts[i].Port, rulePorts[i].EndPort, dst) + startPort, endPort, portName, err := np.getPortsRange(rulePorts[i], dst) if err != nil { return res, err } - if (dst == nil || isPeerRepresentative(dst)) && portName != "" { + // valid returned values from `getsPortsRange` : + // 1. empty-numbered-range with the string namedPort, if the rule has a named-port which is not (or cannot be) converted to a + // numbered-range by the dst's ports. + // 2. non empty-range when the rule ports are numbered or the named-port was converted + if (dst == nil || isPeerRepresentative(dst)) && isEmptyPortRange(startPort, endPort) && portName != "" { // this func may be called: - // - for computing cluster-wide exposure of the policy (dst is nil); - // - in-order to get a connection from a real workload to a representative dst. - // - in order to get a connection from any pod to a real dst or an ip dst (will not get to this if) + // 1- for computing cluster-wide exposure of the policy (dst is nil); + // 2- in-order to get a connection from a real workload to a representative dst. // in both first cases, we can't convert the named port to its number, like when dst peer is a real // pod from manifests, so we use the named-port as is. + // 3- in order to get a connection from any pod to a real dst. + // in the third case the namedPort of the policy rule may not have a match in the Pod's configuration, + // so it will be ignored (the pod has no matching named-port in its configuration - unknown connection is not allowed) + // 4- in order to get a connection from any pod to an ip dst (will not get here, as named ports are not defined for ip-blocks) + // adding portName string to the portSet ports.AddPort(intstr.FromString(portName)) } @@ -167,7 +182,7 @@ func (np *NetworkPolicy) ruleConnsContain(rulePorts []netv1.NetworkPolicyPort, p if rulePorts[i].Port == nil { // If this field is not provided, this matches all port names and numbers. return true, nil } - startPort, endPort, _, err := np.getPortsRange(rulePorts[i].Port, rulePorts[i].EndPort, dst) + startPort, endPort, _, err := np.getPortsRange(rulePorts[i], dst) if err != nil { return false, err } diff --git a/pkg/netpol/eval/internal/k8s/netpol_test.go b/pkg/netpol/eval/internal/k8s/netpol_test.go index 69a28b8f..e36c0671 100644 --- a/pkg/netpol/eval/internal/k8s/netpol_test.go +++ b/pkg/netpol/eval/internal/k8s/netpol_test.go @@ -87,7 +87,8 @@ var ( func TestNetworkPolicyPortAnalysis(t *testing.T) { // tested function: func ruleConnections(rulePorts []netv1.NetworkPolicyPort, dst Peer) ConnectionSet dst := PodPeer{Pod: &Pod{Name: "A", Namespace: "default"}} - dst.Pod.Ports = []v1.ContainerPort{{Name: PortHello.StrVal, ContainerPort: 22}} + dst.Pod.Ports = []v1.ContainerPort{{Name: PortHello.StrVal, ContainerPort: 22, Protocol: "UDP"}} + // default protocol for containerPort is TCP, if the Protocol is not defined will get a mismatch var AllowNamedPortOnProtocol = netv1.NetworkPolicyPort{ Protocol: &UDP, Port: &PortHello, diff --git a/pkg/netpol/eval/internal/k8s/pod.go b/pkg/netpol/eval/internal/k8s/pod.go index 7ba4a3d5..d84d93e0 100644 --- a/pkg/netpol/eval/internal/k8s/pod.go +++ b/pkg/netpol/eval/internal/k8s/pod.go @@ -277,16 +277,20 @@ func (pod *Pod) PodExposedTCPConnections() *common.ConnectionSet { return res } -// ConvertPodNamedPort returns the ContainerPort number that matches the named port -// if there is no match, returns -1 +// ConvertPodNamedPort returns the ContainerPort's protocol and number that matches the named port +// if there is no match, returns empty string for protocol and -1 for number // namedPort is unique within the pod -func (pod *Pod) ConvertPodNamedPort(namedPort string) int32 { +func (pod *Pod) ConvertPodNamedPort(namedPort string) (protocol string, portNum int32) { for _, containerPort := range pod.Ports { - if namedPort == containerPort.Name { - return containerPort.ContainerPort + if namedPort == containerPort.Name { // found + if containerPort.Protocol == "" { + // found the named port with unspecified protocol this means "TCP" protocol (default) + return string(corev1.ProtocolTCP), containerPort.ContainerPort + } + return string(containerPort.Protocol), containerPort.ContainerPort } } - return common.NoPort + return "", common.NoPort } // updatePodXgressExposureToEntireClusterData updates the pods' fields which are related to entire class exposure on ingress/egress @@ -313,11 +317,13 @@ func (pod *Pod) checkAndConvertNamedPortsInConnection(conns *common.ConnectionSe return nil } // else - found named ports connsCopy := conns.Copy() // copying the connectionSet; in order to replace - // the named ports with pod's port numbers + // the named ports with pod's port numbers if possible for protocol, namedPorts := range connNamedPorts { for _, namedPort := range namedPorts { - portNum := pod.ConvertPodNamedPort(namedPort) - connsCopy.ReplaceNamedPortWithMatchingPortNum(protocol, namedPort, portNum) + podProtocol, portNum := pod.ConvertPodNamedPort(namedPort) + if podProtocol == string(protocol) && portNum != common.NoPort { // matching port and protocol + connsCopy.ReplaceNamedPortWithMatchingPortNum(protocol, namedPort, portNum) + } } } return connsCopy diff --git a/pkg/netpol/eval/resources.go b/pkg/netpol/eval/resources.go index d7141bae..f0cfcd57 100644 --- a/pkg/netpol/eval/resources.go +++ b/pkg/netpol/eval/resources.go @@ -579,14 +579,16 @@ func (pe *PolicyEngine) GetSelectedPeers(selectors labels.Selector, namespace st // ConvertPeerNamedPort returns the peer.pod.containerPort matching the named port of the peer // if there is no match for the input named port, return -1 -func (pe *PolicyEngine) ConvertPeerNamedPort(namedPort string, peer Peer) (int32, error) { +func (pe *PolicyEngine) ConvertPeerNamedPort(namedPort string, peer Peer) (protocol string, portNum int32, err error) { switch currentPeer := peer.(type) { case *k8s.WorkloadPeer: - return currentPeer.Pod.ConvertPodNamedPort(namedPort), nil + protocol, portNum := currentPeer.Pod.ConvertPodNamedPort(namedPort) + return protocol, portNum, nil case *k8s.PodPeer: - return currentPeer.Pod.ConvertPodNamedPort(namedPort), nil + protocol, portNum := currentPeer.Pod.ConvertPodNamedPort(namedPort) + return protocol, portNum, nil default: - return 0, errors.New("peer type does not have ports") // should not get here + return "", 0, errors.New("peer type does not have ports") // should not get here } } diff --git a/test_outputs/connlist/netpol_named_port_test_2_connlist_output.txt b/test_outputs/connlist/netpol_named_port_test_2_connlist_output.txt new file mode 100644 index 00000000..3cebb7b3 --- /dev/null +++ b/test_outputs/connlist/netpol_named_port_test_2_connlist_output.txt @@ -0,0 +1,3 @@ +0.0.0.0-255.255.255.255 => helloworld/pod-a[Deployment] : All Connections +helloworld/pod-a[Deployment] => 0.0.0.0-255.255.255.255 : All Connections +helloworld/pod-a[Deployment] => helloworld/new-pod[Deployment] : SCTP 8956,9090 \ No newline at end of file diff --git a/test_outputs/connlist/netpol_named_port_test_connlist_output.txt b/test_outputs/connlist/netpol_named_port_test_connlist_output.txt new file mode 100644 index 00000000..3757be36 --- /dev/null +++ b/test_outputs/connlist/netpol_named_port_test_connlist_output.txt @@ -0,0 +1,2 @@ +0.0.0.0-255.255.255.255 => helloworld/new-pod[Deployment] : All Connections +helloworld/new-pod[Deployment] => 0.0.0.0-255.255.255.255 : All Connections \ No newline at end of file diff --git a/tests/netpol_named_port_test/netpol.yaml b/tests/netpol_named_port_test/netpol.yaml new file mode 100644 index 00000000..ee2f0f5f --- /dev/null +++ b/tests/netpol_named_port_test/netpol.yaml @@ -0,0 +1,18 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: enable-ingress-from-named-port + namespace: helloworld +spec: + podSelector: + matchLabels: + app: app-a + policyTypes: + - Ingress + - Egress + ingress: + - from: + - namespaceSelector: {} + ports: + - port: newport # this port with its protocol has no match in the pod configuration + protocol: UDP diff --git a/tests/netpol_named_port_test/pods.yaml b/tests/netpol_named_port_test/pods.yaml new file mode 100644 index 00000000..619bafed --- /dev/null +++ b/tests/netpol_named_port_test/pods.yaml @@ -0,0 +1,59 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: helloworld +spec: {} +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: pod-a + namespace: helloworld + labels: + app: app-a +spec: + selector: + matchLabels: + app: app-a + template: + metadata: + labels: + app: app-a + spec: + containers: + - name: helloworld + image: quay.io/shfa/app-a:latest + ports: + - name: newport + containerPort: 8956 + protocol: SCTP + - name: udps + containerport: 3535 + protocol: UDP +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: new-pod + namespace: helloworld + labels: + app: app-b +spec: + selector: + matchLabels: + app: app-b + template: + metadata: + labels: + app: app-b + spec: + containers: + - name: helloworld + image: quay.io/shfa/ingress-world:latest + ports: + - containerPort: 8956 # containerport1 + protocol: SCTP + - containerPort: 8050 # containerport2 + - containerPort: 8090 # containerport3 +--- \ No newline at end of file diff --git a/tests/netpol_named_port_test_2/netpol.yaml b/tests/netpol_named_port_test_2/netpol.yaml new file mode 100644 index 00000000..e91364cf --- /dev/null +++ b/tests/netpol_named_port_test_2/netpol.yaml @@ -0,0 +1,24 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: enable-ingress-from-named-port + namespace: helloworld +spec: + podSelector: + matchLabels: + app: app-b + policyTypes: + - Ingress + - Egress + ingress: + - from: + - namespaceSelector: {} + ports: + - port: newport # this port with its protocol has no match in the pod configuration + protocol: UDP + - port: sctp-port # its matching port number in the pod configuration is 8956 + protocol: SCTP + - port: newport # its matching port number in the pod configuration is 9090 + protocol: SCTP + - port: not-found # this port with its protocol has no match in the pod configuration + protocol: SCTP diff --git a/tests/netpol_named_port_test_2/pods.yaml b/tests/netpol_named_port_test_2/pods.yaml new file mode 100644 index 00000000..a9978a1d --- /dev/null +++ b/tests/netpol_named_port_test_2/pods.yaml @@ -0,0 +1,63 @@ +--- +apiVersion: v1 +kind: Namespace +metadata: + name: helloworld +spec: {} +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: pod-a + namespace: helloworld + labels: + app: app-a +spec: + selector: + matchLabels: + app: app-a + template: + metadata: + labels: + app: app-a + spec: + containers: + - name: helloworld + image: quay.io/shfa/app-a:latest + ports: + - name: newport + containerPort: 8956 + protocol: SCTP + - name: udps + containerport: 3535 + protocol: UDP +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: new-pod + namespace: helloworld + labels: + app: app-b +spec: + selector: + matchLabels: + app: app-b + template: + metadata: + labels: + app: app-b + spec: + containers: + - name: helloworld + image: quay.io/shfa/ingress-world:latest + ports: + - name: sctp-port + containerPort: 8956 + protocol: SCTP + - name: newport + containerPort: 9090 + protocol: SCTP + - containerPort: 8050 + - containerPort: 8090 +--- \ No newline at end of file