Skip to content

Commit

Permalink
update hostname/fqdn/ip validation
Browse files Browse the repository at this point in the history
  • Loading branch information
pdabelf5 committed Dec 13, 2024
1 parent 95672a3 commit 4a3e136
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 121 deletions.
20 changes: 12 additions & 8 deletions internal/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
24 changes: 16 additions & 8 deletions internal/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 20 additions & 7 deletions pkg/apis/dos/validation/dos.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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: <ip-address | localhost | dns name>:<port> 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: <ip-address | localhost | dns name>:<port> or stderr", dstAntn)
}

func validateAppProtectDosName(name string) error {
Expand Down
103 changes: 5 additions & 98 deletions pkg/apis/dos/validation/dos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: <ip-address | localhost | dns name>:<port> or stderr",
expectErr: "error validating DosProtectedResource: invalid field: dosAccessLogDest err: invalid log destination: bad&$%^logdest, must follow format: <ip-address | localhost | dns name>:<port> or stderr",
msg: "invalid DosAccessLogDest specified",
},
{
Expand Down Expand Up @@ -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: <ip-address | localhost | dns name>:<port> or stderr",
expectErr: "error validating DosProtectedResource: invalid field: dosSecurityLog/dosLogDest err: invalid log destination: , must follow format: <ip-address | localhost | dns name>:<port> or stderr",
msg: "empty DosSecurityLog specified",
},
{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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: <ip-address | localhost | dns name>:<port> or stderr"},
{"cluster.local", "invalid host: cluster.local, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
{"-cluster.local:514", "invalid host: -cluster.local:514, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
{"NotValid", "invalid log destination: NotValid, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
{"cluster.local", "invalid log destination: cluster.local, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
{"-cluster.local:514", "invalid log destination: -cluster.local:514, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
{"10.10.1.1:99999", "not a valid port number"},
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 4a3e136

Please sign in to comment.