From 8b6e4d4697847f671f027d0a9f227b78a59c4dc5 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Wed, 20 Feb 2019 16:34:51 -0300 Subject: [PATCH] Use UsePortInRedirects only if enabled --- rootfs/etc/nginx/template/nginx.tmpl | 19 +++++-- test/e2e/annotations/forcesslredirect.go | 2 +- test/e2e/settings/tls.go | 65 ++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index b6f42af18d..93308543a1 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1126,14 +1126,25 @@ stream { {{ if not (isLocationInLocationList $location $all.Cfg.NoTLSRedirectLocations) }} # enforce ssl on server side if ($redirect_to_https) { + set_by_lua_block $redirect_host { + local ngx_re = require "ngx.re" + + local host_port, err = ngx_re.split(ngx.var.best_http_host, ":") + if err then + ngx.log(ngx.ERR, "could not parse variable: ", err) + return ngx.var.best_http_host; + end + + return host_port[1]; + } + {{ if $location.UsePortInRedirects }} # using custom ports require a different rewrite directive - {{ $redirect_port := (printf ":%v" $all.ListenPorts.HTTPS) }} - error_page 497 ={{ $all.Cfg.HTTPRedirectCode }} https://$host{{ $redirect_port }}$request_uri; - + # https://forum.nginx.org/read.php?2,155978,155978#msg-155978 + error_page 497 ={{ $all.Cfg.HTTPRedirectCode }} https://$redirect_host{{ printf ":%v" $all.ListenPorts.HTTPS }}$request_uri; return 497; {{ else }} - return {{ $all.Cfg.HTTPRedirectCode }} https://$best_http_host$request_uri; + return {{ $all.Cfg.HTTPRedirectCode }} https://$redirect_host$request_uri; {{ end }} } {{ end }} diff --git a/test/e2e/annotations/forcesslredirect.go b/test/e2e/annotations/forcesslredirect.go index e46eb6465f..a7ec4635cf 100644 --- a/test/e2e/annotations/forcesslredirect.go +++ b/test/e2e/annotations/forcesslredirect.go @@ -49,7 +49,7 @@ var _ = framework.IngressNginxDescribe("Annotations - Forcesslredirect", func() f.WaitForNginxServer(host, func(server string) bool { return Expect(server).Should(ContainSubstring(`if ($redirect_to_https) {`)) && - Expect(server).Should(ContainSubstring(`return 308 https://$best_http_host$request_uri;`)) + Expect(server).Should(ContainSubstring(`return 308 https://$redirect_host$request_uri;`)) }) resp, _, errs := gorequest.New(). diff --git a/test/e2e/settings/tls.go b/test/e2e/settings/tls.go index 95101ce2b1..986e02a709 100644 --- a/test/e2e/settings/tls.go +++ b/test/e2e/settings/tls.go @@ -21,6 +21,7 @@ import ( "fmt" "net/http" "strings" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -29,12 +30,17 @@ import ( "k8s.io/ingress-nginx/test/e2e/framework" ) +func noRedirectPolicyFunc(gorequest.Request, []gorequest.Request) error { + return http.ErrUseLastResponse +} + var _ = framework.IngressNginxDescribe("Settings - TLS)", func() { f := framework.NewDefaultFramework("settings-tls") host := "settings-tls" BeforeEach(func() { f.NewEchoDeployment() + f.UpdateNginxConfigMapData("use-forwarded-headers", "false") }) AfterEach(func() { @@ -164,4 +170,63 @@ var _ = framework.IngressNginxDescribe("Settings - TLS)", func() { Expect(resp.StatusCode).Should(Equal(http.StatusOK)) Expect(resp.Header.Get("Strict-Transport-Security")).Should(ContainSubstring("preload")) }) + + It("should not use ports during the HTTP to HTTPS redirection", func() { + ing := f.EnsureIngress(framework.NewSingleIngressWithTLS(host, "/", host, []string{host}, f.IngressController.Namespace, "http-svc", 80, nil)) + tlsConfig, err := framework.CreateIngressTLSSecret(f.KubeClientSet, + ing.Spec.TLS[0].Hosts, + ing.Spec.TLS[0].SecretName, + ing.Namespace) + Expect(err).NotTo(HaveOccurred()) + + framework.WaitForTLS(f.IngressController.HTTPSURL, tlsConfig) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring(`if ($redirect_to_https) {`)) && + Expect(server).Should(ContainSubstring(`return 308 https://$redirect_host$request_uri;`)) + }) + + resp, _, errs := gorequest.New(). + Get(fmt.Sprintf(f.IngressController.HTTPURL)). + Retry(10, 1*time.Second, http.StatusNotFound). + RedirectPolicy(noRedirectPolicyFunc). + Set("Host", host). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusPermanentRedirect)) + Expect(resp.Header.Get("Location")).Should(Equal(fmt.Sprintf("https://%v/", host))) + }) + + It("should not use ports or X-Forwarded-Host during the HTTP to HTTPS redirection", func() { + f.UpdateNginxConfigMapData("use-forwarded-headers", "true") + + ing := f.EnsureIngress(framework.NewSingleIngressWithTLS(host, "/", host, []string{host}, f.IngressController.Namespace, "http-svc", 80, nil)) + tlsConfig, err := framework.CreateIngressTLSSecret(f.KubeClientSet, + ing.Spec.TLS[0].Hosts, + ing.Spec.TLS[0].SecretName, + ing.Namespace) + Expect(err).NotTo(HaveOccurred()) + + framework.WaitForTLS(f.IngressController.HTTPSURL, tlsConfig) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring(`if ($redirect_to_https) {`)) && + Expect(server).Should(ContainSubstring(`return 308 https://$redirect_host$request_uri;`)) + }) + + resp, _, errs := gorequest.New(). + Get(fmt.Sprintf(f.IngressController.HTTPURL)). + Retry(10, 1*time.Second, http.StatusNotFound). + RedirectPolicy(noRedirectPolicyFunc). + Set("Host", host). + Set("X-Forwarded-Host", "example.com:80"). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusPermanentRedirect)) + Expect(resp.Header.Get("Location")).Should(Equal("https://example.com/")) + }) })