From 4a3e136151fbaf52a2b46e3deaf594b891dcacb5 Mon Sep 17 00:00:00 2001 From: Paul Abel Date: Fri, 13 Dec 2024 14:52:49 +0000 Subject: [PATCH] update hostname/fqdn/ip validation --- internal/validation/validation.go | 20 +++-- internal/validation/validation_test.go | 24 ++++-- pkg/apis/dos/validation/dos.go | 27 +++++-- pkg/apis/dos/validation/dos_test.go | 103 ++----------------------- 4 files changed, 53 insertions(+), 121 deletions(-) diff --git a/internal/validation/validation.go b/internal/validation/validation.go index f7c6953f8..bfeaaef45 100644 --- a/internal/validation/validation.go +++ b/internal/validation/validation.go @@ -8,11 +8,12 @@ import ( ) var ( - validDNSRegex = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,6}(?::\d{1,5})?$`) - validIPRegex = regexp.MustCompile(`^(\d{1,3}\.){3}\d{1,3}(?::\d{1,5})?$`) - validLocalhostRegex = regexp.MustCompile(`^localhost(?::\d{1,5})?$`) + validDNSRegex = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,6}(?::\d{1,5})?$`) + validIPRegex = regexp.MustCompile(`^(?:(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])(?::\d{1,5})?$`) + validHostnameRegex = regexp.MustCompile(`^[a-z][A-Za-z0-9-]{1,62}(?::\d{1,5})?$`) ) +// 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 { @@ -24,18 +25,21 @@ func ValidatePort(value string) error { return nil } +// ValidateHost ensures the host is a valid hostname/IP address or FQDN with an optional :port appended func ValidateHost(host string) error { if host == "" { return fmt.Errorf("error parsing host: empty host") } - if validIPRegex.MatchString(host) || validDNSRegex.MatchString(host) || validLocalhostRegex.MatchString(host) { + if validIPRegex.MatchString(host) || validDNSRegex.MatchString(host) || validHostnameRegex.MatchString(host) { chunks := strings.Split(host, ":") - err := ValidatePort(chunks[1]) - if err != nil { - return fmt.Errorf("invalid port: %w", err) + if len(chunks) > 1 { + err := ValidatePort(chunks[1]) + if err != nil { + return fmt.Errorf("invalid port: %w", err) + } } return nil } - return fmt.Errorf("invalid host: %s", host) + return fmt.Errorf("error parsing host: %s not a valid host", host) } diff --git a/internal/validation/validation_test.go b/internal/validation/validation_test.go index d64485668..9ed4cf6b2 100644 --- a/internal/validation/validation_test.go +++ b/internal/validation/validation_test.go @@ -39,20 +39,28 @@ func TestValidateHost(t *testing.T) { t.Parallel() // Positive test cases posHosts := []string{ - "10.10.1.1:514", - "localhost:514", - "dns.test.svc.cluster.local:514", - "cluster.local:514", - "dash-test.cluster.local:514", + "10.10.1.1:443", + "10.10.1.1", + "123.112.224.43:443", + "172.120.3.222", + "localhost:80", + "localhost", + "myhost:54321", + "myhost", + "my-host:54321", + "my-host", + "dns.test.svc.cluster.local:8443", + "cluster.local:8443", "product.example.com", + "product.example.com:443", } // Negative test cases item, expected error message negHosts := [][]string{ - {"NotValid", "invalid host: NotValid"}, - {"cluster.local", "invalid host: cluster.local"}, - {"-cluster.local:514", "invalid host: -cluster.local:514"}, + {"NotValid", "not a valid host"}, + {"-cluster.local:514", "not a valid host"}, {"10.10.1.1:99999", "not a valid port number"}, + {"333.333.333.333", "not a valid host"}, } for _, tCase := range posHosts { diff --git a/pkg/apis/dos/validation/dos.go b/pkg/apis/dos/validation/dos.go index 7274e03fd..9f3970fc3 100644 --- a/pkg/apis/dos/validation/dos.go +++ b/pkg/apis/dos/validation/dos.go @@ -2,14 +2,17 @@ package validation import ( "fmt" - k8svalidation "github.com/nginxinc/kubernetes-ingress/internal/validation" + "net" + "net/url" + "regexp" + "strings" + + internalValidation "github.com/nginxinc/kubernetes-ingress/internal/validation" validation2 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/validation" "github.com/nginxinc/kubernetes-ingress/pkg/apis/dos/v1beta1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" - "net" - "net/url" ) var appProtectDosPolicyRequiredFields = [][]string{ @@ -113,15 +116,25 @@ func ValidateAppProtectDosLogConf(logConf *unstructured.Unstructured) (string, e return warning, nil } +var ( + validDNSRegex = regexp.MustCompile(`^([A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,6}:\d{1,5}$`) + validIPRegex = regexp.MustCompile(`^(\d{1,3}\.){3}\d{1,3}:\d{1,5}$`) + validLocalhostRegex = regexp.MustCompile(`^localhost:\d{1,5}$`) +) + func validateAppProtectDosLogDest(dstAntn string) error { if dstAntn == "stderr" { return nil } - err := k8svalidation.ValidateHost(dstAntn) - if err != nil { - return fmt.Errorf("%w, must follow format: : or stderr", err) + if validIPRegex.MatchString(dstAntn) || validDNSRegex.MatchString(dstAntn) || validLocalhostRegex.MatchString(dstAntn) { + chunks := strings.Split(dstAntn, ":") + err := internalValidation.ValidatePort(chunks[1]) + if err != nil { + return fmt.Errorf("invalid log destination: %w", err) + } + return nil } - return nil + return fmt.Errorf("invalid log destination: %s, must follow format: : or stderr", dstAntn) } func validateAppProtectDosName(name string) error { diff --git a/pkg/apis/dos/validation/dos_test.go b/pkg/apis/dos/validation/dos_test.go index ba2bbecae..b70dcd921 100644 --- a/pkg/apis/dos/validation/dos_test.go +++ b/pkg/apis/dos/validation/dos_test.go @@ -63,7 +63,7 @@ func TestValidateDosProtectedResource(t *testing.T) { DosAccessLogDest: "bad&$%^logdest", }, }, - expectErr: "error validating DosProtectedResource: invalid field: dosAccessLogDest err: invalid host: bad&$%^logdest, must follow format: : or stderr", + expectErr: "error validating DosProtectedResource: invalid field: dosAccessLogDest err: invalid log destination: bad&$%^logdest, must follow format: : or stderr", msg: "invalid DosAccessLogDest specified", }, { @@ -105,7 +105,7 @@ func TestValidateDosProtectedResource(t *testing.T) { DosSecurityLog: &v1beta1.DosSecurityLog{}, }, }, - expectErr: "error validating DosProtectedResource: invalid field: dosSecurityLog/dosLogDest err: error parsing host: empty host, must follow format: : or stderr", + expectErr: "error validating DosProtectedResource: invalid field: dosSecurityLog/dosLogDest err: invalid log destination: , must follow format: : or stderr", msg: "empty DosSecurityLog specified", }, { @@ -159,7 +159,6 @@ func TestValidateDosProtectedResource(t *testing.T) { msg: "DosSecurityLog with valid apDosLogConf", }, } - for _, test := range tests { err := ValidateDosProtectedResource(test.protected) if err != nil { @@ -191,9 +190,9 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) { // Negative test cases item, expected error message negDstAntns := [][]string{ - {"NotValid", "invalid host: NotValid, must follow format: : or stderr"}, - {"cluster.local", "invalid host: cluster.local, must follow format: : or stderr"}, - {"-cluster.local:514", "invalid host: -cluster.local:514, must follow format: : or stderr"}, + {"NotValid", "invalid log destination: NotValid, must follow format: : or stderr"}, + {"cluster.local", "invalid log destination: cluster.local, must follow format: : or stderr"}, + {"-cluster.local:514", "invalid log destination: -cluster.local:514, must follow format: : or stderr"}, {"10.10.1.1:99999", "not a valid port number"}, } @@ -203,7 +202,6 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) { t.Errorf("expected nil, got %v", err) } } - for _, nTCase := range negDstAntns { err := validateAppProtectDosLogDest(nTCase[0]) if err == nil { @@ -216,97 +214,6 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) { } } -func TestValidateAppProtectDosLogConf(t *testing.T) { - t.Parallel() - tests := []struct { - logConf *unstructured.Unstructured - expectErr bool - expectWarn bool - msg string - }{ - { - logConf: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "filter": map[string]interface{}{}, - }, - }, - }, - expectErr: false, - expectWarn: false, - msg: "valid log conf", - }, - { - logConf: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{}, - }, - }, - expectErr: true, - expectWarn: false, - msg: "invalid log conf with no filter field", - }, - { - logConf: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "something": map[string]interface{}{ - "filter": map[string]interface{}{}, - }, - }, - }, - expectErr: true, - expectWarn: false, - msg: "invalid log conf with no spec field", - }, - { - logConf: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "content": map[string]interface{}{ - "format": "user-defined", - }, - "filter": map[string]interface{}{}, - }, - }, - }, - expectErr: false, - expectWarn: true, - msg: "Support only splunk format", - }, - { - logConf: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "spec": map[string]interface{}{ - "filter": map[string]interface{}{}, - "content": map[string]interface{}{ - "format": "user-defined", - }, - }, - }, - }, - expectErr: false, - expectWarn: true, - msg: "valid log conf with warning filter field", - }, - } - - for _, test := range tests { - warn, err := ValidateAppProtectDosLogConf(test.logConf) - if test.expectErr && err == nil { - t.Errorf("validateAppProtectDosLogConf() returned no error for the case of %s", test.msg) - } - if !test.expectErr && err != nil { - t.Errorf("validateAppProtectDosLogConf() returned unexpected error %v for the case of %s", err, test.msg) - } - if test.expectWarn && warn == "" { - t.Errorf("validateAppProtectDosLogConf() returned no warning for the case of %s", test.msg) - } - if !test.expectWarn && warn != "" { - t.Errorf("validateAppProtectDosLogConf() returned unexpected warning: %s, for the case of %s", warn, test.msg) - } - } -} - func TestValidateAppProtectDosPolicy(t *testing.T) { t.Parallel() tests := []struct {