From 989a13e9332d01ff392bc91fb61b4ed6279b2513 Mon Sep 17 00:00:00 2001 From: Zhefeng C <38037704+catbro666@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:31:07 +0800 Subject: [PATCH] fix(runloop): the sni cache isn't invalidated when a sni is updated (#13165) 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 --- .../kong/fix-sni-cache-invalidate.yml | 4 ++ kong/runloop/events.lua | 23 ++++--- .../02-core_entities_invalidations_spec.lua | 62 +++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/kong/fix-sni-cache-invalidate.yml diff --git a/changelog/unreleased/kong/fix-sni-cache-invalidate.yml b/changelog/unreleased/kong/fix-sni-cache-invalidate.yml new file mode 100644 index 000000000000..a898826b2754 --- /dev/null +++ b/changelog/unreleased/kong/fix-sni-cache-invalidate.yml @@ -0,0 +1,4 @@ +message: | + Fixed an issue where the sni cache isn't invalidated when a sni is updated. +type: bugfix +scope: Core diff --git a/kong/runloop/events.lua b/kong/runloop/events.lua index ddc3e9750a41..b8faba64cbee 100644 --- a/kong/runloop/events.lua +++ b/kong/runloop/events.lua @@ -258,24 +258,33 @@ local function crud_plugins_handler(data) 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 function invalidate_snis(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 + if sni_wild_pref and sni_wild_pref ~= sni_name then core_cache:invalidate("snis:" .. sni_wild_pref) end - if sni_wild_suf then + if sni_wild_suf and sni_wild_suf ~= sni_name then core_cache:invalidate("snis:" .. sni_wild_suf) end end +local function crud_snis_handler(data) + log(DEBUG, "[events] SNI updated, invalidating cached certificates") + + local new_name = data.entity.name + local old_name = data.old_entity and data.old_entity.name + + invalidate_snis(new_name) + if old_name and old_name ~= new_name then + invalidate_snis(old_name) + end +end + + local function crud_consumers_handler(data) workspaces.set_workspace(data.workspace) diff --git a/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua b/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua index d9946e39b04d..0f2ec5d3931c 100644 --- a/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua +++ b/spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua @@ -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. @@ -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 @@ -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]) @@ -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" }, @@ -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") @@ -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, @@ -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 @@ -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)) @@ -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" }, @@ -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")