Skip to content

Commit

Permalink
feat(acme): allow to store account_key in keys and key_sets (#9746)
Browse files Browse the repository at this point in the history
There is currently no way to configure an existing private key to be used for the acme plugin. If the account does not have a key in storage, then a new one is created. This is a problem when external account binding credentials have already been associated with a key elsewhere (e.g. cert-manager or cert-bot). Some issuers like ZeroSSL allow for EAB credentials to be reused and multiple accounts can be created with different private keys. Others like GCP Public CA only allow for EAB credentials to be associated with one key so when the plugin tries to create a new account with a new key it fails.

The current workaround for this has been to manually insert the key in storage so a new key is not created.

This PR exposes account_key and will use that if the account was not found in storage and the value of account_key is not nil. Otherwise, it will generate a key on the fly as it does currently.
  • Loading branch information
szesch authored Feb 28, 2023
1 parent 42411cc commit f14d68a
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 6 deletions.
6 changes: 6 additions & 0 deletions kong/clustering/compat/removed_fields.lua
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,10 @@ return {
"phase_duration_flavor",
}
},
-- Any dataplane older than 3.3.0
[3003000000] = {
acme = {
"account_key",
}
},
}
52 changes: 50 additions & 2 deletions kong/plugins/acme/client.lua
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,42 @@ local function store_renew_config(conf, host)
return err
end

local function get_account_key(conf)
local kid = conf.key_id
local lookup = {kid = kid}

if conf.key_set then
local key_set, key_set_err = kong.db.key_sets:select_by_name(conf.key_set)

if key_set_err then
kong.log.warn("error loading keyset ", conf.key_set, " : ", key_set_err)
return nil, key_set_err
end

if not key_set then
kong.log.warn("could not load keyset nil value was returned")
return nil, error("nil returned by key_sets:select_by_name for key_set ", conf.key_set)
end

lookup.set = {id = key_set.id}
end

local cache_key = kong.db.keys:cache_key(lookup)
local key, key_err = kong.db.keys:select_by_cache_key(cache_key)

if key_err then
kong.log.warn("error loading key ", kid, " : ", key_err)
return nil, key_err
end

if not key then
kong.log.warn("could not load key nil value was returned")
return nil, error("nil returned by keys:select_by_cache_key for key ", conf.key_id)
end

return kong.db.keys:get_privkey(key)
end

local function create_account(conf)
local _, st, err = new_storage_adapter(conf)
if err then
Expand All @@ -236,8 +272,19 @@ local function create_account(conf)
elseif account then
return
end
-- no account yet, create one now
local pkey = util.create_pkey(4096, "RSA")

local pkey
if conf.account_key then
local account_key, err = get_account_key(conf.account_key)
if err then
return err
end

pkey = account_key
else
-- no account yet, create one now
pkey = util.create_pkey(4096, "RSA")
end

local err = st:set(account_name, cjson_encode({
key = pkey,
Expand Down Expand Up @@ -512,4 +559,5 @@ return {
_renew_certificate_storage = renew_certificate_storage,
_check_expire = check_expire,
_set_is_dbless = function(d) dbless = d end,
_create_account = create_account,
}
10 changes: 10 additions & 0 deletions kong/plugins/acme/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ local VAULT_STORAGE_SCHEMA = {
{ jwt_path = { type = "string" }, },
}

local ACCOUNT_KEY_SCHEMA = {
{ key_id = { type = "string", required = true }},
{ key_set = { type = "string" }}
}

local schema = {
name = "acme",
fields = {
Expand All @@ -71,6 +76,11 @@ local schema = {
encrypted = true, -- Kong Enterprise-exclusive feature, does nothing in Kong CE
referenceable = true,
}, },
{ account_key = {
type = "record",
required = false,
fields = ACCOUNT_KEY_SCHEMA,
}, },
{ api_uri = typedefs.url({ default = "https://acme-v02.api.letsencrypt.org/directory" }),
},
{ tos_accepted = {
Expand Down
165 changes: 165 additions & 0 deletions spec/03-plugins/29-acme/01-client_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ local cjson = require "cjson"
local pkey = require("resty.openssl.pkey")
local x509 = require("resty.openssl.x509")

local tablex = require "pl.tablex"

local client

local function new_cert_key_pair(expire)
Expand Down Expand Up @@ -90,6 +92,169 @@ for _, strategy in ipairs(strategies) do
end)
end

for _, strategy in ipairs(strategies) do
local account_name, account_key
local c, config, db

local KEY_ID = "123"
local KEY_SET_NAME = "key_set_foo"

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

lazy_setup(function()
client = require("kong.plugins.acme.client")
account_name = client._account_name(proper_config)
end)

describe("Plugin: acme (client.create_account) [#" .. strategy .. "]", function()
describe("create with preconfigured account_key with key_set", function()
lazy_setup(function()
account_key = {key_id = KEY_ID, key_set = KEY_SET_NAME}
config = tablex.deepcopy(proper_config)
config.account_key = account_key
c = client.new(config)

_, db = helpers.get_db_utils(strategy ~= "off" and strategy or nil, {"keys", "key_sets"})

local ks, err = assert(db.key_sets:insert({name = KEY_SET_NAME}))
assert.is_nil(err)

local k, err = db.keys:insert({
name = "Test PEM",
pem = {
private_key = pem_priv,
public_key = pem_pub
},
set = ks,
kid = KEY_ID
})
assert(k)
assert.is_nil(err)
end)

lazy_teardown(function()
c.storage:delete(account_name)
end)

-- The first call should result in the account key being persisted.
it("persists account", function()
local err = client._create_account(config)
assert.is_nil(err)

local account, err = c.storage:get(account_name)
assert.is_nil(err)
assert.not_nil(account)

local account_data = cjson.decode(account)
assert.equal(account_data.key, pem_priv)
end)

-- The second call should be a nop because the key is found in the db.
-- Validate that the second call does not result in the key being changed.
it("skips persisting existing account", function()
local err = client._create_account(config)
assert.is_nil(err)

local account, err = c.storage:get(account_name)
assert.is_nil(err)
assert.not_nil(account)

local account_data = cjson.decode(account)
assert.equal(account_data.key, pem_priv)
end)
end)

describe("create with preconfigured account_key without key_set", function()
lazy_setup(function()
account_key = {key_id = KEY_ID}
config = tablex.deepcopy(proper_config)
config.account_key = account_key
c = client.new(config)

_, db = helpers.get_db_utils(strategy ~= "off" and strategy or nil, {"keys", "key_sets"})

local k, err = db.keys:insert({
name = "Test PEM",
pem = {
private_key = pem_priv,
public_key = pem_pub
},
kid = KEY_ID
})
assert(k)
assert.is_nil(err)
end)

lazy_teardown(function()
c.storage:delete(account_name)
end)

-- The first call should result in the account key being persisted.
it("persists account", function()
local err = client._create_account(config)
assert.is_nil(err)

local account, err = c.storage:get(account_name)
assert.is_nil(err)
assert.not_nil(account)

local account_data = cjson.decode(account)
assert.equal(account_data.key, pem_priv)
end)
end)

describe("create with generated account_key", function()
local i = 1
local account_keys = {}

lazy_setup(function()
config = tablex.deepcopy(proper_config)
c = client.new(config)

account_keys[1] = util.create_pkey()
account_keys[2] = util.create_pkey()

util.create_pkey = function(size, type)
local key = account_keys[i]
i = i + 1
return key
end
end)

lazy_teardown(function()
c.storage:delete(account_name)
end)

-- The first call should result in a key being generated and the account
-- should then be persisted.
it("persists account", function()
local err = client._create_account(config)
assert.is_nil(err)

local account, err = c.storage:get(account_name)
assert.is_nil(err)
assert.not_nil(account)

local account_data = cjson.decode(account)
assert.equal(account_data.key, account_keys[1])
end)

-- The second call should be a nop because the key is found in the db.
it("skip persisting existing account", function()
local err = client._create_account(config)
assert.is_nil(err)

local account, err = c.storage:get(account_name)
assert.is_nil(err)
assert.not_nil(account)

local account_data = cjson.decode(account)
assert.equal(account_data.key, account_keys[1])
end)
end)
end)
end

for _, strategy in helpers.each_strategy() do
describe("Plugin: acme (client.save) [#" .. strategy .. "]", function()
local bp, db
Expand Down
30 changes: 26 additions & 4 deletions spec/03-plugins/29-acme/04-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe("Plugin: acme (schema)", function()
},
----------------------------------------
{
name = "must accpet ToS for Let's Encrypt (unaccpeted,staging)",
name = "must accept ToS for Let's Encrypt (unaccepted,staging)",
input = {
account_email = "[email protected]",
api_uri = "https://acme-staging-v02.api.letsencrypt.org",
Expand All @@ -43,7 +43,7 @@ describe("Plugin: acme (schema)", function()
},
----------------------------------------
{
name = "must accpet ToS for Let's Encrypt (unaccpeted)",
name = "must accept ToS for Let's Encrypt (unaccepted)",
input = {
account_email = "[email protected]",
api_uri = "https://acme-v02.api.letsencrypt.org",
Expand All @@ -59,14 +59,36 @@ describe("Plugin: acme (schema)", function()
},
----------------------------------------
{
name = "must accpet ToS for Let's Encrypt (accepted)",
name = "must accept ToS for Let's Encrypt (accepted)",
input = {
account_email = "[email protected]",
api_uri = "https://acme-v02.api.letsencrypt.org",
tos_accepted = true,
},
},
----------------------------------------
{
name = "accepts valid account_key with key_set",
input = {
account_email = "[email protected]",
api_uri = "https://api.acme.org",
account_key = {
key_id = "123",
key_set = "my-key-set",
}
},
},
----------------------------------------
{
name = "accepts valid account_key without key_set",
input = {
account_email = "[email protected]",
api_uri = "https://api.acme.org",
account_key = {
key_id = "123",
}
},
},
}

for _, t in ipairs(tests) do
Expand All @@ -80,4 +102,4 @@ describe("Plugin: acme (schema)", function()
end
end)
end
end)
end)

1 comment on commit f14d68a

@khcp-gha-bot
Copy link

Choose a reason for hiding this comment

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

Bazel Build

Docker image available kong/kong:f14d68a9e8c2462cbf3ec830404a4da131d4a706
Artifacts available https://github.com/Kong/kong/actions/runs/4291740004

Please sign in to comment.