lestrrat-go / jwx

Implementation of various JWx (Javascript Object Signing and Encryption/JOSE) technologies
MIT License
1.94k stars 162 forks source link

Expose Registering new JWK types #912

Closed decentralgabe closed 8 months ago

decentralgabe commented 1 year ago

Abstract I'd like to experiment with Post Quantum signature schemes using JWKs. There are a number of IETF drafts that enable this possibility:

Describe the proposed solution/change The ask would be to expose a method similar to RegisterCurve for experimental key types and signature schemes.

Additionally there would need to be support for these types in functions like this, and complementary signing code.

I would recommend enabling this as an experimental feature, similar to the build tag solution for secp256k1 support today.

Analysis Use a different library for signing with these keys/signature algorithms which would make me sad 😢

Additional context If there's a way to extend the library without forking today I missed - please let me know.

lestrrat commented 1 year ago

I briefly looked at this before, but if I understand correctly you don't need to register a JWK.

It's the signer/verifier and encrypter/decrypter that needs to be registered because the object that handles the process is triggered by looking at the algorithm name. But the jwx framework the key is just an opaque object until the signer/verifier/encrypter/decrypter sees it, at which point the algorithm may dictate a particular type of JWK

So as far as the original question in the title goes, you don't need to register a JWK type and there's already code to register algorithms (but yes, apparently we have a bug -- also see #911).


There could be some improvement in FromRaw, but I'm not immediately convinced that it's necessary to achieve what you want. At the moment I believe it's a separate issue from allowing algorithms not present in the JOSE RFCs.

decentralgabe commented 1 year ago

Thanks @lestrrat you're right as usual 😄

I put up a PR in my repo that follows the pattern from your docs -- https://github.com/TBD54566975/ssi-sdk/pull/364/commits/a5d60fbcd229c5257524dbd3eae9d568e53d4abd#diff-e90df833f7a92629e6a170157818c5ac1779d6dfe6df2c184aee39323f27c1cb

It's not working because of the bug you noted, which the fix in #911 should address.

Closing...

Edit: re: FromRaw - yes that would be nice to have native support for registered experimental key types.

lestrrat commented 1 year ago

@decentralgabe thanks, since by coincidence at least two people need the same functionality in #911 at the same time, I should just merge it :)

decentralgabe commented 1 year ago

@lestrrat should I open up a new issue about the jwk.Key interface? it's getting fairly ugly in our code to work around supporting key types supported by the interface and those not in it. It would be nice if consumers of this lib could add additional key types.

lestrrat commented 1 year ago

@decentralgabe Yes, and please provide (pseudo)code. In your case I don't understand (yet) the reason why you would need to workaround so much. I'd say code for the version that's giving you problems, and a proposed new code that makes it better would make it much easier.

decentralgabe commented 1 year ago

After thing a bit further, it's not necessary, but it does make the code a lot cleaner/simpler.

For example: converting between crypto.Public/PrivateKey is streamlined, instead of needing to write an abstraction to handle the keys that jwk.FromRaw covers and other keys.

It is probably a better design to not be too tightly coupled to this library's behavior anyway - so I'm fine with keeping things as-is.

lestrrat commented 1 year ago

@decentralgabe Not sure exactly what you are intending with a "streamlined" approach, but in general ultimately crypto and jwx are separate projects and separate code bases, so the conversion between the two will always need some sort of adapter -- therefore my immediate reaction is that it's always going to be messy going back and forth between the two. I don't know, again, I'm not sure what exactly is going on in your code.

If you indeed have a better solution I'm interested, I just need some concrete examples. Anyways, no pressure, let me know when/if you feel like you want to show me some code :)

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 7 days with no activity. This does not mean your issue is rejected, but rather it is done to hide it from the view of the maintains for the time being. Feel free to reopen if you have new comments

lestrrat commented 8 months ago

This should be doable in v3 since #1012. Closing