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(hmac-auth): add support for RSA signatures #11133

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
5 changes: 5 additions & 0 deletions CHANGELOG/unreleased/kong/11133.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
message: "hmac-auth: Added support for RSA signatures"
type: feature
scope: Plugin
prs:
- 11133
1 change: 1 addition & 0 deletions kong-3.5.0-0.rockspec
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ build = {
["kong.plugins.hmac-auth.migrations.000_base_hmac_auth"] = "kong/plugins/hmac-auth/migrations/000_base_hmac_auth.lua",
["kong.plugins.hmac-auth.migrations.002_130_to_140"] = "kong/plugins/hmac-auth/migrations/002_130_to_140.lua",
["kong.plugins.hmac-auth.migrations.003_200_to_210"] = "kong/plugins/hmac-auth/migrations/003_200_to_210.lua",
["kong.plugins.hmac-auth.migrations.004_330_to_340"] = "kong/plugins/hmac-auth/migrations/004_330_to_340.lua",
["kong.plugins.hmac-auth.handler"] = "kong/plugins/hmac-auth/handler.lua",
["kong.plugins.hmac-auth.access"] = "kong/plugins/hmac-auth/access.lua",
["kong.plugins.hmac-auth.schema"] = "kong/plugins/hmac-auth/schema.lua",
Expand Down
26 changes: 26 additions & 0 deletions kong/clustering/compat/checkers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,32 @@ local compatible_checkers = {
return has_update
end
},

{ 3004000000, --[[ 3.4.0.0 ]]
function(config_table, dp_version, log_suffix)
local config_hmacauth_credentials = config_table["hmacauth_credentials"]
if not config_hmacauth_credentials then
return nil
end

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

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

return has_update
end
},
}


Expand Down
5 changes: 4 additions & 1 deletion kong/clustering/compat/removed_fields.lua
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ return {
[3005000000] = {
cors = {
"private_network",
},
hmac_auth = {
"public_key",
}
}
},
}
69 changes: 51 additions & 18 deletions kong/plugins/hmac-auth/access.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
local constants = require "kong.constants"
local sha256 = require "resty.sha256"
local openssl_hmac = require "resty.openssl.hmac"
local openssl_pkey = require "resty.openssl.pkey"
local utils = require "kong.tools.utils"


Expand Down Expand Up @@ -31,18 +32,45 @@ local SIGNATURE_NOT_VALID = "HMAC signature cannot be verified"
local SIGNATURE_NOT_SAME = "HMAC signature does not match"
Copy link
Contributor

Choose a reason for hiding this comment

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

error message needs to be changed accordingly

Copy link
Contributor

Choose a reason for hiding this comment

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

The specific string format for the "Authorization" header that includes the method (e.g., "Hmac" or "Rsa") and the various components (e.g., "username," "algorithm," "headers," and "signature") may not be standardized in the HTTP Signatures draft (draft-cavage-http-signatures). The draft defines the concept of HTTP signatures and how they can be generated and verified, but it doesn't prescribe a specific format for the "Authorization" header.

The format you are using, which includes "Hmac" or "Rsa" as the method prefix, is a common convention used in some implementations for clarity and to distinguish between different authentication methods (HMAC and RSA, in this case). However, this specific format might not be explicitly detailed in the draft itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree. need to output the right error message



local function verify_rsa(public_key, signature, signing_string, md_alg)
local pub, err = openssl_pkey.new(public_key)
if not pub then
kong.log.err("failed to create public key : ", err)
return false
end

local verified, err = pub:verify(signature, signing_string, md_alg)
if not err then
return verified
else
kong.log.err("failed to verify signature : ", err)
return false
end
end


local rsa = {
["rsa-sha256"] = function(public_key, signature, signing_string)
return verify_rsa(public_key, signature, signing_string, "sha256")
end,
["rsa-sha512"] = function(public_key, signature, signing_string)
return verify_rsa(public_key, signature, signing_string, "sha512")
end,
}


local hmac = {
["hmac-sha1"] = function(secret, data)
return hmac_sha1(secret, data)
["hmac-sha1"] = function(secret, signature, signing_string)
return signature == hmac_sha1(secret, signing_string)
end,
["hmac-sha256"] = function(secret, data)
return openssl_hmac.new(secret, "sha256"):final(data)
["hmac-sha256"] = function(secret, signature, signing_string)
return signature == openssl_hmac.new(secret, "sha256"):final(signing_string)
end,
["hmac-sha384"] = function(secret, data)
return openssl_hmac.new(secret, "sha384"):final(data)
["hmac-sha384"] = function(secret, signature, signing_string)
return signature == openssl_hmac.new(secret, "sha384"):final(signing_string)
end,
["hmac-sha512"] = function(secret, data)
return openssl_hmac.new(secret, "sha512"):final(data)
["hmac-sha512"] = function(secret, signature, signing_string)
return signature == openssl_hmac.new(secret, "sha512"):final(signing_string)
end,
}

Expand Down Expand Up @@ -97,7 +125,7 @@ local function retrieve_hmac_fields(authorization_header)
-- parse the header to retrieve hamc parameters
if authorization_header then
local iterator, iter_err = re_gmatch(authorization_header,
"\\s*[Hh]mac\\s*username=\"(.+)\"," ..
"(\\s*[Hh]mac)?\\s*username=\"(.+)\"," ..
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to specify rsa here when using rsa signature for consistency?

"\\s*algorithm=\"(.+)\",\\s*header" ..
"s=\"(.+)\",\\s*signature=\"(.+)\"",
"jo")
Expand All @@ -113,10 +141,10 @@ local function retrieve_hmac_fields(authorization_header)
end

if m and #m >= 4 then
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we increment the check in the if as well?

Suggested change
if m and #m >= 4 then
if m and #m >= 5 then

hmac_params.username = m[1]
hmac_params.algorithm = m[2]
hmac_params.hmac_headers = utils.split(m[3], " ")
hmac_params.signature = m[4]
hmac_params.username = m[2]
hmac_params.algorithm = m[3]
hmac_params.hmac_headers = utils.split(m[4], " ")
hmac_params.signature = m[5]
end
end

Expand All @@ -126,7 +154,7 @@ end

-- plugin assumes the request parameters being used for creating
-- signature by client are not changed by core or any other plugin
local function create_hash(request_uri, hmac_params)
local function generate_signing_string(request_uri, hmac_params)
local signing_string = ""
local hmac_headers = hmac_params.hmac_headers

Expand Down Expand Up @@ -161,14 +189,18 @@ local function create_hash(request_uri, hmac_params)
end
end

return hmac[hmac_params.algorithm](hmac_params.secret, signing_string)
return signing_string
end


local function validate_signature(hmac_params)
local signature_1 = create_hash(kong_request.get_path_with_query(), hmac_params)
local signature_2 = decode_base64(hmac_params.signature)
return signature_1 == signature_2
local signature = decode_base64(hmac_params.signature)
local signing_string = generate_signing_string(kong_request.get_path_with_query(), hmac_params)
if hmac_params.algorithm:sub(1, 4) == "rsa-" then
return rsa[hmac_params.algorithm](hmac_params.public_key, signature, signing_string)
else
return hmac[hmac_params.algorithm](hmac_params.secret, signature, signing_string)
end
end


Expand Down Expand Up @@ -324,6 +356,7 @@ local function do_authentication(conf)
end

hmac_params.secret = credential.secret
hmac_params.public_key = credential.public_key

if not validate_signature(hmac_params) then
return false, { status = 401, message = SIGNATURE_NOT_SAME }
Expand Down
1 change: 1 addition & 0 deletions kong/plugins/hmac-auth/daos.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ return {
{ consumer = { type = "foreign", reference = "consumers", required = true, on_delete = "cascade", }, },
{ username = { type = "string", required = true, unique = true }, },
{ secret = { type = "string", auto = true }, },
{ public_key = { type = "string" }, },
Copy link
Contributor

Choose a reason for hiding this comment

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

how about typedefs.key { required = false, referenceable = true, encrypted = true }, }, ?

Suggested change
{ public_key = { type = "string" }, },
{ public_key = typedefs.key {
description = "The RSA public key used to validate signature.",
type = "string",
encrypted = true, -- Kong Enterprise-exclusive feature, does nothing in Kong CE
referenceable = true,
}, },

An additional validator to validate it's a RSA key (not EC or ECX keys) would be good.

{ tags = typedefs.tags },
},
},
Expand Down
13 changes: 13 additions & 0 deletions kong/plugins/hmac-auth/migrations/004_330_to_340.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
return {
postgres = {
up = [[
DO $$
BEGIN
ALTER TABLE IF EXISTS ONLY hmacauth_credentials ADD public_key TEXT;
EXCEPTION WHEN DUPLICATE_COLUMN THEN
-- Do nothing, accept existing state
END$$;

]],
},
}
1 change: 1 addition & 0 deletions kong/plugins/hmac-auth/migrations/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ return {
"000_base_hmac_auth",
"002_130_to_140",
"003_200_to_210",
"004_330_to_340",
}
2 changes: 2 additions & 0 deletions kong/plugins/hmac-auth/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ local ALGORITHMS = {
"hmac-sha256",
"hmac-sha384",
"hmac-sha512",
"rsa-sha256",
"rsa-sha512",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add these new options in the description of the algorithms field? (line 30)

}


Expand Down
28 changes: 28 additions & 0 deletions spec/01-unit/19-hybrid/03-compat_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ describe("kong.clustering.compat", function()
"plugins",
"consumers",
"upstreams",
"hmacauth_credentials",
})
_G.kong.db = db

Expand All @@ -427,10 +428,18 @@ describe("kong.clustering.compat", function()
cert = ssl_fixtures.cert_ca,
}

local consumer_def = {
_tags = ngx.null,
created_at = 1687800000,
id = "f6c12564-47c8-48b4-b171-0a0d9dbf7cb2",
username = "gruceo",
custom_id = "201710",
}

assert(declarative.load_into_db({
ca_certificates = { [ca_certificate_def.id] = ca_certificate_def },
certificates = { [certificate_def.id] = certificate_def },
consumers = { [consumer_def.id] = consumer_def },
upstreams = {
upstreams1 = {
id = "01a2b3c4-d5e6-f7a8-b9c0-d1e2f3a4b5c6",
Expand Down Expand Up @@ -557,10 +566,19 @@ describe("kong.clustering.compat", function()
enabled = true,
},
},
hmacauth_credentials = {
hmacauth_credential1 = {
id = "1aeb7ca9-3317-49c2-a607-6a8759c27318",
username = "gruceo",
consumer = { id = consumer_def.id },
public_key = "notakeytbh",
},
},
}, { _transform = true }))

config = { config_table = declarative.export_config() }
end)

it(function()
local has_update, result = compat.update_compatible_payload(config, "3.0.0", "test_")
assert.truthy(has_update)
Expand Down Expand Up @@ -630,5 +648,15 @@ describe("kong.clustering.compat", function()
assert.is_nil(assert(services[3]).ca_certificates)
end)

it("hmacauth_credentials.public_key", function()
local has_update, result = compat.update_compatible_payload(config, "3.3.0", "test_")
assert.truthy(has_update)
result = cjson_decode(inflate_gzip(result)).config_table

local credentials = assert(assert(assert(result).hmacauth_credentials))
assert.equals(assert(credentials[1]).username, "gruceo")
assert.is_nil(assert(credentials[1]).public_key)
end)

end)
end)
2 changes: 1 addition & 1 deletion spec/03-plugins/19-hmac-auth/01-schema_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe("Plugin: hmac-auth (schema)", function()
it("errors with wrong algorithm", function()
local ok, err = v({ algorithms = { "sha1024" } }, schema_def)
assert.is_falsy(ok)
assert.equal("expected one of: hmac-sha1, hmac-sha256, hmac-sha384, hmac-sha512",
assert.equal("expected one of: hmac-sha1, hmac-sha256, hmac-sha384, hmac-sha512, rsa-sha256, rsa-sha512",
err.config.algorithms[1])
end)
end)
Loading