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 2020/09/24 07:17:03 UTC
[GitHub] [apisix] nic-chen opened a new pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
nic-chen opened a new pull request #2301:
URL: https://github.com/apache/apisix/pull/2301
### What this PR does / why we need it:
replace timestamp with date and time in GMT format in plugin `hmac-auth`
### 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?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gxthrj commented on pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#issuecomment-698194182
I hope to change the default value of `clock_skew` to 0,
it is more useful when migrating, In this way, the client does not need to modify the client call mode.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494148437
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
The value obtained here may be empty, we need to perform error handling, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494699403
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495427596
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -128,7 +129,35 @@ Accept-Ranges: bytes
* The signature information is separately placed in the request header:
```shell
-$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'X-HMAC-TIMESTAMP: TIMESTAMP' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'Date: DATE' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+HTTP/1.1 200 OK
+Content-Type: text/html
+Content-Length: 13175
+...
+Accept-Ranges: bytes
+
+<!DOCTYPE html>
+<html lang="cn">
+```
+
+## Custom header key
+
+We can customize header key for auth parameters by adding the attribute configuration of the plugin under `plugin_attr` in `conf / config.yaml`.
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
+ algorithm_key: X-APISIX-HMAC-ALGORITHM
+ http_date_key: X-APISIX-DATE
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495457731
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `Date` |
Review comment:
need to update the `default` value to `0`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495427863
##########
File path: doc/zh-cn/plugins/hmac-auth.md
##########
@@ -130,7 +131,36 @@ Accept-Ranges: bytes
* 签名信息分开分别放到请求头：
```shell
-$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'X-HMAC-TIMESTAMP: TIMESTAMP' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'Date: DATE' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+HTTP/1.1 200 OK
+Content-Type: text/html
+Content-Length: 13175
+...
+Accept-Ranges: bytes
+
+<!DOCTYPE html>
+<html lang="cn">
+```
+
+
+## 自定义 header 名称
+
+我们可以在 `conf/config.yaml` 中，`plugin_attr` 下添加插件的属性配置来自定义参数 header 名称。
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
Review comment:
it's not a signature but a key for fetching signature from http header. similar to `X-GitHub-Request-Id`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494154952
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
it hasn't a err here. if params.date is empty string or other invalid string, it will return nil.
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
we allow it to be empty here
##########
File path: t/plugin/custom_hmac_auth.t
##########
@@ -210,17 +211,17 @@ X-APISIX-HMAC-ACCESS-KEY: sdf
-=== TEST 8: verify: invalid timestamp
+=== TEST 8: verify: Invalid GMT format time
--- request
GET /hello
--- more_headers
X-APISIX-HMAC-SIGNATURE: asdf
X-APISIX-HMAC-ALGORITHM: hmac-sha256
-X-APISIX-HMAC-TIMESTAMP: 112
+Date: Thu, 24 Sep 2020 06:39:52 GMT
Review comment:
not sensitive.
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
## Test Plugin
### generate signature:
-The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string`
+The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string`
1. **HTTP Method** : Refers to the GET, PUT, POST and other request methods defined in the HTTP protocol, and must be in all uppercase.
2. **HTTP URI** : `HTTP URI` requirements must start with "/", those that do not start with "/" need to be added, and the empty path is "/".
-3. **canonical_query_string** :`canonical_query_string` is the result of encoding the `query` in the URL (`query` is the string "key1 = valve1 & key2 = valve2" after the "?" in the URL).
-4. **signed_headers_string** :`signed_headers_string` is the result of obtaining the fields specified by the client from the request header and concatenating the strings in order.
+3. **HTTP Date** : Date and time string in GMT format.
Review comment:
fixed.
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] gxthrj commented on pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#issuecomment-698194182
I hope to change the default value of `clock_skew` to 0,
it is more useful when migrating, In this way, the client does not need to modify the client call mode.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis merged pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #2301:
URL: https://github.com/apache/apisix/pull/2301
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494099405
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.gmt)
+ if time then
+ local diff = abs(ngx_time() - time)
+ core.log.info("gmt diff: ", diff)
+ if diff > conf.clock_skew then
+ return nil, {message = "Invalid GMT format time"}
Review comment:
bad indentation
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -285,22 +288,22 @@ local function get_params(ctx)
local access_key = ACCESS_KEY
local signature_key = SIGNATURE_KEY
local algorithm_key = ALGORITHM_KEY
- local timestamp_key = TIMESTAMP_KEY
+ local http_date_key = HTTP_DATE_KEY
local signed_headers_key = SIGNED_HEADERS_KEY
if try_attr(local_conf, "plugin_attr", "hmac-auth") then
local attr = local_conf.plugin_attr["hmac-auth"]
access_key = attr.access_key or access_key
signature_key = attr.signature_key or signature_key
algorithm_key = attr.algorithm_key or algorithm_key
- timestamp_key = attr.timestamp_key or timestamp_key
+ http_date_key = attr.http_date_key or http_date_key
signed_headers_key = attr.signed_headers_key or signed_headers_key
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 timestamp = core.request.header(ctx, timestamp_key)
+ local gmt = core.request.header(ctx, http_date_key)
Review comment:
`gmt` is not a good name
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -218,7 +218,7 @@ local function generate_signature(ctx, secret_key, params)
local signing_string = request_method .. canonical_uri
.. canonical_query_string
- .. params.access_key .. params.timestamp
+ .. params.access_key .. params.gmt
Review comment:
I think `params.date` is 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495425245
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `HTTP Date` |
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#issuecomment-699445992
@moonming please take a review again. thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494187478
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
if the `time` is nil, I think we should throw an error message and write an error log
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495425796
##########
File path: doc/zh-cn/plugins/hmac-auth.md
##########
@@ -142,6 +143,21 @@ Accept-Ranges: bytes
```
+## 自定义 header 名称
+
+我们可以在 `conf/config.yaml` 中，`plugin_attr` 下添加插件的属性配置来自定义参数 header 名称。
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
+ algorithm_key: X-APISIX-HMAC-ALGORITHM
+ http_date_key: X-APISIX-DATE
+ access_key: X-APISIX-HMAC-ACCESS-KEY
+ signed_headers_key: X-APISIX-HMAC-SIGNED-HEADERS
+```
+
Review comment:
added.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494148437
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
The value obtained here may be empty, we need to perform error handling, right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494339665
##########
File path: t/plugin/custom_hmac_auth.t
##########
@@ -210,17 +211,17 @@ X-APISIX-HMAC-ACCESS-KEY: sdf
-=== TEST 8: verify: invalid timestamp
+=== TEST 8: verify: Invalid GMT format time
--- request
GET /hello
--- more_headers
X-APISIX-HMAC-SIGNATURE: asdf
X-APISIX-HMAC-ALGORITHM: hmac-sha256
-X-APISIX-HMAC-TIMESTAMP: 112
+Date: Thu, 24 Sep 2020 06:39:52 GMT
Review comment:
not sensitive.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494339821
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
## Test Plugin
### generate signature:
-The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string`
+The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string`
1. **HTTP Method** : Refers to the GET, PUT, POST and other request methods defined in the HTTP protocol, and must be in all uppercase.
2. **HTTP URI** : `HTTP URI` requirements must start with "/", those that do not start with "/" need to be added, and the empty path is "/".
-3. **canonical_query_string** :`canonical_query_string` is the result of encoding the `query` in the URL (`query` is the string "key1 = valve1 & key2 = valve2" after the "?" in the URL).
-4. **signed_headers_string** :`signed_headers_string` is the result of obtaining the fields specified by the client from the request header and concatenating the strings in order.
+3. **HTTP Date** : Date and time string in GMT format.
Review comment:
fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494099405
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.gmt)
+ if time then
+ local diff = abs(ngx_time() - time)
+ core.log.info("gmt diff: ", diff)
+ if diff > conf.clock_skew then
+ return nil, {message = "Invalid GMT format time"}
Review comment:
bad indentation
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -285,22 +288,22 @@ local function get_params(ctx)
local access_key = ACCESS_KEY
local signature_key = SIGNATURE_KEY
local algorithm_key = ALGORITHM_KEY
- local timestamp_key = TIMESTAMP_KEY
+ local http_date_key = HTTP_DATE_KEY
local signed_headers_key = SIGNED_HEADERS_KEY
if try_attr(local_conf, "plugin_attr", "hmac-auth") then
local attr = local_conf.plugin_attr["hmac-auth"]
access_key = attr.access_key or access_key
signature_key = attr.signature_key or signature_key
algorithm_key = attr.algorithm_key or algorithm_key
- timestamp_key = attr.timestamp_key or timestamp_key
+ http_date_key = attr.http_date_key or http_date_key
signed_headers_key = attr.signed_headers_key or signed_headers_key
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 timestamp = core.request.header(ctx, timestamp_key)
+ local gmt = core.request.header(ctx, http_date_key)
Review comment:
`gmt` is not a good name
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -218,7 +218,7 @@ local function generate_signature(ctx, secret_key, params)
local signing_string = request_method .. canonical_uri
.. canonical_query_string
- .. params.access_key .. params.timestamp
+ .. params.access_key .. params.gmt
Review comment:
I think `params.date` is better
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
if the `time` is nil, I think we should throw an error message and write an error log
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
This makes it easy to copy requests, bypassing the `clock_skew` option.
I don't think it can be empty here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494270999
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `HTTP Date` |
Review comment:
`HTTP Date` -> `Date`
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
## Test Plugin
### generate signature:
-The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string`
+The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string`
Review comment:
HTTP Date -> Date
##########
File path: t/plugin/custom_hmac_auth.t
##########
@@ -210,17 +211,17 @@ X-APISIX-HMAC-ACCESS-KEY: sdf
-=== TEST 8: verify: invalid timestamp
+=== TEST 8: verify: Invalid GMT format time
--- request
GET /hello
--- more_headers
X-APISIX-HMAC-SIGNATURE: asdf
X-APISIX-HMAC-ALGORITHM: hmac-sha256
-X-APISIX-HMAC-TIMESTAMP: 112
+Date: Thu, 24 Sep 2020 06:39:52 GMT
Review comment:
Need to test whether uppercase and lowercase letters are sensitive
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
## Test Plugin
### generate signature:
-The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string`
+The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string`
1. **HTTP Method** : Refers to the GET, PUT, POST and other request methods defined in the HTTP protocol, and must be in all uppercase.
2. **HTTP URI** : `HTTP URI` requirements must start with "/", those that do not start with "/" need to be added, and the empty path is "/".
-3. **canonical_query_string** :`canonical_query_string` is the result of encoding the `query` in the URL (`query` is the string "key1 = valve1 & key2 = valve2" after the "?" in the URL).
-4. **signed_headers_string** :`signed_headers_string` is the result of obtaining the fields specified by the client from the request header and concatenating the strings in order.
+3. **HTTP Date** : Date and time string in GMT format.
Review comment:
ditto. Date in http header
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494199196
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
we allow it to be empty here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495425998
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -128,7 +129,35 @@ Accept-Ranges: bytes
* The signature information is separately placed in the request header:
```shell
-$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'X-HMAC-TIMESTAMP: TIMESTAMP' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'Date: DATE' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+HTTP/1.1 200 OK
+Content-Type: text/html
+Content-Length: 13175
+...
+Accept-Ranges: bytes
+
+<!DOCTYPE html>
+<html lang="cn">
+```
+
+## Custom header key
+
+We can customize header key for auth parameters by adding the attribute configuration of the plugin under `plugin_attr` in `conf / config.yaml`.
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
+ algorithm_key: X-APISIX-HMAC-ALGORITHM
+ http_date_key: X-APISIX-DATE
Review comment:
date_key
##########
File path: doc/zh-cn/plugins/hmac-auth.md
##########
@@ -130,7 +131,36 @@ Accept-Ranges: bytes
* 签名信息分开分别放到请求头：
```shell
-$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'X-HMAC-TIMESTAMP: TIMESTAMP' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'Date: DATE' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+HTTP/1.1 200 OK
+Content-Type: text/html
+Content-Length: 13175
+...
+Accept-Ranges: bytes
+
+<!DOCTYPE html>
+<html lang="cn">
+```
+
+
+## 自定义 header 名称
+
+我们可以在 `conf/config.yaml` 中，`plugin_attr` 下添加插件的属性配置来自定义参数 header 名称。
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
+ algorithm_key: X-APISIX-HMAC-ALGORITHM
+ http_date_key: X-APISIX-DATE
Review comment:
ditto
##########
File path: doc/zh-cn/plugins/hmac-auth.md
##########
@@ -130,7 +131,36 @@ Accept-Ranges: bytes
* 签名信息分开分别放到请求头：
```shell
-$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'X-HMAC-TIMESTAMP: TIMESTAMP' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+$ curl http://127.0.0.1:9080/index.html -H 'X-HMAC-SIGNATURE: base64_encode(SIGNATURE)' -H 'X-HMAC-ALGORITHM: ALGORITHM' -H 'Date: DATE' -H 'X-HMAC-ACCESS-KEY: ACCESS_KEY' -H 'X-HMAC-SIGNED-HEADERS: SIGNED_HEADERS' -i
+HTTP/1.1 200 OK
+Content-Type: text/html
+Content-Length: 13175
+...
+Accept-Ranges: bytes
+
+<!DOCTYPE html>
+<html lang="cn">
+```
+
+
+## 自定义 header 名称
+
+我们可以在 `conf/config.yaml` 中，`plugin_attr` 下添加插件的属性配置来自定义参数 header 名称。
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
Review comment:
should use the real value
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494154952
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
it hasn't a err here. if params.date is empty string or other invalid string, it will return nil.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495457768
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `Date` |
Review comment:
please update the Chinese version too
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#issuecomment-699718637
@nic-chen many thx
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494270999
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `HTTP Date` |
Review comment:
`HTTP Date` -> `Date`
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
## Test Plugin
### generate signature:
-The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string`
+The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string`
Review comment:
HTTP Date -> Date
##########
File path: t/plugin/custom_hmac_auth.t
##########
@@ -210,17 +211,17 @@ X-APISIX-HMAC-ACCESS-KEY: sdf
-=== TEST 8: verify: invalid timestamp
+=== TEST 8: verify: Invalid GMT format time
--- request
GET /hello
--- more_headers
X-APISIX-HMAC-SIGNATURE: asdf
X-APISIX-HMAC-ALGORITHM: hmac-sha256
-X-APISIX-HMAC-TIMESTAMP: 112
+Date: Thu, 24 Sep 2020 06:39:52 GMT
Review comment:
Need to test whether uppercase and lowercase letters are sensitive
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13
## Test Plugin
### generate signature:
-The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string`
+The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string`
1. **HTTP Method** : Refers to the GET, PUT, POST and other request methods defined in the HTTP protocol, and must be in all uppercase.
2. **HTTP URI** : `HTTP URI` requirements must start with "/", those that do not start with "/" need to be added, and the empty path is "/".
-3. **canonical_query_string** :`canonical_query_string` is the result of encoding the `query` in the URL (`query` is the string "key1 = valve1 & key2 = valve2" after the "?" in the URL).
-4. **signed_headers_string** :`signed_headers_string` is the result of obtaining the fields specified by the client from the request header and concatenating the strings in order.
+3. **HTTP Date** : Date and time string in GMT format.
Review comment:
ditto. Date in http header
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495458410
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `Date` |
Review comment:
fixed.thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494293766
##########
File path: apisix/plugins/hmac-auth.lua
##########
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
- local diff = abs(ngx_time() - params.timestamp)
- core.log.info("timestamp diff: ", diff)
- if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+ local time = ngx.parse_http_time(params.date)
Review comment:
This makes it easy to copy requests, bypassing the `clock_skew` option.
I don't think it can be empty here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r495362521
##########
File path: doc/plugins/hmac-auth.md
##########
@@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request.
| access_key | string | required | | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. |
| secret_key | string | required | | | Use as a pair with `access_key`. |
| algorithm | string | optional | "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. |
-| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp |
+| clock_skew | integer | optional | 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `HTTP Date` |
Review comment:
missing update?
##########
File path: doc/zh-cn/plugins/hmac-auth.md
##########
@@ -142,6 +143,21 @@ Accept-Ranges: bytes
```
+## 自定义 header 名称
+
+我们可以在 `conf/config.yaml` 中，`plugin_attr` 下添加插件的属性配置来自定义参数 header 名称。
+
+```yaml
+plugin_attr:
+ hmac-auth:
+ signature_key: X-APISIX-HMAC-SIGNATURE
+ algorithm_key: X-APISIX-HMAC-ALGORITHM
+ http_date_key: X-APISIX-DATE
+ access_key: X-APISIX-HMAC-ACCESS-KEY
+ signed_headers_key: X-APISIX-HMAC-SIGNED-HEADERS
+```
+
Review comment:
After setting, how to use it, need an example.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org