DevFW-CICD/ingress-nginx-helm
12
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Controller: Several security fixes. (#13069)

Browse source 
Co-authored-by: Tabitha Sable <tabitha.c.sable@gmail.com>
This commit is contained in:
Marco Ebert 2025-03-25 00:00:39 +01:00 committed by GitHub
parent cfd4d89a56
commit 626305229f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 36 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
internal/ingress/annotations/auth/main.go
View file

@ -19,6 +19,7 @@ package auth
import (
"fmt"
"os"
"path/filepath"
"regexp"
"strings"
@ -203,16 +204,24 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) {
return nil, err
}
passFilename := fmt.Sprintf("%v/%v-%v-%v.passwd", a.authDirectory, ing.GetNamespace(), ing.UID, secret.UID)
passFileName := fmt.Sprintf("%v-%v-%v.passwd", ing.GetNamespace(), ing.UID, secret.UID)
passFilePath := filepath.Join(a.authDirectory, passFileName)
// Ensure password file name does not contain any path traversal characters.
if a.authDirectory != filepath.Dir(passFilePath) || passFileName != filepath.Base(passFilePath) {
return nil, ing_errors.LocationDeniedError{
Reason: fmt.Errorf("invalid password file name: %s", passFileName),
}
}
switch secretType {
case fileAuth:
err = dumpSecretAuthFile(passFilename, secret)
err = dumpSecretAuthFile(passFilePath, secret)
if err != nil {
return nil, err
}
case mapAuth:
err = dumpSecretAuthMap(passFilename, secret)
err = dumpSecretAuthMap(passFilePath, secret)
if err != nil {
return nil, err
}
@ -225,9 +234,9 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) {
return &Config{
Type: at,
Realm: realm,
File: passFilename,
File: passFilePath,
Secured: true,
FileSHA: file.SHA1(passFilename),
FileSHA: file.SHA1(passFilePath),
Secret: name,
SecretType: secretType,
}, nil

4
internal/ingress/controller/controller.go
View file

@ -420,11 +420,15 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error {
return err
}
/* Deactivated to mitigate CVE-2025-1974
// TODO: Implement sandboxing so this test can be done safely
err = n.testTemplate(content)
if err != nil {
n.metricCollector.IncCheckErrorCount(ing.ObjectMeta.Namespace, ing.Name)
return err
}
*/
n.metricCollector.IncCheckCount(ing.ObjectMeta.Namespace, ing.Name)
endCheck := time.Now().UnixNano() / 1000000
n.metricCollector.SetAdmissionMetrics(

3
internal/ingress/controller/controller_test.go
View file

@ -250,6 +250,8 @@ func TestCheckIngress(t *testing.T) {
}
})
/* Deactivated to mitigate CVE-2025-1974
// TODO: Implement sandboxing so this test can be done safely
t.Run("When nginx test returns an error", func(t *testing.T) {
nginx.command = testNginxTestCommand{
t: t,
@ -261,6 +263,7 @@ func TestCheckIngress(t *testing.T) {
t.Errorf("with a new ingress with an error, an error should be returned")
}
})
*/
t.Run("When the default annotation prefix is used despite an override", func(t *testing.T) {
defer func() {

6
internal/ingress/controller/template/template.go
View file

@ -1627,11 +1627,11 @@ func buildMirrorLocations(locs []*ingress.Location) string {
mapped.Insert(loc.Mirror.Source)
buffer.WriteString(fmt.Sprintf(`location = %v {
internal;
proxy_set_header Host "%v";
proxy_pass "%v";
proxy_set_header Host %v;
proxy_pass %v;
}
`, loc.Mirror.Source, loc.Mirror.Host, loc.Mirror.Target))
`, strconv.Quote(loc.Mirror.Source), strconv.Quote(loc.Mirror.Host), strconv.Quote(loc.Mirror.Target)))
}
return buffer.String()

6
rootfs/etc/nginx/template/nginx.tmpl
View file

@ -875,7 +875,7 @@ stream {
{{ if not ( empty $server.CertificateAuth.MatchCN ) }}
{{ if gt (len $server.CertificateAuth.MatchCN) 0 }}
if ( $ssl_client_s_dn !~ {{ $server.CertificateAuth.MatchCN }} ) {
if ( $ssl_client_s_dn !~ {{ $server.CertificateAuth.MatchCN | quote }} ) {
return 403 "client certificate unauthorized";
}
{{ end }}
@ -1077,7 +1077,7 @@ stream {
set $target {{ changeHostPort $externalAuth.URL $authUpstreamName }};
{{ else }}
proxy_http_version {{ $location.Proxy.ProxyHTTPVersion }};
set $target {{ $externalAuth.URL }};
set $target {{ $externalAuth.URL | quote }};
{{ end }}
proxy_pass $target;
}
@ -1115,7 +1115,7 @@ stream {
{{ buildOpentelemetryForLocation $all.Cfg.EnableOpentelemetry $all.Cfg.OpentelemetryTrustIncomingSpan $location }}
{{ if $location.Mirror.Source }}
mirror {{ $location.Mirror.Source }};
mirror {{ $location.Mirror.Source | quote }};
mirror_request_body {{ $location.Mirror.RequestBody }};
{{ end }}

7
test/e2e/admission/admission.go
View file

@ -26,7 +26,6 @@ import (
"github.com/onsi/ginkgo/v2"
"github.com/stretchr/testify/assert"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/test/e2e/framework"
@ -99,6 +98,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid path should return an error")
})
/* Deactivated to mitigate CVE-2025-1974
// TODO: Implement sandboxing so this test can be done safely
ginkgo.It("should return an error if there is an error validating the ingress definition", func() {
disableSnippet := f.AllowSnippetConfiguration()
defer disableSnippet()
@ -112,6 +113,7 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
_, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error")
})
*/
ginkgo.It("should return an error if there is an invalid value in some annotation", func() {
host := admissionTestHost
@ -207,6 +209,8 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
Status(http.StatusOK)
})
/* Deactivated to mitigate CVE-2025-1974
// TODO: Implement sandboxing so this test can be done safely
ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() {
disableSnippet := f.AllowSnippetConfiguration()
defer disableSnippet()
@ -220,6 +224,7 @@ var _ = framework.IngressNginxDescribeSerial("[Admission] admission controller",
assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error")
}
})
*/
ginkgo.It("should not return an error for an invalid Ingress when it has unknown class", func() {
disableSnippet := f.AllowSnippetConfiguration()

6
test/e2e/annotations/mirror.go
View file

@ -43,7 +43,7 @@ var _ = framework.DescribeAnnotation("mirror-*", func() {
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) &&
return strings.Contains(server, fmt.Sprintf("mirror \"/_mirror-%v\";", ing.UID)) &&
strings.Contains(server, "mirror_request_body on;")
})
})
@ -58,7 +58,7 @@ var _ = framework.DescribeAnnotation("mirror-*", func() {
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) &&
return strings.Contains(server, fmt.Sprintf("mirror \"/_mirror-%v\";", ing.UID)) &&
strings.Contains(server, "mirror_request_body on;") &&
strings.Contains(server, `proxy_pass "https://test.env.com/$request_uri";`)
})
@ -75,7 +75,7 @@ var _ = framework.DescribeAnnotation("mirror-*", func() {
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, fmt.Sprintf("mirror /_mirror-%v;", ing.UID)) &&
return strings.Contains(server, fmt.Sprintf("mirror \"/_mirror-%v\";", ing.UID)) &&
strings.Contains(server, "mirror_request_body off;")
})
})