Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

mideuger
Copy link

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.

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

@CLAassistant
Copy link

CLAassistant commented Mar 10, 2022

CLA assistant check
All committers have signed the CLA.

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.
@mideuger mideuger force-pushed the feat/rsa-http-signature branch from deab67e to d7a471d Compare March 28, 2022 10:52
@mideuger
Copy link
Author

The branch has been rebased following 88abdb8, which created some conflicts.
That allowed to slightly improve the implementation of the validate_signature function 🙂

@@ -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" }, },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change for version compatibility with older data planes; updates will need to be made to the compatibility layer to handle these changes ahead of time of before a release.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to understand what needs to be updated. You linked to a file named removed_fields.lua but no field has been removed, only added. I've checked in the folder if there was a file named added_fields.lua but could not find any. I've also checked other PR doing similar changes on the configuration and could not find any modification related to the compatibility layer.

Could you provide more informations on what needs to be done exactly ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mideuger This is something that will need to be handled closer to release time when this feature is targeting a particular version of Kong Gateway; I just wanted to mark this PR as something that is going to need further testing and updates when it comes to release time.

I've also checked other PR doing similar changes on the configuration and could not find any modification related to the compatibility layer.

Since you are adding a new field to the hmac-auth plugin, older data planes will not know how to process this field when it is transmitted in the payload in hybrid mode. To allow for newer control planes to interoperate with older data planes the field public_key needs to be removed before the payload is sent down.

Copy link
Contributor

@chobits chobits Sep 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @mideuger

The comment of @mikefero means that you need to add your new field "public_key" into this file removed_fields.lua, here is an example of how to add a new field to plugin conf and make it compatitble with old version: #11523

@mikefero mikefero added the version-compatibility Incompatibility with older data plane versions label Apr 7, 2022
@guanlan guanlan requested a review from a team as a August 25, 2022 01:38
@hbagdi hbagdi requested review from jschmid1 and mayocream and removed request for a team December 21, 2022 14:53
@hbagdi hbagdi added the author/community PRs from the open-source community (not Kong Inc) label Mar 30, 2023
@locao locao self-assigned this May 23, 2023
@hanshuebner hanshuebner assigned raoxiaoyan and unassigned mideuger and locao Sep 11, 2023
@chobits chobits assigned chobits and unassigned raoxiaoyan Sep 17, 2023

]],
},
cassandra = {
Copy link
Contributor

@chobits chobits Sep 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, cassandra is not supported at present, it might be removed from this commit. Otherwise it will report error:

with kong 3.4+ or kong latest master branch


(kong-dev) xc kong-ee $ kong start
Error: [PostgreSQL error] [PostgreSQL error] migration 'kong.plugins.hmac-auth.migrations.004_add_public_key
' of 'hmac-auth' subsystem is invalid: schema violation (cassandra: unknown field)

  Run with --v (verbose) or --vv (debug) for more details
(kong-dev) xc kong-ee $ kong migrations bootstrap
Error: [PostgreSQL error] [PostgreSQL error] migration 'kong.plugins.hmac-auth.migrations.004_add_public_key
' of 'hmac-auth' subsystem is invalid: schema violation (cassandra: unknown field)

  Run with --v (verbose) or --vv (debug) for more details

@chobits chobits removed the request for review from mayocream September 17, 2023 09:34
@chobits
Copy link
Contributor

chobits commented Sep 17, 2023

  • ✅ I've examined other parts of the source code and believe it works correctly.
    • I mannually fix the 'kong/plugins/hmac-auth/access.lua.rej' and test it ok with the simple scripts from this pr's first comment.

TODO

  • Test case: with some efforts to resolve the rejections in the test case, I'm unable to get the test case to pass. Maybe there are some modifications of the hmac-auth test cases since this commit.
  • unnecessary to support cassandra in this PR, as mentioned in this comment
  • need to compatitble with old version kong as mentioned in this comment.
  • need to rebase on the master, there are some conflicts
    • 1 out of 7 hunks failed--saving rejects to 'kong/plugins/hmac-auth/access.lua.rej'
    • 1 out of 1 hunks failed--saving rejects to 'spec/03-plugins/19-hmac-auth/01-schema_spec.lua.rej'
    • 1 out of 5 hunks failed--saving rejects to 'spec/03-plugins/19-hmac-auth/03-access_spec.lua.rej'

@hanshuebner hanshuebner added the pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... label Sep 18, 2023
@chobits
Copy link
Contributor

chobits commented Sep 19, 2023

I found that we have an updated pr here: #11133 @hanshuebner
so I'll try to review that pr. not opening a new one

@hanshuebner
Copy link
Contributor

Closing in favor of #11133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author/community PRs from the open-source community (not Kong Inc) pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... plugins/hmac-auth size/L version-compatibility Incompatibility with older data plane versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants