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

AWS KMS support for OAuth2 Client Credentials JWT authentication #5892

Closed
prasanthu opened this issue May 3, 2023 · 11 comments
Closed

AWS KMS support for OAuth2 Client Credentials JWT authentication #5892

prasanthu opened this issue May 3, 2023 · 11 comments

Comments

@prasanthu
Copy link
Contributor

What is the underlying problem you're trying to solve?

We would like to add support for AWS KMS keys to sign the client assertions used in OAuth2 Client Credentials JWT authentication. The current implementation signs the assertions using the signing_key.private_key value specified in the configuration. KMS keys or the private portion of an asymmetric KMS key cannot be exported in plain text from the HSMs. AWS KMS only allows to sign data using the signing API. The keyID, algorithm and the message(or digest) are the only parameters required for this API. This helps in improving the security of services using OPA.

Describe the ideal solution

The signing_key configuration could support additional parameters for specifying keyId and cloud vendor. We can add support for signing using KMS in oauth2ClientCredentialsAuthPlugin here.
This will require changes in the way plugin uses jws.SignLiteral for signing assertions.

Describe a "Good Enough" solution

We could add new plugin like oauth2ClientCredentialsAuthPlugin which supports only AWS KMS.

Additional Context

Generating base64 encoded signature

When used with the ECDSA_SHA_256, ECDSA_SHA_384, or ECDSA_SHA_512 signing algorithms, the signature value is a DER-encoded object as defined by ANS X9.62–2005 and RFC 3279 Section 2.2.3.
This is the most commonly used signature format and is appropriate for most uses. We need to perform the following steps to convert the DER-encoded object to a valid signature

  1. Retrieve r & s values from the encoded structure
  2. Compute signature size based on algorithm and size of r and s values
  3. Copy r and s values after adding necessary padding to signature
  4. Convert signature bytes to base64 using URL encoding without padding
@ashutosh-narkar
Copy link
Member

Extending oauth2ClientCredentialsAuthPlugin seems reasonable if we want to support this in OPA. If this is something you'd like to contribute, feel free to do so. We're happy to help.

@prasanthu
Copy link
Contributor Author

Let me come up with some code & we can discuss more in detail.

@prasanthu
Copy link
Contributor Author

prasanthu commented May 5, 2023

Can you take a look at commit: 876745e

This is not tested code, this is how I am thinking of adding the id of the key in AMS

@ashutosh-narkar
Copy link
Member

Just glanced at the code, it looks fine. Thanks for looking into this. I did see we pull in new deps, can we do w/o it or do we need to pull all of them in?

@prasanthu
Copy link
Contributor Author

I am calling AWS KMS API to sign the digest. https://docs.aws.amazon.com/sdk-for-go/api/service/kms/#KMS.Sign
Do you suggest any other alternatives for calling AWS APIs

@ashutosh-narkar
Copy link
Member

We have some AWS related code in here. The question is the code that you are pulling in self-contained enough that we don't need to import the entire library.

@prasanthu
Copy link
Contributor Author

Sorry for the late response, was busy last week. I will check this code and see if we can use this

@prasanthu
Copy link
Contributor Author

@ashutosh-narkar I have changed the implementation to use Signed AWS request instead of pulling in AWS go libraries: d5ed3fc
Sample config.yaml: https://gist.github.com/prasanthu/16e5e668ea864487d44dd7e95ff0d34b

I have done some basic testing, it works as expected.

Pls give feedback

@ashutosh-narkar
Copy link
Member

This looks much better. Can you please open a draft PR so that we can give more feedback. Thanks for looking into this.

@stale
Copy link

stale bot commented Jun 22, 2023

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.

@stale stale bot added the inactive label Jun 22, 2023
@ashutosh-narkar
Copy link
Member

Closed via #5942.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants