Add unprivileged port validation (#7034)

Author: pdabelf5

cmd/nginx-ingress/flags.go

@@ -9,6 +9,7 @@ import (
 	"regexp"
 	"strings"
 
+	internalValidation "github.com/nginxinc/kubernetes-ingress/internal/validation"
 	api_v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -345,22 +346,22 @@ func mustValidateFlags(ctx context.Context) {
 		nl.Fatalf(l, "Invalid value for leader-election-lock-name: %v", statusLockNameValidationError)
 	}
 
-	statusPortValidationError := validatePort(*nginxStatusPort)
+	statusPortValidationError := internalValidation.ValidateUnprivilegedPort(*nginxStatusPort)
 	if statusPortValidationError != nil {
 		nl.Fatalf(l, "Invalid value for nginx-status-port: %v", statusPortValidationError)
 	}
 
-	metricsPortValidationError := validatePort(*prometheusMetricsListenPort)
+	metricsPortValidationError := internalValidation.ValidateUnprivilegedPort(*prometheusMetricsListenPort)
 	if metricsPortValidationError != nil {
 		nl.Fatalf(l, "Invalid value for prometheus-metrics-listen-port: %v", metricsPortValidationError)
 	}
 
-	readyStatusPortValidationError := validatePort(*readyStatusPort)
+	readyStatusPortValidationError := internalValidation.ValidateUnprivilegedPort(*readyStatusPort)
 	if readyStatusPortValidationError != nil {
 		nl.Fatalf(l, "Invalid value for ready-status-port: %v", readyStatusPortValidationError)
 	}
 
-	healthProbePortValidationError := validatePort(*serviceInsightListenPort)
+	healthProbePortValidationError := internalValidation.ValidateUnprivilegedPort(*serviceInsightListenPort)
 	if healthProbePortValidationError != nil {
 		nl.Fatalf(l, "Invalid value for service-insight-listen-port: %v", metricsPortValidationError)
 	}
@@ -464,14 +465,6 @@ func validateResourceName(name string) error {
 	return nil
 }
 
-// validatePort makes sure a given port is inside the valid port range for its usage
-func validatePort(port int) error {
-	if port < 1024 || port > 65535 {
-		return fmt.Errorf("port outside of valid port range [1024 - 65535]: %v", port)
-	}
-	return nil
-}
-
 // validateLogLevel makes sure a given logLevel is one of the allowed values
 func validateLogLevel(logLevel string) error {
 	switch strings.ToLower(logLevel) {

cmd/nginx-ingress/flags_test.go

@@ -7,24 +7,6 @@ import (
 	"testing"
 )
 
-func TestValidatePort(t *testing.T) {
-	badPorts := []int{80, 443, 1, 1023, 65536}
-	for _, badPort := range badPorts {
-		err := validatePort(badPort)
-		if err == nil {
-			t.Errorf("Expected error for port %v\n", badPort)
-		}
-	}
-
-	goodPorts := []int{8080, 8081, 8082, 1024, 65535}
-	for _, goodPort := range goodPorts {
-		err := validatePort(goodPort)
-		if err != nil {
-			t.Errorf("Error for valid port:  %v err: %v\n", goodPort, err)
-		}
-	}
-}
-
 func TestParseNginxStatusAllowCIDRs(t *testing.T) {
 	badCIDRs := []struct {
 		input         string

internal/validation/validation.go

@@ -14,13 +14,17 @@ var (
 )
 
 // ValidatePort ensure port matches rfc6335 https://www.rfc-editor.org/rfc/rfc6335.html
-func ValidatePort(value string) error {
-	port, err := strconv.Atoi(value)
-	if err != nil {
-		return fmt.Errorf("error parsing port number: %w", err)
+func ValidatePort(value int) error {
+	if value > 65535 || value < 1 {
+		return fmt.Errorf("error parsing port: %d not a valid port number", value)
 	}
-	if port > 65535 || port < 1 {
-		return fmt.Errorf("error parsing port: %v not a valid port number", port)
+	return nil
+}
+
+// ValidateUnprivilegedPort ensure port is in the 1024-65535 range
+func ValidateUnprivilegedPort(value int) error {
+	if value > 65535 || value < 1023 {
+		return fmt.Errorf("port outside of valid port range [1024 - 65535]: %d", value)
 	}
 	return nil
 }
@@ -34,7 +38,11 @@ func ValidateHost(host string) error {
 	if validIPRegex.MatchString(host) || validDNSRegex.MatchString(host) || validHostnameRegex.MatchString(host) {
 		chunks := strings.Split(host, ":")
 		if len(chunks) > 1 {
-			err := ValidatePort(chunks[1])
+			port, err := strconv.Atoi(chunks[1])
+			if err != nil {
+				return err
+			}
+			err = ValidatePort(port)
 			if err != nil {
 				return fmt.Errorf("invalid port: %w", err)
 			}

internal/validation/validation_test.go

@@ -8,28 +8,42 @@ import (
 func TestValidatePort_IsValidOnValidInput(t *testing.T) {
 	t.Parallel()
 
-	ports := []string{"1", "65535"}
+	ports := []int{1, 65535}
 	for _, p := range ports {
 		if err := ValidatePort(p); err != nil {
 			t.Error(err)
 		}
 	}
 }
 
-func TestValidatePort_ErrorsOnInvalidString(t *testing.T) {
+func TestValidatePort_ErrorsOnInvalidRange(t *testing.T) {
 	t.Parallel()
 
-	if err := ValidatePort(""); err == nil {
-		t.Error("want error, got nil")
+	ports := []int{0, -1, 65536}
+	for _, p := range ports {
+		if err := ValidatePort(p); err == nil {
+			t.Error("want error, got nil")
+		}
 	}
 }
 
-func TestValidatePort_ErrorsOnInvalidRange(t *testing.T) {
+func TestValidateUnprivilegedPort_IsValidOnValidInput(t *testing.T) {
 	t.Parallel()
 
-	ports := []string{"0", "-1", "65536"}
+	ports := []int{1024, 65535}
 	for _, p := range ports {
-		if err := ValidatePort(p); err == nil {
+		if err := ValidateUnprivilegedPort(p); err != nil {
+			t.Error(err)
+		}
+	}
+}
+
+func TestValidateUnprivilegedPort_ErrorsOnInvalidRange(t *testing.T) {
+	t.Parallel()
+
+	ports := []int{0, -1, 80, 443, 65536}
+	for _, p := range ports {
+		if err := ValidateUnprivilegedPort(p); err == nil {
 			t.Error("want error, got nil")
 		}
 	}

pkg/apis/dos/validation/dos.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"net/url"
 	"regexp"
+	"strconv"
 	"strings"
 
 	internalValidation "github.com/nginxinc/kubernetes-ingress/internal/validation"
@@ -128,7 +129,11 @@ func validateAppProtectDosLogDest(dstAntn string) error {
 	}
 	if validIPRegex.MatchString(dstAntn) || validDNSRegex.MatchString(dstAntn) || validLocalhostRegex.MatchString(dstAntn) {
 		chunks := strings.Split(dstAntn, ":")
-		err := internalValidation.ValidatePort(chunks[1])
+		port, err := strconv.Atoi(chunks[1])
+		if err != nil {
+			return err
+		}
+		err = internalValidation.ValidatePort(port)
 		if err != nil {
 			return fmt.Errorf("invalid log destination: %w", err)
 		}