Skip to content

Commit

Permalink
fix(hmac): add missing www-authenticate headers
Browse files Browse the repository at this point in the history
When server returns 401 Unauthorized response it should
return WWW-Authenticate header as well with proper challenge.
HMAC auth was missing this header.

Fix: #7772
KAG-321
  • Loading branch information
nowNick committed Jun 14, 2024
1 parent 7585007 commit 2517d20
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 43 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/hmac_www_authenticate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**hmac-auth**: Add WWW-Authenticate headers to 401 responses."
type: bugfix
scope: Plugin
3 changes: 3 additions & 0 deletions kong/clustering/compat/removed_fields.lua
Original file line number Diff line number Diff line change
Expand Up @@ -154,5 +154,8 @@ return {
ldap_auth = {
"realm",
},
hmac_auth = {
"realm",
},
},
}
81 changes: 52 additions & 29 deletions kong/plugins/hmac-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -275,24 +275,29 @@ local function set_consumer(consumer, credential)
end
end

local function unauthorized(message, www_auth_content)
return { status = 401, message = message, headers = { ["WWW-Authenticate"] = www_auth_content } }
end


local function do_authentication(conf)
local authorization = kong_request.get_header(AUTHORIZATION)
local proxy_authorization = kong_request.get_header(PROXY_AUTHORIZATION)
local www_auth_content = conf.realm and fmt('hmac realm="%s"', conf.realm) or 'hmac'

-- If both headers are missing, return 401
if not (authorization or proxy_authorization) then
return false, { status = 401, message = "Unauthorized" }
return false, unauthorized("Unauthorized", www_auth_content)
end

-- validate clock skew
if not (validate_clock_skew(X_DATE, conf.clock_skew) or
validate_clock_skew(DATE, conf.clock_skew)) then
return false, {
status = 401,
message = "HMAC signature cannot be verified, a valid date or " ..
"x-date header is required for HMAC Authentication"
}
return false, unauthorized(
"HMAC signature cannot be verified, a valid date or " ..
"x-date header is required for HMAC Authentication",
www_auth_content
)
end

-- retrieve hmac parameter from Proxy-Authorization header
Expand All @@ -312,26 +317,26 @@ local function do_authentication(conf)
local ok, err = validate_params(hmac_params, conf)
if not ok then
kong.log.debug(err)
return false, { status = 401, message = SIGNATURE_NOT_VALID }
return false, unauthorized(SIGNATURE_NOT_VALID, www_auth_content)
end

-- validate signature
local credential = load_credential(hmac_params.username)
if not credential then
kong.log.debug("failed to retrieve credential for ", hmac_params.username)
return false, { status = 401, message = SIGNATURE_NOT_VALID }
return false, unauthorized(SIGNATURE_NOT_VALID, www_auth_content)
end

hmac_params.secret = credential.secret

if not validate_signature(hmac_params) then
return false, { status = 401, message = SIGNATURE_NOT_SAME }
return false, unauthorized(SIGNATURE_NOT_SAME, www_auth_content)
end

-- If request body validation is enabled, then verify digest.
if conf.validate_request_body and not validate_body() then
kong.log.debug("digest validation failed")
return false, { status = 401, message = SIGNATURE_NOT_SAME }
return false, unauthorized(SIGNATURE_NOT_SAME, www_auth_content)
end

-- Retrieve consumer
Expand All @@ -349,34 +354,52 @@ local function do_authentication(conf)
return true
end

local function set_anonymous_consumer(anonymous)
local consumer_cache_key = kong.db.consumers:cache_key(anonymous)
local consumer, err = kong.cache:get(consumer_cache_key, nil,
kong.client.load_consumer,
anonymous, true)
if err then
return error(err)
end

local _M = {}

set_consumer(consumer)
end

function _M.execute(conf)
if conf.anonymous and kong_client.get_credential() then
-- we're already authenticated, and we're configured for using anonymous,
-- hence we're in a logical OR between auth methods and we're already done.
--- When conf.anonymous is enabled we are in "logical OR" authentication flow.
--- Meaning - either anonymous consumer is enabled or there are multiple auth plugins
--- and we need to passthrough on failed authentication.
local function logical_OR_authentication(conf)
if kong.client.get_credential() then
-- we're already authenticated and in "logical OR" between auth methods -- early exit
return
end

local ok, _ = do_authentication(conf)
if not ok then
set_anonymous_consumer(conf.anonymous)
end
end

--- When conf.anonymous is not set we are in "logical AND" authentication flow.
--- Meaning - if this authentication fails the request should not be authorized
--- even though other auth plugins might have successfully authorized user.
local function logical_AND_authentication(conf)
local ok, err = do_authentication(conf)
if not ok then
if conf.anonymous then
-- get anonymous user
local consumer_cache_key = kong.db.consumers:cache_key(conf.anonymous)
local consumer, err = kong.cache:get(consumer_cache_key, nil,
kong_client.load_consumer,
conf.anonymous, true)
if err then
return error(err)
end
return kong.response.error(err.status, err.message, err.headers)
end
end

set_consumer(consumer)

else
return kong.response.error(err.status, err.message, err.headers)
end
local _M = {}

function _M.execute(conf)

if conf.anonymous then
return logical_OR_authentication(conf)
else
return logical_AND_authentication(conf)
end
end

Expand Down
1 change: 1 addition & 0 deletions kong/plugins/hmac-auth/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ return {
elements = { type = "string", one_of = ALGORITHMS },
default = ALGORITHMS,
}, },
{ realm = { description = "When authentication fails the plugin sends `WWW-Authenticate` header with `realm` attribute value.", type = "string", required = false }, },
},
},
},
Expand Down
14 changes: 14 additions & 0 deletions spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,20 @@ describe("CP/DP config compat transformations #" .. strategy, function()
-- cleanup
admin.plugins:remove({ id = ldap_auth.id })
end)

it("[hmac-auth] removes realm for versions below 3.8", function()
local hmac_auth = admin.plugins:insert {
name = "hmac-auth",
config = {
realm = "test"
}
}
local expected_hmac_auth_prior_38 = cycle_aware_deep_copy(hmac_auth)
expected_hmac_auth_prior_38.config.realm = nil
do_assert(utils.uuid(), "3.7.0", expected_hmac_auth_prior_38)
-- cleanup
admin.plugins:remove({ id = hmac_auth.id })
end)
end)

describe("compatibility test for response-transformer plugin", function()
Expand Down
Loading

0 comments on commit 2517d20

Please sign in to comment.