From 8c64b12a967670934cf50b2757c0f827e62d313d Mon Sep 17 00:00:00 2001 From: Elvin Efendi Date: Mon, 23 Sep 2019 23:40:47 -0400 Subject: [PATCH] refactor force ssl redirect logic --- .../ingress/controller/template/template.go | 20 +++++++++---------- rootfs/etc/nginx/lua/certificate.lua | 8 ++++++++ rootfs/etc/nginx/lua/lua_ingress.lua | 20 ++++++++++++++++--- .../etc/nginx/lua/test/certificate_test.lua | 18 +++++++++++++++++ rootfs/etc/nginx/template/nginx.tmpl | 2 +- 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index af810570a..181f9fb0a 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -306,32 +306,30 @@ func configForLua(input interface{}) string { } // locationConfigForLua formats some location specific configuration into Lua table represented as string -func locationConfigForLua(l interface{}, s interface{}, a interface{}) string { +func locationConfigForLua(l interface{}, a interface{}) string { location, ok := l.(*ingress.Location) if !ok { klog.Errorf("expected an '*ingress.Location' type but %T was given", l) return "{}" } - server, ok := s.(*ingress.Server) - if !ok { - klog.Errorf("expected an '*ingress.Server' type but %T was given", s) - return "{}" - } - all, ok := a.(config.TemplateConfig) if !ok { klog.Errorf("expected a 'config.TemplateConfig' type but %T was given", a) return "{}" } - forceSSLRedirect := location.Rewrite.ForceSSLRedirect || (server.SSLCert != nil && location.Rewrite.SSLRedirect) - forceSSLRedirect = forceSSLRedirect && !isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations) - return fmt.Sprintf(`{ force_ssl_redirect = %t, + ssl_redirect = %t, + force_no_ssl_redirect = %t, use_port_in_redirects = %t, - }`, forceSSLRedirect, location.UsePortInRedirects) + }`, + location.Rewrite.ForceSSLRedirect, + location.Rewrite.SSLRedirect, + isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations), + location.UsePortInRedirects, + ) } // buildResolvers returns the resolvers reading the /etc/resolv.conf file diff --git a/rootfs/etc/nginx/lua/certificate.lua b/rootfs/etc/nginx/lua/certificate.lua index cf46f92e5..03d177669 100644 --- a/rootfs/etc/nginx/lua/certificate.lua +++ b/rootfs/etc/nginx/lua/certificate.lua @@ -48,6 +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 + end + + return get_pem_cert_key(hostname) ~= nil +end + function _M.call() local hostname, hostname_err = ssl.server_name() if hostname_err then diff --git a/rootfs/etc/nginx/lua/lua_ingress.lua b/rootfs/etc/nginx/lua/lua_ingress.lua index bee1d3cba..4fdb04618 100644 --- a/rootfs/etc/nginx/lua/lua_ingress.lua +++ b/rootfs/etc/nginx/lua/lua_ingress.lua @@ -1,5 +1,7 @@ local ngx_re_split = require("ngx.re").split +local certificate_configured_for_server = require("certificate").configured_for_server + local original_randomseed = math.randomseed local string_format = string.format local ngx_redirect = ngx.redirect @@ -54,8 +56,20 @@ local function randomseed() math.randomseed(seed) end -local function redirect_to_https() - return ngx.var.pass_access_scheme == "http" and (ngx.var.scheme == "http" or ngx.var.scheme == "https") +local function redirect_to_https(location_config) + if location_config.force_no_ssl_redirect then + return false + end + + if ngx.var.pass_access_scheme ~= "http" then + return false + end + + if location_config.force_ssl_redirect then + return true + end + + return location_config.ssl_redirect and certificate_configured_for_server(ngx.var.host) end local function redirect_host() @@ -119,7 +133,7 @@ function _M.rewrite(location_config) ngx.var.pass_port = 443 end - if location_config.force_ssl_redirect and redirect_to_https() then + if redirect_to_https(location_config) then local uri = string_format("https://%s%s", redirect_host(), ngx.var.request_uri) if location_config.use_port_in_redirects then diff --git a/rootfs/etc/nginx/lua/test/certificate_test.lua b/rootfs/etc/nginx/lua/test/certificate_test.lua index 5cbfbe4fc..3d9e44e66 100644 --- a/rootfs/etc/nginx/lua/test/certificate_test.lua +++ b/rootfs/etc/nginx/lua/test/certificate_test.lua @@ -129,4 +129,22 @@ describe("Certificate", function() assert.spy(ngx.log).was_called_with(ngx.ERR, "failed to convert certificate chain from PEM to DER: PEM_read_bio_X509_AUX() failed") end) end) + + describe("configured_for_server", function() + before_each(function() + 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")) + end) + + it("returns false when certificate does not exist for given server", function() + assert.is_false(certificate.configured_for_server("hostname.xyz")) + end) + + it("returns false when no server given", function() + assert.is_false(certificate.configured_for_server()) + end) + end) end) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index f926a6f43..6e6a30d06 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -967,7 +967,7 @@ stream { {{ end }} rewrite_by_lua_block { - lua_ingress.rewrite({{ locationConfigForLua $location $server $all }}) + lua_ingress.rewrite({{ locationConfigForLua $location $all }}) balancer.rewrite() plugins.run() }