diff --git a/pkg/network/node/networkpolicy.go b/pkg/network/node/networkpolicy.go index ae0efb574..e0b23c587 100644 --- a/pkg/network/node/networkpolicy.go +++ b/pkg/network/node/networkpolicy.go @@ -20,11 +20,13 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/util/async" + utilnet "k8s.io/utils/net" osdnv1 "github.com/openshift/api/network/v1" "github.com/openshift/library-go/pkg/network/networkutils" "github.com/openshift/sdn/pkg/network/common" "github.com/openshift/sdn/pkg/util/ovs" + "github.com/openshift/sdn/pkg/util/ranges" ) const HostNetworkNamespace = "openshift-host-network" @@ -43,6 +45,9 @@ type networkPolicyPlugin struct { namespaces map[uint32]*npNamespace // nsMatchCache caches matches for namespaceSelectors; see selectNamespacesInternal nsMatchCache map[string]*npCacheEntry + + warnedPolicies map[ktypes.UID]string + skippedPolicies map[ktypes.UID]string } // npNamespace tracks NetworkPolicy-related data for a Namespace @@ -101,6 +106,9 @@ func NewNetworkPolicyPlugin() osdnPolicy { namespacesByName: make(map[string]*npNamespace), nsMatchCache: make(map[string]*npCacheEntry), + + warnedPolicies: make(map[ktypes.UID]string), + skippedPolicies: make(map[ktypes.UID]string), } } @@ -356,15 +364,23 @@ func (np *networkPolicyPlugin) generateNamespaceFlows(otx ovs.Transaction, npns if npp.selectsAllIPs { allPodsSelected = true } - if npp.affectsIngress { affectsIngress = true + } + if npp.affectsEgress { + affectsEgress = true + } + + if np.skipIfTooManyFlows(&npp.policy, len(npp.ingressFlows)+len(npp.egressFlows)) { + continue + } + + if npp.affectsIngress { for _, flow := range npp.ingressFlows { otx.AddFlow("table=80, priority=150, reg1=%d, %s actions=output:NXM_NX_REG2[]", npns.vnid, flow) } } if npp.affectsEgress { - affectsEgress = true for _, flow := range npp.egressFlows { otx.AddFlow("table=27, priority=150, reg0=%d, %s actions=goto_table:30", npns.vnid, flow) } @@ -405,6 +421,51 @@ func (np *networkPolicyPlugin) generateNamespaceFlows(otx ovs.Transaction, npns } } +func (np *networkPolicyPlugin) skipIfTooManyFlows(policy *networkingv1.NetworkPolicy, numFlows int) bool { + skip := numFlows >= 10000 + skippedVersion := np.skippedPolicies[policy.UID] + + warn := !skip && numFlows >= 1000 + warnedVersion := np.warnedPolicies[policy.UID] + + npRef := &corev1.ObjectReference{ + APIVersion: "networking.k8s.io/v1", + Kind: "NetworkPolicy", + Namespace: policy.Namespace, + Name: policy.Name, + UID: policy.UID, + } + + switch { + case skip && skippedVersion != policy.ResourceVersion: + np.node.recorder.Eventf(npRef, corev1.EventTypeWarning, + "NetworkPolicySize", "TooManyFlows", + "This NetworkPolicy generates an extremely large number of OVS flows (%d) and so it will be ignored to prevent network degradation.", numFlows) + np.skippedPolicies[policy.UID] = policy.ResourceVersion + delete(np.warnedPolicies, policy.UID) + klog.Warningf("Ignoring NetworkPolicy %s/%s because it generates an unreasonable number of flows (%d)", + policy.Namespace, policy.Name, numFlows) + + case warn && warnedVersion != policy.ResourceVersion: + np.node.recorder.Eventf(npRef, corev1.EventTypeWarning, + "NetworkPolicySize", "TooManyFlows", + "This NetworkPolicy generates a very large number of OVS flows (%d) and may degrade network performance.", numFlows) + np.warnedPolicies[policy.UID] = policy.ResourceVersion + delete(np.skippedPolicies, policy.UID) + klog.Warningf("NetworkPolicy %s/%s generates a very large number of flows (%d)", + policy.Namespace, policy.Name, numFlows) + + case !skip && !warn && (skippedVersion != "" || warnedVersion != ""): + np.node.recorder.Eventf(npRef, corev1.EventTypeNormal, + "NetworkPolicySize", "OK", + "This NetworkPolicy now generates an acceptable number of OVS flows.") + delete(np.skippedPolicies, policy.UID) + delete(np.warnedPolicies, policy.UID) + } + + return skip +} + func (np *networkPolicyPlugin) EnsureVNIDRules(vnid uint32) { np.lock.Lock() defer np.lock.Unlock() @@ -596,6 +657,12 @@ func (np *networkPolicyPlugin) parsePortFlows(policy *networkingv1.NetworkPolicy } else if port.Port.Type != intstr.Int { klog.Warningf("Ignoring rule in NetworkPolicy %s/%s with unsupported named port %q", policy.Namespace, policy.Name, port.Port.StrVal) continue + } else if port.EndPort != nil { + start := int(port.Port.IntVal) + end := int(*port.EndPort) + for _, portMask := range ranges.PortRangeToPortMasks(start, end) { + portFlows = append(portFlows, fmt.Sprintf("%s, tp_dst=%s, ", protocol, portMask)) + } } else { portNum = int(port.Port.IntVal) } @@ -660,15 +727,17 @@ func (np *networkPolicyPlugin) parsePeerFlows(npns *npNamespace, npp *npPolicy, npp.watchesAllPods = true peerFlows = append(peerFlows, np.selectPodsFromNamespaces(peer.NamespaceSelector, peer.PodSelector, dir)...) } else if peer.IPBlock != nil { - if peer.IPBlock.Except != nil { - // Currently IPBlocks with except rules are skipped. - klog.Warningf("IPBlocks with except rules are not supported (NetworkPolicy [%s], Namespace [%s])", npp.policy.Name, npp.policy.Namespace) - } else { - // Network Policy has ipBlocks, allow traffic from/to those ips. + // Network Policy has ipBlocks, allow traffic from/to those ips. + if !utilnet.IsIPv4CIDRString(peer.IPBlock.CIDR) { + // We don't support IPv6, so we don't need to do anything + // to allow IPv6 CIDRs. + continue + } + for _, cidr := range ranges.IPBlockToCIDRs(peer.IPBlock) { if dir == ingressFlow { - peerFlows = append(peerFlows, fmt.Sprintf("ip, nw_src=%s, ", peer.IPBlock.CIDR)) + peerFlows = append(peerFlows, fmt.Sprintf("ip, nw_src=%s, ", cidr)) } else { - peerFlows = append(peerFlows, fmt.Sprintf("ip, nw_dst=%s, ", peer.IPBlock.CIDR)) + peerFlows = append(peerFlows, fmt.Sprintf("ip, nw_dst=%s, ", cidr)) } } } @@ -824,6 +893,8 @@ func (np *networkPolicyPlugin) handleDeleteNetworkPolicy(obj interface{}) { np.lock.Lock() defer np.lock.Unlock() + delete(np.warnedPolicies, policy.UID) + delete(np.skippedPolicies, policy.UID) if npns, exists := np.namespaces[vnid]; exists { np.cleanupNetworkPolicy(policy) delete(npns.policies, policy.UID) diff --git a/pkg/network/node/networkpolicy_test.go b/pkg/network/node/networkpolicy_test.go index ccde53d66..8247e33cf 100644 --- a/pkg/network/node/networkpolicy_test.go +++ b/pkg/network/node/networkpolicy_test.go @@ -17,6 +17,7 @@ import ( "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/util/async" osdnv1 "github.com/openshift/api/network/v1" @@ -28,19 +29,14 @@ func newTestNPP() (*networkPolicyPlugin, ovs.Interface, *atomic.Value, chan stru ovsif := ovs.NewFake("br0") ovsif.AddBridge() - np := &networkPolicyPlugin{ - node: &OsdnNode{ - kClient: kubeClient, - kubeInformers: informers.NewSharedInformerFactory(kubeClient, time.Hour), + np := NewNetworkPolicyPlugin().(*networkPolicyPlugin) + np.node = &OsdnNode{ + kClient: kubeClient, + kubeInformers: informers.NewSharedInformerFactory(kubeClient, time.Hour), - oc: &ovsController{ - ovs: ovsif, - }, + oc: &ovsController{ + ovs: ovsif, }, - - namespaces: make(map[uint32]*npNamespace), - namespacesByName: make(map[string]*npNamespace), - nsMatchCache: make(map[string]*npCacheEntry), } np.vnids = newNodeVNIDMap(np, nil) @@ -134,6 +130,7 @@ func delNamespace(np *networkPolicyPlugin, name string, vnid uint32) { } func addNetworkPolicy(np *networkPolicyPlugin, policy *networkingv1.NetworkPolicy) { + policy.ResourceVersion = "0" _, err := np.node.kClient.NetworkingV1().NetworkPolicies(policy.Namespace).Create(context.TODO(), policy, metav1.CreateOptions{}) if err != nil { panic(fmt.Sprintf("Unexpected error creating policy %q: %v", policy.Name, err)) @@ -144,6 +141,23 @@ func addNetworkPolicy(np *networkPolicyPlugin, policy *networkingv1.NetworkPolic } } +var resourceVersion = 1 + +func updateNetworkPolicy(np *networkPolicyPlugin, policy *networkingv1.NetworkPolicy) { + policy.ResourceVersion = fmt.Sprintf("%d", resourceVersion) + resourceVersion++ + _, err := np.node.kClient.NetworkingV1().NetworkPolicies(policy.Namespace).Update(context.TODO(), policy, metav1.UpdateOptions{}) + if err != nil { + panic(fmt.Sprintf("Unexpected error updating policy %q: %v", policy.Name, err)) + } + err = waitForEvent(np, func() bool { + return np.namespacesByName[policy.Namespace].policies[policy.UID].policy.ResourceVersion == policy.ResourceVersion + }) + if err != nil { + panic(fmt.Sprintf("Unexpected error waiting for policy %q: %v", policy.Name, err)) + } +} + func delNetworkPolicy(np *networkPolicyPlugin, policy *networkingv1.NetworkPolicy) { err := np.node.kClient.NetworkingV1().NetworkPolicies(policy.Namespace).Delete(context.TODO(), policy.Name, metav1.DeleteOptions{}) if err != nil { @@ -1009,8 +1023,7 @@ func TestNetworkPolicy_ipBlock(t *testing.T) { t.Error(err.Error()) } - // Add a policy with multiple ipBlocks, one of which will be ignored because - // of an "except" clause. + // Add a policy with multiple ipBlocks, including an "except" clause. synced.Store(false) addNetworkPolicy(np, &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -1033,7 +1046,7 @@ func TestNetworkPolicy_ipBlock(t *testing.T) { IPBlock: &networkingv1.IPBlock{ CIDR: "192.168.1.0/24", Except: []string{ - "192.168.1.1", + "192.168.1.1/32", }, }, }, @@ -1065,8 +1078,17 @@ func TestNetworkPolicy_ipBlock(t *testing.T) { watchesOwnPods: false, ingressFlows: []string{ fmt.Sprintf("ip, nw_src=192.168.0.0/24"), - // There is no rule allowing 192.168.1.0/24 because we can't - // implement the exception. + + // rule with except gets exploded to multiple flows + fmt.Sprintf("ip, nw_src=192.168.1.128/25"), + fmt.Sprintf("ip, nw_src=192.168.1.64/26"), + fmt.Sprintf("ip, nw_src=192.168.1.32/27"), + fmt.Sprintf("ip, nw_src=192.168.1.16/28"), + fmt.Sprintf("ip, nw_src=192.168.1.8/29"), + fmt.Sprintf("ip, nw_src=192.168.1.4/30"), + fmt.Sprintf("ip, nw_src=192.168.1.2/31"), + fmt.Sprintf("ip, nw_src=192.168.1.0/32"), + fmt.Sprintf("ip, nw_src=192.168.10.0/24"), fmt.Sprintf("ip, nw_src=192.168.20.0/24"), }, @@ -1555,3 +1577,343 @@ func _TestNetworkPolicy_MultiplePoliciesOneNamespace(t *testing.T) { } } } + +func TestNetworkPolicyPathological(t *testing.T) { + np, ovsif, synced, stopCh := newTestNPP() + defer close(stopCh) + + fakeRecorder := record.NewFakeRecorder(5) + np.node.recorder = fakeRecorder + + origFlows, err := ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + + // create a namespace + synced.Store(false) + addNamespace(np, "default", 0, nil) + npns := np.namespaces[0] + waitForSync(np, synced, "initialization") + + synced.Store(false) + addNetworkPolicy(np, &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deny-all", + UID: uid(npns, "deny-all"), + Namespace: npns.name, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{}, + }, + }) + waitForSync(np, synced, "default-deny") + + // Creating the namespace will have added "allow" flows, but the default-deny + // policy will remove them, so there should be no change from the initial state + flows, err := ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + err = assertFlowChanges(origFlows, flows) // no changes + if err != nil { + t.Fatalf("Unexpected flow changes: %v", err) + } + + // add 200 pods + for i := 0; i < 200; i++ { + name := fmt.Sprintf("pod-%d", i) + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: npns.name, + Name: name, + UID: uid(npns, name), + Labels: map[string]string{ + "friendly": "true", + }, + }, + Status: corev1.PodStatus{ + PodIP: fmt.Sprintf("10.0.0.%d", i), + }, + } + if i%2 == 0 { + pod.Labels["even"] = "true" + } + if i%10 == 0 { + pod.Labels["ten"] = "true" + } + + _, err := np.node.kClient.CoreV1().Pods(npns.name).Create(context.TODO(), pod, metav1.CreateOptions{}) + if err != nil { + panic(fmt.Sprintf("Unexpected error creating pod: %v", err)) + } + } + forceSync(np, synced) + + // Still no changes, because they're all stuck behind the default-deny + flows, err = ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + err = assertFlowChanges(origFlows, flows) // no changes + if err != nil { + t.Fatalf("Unexpected flow changes: %v", err) + } + + // Now create a pathological NetworkPolicy + synced.Store(false) + addNetworkPolicy(np, &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "allow-friendly", + UID: uid(npns, "allow-friendly"), + Namespace: npns.name, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "friendly": "true", + }, + }, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{{ + From: []networkingv1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "friendly": "true", + }, + }, + }}, + }}, + }, + }) + waitForSync(np, synced, "pathological NP") + + // There should *still* be no changes, because the pathological policy should have + // been ignored + flows, err = ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + err = assertFlowChanges(origFlows, flows) // no changes + if err != nil { + t.Fatalf("Unexpected flow changes: %v\nOrig: %#v\nNew: %#v", err, origFlows, flows) + } + + // Check that a single event was emitted + var event string + select { + case event = <-fakeRecorder.Events: + break + default: + break + } + if event == "" { + t.Fatalf("no Event emitted after adding pathological NetworkPolicy") + } + if !strings.HasPrefix(event, "Warning NetworkPolicySize") || !strings.Contains(event, "ignored") { + t.Fatalf("incorrect Event emitted after adding pathological NetworkPolicy: %s", event) + } + + event = "" + select { + case event = <-fakeRecorder.Events: + break + default: + break + } + if event != "" { + t.Fatalf("too many Events emitted after adding pathological NetworkPolicy: %s", event) + } + + // Changing pods (in a way that does not cause the policy to stop being + // pathological) should not result in the policy being accepted, or another event + // being emitted. + synced.Store(false) + err = np.node.kClient.CoreV1().Pods(npns.name).Delete(context.TODO(), "pod-1", metav1.DeleteOptions{}) + if err != nil { + panic(fmt.Sprintf("Unexpected error deleting pod: %v", err)) + } + waitForSync(np, synced, "delete pod") + + flows, err = ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + err = assertFlowChanges(origFlows, flows) // no changes + if err != nil { + t.Fatalf("Unexpected flow changes: %v\nOrig: %#v\nNew: %#v", err, origFlows, flows) + } + + event = "" + select { + case event = <-fakeRecorder.Events: + break + default: + break + } + if event != "" { + t.Fatalf("unexpected Event emitted after deleting pod: %s", event) + } + + // Changing the policy in a way that doesn't make it stop being pathological + // should not result in the policy being accepted, but will result in another + // event being emitted. + synced.Store(false) + updateNetworkPolicy(np, &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "allow-friendly", + UID: uid(npns, "allow-friendly"), + Namespace: npns.name, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "friendly": "true", + }, + }, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{{ + From: []networkingv1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "friendly": "true", + "even": "true", + }, + }, + }}, + }}, + }, + }) + waitForSync(np, synced, "updated pathological NP") + + flows, err = ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + err = assertFlowChanges(origFlows, flows) // no changes + if err != nil { + t.Fatalf("Unexpected flow changes: %v\nOrig: %#v\nNew: %#v", err, origFlows, flows) + } + + event = "" + select { + case event = <-fakeRecorder.Events: + break + default: + break + } + if event == "" { + t.Fatalf("no Event emitted after modifying pathological NetworkPolicy") + } + if !strings.HasPrefix(event, "Warning NetworkPolicySize") || !strings.Contains(event, "ignored") { + t.Fatalf("incorrect Event emitted after modifying pathological NetworkPolicy: %s", event) + } + + // Changing the NP to be bad-but-not-quite-pathological should result in another + // event, but it will be accepted now + synced.Store(false) + updateNetworkPolicy(np, &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "allow-friendly", + UID: uid(npns, "allow-friendly"), + Namespace: npns.name, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "friendly": "true", + }, + }, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{{ + From: []networkingv1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "friendly": "true", + "ten": "true", + }, + }, + }}, + }}, + }, + }) + waitForSync(np, synced, "updated pathological NP to less pathological") + + flows, err = ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + // The target podSelector "friendly=true" matches all 199 remaining pods. The + // source podSelector "ten=true" matches 20 pods. + if len(flows) != 199*20 { + t.Fatalf("Expected %d flows, got %d", 199*20, len(flows)) + } + + event = "" + select { + case event = <-fakeRecorder.Events: + break + default: + break + } + if event == "" { + t.Fatalf("no Event emitted after simplifying pathological NetworkPolicy") + } + if !strings.HasPrefix(event, "Warning NetworkPolicySize") || strings.Contains(event, "ignored") { + t.Fatalf("incorrect Event emitted after simplifying pathological NetworkPolicy: %s", event) + } + + // Changing the NP to something non-pathological should emit a non-warning event + // and result in flow changes + synced.Store(false) + updateNetworkPolicy(np, &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "allow-friendly", + UID: uid(npns, "allow-friendly"), + Namespace: npns.name, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + Ingress: []networkingv1.NetworkPolicyIngressRule{{ + From: []networkingv1.NetworkPolicyPeer{{ + PodSelector: &metav1.LabelSelector{}, + }}, + }}, + }, + }) + waitForSync(np, synced, "updated non-pathological NP") + + flows, err = ovsif.DumpFlows("") + if err != nil { + t.Fatalf("Unexpected error dumping flows: %v", err) + } + err = assertFlowChanges(origFlows, flows, + flowChange{ + kind: flowAdded, + match: []string{"table=80", "reg1=0", "actions=output:NXM_NX_REG2[]"}, + }, + ) + if err != nil { + t.Fatalf("Unexpected flow changes: %v\nOrig: %#v\nNew: %#v", err, origFlows, flows) + } + + event = "" + select { + case event = <-fakeRecorder.Events: + break + default: + break + } + if event == "" { + t.Fatalf("no Event emitted after adding non-pathological NetworkPolicy") + } + if !strings.HasPrefix(event, "Normal NetworkPolicySize") { + t.Fatalf("incorrect Event emitted after adding pathological NetworkPolicy: %s", event) + } +} diff --git a/pkg/util/ranges/doc.go b/pkg/util/ranges/doc.go new file mode 100644 index 000000000..ba45d670f --- /dev/null +++ b/pkg/util/ranges/doc.go @@ -0,0 +1,5 @@ +package ranges + +// ranges contains utilities for converting ranges from their API forms (eg, "start - end" +// or "start, except exception") to a form usable by OVS ([ start1/mask1, start2/mask2, +// start3/mask3 ]) diff --git a/pkg/util/ranges/networkpolicy.go b/pkg/util/ranges/networkpolicy.go new file mode 100644 index 000000000..f3893682a --- /dev/null +++ b/pkg/util/ranges/networkpolicy.go @@ -0,0 +1,115 @@ +package ranges + +import ( + "encoding/binary" + "fmt" + "net" + + networkingv1 "k8s.io/api/networking/v1" +) + +// IPBlockToCIDRs returns an array of CIDRs corresponding to ipBlock. +// +// To match a NetworkPolicy IPBlock with an "Except", we need to generate the OpenFlow +// equivalent of "nw_src=${CIDR} && nw_src!=${Except}". OVS has conjunctive matches to get +// the effect of "&&", but there's no way to say "!="... The only way to make this work is +// to rewrite +// +// nw_src=[A-H] && nw_src!=B && nw_src!=E +// +// as +// +// nw_src=A || nw_src=[C-D] || nw_src=[F-H] +// +// except that it's more complicated than that because CIDRs can only express ranges +// whose lengths are powers of 2. So, we call rangesForIPBlock() to generate the list +// "[[A-A], [C-D], [F-H]]", and then call .toRangeMasks() on each of those ranges to turn +// them into an equivalent list of VALUE/MASK values. +func IPBlockToCIDRs(ipBlock *networkingv1.IPBlock) []string { + if len(ipBlock.Except) == 0 { + return []string{ipBlock.CIDR} + } + cidrs := []string{} + for _, r := range rangesForIPBlock(ipBlock) { + for _, rangeMask := range r.toRangeMasks() { + cidr := &net.IPNet{ + IP: uint32ToBytes(rangeMask.start), + Mask: uint32ToBytes(rangeMask.mask), + } + cidrs = append(cidrs, cidr.String()) + } + } + return cidrs +} + +func uint32ToBytes(u uint32) []byte { + bytes := make([]byte, 4) + binary.BigEndian.PutUint32(bytes, u) + return bytes +} + +func bytesToUint32(bytes []byte) uint32 { + if len(bytes) == 16 { + to4 := net.IP(bytes).To4() + if to4 != nil { + bytes = to4 + } + } + return binary.BigEndian.Uint32(bytes) +} + +// rangeForCIDR takes a net.IPNet and returns an intRange +func rangeForCIDR(cidr *net.IPNet) intRange { + rangeMask := intRangeMask{ + start: bytesToUint32(cidr.IP), + mask: bytesToUint32(cidr.Mask), + } + return rangeMask.toRange() +} + +// rangesForIPBlock returns an array of ipRanges corresponding to ipBlock +func rangesForIPBlock(ipBlock *networkingv1.IPBlock) []intRange { + _, baseCIDR, _ := net.ParseCIDR(ipBlock.CIDR) + if baseCIDR == nil { + // can't happen + return nil + } + ranges := []intRange{rangeForCIDR(baseCIDR)} + + for _, except := range ipBlock.Except { + _, exceptCIDR, _ := net.ParseCIDR(except) + if exceptCIDR == nil { + // can't happen + return nil + } + + newRanges := make([]intRange, 0, len(ranges)+2) + for _, r := range ranges { + newRanges = append(newRanges, r.except(rangeForCIDR(exceptCIDR))...) + } + ranges = newRanges + } + + return ranges +} + +// PortRangeToPortMasks returns an array of port/mask strings corresponding to the given +// start and end values. +// +// Here the problem is that we need ">=" and "<=", which OpenFlow doesn't have. So we have +// to figure out how to express "tp_dst >= ${START} && tp_dst <= ${END}" as a series of +// "tp_dst=${VALUE}/${MASK}" matches. +// +// (The naive implementation of port range matching would be to just check tp_dst against +// each value from start to end, for a total of (end-start+1) rules. PortRangeToPortMasks +// generates the same number of rules as the naive implementation when start==end or when +// start is odd and end==start+1, but in all other cases it generates fewer total rules.) +func PortRangeToPortMasks(start, end int) []string { + portRange := intRange{uint32(start), uint32(end)} + rangeMasks := portRange.toRangeMasks() + masks := make([]string, len(rangeMasks)) + for i, rm := range rangeMasks { + masks[i] = fmt.Sprintf("0x%04x/0x%04x", uint16(rm.start), uint16(rm.mask)) + } + return masks +} diff --git a/pkg/util/ranges/networkpolicy_test.go b/pkg/util/ranges/networkpolicy_test.go new file mode 100644 index 000000000..50de69464 --- /dev/null +++ b/pkg/util/ranges/networkpolicy_test.go @@ -0,0 +1,273 @@ +package ranges + +import ( + "fmt" + "net" + "reflect" + "testing" + + networkingv1 "k8s.io/api/networking/v1" +) + +func mustParseCIDR(cidrString string) *net.IPNet { + _, cidr, err := net.ParseCIDR(cidrString) + if err != nil { + panic(err.Error()) + } + return cidr +} + +func Test_rangeForCIDR(t *testing.T) { + var cidr *net.IPNet + var r intRange + var expectStart, expectEnd uint32 + + cidr = mustParseCIDR("10.0.0.0/8") + r = rangeForCIDR(cidr) + expectStart = 10 * 256 * 256 * 256 + expectEnd = 11*256*256*256 - 1 + if r.start != expectStart { + t.Fatalf("bad start %d != %d", r.start, expectStart) + } + if r.end != expectEnd { + t.Fatalf("bad end %d != %d", r.end, expectEnd) + } + + cidr = mustParseCIDR("192.168.0.0/24") + r = rangeForCIDR(cidr) + expectStart = 192*256*256*256 + 168*256*256 + expectEnd = 192*256*256*256 + 168*256*256 + 255 + if r.start != expectStart { + t.Fatalf("bad start %d != %d", r.start, expectStart) + } + if r.end != expectEnd { + t.Fatalf("bad end %d != %d", r.end, expectEnd) + } +} + +func parseRange(start, end string) intRange { + r := intRange{ + start: bytesToUint32(net.ParseIP(start)), + end: bytesToUint32(net.ParseIP(end)), + } + return r +} + +func Test_rangesForIPBlock(t *testing.T) { + for i, tc := range []struct { + ipBlock networkingv1.IPBlock + result []intRange + }{ + { + ipBlock: networkingv1.IPBlock{ + CIDR: "10.0.0.0/8", + Except: []string{"10.0.1.0/24"}, + }, + result: []intRange{ + parseRange("10.0.0.0", "10.0.0.255"), + parseRange("10.0.2.0", "10.255.255.255"), + }, + }, + { + ipBlock: networkingv1.IPBlock{ + CIDR: "192.168.0.0/16", + Except: []string{ + "192.168.2.0/24", + "192.168.3.6/32", + }, + }, + result: []intRange{ + parseRange("192.168.0.0", "192.168.1.255"), + parseRange("192.168.3.0", "192.168.3.5"), + parseRange("192.168.3.7", "192.168.255.255"), + }, + }, + { + ipBlock: networkingv1.IPBlock{ + CIDR: "192.168.1.0/24", + Except: []string{ + "192.168.1.0/32", + "192.168.1.9/32", + "192.168.1.255/32", + }, + }, + result: []intRange{ + parseRange("192.168.1.1", "192.168.1.8"), + parseRange("192.168.1.10", "192.168.1.254"), + }, + }, + } { + ranges := rangesForIPBlock(&tc.ipBlock) + + if !reflect.DeepEqual(tc.result, ranges) { + t.Fatalf("bad result for %d\nexpected %v\ngot %v", i, tc.result, ranges) + } + } +} + +func TestIPBlockToCIDRs(t *testing.T) { + for i, tc := range []struct { + ipBlock networkingv1.IPBlock + result []string + }{ + { + ipBlock: networkingv1.IPBlock{ + CIDR: "10.0.0.0/8", + Except: []string{ + "10.0.1.0/24", + }, + }, + result: []string{ + "10.0.0.0/24", // 10.0.0.0 - 10.0.0.255 + "10.0.2.0/23", // 10.0.2.0 - 10.0.3.255 + "10.0.4.0/22", // 10.0.4.0 - 10.0.7.255 + "10.0.8.0/21", // 10.0.8.0 - 10.0.15.255 + "10.0.16.0/20", // 10.0.16.0 - 10.0.31.255 + "10.0.32.0/19", // 10.0.32.0 - 10.0.63.255 + "10.0.64.0/18", // 10.0.64.0 - 10.0.127.255 + "10.0.128.0/17", // 10.0.128.0 - 10.0.255.255 + "10.1.0.0/16", // 10.1.0.0 - 10.1.255.255 + "10.2.0.0/15", // 10.2.0.0 - 10.3.255.255 + "10.4.0.0/14", // 10.4.0.0 - 10.7.255.255 + "10.8.0.0/13", // 10.8.0.0 - 10.15.255.255 + "10.16.0.0/12", // 10.16.0.0 - 10.31.255.255 + "10.32.0.0/11", // 10.32.0.0 - 10.63.255.255 + "10.64.0.0/10", // 10.64.0.0 - 10.127.255.255 + "10.128.0.0/9", // 10.128.0.0 - 10.255.255.255 + }, + }, + { + ipBlock: networkingv1.IPBlock{ + CIDR: "192.168.0.0/16", + Except: []string{ + "192.168.2.0/24", + "192.168.3.6/32", + }, + }, + result: []string{ + "192.168.0.0/23", // 192.168.0.0 - 192.168.1.255 + "192.168.3.0/30", // 192.168.3.0 - 192.168.3.3 + "192.168.3.4/31", // 192.168.3.4 - 192.168.3.5 + "192.168.3.7/32", // 192.168.3.7 - 192.168.3.7 + "192.168.3.8/29", // 192.168.3.8 - 192.168.3.15 + "192.168.3.16/28", // 192.168.3.16 - 192.168.3.31 + "192.168.3.32/27", // 192.168.3.32 - 192.168.3.63 + "192.168.3.64/26", // 192.168.3.64 - 192.168.3.127 + "192.168.3.128/25", // 192.168.3.128 - 192.168.3.255 + "192.168.4.0/22", // 192.168.4.0 - 192.168.7.255 + "192.168.8.0/21", // 192.168.8.0 - 192.168.15.255 + "192.168.16.0/20", // 192.168.16.0 - 192.168.31.255 + "192.168.32.0/19", // 192.168.32.0 - 192.168.63.255 + "192.168.64.0/18", // 192.168.64.0 - 192.168.127.255 + "192.168.128.0/17", // 192.168.128.0 - 192.168.255.255 + }, + }, + { + ipBlock: networkingv1.IPBlock{ + CIDR: "192.168.1.0/24", + Except: []string{ + "192.168.1.0/32", + "192.168.1.9/32", + "192.168.1.255/32", + }, + }, + result: []string{ + "192.168.1.1/32", // 192.168.1.1 - 192.168.1.1 + "192.168.1.2/31", // 192.168.1.2 - 192.168.1.3 + "192.168.1.4/30", // 192.168.1.4 - 192.168.1.7 + "192.168.1.8/32", // 192.168.1.8 - 192.168.1.8 + "192.168.1.10/31", // 192.168.1.10 - 192.168.1.11 + "192.168.1.12/30", // 192.168.1.12 - 192.168.1.15 + "192.168.1.16/28", // 192.168.1.16 - 192.168.1.31 + "192.168.1.32/27", // 192.168.1.32 - 192.168.1.63 + "192.168.1.64/26", // 192.168.1.64 - 192.168.1.127 + "192.168.1.128/26", // 192.168.1.128 - 192.168.1.191 + "192.168.1.192/27", // 192.168.1.192 - 192.168.1.223 + "192.168.1.224/28", // 192.168.1.224 - 192.168.1.239 + "192.168.1.240/29", // 192.168.1.240 - 192.168.1.247 + "192.168.1.248/30", // 192.168.1.248 - 192.168.1.251 + "192.168.1.252/31", // 192.168.1.252 - 192.168.1.253 + "192.168.1.254/32", // 192.168.1.254 - 192.168.1.254 + }, + }, + } { + cidrs := IPBlockToCIDRs(&tc.ipBlock) + + if !reflect.DeepEqual(tc.result, cidrs) { + fmt.Printf("\t\t\tresult: []string{\n") + for _, cidr := range cidrs { + r := rangeForCIDR(mustParseCIDR(cidr)) + fmt.Printf("\t\t\t\t\"%s\", // %s - %s\n", + cidr, + net.IP(uint32ToBytes(r.start)), + net.IP(uint32ToBytes(r.end)), + ) + } + fmt.Printf("\t\t\t}\n") + t.Fatalf("bad result for %d", i) + } + } +} + +func TestPortRangeToPortMasks(t *testing.T) { + for i, tc := range []struct { + start uint16 + end uint16 + result []string + }{ + { + start: 0, + end: 0, + result: []string{ + "0x0000/0xffff", + }, + }, + { + start: 0, + end: 65535, + result: []string{ + "0x0000/0x0000", + }, + }, + { + start: 0, + end: 1023, + result: []string{ + "0x0000/0xfc00", + }, + }, + { + start: 1024, + end: 65535, + result: []string{ + "0x0400/0xfc00", + "0x0800/0xf800", + "0x1000/0xf000", + "0x2000/0xe000", + "0x4000/0xc000", + "0x8000/0x8000", + }, + }, + { + start: 6000, + end: 6100, + result: []string{ + "0x1770/0xfff0", + "0x1780/0xffc0", + "0x17c0/0xfff0", + "0x17d0/0xfffc", + "0x17d4/0xffff", + }, + }, + } { + masks := PortRangeToPortMasks(int(tc.start), int(tc.end)) + if !reflect.DeepEqual(masks, tc.result) { + fmt.Printf("\t\t\tresult: []string{\n") + for _, mask := range masks { + fmt.Printf("\t\t\t\t%q,\n", mask) + } + fmt.Printf("\t\t\t},\n") + t.Fatalf("bad result for %d\nexpected %v\ngot %v", i, tc.result, masks) + } + } +} diff --git a/pkg/util/ranges/range.go b/pkg/util/ranges/range.go new file mode 100644 index 000000000..9c643da1c --- /dev/null +++ b/pkg/util/ranges/range.go @@ -0,0 +1,155 @@ +package ranges + +import ( + "fmt" + "math" + "math/bits" +) + +// An intRange represents a range of ints by its start and end values (inclusive) +type intRange struct { + start uint32 + end uint32 +} + +// for debugging +func (r intRange) String() string { + return fmt.Sprintf("[ 0x%x, 0x%x ]", r.start, r.end) +} + +// An intRangeMask represents a range of ints by its start and a mask value +type intRangeMask struct { + start uint32 + mask uint32 +} + +// for debugging +func (r intRangeMask) String() string { + return fmt.Sprintf("0x%x/0x%x", r.start, r.mask) +} + +// except returns a set of ranges equivalent to r with except removed +func (r intRange) except(except intRange) []intRange { + switch { + case r.start > except.end || r.end < except.start: + // The range is either entirely after or entirely before the exception, so + // keep the whole range. + return []intRange{r} + + case r.start >= except.start && r.end <= except.end: + // The exception completely overlaps the range, so omit the whole range. + return nil + } + + // At this point we know there is a partial, but not complete, overlap + + switch { + case except.start <= r.start: + // The exception starts before (or at) the range start, but does not + // completely overlap the range, so keep the portion of the range after + // the exception. + return []intRange{ + {except.end + 1, r.end}, + } + + case except.end >= r.end: + // The exception ends after (or at) the range end, but does not completely + // overlap the range, so keep the portion of the range before the + // exception. + return []intRange{ + {r.start, except.start - 1}, + } + } + + // At this point we know by process of elimination that the exception both starts + // and ends inside the range (with at least one element of r on either side of + // it), so split the range into the segment before the exception and the segment + // after it. + return []intRange{ + {r.start, except.start - 1}, + {except.end + 1, r.end}, + } +} + +// toRangeMasks converts an intRange to an equivalent array of intRangeMasks +func (r intRange) toRangeMasks() []intRangeMask { + rangeMasks := []intRangeMask{} + + // Repeatedly find the largest usable intRangeMask starting at start, then + // update start to point to after that range mask, and loop until we reach + // r.end. + start := r.start + for { + rangeMask, rangeEnd := nextMask(start, r.end) + rangeMasks = append(rangeMasks, rangeMask) + + if rangeEnd == r.end { + // Reached the end + break + } + start = rangeEnd + 1 + } + + return rangeMasks +} + +// nextMask computes the mask and end value for the largest intRangeMask starting at start +// and ending at or before end. +func nextMask(start, end uint32) (intRangeMask, uint32) { + // An intRangeMask covers a range from a starting value, to that value with some + // consecutive number of its trailing "0" bits flipped to "1". Eg, if start is + // 0xa120, then the intRangeMasks we can generate are: + // + // { start = 0xa120, mask = 0xffe0 } => 0xa120 - 0xa13f (len = 32) + // { start = 0xa120, mask = 0xfff0 } => 0xa120 - 0xa12f (len = 16) + // { start = 0xa120, mask = 0xfff8 } => 0xa120 - 0xa127 (len = 8) + // { start = 0xa120, mask = 0xfffc } => 0xa120 - 0xa123 (len = 4) + // { start = 0xa120, mask = 0xfffe } => 0xa120 - 0xa121 (len = 2) + // { start = 0xa120, mask = 0xffff } => 0xa120 - 0xa120 (len = 1) + // + // The largest range we can generate is the one with a mask that flips every + // trailing "0" bit in start to "1", which is to say, the mask that has "1" bits + // up to the last "1" bit in start and "0" bits after that: + // + // start = 0xa120 = 0b1010000100100000 + // mask = 0xffe0 = 0b1111111111100000 + // rangeEnd = 0xa13f = 0b1010000100111111 + // + // (Any mask that doesn't have "1" bits up to the last "1" bit in start would + // allow generating values that are *less than* start.) + mask := uint32(math.MaxUint32) << bits.TrailingZeros32(start) + rangeEnd := start ^ ^mask + + // If that ends within our range, then use it + if rangeEnd <= end { + return intRangeMask{start, mask}, rangeEnd + } + + // OK, we need to find the intRangeMask with the largest power-of-2 length such + // that (length <= end - start + 1). Eg, in the examples above, if end was 0x12a, + // then the largest range we can use would be the 8 byte range from 0x120 to + // 0x127. A power of 2 in binary is a number with a single "1" bit, and the + // largest power of 2 less than or equal to (end - start + 1) is the number that + // has a single "1" bit in the same position as the first "1" bit in (end - start + // + 1). The mask to generate that power-of-2-length range is the one with "1" + // bits up to that first "1" bit: + // + // start = 0xa120 = 0b1010000100100000 + // end = 0xa12a = 0b1010000100101010 + // end-start+1 = 0x000b = 0b0000000000001011 + // rangeLength = 0x0008 = 0b0000000000001000 + // mask = 0xfff8 = 0b1111111111111000 + // rangeEnd = 0xa127 = 0b1010000100100111 + maskLen := bits.LeadingZeros32(end-start+1) + 1 + mask = math.MaxUint32 << (32 - maskLen) + rangeEnd = start ^ ^mask + return intRangeMask{start, mask}, rangeEnd +} + +// toRange converts an intRangeMask to an equivalent intRange +func (r intRangeMask) toRange() intRange { + return intRange{ + start: r.start, + end: r.start ^ ^r.mask, + } +} diff --git a/pkg/util/ranges/range_test.go b/pkg/util/ranges/range_test.go new file mode 100644 index 000000000..778b98141 --- /dev/null +++ b/pkg/util/ranges/range_test.go @@ -0,0 +1,135 @@ +package ranges + +import ( + "reflect" + "testing" +) + +func Test_intRange_except(t *testing.T) { + ranges := []intRange{ + { + start: 17, + end: 135, + }, + } + + // Note that the tests are cumulative + for i, tc := range []struct { + except intRange + result []intRange + }{ + { + except: intRange{ + start: 20, + end: 40, + }, + + result: []intRange{ + { + start: 17, + end: 19, + }, + { + start: 41, + end: 135, + }, + }, + }, + { + except: intRange{ + start: 130, + end: 140, + }, + + result: []intRange{ + { + start: 17, + end: 19, + }, + { + start: 41, + end: 129, + }, + }, + }, + { + except: intRange{ + start: 100, + end: 109, + }, + + result: []intRange{ + { + start: 17, + end: 19, + }, + { + start: 41, + end: 99, + }, + { + start: 110, + end: 129, + }, + }, + }, + { + except: intRange{ + start: 105, + end: 200, + }, + + result: []intRange{ + { + start: 17, + end: 19, + }, + { + start: 41, + end: 99, + }, + }, + }, + { + except: intRange{ + start: 80, + end: 99, + }, + result: []intRange{ + { + start: 17, + end: 19, + }, + { + start: 41, + end: 79, + }, + }, + }, + { + except: intRange{ + start: 100, + end: 200, + }, + result: []intRange{ + { + start: 17, + end: 19, + }, + { + start: 41, + end: 79, + }, + }, + }, + } { + newRanges := []intRange{} + for _, r := range ranges { + newRanges = append(newRanges, r.except(tc.except)...) + } + ranges = newRanges + if !reflect.DeepEqual(ranges, tc.result) { + t.Fatalf("bad result for %d\nexpected %v\ngot %v", i, tc.result, ranges) + } + } +}