Skip to content

Commit

Permalink
Merge pull request #4826 from ElvinEfendi/fix-duplicate-hsts
Browse files Browse the repository at this point in the history
regression test and fix for duplicate hsts bug
  • Loading branch information
k8s-ci-robot authored Dec 12, 2019
2 parents d5e197c + 162ecb9 commit cf03ae3
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 1 deletion.
3 changes: 2 additions & 1 deletion build/dev-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ DEV_IMAGE=${REGISTRY}/nginx-ingress-controller:${TAG}
{ [ "$(minikube status | grep -c Running)" -ge 2 ] && minikube status | grep -qE ': Configured$|Correctly Configured'; } \
|| minikube start \
--extra-config=kubelet.sync-frequency=1s \
--extra-config=apiserver.authorization-mode=RBAC
--extra-config=apiserver.authorization-mode=RBAC \
--kubernetes-version=v1.15.0

# shellcheck disable=SC2046
eval $(minikube docker-env --shell bash)
Expand Down
2 changes: 2 additions & 0 deletions build/run-e2e-suite.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ until kubectl get secret | grep -q -e ^ingress-nginx-e2e-token; do \
sleep 3; \
done

echo -e "Starting the e2e test pod"

kubectl run --rm \
--attach \
--restart=Never \
Expand Down
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/lua_ingress.lua
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,9 @@ function _M.rewrite(location_config)

ngx_redirect(uri, config.http_redirect_code)
end
end

function _M.header()
if config.hsts and ngx.var.scheme == "https" and certificate_configured_for_current_request then
local value = "max-age=" .. config.hsts_max_age
if config.hsts_include_subdomains then
Expand Down
1 change: 1 addition & 0 deletions rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,7 @@ stream {
#}

header_filter_by_lua_block {
lua_ingress.header()
plugins.run()
}

Expand Down
6 changes: 6 additions & 0 deletions test/e2e/framework/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ Request Body:
location / {
lua_need_request_body on;
header_filter_by_lua_block {
if ngx.var.arg_hsts == "true" then
ngx.header["Strict-Transport-Security"] = "max-age=3600; preload"
end
}
content_by_lua_block {
ngx.header["Server"] = "echoserver"
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/settings/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,18 @@ var _ = framework.IngressNginxDescribe("Settings - TLS)", func() {
Expect(errs).Should(BeEmpty())
Expect(resp.StatusCode).Should(Equal(http.StatusOK))
Expect(resp.Header.Get("Strict-Transport-Security")).Should(Equal("max-age=86400; preload"))

By("overriding what's set from the upstream")

// we can not use gorequest here because it flattens the duplicate headers
// and specifically in case of Strict-Transport-Security it ignore extra headers
// intead of concatenating, rightfully. And I don't know of any API it provides for getting raw headers.
curlCmd := fmt.Sprintf("curl -I -k --fail --silent --resolve settings-tls:443:127.0.0.1 https://settings-tls/%v", "?hsts=true")
output, err := f.ExecIngressPod(curlCmd)
Expect(err).ToNot(HaveOccurred())
Expect(output).Should(ContainSubstring("strict-transport-security: max-age=86400; preload"))
// this is what the upstream sets
Expect(output).ShouldNot(ContainSubstring("strict-transport-security: max-age=3600; preload"))
})

It("should not use ports during the HTTP to HTTPS redirection", func() {
Expand Down

0 comments on commit cf03ae3

Please sign in to comment.