Validate AntreaIPAM IP ranges

Verify that: - There are no overlaps between an IPPool ranges while creating and updating pools. - Validate that the gateway IP belongs to the same subnet as the IP range addresses. Signed-off-by: Kobi Samoray <[email protected]>
antrea-io · Nov 18, 2021 · 85ab7f9 · 85ab7f9
1 parent 416e1ec
commit 85ab7f9
Show file tree

Hide file tree

Showing 9 changed files with 257 additions and 6 deletions.
diff --git a/build/yamls/antrea-aks.yml b/build/yamls/antrea-aks.yml
@@ -4955,6 +4955,7 @@ webhooks:
     apiVersions:
     - v1alpha2
     operations:
+    - CREATE
     - UPDATE
     - DELETE
     resources:

diff --git a/build/yamls/antrea-eks.yml b/build/yamls/antrea-eks.yml
@@ -4957,6 +4957,7 @@ webhooks:
     apiVersions:
     - v1alpha2
     operations:
+    - CREATE
     - UPDATE
     - DELETE
     resources:

diff --git a/build/yamls/antrea-gke.yml b/build/yamls/antrea-gke.yml
@@ -4955,6 +4955,7 @@ webhooks:
     apiVersions:
     - v1alpha2
     operations:
+    - CREATE
     - UPDATE
     - DELETE
     resources:

diff --git a/build/yamls/antrea-ipsec.yml b/build/yamls/antrea-ipsec.yml
@@ -5004,6 +5004,7 @@ webhooks:
     apiVersions:
     - v1alpha2
     operations:
+    - CREATE
     - UPDATE
     - DELETE
     resources:

diff --git a/build/yamls/antrea-kind.yml b/build/yamls/antrea-kind.yml
@@ -4956,6 +4956,7 @@ webhooks:
     apiVersions:
     - v1alpha2
     operations:
+    - CREATE
     - UPDATE
     - DELETE
     resources:

diff --git a/build/yamls/antrea.yml b/build/yamls/antrea.yml
@@ -4960,6 +4960,7 @@ webhooks:
     apiVersions:
     - v1alpha2
     operations:
+    - CREATE
     - UPDATE
     - DELETE
     resources:

diff --git a/build/yamls/base/controller.yml b/build/yamls/base/controller.yml
@@ -175,7 +175,7 @@ webhooks:
         namespace: "kube-system"
         path: "/validate/ippool"
     rules:
-      - operations: ["UPDATE", "DELETE"]
+      - operations: ["CREATE", "UPDATE", "DELETE"]
         apiGroups: ["crd.antrea.io"]
         apiVersions: ["v1alpha2"]
         resources: ["ippools"]

diff --git a/pkg/controller/ipam/validate.go b/pkg/controller/ipam/validate.go
@@ -15,8 +15,10 @@
 package ipam
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
+	"net"
 	"strings"
 
 	admv1 "k8s.io/api/admission/v1"
@@ -27,7 +29,6 @@ import (
 )
 
 func ValidateIPPool(review *admv1.AdmissionReview) *admv1.AdmissionResponse {
-	var result *metav1.Status
 	var msg string
 	allowed := true
 
@@ -48,16 +49,46 @@ func ValidateIPPool(review *admv1.AdmissionReview) *admv1.AdmissionResponse {
 
 	switch review.Request.Operation {
 	case admv1.Create:
-		// This shouldn't happen with the webhook configuration we include in the Antrea YAML manifests.
 		klog.V(2).Info("Validating CREATE request for IPPool")
-		// Always allow CREATE request.
+
+		// Validate individual ranges
+		for _, r := range newObj.Spec.IPRanges {
+			allowed, msg = validateIPRange(r, newObj.Spec.IPVersion)
+			if !allowed {
+				return validationResult(allowed, msg)
+			}
+		}
+
+		// Validate that the added ipRanges do not overlap
+		for i, r1 := range newObj.Spec.IPRanges[0 : len(newObj.Spec.IPRanges)-1] {
+			for _, r2 := range newObj.Spec.IPRanges[i+1 : len(newObj.Spec.IPRanges)] {
+				if rangesOverlap(r1, r2) {
+					msg = fmt.Sprintf("IPRanges %s overlap", humanReadableIPRanges([]crdv1alpha2.SubnetIPRange{r1, r2}))
+					return validationResult(false, msg)
+				}
+			}
+		}
 	case admv1.Update:
 		klog.V(2).Info("Validating UPDATE request for IPPool")
 		deletedIPRanges := getIPRangeDifference(oldObj.Spec.IPRanges, newObj.Spec.IPRanges)
 		if len(deletedIPRanges) > 0 {
-			allowed = false
-
 			msg = fmt.Sprintf("existing IPRanges %s cannot be updated or deleted", humanReadableIPRanges(deletedIPRanges))
+			return validationResult(false, msg)
+		}
+
+		addedIPRanges := getIPRangeDifference(newObj.Spec.IPRanges, oldObj.Spec.IPRanges)
+		for _, r1 := range addedIPRanges {
+			allowed, msg = validateIPRange(r1, newObj.Spec.IPVersion)
+			if !allowed {
+				return validationResult(allowed, msg)
+			}
+			for _, r2 := range newObj.Spec.IPRanges {
+				if r1 != r2 && rangesOverlap(r1, r2) {
+					msg = fmt.Sprintf("IPRanges %s overlap",
+						humanReadableIPRanges([]crdv1alpha2.SubnetIPRange{r1, r2}))
+					return validationResult(false, msg)
+				}
+			}
 		}
 	case admv1.Delete:
 		klog.V(2).Info("Validating DELETE request for IPPool")
@@ -67,6 +98,12 @@ func ValidateIPPool(review *admv1.AdmissionReview) *admv1.AdmissionResponse {
 		}
 	}
 
+	return validationResult(allowed, msg)
+}
+
+func validationResult(allowed bool, msg string) *admv1.AdmissionResponse {
+	var result *metav1.Status
+
 	if msg != "" {
 		result = &metav1.Status{
 			Message: msg,
@@ -107,6 +144,92 @@ func humanReadableIPRanges(ipRanges []crdv1alpha2.SubnetIPRange) string {
 	return fmt.Sprintf("[%s]", strings.Join(strs, ","))
 }
 
+func rangesOverlap(r1, r2 crdv1alpha2.SubnetIPRange) bool {
+	if r1.CIDR == "" {
+		if r2.CIDR == "" {
+			r1start := net.ParseIP(r1.Start).To16()
+			r1end := net.ParseIP(r1.End).To16()
+			r2start := net.ParseIP(r2.Start).To16()
+			r2end := net.ParseIP(r2.End).To16()
+
+			if ipInRange(r1start, r1end, r2start) || ipInRange(r2start, r2end, r1start) {
+				return true
+			}
+		} else {
+			_, cidr2, _ := net.ParseCIDR(r2.CIDR)
+			if cidr2.Contains(net.ParseIP(r1.Start)) || cidr2.Contains(net.ParseIP(r1.End)) {
+				return true
+			}
+			return ipInRange(net.ParseIP(r1.Start).To16(), net.ParseIP(r1.End).To16(), cidr2.IP.To16())
+		}
+	} else {
+		_, cidr1, _ := net.ParseCIDR(r1.CIDR)
+		if r2.CIDR == "" {
+			if cidr1.Contains(net.ParseIP(r2.Start)) || cidr1.Contains(net.ParseIP(r2.End)) {
+				return true
+			}
+			return ipInRange(net.ParseIP(r2.Start).To16(), net.ParseIP(r2.End).To16(), cidr1.IP.To16())
+		} else {
+			_, cidr2, _ := net.ParseCIDR(r2.CIDR)
+			return cidr1.Contains(cidr2.IP) || cidr2.Contains(cidr1.IP)
+		}
+	}
+	return false
+}
+
+func ipInRange(rangeStart, rangeEnd, ip net.IP) bool {
+	// Validate that ip is within start and end of range
+	return bytes.Compare(ip, rangeStart) >= 0 && bytes.Compare(ip, rangeEnd) <= 0
+}
+
+func ipVersion(ip net.IP) int {
+	if ip.To4() != nil {
+		return 4
+	}
+	return 6
+}
+
+func validateIPRange(r crdv1alpha2.SubnetIPRange, poolIPVersion int) (bool, string) {
+	// Validate the integrity of IPs within the IP range
+	gateway := net.ParseIP(r.Gateway)
+	if r.CIDR != "" {
+		_, cidr, _ := net.ParseCIDR(r.CIDR)
+		var mask net.IPMask
+		if ipVersion(cidr.IP) == 4 {
+			mask = net.CIDRMask(int(r.PrefixLength), 32)
+		} else {
+			mask = net.CIDRMask(int(r.PrefixLength), 128)
+		}
+		netCidr := net.IPNet{IP: cidr.IP, Mask: mask}
+
+		if !netCidr.Contains(gateway) {
+			return false, fmt.Sprintf(
+				"Range is invalid. Gateway %s is unreachable from CIDR %s", r.Gateway, r.CIDR)
+		}
+		if ipVersion(cidr.IP) != poolIPVersion {
+			return false, fmt.Sprintf(
+				"Range is invalid. IP version of range %s differs from Pool IP version", r.CIDR)
+		}
+	} else {
+		if ipVersion(net.ParseIP(r.Start)) != poolIPVersion || ipVersion(net.ParseIP(r.End)) != poolIPVersion {
+			return false, fmt.Sprintf(
+				"Range is invalid. IP version of range %s-%s differs from Pool IP version", r.Start, r.End)
+		}
+		_, startCIDR, _ := net.ParseCIDR(fmt.Sprintf("%s/%d", r.Start, r.PrefixLength))
+		_, endCIDR, _ := net.ParseCIDR(fmt.Sprintf("%s/%d", r.End, r.PrefixLength))
+		if !startCIDR.Contains(gateway) {
+			return false, fmt.Sprintf(
+				"Range is invalid. Gateway %s is unreachable from IP %s", r.Gateway, r.Start)
+
+		}
+		if !endCIDR.Contains(gateway) {
+			return false, fmt.Sprintf(
+				"Range is invalid. Gateway %s is unreachable from IP %s", r.Gateway, r.End)
+		}
+	}
+	return true, ""
+}
+
 func newAdmissionResponseForErr(err error) *admv1.AdmissionResponse {
 	return &admv1.AdmissionResponse{
 		Result: &metav1.Status{

diff --git a/pkg/controller/ipam/validate_test.go b/pkg/controller/ipam/validate_test.go
@@ -92,6 +92,102 @@ func TestEgressControllerValidateExternalIPPool(t *testing.T) {
 			},
 			expectedResponse: &admv1.AdmissionResponse{Allowed: true},
 		},
+		{
+			name: "CREATE operation with IP overlap should not be allowed",
+			request: &admv1.AdmissionRequest{
+				Name:      "foo",
+				Operation: "CREATE",
+				Object: runtime.RawExtension{Raw: marshal(copyAndMutateIPPool(testIPPool, func(pool *crdv1alpha2.IPPool) {
+					pool.Spec.IPRanges = append(pool.Spec.IPRanges, crdv1alpha2.SubnetIPRange{
+						IPRange: crdv1alpha2.IPRange{
+							CIDR: "192.168.3.0/26",
+						},
+						SubnetInfo: crdv1alpha2.SubnetInfo{
+							Gateway:      "192.168.3.1",
+							PrefixLength: 24,
+						},
+					})
+				}))},
+			},
+			expectedResponse: &admv1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Message: "IPRanges [192.168.3.10-192.168.3.20,192.168.3.0/26] overlap",
+				},
+			},
+		},
+		{
+			name: "CREATE operation with CIDR overlap with IP range should not be allowed",
+			request: &admv1.AdmissionRequest{
+				Name:      "foo",
+				Operation: "CREATE",
+				Object: runtime.RawExtension{Raw: marshal(copyAndMutateIPPool(testIPPool, func(pool *crdv1alpha2.IPPool) {
+					pool.Spec.IPRanges = append(pool.Spec.IPRanges, crdv1alpha2.SubnetIPRange{
+						IPRange: crdv1alpha2.IPRange{
+							CIDR: "192.168.3.12/30",
+						},
+						SubnetInfo: crdv1alpha2.SubnetInfo{
+							Gateway:      "192.168.3.13",
+							PrefixLength: 24,
+						},
+					})
+				}))},
+			},
+			expectedResponse: &admv1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Message: "IPRanges [192.168.3.10-192.168.3.20,192.168.3.12/30] overlap",
+				},
+			},
+		},
+		{
+			name: "CREATE operation with mixed IP version should not be allowed",
+			request: &admv1.AdmissionRequest{
+				Name:      "foo",
+				Operation: "CREATE",
+				Object: runtime.RawExtension{Raw: marshal(copyAndMutateIPPool(testIPPool, func(pool *crdv1alpha2.IPPool) {
+					pool.Spec.IPRanges = append(pool.Spec.IPRanges, crdv1alpha2.SubnetIPRange{
+						IPRange: crdv1alpha2.IPRange{
+							CIDR: "10:2400::0/96",
+						},
+						SubnetInfo: crdv1alpha2.SubnetInfo{
+							Gateway:      "10:2400::01",
+							PrefixLength: 64,
+						},
+					})
+				}))},
+			},
+			expectedResponse: &admv1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Message: "Range is invalid. IP version of range 10:2400::0/96 differs from Pool IP version",
+				},
+			},
+		},
+		{
+			name: "CREATE operation with bad gateway not be allowed",
+			request: &admv1.AdmissionRequest{
+				Name:      "foo",
+				Operation: "CREATE",
+				Object: runtime.RawExtension{Raw: marshal(copyAndMutateIPPool(testIPPool, func(pool *crdv1alpha2.IPPool) {
+					pool.Spec.IPRanges = append(pool.Spec.IPRanges, crdv1alpha2.SubnetIPRange{
+						IPRange: crdv1alpha2.IPRange{
+							CIDR: "192.168.10.0/26",
+						},
+						SubnetInfo: crdv1alpha2.SubnetInfo{
+							Gateway:      "192.168.1.1",
+							PrefixLength: 24,
+						},
+					})
+				}))},
+			},
+			expectedResponse: &admv1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Message: "Range is invalid. Gateway 192.168.1.1 is unreachable from CIDR 192.168.10.0/26",
+				},
+			},
+		},
 		{
 			name: "Deleting IPRange should not be allowed",
 			request: &admv1.AdmissionRequest{
@@ -146,6 +242,32 @@ func TestEgressControllerValidateExternalIPPool(t *testing.T) {
 			},
 			expectedResponse: &admv1.AdmissionResponse{Allowed: true},
 		},
+		{
+			name: "Adding overlapping IPRange should not be allowed",
+			request: &admv1.AdmissionRequest{
+				Name:      "foo",
+				Operation: "UPDATE",
+				OldObject: runtime.RawExtension{Raw: marshal(testIPPool)},
+				Object: runtime.RawExtension{Raw: marshal(copyAndMutateIPPool(testIPPool, func(pool *crdv1alpha2.IPPool) {
+					pool.Spec.IPRanges = append(pool.Spec.IPRanges, crdv1alpha2.SubnetIPRange{
+						IPRange: crdv1alpha2.IPRange{
+							Start: "192.168.3.10",
+							End:   "192.168.3.30",
+						},
+						SubnetInfo: crdv1alpha2.SubnetInfo{
+							Gateway:      "192.168.3.1",
+							PrefixLength: 24,
+						},
+					})
+				}))},
+			},
+			expectedResponse: &admv1.AdmissionResponse{
+				Allowed: false,
+				Result: &metav1.Status{
+					Message: "IPRanges [192.168.3.10-192.168.3.30,192.168.3.10-192.168.3.20] overlap",
+				},
+			},
+		},
 		{
 			name: "Deleting IPPool in use should not be allowed",
 			request: &admv1.AdmissionRequest{