Skip to content

Commit

Permalink
fix(runloop): the sni cache isn't invalidated when a sni is updated
Browse files Browse the repository at this point in the history
Both `data.entity` and `data.old_entity` should be invalidated when
a sni is updated. A non-existent sni may also have been cached.

https://konghq.atlassian.net/browse/FTI-6009
  • Loading branch information
catbro666 committed Jun 5, 2024
1 parent 4adb677 commit 7ff3071
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 10 deletions.
4 changes: 4 additions & 0 deletions changelog/unreleased/kong/fix-sni-cache-invalidate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
Fixed an issue where the sni cache isn't invalidated when a sni is updated.
type: bugfix
scope: Core
22 changes: 12 additions & 10 deletions kong/runloop/events.lua
Original file line number Diff line number Diff line change
Expand Up @@ -261,17 +261,19 @@ end
local function crud_snis_handler(data)
log(DEBUG, "[events] SNI updated, invalidating cached certificates")

local sni = data.old_entity or data.entity
local sni_name = sni.name
local sni_wild_pref, sni_wild_suf = certificate.produce_wild_snis(sni_name)
core_cache:invalidate("snis:" .. sni_name)

if sni_wild_pref then
core_cache:invalidate("snis:" .. sni_wild_pref)
end
-- data.entity won't be nil so we put it in the front
for _, sni in ipairs { data.entity, data.old_entity } do
local sni_name = sni.name
local sni_wild_pref, sni_wild_suf = certificate.produce_wild_snis(sni_name)
core_cache:invalidate("snis:" .. sni_name)

if sni_wild_pref then
core_cache:invalidate("snis:" .. sni_wild_pref)
end

if sni_wild_suf then
core_cache:invalidate("snis:" .. sni_wild_suf)
if sni_wild_suf then
core_cache:invalidate("snis:" .. sni_wild_suf)
end
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,12 @@ for _, strategy in helpers.each_strategy() do
end)

it("on certificate delete+re-creation", function()
-- populate cache
get_cert(8443, "ssl-example.com")
get_cert(8443, "new-ssl-example.com")
get_cert(9443, "ssl-example.com")
get_cert(9443, "new-ssl-example.com")

-- TODO: PATCH update are currently not possible
-- with the admin API because snis have their name as their
-- primary key and the DAO has limited support for such updates.
Expand Down Expand Up @@ -514,6 +520,10 @@ for _, strategy in helpers.each_strategy() do
end)

it("on certificate update", function()
-- populate cache
get_cert(8443, "new-ssl-example.com")
get_cert(9443, "new-ssl-example.com")

-- update our certificate *without* updating the
-- attached sni

Expand Down Expand Up @@ -548,6 +558,12 @@ for _, strategy in helpers.each_strategy() do
end)

it("on sni update via id", function()
-- populate cache
get_cert(8443, "new-ssl-example.com")
get_cert(8443, "updated-sn-via-id.com")
get_cert(9443, "new-ssl-example.com")
get_cert(9443, "updated-sn-via-id.com")

local admin_res = admin_client_1:get("/snis")
local body = assert.res_status(200, admin_res)
local sni = assert(cjson.decode(body).data[1])
Expand Down Expand Up @@ -579,6 +595,12 @@ for _, strategy in helpers.each_strategy() do
end)

it("on sni update via name", function()
-- populate cache
get_cert(8443, "updated-sn-via-id.com")
get_cert(8443, "updated-sn.com")
get_cert(9443, "updated-sn-via-id.com")
get_cert(9443, "updated-sn.com")

local admin_res = admin_client_1:patch("/snis/updated-sn-via-id.com", {
body = { name = "updated-sn.com" },
headers = { ["Content-Type"] = "application/json" },
Expand Down Expand Up @@ -606,6 +628,10 @@ for _, strategy in helpers.each_strategy() do
end)

it("on certificate delete", function()
-- populate cache
get_cert(8443, "updated-sn.com")
get_cert(9443, "updated-sn.com")

-- delete our certificate

local admin_res = admin_client_1:delete("/certificates/updated-sn.com")
Expand All @@ -630,6 +656,14 @@ for _, strategy in helpers.each_strategy() do

describe("wildcard snis", function()
it("on create", function()
-- populate cache
get_cert(8443, "test.wildcard.com")
get_cert(8443, "test2.wildcard.com")
get_cert(8443, "wildcard.com")
get_cert(9443, "test.wildcard.com")
get_cert(9443, "test2.wildcard.com")
get_cert(9443, "wildcard.com")

local admin_res = admin_client_1:post("/certificates", {
body = {
cert = ssl_fixtures.cert_alt,
Expand Down Expand Up @@ -680,6 +714,12 @@ for _, strategy in helpers.each_strategy() do
end)

it("on certificate update", function()
-- populate cache
get_cert(8443, "test.wildcard.com")
get_cert(8443, "test2.wildcard.com")
get_cert(9443, "test.wildcard.com")
get_cert(9443, "test2.wildcard.com")

-- update our certificate *without* updating the
-- attached sni

Expand Down Expand Up @@ -723,6 +763,14 @@ for _, strategy in helpers.each_strategy() do
end)

it("on sni update via id", function()
-- populate cache
get_cert(8443, "test.wildcard.com")
get_cert(8443, "test2.wildcard.com")
get_cert(8443, "test.wildcard_updated.com")
get_cert(9443, "test.wildcard.com")
get_cert(9443, "test2.wildcard.com")
get_cert(9443, "test.wildcard_updated.com")

local admin_res = admin_client_1:get("/snis/%2A.wildcard.com")
local body = assert.res_status(200, admin_res)
local sni = assert(cjson.decode(body))
Expand Down Expand Up @@ -762,6 +810,14 @@ for _, strategy in helpers.each_strategy() do
end)

it("on sni update via name", function()
-- populate cache
get_cert(8443, "test.wildcard.org")
get_cert(8443, "test2.wildcard.org")
get_cert(8443, "test.wildcard_updated.com")
get_cert(9443, "test.wildcard.org")
get_cert(9443, "test2.wildcard.org")
get_cert(9443, "test.wildcard_updated.com")

local admin_res = admin_client_1:patch("/snis/%2A.wildcard_updated.com", {
body = { name = "*.wildcard.org" },
headers = { ["Content-Type"] = "application/json" },
Expand Down Expand Up @@ -797,6 +853,12 @@ for _, strategy in helpers.each_strategy() do
end)

it("on certificate delete", function()
-- populate cache
get_cert(8443, "test.wildcard.org")
get_cert(8443, "test2.wildcard.org")
get_cert(9443, "test.wildcard.org")
get_cert(9443, "test2.wildcard.org")

-- delete our certificate

local admin_res = admin_client_1:delete("/certificates/%2A.wildcard.org")
Expand Down

0 comments on commit 7ff3071

Please sign in to comment.