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(api)!: Make SignKM accept generic crypto.Signer #392

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

werwurm
Copy link

@werwurm werwurm commented Sep 6, 2024

Accepting crypto.PrivateKey uneccessarily restricts this library to software crypto algorithms provided by the golang crypto packages. By allowing the more generic crypto.Signer interface alternative implementations, e.g., backed by HSMs, can be supported.

@werwurm
Copy link
Author

werwurm commented Sep 6, 2024

I realize that this changes the API. It could be hidden by adding new API, e.g., SignKMGeneric. Let me know what you think.

Also, to really make use of this change, the fiano back end needs to be changed. Which I have proposed here [1] [2]

[1] linuxboot/fiano#421
[2] linuxboot/fiano#420

Copy link
Collaborator

@walterchris walterchris left a comment

Choose a reason for hiding this comment

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

I think changing the API here is fine.

@walterchris
Copy link
Collaborator

Could you satisfy the linter? :)

@werwurm werwurm changed the title Make SignKM accept generic crypto.Signer feat(api!: Make SignKM accept generic crypto.Signer Sep 9, 2024
@werwurm werwurm changed the title feat(api!: Make SignKM accept generic crypto.Signer feat(api)!: Make SignKM accept generic crypto.Signer Sep 9, 2024
Accepting crypto.PrivateKey uneccessarily restricts this library to
software crypto algorithms provided by the golang crypto packages. By
allowing the more generic crypto.Signer interface alternative
implementations, e.g., backed by HSMs, can be supported.

Signed-off-by: Janis Danisevskis <jdanisevskis@aurora.tech>
@werwurm
Copy link
Author

werwurm commented Sep 9, 2024

Could you satisfy the linter? :)

Sure thing. I hope this fixes it. commitlint is new to me. So bare with me, please.
Is the final commit message constructed from the pull request or from the commit on the feature branch?

@walterchris
Copy link
Collaborator

We will rebase and cherry pick the commits - so no squash here :)

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

Successfully merging this pull request may close these issues.

2 participants