feat(hmac-auth): add support for RSA signatures #11133

locao · 2023-06-27T19:00:46Z

Summary

The hmac-auth plugin allow authentication with HMAC signatures based on the draft-cavage-http-signatures draft.
This commit aims to add support for RSA signatures as described in the draft, providing a stronger layer
of security via asymmetric encryption.

Co-authored-by: Jérémy Quilleré [email protected]

Note: the feature was coded by @mideuger in #8530, this PR adds cluster compatibility support.

Checklist

The Pull Request has tests
There's an entry in the CHANGELOG
There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Full changelog

Add possible values to algorithms (rsa-sha256 and rsa-sha512)
Add a new field to this plugin's credential (public_key)
Add tests showing failed and succeeded authentications using rsa algorithms

How to test

First, create a RSA key pair :

openssl genrsa -out private_key.pem
openssl rsa -in private_key.pem -pubout -out public_key.pem

Then, enable the plugin, create a consumer and a corresponding credential with the public key :

curl -X POST http://localhost:8001/plugins \
    --form "name=hmac-auth"  \
    --form "config.algorithms=rsa-sha256" \
    --form "config.algorithms=rsa-sha512"

curl -X POST http://localhost:8001/consumers \
    --form "username=alice"

curl -X POST http://localhost:8001/consumers/alice/hmac-auth \
    --form "username=alice" \
    --form "public_key=@public_key.pem"

Finally, make a signed request :

export DATE="date: $(echo -n $(TZ=GMT date '+%a, %d %b %Y %T %Z'))"
export SIGNATURE=$(printf %s "${DATE}" | openssl dgst -binary -sha512 -sign private_key.pem | openssl base64 -A)
export AUTHORIZATION='authorization: username="alice", algorithm="rsa-sha512", headers="date", signature="'${SIGNATURE}'"'

curl -X GET http://localhost:8000 \
    --header "${DATE}" \
    --header "${AUTHORIZATION}"

Possible improvements

Here are some improvements that we might want to implement after this one :

Check public key validity during credential creation/update
Rebrand the plugin to HTTP Signature
Use Signature header to provide the signature (or let it be configurable)
Implement keyId from the draft

Issue reference

KAG-1934
Closes: #8530

kikito · 2023-08-22T16:40:35Z

@bungle please give this a review

catbro666 · 2023-09-07T03:25:25Z

kong/plugins/hmac-auth/access.lua

@@ -97,7 +125,7 @@ local function retrieve_hmac_fields(authorization_header)
  -- parse the header to retrieve hamc parameters
  if authorization_header then
    local iterator, iter_err = re_gmatch(authorization_header,
-                                         "\\s*[Hh]mac\\s*username=\"(.+)\"," ..
+                                         "(\\s*[Hh]mac)?\\s*username=\"(.+)\"," ..


Do we need to specify rsa here when using rsa signature for consistency?

catbro666 · 2023-09-07T03:34:10Z

kong/plugins/hmac-auth/access.lua

@@ -31,18 +32,45 @@ local SIGNATURE_NOT_VALID = "HMAC signature cannot be verified"
 local SIGNATURE_NOT_SAME = "HMAC signature does not match"


error message needs to be changed accordingly

The specific string format for the "Authorization" header that includes the method (e.g., "Hmac" or "Rsa") and the various components (e.g., "username," "algorithm," "headers," and "signature") may not be standardized in the HTTP Signatures draft (draft-cavage-http-signatures). The draft defines the concept of HTTP signatures and how they can be generated and verified, but it doesn't prescribe a specific format for the "Authorization" header.

The format you are using, which includes "Hmac" or "Rsa" as the method prefix, is a common convention used in some implementations for clarity and to distinguish between different authentication methods (HMAC and RSA, in this case). However, this specific format might not be explicitly detailed in the draft itself.

agree. need to output the right error message

catbro666 · 2023-09-07T03:42:00Z

Strictly speaking, rsa doesn't belong to hmac. The scope of the plugin name feels a little small now. Maybe something like "sign-auth" would have been better

chobits · 2023-09-23T14:02:35Z

hi @locao I'm helping to resolve this conflict, recently I have also reviewed original community PRs. Hans and me will be helping to move the progress of this communit PR into our master.

fffonion · 2023-09-25T07:18:58Z

kong/plugins/hmac-auth/daos.lua

@@ -17,6 +17,7 @@ return {
      { consumer = { type = "foreign", reference = "consumers", required = true, on_delete = "cascade", }, },
      { username = { type = "string", required = true, unique = true }, },
      { secret = { type = "string", auto = true }, },
+      { public_key = { type = "string" }, },


how about typedefs.key { required = false, referenceable = true, encrypted = true }, }, ?

Suggested change

{ public_key = { type = "string" }, },

{ public_key = typedefs.key {

description = "The RSA public key used to validate signature.",

type = "string",

encrypted = true, -- Kong Enterprise-exclusive feature, does nothing in Kong CE

referenceable = true,

}, },

An additional validator to validate it's a RSA key (not EC or ECX keys) would be good.

The hmac-auth plugin allow authentication with HMAC signatures based on the draft-cavage-http-signatures draft. This commit aims to add support for RSA signatures as described in the draft, providing a stronger layer of security via asymmetric encryption. This implementation has been made with backward compatibility in mind and only one new field has been added to the DAOs to store the RSA public key. Depending on the algorithm used during the request, the plugin will use either the HMAC secret or the RSA public key to verify the signature.

GGabriele · 2023-10-10T13:55:07Z

kong/plugins/hmac-auth/schema.lua

@@ -6,6 +6,8 @@ local ALGORITHMS = {
  "hmac-sha256",
  "hmac-sha384",
  "hmac-sha512",
+  "rsa-sha256",
+  "rsa-sha512",


Should we add these new options in the description of the algorithms field? (line 30)

nowNick

Just one concern regarding the if statement. Other than that it looks good 👍

nowNick · 2023-11-02T13:51:34Z

kong/plugins/hmac-auth/access.lua

@@ -113,10 +141,10 @@ local function retrieve_hmac_fields(authorization_header)
    end

    if m and #m >= 4 then


Shouldn't we increment the check in the if as well?

Suggested change

if m and #m >= 4 then

if m and #m >= 5 then

nowNick · 2023-11-02T14:20:24Z

spec/03-plugins/19-hmac-auth/03-access_spec.lua

+            ["proxy-authorization"] = hmacAuth,
+          },
+        })
+        assert.res_status(401, res)


Depending on which PR goes it first - we need to either change it here or there (#11791) but in the end we need to make sure we return proper WWW-Authenticate header on 401 response

bungle · 2023-12-07T15:53:44Z

Looks like this is the latest draft:
https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures

So in theory at least I would rather move this plugin in direction of this:
https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures#iana-hsa-contents

That lists there:

rsa-pss-sha512
rsa-v1_5-sha256
hmac-sha256
ecdsa-p256-sha256
ecdsa-p384-sha384
ed25519

And perhaps later:
https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-message-signatures#name-json-web-signature-jws-algo

AndyZhang0707 · 2024-08-20T04:01:04Z

3.8 FF is done, so I don't think we will ship this feature in 3.8. Removing it from the 3.8.0 milestone. cc: @locao

pull-request-size bot added the size/L label Jun 27, 2023

github-actions bot added core/clustering plugins/hmac-auth schema-change-noteworthy labels Jun 27, 2023

github-actions bot assigned locao Jun 27, 2023

locao marked this pull request as draft June 27, 2023 19:29

locao force-pushed the feat/rsa-http-signature branch from f404e20 to 46ae28a Compare June 27, 2023 21:37

github-actions bot added the changelog label Jun 27, 2023

locao marked this pull request as ready for review June 28, 2023 11:36

locao force-pushed the feat/rsa-http-signature branch from 46ae28a to ab0a2bc Compare June 30, 2023 20:24

kikito requested a review from bungle August 15, 2023 08:43

This comment was marked as resolved.

Sign in to view

catbro666 reviewed Sep 7, 2023

View reviewed changes

chobits mentioned this pull request Sep 19, 2023

feat(hmac-auth) add support for RSA signatures #8530

Closed

chobits force-pushed the feat/rsa-http-signature branch from ab0a2bc to e048cfa Compare September 23, 2023 13:59

github-actions bot removed the changelog label Sep 23, 2023

chobits force-pushed the feat/rsa-http-signature branch from 18ca824 to f5ac767 Compare September 24, 2023 03:17

hanshuebner mentioned this pull request Sep 25, 2023

feat(hmac-auth) enforce unique consumer #8542

Closed

fffonion reviewed Sep 25, 2023

View reviewed changes

mideuger and others added 7 commits September 25, 2023 15:41

chore(plugins/hmac-auth): renamed migrations file

b5db139

tests(unit): check public_key field cluster compatibility

49ce30a

feat(clustering): public_key field compatibility

d309fff

tests(migration): check the new public_key column

79f8e15

add unreleased changelog entry

ea5c7bd

chore(plugins/hmac-auth): add public_key into removed_fields

54edb06

chore(plugins/hmac-auth): fix public_key overflow in removed_fields

f52df68

chobits force-pushed the feat/rsa-http-signature branch from 8f737a8 to f52df68 Compare September 25, 2023 07:41

team-eng-enablement added author/community PRs from the open-source community (not Kong Inc) and removed author/community PRs from the open-source community (not Kong Inc) labels Sep 26, 2023

GGabriele reviewed Oct 10, 2023

View reviewed changes

AndyZhang0707 modified the milestones: 3.5.0, 3.6.0 Oct 19, 2023

kikito requested review from nowNick and gszr October 31, 2023 09:45

nowNick reviewed Nov 2, 2023

View reviewed changes

kikito modified the milestones: 3.6.0, 3.7.0 Jan 22, 2024

locao marked this pull request as draft February 20, 2024 18:12

kikito modified the milestones: 3.7.0, 3.8.0 Apr 23, 2024

AndyZhang0707 removed this from the 3.8.0 milestone Aug 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(hmac-auth): add support for RSA signatures #11133

feat(hmac-auth): add support for RSA signatures #11133

locao commented Jun 27, 2023 •

edited

Loading

kikito commented Aug 22, 2023

This comment was marked as resolved.

catbro666 Sep 7, 2023

catbro666 Sep 7, 2023

chobits Sep 25, 2023

chobits Sep 25, 2023

catbro666 commented Sep 7, 2023 •

edited

Loading

chobits commented Sep 23, 2023

fffonion Sep 25, 2023

GGabriele Oct 10, 2023

nowNick left a comment

nowNick Nov 2, 2023

nowNick Nov 2, 2023

bungle commented Dec 7, 2023 •

edited

Loading

AndyZhang0707 commented Aug 20, 2024

		@@ -31,18 +32,45 @@ local SIGNATURE_NOT_VALID = "HMAC signature cannot be verified"
		local SIGNATURE_NOT_SAME = "HMAC signature does not match"

-      { public_key = { type = "string" }, },
+      { public_key = typedefs.key {
+          description = "The RSA public key used to validate signature.",
+          type = "string",
+          encrypted = true, -- Kong Enterprise-exclusive feature, does nothing in Kong CE
+          referenceable = true,
+      }, },

		@@ -113,10 +141,10 @@ local function retrieve_hmac_fields(authorization_header)
		end

		if m and #m >= 4 then

feat(hmac-auth): add support for RSA signatures #11133

Are you sure you want to change the base?

feat(hmac-auth): add support for RSA signatures #11133

Conversation

locao commented Jun 27, 2023 • edited Loading

Summary

Checklist

Full changelog

How to test

Possible improvements

Issue reference

kikito commented Aug 22, 2023

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catbro666 commented Sep 7, 2023 • edited Loading

chobits commented Sep 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nowNick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bungle commented Dec 7, 2023 • edited Loading

AndyZhang0707 commented Aug 20, 2024

locao commented Jun 27, 2023 •

edited

Loading

catbro666 commented Sep 7, 2023 •

edited

Loading

bungle commented Dec 7, 2023 •

edited

Loading