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 12:27:07 UTC

[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`

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