Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hsts refactoring #4601

Merged
merged 2 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,23 @@ func configForLua(input interface{}) string {
is_ssl_passthrough_enabled = %t,
http_redirect_code = %v,
listen_ports = { ssl_proxy = "%v", https = "%v" },
}`, all.Cfg.UseForwardedHeaders, all.IsSSLPassthroughEnabled, all.Cfg.HTTPRedirectCode, all.ListenPorts.SSLProxy, all.ListenPorts.HTTPS)

hsts = %t,
hsts_max_age = %v,
hsts_include_subdomains = %t,
hsts_preload = %t,
}`,
all.Cfg.UseForwardedHeaders,
all.IsSSLPassthroughEnabled,
all.Cfg.HTTPRedirectCode,
all.ListenPorts.SSLProxy,
all.ListenPorts.HTTPS,

all.Cfg.HSTS,
all.Cfg.HSTSMaxAge,
all.Cfg.HSTSIncludeSubdomains,
all.Cfg.HSTSPreload,
)
}

// locationConfigForLua formats some location specific configuration into Lua table represented as string
Expand Down
10 changes: 6 additions & 4 deletions rootfs/etc/nginx/lua/certificate.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ local function get_pem_cert_key(raw_hostname)
return pem_cert_key
end

function _M.configured_for_server(hostname)
if not hostname then
return false
function _M.configured_for_current_request()
if ngx.ctx.configured_for_current_request ~= nil then
return ngx.ctx.configured_for_current_request
end

return get_pem_cert_key(hostname) ~= nil
ngx.ctx.configured_for_current_request = get_pem_cert_key(ngx.var.host) ~= nil

return ngx.ctx.configured_for_current_request
end

function _M.call()
Expand Down
15 changes: 13 additions & 2 deletions rootfs/etc/nginx/lua/lua_ingress.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
local ngx_re_split = require("ngx.re").split

local certificate_configured_for_server = require("certificate").configured_for_server
local certificate_configured_for_current_request = require("certificate").configured_for_current_request

local original_randomseed = math.randomseed
local string_format = string.format
Expand Down Expand Up @@ -69,7 +69,7 @@ local function redirect_to_https(location_config)
return true
end

return location_config.ssl_redirect and certificate_configured_for_server(ngx.var.host)
return location_config.ssl_redirect and certificate_configured_for_current_request()
end

local function redirect_host()
Expand Down Expand Up @@ -142,6 +142,17 @@ function _M.rewrite(location_config)

ngx_redirect(uri, config.http_redirect_code)
end

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
value = value .. "; includeSubDomains"
end
if config.hsts_preload then
value = value .. "; preload"
end
ngx.header["Strict-Transport-Security"] = value
end
end

return _M
17 changes: 12 additions & 5 deletions rootfs/etc/nginx/lua/test/certificate_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -130,21 +130,28 @@ describe("Certificate", function()
end)
end)

describe("configured_for_server", function()
describe("configured_for_current_request", function()
before_each(function()
local _ngx = { var = { host = "hostname" } }
setmetatable(_ngx, {__index = _G.ngx})
_G.ngx = _ngx
ngx.ctx.configured_for_current_request = nil

set_certificate("hostname", EXAMPLE_CERT, UUID)
end)

it("returns true when certificate exists for given server", function()
assert.is_true(certificate.configured_for_server("hostname"))
assert.is_true(certificate.configured_for_current_request())
end)

it("returns false when certificate does not exist for given server", function()
assert.is_false(certificate.configured_for_server("hostname.xyz"))
ngx.var.host = "hostname.xyz"
assert.is_false(certificate.configured_for_current_request())
end)

it("returns false when no server given", function()
assert.is_false(certificate.configured_for_server())
it("returns cached value from ngx.ctx", function()
ngx.ctx.configured_for_current_request = false
assert.is_false(certificate.configured_for_current_request())
end)
end)
end)
6 changes: 0 additions & 6 deletions rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -1051,12 +1051,6 @@ stream {
plugins.run()
}

{{ if (and $server.SSLCert $all.Cfg.HSTS) }}
if ($scheme = https) {
more_set_headers "Strict-Transport-Security: max-age={{ $all.Cfg.HSTSMaxAge }}{{ if $all.Cfg.HSTSIncludeSubdomains }}; includeSubDomains{{ end }}{{ if $all.Cfg.HSTSPreload }}; preload{{ end }}";
}
{{ end }}

{{ if not $location.Logs.Access }}
access_log off;
{{ end }}
Expand Down