fix(certificate): hybrid dp cannot refresh certificate entity with va…

…ult reference (#12868)

This PR fixes a problem that a certificate entity with cert/keys stored in a vault-referenced type cannot be refreshed even if the vault reference secret value has been updated both in L1/L2 vault PDK.

FTI-5881
---------

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
Co-authored-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>

changelog/unreleased/kong/fix-hybrid-dp-certificate-with-vault-not-refresh.yml

@@ -0,0 +1,3 @@
+message: Fixed a problem that in hybrid DP mode a certificate entity configured with vault reference may not get refreshed on time
+type: bugfix
+scope: Core

kong/runloop/certificate.lua

@@ -46,43 +46,88 @@ local function log(lvl, ...)
   ngx_log(lvl, "[ssl] ", ...)
 end
 
-local function parse_key_and_cert(row)
-  if row == false then
-    return default_cert_and_key
+
+local function parse_cert(cert, parsed)
+  if cert == nil then
+    return nil, nil, parsed
   end
 
-  -- parse cert and priv key for later usage by ngx.ssl
+  if type(cert) == "cdata" then
+    return cert, nil, parsed
+  end
 
-  local cert, err = parse_pem_cert(row.cert)
+  local err
+  cert, err = parse_pem_cert(cert)
   if not cert then
     return nil, "could not parse PEM certificate: " .. err
   end
+  return cert, nil, true
+end
+
+
+
+local function parse_key(key, parsed)
+  if key == nil then
+    return nil, nil, parsed
+  end
+
+  if type(key) == "cdata" then
+    return key, nil, parsed
+  end
 
-  local key, err = parse_pem_priv_key(row.key)
+  local err
+  key, err = parse_pem_priv_key(key)
   if not key then
     return nil, "could not parse PEM private key: " .. err
   end
+  return key, nil, true
+end
 
-  local cert_alt
-  local key_alt
-  if row.cert_alt and row.key_alt then
-    cert_alt, err = parse_pem_cert(row.cert_alt)
-    if not cert_alt then
-      return nil, "could not parse alternate PEM certificate: " .. err
-    end
 
-    key_alt, err = parse_pem_priv_key(row.key_alt)
-    if not key_alt then
-      return nil, "could not parse alternate PEM private key: " .. err
+local function parse_key_and_cert(row)
+  if row == false then
+    return default_cert_and_key
+  end
+
+  -- parse cert and priv key for later usage by ngx.ssl
+
+  local err, parsed
+  local key, key_alt
+  local cert, cert_alt
+
+  cert, err, parsed = parse_cert(row.cert)
+  if err then
+    return nil, err
+  end
+
+  key, err, parsed = parse_key(row.key, parsed)
+  if err then
+    return nil, err
+  end
+
+  cert_alt, err, parsed = parse_cert(row.cert_alt, parsed)
+  if err then
+    return nil, err
+  end
+
+  if cert_alt then
+    key_alt, err, parsed = parse_key(row.key_alt, parsed)
+    if err then
+      return nil, err
     end
   end
 
-  return {
-    cert = cert,
-    key = key,
-    cert_alt = cert_alt,
-    key_alt = key_alt,
-  }
+  if parsed then
+    return {
+      cert = cert,
+      key = key,
+      cert_alt = cert_alt,
+      key_alt = key_alt,
+      ["$refs"] = row["$refs"],
+    }
+  end
+
+  return row
 end
 
 
@@ -213,8 +258,8 @@ local function get_certificate(pk, sni_name)
                                                           fetch_certificate,
                                                           pk, sni_name)
 
-  if certificate and hit_level ~= 3 then
-    kong.vault.update(certificate)
+  if certificate and hit_level ~= 3 and certificate["$refs"] then
+    certificate = parse_key_and_cert(kong.vault.update(certificate))
   end
 
   return certificate, err

spec/02-integration/05-proxy/18-upstream_tls_spec.lua

@@ -309,13 +309,13 @@ for _, strategy in helpers.each_strategy() do
     lazy_teardown(function()
       helpers.stop_kong()
     end)
-  
+
     local function get_tls_service_id(subsystems)
       if subsystems == "http" then
         return service_mtls.id
       else
         return tls_service_mtls.id
-      end      
+      end
     end
 
     local function get_proxy_client(subsystems, stream_port)
@@ -326,7 +326,7 @@ for _, strategy in helpers.each_strategy() do
       end
     end
 
-    local function wait_for_all_config_update(subsystems) 
+    local function wait_for_all_config_update(subsystems)
       local opt = {}
       if subsystems == "stream" then
         opt.stream_enabled = true
@@ -881,7 +881,7 @@ for _, strategy in helpers.each_strategy() do
               end)
             end
           end, 10)
-          
+
           if subsystems == "http" then
             assert.matches("An invalid response was received from the upstream server", body)
           end

spec/02-integration/13-vaults/05-ttl_spec.lua

@@ -1,4 +1,5 @@
 local helpers = require "spec.helpers"
+local ssl_fixtures = require "spec.fixtures.ssl"
 
 -- using the full path so that we don't have to modify package.path in
 -- this context
@@ -221,5 +222,200 @@ describe("vault ttl and rotation (#" .. strategy .. ") #" .. vault.name, functio
   end)
 end)
 
+
+describe("#hybrid mode dp vault ttl and rotation (#" .. strategy .. ") #" .. vault.name, function()
+  local client
+  local admin_client
+  local secret = "my-secret"
+  local certificate
+
+  local tls_fixtures = {
+    http_mock = {
+      upstream_tls = [[
+        server {
+            server_name example.com;
+            listen 16799 ssl;
+
+            ssl_certificate         ../spec/fixtures/mtls_certs/example.com.crt;
+            ssl_certificate_key     ../spec/fixtures/mtls_certs/example.com.key;
+            ssl_client_certificate  ../spec/fixtures/mtls_certs/ca.crt;
+            ssl_verify_client      on;
+            ssl_verify_depth       3;
+            ssl_session_tickets    off;
+            ssl_session_cache      off;
+            keepalive_requests     0;
+
+            location = / {
+                echo 'it works';
+            }
+        }
+      ]]
+    },
+  }
+
+  tls_fixtures.dns_mock = helpers.dns_mock.new({mocks_only = true})
+  tls_fixtures.dns_mock:A {
+    name = "example.com",
+    address = "127.0.0.1",
+  }
+
+  local vault_fixtures = vault:fixtures()
+  vault_fixtures.dns_mock = tls_fixtures.dns_mock
+
+  lazy_setup(function()
+    helpers.setenv("KONG_LUA_PATH_OVERRIDE", LUA_PATH)
+    helpers.setenv("KONG_VAULT_ROTATION_INTERVAL", "1")
+
+    vault:setup()
+    vault:create_secret(secret, ssl_fixtures.key_alt)
+
+    local bp = helpers.get_db_utils(strategy,
+                                    { "vaults", "routes", "services", "certificates", "ca_certificates" },
+                                    {},
+                                    { vault.name })
+
+
+    assert(bp.vaults:insert({
+      name     = vault.name,
+      prefix   = vault.prefix,
+      config   = vault.config,
+    }))
+
+    -- Prepare TLS upstream service
+    -- cert_alt & key_alt pair is not a correct client certificate
+    -- and it will fail the client TLS verification on server side
+    --
+    -- On the other hand, cert_client & key_client pair is a correct
+    -- client certificate
+    certificate = assert(bp.certificates:insert({
+      key = ssl_fixtures.key_alt,
+      cert = ssl_fixtures.cert_alt,
+    }))
+
+    local service_tls = assert(bp.services:insert({
+      name = "tls-service",
+      url = "https://example.com:16799",
+      client_certificate = certificate,
+    }))
+
+    assert(bp.routes:insert({
+      name      = "tls-route",
+      hosts     = { "example.com" },
+      paths = { "/tls", },
+      service   = { id = service_tls.id },
+    }))
+
+    assert(helpers.start_kong({
+      role = "control_plane",
+      cluster_cert = "spec/fixtures/kong_clustering.crt",
+      cluster_cert_key = "spec/fixtures/kong_clustering.key",
+      database = strategy,
+      prefix = "vault_ttl_test_cp",
+      cluster_listen = "127.0.0.1:9005",
+      admin_listen = "127.0.0.1:9001",
+      nginx_conf = "spec/fixtures/custom_nginx.template",
+      vaults         = vault.name,
+      plugins        = "dummy",
+      log_level      = "debug",
+    }, nil, nil, tls_fixtures ))
+
+    assert(helpers.start_kong({
+      role = "data_plane",
+      database = "off",
+      prefix = "vault_ttl_test_dp",
+      vaults         = vault.name,
+      plugins        = "dummy",
+      log_level      = "debug",
+      nginx_conf = "spec/fixtures/custom_nginx.template",
+      cluster_cert = "spec/fixtures/kong_clustering.crt",
+      cluster_cert_key = "spec/fixtures/kong_clustering.key",
+      cluster_control_plane = "127.0.0.1:9005",
+      proxy_listen = "127.0.0.1:9002",
+      nginx_worker_processes = 1,
+    }, nil, nil, vault_fixtures ))
+
+    admin_client = helpers.admin_client(nil, 9001)
+    client = helpers.proxy_client(nil, 9002)
+  end)
+
+
+  lazy_teardown(function()
+    if client then
+      client:close()
+    end
+    if admin_client then
+      admin_client:close()
+    end
+
+    helpers.stop_kong("vault_ttl_test_cp")
+    helpers.stop_kong("vault_ttl_test_dp")
+    vault:teardown()
+
+    helpers.unsetenv("KONG_LUA_PATH_OVERRIDE")
+  end)
+
+
+  it("updates plugin config references (backend: #" .. vault.name .. ")", function()
+    helpers.wait_for_all_config_update({
+      forced_admin_port = 9001,
+      forced_proxy_port = 9002,
+    })
+    -- Wrong cert-key pair is being used in the pre-configured cert object
+    local res = client:get("/tls", {
+      headers = {
+        host = "example.com",
+      },
+      timeout = 2,
+    })
+    local body = assert.res_status(400, res)
+    assert.matches("The SSL certificate error", body)
+
+    -- Switch to vault referenced key field
+    local res = assert(admin_client:patch("/certificates/"..certificate.id, {
+      body = {
+        key = fmt("{vault://%s/%s?ttl=%s}", vault.prefix, secret, 2),
+        cert = ssl_fixtures.cert_client,
+      },
+      headers = {
+        ["Content-Type"] = "application/json",
+      },
+    }))
+    assert.res_status(200, res)
+    helpers.wait_for_all_config_update({
+      forced_admin_port = 9001,
+      forced_proxy_port = 9002,
+    })
+
+    -- Assume wrong cert-key pair still being used
+    local res = client:get("/tls", {
+      headers = {
+        host = "example.com",
+      },
+      timeout = 2,
+    })
+
+    local body = assert.res_status(400, res)
+    assert.matches("No required SSL certificate was sent", body)
+
+    -- Update secret value and let cert be correct
+    vault:update_secret(secret, ssl_fixtures.key_client, { ttl = 2 })
+    assert.with_timeout(7)
+          .with_step(0.5)
+          .ignore_exceptions(true)
+          .eventually(function()
+            local res = client:get("/tls", {
+              headers = {
+                host = "example.com",
+              },
+              timeout = 2,
+            })
+
+            local body = assert.res_status(200, res)
+            assert.matches("it works", body)
+            return true
+          end).is_truthy("Expected certificate being refreshed")
+  end)
+end)
+
 end -- each vault backend
 end -- each strategy