You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/09/10 11:45:50 UTC
[GitHub] [apisix] arthur-zhang opened a new pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
arthur-zhang opened a new pull request #5038:
URL: https://github.com/apache/apisix/pull/5038
### What this PR does / why we need it:
Add validate request body for hmac auth plugin, by calculate body hmac hash and put it in Digest Header
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706953316
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -186,6 +188,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Body 校验
+
+把 `validate_request_body` 设置为 true 来进行请求 body 的校验。 插件将计算 hmac-sha 值，对比头部中的 Digest 头部值。
Review comment:
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708669612
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -327,6 +351,18 @@ local function validate(ctx, params)
return nil, {message = "Invalid signature"}
end
+ local validate_request_body = get_conf_field(params.access_key, "validate_request_body")
+ if validate_request_body then
+ local max_req_body = get_conf_field(params.access_key, "max_req_body")
Review comment:
@arthur-zhang
OK. Thanks for your explanation. How about when `validate_request_body` is enabled, `X-APISIX-HMAC-BODY-DIGEST` is required? Just pass empty strings for it when no body.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707946396
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -192,6 +194,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Request body checking
+
+When `validate_request_body` is assigned to `true`, the plugin will check the request body. The plugin will calculate the hmac-sha value of the request body，and check against the `X-HMAC-DIGEST` header.
Review comment:
Better to mention the max_req_body limitation
##########
File path: t/plugin/hmac-auth3.t
##########
@@ -521,3 +520,134 @@ passed
--- error_code: 401
--- response_body eval
qr/\{"message":"Exceed body limit size"}/
+
+
+
+=== TEST 10: Test custom request body digest header name with mismatched header.
+--- yaml_config
+plugin_attr:
+ hmac-auth:
+ body_digest_key: "X-Digest-Custom"
+--- config
+ location /t {
+ content_by_lua_block {
+ local ngx_time = ngx.time
+ local ngx_http_time = ngx.http_time
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local hmac = require("resty.hmac")
+ local ngx_encode_base64 = ngx.encode_base64
+
+ local secret_key = "my-secret-key"
+ local timestamp = ngx_time()
+ local gmt = ngx_http_time(timestamp)
+ local access_key = "my-access-key"
+ local custom_header_a = "asld$%dfasf"
+ local custom_header_b = "23879fmsldfk"
+ local body = "{\"name\": \"world\"}"
+
+ local signing_string = {
+ "POST",
+ "/hello",
+ "",
+ access_key,
+ gmt,
+ "x-custom-header-a:" .. custom_header_a,
+ "x-custom-header-b:" .. custom_header_b
+ }
+ signing_string = core.table.concat(signing_string, "\n") .. "\n"
+ core.log.info("signing_string:", signing_string)
+
+ local signature = hmac:new(secret_key, hmac.ALGOS.SHA256):final(signing_string)
+ local body_digest = hmac:new(secret_key, hmac.ALGOS.SHA256):final(body)
+
+ core.log.info("signature:", ngx_encode_base64(signature))
+ local headers = {}
+ headers["X-HMAC-SIGNATURE"] = ngx_encode_base64(signature)
+ headers["X-HMAC-ALGORITHM"] = "hmac-sha256"
+ headers["Date"] = gmt
+ headers["X-Digest"] = ngx_encode_base64(body_digest)
Review comment:
Should be `X-HMAC-DIGEST`?
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -192,6 +194,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Request body checking
+
+When `validate_request_body` is assigned to `true`, the plugin will check the request body. The plugin will calculate the hmac-sha value of the request body，and check against the `X-HMAC-DIGEST` header.
+
+```
+X-HMAC-DIGEST: base64(hmac-sha(<body>))
+```
+
+When there is no request body, the `X-HMAC-DIGEST` header can be omitted. If you want to send request with this header whether the body is empty or not, you can set `X-HMAC-DIGEST` value to the hmac-sha of empty string.
Review comment:
```suggestion
When there is no request body, the `X-HMAC-DIGEST` header can be omitted. If you want to send a request with this header when the body is missing, you can set `X-HMAC-DIGEST` value to the hmac-sha of empty string.
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706521252
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -272,6 +284,29 @@ Accept-Ranges: bytes
<html lang="cn">
```
+### 开启 body 校验
+
+```shell
+$ curl -X "POST" "http://localhost:9080/index.html?age=36&name=james" \
+ -H 'X-HMAC-ACCESS-KEY: zyedu-hmac-01' \
+ -H 'X-HMAC-SIGNATURE: ivlwjZPoVdSVvdSSM4drEFk9q9HS2jeJ5cAN9JffmdA=' \
+ -H 'X-HMAC-ALGORITHM: hmac-sha256' \
+ -H 'Date: Tue, 24 Aug 2021 03:19:21 GMT' \
+ -H 'X-HMAC-SIGNED-HEADERS: User-Agent;Digest' \
+ -H 'User-Agent: curl/7.29.0' \
+ -H 'Digest: L9b/+QMvhvnoUlSw5vq+kHPqnZiHGl61T8oavMVTaC4=' \
+ -H 'Content-Type: text/plain; charset=utf-8' \
+ -d "{\"hello\":\"world\"}"
+
+HTTP/1.1 200 OK
+Content-Type: text/html; charset=utf-8
+Transfer-Encoding: chunked
+Connection: keep-alive
+Date: Tue, 19 Jan 2021 11:33:20 GMT
+Server: APISIX/2.2
+......
+```
Review comment:
I will
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706522940
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
Review comment:
The reason I use this header is that:
The Digest HTTP header provides a digest of the requested resource.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang edited a comment on pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang edited a comment on pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#issuecomment-917909479
> hi @arthur-zhang, I notice this `当无请求 body 时，可不传 Digest 头部，网关会校验是否确实无请求 body。如果要传 Digest 头部，可计算长度为 0 的空字符串的 hmac-sha 值。`, should we add test cases about GET method?
I Add test `=== TEST 4: missing digest header and body is empty` for post request without body, GET is no difference.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708303013
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -327,6 +351,18 @@ local function validate(ctx, params)
return nil, {message = "Invalid signature"}
end
+ local validate_request_body = get_conf_field(params.access_key, "validate_request_body")
+ if validate_request_body then
+ local max_req_body = get_conf_field(params.access_key, "max_req_body")
Review comment:
When there is no request body, the `X-HMAC-DIGEST` header can be omitted.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708781018
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -192,6 +194,18 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Request body checking
+
+When `validate_request_body` is assigned to `true`, the plugin will check the request body. The plugin will calculate the hmac-sha value of the request body，and check against the `X-HMAC-DIGEST` header.
+
+```
+X-HMAC-DIGEST: base64(hmac-sha(<body>))
+```
+
+When there is no request body, the `X-HMAC-DIGEST` header can be omitted. If you want to send a request with this header when the body is missing, you can set `X-HMAC-DIGEST` value to the hmac-sha of empty string.
+
+**Note:** The plugin will load the request body to memory to calculate the digest of the request body, which wight cause high memory consumption with large bodies. You can limit the max allowed body size by the configuration of `max_req_body`(default=512KB), request body larger than that will be rejected.
Review comment:
```suggestion
**Note:** The plugin will load the request body to memory to calculate the digest of the request body, which might cause high memory consumption with large bodies. You can limit the max allowed body size by the configuration of `max_req_body`(default=512KB), request body larger than that will be rejected.
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706849116
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
Review comment:
```suggestion
req_body = req_body or ""
```
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -186,6 +188,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Body 校验
+
+把 `validate_request_body` 设置为 true 来进行请求 body 的校验。 插件将计算 hmac-sha 值，对比头部中的 Digest 头部值。
Review comment:
```suggestion
把 `validate_request_body` 设置为 true 来进行请求 body 的校验。插件将计算 hmac-sha 值，对比头部中的 Digest 头部值。
```
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
+ if not digest_header then
+ -- it's ok if there is no digest header and no body
+ return req_body == ""
+ end
Review comment:
Can we judge this first?
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -186,6 +188,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Body 校验
+
+把 `validate_request_body` 设置为 true 来进行请求 body 的校验。 插件将计算 hmac-sha 值，对比头部中的 Digest 头部值。
Review comment:
`Digest` in code is `digest`, better to keep same.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706581008
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
+ if not digest_header then
Review comment:
we could check this before reading the body
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5038:
URL: https://github.com/apache/apisix/pull/5038
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708784161
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -327,6 +351,18 @@ local function validate(ctx, params)
return nil, {message = "Invalid signature"}
end
+ local validate_request_body = get_conf_field(params.access_key, "validate_request_body")
+ if validate_request_body then
+ local max_req_body = get_conf_field(params.access_key, "max_req_body")
Review comment:
Fix 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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708268100
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -327,6 +351,18 @@ local function validate(ctx, params)
return nil, {message = "Invalid signature"}
end
+ local validate_request_body = get_conf_field(params.access_key, "validate_request_body")
+ if validate_request_body then
+ local max_req_body = get_conf_field(params.access_key, "max_req_body")
Review comment:
when validate_request_body == true and no params.body_digest, why not return false?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707962050
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -192,6 +194,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Request body checking
+
+When `validate_request_body` is assigned to `true`, the plugin will check the request body. The plugin will calculate the hmac-sha value of the request body，and check against the `X-HMAC-DIGEST` header.
+
+```
+X-HMAC-DIGEST: base64(hmac-sha(<body>))
+```
+
+When there is no request body, the `X-HMAC-DIGEST` header can be omitted. If you want to send request with this header whether the body is empty or not, you can set `X-HMAC-DIGEST` value to the hmac-sha of empty string.
Review comment:
done
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707941893
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -327,6 +351,18 @@ local function validate(ctx, params)
return nil, {message = "Invalid signature"}
end
+ local validate_request_body = get_conf_field(params.access_key, "validate_request_body")
+ if validate_request_body then
+ local max_req_body = get_conf_field(params.access_key, "max_req_body")
Review comment:
I think we could return here if no params.body_digest.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707856262
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -79,7 +82,17 @@ local consumer_schema = {
type = "boolean",
title = "Whether to escape the uri parameter",
default = true,
- }
+ },
+ validate_request_body = {
+ type = "boolean",
+ title = "A boolean value telling the plugin to enable body validation.",
+ default = false,
+ },
+ max_req_body = {
+ type = "number",
Review comment:
```suggestion
type = "integer",
```
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -278,6 +290,29 @@ Accept-Ranges: bytes
<html lang="cn">
```
+### Enable request body checking
+
+```shell
+$ curl -X "POST" "http://localhost:9080/index.html?age=36&name=james" \
+ -H 'X-HMAC-ACCESS-KEY: zyedu-hmac-01' \
+ -H 'X-HMAC-SIGNATURE: ivlwjZPoVdSVvdSSM4drEFk9q9HS2jeJ5cAN9JffmdA=' \
+ -H 'X-HMAC-ALGORITHM: hmac-sha256' \
+ -H 'Date: Tue, 24 Aug 2021 03:19:21 GMT' \
+ -H 'X-HMAC-SIGNED-HEADERS: User-Agent;Digest' \
+ -H 'User-Agent: curl/7.29.0' \
+ -H 'Digest: L9b/+QMvhvnoUlSw5vq+kHPqnZiHGl61T8oavMVTaC4=' \
+ -H 'Content-Type: text/plain; charset=utf-8' \
+ -d "{\"hello\":\"world\"}"
+
+HTTP/1.1 200 OK
Review comment:
Better to only show the first line? APISIX 2.2 doesn't support this feature. We can avoid confusing users by only show the first line.
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -51,6 +51,8 @@ The `consumer` then adds its key to request header to verify its request.
| signed_headers | array[string] | optional | | | Restrict the headers that are added to the encrypted calculation. After the specified, the client request can only specify the headers within this range. When this item is empty, all the headers specified by the client request will be added to the encrypted calculation |
| keep_headers | boolean | optional | false | [ true, false ] | Whether it is necessary to keep the request headers of `X-HMAC-SIGNATURE`, `X-HMAC-ALGORITHM` and `X-HMAC-SIGNED-HEADERS` in the http request after successful authentication. true: means to keep the http request header, false: means to remove the http request header. |
| encode_uri_params | boolean | optional | true | [ true, false ] | Whether to encode the uri parameter in the signature, for example: `params1=hello%2Cworld` is encoded, `params2=hello,world` is not encoded. true: means to encode the uri parameter in the signature, false: not to encode the uri parameter in the signature. |
+| validate_request_body | boolean | optional | false | [ true, false ] | Whether to check request body. |
+| max_req_body | number | optional | 512KB | | Max allowed body size. |
Review comment:
Better to use a number instead of string in the doc's default value cell
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -29,13 +29,16 @@ local hmac = require("resty.hmac")
local consumer = require("apisix.consumer")
local plugin = require("apisix.plugin")
local ngx_decode_base64 = ngx.decode_base64
+local ngx_encode_base64 = ngx.encode_base64
+local DIGEST = "Digest"
Review comment:
Can we make this header configurable and default to "X-HMAC-DIGEST"?
There is already a Digest header in the HTTP standard and it is not relative to the hmac.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Digest
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706522940
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
Review comment:
The reason I use this header is that:
The Digest response HTTP header provides a digest of the requested resource.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708321755
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -79,7 +82,17 @@ local consumer_schema = {
type = "boolean",
title = "Whether to escape the uri parameter",
default = true,
- }
+ },
+ validate_request_body = {
+ type = "boolean",
+ title = "A boolean value telling the plugin to enable body validation.",
+ default = false,
+ },
+ max_req_body = {
+ type = "integer",
+ title = "Max request body allowed",
Review comment:
"Max request body size" would be better.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707962535
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -192,6 +194,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Request body checking
+
+When `validate_request_body` is assigned to `true`, the plugin will check the request body. The plugin will calculate the hmac-sha value of the request body，and check against the `X-HMAC-DIGEST` header.
Review comment:
I add a **note** to that
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706284789
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -272,6 +284,29 @@ Accept-Ranges: bytes
<html lang="cn">
```
+### 开启 body 校验
+
+```shell
+$ curl -X "POST" "http://localhost:9080/index.html?age=36&name=james" \
+ -H 'X-HMAC-ACCESS-KEY: zyedu-hmac-01' \
+ -H 'X-HMAC-SIGNATURE: ivlwjZPoVdSVvdSSM4drEFk9q9HS2jeJ5cAN9JffmdA=' \
+ -H 'X-HMAC-ALGORITHM: hmac-sha256' \
+ -H 'Date: Tue, 24 Aug 2021 03:19:21 GMT' \
+ -H 'X-HMAC-SIGNED-HEADERS: User-Agent;Digest' \
+ -H 'User-Agent: curl/7.29.0' \
+ -H 'Digest: L9b/+QMvhvnoUlSw5vq+kHPqnZiHGl61T8oavMVTaC4=' \
+ -H 'Content-Type: text/plain; charset=utf-8' \
+ -d "{\"hello\":\"world\"}"
+
+HTTP/1.1 200 OK
+Content-Type: text/html; charset=utf-8
+Transfer-Encoding: chunked
+Connection: keep-alive
+Date: Tue, 19 Jan 2021 11:33:20 GMT
+Server: APISIX/2.2
+......
+```
Review comment:
Can you help to update the English doc?
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
Review comment:
`digest` header looks not good for me.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r708344348
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -79,7 +82,17 @@ local consumer_schema = {
type = "boolean",
title = "Whether to escape the uri parameter",
default = true,
- }
+ },
+ validate_request_body = {
+ type = "boolean",
+ title = "A boolean value telling the plugin to enable body validation.",
+ default = false,
+ },
+ max_req_body = {
+ type = "integer",
+ title = "Max request body allowed",
Review comment:
OK
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707039593
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -186,6 +188,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Body 校验
+
+把 `validate_request_body` 设置为 true 来进行请求 body 的校验。插件将计算 hmac-sha 值，对比头部中的 Digest 头部值。
+
+```
+Digest: base64(hmac-sha(<body>))
+```
+
+当无请求 body 时，可不传 Digest 头部，网关会校验是否确实无请求 body。如果要传 Digest 头部，可计算长度为 0 的空字符串的 hmac-sha 值。
Review comment:
```suggestion
当没有请求 body 时，可不传 Digest 头部，网关会校验是否确实无请求 body。如果要传 Digest 头部，插件将请求 body 默认为长度为 0 的空字符串，并参与到计算 hmac-sha 值。
```
##########
File path: t/plugin/hmac-auth3.t
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with validate_request_body
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "robin",
+ "plugins": {
+ "hmac-auth": {
+ "access_key": "my-access-key",
+ "secret_key": "my-secret-key",
+ "validate_request_body": true
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "robin",
+ "plugins": {
+ "hmac-auth": {
+ "access_key": "my-access-key",
+ "secret_key": "my-secret-key",
+ "algorithm": "hmac-sha256",
+ "validate_request_body": true
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable hmac auth plugin using admin api
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "hmac-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: missing body digest when validate_request_body is true
Review comment:
```suggestion
=== TEST 3: missing body digest when validate_request_body is enable
```
##########
File path: t/plugin/hmac-auth3.t
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with validate_request_body
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "robin",
+ "plugins": {
+ "hmac-auth": {
+ "access_key": "my-access-key",
+ "secret_key": "my-secret-key",
+ "validate_request_body": true
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "robin",
+ "plugins": {
+ "hmac-auth": {
+ "access_key": "my-access-key",
+ "secret_key": "my-secret-key",
+ "algorithm": "hmac-sha256",
+ "validate_request_body": true
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable hmac auth plugin using admin api
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "hmac-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: missing body digest when validate_request_body is true
+--- config
+ location /t {
+ content_by_lua_block {
+ local ngx_time = ngx.time
+ local ngx_http_time = ngx.http_time
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local hmac = require("resty.hmac")
+ local ngx_encode_base64 = ngx.encode_base64
+
+ local secret_key = "my-secret-key"
+ local timestamp = ngx_time()
+ local gmt = ngx_http_time(timestamp)
+ local access_key = "my-access-key"
+ local custom_header_a = "asld$%dfasf"
+ local custom_header_b = "23879fmsldfk"
+ local body = "{\"name\": \"world\"}"
+
+ local signing_string = {
+ "POST",
+ "/hello",
+ "",
+ access_key,
+ gmt,
+ "x-custom-header-a:" .. custom_header_a,
+ "x-custom-header-b:" .. custom_header_b
+ }
+ signing_string = core.table.concat(signing_string, "\n") .. "\n"
+ core.log.info("signing_string:", signing_string)
+
+ local signature = hmac:new(secret_key, hmac.ALGOS.SHA256):final(signing_string)
+
+ core.log.info("signature:", ngx_encode_base64(signature))
+ local headers = {}
+ headers["X-HMAC-SIGNATURE"] = ngx_encode_base64(signature)
+ headers["X-HMAC-ALGORITHM"] = "hmac-sha256"
+ headers["Date"] = gmt
+ headers["X-HMAC-ACCESS-KEY"] = access_key
+ headers["X-HMAC-SIGNED-HEADERS"] = "x-custom-header-a;x-custom-header-b"
+ headers["x-custom-header-a"] = custom_header_a
+ headers["x-custom-header-b"] = custom_header_b
+
+ local code, body = t.test('/hello',
+ ngx.HTTP_POST,
+ body,
+ nil,
+ headers
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- error_code: 401
+--- response_body eval
+qr/\{"message":"Invalid digest"\}/
+
+
+
+=== TEST 4: missing digest header and body is empty
+--- config
+ location /t {
+ content_by_lua_block {
+ local ngx_time = ngx.time
+ local ngx_http_time = ngx.http_time
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local hmac = require("resty.hmac")
+ local ngx_encode_base64 = ngx.encode_base64
+
+ local secret_key = "my-secret-key"
+ local timestamp = ngx_time()
+ local gmt = ngx_http_time(timestamp)
+ local access_key = "my-access-key"
+ local custom_header_a = "asld$%dfasf"
+ local custom_header_b = "23879fmsldfk"
+ local body = ""
+
+ local signing_string = {
+ "POST",
+ "/hello",
+ "",
+ access_key,
+ gmt,
+ "x-custom-header-a:" .. custom_header_a,
+ "x-custom-header-b:" .. custom_header_b
+ }
+ signing_string = core.table.concat(signing_string, "\n") .. "\n"
+ core.log.info("signing_string:", signing_string)
+
+ local signature = hmac:new(secret_key, hmac.ALGOS.SHA256):final(signing_string)
+ local body_digest = hmac:new(secret_key, hmac.ALGOS.SHA256):final(body)
+
+ core.log.info("signature:", ngx_encode_base64(signature))
+ local headers = {}
+ headers["X-HMAC-SIGNATURE"] = ngx_encode_base64(signature)
+ headers["X-HMAC-ALGORITHM"] = "hmac-sha256"
+ headers["Date"] = gmt
+ headers["Digest"] = ngx_encode_base64(body_digest)
+ headers["X-HMAC-ACCESS-KEY"] = access_key
+ headers["X-HMAC-SIGNED-HEADERS"] = "x-custom-header-a;x-custom-header-b"
+ headers["x-custom-header-a"] = custom_header_a
+ headers["x-custom-header-b"] = custom_header_b
+
+ local code, body = t.test('/hello',
+ ngx.HTTP_POST,
+ body,
+ nil,
+ headers
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 5: digest header with empty string and body is empty
+--- config
+ location /t {
+ content_by_lua_block {
+ local ngx_time = ngx.time
+ local ngx_http_time = ngx.http_time
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local hmac = require("resty.hmac")
+ local ngx_encode_base64 = ngx.encode_base64
+
+ local secret_key = "my-secret-key"
+ local timestamp = ngx_time()
+ local gmt = ngx_http_time(timestamp)
+ local access_key = "my-access-key"
+ local custom_header_a = "asld$%dfasf"
+ local custom_header_b = "23879fmsldfk"
+ local body = ""
+
+ local signing_string = {
+ "POST",
+ "/hello",
+ "",
+ access_key,
+ gmt,
+ "x-custom-header-a:" .. custom_header_a,
+ "x-custom-header-b:" .. custom_header_b
+ }
+ signing_string = core.table.concat(signing_string, "\n") .. "\n"
+ core.log.info("signing_string:", signing_string)
+
+ local signature = hmac:new(secret_key, hmac.ALGOS.SHA256):final(signing_string)
+
+ core.log.info("signature:", ngx_encode_base64(signature))
+ local headers = {}
+ headers["X-HMAC-SIGNATURE"] = ngx_encode_base64(signature)
+ headers["X-HMAC-ALGORITHM"] = "hmac-sha256"
+ headers["Date"] = gmt
+ headers["X-HMAC-ACCESS-KEY"] = access_key
+ headers["X-HMAC-SIGNED-HEADERS"] = "x-custom-header-a;x-custom-header-b"
+ headers["x-custom-header-a"] = custom_header_a
+ headers["x-custom-header-b"] = custom_header_b
+
+ local code, body = t.test('/hello',
+ ngx.HTTP_POST,
+ body,
Review comment:
here body should be `nil`?
##########
File path: t/plugin/hmac-auth3.t
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_shuffle();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with validate_request_body
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "robin",
+ "plugins": {
+ "hmac-auth": {
+ "access_key": "my-access-key",
+ "secret_key": "my-secret-key",
+ "validate_request_body": true
+ }
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "robin",
+ "plugins": {
+ "hmac-auth": {
+ "access_key": "my-access-key",
+ "secret_key": "my-secret-key",
+ "algorithm": "hmac-sha256",
+ "validate_request_body": true
+ }
+ }
+ }
+ },
+ "action": "set"
+ }]]
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable hmac auth plugin using admin api
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "hmac-auth": {}
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 3: missing body digest when validate_request_body is true
+--- config
+ location /t {
+ content_by_lua_block {
+ local ngx_time = ngx.time
+ local ngx_http_time = ngx.http_time
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local hmac = require("resty.hmac")
+ local ngx_encode_base64 = ngx.encode_base64
+
+ local secret_key = "my-secret-key"
+ local timestamp = ngx_time()
+ local gmt = ngx_http_time(timestamp)
+ local access_key = "my-access-key"
+ local custom_header_a = "asld$%dfasf"
+ local custom_header_b = "23879fmsldfk"
+ local body = "{\"name\": \"world\"}"
+
+ local signing_string = {
+ "POST",
+ "/hello",
+ "",
+ access_key,
+ gmt,
+ "x-custom-header-a:" .. custom_header_a,
+ "x-custom-header-b:" .. custom_header_b
+ }
+ signing_string = core.table.concat(signing_string, "\n") .. "\n"
+ core.log.info("signing_string:", signing_string)
+
+ local signature = hmac:new(secret_key, hmac.ALGOS.SHA256):final(signing_string)
+
+ core.log.info("signature:", ngx_encode_base64(signature))
+ local headers = {}
+ headers["X-HMAC-SIGNATURE"] = ngx_encode_base64(signature)
+ headers["X-HMAC-ALGORITHM"] = "hmac-sha256"
+ headers["Date"] = gmt
+ headers["X-HMAC-ACCESS-KEY"] = access_key
+ headers["X-HMAC-SIGNED-HEADERS"] = "x-custom-header-a;x-custom-header-b"
+ headers["x-custom-header-a"] = custom_header_a
+ headers["x-custom-header-b"] = custom_header_b
+
+ local code, body = t.test('/hello',
+ ngx.HTTP_POST,
+ body,
+ nil,
+ headers
+ )
+
+ ngx.status = code
+ ngx.say(body)
+ }
+ }
+--- error_code: 401
+--- response_body eval
+qr/\{"message":"Invalid digest"\}/
+
+
+
+=== TEST 4: missing digest header and body is empty
Review comment:
```suggestion
=== TEST 4: no digest header and request body is empty
```
##########
File path: docs/zh/latest/plugins/hmac-auth.md
##########
@@ -186,6 +188,16 @@ print(base64.b64encode(hash.digest()))
| --------- | -------------------------------------------- |
| SIGNATURE | 8XV1GB7Tq23OJcoz6wjqTs4ZLxr9DiLoY4PxzScWGYg= |
+### Body 校验
+
+把 `validate_request_body` 设置为 true 来进行请求 body 的校验。插件将计算 hmac-sha 值，对比头部中的 Digest 头部值。
Review comment:
```suggestion
`validate_request_body` 设置为 true 时，插件将计算请求 body 的 `hmac-sha` 值，并与请求 headers 中的 Digest 的值进行校验。
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706941186
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
+ if not digest_header then
+ -- it's ok if there is no digest header and no body
+ return req_body == ""
+ end
Review comment:
Maybe not, because we can omit digest header when body is empty.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#issuecomment-917882793
hi @arthur-zhang, I notice this `当无请求 body 时，可不传 Digest 头部，网关会校验是否确实无请求 body。如果要传 Digest 头部，可计算长度为 0 的空字符串的 hmac-sha 值。`, should we add test cases about GET method?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707867948
##########
File path: docs/en/latest/plugins/hmac-auth.md
##########
@@ -278,6 +290,29 @@ Accept-Ranges: bytes
<html lang="cn">
```
+### Enable request body checking
+
+```shell
+$ curl -X "POST" "http://localhost:9080/index.html?age=36&name=james" \
+ -H 'X-HMAC-ACCESS-KEY: zyedu-hmac-01' \
+ -H 'X-HMAC-SIGNATURE: ivlwjZPoVdSVvdSSM4drEFk9q9HS2jeJ5cAN9JffmdA=' \
+ -H 'X-HMAC-ALGORITHM: hmac-sha256' \
+ -H 'Date: Tue, 24 Aug 2021 03:19:21 GMT' \
+ -H 'X-HMAC-SIGNED-HEADERS: User-Agent;Digest' \
+ -H 'User-Agent: curl/7.29.0' \
+ -H 'Digest: L9b/+QMvhvnoUlSw5vq+kHPqnZiHGl61T8oavMVTaC4=' \
+ -H 'Content-Type: text/plain; charset=utf-8' \
+ -d "{\"hello\":\"world\"}"
+
+HTTP/1.1 200 OK
Review comment:
Got it
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -29,13 +29,16 @@ local hmac = require("resty.hmac")
local consumer = require("apisix.consumer")
local plugin = require("apisix.plugin")
local ngx_decode_base64 = ngx.decode_base64
+local ngx_encode_base64 = ngx.encode_base64
+local DIGEST = "Digest"
Review comment:
OK
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707963227
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -327,6 +351,18 @@ local function validate(ctx, params)
return nil, {message = "Invalid signature"}
end
+ local validate_request_body = get_conf_field(params.access_key, "validate_request_body")
+ if validate_request_body then
+ local max_req_body = get_conf_field(params.access_key, "max_req_body")
Review comment:
I think we should check whether the params.body_digest exists or not.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r707949898
##########
File path: t/plugin/hmac-auth3.t
##########
@@ -521,3 +520,134 @@ passed
--- error_code: 401
--- response_body eval
qr/\{"message":"Exceed body limit size"}/
+
+
+
+=== TEST 10: Test custom request body digest header name with mismatched header.
+--- yaml_config
+plugin_attr:
+ hmac-auth:
+ body_digest_key: "X-Digest-Custom"
+--- config
+ location /t {
+ content_by_lua_block {
+ local ngx_time = ngx.time
+ local ngx_http_time = ngx.http_time
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local hmac = require("resty.hmac")
+ local ngx_encode_base64 = ngx.encode_base64
+
+ local secret_key = "my-secret-key"
+ local timestamp = ngx_time()
+ local gmt = ngx_http_time(timestamp)
+ local access_key = "my-access-key"
+ local custom_header_a = "asld$%dfasf"
+ local custom_header_b = "23879fmsldfk"
+ local body = "{\"name\": \"world\"}"
+
+ local signing_string = {
+ "POST",
+ "/hello",
+ "",
+ access_key,
+ gmt,
+ "x-custom-header-a:" .. custom_header_a,
+ "x-custom-header-b:" .. custom_header_b
+ }
+ signing_string = core.table.concat(signing_string, "\n") .. "\n"
+ core.log.info("signing_string:", signing_string)
+
+ local signature = hmac:new(secret_key, hmac.ALGOS.SHA256):final(signing_string)
+ local body_digest = hmac:new(secret_key, hmac.ALGOS.SHA256):final(body)
+
+ core.log.info("signature:", ngx_encode_base64(signature))
+ local headers = {}
+ headers["X-HMAC-SIGNATURE"] = ngx_encode_base64(signature)
+ headers["X-HMAC-ALGORITHM"] = "hmac-sha256"
+ headers["Date"] = gmt
+ headers["X-Digest"] = ngx_encode_base64(body_digest)
Review comment:
Any string here is ok, I will change it to X-HMAC-DIGEST.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#issuecomment-917909479
> hi @arthur-zhang, I notice this `当无请求 body 时，可不传 Digest 头部，网关会校验是否确实无请求 body。如果要传 Digest 头部，可计算长度为 0 的空字符串的 hmac-sha 值。`, should we add test cases about GET method?
I Add test `=== TEST 4: missing digest header and body is empty` for post quest without body, GET is no difference.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] arthur-zhang commented on a change in pull request #5038: feat(hmac-auth): Add validate request body for hmac auth plugin
Posted by GitBox <gi...@apache.org>.
arthur-zhang commented on a change in pull request #5038:
URL: https://github.com/apache/apisix/pull/5038#discussion_r706953345
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -193,6 +206,19 @@ local function do_nothing(v)
return v
end
+local function validate_body(ctx, secret_key, params, req_body)
+ if not req_body then
+ req_body = ""
+ end
+ local digest_header = core.request.header(ctx, DIGEST)
+ if not digest_header then
Review comment:
Maybe not, because we can omit digest header when body is empty.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org