Skip to content

Commit

Permalink
pre proccessing policy for general conns (#306)
Browse files Browse the repository at this point in the history
* first commit: pre proccessing policy for general conns

* func doc

* fixing to PortSet; instead of PortSet{}, call MakePortSet(false) to ensure initializing empty maps for named ports

* fixing handling connections with namedPort

* fixing lint issued by github - not relevant to PR

* tiny fix

* fixes
  • Loading branch information
shireenf-ibm authored Feb 12, 2024
1 parent 9025216 commit db2c484
Show file tree
Hide file tree
Showing 10 changed files with 212 additions and 63 deletions.
3 changes: 2 additions & 1 deletion pkg/netpol/connlist/connlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ func (c *connection) ProtocolsAndPorts() map[v1.Protocol][]common.PortRange {
func GetConnectionSetFromP2PConnection(c Peer2PeerConnection) *common.ConnectionSet {
protocolsToPortSetMap := make(map[v1.Protocol]*common.PortSet, len(c.ProtocolsAndPorts()))
for protocol, portRageArr := range c.ProtocolsAndPorts() {
protocolsToPortSetMap[protocol] = &common.PortSet{}
portSet := common.MakePortSet(false)
protocolsToPortSetMap[protocol] = &portSet
for _, portRange := range portRageArr {
protocolsToPortSetMap[protocol].AddPortRange(portRange.Start(), portRange.End())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ func (ia *IngressAnalyzer) getIngressPeerConnection(peer eval.Peer, actualServic
}

if peerTCPConn.Contains(strconv.Itoa(portNum), string(corev1.ProtocolTCP)) {
permittedPort := common.PortSet{}
permittedPort := common.MakePortSet(false)
permittedPort.AddPort(intstr.FromInt(portNum))
res.AddConnection(corev1.ProtocolTCP, permittedPort)
}
Expand Down
14 changes: 4 additions & 10 deletions pkg/netpol/eval/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,24 +276,18 @@ func (pe *PolicyEngine) allallowedXgressConnections(src, dst k8s.Peer, isIngress
if isIngress {
// policy selecting dst
policyAllowedConnectionsPerDirection, err = policy.GetIngressAllowedConns(src, dst)
if err != nil {
return allowedConns, err
}
if !ingressSet[dst] {
// if this is first time scanning policies selecting this dst peer,
// check and store if it is exposed to entire class on ingress
err = policy.ScanRulesForIngressFromEntireCluster(dst)
// update its ingress entire cluster connection relying on policy data
dst.GetPeerPod().UpdatePodXgressExposureToEntireClusterData(policy.IngressGeneralConns.EntireClusterConns, isIngress)
}
} else {
// policy selecting src
policyAllowedConnectionsPerDirection, err = policy.GetEgressAllowedConns(dst)
if err != nil {
return allowedConns, err
}
if !egressSet[src] {
// if this is first time scanning policies selecting this src peer,
// check and store if it is exposed to entire class on egress
err = policy.ScanRulesForEgressToEntireCluster(src, dst)
// update its egress entire cluster connection relying on policy data
src.GetPeerPod().UpdatePodXgressExposureToEntireClusterData(policy.EgressGeneralConns.EntireClusterConns, isIngress)
}
}
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/netpol/eval/internal/k8s/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func PodNamespace() (string, error) {
return ns, nil
}
if data, err := os.ReadFile(nsFile); err == nil {
if ns := strings.TrimSpace(string(data)); len(ns) > 0 {
if ns := strings.TrimSpace(string(data)); ns != "" {
return ns, nil
}
return "", err
Expand Down
125 changes: 89 additions & 36 deletions pkg/netpol/eval/internal/k8s/netpol.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,32 @@ import (
"github.com/np-guard/netpol-analyzer/pkg/netpol/internal/common"
)

// NetworkPolicy is an alias for k8s network policy object
// @todo is there a preprocessed form of the object that would make more sense?
// @todo is there another preprocessed form of the object that would make more sense?
//
// for example, converting Spec.PodSelector to labels.Selector on initialization
// or preprocessing namespaces and pods that match selector in ingress/egress rules, etc
//
// -> might help to preprocess and store peers that match policy selectors + selectors in rules + set of allowed connections per rule
type NetworkPolicy netv1.NetworkPolicy
type NetworkPolicy struct {
*netv1.NetworkPolicy // embedding k8s network policy object
// following data stored in preprocessing;
// IngressGeneralConns contains:
// - the maximal connection-set which the policy's rules allow to all destinations on ingress direction
// - the maximal connection-set which the policy's rules allow to all namespaces in the cluster on ingress direction
IngressGeneralConns PolicyGeneralRulesConns
// EgressGeneralConns contains:
// - the maximal connection-set which the policy's rules allow to all destinations on egress direction
// - the maximal connection-set which the policy's rules allow to all namespaces in the cluster on egress direction
EgressGeneralConns PolicyGeneralRulesConns
}

type PolicyGeneralRulesConns struct {
// AllDestinationsConns contains the maximal connection-set which the policy's rules allow to all destinations
// (all namespaces, pods and IP addresses)
AllDestinationsConns *common.ConnectionSet
// EntireClusterConns contains the maximal connection-set which the policy's rules allow to all namespaces in the cluster
EntireClusterConns *common.ConnectionSet
}

// @todo need a network policy collection type along with convenience methods?
// if so, also consider concurrent access (or declare not goroutine safe?)
Expand All @@ -58,15 +76,22 @@ func (np *NetworkPolicy) convertNamedPort(namedPort string, pod *Pod) int32 {
}

// getPortsRange returns the start and end port numbers given input port, endPort and dest peer
// if input port is a named port, and the dst peer does not have a matching named port defined, return
// 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, err error) {
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 {
if dst == nil {
return common.NoPort, common.NoPort, port.StrVal, nil
}
if dst.PeerType() != PodType {
return start, end, np.netpolErr(netpolerrors.NamedPortErrTitle, netpolerrors.ConvertNamedPortErrStr)
return start, end, "", np.netpolErr(netpolerrors.NamedPortErrTitle, netpolerrors.ConvertNamedPortErrStr)
}
portNum := np.convertNamedPort(port.StrVal, dst.GetPeerPod())
portName = port.StrVal
portNum := np.convertNamedPort(portName, dst.GetPeerPod())
start = portNum
end = portNum
} else {
Expand All @@ -76,11 +101,11 @@ func (np *NetworkPolicy) getPortsRange(port *intstr.IntOrString, endPort *int32,
end = *endPort
}
}
return start, end, nil
return start, end, portName, nil
}

func isEmptyPortRange(start, end int32) bool {
return start == noPort && end == noPort
return start == common.NoPort && end == common.NoPort
}

func (np *NetworkPolicy) ruleConnections(rulePorts []netv1.NetworkPolicyPort, dst Peer) (*common.ConnectionSet, error) {
Expand All @@ -94,19 +119,23 @@ func (np *NetworkPolicy) ruleConnections(rulePorts []netv1.NetworkPolicyPort, ds
if rulePorts[i].Protocol != nil {
protocol = *rulePorts[i].Protocol
}
ports := common.PortSet{}
ports := common.MakePortSet(false)
if rulePorts[i].Port == nil {
ports = common.MakePortSet(true)
} else {
startPort, endPort, err := np.getPortsRange(rulePorts[i].Port, rulePorts[i].EndPort, dst)
startPort, endPort, portName, err := np.getPortsRange(rulePorts[i].Port, rulePorts[i].EndPort, dst)
if err != nil {
return res, err
}
if isEmptyPortRange(startPort, endPort) {
continue
if dst == nil && portName != "" {
// adding namedPort to connectionSet in case of :
// - dst is nil - for general connections;
// - TODO: if dst is a fake pod (representative)
ports.AddPort(intstr.FromString(portName))
}
if !isEmptyPortRange(startPort, endPort) {
ports.AddPortRange(int64(startPort), int64(endPort))
}

ports.AddPortRange(int64(startPort), int64(endPort))
}
res.AddConnection(protocol, ports)
}
Expand All @@ -125,7 +154,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].Port, rulePorts[i].EndPort, dst)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -408,50 +437,74 @@ func (np *NetworkPolicy) fullName() string {
return types.NamespacedName{Name: np.Name, Namespace: np.Namespace}.String()
}

// ScanRulesForIngressFromEntireCluster scans ingress spec of the policy to check if the rules exposes given pod
// to entire cluster; if yes updates the pod's connection to entire cluster field
func (np *NetworkPolicy) ScanRulesForIngressFromEntireCluster(dst Peer) error {
// since we get here after scanning rules for allowed conns between peers, we can assume the rules are ok
// /////////////////////////////////////////////////////////////////////////////////////////////
// pre-processing computations - currently performed for exposure-analysis goals;

// DetermineGeneralConnectionsOfPolicy scans policy rules and updates if it allows conns with all destinations or/ and with entire cluster
// for ingress and/ or egress directions
func (np *NetworkPolicy) DetermineGeneralConnectionsOfPolicy() (err error) {
if np.policyAffectsDirection(netv1.PolicyTypeIngress) {
err = np.scanRulesForGeneralIngressConns()
if err != nil {
return err
}
}
if np.policyAffectsDirection(netv1.PolicyTypeEgress) {
err = np.scanRulesForGeneralEgressConns()
}
return err
}

func (np *NetworkPolicy) scanRulesForGeneralIngressConns() error {
for _, rule := range np.Spec.Ingress {
rulePeers := rule.From
rulePorts := rule.Ports
if !np.doRulesExposeToEntireCluster(rulePeers) {
allDests, entireCluster := np.doRulesExposeToAllDestOrEntireCluster(rulePeers)
if !allDests && !entireCluster {
continue
}
ruleConns, err := np.ruleConnections(rulePorts, dst)
ruleConns, err := np.ruleConnections(rulePorts, nil)
if err != nil {
return err
}
dst.GetPeerPod().updatePodXgressExposureToEntireClusterData(ruleConns, true)
if allDests {
np.IngressGeneralConns.AllDestinationsConns.Union(ruleConns)
}
if entireCluster {
np.IngressGeneralConns.EntireClusterConns.Union(ruleConns)
}
}
return nil
}

// ScanRulesForEgressToEntireCluster scans egress spec of the policy to check if the rules exposes src peer
// to entire class, if yes update egress to entire class connection details of the src
// dst is provided for computing the correct connection of the rules
func (np *NetworkPolicy) ScanRulesForEgressToEntireCluster(src, dst Peer) error {
func (np *NetworkPolicy) scanRulesForGeneralEgressConns() error {
for _, rule := range np.Spec.Egress {
rulePeers := rule.To
rulePorts := rule.Ports
if !np.doRulesExposeToEntireCluster(rulePeers) {
allDests, entireCluster := np.doRulesExposeToAllDestOrEntireCluster(rulePeers)
if !allDests && !entireCluster {
continue
}
ruleConns, err := np.ruleConnections(rulePorts, dst)
ruleConns, err := np.ruleConnections(rulePorts, nil)
if err != nil {
return err
}
src.GetPeerPod().updatePodXgressExposureToEntireClusterData(ruleConns, false)
if allDests {
np.EgressGeneralConns.AllDestinationsConns.Union(ruleConns)
}
if entireCluster {
np.EgressGeneralConns.EntireClusterConns.Union(ruleConns)
}
}
return nil
}

// doRulesExposeToEntireCluster checks if the given rules list is exposed to entire cluster;
// doRulesExposeToAllDestOrEntireCluster checks if the given rules list is exposed to entire cluster;
// i.e. if the rules list is empty or if there is a rule with empty namespaceSelector
// since this func is definitely called after ruleSelectsPeer, there is no rules' correctness check here
func (np *NetworkPolicy) doRulesExposeToEntireCluster(rules []netv1.NetworkPolicyPeer) bool {
// this func assumes rules are legal (rules correctness check occurs later)
func (np *NetworkPolicy) doRulesExposeToAllDestOrEntireCluster(rules []netv1.NetworkPolicyPeer) (alldests, entireCluster bool) {
if len(rules) == 0 {
return true
return true, true
}
for i := range rules {
if rules[i].IPBlock != nil {
Expand All @@ -463,8 +516,8 @@ func (np *NetworkPolicy) doRulesExposeToEntireCluster(rules []netv1.NetworkPolic
// ns selector is not nil
selector, _ := np.parseNetpolLabelSelector(rules[i].NamespaceSelector)
if selector.Empty() {
return true
return false, true
}
}
return false
return false, false
}
26 changes: 20 additions & 6 deletions pkg/netpol/eval/internal/k8s/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,31 +255,45 @@ func (pod *Pod) PodExposedTCPConnections() *common.ConnectionSet {
for _, cPort := range pod.Ports {
protocol := corev1.ProtocolTCP
if cPort.Protocol == "" || protocol == corev1.ProtocolTCP {
ports := common.PortSet{}
ports := common.MakePortSet(false)
ports.AddPortRange(int64(cPort.ContainerPort), int64(cPort.ContainerPort))
res.AddConnection(protocol, ports)
}
}
return res
}

const noPort = -1

// ConvertPodNamedPort returns the ContainerPort number that matches the named port
// if there is no match, returns -1
// namedPort is unique within the pod
func (pod *Pod) ConvertPodNamedPort(namedPort string) int32 {
for _, containerPort := range pod.Ports {
if namedPort == containerPort.Name {
return containerPort.ContainerPort
}
}
return noPort
return common.NoPort
}

// updatePodXgressExposureToEntireClusterData updates the pods' fields which are related to entire class exposure on ingress/egress
func (pod *Pod) updatePodXgressExposureToEntireClusterData(ruleConns *common.ConnectionSet, isIngress bool) {
func (pod *Pod) UpdatePodXgressExposureToEntireClusterData(ruleConns *common.ConnectionSet, isIngress bool) {
if isIngress {
pod.IngressExposureData.EntireClusterConnection.Union(ruleConns)
// for a dst pod check if the given ruleConns contains namedPorts; if yes replace them with pod's
// matching port number
connNamedPorts := ruleConns.GetNamedPorts()
if len(connNamedPorts) > 0 {
ruleConnsCopy := ruleConns.Copy() // copying the connectionSet; in order to replace
// the named ports with pod's port numbers
for protocol, namedPorts := range connNamedPorts {
for _, namedPort := range namedPorts {
portNum := pod.ConvertPodNamedPort(namedPort)
ruleConnsCopy.ReplaceNamedPortWithMatchingPortNum(protocol, namedPort, portNum)
}
}
pod.IngressExposureData.EntireClusterConnection.Union(ruleConnsCopy)
} else {
pod.IngressExposureData.EntireClusterConnection.Union(ruleConns)
}
} else {
pod.EgressExposureData.EntireClusterConnection.Union(ruleConns)
}
Expand Down
21 changes: 19 additions & 2 deletions pkg/netpol/eval/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,13 @@ func (pe *PolicyEngine) upsertPod(pod *corev1.Pod) error {
return nil
}

func initPolicyGeneralConns() k8s.PolicyGeneralRulesConns {
return k8s.PolicyGeneralRulesConns{
AllDestinationsConns: common.MakeConnectionSet(false),
EntireClusterConns: common.MakeConnectionSet(false),
}
}

func (pe *PolicyEngine) upsertNetworkPolicy(np *netv1.NetworkPolicy) error {
netpolNamespace := np.ObjectMeta.Namespace
if netpolNamespace == "" {
Expand All @@ -320,11 +327,21 @@ func (pe *PolicyEngine) upsertNetworkPolicy(np *netv1.NetworkPolicy) error {
if _, ok := pe.netpolsMap[netpolNamespace]; !ok {
pe.netpolsMap[netpolNamespace] = make(map[string]*k8s.NetworkPolicy)
}
pe.netpolsMap[netpolNamespace][np.Name] = (*k8s.NetworkPolicy)(np)

newNetpol := &k8s.NetworkPolicy{
NetworkPolicy: np,
IngressGeneralConns: initPolicyGeneralConns(),
EgressGeneralConns: initPolicyGeneralConns(),
}
pe.netpolsMap[netpolNamespace][np.Name] = newNetpol

// scan policy ingress and egress rules to store allowed connections
// to entire cluster and to all destinations (if such connections are allowed by the policy)
err := newNetpol.DetermineGeneralConnectionsOfPolicy()

// clear the cache on netpols changes
pe.cache.clear()
return nil
return err
}

func (pe *PolicyEngine) deleteNamespace(ns *corev1.Namespace) error {
Expand Down
4 changes: 3 additions & 1 deletion pkg/netpol/internal/common/CanonicalIntervalSet.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,12 @@ func getNumAsStr(num int64) string {
return fmt.Sprintf("%v", num)
}

const emptyStr = "Empty"

// String returns a string representation of the current CanonicalIntervalSet object
func (c *CanonicalIntervalSet) String() string {
if c.IsEmpty() {
return "Empty"
return emptyStr
}
res := ""
for _, interval := range c.IntervalSet {
Expand Down
Loading

0 comments on commit db2c484

Please sign in to comment.