Skip to content

Commit

Permalink
fix(core): add tls_verify, tls_verify_depth, ca_certificates of a
Browse files Browse the repository at this point in the history
           service into the upstream keepalive pool name

https://konghq.atlassian.net/browse/FTI-6380
  • Loading branch information
catbro666 committed Dec 9, 2024
1 parent 88ffdda commit c6be3a0
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Fixed an issue where `tls_verify`, `tls_verify_depth`, `ca_certificates` of a service were not included in the upstream keepalive pool name.
type: bugfix
scope: Core
10 changes: 8 additions & 2 deletions kong/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1387,8 +1387,14 @@ function Kong.balancer()
-- upstream_host is SNI
pool = pool .. "|" .. var.upstream_host

if ctx.service and ctx.service.client_certificate then
pool = pool .. "|" .. ctx.service.client_certificate.id
local ctx_service = ctx.service
if ctx_service then
pool = string.format("%s|%s|%s|%s|%s",
pool,
ctx_service.tls_verify ~= nil and tostring(ctx_service.tls_verify) or "",
ctx_service.tls_verify_depth or "",
ctx_service.ca_certificates and table.concat(ctx_service.ca_certificates, ",") or "",
ctx_service.client_certificate and ctx_service.client_certificate.id or "")
end
end

Expand Down
89 changes: 64 additions & 25 deletions spec/02-integration/05-proxy/25-upstream_keepalive_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ local fixtures = {

describe("#postgres upstream keepalive", function()
local proxy_client
local ca_certificate, client_cert1, client_cert2

local function start_kong(opts)
local kopts = {
Expand All @@ -51,8 +52,23 @@ describe("#postgres upstream keepalive", function()
"routes",
"services",
"certificates",
"ca_certificates",
})

ca_certificate = assert(bp.ca_certificates:insert({
cert = ssl_fixtures.cert_ca,
}))

client_cert1 = bp.certificates:insert {
cert = ssl_fixtures.cert_client,
key = ssl_fixtures.key_client,
}

client_cert2 = bp.certificates:insert {
cert = ssl_fixtures.cert_client2,
key = ssl_fixtures.key_client2,
}

-- upstream TLS
bp.routes:insert {
hosts = { "one.test" },
Expand All @@ -61,6 +77,9 @@ describe("#postgres upstream keepalive", function()
protocol = helpers.mock_upstream_ssl_protocol,
host = helpers.mock_upstream_hostname,
port = helpers.mock_upstream_ssl_port,
tls_verify = false,
tls_verify_depth = 3,
ca_certificates = { ca_certificate.id },
},
}

Expand All @@ -71,6 +90,9 @@ describe("#postgres upstream keepalive", function()
protocol = helpers.mock_upstream_ssl_protocol,
host = helpers.mock_upstream_hostname,
port = helpers.mock_upstream_ssl_port,
tls_verify = false,
tls_verify_depth = 3,
ca_certificates = { ca_certificate.id },
},
}

Expand Down Expand Up @@ -100,21 +122,21 @@ describe("#postgres upstream keepalive", function()
hosts = { "example.test", },
service = bp.services:insert {
url = "https://127.0.0.1:16798/",
client_certificate = bp.certificates:insert {
cert = ssl_fixtures.cert_client,
key = ssl_fixtures.key_client,
},
client_certificate = client_cert1,
tls_verify = false,
tls_verify_depth = 3,
ca_certificates = { ca_certificate.id },
},
}

bp.routes:insert {
hosts = { "example2.test", },
service = bp.services:insert {
url = "https://127.0.0.1:16798/",
client_certificate = bp.certificates:insert {
cert = ssl_fixtures.cert_client2,
key = ssl_fixtures.key_client2,
},
client_certificate = client_cert2,
tls_verify = false,
tls_verify_depth = 3,
ca_certificates = { ca_certificate.id },
},
}
end)
Expand Down Expand Up @@ -143,12 +165,15 @@ describe("#postgres upstream keepalive", function()
assert.equal("SNI=one.test", body)
assert.errlog()
.has
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test]])
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. "|")

assert.errlog()
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test, cpool: 0+]])
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, cpool: 0+]])
assert.errlog()
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|one.test, size: \d+]])
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, size: \d+]])
assert.errlog()
.has.line([[lua balancer: keepalive no free connection, cpool: [A-F0-9]+]])
assert.errlog()
Expand All @@ -167,12 +192,15 @@ describe("#postgres upstream keepalive", function()
assert.equal("SNI=two.test", body)
assert.errlog()
.has
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|two.test]])
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|two.test|false|3|]] ..
ca_certificate.id .. "|")

assert.errlog()
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|two.test, cpool: 0+]])
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|two.test|false|3|]] ..
ca_certificate.id .. [[|, cpool: 0+]])
assert.errlog()
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|two.test, size: \d+]])
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|two.test|false|3|]] ..
ca_certificate.id .. [[|, size: \d+]])
assert.errlog()
.has.line([[lua balancer: keepalive no free connection, cpool: [A-F0-9]+]])
assert.errlog()
Expand Down Expand Up @@ -225,9 +253,11 @@ describe("#postgres upstream keepalive", function()
assert.not_equal(fingerprint_1, fingerprint_2)

assert.errlog()
.has.line([[enabled connection keepalive \(pool=[0-9.]+|\d+|[0-9.]+:\d+|[a-f0-9-]+]])
.has.line([[enabled connection keepalive \(pool=[0-9.]+|\d+|[0-9.]+:\d+|[a-f0-9-]+|false|3|]] ..
ca_certificate.id .. "|" .. client_cert1.id)
assert.errlog()
.has.line([[keepalive get pool, name: [0-9.]+|\d+|[0-9.]+:\d+|[a-f0-9-]+, cpool: 0+]])
.has.line([[keepalive get pool, name: [0-9.]+|\d+|[0-9.]+:\d+|[a-f0-9-]+|false|3|]] ..
ca_certificate.id .. "|" .. client_cert1.id .. [[, cpool: 0+]])
assert.errlog()
.has.line([[keepalive create pool, name: [0-9.]+|\d+|[0-9.]+:\d+|[a-f0-9-]+, size: \d+]])
assert.errlog()
Expand Down Expand Up @@ -299,12 +329,15 @@ describe("#postgres upstream keepalive", function()
assert.equal("SNI=one.test", body)
assert.errlog()
.has
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test]])
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. "|")

assert.errlog()
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test, cpool: 0+]])
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, cpool: 0+]])
assert.errlog()
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|one.test, size: \d+]])
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, size: \d+]])
assert.errlog()
.has.line([[keepalive no free connection, cpool: [A-F0-9]+]])
assert.errlog()
Expand All @@ -330,10 +363,12 @@ describe("#postgres upstream keepalive", function()
assert.equal("SNI=one.test", body)
assert.errlog()
.has
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test]])
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. "|")

assert.errlog()
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test, ]] .. upool_ptr)
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, ]] .. upool_ptr)
assert.errlog()
.has.line([[keepalive reusing connection [A-F0-9]+, requests: \d+, ]] .. upool_ptr)
assert.errlog()
Expand All @@ -357,18 +392,22 @@ describe("#postgres upstream keepalive", function()
assert.equal("SNI=one.test", body)
assert.errlog()
.has
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test]])
.line([[enabled connection keepalive \(pool=[A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. "|")

assert.errlog()
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test, cpool: 0+]])
.has.line([[keepalive get pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, cpool: 0+]])
assert.errlog()
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|one.test, size: \d+]])
.has.line([[keepalive create pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, size: \d+]])
assert.errlog()
.has.line([[keepalive no free connection, cpool: [A-F0-9]+]])
assert.errlog()
.has.line([[keepalive not saving connection [A-F0-9]+, cpool: [A-F0-9]+]])
assert.errlog()
.has.line([[keepalive free pool, name: [A-F0-9.:]+\|\d+\|one.test, cpool: [A-F0-9]+]])
.has.line([[keepalive free pool, name: [A-F0-9.:]+\|\d+\|one.test|false|3|]] ..
ca_certificate.id .. [[|, cpool: [A-F0-9]+]])

assert.errlog()
.not_has.line([[keepalive saving connection]], true)
Expand Down

0 comments on commit c6be3a0

Please sign in to comment.