remove tests and regex path checks (#9626)

Signed-off-by: James Strong <strong.james.e@gmail.com>
This commit is contained in:
James Strong 2023-02-14 21:33:52 +01:00 committed by GitHub
parent bbc8bd4bce
commit 5e85747db7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 0 additions and 252 deletions

1
go.mod
View file

@ -77,7 +77,6 @@ require (
github.com/inconshreveable/mousetrap v1.0.1 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
github.com/magefile/mage v1.14.0 // indirect
github.com/mailru/easyjson v0.7.6 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect

2
go.sum
View file

@ -235,8 +235,6 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0=
github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE=
github.com/magefile/mage v1.14.0 h1:6QDX3g6z1YvJ4olPhT1wksUcSa/V0a1B+pJb73fBjyo=
github.com/magefile/mage v1.14.0/go.mod h1:z5UZb/iS3GoOSn0JgWuiw7dxlurVYTu+/jHXqQg881A=
github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
github.com/mailru/easyjson v0.7.6 h1:8yTIVnZgCoiM1TgqoeTl+LfU5Jg6/xL3QhGQnimLYnA=

View file

@ -59,7 +59,6 @@ import (
"k8s.io/ingress-nginx/internal/ingress/resolver"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/pkg/apis/ingress"
ingressutils "k8s.io/ingress-nginx/pkg/util/ingress"
)
// IngressFilterFunc decides if an Ingress should be omitted or not
@ -865,10 +864,6 @@ func (s *k8sStore) syncIngress(ing *networkingv1.Ingress) {
if path.Path == "" {
copyIng.Spec.Rules[ri].HTTP.Paths[pi].Path = "/"
}
if !ingressutils.IsSafePath(copyIng, path.Path) {
klog.Warningf("ingress %s contains invalid path %s", key, path.Path)
return
}
}
}

View file

@ -18,30 +18,15 @@ package ingress
import (
"fmt"
"regexp"
"strings"
networkingv1 "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/internal/net/ssl"
"k8s.io/ingress-nginx/pkg/apis/ingress"
"k8s.io/klog/v2"
)
const (
alphaNumericChars = `\-\.\_\~a-zA-Z0-9/`
regexEnabledChars = `\^\$\[\]\(\)\{\}\*\+`
)
var (
// pathAlphaNumeric is a regex validation of something like "^/[a-zA-Z]+$" on path
pathAlphaNumeric = regexp.MustCompile("^/[" + alphaNumericChars + "]*$").MatchString
// pathRegexEnabled is a regex validation of paths that may contain regex.
pathRegexEnabled = regexp.MustCompile("^/[" + alphaNumericChars + regexEnabledChars + "]*$").MatchString
)
func GetRemovedHosts(rucfg, newcfg *ingress.Configuration) []string {
oldSet := sets.NewString()
newSet := sets.NewString()
@ -246,13 +231,3 @@ func BuildRedirects(servers []*ingress.Server) []*redirect {
return redirectServers
}
// IsSafePath verifies if the path used in ingress object contains only valid characters.
// It will behave differently if regex is enabled or not
func IsSafePath(copyIng *networkingv1.Ingress, path string) bool {
isRegex, _ := parser.GetBoolAnnotation("use-regex", copyIng)
if isRegex {
return pathRegexEnabled(path)
}
return pathAlphaNumeric(path)
}

View file

@ -17,13 +17,8 @@ limitations under the License.
package ingress
import (
"fmt"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
networkingv1 "k8s.io/api/networking/v1"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/pkg/apis/ingress"
)
@ -135,83 +130,3 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
t.Errorf("Expected new config to not change")
}
}
func generateDumbIngressforPathTest(regexEnabled bool) *networkingv1.Ingress {
var annotations = make(map[string]string)
regexAnnotation := fmt.Sprintf("%s/use-regex", parser.AnnotationsPrefix)
if regexEnabled {
annotations[regexAnnotation] = "true"
}
return &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "dumb",
Namespace: "default",
Annotations: annotations,
},
}
}
func TestIsSafePath(t *testing.T) {
tests := []struct {
name string
copyIng *networkingv1.Ingress
path string
want bool
}{
{
name: "should accept valid path with regex disabled",
want: true,
copyIng: generateDumbIngressforPathTest(false),
path: "/xpto/~user/t-e_st.exe",
},
{
name: "should accept valid path / with regex disabled",
want: true,
copyIng: generateDumbIngressforPathTest(false),
path: "/",
},
{
name: "should reject invalid path with invalid chars",
want: false,
copyIng: generateDumbIngressforPathTest(false),
path: "/foo/bar/;xpto",
},
{
name: "should reject regex path when regex is disabled",
want: false,
copyIng: generateDumbIngressforPathTest(false),
path: "/foo/bar/(.+)",
},
{
name: "should accept valid path / with regex enabled",
want: true,
copyIng: generateDumbIngressforPathTest(true),
path: "/",
},
{
name: "should accept regex path when regex is enabled",
want: true,
copyIng: generateDumbIngressforPathTest(true),
path: "/foo/bar/(.+)",
},
{
name: "should reject regex path when regex is enabled but the path is invalid",
want: false,
copyIng: generateDumbIngressforPathTest(true),
path: "/foo/bar/;xpto",
},
{
name: "should reject regex path when regex is enabled but the path is invalid",
want: false,
copyIng: generateDumbIngressforPathTest(true),
path: ";xpto",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := IsSafePath(tt.copyIng, tt.path); got != tt.want {
t.Errorf("IsSafePath() = %v, want %v", got, tt.want)
}
})
}
}

View file

@ -1,134 +0,0 @@
/*
Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package security
import (
"fmt"
"net/http"
"strings"
"github.com/onsi/ginkgo/v2"
"k8s.io/ingress-nginx/test/e2e/framework"
)
const (
validPath = "/xpto/~user/t-e_st.exe"
invalidPath = "/foo/bar/;xpto"
regexPath = "/foo/bar/(.+)"
host = "securitytest.com"
)
var (
annotationRegex = map[string]string{
"nginx.ingress.kubernetes.io/use-regex": "true",
}
)
var _ = framework.IngressNginxDescribe("[Security] validate path fields", func() {
f := framework.NewDefaultFramework("validate-path")
ginkgo.BeforeEach(func() {
f.NewEchoDeployment()
})
ginkgo.It("should accept an ingress with valid path", func() {
ing := framework.NewSingleIngress(host, validPath, host, f.Namespace, framework.EchoService, 80, nil)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("server_name %s ;", host))
})
f.HTTPTestClient().
GET(validPath).
WithHeader("Host", host).
Expect().
Status(http.StatusOK)
})
ginkgo.It("should drop an ingress with invalid path", func() {
ing := framework.NewSingleIngress(host, invalidPath, host, f.Namespace, framework.EchoService, 80, nil)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return !strings.Contains(server, fmt.Sprintf("server_name %s ;", host))
})
f.HTTPTestClient().
GET(invalidPath).
WithHeader("Host", host).
Expect().
Status(http.StatusNotFound)
})
ginkgo.It("should drop an ingress with regex path and regex disabled", func() {
ing := framework.NewSingleIngress(host, regexPath, host, f.Namespace, framework.EchoService, 80, nil)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return !strings.Contains(server, fmt.Sprintf("server_name %s ;", host))
})
f.HTTPTestClient().
GET("/foo/bar/lalala").
WithHeader("Host", host).
Expect().
Status(http.StatusNotFound)
})
ginkgo.It("should accept an ingress with regex path and regex enabled", func() {
ing := framework.NewSingleIngress(host, regexPath, host, f.Namespace, framework.EchoService, 80, annotationRegex)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("server_name %s ;", host))
})
f.HTTPTestClient().
GET("/foo/bar/lalala").
WithHeader("Host", host).
Expect().
Status(http.StatusOK)
})
ginkgo.It("should reject an ingress with invalid path and regex enabled", func() {
ing := framework.NewSingleIngress(host, invalidPath, host, f.Namespace, framework.EchoService, 80, annotationRegex)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return !strings.Contains(server, fmt.Sprintf("server_name %s ;", host))
})
f.HTTPTestClient().
GET(invalidPath).
WithHeader("Host", host).
Expect().
Status(http.StatusNotFound)
})
})