membphis commented on code in PR #11581:
URL: https://github.com/apache/apisix/pull/11581#discussion_r1764650470
##########
apisix/plugins/hmac-auth.lua:
##########
@@ -148,204 +118,163 @@ function _M.check_schema(conf, schema_type)
end
-local function get_consumer(access_key)
- if not access_key then
- return nil, "missing access key"
+local function get_consumer(key_id)
+ if not key_id then
+ return nil, "missing key_id"
end
local consumer_conf = consumer.plugin(plugin_name)
if not consumer_conf then
return nil, "Missing related consumer"
end
- local consumers = consumer.consumers_kv(plugin_name, consumer_conf,
"access_key")
- local consumer = consumers[access_key]
+ local consumers = consumer.consumers_kv(plugin_name, consumer_conf,
"key_id")
+ local consumer = consumers[key_id]
if not consumer then
- return nil, "Invalid access key"
+ return nil, "Invalid key_id"
end
core.log.info("consumer: ", core.json.delay_encode(consumer))
return consumer
end
-local function get_conf_field(access_key, field_name)
- local consumer, err = get_consumer(access_key)
- if err then
- return false, err
- end
-
- return consumer.auth_conf[field_name]
-end
-
-
-local function do_nothing(v)
- return v
-end
-
local function generate_signature(ctx, secret_key, params)
- local canonical_uri = ctx.var.uri
- local canonical_query_string = ""
+ local uri = ctx.var.request_uri
local request_method = core.request.get_method()
- local args = core.request.get_uri_args(ctx)
- if canonical_uri == "" then
- canonical_uri = "/"
+ if uri == "" then
+ uri = "/"
end
- if type(args) == "table" then
- local keys = {}
- local query_tab = {}
-
- for k, v in pairs(args) do
- core.table.insert(keys, k)
- end
- core.table.sort(keys)
-
- local field_val = get_conf_field(params.access_key,
"encode_uri_params")
- core.log.info("encode_uri_params: ", field_val)
-
- local encode_or_not = do_nothing
- if field_val then
- encode_or_not = escape_uri
- end
-
- for _, key in pairs(keys) do
- local param = args[key]
- -- when args without `=<value>`, value is treated as true.
- -- In order to be compatible with args lacking `=<value>`,
- -- we need to replace true with an empty string.
- if type(param) == "boolean" then
- param = ""
- end
-
- -- whether to encode the uri parameters
- if type(param) == "table" then
- local vals = {}
- for _, val in pairs(param) do
- if type(val) == "boolean" then
- val = ""
- end
- core.table.insert(vals, val)
- end
- core.table.sort(vals)
-
- for _, val in pairs(vals) do
- core.table.insert(query_tab, encode_or_not(key) .. "=" ..
encode_or_not(val))
- end
- else
- core.table.insert(query_tab, encode_or_not(key) .. "=" ..
encode_or_not(param))
- end
- end
- canonical_query_string = core.table.concat(query_tab, "&")
- end
-
- core.log.info("all headers: ",
- core.json.delay_encode(core.request.headers(ctx), true))
-
local signing_string_items = {
- request_method,
- canonical_uri,
- canonical_query_string,
- params.access_key,
- params.date,
+ params.keyId,
}
- if params.signed_headers then
- for _, h in ipairs(params.signed_headers) do
- local canonical_header = core.request.header(ctx, h) or ""
- core.table.insert(signing_string_items,
- h .. ":" .. canonical_header)
- core.log.info("canonical_header name:", core.json.delay_encode(h))
- core.log.info("canonical_header value: ",
- core.json.delay_encode(canonical_header))
+ if params.headers then
+ for _, h in ipairs(params.headers) do
+ local canonical_header = core.request.header(ctx, h)
+ if not canonical_header then
+ if h == "@request-target" then
+ local request_target = request_method .. " " .. uri
+ core.table.insert(signing_string_items, request_target)
+ core.log.info("canonical_header name:",
core.json.delay_encode(h))
+ core.log.info("canonical_header value: ",
+ core.json.delay_encode(request_target))
+ end
+ else
+ core.table.insert(signing_string_items,
Review Comment:
bad alignment
<img width="650" alt="image"
src="https://github.com/user-attachments/assets/d832f906-fbf4-47c6-bc71-f47b3c074e1a";>
##########
apisix/plugins/hmac-auth.lua:
##########
@@ -354,86 +283,62 @@ local function validate(ctx, params)
end
-local function get_params(ctx)
- local params = {}
- local access_key = ACCESS_KEY
- local signature_key = SIGNATURE_KEY
- local algorithm_key = ALGORITHM_KEY
- local date_key = DATE_KEY
- local signed_headers_key = SIGNED_HEADERS_KEY
- local body_digest_key = BODY_DIGEST_KEY
-
-
- local attr = plugin.plugin_attr(plugin_name)
- if attr then
- access_key = attr.access_key or access_key
- signature_key = attr.signature_key or signature_key
- algorithm_key = attr.algorithm_key or algorithm_key
- date_key = attr.date_key or date_key
- signed_headers_key = attr.signed_headers_key or signed_headers_key
- body_digest_key = attr.body_digest_key or body_digest_key
+local function retrieve_hmac_fields(ctx)
+ local hmac_params = {}
+ local auth_string = core.request.header(ctx, "Authorization")
+ if not auth_string then
+ return nil, "missing Authorization header"
end
- local app_key = core.request.header(ctx, access_key)
- local signature = core.request.header(ctx, signature_key)
- local algorithm = core.request.header(ctx, algorithm_key)
- local date = core.request.header(ctx, date_key)
- local signed_headers = core.request.header(ctx, signed_headers_key)
- local body_digest = core.request.header(ctx, body_digest_key)
- core.log.info("signature_key: ", signature_key)
-
- -- get params from header `Authorization`
- if not app_key then
- local auth_string = core.request.header(ctx, "Authorization")
- if not auth_string then
- return params
- end
-
- local auth_data = ngx_re.split(auth_string, "#")
- core.log.info("auth_string: ", auth_string, " #auth_data: ",
- #auth_data, " auth_data: ",
- core.json.delay_encode(auth_data))
-
- if #auth_data == 6 and auth_data[1] == "hmac-auth-v1" then
- app_key = auth_data[2]
- signature = auth_data[3]
- algorithm = auth_data[4]
- date = auth_data[5]
- signed_headers = auth_data[6]
- end
+ if not auth_string:match("^Signature") then
Review Comment:
a high performance way: `core.string.has_prefix`
##########
apisix/plugins/hmac-auth.lua:
##########
@@ -354,86 +283,62 @@ local function validate(ctx, params)
end
-local function get_params(ctx)
- local params = {}
- local access_key = ACCESS_KEY
- local signature_key = SIGNATURE_KEY
- local algorithm_key = ALGORITHM_KEY
- local date_key = DATE_KEY
- local signed_headers_key = SIGNED_HEADERS_KEY
- local body_digest_key = BODY_DIGEST_KEY
-
-
- local attr = plugin.plugin_attr(plugin_name)
- if attr then
- access_key = attr.access_key or access_key
- signature_key = attr.signature_key or signature_key
- algorithm_key = attr.algorithm_key or algorithm_key
- date_key = attr.date_key or date_key
- signed_headers_key = attr.signed_headers_key or signed_headers_key
- body_digest_key = attr.body_digest_key or body_digest_key
+local function retrieve_hmac_fields(ctx)
+ local hmac_params = {}
+ local auth_string = core.request.header(ctx, "Authorization")
+ if not auth_string then
+ return nil, "missing Authorization header"
end
- local app_key = core.request.header(ctx, access_key)
- local signature = core.request.header(ctx, signature_key)
- local algorithm = core.request.header(ctx, algorithm_key)
- local date = core.request.header(ctx, date_key)
- local signed_headers = core.request.header(ctx, signed_headers_key)
- local body_digest = core.request.header(ctx, body_digest_key)
- core.log.info("signature_key: ", signature_key)
-
- -- get params from header `Authorization`
- if not app_key then
- local auth_string = core.request.header(ctx, "Authorization")
- if not auth_string then
- return params
- end
-
- local auth_data = ngx_re.split(auth_string, "#")
- core.log.info("auth_string: ", auth_string, " #auth_data: ",
- #auth_data, " auth_data: ",
- core.json.delay_encode(auth_data))
-
- if #auth_data == 6 and auth_data[1] == "hmac-auth-v1" then
- app_key = auth_data[2]
- signature = auth_data[3]
- algorithm = auth_data[4]
- date = auth_data[5]
- signed_headers = auth_data[6]
- end
+ if not auth_string:match("^Signature") then
+ return nil, "Authorization header does not start with 'Signature'"
end
- params.access_key = app_key
- params.algorithm = algorithm
- params.signature = signature
- params.date = date or ""
- params.signed_headers = signed_headers and ngx_re.split(signed_headers,
";")
- params.body_digest = body_digest
+ local signature_fields = auth_string:sub(10):gmatch('[^,]+')
+
+ for field in signature_fields do
+ local key, value = field:match('%s*(%w+)="(.-)"')
+ if key and value then
+ if key == "keyId" or key == "algorithm" or key == "signature" then
+ hmac_params[key] = value
- local keep_headers = get_conf_field(params.access_key, "keep_headers")
- core.log.info("keep_headers: ", keep_headers)
+ elseif key == "headers" then
+ hmac_params.headers = ngx_re.split(value, " ")
+ end
+ end
+ end
- if not keep_headers then
- remove_headers(ctx, signature_key, algorithm_key, signed_headers_key)
+ -- will be required to check clock skew
+ if core.request.header(ctx, "Date") then
+ hmac_params.date = core.request.header(ctx, "Date")
end
- core.log.info("params: ", core.json.delay_encode(params))
+ if core.request.header(ctx, "Digest") then
+ hmac_params.body_digest = core.request.header(ctx, "Digest")
+ end
- return params
+ return hmac_params
end
function _M.rewrite(conf, ctx)
- local params = get_params(ctx)
- local validated_consumer, err = validate(ctx, params)
+ local params,err = retrieve_hmac_fields(ctx)
+ if err then
+ core.log.warn("client request can't be validated: ", err)
+ return 401, {message = "client request can't be validated: " .. err}
+ end
+
+ if conf.hide_credentials then
+ core.request.set_header("Authorization", nil)
+ end
+ local validated_consumer, err = validate(ctx, conf, params)
if not validated_consumer then
core.log.warn("client request can't be validated: ", err or "Invalid
signature")
return 401, {message = "client request can't be validated"}
end
local consumer_conf = consumer.plugin(plugin_name)
consumer.attach_consumer(ctx, validated_consumer, consumer_conf)
- core.log.info("hit hmac-auth rewrite")
Review Comment:
we do not need to remove it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
