From 7ff307148234bb005146391be2ee2e4b4ba6c473 Mon Sep 17 00:00:00 2001 From: Zhefeng Chen Date: Wed, 5 Jun 2024 16:01:38 +0800 Subject: [PATCH] fix(runloop): the sni cache isn't invalidated when a sni is updated 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 | 22 ++++--- .../02-core_entities_invalidations_spec.lua | 62 +++++++++++++++++++ 3 files changed, 78 insertions(+), 10 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..3db4cac30a7e 100644 --- a/kong/runloop/events.lua +++ b/kong/runloop/events.lua @@ -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 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")