diff --git a/changelog/unreleased/kong/ca_certificates_reference_check.yml b/changelog/unreleased/kong/ca_certificates_reference_check.yml new file mode 100644 index 000000000000..3ac9d8a3aab5 --- /dev/null +++ b/changelog/unreleased/kong/ca_certificates_reference_check.yml @@ -0,0 +1,3 @@ +message: prevent ca to be deleted when it's still referenced by other entities and invalidate the related ca store caches when a ca cert is updated. +type: bugfix +scope: Core diff --git a/kong-3.6.0-0.rockspec b/kong-3.6.0-0.rockspec index 4a07e972a13b..757370c35d31 100644 --- a/kong-3.6.0-0.rockspec +++ b/kong-3.6.0-0.rockspec @@ -144,6 +144,7 @@ build = { ["kong.api.routes.tags"] = "kong/api/routes/tags.lua", ["kong.api.routes.targets"] = "kong/api/routes/targets.lua", ["kong.api.routes.upstreams"] = "kong/api/routes/upstreams.lua", + ["kong.api.routes.ca_certificates"] = "kong/api/routes/ca_certificates.lua", ["kong.admin_gui"] = "kong/admin_gui/init.lua", ["kong.admin_gui.utils"] = "kong/admin_gui/utils.lua", diff --git a/kong/api/routes/ca_certificates.lua b/kong/api/routes/ca_certificates.lua new file mode 100644 index 000000000000..1b73db3e0660 --- /dev/null +++ b/kong/api/routes/ca_certificates.lua @@ -0,0 +1,24 @@ +local certificates = require "kong.runloop.certificate" +local fmt = string.format +local kong = kong + +return { + ["/ca_certificates/:ca_certificates"] = { + DELETE = function(self, db, helpers, parent) + local ca_id = self.params.ca_certificates + local entity, element_or_err = certificates.check_ca_references(ca_id) + + if entity then + local msg = fmt("ca_certificate %s is still referenced by %s (id = %s)", ca_id, entity, element_or_err.id) + kong.log.notice(msg) + return kong.response.exit(400, { message = msg }) + elseif element_or_err then + local msg = "failed to check_ca_references, " .. element_or_err + kong.log.err(msg) + return kong.response.exit(500, { message = msg }) + end + + return parent() + end, + }, +} diff --git a/kong/runloop/certificate.lua b/kong/runloop/certificate.lua index 53da6b3d8d35..43a9d6d0d44a 100644 --- a/kong/runloop/certificate.lua +++ b/kong/runloop/certificate.lua @@ -19,6 +19,7 @@ local set_cert = ngx_ssl.set_cert local set_priv_key = ngx_ssl.set_priv_key local tb_concat = table.concat local tb_sort = table.sort +local tb_insert = table.insert local kong = kong local type = type local error = error @@ -28,6 +29,8 @@ local ipairs = ipairs local ngx_md5 = ngx.md5 local ngx_exit = ngx.exit local ngx_ERROR = ngx.ERROR +local null = ngx.null +local fmt = string.format local default_cert_and_key @@ -371,6 +374,151 @@ local function get_ca_certificate_store(ca_ids) end +-- ordinary entities that reference ca certificates +-- the value denotes which cache (kong.cache or kong.core_cache) is used +local CA_CERT_REFERENCE_ENTITIES = { + "services", +} + +-- plugins that reference ca certificates +-- Format: +-- mtls-auth = true +local CA_CERT_REFERENCE_PLUGINS = { +} + +local loaded_plugins +local reference_plugins + +-- Examples: +-- gen_iterator("services") +-- gen_iterator("plugins", { mtls-auth = true }) +-- gen_iterator("plugins", "mtls-auth") +-- We assume the field name is always `ca_certificates` +local function gen_iterator(entity, plugins) + local options = { + workspace = null, + } + + local iter = kong.db[entity]:each(1000, options) + + local function iterator() + local element, err = iter() + if err then + return nil, err + + elseif element == nil then + return nil + + else + if plugins then + if type(plugins) ~= "table" then + plugins = { [plugins] = true } + end + + if plugins[element.name] and element.config.ca_certificates and + next(element.config.ca_certificates) then + return element + else + return iterator() + end + else + if element.ca_certificates and next(element.ca_certificates) then + return element + else + return iterator() + end + end + end + end + + return iterator +end + + +-- returns the first encountered entity element that is referencing `ca_id` +-- otherwise, returns nil, err +local function check_ca_references(ca_id) + for _, entity in ipairs(CA_CERT_REFERENCE_ENTITIES) do + for element, err in gen_iterator(entity) do + if err then + local msg = fmt("failed to list %s: %s", entity, err) + return nil, msg + end + + for _, id in ipairs(element.ca_certificates) do + if id == ca_id then + return entity, element + end + end + end + end + + if not reference_plugins then + reference_plugins = {} + loaded_plugins = loaded_plugins or kong.configuration.loaded_plugins + + for k, _ in pairs(CA_CERT_REFERENCE_PLUGINS) do + if loaded_plugins[k] then + reference_plugins[k] = true + end + end + end + + if next(reference_plugins) then + local entity = "plugins" + for element, err in gen_iterator(entity, reference_plugins) do + if err then + local msg = fmt("failed to list plugins: %s", err) + return nil, msg + end + + for _, id in ipairs(element.config.ca_certificates) do + if id == ca_id then + return entity, element + end + end + end + end +end + + +-- returns an array of entities that are referencing `ca_id` +-- return nil, err when error +-- Examples: +-- get_ca_certificate_references(ca_id, "services") +-- get_ca_certificate_references(ca_id, "plugins", "mtls-auth") +-- +-- Note we don't invalidate the ca store caches here directly because +-- different entities use different caches (kong.cache or kong.core_cache) +-- and use different functions to calculate the ca store cache key. +-- And it's not a good idea to depend on the plugin implementations in Core. +local function get_ca_certificate_references(ca_id, entity, plugin) + local elements = {} + + for element, err in gen_iterator(entity, plugin) do + if err then + local msg = fmt("failed to list %s: %s", entity, err) + return nil, msg + end + + local ca_certificates + if entity == "plugins" then + ca_certificates = element.config.ca_certificates + else + ca_certificates = element.ca_certificates + end + + for _, id in ipairs(ca_certificates) do + if id == ca_id then + tb_insert(elements, element) + end + end + end + + return elements +end + + return { init = init, find_certificate = find_certificate, @@ -378,4 +526,7 @@ return { execute = execute, get_certificate = get_certificate, get_ca_certificate_store = get_ca_certificate_store, + ca_ids_cache_key = ca_ids_cache_key, + check_ca_references = check_ca_references, + get_ca_certificate_references = get_ca_certificate_references, } diff --git a/kong/runloop/events.lua b/kong/runloop/events.lua index 6e6b42c0db37..6c02cbd110f7 100644 --- a/kong/runloop/events.lua +++ b/kong/runloop/events.lua @@ -319,6 +319,32 @@ local function crud_wasm_handler(data, schema_name) end +local function crud_ca_certificates_handler(data) + if data.operation ~= "update" then + return + end + + log(DEBUG, "[events] Ca_certificates updated, invalidating ca certificate store caches for services") + + local elements, err = certificate.get_ca_certificate_references(data.entity.id, "services") + if err then + log(ERR, "[events] failed to get ca certificate references, ", err) + end + + if elements then + local done_keys = {} + for _, e in ipairs(elements) do + local key = certificate.ca_ids_cache_key(e.ca_certificates) + + if not done_keys[key] then + done_keys[key] = true + kong.core_cache:invalidate(key) + end + end + end +end + + local LOCAL_HANDLERS = { { "dao:crud", nil , dao_crud_handler }, @@ -338,6 +364,9 @@ local LOCAL_HANDLERS = { { "crud" , "filter_chains" , crud_wasm_handler }, { "crud" , "services" , crud_wasm_handler }, { "crud" , "routes" , crud_wasm_handler }, + + -- ca certificate store caches invalidations + { "crud" , "ca_certificates" , crud_ca_certificates_handler }, } diff --git a/spec/02-integration/04-admin_api/16-ca_certificates_routes_spec.lua b/spec/02-integration/04-admin_api/16-ca_certificates_routes_spec.lua index 10d81b88a3b3..f70db23271eb 100644 --- a/spec/02-integration/04-admin_api/16-ca_certificates_routes_spec.lua +++ b/spec/02-integration/04-admin_api/16-ca_certificates_routes_spec.lua @@ -42,6 +42,7 @@ for _, strategy in helpers.each_strategy() do lazy_setup(function() bp, db = helpers.get_db_utils(strategy, { "ca_certificates", + "services", }) assert(helpers.start_kong { @@ -148,6 +149,32 @@ for _, strategy in helpers.each_strategy() do ca = assert(bp.ca_certificates:insert()) end) + it("not allowed to delete if it is referenced by other entities", function() + -- add a service that references the ca + local res = client:post("/services/", { + body = { + url = "https://" .. helpers.mock_upstream_host .. ":" .. helpers.mock_upstream_port, + protocol = "https", + ca_certificates = { ca.id }, + }, + headers = { ["Content-Type"] = "application/json" }, + }) + local body = assert.res_status(201, res) + local service = cjson.decode(body) + + helpers.wait_for_all_config_update() + + local res = client:delete("/ca_certificates/" .. ca.id) + + local body = assert.res_status(400, res) + local json = cjson.decode(body) + + assert.equal("ca_certificate " .. ca.id .. " is still referenced by services (id = " .. service.id .. ")", json.message) + + local res = client:delete("/services/" .. service.id) + assert.res_status(204, res) + end) + it("works", function() local res = client:delete("/ca_certificates/" .. ca.id) assert.res_status(204, res) diff --git a/spec/02-integration/05-proxy/18-upstream_tls_spec.lua b/spec/02-integration/05-proxy/18-upstream_tls_spec.lua index ec1723d9a71e..df51053ffb0f 100644 --- a/spec/02-integration/05-proxy/18-upstream_tls_spec.lua +++ b/spec/02-integration/05-proxy/18-upstream_tls_spec.lua @@ -3,6 +3,37 @@ local ssl_fixtures = require "spec.fixtures.ssl" local atc_compat = require "kong.router.compat" +local other_ca_cert = [[ +-----BEGIN CERTIFICATE----- +MIIEvjCCAqagAwIBAgIJALabx/Nup200MA0GCSqGSIb3DQEBCwUAMBMxETAPBgNV +BAMMCFlvbG80Mi4xMCAXDTE5MDkxNTE2Mjc1M1oYDzIxMTkwODIyMTYyNzUzWjAT +MREwDwYDVQQDDAhZb2xvNDIuMTCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoC +ggIBANIW67Ay0AtTeBY2mORaGet/VPL5jnBRz0zkZ4Jt7fEq3lbxYaJBnFI8wtz3 +bHLtLsxkvOFujEMY7HVd+iTqbJ7hLBtK0AdgXDjf+HMmoWM7x0PkZO+3XSqyRBbI +YNoEaQvYBNIXrKKJbXIU6higQaXYszeN8r3+RIbcTIlZxy28msivEGfGTrNujQFc +r/eyf+TLHbRqh0yg4Dy/U/T6fqamGhFrjupRmOMugwF/BHMH2JHhBYkkzuZLgV2u +7Yh1S5FRlh11am5vWuRSbarnx72hkJ99rUb6szOWnJKKew8RSn3CyhXbS5cb0QRc +ugRc33p/fMucJ4mtCJ2Om1QQe83G1iV2IBn6XJuCvYlyWH8XU0gkRxWD7ZQsl0bB +8AFTkVsdzb94OM8Y6tWI5ybS8rwl8b3r3fjyToIWrwK4WDJQuIUx4nUHObDyw+KK ++MmqwpAXQWbNeuAc27FjuJm90yr/163aGuInNY5Wiz6CM8WhFNAi/nkEY2vcxKKx +irSdSTkbnrmLFAYrThaq0BWTbW2mwkOatzv4R2kZzBUOiSjRLPnbyiPhI8dHLeGs +wMxiTXwyPi8iQvaIGyN4DPaSEiZ1GbexyYFdP7sJJD8tG8iccbtJYquq3cDaPTf+ +qv5M6R/JuMqtUDheLSpBNK+8vIe5e3MtGFyrKqFXdynJtfHVAgMBAAGjEzARMA8G +A1UdEwQIMAYBAf8CAQAwDQYJKoZIhvcNAQELBQADggIBAK0BmL5B1fPSMbFy8Hbc +/ESEunt4HGaRWmZZSa/aOtTjhKyDXLLJZz3C4McugfOf9BvvmAOZU4uYjfHTnNH2 +Z3neBkdTpQuJDvrBPNoCtJns01X/nuqFaTK/Tt9ZjAcVeQmp51RwhyiD7nqOJ/7E +Hp2rC6gH2ABXeexws4BDoZPoJktS8fzGWdFBCHzf4mCJcb4XkI+7GTYpglR818L3 +dMNJwXeuUsmxxKScBVH6rgbgcEC/6YwepLMTHB9VcH3X5VCfkDIyPYLWmvE0gKV7 +6OU91E2Rs8PzbJ3EuyQpJLxFUQp8ohv5zaNBlnMb76UJOPR6hXfst5V+e7l5Dgwv +Dh4CeO46exmkEsB+6R3pQR8uOFtubH2snA0S3JA1ji6baP5Y9Wh9bJ5McQUgbAPE +sCRBFoDLXOj3EgzibohC5WrxN3KIMxlQnxPl3VdQvp4gF899mn0Z9V5dAsGPbxRd +quE+DwfXkm0Sa6Ylwqrzu2OvSVgbMliF3UnWbNsDD5KcHGIaFxVC1qkwK4cT3pyS +58i/HAB2+P+O+MltQUDiuw0OSUFDC0IIjkDfxLVffbF+27ef9C5NG81QlwTz7TuN +zeigcsBKooMJTszxCl6dtxSyWTj7hJWXhy9pXsm1C1QulG6uT4RwCa3m0QZoO7G+ +6Wu6lP/kodPuoNubstIuPdi2 +-----END CERTIFICATE----- +]] + local fixtures = { http_mock = { upstream_mtls = [[ @@ -952,6 +983,129 @@ for _, strategy in helpers.each_strategy() do assert.equals("it works", body) end end) + + it("#db request is not allowed through once the CA certificate is updated to other ca", function() + local res = assert(admin_client:patch("/ca_certificates/" .. ca_certificate.id, { + body = { + cert = other_ca_cert, + }, + headers = { ["Content-Type"] = "application/json" }, + })) + + assert.res_status(200, res) + + wait_for_all_config_update(subsystems) + + local body + helpers.wait_until(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 + return pcall(function() + body = assert.res_status(502, res) + assert(proxy_client:close()) + end) + else + return pcall(function() + assert.equals("connection reset by peer", err) + assert(proxy_client:close()) + end) + end + end, 10) + + if subsystems == "http" then + assert.matches("An invalid response was received from the upstream server", body) + end + + -- buffered_proxying + if subsystems == "http" then + helpers.wait_until(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", + } + } + + return pcall(function() + body = assert.res_status(502, res) + assert(proxy_client:close()) + end) + end, 10) + assert.matches("An invalid response was received from the upstream server", body) + end + end) + + it("#db request is allowed through once the CA certificate is updated back to the correct ca", function() + local res = assert(admin_client:patch("/ca_certificates/" .. ca_certificate.id, { + body = { + cert = ssl_fixtures.cert_ca, + }, + headers = { ["Content-Type"] = "application/json" }, + })) + + assert.res_status(200, res) + + wait_for_all_config_update(subsystems) + + local body + helpers.wait_until(function() + local proxy_client = get_proxy_client(subsystems, 19001) + local path + if subsystems == "http" then + path = "/tls" + else + path = "/" + end + local res = proxy_client:send { + path = path, + headers = { + ["Host"] = "example.com", + } + } + + return pcall(function() + body = assert.res_status(200, res) + assert(proxy_client:close()) + end) + end, 10) + + assert.equals("it works", body) + + -- buffered_proxying + if subsystems == "http" then + helpers.wait_until(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", + } + } + + return pcall(function() + body = assert.res_status(200, res) + assert(proxy_client:close()) + end) + end, 10) + assert.equals("it works", body) + end + end) end) describe("#db tls_verify_depth", function() @@ -1004,19 +1158,17 @@ for _, strategy in helpers.each_strategy() do } } - return pcall(function() - if subsystems == "http" then - return pcall(function() - body = assert.res_status(502, res) - assert(proxy_client:close()) - end) - else - return pcall(function() - assert.equals("connection reset by peer", err) - assert(proxy_client:close()) - end) - end - end) + if subsystems == "http" then + return pcall(function() + body = assert.res_status(502, res) + assert(proxy_client:close()) + end) + else + return pcall(function() + assert.equals("connection reset by peer", err) + assert(proxy_client:close()) + end) + end end, 10) if subsystems == "http" then assert.matches("An invalid response was received from the upstream server", body)