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: introduce AWS KMS types.MessageTypeRaw for AWS KMS signing operations #573

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexatcanva
Copy link

Hi smallstep/crypto reviewers! 👋

This is my first contributing PR, please feel free to provide any feedback! I'm happy to change things up where required!

Name of feature:

Implement a means for callers to specify types.MessageTypeRaw in AWS KMS Sign operations.

Pain or issue this feature alleviates:

Currently, the awskms.Sign operation exclusively supports the signing of messages, where the message is expected to be a types.MessageTypeDigest.

This limitation prevents callers from being able to specify that their message is un-hashed, and thus unable to inform the AWS KMS API that the Sign operation should have a types.MessageType of types.MessageTypeRaw (RAW).

Limitations

The objective of this package, (and the other Signing packages) is to provide an implementation of the crypto.Signer interface that performs the relevant cloud based signing operations.

As a part of the crypto.Signer interface, the crypto.SignerOpts interface is include as the third and final argument to the Sign() method.

A perk of the crypto.SignerOpts interface is the promise that:

// HashFunc returns an identifier for the hash function used to produce
// the message passed to Signer.Sign, or else zero to indicate that no
// hashing was done.
HashFunc() Hash

However, within the awskms implementation of the crypto.Signer interface, this promise cannot be fulfilled, as the AWS KMS Sign operation requires a types.SigningAlgorithmSpec must be provided when performing an AWS KMS Signing operation.

AWS KMS Go SDK v2 Documentation:

// Specifies the signing algorithm to use when signing the message.
//
// Choose an algorithm that is compatible with the type and size of the specified
// asymmetric KMS key. When signing with RSA key pairs, RSASSA-PSS algorithms are
// preferred. We include RSASSA-PKCS1-v1_5 algorithms for compatibility with
// existing applications.
//
// This member is required.
SigningAlgorithm types.SigningAlgorithmSpec

The only exclusion to this limitation is when signing with ECDSA keys. As AWS does not enable the user to specify arbirtary signing algorithms, but rather the specified signing algorithm for that ECDSA key type. See The ECDSA AWS Key Spec

As such, when calling the awskms Sign() we must always provide a non-zero crypto.Hash as our crypto.SignerOpts argument.

Mitigation

To mitigate this limitation, a new AWSOptions type is introduced to the awskms package.

This AWSOptions type implements the crypto.SignerOpts interface, and allows callers to specify whether or not the message is RAW.

Additionally, it posses a crypto.SignerOpts field Options where callers can provide the underlying crypto.Hash that can be matched with the appropriate types.SigningAlgorithmSpec that is expected by the AWS KMS Sign API.

Approach

I have opted to use a more generic AWSOptions struct with the Raw field as a boolean to indicate if the types.MessageType is types.MessageTypeRaw or not. I did consider using a struct such as AWSRawMessageOptions and removing the field (since the only logical difference is the truthy value of the Raw field.) However I decided against it such that we open ourselves up to adding or removing additional options in the future.

Why is this important to the project (if not answered above):

Allow callers to have the additional flexibility of using both types.MessageTypeRaw and types.MessageTypeDigest when using the awskms Signer.

Is there documentation on how to use this feature? If so, where?

GoDoc comments on the awskms.AWSOptions struct has been introduced.

In what environments or workflows is this feature supported?

AWS Environments that leverage AWS KMS Sign operations.

In what environments or workflows is this feature explicitly NOT supported (if any)?

Non-AWS Environments.

Supporting links/other PRs/issues:

N/a

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@alexatcanva
Copy link
Author

alexatcanva commented Aug 26, 2024

Hi @hslatman, or @maraino, by the looks of the contributors insights you might be an appropriate reviewer(s) of this PR, please let me know if you have any feedback regarding the change 🙏

@maraino
Copy link
Contributor

maraino commented Aug 26, 2024

I think the approach looks good. We will look at it tomorrow and assign a reviewer.

@hslatman hslatman self-assigned this Aug 27, 2024
@hslatman
Copy link
Member

hslatman commented Oct 3, 2024

@alexatcanva thank you for your contribution. Can you (re)check the CLA? Let me know if it's not an option for you.

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

Successfully merging this pull request may close these issues.

4 participants