Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Add "kty" to jose.JSONWebKey #229

Closed
aeneasr opened this issue May 6, 2019 · 11 comments
Closed

Add "kty" to jose.JSONWebKey #229

aeneasr opened this issue May 6, 2019 · 11 comments
Labels
v3 Track issues related to v3 release
Milestone

Comments

@aeneasr
Copy link
Contributor

aeneasr commented May 6, 2019

While fields like kid or alg are added to jose.JSONWebKey, kty is not. In some cases however (e.g. when fetching JWKs from remote) it is important to have that information available. I therefore suggest to add the kty field to jose.JSONWebKey.

@csstaub
Copy link
Collaborator

csstaub commented May 6, 2019

To a certain extent, you can get this information by looking at the type of the key embedded in the struct, e.g. to see if it's *rsa.PublicKey or *ecdsa.PrivateKey etc. Would that address your particular use-case?

@aeneasr
Copy link
Contributor Author

aeneasr commented May 7, 2019

Yup, but that needs a larger type assertion with all the different Public/Private types - wouldn't it be easier to just add the kty field which is available per default?

@csstaub
Copy link
Collaborator

csstaub commented May 7, 2019

I'd be happy to expose kty also, just wanted to note an alternate solution you could use in the interim. I don't have a lot of time on my hands right now but if you send me a pull request I can merge it in and release.

@aeneasr
Copy link
Contributor Author

aeneasr commented May 13, 2019

Same here, I'll try to look into it however!

@csstaub csstaub added the v3 Track issues related to v3 release label May 24, 2019
@mbyczkowski mbyczkowski added this to the V3 Release milestone May 24, 2019
@csstaub
Copy link
Collaborator

csstaub commented May 28, 2019

Looking into this, we set the kty from the type fo the key when marshalling into JSON. If we expose the key type directly, it would be possible to set up a JSONWebKey struct where the type of key and the string in kty conflict. What should we do then?

@mogthesprog
Copy link

So, i think i'm actually running into a bug because of this.

We're using the AzureAD support in go golang.org/x/oauth2, and we use jose.v2 to check signatures on our JWTs. With Azure AD, the alg field is omitted from the JWKs. The kty field is present.

According to the RFC, the alg field is actually optional, and only kty is compulsory. However alg is a compulsory header in JWTs, looking at the JWT RFC.

Instead of relying on the existence of the alg header in the JWK, could we check the JWT headers for an alg and use that in JSONWebToken.Claims()?

What's happening now is, the Azure AD JWKs omit the alg header, and the signature verification failed with

square/go-jose: error in cryptographic primitive

Or maybe i'm way off, and i've got things a bit screwed up on my end.

@csstaub
Copy link
Collaborator

csstaub commented Aug 9, 2019

Do you have an example token/code I could take a look at? alg is mandatory in a JWT, but not in a JWK. The absence of alg in the JWK should not affect how go-jose handles the token. Looking at the alg header from the JWK to decide how to verify the JWT wouldn't work.

@mogthesprog
Copy link

Sure. I've got some now revoked tokens, and the JWKs that should work but they have some some company IDs in them. Do you have an e-mail address I can send them to?

@csstaub
Copy link
Collaborator

csstaub commented Aug 9, 2019

Yeah, you can email them to css (at) css.bio. An excerpt of the code you're using to verify etc. would also help me understand what you're trying to do.

@cgarvis
Copy link

cgarvis commented Oct 8, 2019

I may be running into this. Have you been able to replicate?

@zamicol
Copy link
Contributor

zamicol commented Nov 11, 2019

Could we add key_ops too?

As far as I've looked, that is also the only other missing parameter. (See the registry under the "JSON Web Key Parameters" heading)

@cpu cpu closed this as completed Jan 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
v3 Track issues related to v3 release
Projects
None yet
Development

No branches or pull requests

7 participants