diff --git a/Makefile b/Makefile index 9ed73946a..0e078423d 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ all: all-container # Use the 0.0 tag for testing, it shouldn't clobber any release builds -TAG ?= 0.26.1 +TAG ?= 0.26.2 REGISTRY ?= quay.io/kubernetes-ingress-controller DOCKER ?= docker SED_I ?= sed -i @@ -77,7 +77,7 @@ export E2E_CHECK_LEAKS export SLOW_E2E_THRESHOLD # Set default base image dynamically for each arch -BASEIMAGE?=quay.io/kubernetes-ingress-controller/nginx-$(ARCH):870be3bcd88c267f14fd82da82303472f383cd14 +BASEIMAGE?=quay.io/kubernetes-ingress-controller/nginx-$(ARCH):daf8634acf839708722cffc67a62e9316a2771c6 ifeq ($(ARCH),arm) QEMUARCH=arm diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 79d130bf1..f611afb43 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -18,7 +18,9 @@ package template import ( "bytes" + "crypto/sha1" "encoding/base64" + "encoding/hex" "encoding/json" "fmt" "io/ioutil" @@ -164,6 +166,7 @@ var ( "isValidByteSize": isValidByteSize, "buildForwardedFor": buildForwardedFor, "buildAuthSignURL": buildAuthSignURL, + "buildAuthSignURLLocation": buildAuthSignURLLocation, "buildOpentracing": buildOpentracing, "proxySetHeader": proxySetHeader, "buildInfluxDB": buildInfluxDB, @@ -883,24 +886,25 @@ func buildForwardedFor(input interface{}) string { return fmt.Sprintf("$http_%v", ffh) } -func buildAuthSignURL(input interface{}) string { - s, ok := input.(string) - if !ok { - klog.Errorf("expected an 'string' type but %T was returned", input) - return "" - } - - u, _ := url.Parse(s) +func buildAuthSignURL(authSignURL string) string { + u, _ := url.Parse(authSignURL) q := u.Query() if len(q) == 0 { - return fmt.Sprintf("%v?rd=$pass_access_scheme://$http_host$escaped_request_uri", s) + return fmt.Sprintf("%v?rd=$pass_access_scheme://$http_host$escaped_request_uri", authSignURL) } if q.Get("rd") != "" { - return s + return authSignURL } - return fmt.Sprintf("%v&rd=$pass_access_scheme://$http_host$escaped_request_uri", s) + return fmt.Sprintf("%v&rd=$pass_access_scheme://$http_host$escaped_request_uri", authSignURL) +} + +func buildAuthSignURLLocation(location, authSignURL string) string { + hasher := sha1.New() + hasher.Write([]byte(location)) + hasher.Write([]byte(authSignURL)) + return "@" + hex.EncodeToString(hasher.Sum(nil)) } var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 062da89b8..0d66f75d3 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -763,14 +763,6 @@ func TestFilterRateLimits(t *testing.T) { } func TestBuildAuthSignURL(t *testing.T) { - invalidType := &ingress.Ingress{} - expected := "" - actual := buildAuthSignURL(invalidType) - - if expected != actual { - t.Errorf("Expected '%v' but returned '%v'", expected, actual) - } - cases := map[string]struct { Input, Output string }{ diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 95f72b19a..c6ae5a373 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -952,6 +952,14 @@ stream { {{ end }} + {{ if $externalAuth.SigninURL }} + location {{ buildAuthSignURLLocation $location.Path $externalAuth.SigninURL }} { + internal; + + return 302 {{ buildAuthSignURL $externalAuth.SigninURL }}; + } + {{ end }} + location {{ $path }} { {{ $ing := (getIngressInformation $location.Ingress $server.Hostname $location.Path) }} set $namespace {{ $ing.Namespace | quote}}; @@ -1071,7 +1079,7 @@ stream { {{ if $externalAuth.SigninURL }} set_escape_uri $escaped_request_uri $request_uri; - error_page 401 = {{ buildAuthSignURL $externalAuth.SigninURL }}; + error_page 401 = {{ buildAuthSignURLLocation $location.Path $externalAuth.SigninURL }}; {{ end }} {{ if $location.BasicDigestAuth.Secured }} diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index e06876c50..8ee9cb881 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -38,6 +38,7 @@ import ( _ "k8s.io/ingress-nginx/test/e2e/leaks" _ "k8s.io/ingress-nginx/test/e2e/loadbalance" _ "k8s.io/ingress-nginx/test/e2e/lua" + _ "k8s.io/ingress-nginx/test/e2e/security" _ "k8s.io/ingress-nginx/test/e2e/servicebackend" _ "k8s.io/ingress-nginx/test/e2e/settings" _ "k8s.io/ingress-nginx/test/e2e/ssl" diff --git a/test/e2e/security/request_smuggling.go b/test/e2e/security/request_smuggling.go new file mode 100644 index 000000000..2b80a77c1 --- /dev/null +++ b/test/e2e/security/request_smuggling.go @@ -0,0 +1,99 @@ +/* +Copyright 2019 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 ( + "bufio" + "fmt" + "net" + "strings" + "time" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Request smuggling", func() { + f := framework.NewDefaultFramework("request-smuggling") + + BeforeEach(func() { + f.NewEchoDeployment() + }) + + AfterEach(func() { + }) + + It("should not return body content from error_page", func() { + host := "foo.bar.com" + + snippet := ` +server { + listen 80; + server_name notlocalhost; + location /_hidden/index.html { + return 200 'This should be hidden!'; + } +}` + + f.UpdateNginxConfigMapData("http-snippet", snippet) + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, map[string]string{ + "nginx.ingress.kubernetes.io/auth-signin": "https://httpbin.org/uuid", + "nginx.ingress.kubernetes.io/auth-url": "https://httpbin.org/basic-auth/user/passwd", + }) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, fmt.Sprintf("server_name %v", host)) + }) + + out, err := smugglingRequest(host, f.GetNginxIP(), 80) + Expect(err).NotTo(HaveOccurred(), "obtaining response of request smuggling check") + Expect(out).ShouldNot(ContainSubstring("This should be hidden!")) + }) +}) + +func smugglingRequest(host, addr string, port int) (string, error) { + conn, err := net.Dial("tcp", fmt.Sprintf("%v:%v", addr, port)) + if err != nil { + return "", err + } + + defer conn.Close() + + conn.SetDeadline(time.Now().Add(time.Second * 10)) + + _, err = fmt.Fprintf(conn, "GET /echo HTTP/1.1\r\nHost: %v\r\nContent-Length: 56\r\n\r\nGET /_hidden/index.html HTTP/1.1\r\nHost: notlocalhost\r\n\r\n", host) + if err != nil { + return "", err + } + + // wait for /_hidden/index.html response + time.Sleep(1 * time.Second) + + var buf = make([]byte, 1024) + r := bufio.NewReader(conn) + _, err = r.Read(buf) + if err != nil { + return "", err + } + + return string(buf), nil +}