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

JWK Parameters #518

Closed
bellebaum opened this issue Oct 6, 2022 · 6 comments
Closed

JWK Parameters #518

bellebaum opened this issue Oct 6, 2022 · 6 comments

Comments

@bellebaum
Copy link
Contributor

Hey there :)

I was trying to use this gem, and the JWK implementation in particular, to handle any kind of cryptographic material for which I would normally go with several files in a format like PEM. This includes public and private key pairs, as well as certificates.
In principle, the JWK specs are built to handle all this kind of stuff, as can be seen e.g. in the IANA registry.
Note in particular the x5* claims for certificates.

It would be nice if there were some option to include these parameters in JWT::JWK and have it be exported alongside the actual key values. Currently this is not the case, because most values (excluding kid and kty) are stored internally in an OpenSSL-compatible format (which is a good thing, I think, since cryptographic operations are often a lot more time-critical than serialization).

I do recognize that supporting the full set of semantics connected to these additional parameters may be a massive increase in scope for this project (e.g. any certificates in x5c must be signed by the next one in the array (let's not start talking about conformance to all X.509v3 extensions limiting who can sign what) with the first certificate referencing the very key in the JWK, and the use parameter should probably also be checked at some point or another).
A much more reasonable approach, if properly documented, would be to just allow people to add custom properties to the JWK. This is in a way similar to how this gem supports adding arbitrary claims to a JWT, but only actively provides guidance and validation for some of them (e.g. iss, aud, etc.).

The way I would envision this is by using the options argument in KeyBase and just saving it after removing key_generator (note that kid is actually one of the custom parameters, and can be saved among them :) ).
When the JWK is later exported, any keys in the saved options are merged into the result.
One step up from this would be a method to access and manipulate these options (maybe even as usability-friendly my_jwk['use']).

I would be happy to hear some feedback on this idea and whether this endeavor and whether it may be beneficial to the project.

@anakinj
Copy link
Member

anakinj commented Oct 6, 2022

Hi, great to hear from you again :)

You're right, the JWK implementation is pretty much based on the JWT needs currently. Pretty soon after implementing the support it started to feel that the JWK parts deserved it's own library, also now knowing a lot more about the context the solution would probably look a lot different :)

About using the options parameter to inject arbitrary/other supported values into the set makes sense.

What about usage being something like

jwk = ::JWT::JWK::RSA.new(OpenSSL::PKey::RSA.new(2048), attrs: { kid: 'custom_kid', use:'enc' }, kid_generator: nil)
jwk.export
=> { :kty =>"RSA",                                                          
 :n => "xxxx",                                                            
 :e =>"AQAB",                                                           
 :kid =>"custom_kid",
 :use => 'enc' }

Also when reading a JWK the values not directly supported by the internals could be stored in the same place these attrs would. So importing a key with "unkowns" would preserve the extra attributes.

Even better would probably be a implementation that would support the specific attributes a JWK is supposed to support, otherwise changing behaviour to a more specific way later will be impossible without breaking the API.

@anakinj
Copy link
Member

anakinj commented Oct 6, 2022

Also the key accessor feels like a natural way to access JWK attributes. 👍 for that. Not sure if mutating the JWK via those is going to be confusing if you cannot change everything via that, what would happen if one would do my_jwk[:n] = 'new_n_value'

@bellebaum
Copy link
Contributor Author

Hey @anakinj, also nice to hear from you again :)

jwk = ::JWT::JWK::RSA.new(OpenSSL::PKey::RSA.new(2048), attrs: { kid: 'custom_kid', use:'enc' }, kid_generator: nil)

I guess collecting all additional attributes under a key attrs makes sense for future extensions. I like it!
Note that you would probably still keep kid as a key outside attrs for backwards compatibility.
This leads to the natural question of which one should take priority if both are specified. I would argue that the kid outside attrs should take priority, since attrs may be coming directly from another source of information and kid feels like an explicit indication by the application programmer for the value. But then, I could be wrong about people would use this.

Also when reading a JWK the values not directly supported by the internals could be stored in the same place these attrs would. So importing a key with "unkowns" would preserve the extra attributes.

Great idea 👍
The details may be fiddly though. Consider oth, which is clearly part of the RSA key and not metadata. Treating this as metadata may result in a completely different key.
But I guess this is a limitation of the current implementation as well.
If it did not come up as a problem until now, I guess not many people are using RSA with more than two primes.

Even better would probably be a implementation that would support the specific attributes a JWK is supposed to support, otherwise changing behaviour to a more specific way later will be impossible without breaking the API.

I have not looked much into the history of this gem, but options like verify_iss feel like they were put in place to not break backwards compatibility. Maybe then one could apply something similar to specific functionality added later on. It would maybe have to look a bit different though to support both the claim has to be present and valid and the claim has to be valid if present, if there are use cases for either one.

Not sure if mutating the JWK via those is going to be confusing if you cannot change everything via that, what would happen if one would do my_jwk[:n] = 'new_n_value'

As a user, I would expect JWK to behave like a "hash with benefits".
Conceptually, it has two representations: The internal representation, which is ready for cryptographic operations, and the outer representation, which you get by exporting and which is (in its JSON form) defined in RFC 7517.
If you were to use the key accessor, it should feel like changing the outer representation, with the internal representation (however it may look like) reflecting the change.
Maybe a (not yet efficient) general algorithm would be:

  1. export the JWK
  2. apply the change to the resulting hash (writing) or return the hash key's value (reading)
  3. re-import the hash into a new JWK
  4. copy all internal representations from the new JWK to the existing one

Of course, there are loads of optimizations here.

  • You do not actually need to export and reimport the JWK if the requested key is not kty specific (just act on the saved attrs instead)
  • For read access, you do not need to re-import anything
  • Maybe you can even speed up writing to kty-specific values

Note however that it may not be possible to construct a valid internal representation at all after a single change.
E.g. if I execute my_jwk[:kty] = 'EC' on an RSA JWK, the result would have no valid internal representation.
This means that one can not always gradually transform one key into another (if one wanted to do this for whatever reason).

So maybe this isn't a solution after all.
I think the simplest and most consistent way would be to throw an error when trying to overwrite kty specific values and document this. If application programmers really know what they are doing, they also know the JWK format and can execute the above algorithm themselves.

Alternatively one could allow programmers to access the saved attrs and alter it directly.
There should be a warning though that any kty specific values will eventually be overwritten during export.

@anakinj
Copy link
Member

anakinj commented Oct 7, 2022

Good points.

Are you interested in taking a shot on implementing something like this? Also open for pairing if you're into that :)

Maybe starting with the attrs parts and excluding the hard parts as changing the key type or key parameters. Also the IANA list you provided is a pretty good reference for some validation on what can be combined with what.

@bellebaum
Copy link
Contributor Author

I can send a PR your way probably on monday and mark you for a review. Maybe this is a helpful starting point for further discussions :)

@anakinj
Copy link
Member

anakinj commented Oct 7, 2022

Lovely. Looking forward for that.

@bellebaum bellebaum mentioned this issue Oct 7, 2022
3 tasks
@anakinj anakinj closed this as completed Dec 22, 2022
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

No branches or pull requests

2 participants