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

fix(runloop): Fixed an issue where setting tls_verify to false didn't override the global level config proxy_ssl_verify #13470

Merged
merged 1 commit into from
Aug 13, 2024
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
4 changes: 4 additions & 0 deletions changelog/unreleased/kong/fix-service-tls-verify.yml
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion kong/db/schema/entities/services.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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, }, },
Expand Down
2 changes: 1 addition & 1 deletion kong/runloop/upstream_ssl.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
catbro666 marked this conversation as resolved.
Show resolved Hide resolved
res, err = set_upstream_ssl_verify(tls_verify)
if not res then
log(CRIT, "unable to set upstream TLS verification to: ",
Expand Down
228 changes: 210 additions & 18 deletions spec/02-integration/05-proxy/18-upstream_tls_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Loading