fix: NPM ipv6/CIDR validation

npm/pkg/dataplane/ipsets/ipset.go

@@ -3,12 +3,9 @@ package ipsets
 import (
 	"errors"
 	"fmt"
-	"net"
-	"strings"
 
 	"github.com/Azure/azure-container-networking/log"
 	"github.com/Azure/azure-container-networking/npm/util"
-	npmerrors "github.com/Azure/azure-container-networking/npm/util/errors"
 )
 
 type IPSetMetadata struct {
@@ -397,18 +394,6 @@ func (set *IPSet) canSetBeSelectorIPSet() bool {
 		set.Type == NestedLabelOfPod)
 }
 
-// TODO: This is an adhoc approach for linux, but need to refactor data structure for better management.
-func ValidateIPBlock(ipblock string) error {
-	// TODO: This is fragile code with strong dependency with " "(space).
-	// onlyCidr has only cidr without "space" and "nomatch" in case except ipblock to validate cidr format.
-	onlyCidr := strings.Split(ipblock, " ")[0]
-	_, _, err := net.ParseCIDR(onlyCidr)
-	if err != nil {
-		return npmerrors.SimpleErrorWrapper("failed to parse CIDR", err)
-	}
-	return nil
-}
-
 func GetMembersOfTranslatedSets(members []string) []*IPSetMetadata {
 	memberList := make([]*IPSetMetadata, len(members))
 	i := 0

npm/pkg/dataplane/ipsets/ipsetmanager.go

@@ -554,10 +554,7 @@ func validateIPSetMemberIP(ip string) bool {
 	// 192.168.0.0/24 nomatch
 	// always guaranteed to have ip, not guaranteed to have port + protocol
 	ipDetails := strings.Split(ip, ",")
-	if util.IsIPV4(ipDetails[0]) {
-		return true
-	}
+	ipField := strings.Split(ipDetails[0], " ")
 
-	err := ValidateIPBlock(ip)
-	return err == nil
+	return util.IsIPV4(ipField[0])
 }

npm/pkg/dataplane/ipsets/ipsetmanager_test.go

@@ -1357,48 +1357,84 @@ func TestMain(m *testing.M) {
 	os.Exit(exitCode)
 }
 
-func TestValidateIPBlock(t *testing.T) {
+func TestValidateIPSetMemberIP(t *testing.T) {
 	tests := []struct {
 		name    string
 		ipblock string
-		wantErr bool
+		want    bool
 	}{
 		{
 			name:    "cidr",
 			ipblock: "172.17.0.0/16",
-			wantErr: false,
+			want:    true,
 		},
 		{
 			name:    "except ipblock",
 			ipblock: "172.17.1.0/24 nomatch",
-			wantErr: false,
+			want:    true,
 		},
 		{
 			name:    "incorrect ip format",
 			ipblock: "1234",
-			wantErr: true,
+			want:    false,
 		},
 		{
 			name:    "incorrect ip range",
 			ipblock: "256.1.2.3",
-			wantErr: true,
+			want:    false,
 		},
 		{
 			name:    "empty cidr",
 			ipblock: "",
-			wantErr: true,
+			want:    false,
+		},
+		{
+			name:    "ipv6",
+			ipblock: "2345:0425:2CA1:0000:0000:0567:5673:23b5/24",
+			want:    false,
+		},
+		{
+			name:    "tcp",
+			ipblock: "192.168.0.0/24,tcp:25227",
+			want:    true,
+		},
+		{
+			name:    "valid ip no cidr",
+			ipblock: "10.0.0.0",
+			want:    true,
+		},
+		{
+			name:    "invalid cidr",
+			ipblock: "10.0.0.1/33",
+			want:    false,
+		},
+		{
+			name:    "valid ip nomatch",
+			ipblock: "192.168.0.1 nomatch",
+			want:    true,
+		},
+		{
+			name:    "valid ip tcp",
+			ipblock: "192.168.0.1,tcp:25227",
+			want:    true,
+		},
+		{
+			name:    "ipv6 tcp",
+			ipblock: "2345:0425:2CA1:0000:0000:0567:5673:23b5/24,tcp:25227",
+			want:    false,
+		},
+		{
+			name:    "ipv6 nomatch",
+			ipblock: "2345:0425:2CA1:0000:0000:0567:5673:23b5 nomatch",
+			want:    false,
 		},
 	}
 
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			err := ValidateIPBlock(tt.ipblock)
-			if tt.wantErr {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-			}
+			got := validateIPSetMemberIP(tt.ipblock)
+			require.Equal(t, tt.want, got)
 		})
 	}
 }

npm/util/util.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"hash/fnv"
 	"net"
+	"net/netip"
 	"os"
 	"regexp"
 	"runtime"
@@ -355,10 +356,17 @@ func SliceToString(list []string) string {
 }
 
 func IsIPV4(ip string) bool {
-	if net.ParseIP(ip).To4() != nil {
-		return true
+	isIPBlock := strings.Contains(ip, "/")
+	ipOnly := strings.Split(ip, "/")
+	address, err := netip.ParseAddr(ipOnly[0])
+	if err != nil {
+		return false
+	}
+
+	if address.Is4() && isIPBlock {
+		_, _, err := net.ParseCIDR(ip)
+		return err == nil
 	}
 
-	_, _, err := net.ParseCIDR(ip)
-	return err == nil
+	return address.Is4()
 }