Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(keys): add a new field x5t to the entity keys #14154

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/unreleased/kong/feat-keys-adds-x5t.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
Added a new field `x5t` to the entity `keys`.
type: feature
scope: Core
2 changes: 2 additions & 0 deletions kong-3.10.0-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,11 @@ build = {
["kong.db.strategies.postgres.tags"] = "kong/db/strategies/postgres/tags.lua",
["kong.db.strategies.postgres.services"] = "kong/db/strategies/postgres/services.lua",
["kong.db.strategies.postgres.plugins"] = "kong/db/strategies/postgres/plugins.lua",
["kong.db.strategies.postgres.keys"] = "kong/db/strategies/postgres/keys.lua",
["kong.db.strategies.off"] = "kong/db/strategies/off/init.lua",
["kong.db.strategies.off.connector"] = "kong/db/strategies/off/connector.lua",
["kong.db.strategies.off.tags"] = "kong/db/strategies/off/tags.lua",
["kong.db.strategies.off.keys"] = "kong/db/strategies/off/keys.lua",

["kong.db.migrations.state"] = "kong/db/migrations/state.lua",
["kong.db.migrations.subsystems"] = "kong/db/migrations/subsystems.lua",
Expand Down
25 changes: 25 additions & 0 deletions kong/clustering/compat/checkers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,31 @@ do
end

local compatible_checkers = {
{ 3010000000, --[[ 3.10.0.0 ]]
function(config_table, dp_version, log_suffix)
local config_keys = config_table["keys"]
if not config_keys then
return nil
end

local has_update
for _, t in ipairs(config_keys) do
if t["x5t"] ~= nil then
t["x5t"] = nil
has_update = true
end
end

if has_update then
log_warn_message("contains configuration 'keys.x5t'",
"be removed",
dp_version,
log_suffix)
end

return has_update
end
},
{ 3008000000, --[[ 3.8.0.0 ]]
function (config_table, dp_version, log_suffix)
local has_update
Expand Down
21 changes: 21 additions & 0 deletions kong/db/dao/keys.lua
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,26 @@ function keys:get_privkey(key)
return _get_key(key, "private")
end

-- @x5t: the x5t of key to be searched
-- @set_id: the id of the set the searched key belongs to
-- @return the key entity
function keys:select_by_x5t_set_id(x5t, set_id)
local x5t_type = type(x5t)
local set_id_type = type(set_id)
if x5t_type ~= "string" then
return nil, "parameter `x5t` must be of type string"
end
if set_id_type ~= "string" and set_id_type ~= "nil" then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

miswritten?

Suggested change
if set_id_type ~= "string" and set_id_type ~= "nil" then
if set_id_type ~= "string" or set_id_type ~= "nil" then

return nil, "parameter `set_id` must be of type string or nil"
end

local key, err = self.strategy:select_by_x5t_set_id(x5t, set_id)
if not key then
return nil, err
end

return self:row_to_entity(key), nil
end


return keys
10 changes: 10 additions & 0 deletions kong/db/migrations/core/025_390_to_3100.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ return {
DROP INDEX IF EXISTS clustering_sync_delta_version_idx;
END;
$$;

DO $$
BEGIN
ALTER TABLE IF EXISTS ONLY "keys" ADD "x5t" TEXT;
ALTER TABLE IF EXISTS ONLY "keys" ADD CONSTRAINT "keys_x5t_set_id_unique" UNIQUE ("x5t", "set_id");
CREATE UNIQUE INDEX IF NOT EXISTS "keys_x5t_with_null_set_id_idx" ON "keys" ("x5t") WHERE "set_id" IS NULL;
EXCEPTION WHEN DUPLICATE_COLUMN THEN
-- Do nothing, accept existing state
END;
$$;
]]
}
}
1 change: 1 addition & 0 deletions kong/db/migrations/core/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ return {
"022_350_to_360",
"023_360_to_370",
"024_380_to_390",
"025_390_to_3100",
}
15 changes: 14 additions & 1 deletion kong/db/schema/entities/keys.lua
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ return {
unique = false,
},
},
{
x5t = {
type = "string",
description = "X.509 certificate SHA-1 thumbprint.",
required = false,
unique = false,
},
},
{
jwk = {
-- type string but validate against typedefs.jwk
Expand Down Expand Up @@ -73,7 +81,7 @@ return {
at_least_one_of = supported_key_formats
},
{ custom_entity_check = {
field_sources = { "jwk", "pem", "kid" },
field_sources = { "jwk", "pem", "kid", "x5t" },
fn = function(entity)
-- JWK validation
if type(entity.jwk) == "string" then
Expand All @@ -98,6 +106,11 @@ return {
return nil, "kid in jwk.kid must be equal to keys.kid"
end

-- For JWK the `jwk.x5t` must match the `x5t` from the upper level
if entity.x5t and json_jwk.kid ~= entity.kid then
return nil, "x5t in jwk.x5t must be equal to keys.x5t"
end

-- running customer_validator
local ok, val_err = typedefs.jwk.custom_validator(entity.jwk)
if not ok or val_err then
Expand Down
24 changes: 24 additions & 0 deletions kong/db/strategies/off/keys.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
local Keys = {}

function Keys:select_by_x5t_set_id(x5t, set_id)
local PAGE_SIZE = 100
local next_offset = nil
local rows, err

repeat
rows, err, next_offset = self:page(PAGE_SIZE, next_offset)
if err then
return nil, err
end
for _, row in ipairs(rows) do
if row.x5t == x5t and row.set.id == set_id then
return row
end
end

until next_offset == nil

return nil
end

return Keys
27 changes: 27 additions & 0 deletions kong/db/strategies/postgres/keys.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
local kong = kong
local fmt = string.format

local Keys = {}

function Keys:select_by_x5t_set_id(x5t, set_id)
local qs
if set_id then
qs = fmt(
"SELECT * FROM keys WHERE x5t = %s AND set_id = %s;",
kong.db.connector:escape_literal(x5t),
kong.db.connector:escape_literal(set_id))
else
qs = fmt(
"SELECT * FROM keys WHERE x5t = %s AND set_id IS NULL;",
kong.db.connector:escape_literal(x5t))
end

local res, err = kong.db.connector:query(qs, "read")
if err then
return nil, err
end

return res and res[1]
end

return Keys
35 changes: 35 additions & 0 deletions spec/01-unit/19-hybrid/03-compat_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ local helpers = require ("spec.helpers")
local declarative = require("kong.db.declarative")
local inflate_gzip = require("kong.tools.gzip").inflate_gzip
local cjson_decode = require("cjson.safe").decode
local cjson_encode = require("cjson.safe").encode
local ssl_fixtures = require ("spec.fixtures.ssl")
local merge = kong.table.merge

local function reset_fields()
compat._set_removed_fields(require("kong.clustering.compat.removed_fields"))
Expand Down Expand Up @@ -419,6 +421,7 @@ describe("kong.clustering.compat", function()
"plugins",
"consumers",
"upstreams",
"keys",
})
_G.kong.db = db

Expand All @@ -437,10 +440,33 @@ describe("kong.clustering.compat", function()
cert = ssl_fixtures.cert_ca,
}

local jwk_pub, jwk_priv = helpers.generate_keys("JWK")
local pem_pub, pem_priv = helpers.generate_keys("PEM")
local jwk = merge(cjson_decode(jwk_pub), cjson_decode(jwk_priv))


assert(declarative.load_into_db({
ca_certificates = { [ca_certificate_def.id] = ca_certificate_def },
certificates = { [certificate_def.id] = certificate_def },
keys = {
key1 = {
id = "f0383152-a6b4-4351-983b-1844b14170c1",
kid = jwk.kid,
name = "key1",
x5t = "x5t1",
jwk = cjson_encode(jwk)
},
key2 = {
id = "f0383152-a6b4-4351-983b-1844b14170c2",
kid = "kid2",
name = "key2",
x5t = "x5t2",
pem = {
public_key = pem_pub,
private_key = pem_priv
}
}
},
upstreams = {
upstreams1 = {
id = "01a2b3c4-d5e6-f7a8-b9c0-d1e2f3a4b5c6",
Expand Down Expand Up @@ -640,6 +666,15 @@ describe("kong.clustering.compat", function()
assert.is_nil(assert(services[3]).ca_certificates)
end)

it("key.x5t", function()
local has_update, result = compat.update_compatible_payload(config, "3.9.0", "test_")
assert.truthy(has_update)
result = cjson_decode(inflate_gzip(result)).config_table

local keys = assert(assert(assert(result).keys))
assert.is_nil(assert(keys[1]).x5t)
assert.is_nil(assert(keys[2]).x5t)
end)
end) -- describe

describe("route entities compatible changes", function()
Expand Down
82 changes: 81 additions & 1 deletion spec/02-integration/03-db/18-keys_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ local fmt = string.format

for _, strategy in helpers.all_strategies() do
describe("db.keys #" .. strategy, function()
local init_key_set, init_pem_key, pem_pub, pem_priv, jwk
local init_key_set, key_set2, init_pem_key, pem_pub, pem_priv, jwk
local bp, db

lazy_setup(function()
Expand All @@ -22,6 +22,10 @@ for _, strategy in helpers.all_strategies() do
name = "testset",
})

key_set2 = assert(bp.key_sets:insert {
name = "testset2"
})

local jwk_pub, jwk_priv = helpers.generate_keys("JWK")
pem_pub, pem_priv = helpers.generate_keys("PEM")

Expand Down Expand Up @@ -249,5 +253,81 @@ for _, strategy in helpers.all_strategies() do
assert.is_nil(jwk_priv)
assert.matches("could not load a private key from public key material", p_err)
end)

it(":insert key with x5t", function()
local key, err = db.keys:insert {
name = "insert_key1",
set = init_key_set,
kid = "insert_kid1",
x5t = "insert_x5t1",
pem = { private_key = pem_priv, public_key = pem_pub }
}
assert.is_nil(err)
assert(key)

key, err = db.keys:insert({
name = "insert_key2",
set = init_key_set,
kid = "insert_kid2",
x5t = "insert_x5t1",
pem = { private_key = pem_priv, public_key = pem_pub }
})
assert.is_nil(key)
assert.matches("violation on", err)

key, err = db.keys:insert {
name = "insert_key3",
set = key_set2,
kid = "insert_kid3",
x5t = "insert_x5t1",
pem = { private_key = pem_priv, public_key = pem_pub }
}
assert.is_nil(err)
assert(key)

-- x5t should be unique even when set is not specified
local key, err = db.keys:insert {
name = "insert_key4",
kid = "insert_kid1",
x5t = "insert_x5t1",
pem = { private_key = pem_priv, public_key = pem_pub }
}
assert.is_nil(err)
assert(key)

key, err = db.keys:insert({
name = "insert_key5",
kid = "insert_kid2",
x5t = "insert_x5t1", -- unique voilation
pem = { private_key = pem_priv, public_key = pem_pub }
})
assert.is_nil(key)
assert.matches("violation on", err)
end)

it(":select key by x5t", function()
local key, err = db.keys:insert {
name = "select_key1",
set = init_key_set,
kid = "select_kid1",
x5t = "select_x5t1",
pem = { private_key = pem_priv, public_key = pem_pub }
}
assert.is_nil(err)
assert(key)

local key2, err2 = db.keys:select_by_x5t_set_id("select_x5t1", init_key_set.id)
assert.is_nil(err2)
assert.same("select_key1", key2.name)
assert.same("select_x5t1", key2.x5t)

local key3, err3 = db.keys:select_by_x5t_set_id("select_x5t1", key_set2.id) -- inexistent
assert.is_nil(err3)
assert.is_nil(key3)

local key4, err4 = db.keys:select_by_x5t_set_id("select_x5t1") -- inexistent
assert.is_nil(err4)
assert.is_nil(key4)
end)
end)
end
Loading
Loading