Skip to content

Commit

Permalink
Use UsePortInRedirects only if enabled
Browse files Browse the repository at this point in the history
  • Loading branch information
aledbf committed Feb 21, 2019
1 parent 7534cd5 commit 8b6e4d4
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 5 deletions.
19 changes: 15 additions & 4 deletions rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/annotations/forcesslredirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down
65 changes: 65 additions & 0 deletions test/e2e/settings/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"net/http"
"strings"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
Expand All @@ -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() {
Expand Down Expand Up @@ -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/"))
})
})

0 comments on commit 8b6e4d4

Please sign in to comment.