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

Add AllPrimitives and AllPrimitivesWithKeyManager #16

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

Conversation

theory
Copy link

@theory theory commented Mar 7, 2024

These new methods on the keyset handle complement Primitives and PrimitivesWithKeyManager, but return all primitives, not just enabled primitives. This can be useful for key implementations to display information about key parameters, but should not be used to create sets for encryption duties, since only enabled keys should be used for encryption.

Done by adding a filter function to the private primitives method, and having Primitives and PrimitivesWithKeyManager use it to filter for enabled keys, while AllPrimitives and AllPrimitivesWithKeyManager don't filter. This keeps the code changes to a minimum. Also: add test to ensure that disabled keys are included in the All methods but excluded in the non-All methods.

Requires disabling the check for enabled keys in primitiveset; should probably change it to allow the adding of other keys through a separate method or something, but I wanted to get some feedback and discussion going before I dove into that, so simply commenting out that check (and the corresponding test) for now.

See discussion in #14 for some of the background on this proposal.

These new methods on the keyset handle complement `Primitives` and
`PrimitivesWithKeyManager`, but return all primitives, not just enabled
primitives. This can be useful for key implementations to display
information about key parameters, but should not be used to create sets
for encryption duties, since only enabled keys should be used for
encryption.

Done by adding a filter function to the private `primitives` method, and
having `Primitives` and `PrimitivesWithKeyManager` use it to filter for
enabled keys, while `AllPrimitives` and `AllPrimitivesWithKeyManager`
don't filter. This keeps the code changes to a minimum. Also: add test
to ensure that disabled keys are included in the `All` methods but
excluded in the non-`All` methods.

Requires disabling the check for enabled keys in `primitiveset`; should
probably change it to allow the adding of other keys through a separate
method or something, but I wanted to get some feedback and discussion
going before I dove into that, so simply commenting out that check (and
the corresponding test) for now.
@juergw
Copy link
Contributor

juergw commented Apr 8, 2024

I don't think we want to add this. Sorry.

We do have plans to extend the API of keyset.Handle, similar to what is already possible in Java, see:
https://javadoc.io/static/com.google.crypto.tink/tink/1.13.0/com/google/crypto/tink/KeysetHandle.html

But since this is a bigger redesign, I don't think we will accept pull requests for this until the majority of the re-work is done.

@theory
Copy link
Author

theory commented Apr 8, 2024

I don't think we want to add this. Sorry.

Okay, I'll keep my ugly hack for now, then.

We do have plans to extend the API of keyset.Handle, similar to what is already possible in Java, see: https://javadoc.io/static/com.google.crypto.tink/tink/1.13.0/com/google/crypto/tink/KeysetHandle.html

Do you mean something like getAt, assuming that it returns keys regardless of their status? I think that would be fine for my use case.

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.

None yet

2 participants