diff --git a/changelog/unreleased/kong/fix-service-tls-verify.yml b/changelog/unreleased/kong/fix-service-tls-verify.yml new file mode 100644 index 000000000000..aac9528ed97e --- /dev/null +++ b/changelog/unreleased/kong/fix-service-tls-verify.yml @@ -0,0 +1,4 @@ +message: | + Fixed an issue where setting `tls_verify` to `false` didn't override the global level `proxy_ssl_verify`. +type: bugfix +scope: Core diff --git a/kong/db/schema/entities/services.lua b/kong/db/schema/entities/services.lua index cf2954a36770..71a152e96973 100644 --- a/kong/db/schema/entities/services.lua +++ b/kong/db/schema/entities/services.lua @@ -42,7 +42,7 @@ return { { read_timeout = nonzero_timeout { default = 60000 }, }, { tags = typedefs.tags }, { client_certificate = { description = "Certificate to be used as client certificate while TLS handshaking to the upstream server.", type = "foreign", reference = "certificates" }, }, - { tls_verify = { description = "Whether to enable verification of upstream server TLS certificate.", type = "boolean", }, }, + { tls_verify = { description = "Whether to enable verification of upstream server TLS certificate. If not set, the global level config `proxy_ssl_verify` will be used.", type = "boolean", }, }, { tls_verify_depth = { description = "Maximum depth of chain while verifying Upstream server's TLS certificate.", type = "integer", default = null, between = { 0, 64 }, }, }, { ca_certificates = { description = "Array of CA Certificate object UUIDs that are used to build the trust store while verifying upstream server's TLS certificate.", type = "array", elements = { type = "string", uuid = true, }, }, }, { enabled = { description = "Whether the Service is active. ", type = "boolean", required = true, default = true, }, }, diff --git a/kong/runloop/upstream_ssl.lua b/kong/runloop/upstream_ssl.lua index 990ebe7b77ad..1b4ef35067e1 100644 --- a/kong/runloop/upstream_ssl.lua +++ b/kong/runloop/upstream_ssl.lua @@ -42,7 +42,7 @@ local function set_service_ssl(ctx) end local tls_verify = service.tls_verify - if tls_verify then + if tls_verify ~= nil then res, err = set_upstream_ssl_verify(tls_verify) if not res then log(CRIT, "unable to set upstream TLS verification to: ", diff --git a/spec/02-integration/05-proxy/18-upstream_tls_spec.lua b/spec/02-integration/05-proxy/18-upstream_tls_spec.lua index b560a2a0e0e4..13789ccdaee8 100644 --- a/spec/02-integration/05-proxy/18-upstream_tls_spec.lua +++ b/spec/02-integration/05-proxy/18-upstream_tls_spec.lua @@ -121,6 +121,23 @@ local function gen_plugin(route) } end +local function get_proxy_client(subsystems, stream_port) + if subsystems == "http" then + return assert(helpers.proxy_client()) + else + return assert(helpers.proxy_client(20000, stream_port)) + end +end + +local function wait_for_all_config_update(subsystems) + local opt = {} + if subsystems == "stream" then + opt.stream_enabled = true + opt.stream_port = 19003 + end + + helpers.wait_for_all_config_update(opt) +end for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" }) do for _, strategy in helpers.each_strategy() do @@ -318,24 +335,6 @@ for _, strategy in helpers.each_strategy() do end end - local function get_proxy_client(subsystems, stream_port) - if subsystems == "http" then - return assert(helpers.proxy_client()) - else - return assert(helpers.proxy_client(20000, stream_port)) - end - end - - local function wait_for_all_config_update(subsystems) - local opt = {} - if subsystems == "stream" then - opt.stream_enabled = true - opt.stream_port = 19003 - end - - helpers.wait_for_all_config_update(opt) - end - for _, subsystems in pairs({"http", "stream"}) do describe(subsystems .. " mutual TLS authentication against upstream with Service object", function() describe("no client certificate supplied", function() @@ -1250,3 +1249,196 @@ for _, strategy in helpers.each_strategy() do end) end end -- for flavor + +for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" }) do +for _, strategy in helpers.each_strategy() do + describe("overriding upstream TLS parameters for database [#" .. strategy .. ", flavor = " .. flavor .. "] (nginx_proxy_proxy_ssl_verify: on, nginx_sproxy_proxy_ssl_verify: on)", function() + local admin_client + local bp + local service_tls + local tls_service_tls + local route_tls_buffered_proxying + + reload_router(flavor) + + lazy_setup(function() + bp = helpers.get_db_utils(strategy, { + "routes", + "services", + "certificates", + "ca_certificates", + "upstreams", + "targets", + }) + + service_tls = assert(bp.services:insert({ + name = "protected-service", + url = "https://example.com:16799/", -- domain name needed for hostname check + })) + + assert(bp.routes:insert(gen_route(flavor,{ + service = { id = service_tls.id, }, + hosts = { "example.com", }, + paths = { "/tls", }, + }))) + + route_tls_buffered_proxying = assert(bp.routes:insert(gen_route(flavor,{ + service = { id = service_tls.id, }, + hosts = { "example.com", }, + paths = { "/tls-buffered-proxying", }, + }))) + + -- use pre-function to enable buffered_proxying in order to trigger the + -- `ngx.location.capture("/kong_buffered_http")` in `Kong.response()` + assert(bp.plugins:insert(gen_plugin(route_tls_buffered_proxying))) + + -- tls + tls_service_tls = assert(bp.services:insert({ + name = "tls-protected-service", + url = "tls://example.com:16799", -- domain name needed for hostname check + })) + + assert(bp.routes:insert(gen_route(flavor,{ + service = { id = tls_service_tls.id, }, + destinations = { + { + port = 19001, + }, + }, + protocols = { + "tls", + }, + }))) + + assert(helpers.start_kong({ + router_flavor = flavor, + database = strategy, + nginx_conf = "spec/fixtures/custom_nginx.template", + stream_listen = helpers.get_proxy_ip(false) .. ":19000," + .. helpers.get_proxy_ip(false) .. ":19001," + .. helpers.get_proxy_ip(false) .. ":19002," + .. helpers.get_proxy_ip(false) .. ":19003", + nginx_proxy_proxy_ssl_verify = "on", + -- An unrelated ca, just used as a placeholder to prevent nginx from reporting errors + nginx_proxy_proxy_ssl_trusted_certificate = "../spec/fixtures/kong_clustering_ca.crt", + nginx_sproxy_proxy_ssl_verify = "on", + nginx_sproxy_proxy_ssl_trusted_certificate = "../spec/fixtures/kong_clustering_ca.crt", + }, nil, nil, fixtures)) + + admin_client = assert(helpers.admin_client()) + end) + + lazy_teardown(function() + helpers.stop_kong() + end) + + for _, subsystems in pairs({"http", "stream"}) do + describe(subsystems .. " TLS verification options against upstream", function() + describe("tls_verify", function() + it("default is on, request is blocked", function() + local proxy_client = get_proxy_client(subsystems, 19001) + local path + if subsystems == "http" then + path = "/tls" + else + path = "/" + end + + local res, err = proxy_client:send { + path = path, + headers = { + ["Host"] = "example.com", + } + } + if subsystems == "http" then + local body = assert.res_status(502, res) + assert.matches("An invalid response was received from the upstream server", body) + else + assert.equals("connection reset by peer", err) + end + assert(proxy_client:close()) + end) + + -- buffered_proxying + if subsystems == "http" then + it("default is on, buffered_proxying = true, request is blocked", function() + local proxy_client = get_proxy_client(subsystems, 19001) + local path = "/tls-buffered-proxying" + local res = proxy_client:send { + path = path, + headers = { + ["Host"] = "example.com", + } + } + + local body = assert.res_status(502, res) + assert.matches("An invalid response was received from the upstream server", body) + assert(proxy_client:close()) + end) + end + + it("#db turn it off, request is allowed", function() + local service_tls_id + if subsystems == "http" then + service_tls_id = service_tls.id + else + service_tls_id = tls_service_tls.id + end + local res = assert(admin_client:patch("/services/" .. service_tls_id, { + body = { + tls_verify = false, + }, + headers = { ["Content-Type"] = "application/json" }, + })) + + assert.res_status(200, res) + + wait_for_all_config_update(subsystems) + + local path + if subsystems == "http" then + path = "/tls" + else + path = "/" + end + + helpers.wait_until(function() + local proxy_client = get_proxy_client(subsystems, 19001) + res = proxy_client:send { + path = path, + headers = { + ["Host"] = "example.com", + } + } + return pcall(function() + local body = assert.res_status(200, res) + assert.equals("it works", body) + assert(proxy_client:close()) + end) + end, 10) + + -- buffered_proxying + if subsystems == "http" then + helpers.wait_until(function() + local proxy_client = get_proxy_client(subsystems, 19001) + res = proxy_client:send { + path = "/tls-buffered-proxying", + headers = { + ["Host"] = "example.com", + } + } + + return pcall(function() + local body = assert.res_status(200, res) + assert.equals("it works", body) + assert(proxy_client:close()) + end) + end, 10) + end + end) + end) + end) + end + end) +end +end -- for flavor