Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pre proccessing policy for general conns #306

Merged
merged 7 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
policy.UpdatePodEntireClusterConnFromPolicyData(dst, 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
policy.UpdatePodEntireClusterConnFromPolicyData(src, isIngress)
}
}
if err != nil {
Expand Down
155 changes: 121 additions & 34 deletions pkg/netpol/eval/internal/k8s/netpol.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,35 @@ 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

namedPortsToPortNum map[v1.Protocol]map[string]int32 // internal map; map from protocol to map from namedPort which
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the matching of named port to its number is per pod, so why is it stored here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed the handling of named-ports

adding some examples - locally tested :

  1. an ingress policy; exposes the workload on ingress to entire-cluster on named ports; since the dst is the workload itself - converts the ports to numbers

ing_spec_result

  1. an egress policy - which exposes the workload to entire cluster ; on named ports : we will see these potential named ports on the result.

egress_res

  1. an example combining named ports and port numbers:

example_spec_res

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, the example seem to make sense

// appeared in the policy rules to its matching port number
}

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 @@ -57,16 +78,34 @@ func (np *NetworkPolicy) convertNamedPort(namedPort string, pod *Pod) int32 {
return pod.ConvertPodNamedPort(namedPort)
}

func (np *NetworkPolicy) saveNamedPortToPort(protocol v1.Protocol, namedPort string, portNum int32) {
if np.namedPortsToPortNum == nil {
np.namedPortsToPortNum = make(map[v1.Protocol]map[string]int32)
}
// a namedPort was converted, store in the map
if _, ok := np.namedPortsToPortNum[protocol]; !ok {
np.namedPortsToPortNum[protocol] = make(map[string]int32)
}
np.namedPortsToPortNum[protocol][namedPort] = portNum
}

// 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 -1, -1, port.StrVal, nil
adisos marked this conversation as resolved.
Show resolved Hide resolved
}
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,7 +115,7 @@ 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 {
Expand All @@ -98,15 +137,20 @@ func (np *NetworkPolicy) ruleConnections(rulePorts []netv1.NetworkPolicyPort, ds
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 != "" {
// if dst is nil and rulePorts contains namedPort, we want to save the general connection with namedPort
ports.AddPort(intstr.FromString(portName))
}
if portName != "" && startPort != -1 {
np.saveNamedPortToPort(protocol, portName, startPort)
}
if !isEmptyPortRange(startPort, endPort) {
ports.AddPortRange(int64(startPort), int64(endPort))
}

ports.AddPortRange(int64(startPort), int64(endPort))
}
res.AddConnection(protocol, ports)
}
Expand All @@ -125,7 +169,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 +452,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 +531,27 @@ 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
}

// UpdatePodEntireClusterConnFromPolicyData updates the pod's entire cluster connections on the given direction
func (np *NetworkPolicy) UpdatePodEntireClusterConnFromPolicyData(podPeer Peer, isIngress bool) {
if isIngress {
// if ingress: the pod is the dst; its named port should be converted to numbers;
// if the policy's entire cluster connection includes
// named ports; add the matching port num to the connection
// should replace namedport here or move to another place?
np.IngressGeneralConns.EntireClusterConns.ReplaceNamedPortWithMatchingPortNum(np.namedPortsToPortNum)
podPeer.GetPeerPod().updatePodXgressExposureToEntireClusterData(np.IngressGeneralConns.EntireClusterConns, isIngress)
} else {
// TBD : if egress, the pod is the src; if the policy's entire cluster connection includes named ports;
// should we keep the named port only?
// (for specific real pods; this port is converted when computing the allowed conns between both peers)
// should following line: adding already converted namedPorts be here:
np.EgressGeneralConns.EntireClusterConns.ReplaceNamedPortWithMatchingPortNum(np.namedPortsToPortNum)
podPeer.GetPeerPod().updatePodXgressExposureToEntireClusterData(np.EgressGeneralConns.EntireClusterConns, isIngress)
}
}
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 initiatePolicyGeneralConns() k8s.PolicyGeneralRulesConns {
adisos marked this conversation as resolved.
Show resolved Hide resolved
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)

newNP := &k8s.NetworkPolicy{
adisos marked this conversation as resolved.
Show resolved Hide resolved
NetworkPolicy: np,
IngressGeneralConns: initiatePolicyGeneralConns(),
EgressGeneralConns: initiatePolicyGeneralConns(),
}
pe.netpolsMap[netpolNamespace][np.Name] = newNP

// 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 := newNP.DetermineGeneralConnectionsOfPolicy()

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

func (pe *PolicyEngine) deleteNamespace(ns *corev1.Namespace) error {
Expand Down
22 changes: 22 additions & 0 deletions pkg/netpol/internal/common/connectionset.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)

// ConnectionSet represents a set of allowed connections between two peers on a k8s env
Expand Down Expand Up @@ -250,6 +251,27 @@ func (conn *ConnectionSet) AllConnections() bool {
return conn.AllowAll
}

// ReplaceNamedPortWithMatchingPortNum : checks if the connectionSet contains named ports from the given map;
// if yes, add the matching port numbers to its portSet and delete the named port from the set
func (conn *ConnectionSet) ReplaceNamedPortWithMatchingPortNum(namedPortsMap map[v1.Protocol]map[string]int32) {
for protocol, portSet := range conn.AllowedProtocols {
if _, ok := namedPortsMap[protocol]; !ok {
continue
}
replacedNamedPorts := make([]string, 0)
for portName, portNum := range namedPortsMap[protocol] {
if portSet.NamedPorts[portName] {
portSet.AddPort(intstr.FromInt32(portNum))
replacedNamedPorts = append(replacedNamedPorts, portName)
}
}
// after replacing the named ports with numbers : delete them from the PortSet
for _, portName := range replacedNamedPorts {
portSet.RemovePort(intstr.FromString(portName))
}
}
}

const (
connsAndPortRangeSeparator = ","
allConnsStr = "All Connections"
Expand Down
16 changes: 14 additions & 2 deletions pkg/netpol/internal/common/portset.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ type PortSet struct {
func MakePortSet(all bool) PortSet {
if all {
portsInterval := Interval{Start: minPort, End: maxPort}
return PortSet{Ports: CanonicalIntervalSet{IntervalSet: []Interval{portsInterval}}}
return PortSet{Ports: CanonicalIntervalSet{IntervalSet: []Interval{portsInterval}},
NamedPorts: map[string]bool{},
ExcludedNamedPorts: map[string]bool{},
}
}
return PortSet{
NamedPorts: map[string]bool{},
ExcludedNamedPorts: map[string]bool{},
}
return PortSet{}
}

// Equal: return true if current object equals another PortSet object
Expand All @@ -41,6 +47,8 @@ func (p *PortSet) IsEmpty() bool {
// Copy: return a new copy of a PortSet object
func (p *PortSet) Copy() PortSet {
res := PortSet{}
res.NamedPorts = map[string]bool{}
res.ExcludedNamedPorts = map[string]bool{}
res.Ports = p.Ports.Copy()
for k, v := range p.NamedPorts {
res.NamedPorts[k] = v
Expand All @@ -54,6 +62,10 @@ func (p *PortSet) Copy() PortSet {
// AddPort: update current PortSet object with new added port as allowed
func (p *PortSet) AddPort(port intstr.IntOrString) {
if port.Type == intstr.String {
if p.NamedPorts == nil {
p.NamedPorts = make(map[string]bool)
p.ExcludedNamedPorts = make(map[string]bool)
}
p.NamedPorts[port.StrVal] = true
delete(p.ExcludedNamedPorts, port.StrVal)
} else {
Expand Down
Loading